TUI: a recorded ctl-window id is a handle, not an identity - a reused window id readmits an untagged neighbour

Đã đóng
#750 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@dracic đang làm issue này rồi.

Từ ngày 2/10/2026.

  • #853 của @dracic — đang mở

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
48/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
python
Lĩnh vực
cli, security

Hướng nghiên cứu

Bắt đầu tại src/bmad_loop/tui/launch.py và bài kiểm thử đặc trưng được ghim trong tests/test_tui_launch.py; trước tiên xác định xem psmux có tái sử dụng các ID cửa sổ hay không, như mô tả về hành vi của tmux. Truy vết điểm giao tiếp của bộ ghép kênh list_windows và các triển khai tmux/psmux của nó. Được xem là hoàn tất khi thao tác tra cứu chứng minh được quyền sở hữu cửa sổ mà không chỉ dựa vào ctl_window_id, với tính tương đương giữa cả hai backend và các bài kiểm thử đã được cập nhật.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

area:psmux area:tui needs-design P4

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:

  1. This project mints @7 for run RID and records @7.
  2. That window later dies (killed, pruned) and the backend frees @7.
  3. The backend reuses the freed id and hands @7 to a new window.
  4. 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)

Ngôn ngữ chính
Python
Star
174
Fork
72
Merge trung bình
19 giờ 25 phút
Pull request đã merge (30 ngày)
84

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của bmad-code-org/bmad-loop

Tất cả issue của bmad-code-org/bmad-loop

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.