Skip to content

TUI: ctl-window record write on win32 is check-then-write, with no reparse-safe anchor #527

Description

@pbean

Split out of #518 (see the P1 thread on src/bmad_loop/tui/launch.py), which closes the standing-redirect half but leaves the race.

What is left

_record_ctl_window / _forget_ctl_window anchor their write and their removal at a directory descriptor from platform_util.open_dir_confined, so on POSIX no path is named after the walk and an ancestor swap has nothing to redirect. DIR_FD_ANCHORED_WRITES is False on win32 — CPython's PC/pyconfig.h.in defines neither HAVE_RENAMEAT nor HAVE_OPENAT — so win32 keeps _run_dir_is_confined followed by a path-based write/unlink.

That check is answered about a path and is stale the moment it returns. A coding session that re-plants .bmad-loop/runs/<run_id> as a link or junction between the check and the write redirects the record out of the workspace.

What #518 already fixed

The standing redirect, which was the cheaper attack. The walk used is_symlink(), blind to a directory junction — and mklink /J needs neither elevation nor Developer Mode, unlike mklink /D. So a junction planted at any time defeated the check with no race at all. _is_link_of_any_kind now tests the reparse-point attribute (not os.path.isjunction, which is 3.12+ while win32 py3.11 is a CI leg). What remains needs the attacker to win the window between check and write.

Bound on the residual

  • Same uid as the writer: this is a host-side write by the TUI, which the coding session could not otherwise aim.
  • The filename is always ctl-window, so the reach is another project's record, not an arbitrary path.
  • The payload is a window id, so the primitive is truncation, not injection.
  • A redirected read is re-proved: ctl_window_id matches the recorded id against the live listing, scoped by the project tag, so a bad record degrades to the name scan rather than naming a wilder target.

Shape of a fix

CreateFileW on each component with FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT to get a handle that does not traverse a reparse point, then keep every subsequent operation relative to that handle. Windows has no renameat, so the atomic publish would be SetFileInformationByHandle with FileRenameInfo rather than os.replace. That is a new ctypes surface for platform_util, only verifiable on a Windows runner, which is why it was kept out of #518 rather than bolted onto a targeting bugfix.

Alternative considered and rejected: skipping the record on win32 and warning. It removes the surface but disables the #482 disambiguation on the platform the record's atomicity was designed for.

Reported by Codex during the review gate on #518.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions