Deferred out of #531 / PR #749, deliberately and with the reasoning written down rather than folded in: the remedy is a seam change, and the exposure that remains is narrower than the one #749 closes.
What #749 shipped, so the residual is stated against the right baseline
ctl_window_id (src/bmad_loop/tui/launch.py) used to admit an untagged control-session row whenever this project held a run dir for the run id. #749 replaced that with the record this project's own launch wrote: an untagged row is a candidate only when win_id == recorded. That is a positive claim — this project wrote it, about the window it minted — where the run dir was a coincidence of a caller-supplied id.
The mechanism
The record names a window id, and a window id is a reusable handle rather than a durable identity. So the record proves the mint, not the window:
- This project mints
@7 for run RID and records @7.
- That window later dies (killed, pruned) and the backend frees
@7.
- The backend reuses the freed id and hands
@7 to a new window.
- That new window belongs to a neighbouring project, carries the same scripted
RID in its <kind>-<run_id> name, and its own tag write failed, so it is untagged.
The listing then shows @7 / run-RID / <untagged>, this project's record still says @7, win_id == recorded passes, and a, set_return_pane and x all resolve to the neighbour's window again.
All four conditions must hold at once, which is why this is a conjunction rather than a case. Condition 3 is the load-bearing one: tmux does not recycle @N within one server's life, so on tmux this is reached across a server restart. Whether psmux recycles window ids has not been measured — that is the first thing to establish, and it decides how reachable this actually is.
A second route to the same admission: the record is forgeable
Raised by CodeRabbit on PR #749 (CWE-284) and folded in here, because the remedy is the same one.
The conjunction above needs the backend to reuse an id accidentally. It does not have to be accidental. The record lives at <project>/.bmad-loop/runs/<run_id>/ctl-window, under the project root every coding session can write — _read_ctl_window's own docstring says so. A session that writes a record naming an untagged window it never minted gets that window admitted here, and x resolves through the same function. No id reuse and no dead window required; only that some untagged row carries <kind>-<run_id>.
This is not a regression, and that is why it is tracked rather than treated as a stop-ship. The gate #749 replaced was runs.is_run(runs.run_dir_for(project, run_id)), and anything that can write the record can equally create the run dir and its state.json — which admitted every untagged row under that name, with no window id to name at all. What is new is only the direction the record acts in: since #482/#518 it broke ties among rows scoping had already admitted, and now it can admit one.
Pinned as test_ctl_window_id_admits_a_record_naming_a_window_it_never_minted in tests/test_tui_launch.py, in the repo's characterization idiom — the reachable behaviour, written as the state this issue's fix has to change.
The read side is already hardened and stays out of scope: _read_ctl_window opens O_NOFOLLOW and O_NONBLOCK, checks S_ISREG on the descriptor rather than the path, and caps the read. What it does not do — deliberately, until this issue is closed — is treat the content as a capability.
Direction of the trade, both halves
Wherever runs.is_run still holds, every condition in the conjunction was already satisfied by the gate #749 replaced — which admitted the neighbour on the run-id collision alone, with no id reuse and no dead window required — so over those states #749 is a strict narrowing.
It is not a narrowing everywhere, and the exception is worth stating. _read_ctl_window asks nothing about the run dir, so an untagged row named by a readable record whose run runs.is_run now rejects is admitted after #749 and would have been refused before it. A partial prune is one route there, not the only one.
Both halves are already written into the ctl_window_id docstring, so this issue is the tracking record rather than the only place the fact lives.
Why it could not be closed in #749
Closing it needs a channel that says this window is mine, not an id I once minted — an identity written at mint time and read back at lookup. The only channel ctl_window_id has today is the PROJECT_OPTION tag, and it must read the tag and never write it: writing on read is claiming rather than proving, and would stamp a neighbour's window with this project's tag. A second option key does not help either, because it rides the same write that failed in condition 4.
Proposed remedy
A read-only attribute that does not depend on the write that failed. The window's pane pid is the obvious candidate: recorded alongside the window id at mint time, re-proven against the listing at lookup. A recycled id carries a different pid, so the conjunction breaks at condition 3 without any new write path — and a forged record is broken by the same check, since the session can write the pid it claims but cannot make the listing agree.
That needs a new column through the multiplexer seam (list_windows) with tmux/psmux parity behind it, which is why it is a separate change by size regardless of what it is called.
Related
Same weak-ownership shape as #419, which reaches it through prunable_sessions and _ctl_window_candidates — the pruning consumers, which #531 partitioned there and #749 left untouched. Here it feeds targeting.
Refs: #531, #419, #482, PR #749 (and its CodeRabbit thread)
Deferred out of #531 / PR #749, deliberately and with the reasoning written down rather than folded in: the remedy is a seam change, and the exposure that remains is narrower than the one #749 closes.
What #749 shipped, so the residual is stated against the right baseline
ctl_window_id(src/bmad_loop/tui/launch.py) used to admit an untagged control-session row whenever this project held a run dir for the run id. #749 replaced that with the record this project's own launch wrote: an untagged row is a candidate only whenwin_id == recorded. That is a positive claim — this project wrote it, about the window it minted — where the run dir was a coincidence of a caller-supplied id.The mechanism
The record names a window id, and a window id is a reusable handle rather than a durable identity. So the record proves the mint, not the window:
@7for runRIDand records@7.@7.@7to a new window.RIDin its<kind>-<run_id>name, and its own tag write failed, so it is untagged.The listing then shows
@7 / run-RID / <untagged>, this project's record still says@7,win_id == recordedpasses, anda,set_return_paneandxall resolve to the neighbour's window again.All four conditions must hold at once, which is why this is a conjunction rather than a case. Condition 3 is the load-bearing one: tmux does not recycle
@Nwithin one server's life, so on tmux this is reached across a server restart. Whether psmux recycles window ids has not been measured — that is the first thing to establish, and it decides how reachable this actually is.A second route to the same admission: the record is forgeable
Raised by CodeRabbit on PR #749 (CWE-284) and folded in here, because the remedy is the same one.
The conjunction above needs the backend to reuse an id accidentally. It does not have to be accidental. The record lives at
<project>/.bmad-loop/runs/<run_id>/ctl-window, under the project root every coding session can write —_read_ctl_window's own docstring says so. A session that writes a record naming an untagged window it never minted gets that window admitted here, andxresolves through the same function. No id reuse and no dead window required; only that some untagged row carries<kind>-<run_id>.This is not a regression, and that is why it is tracked rather than treated as a stop-ship. The gate #749 replaced was
runs.is_run(runs.run_dir_for(project, run_id)), and anything that can write the record can equally create the run dir and itsstate.json— which admitted every untagged row under that name, with no window id to name at all. What is new is only the direction the record acts in: since #482/#518 it broke ties among rows scoping had already admitted, and now it can admit one.Pinned as
test_ctl_window_id_admits_a_record_naming_a_window_it_never_mintedintests/test_tui_launch.py, in the repo's characterization idiom — the reachable behaviour, written as the state this issue's fix has to change.The read side is already hardened and stays out of scope:
_read_ctl_windowopensO_NOFOLLOWandO_NONBLOCK, checksS_ISREGon the descriptor rather than the path, and caps the read. What it does not do — deliberately, until this issue is closed — is treat the content as a capability.Direction of the trade, both halves
Wherever
runs.is_runstill holds, every condition in the conjunction was already satisfied by the gate #749 replaced — which admitted the neighbour on the run-id collision alone, with no id reuse and no dead window required — so over those states #749 is a strict narrowing.It is not a narrowing everywhere, and the exception is worth stating.
_read_ctl_windowasks nothing about the run dir, so an untagged row named by a readable record whose runruns.is_runnow rejects is admitted after #749 and would have been refused before it. A partial prune is one route there, not the only one.Both halves are already written into the
ctl_window_iddocstring, so this issue is the tracking record rather than the only place the fact lives.Why it could not be closed in #749
Closing it needs a channel that says this window is mine, not an id I once minted — an identity written at mint time and read back at lookup. The only channel
ctl_window_idhas today is thePROJECT_OPTIONtag, and it must read the tag and never write it: writing on read is claiming rather than proving, and would stamp a neighbour's window with this project's tag. A second option key does not help either, because it rides the same write that failed in condition 4.Proposed remedy
A read-only attribute that does not depend on the write that failed. The window's pane pid is the obvious candidate: recorded alongside the window id at mint time, re-proven against the listing at lookup. A recycled id carries a different pid, so the conjunction breaks at condition 3 without any new write path — and a forged record is broken by the same check, since the session can write the pid it claims but cannot make the listing agree.
That needs a new column through the multiplexer seam (
list_windows) with tmux/psmux parity behind it, which is why it is a separate change by size regardless of what it is called.Related
Same weak-ownership shape as #419, which reaches it through
prunable_sessionsand_ctl_window_candidates— the pruning consumers, which #531 partitioned there and #749 left untouched. Here it feeds targeting.Refs: #531, #419, #482, PR #749 (and its CodeRabbit thread)