platform_util._atomic_write — the shared body behind atomic_write_text and atomic_write_bytes —
resolves the target's parent directories by name, at both the stage and the publish step:
target = path.resolve() if follow_symlinks else path
fd, tmp_name = tempfile.mkstemp(dir=str(target.parent), prefix=target.name + ".", suffix=".tmp")
...
atomic_replace(tmp, target)
follow_symlinks=False skips the resolve, so the final component is replaced rather than written
through — that part is sound, and its docstring argues it well. But mkstemp(dir=target.parent) and
the os.replace destination are still ordinary path lookups. If any directory above the file is a
symlink, both the temp and the published file land wherever that link points, and the no-follow buys
nothing.
Concretely: follow_symlinks=False at .bmad-loop/policy.toml stops a link planted at
policy.toml. It does not stop a link planted at .bmad-loop/. mkdir(parents=True, exist_ok=True), which several of these callers run first, succeeds against a symlink-to-a-directory,
so a planted parent survives the setup step too.
Why this matters at these specific call sites
The reason follow_symlinks=False was chosen for them (#379, #590) is that they are machine-minted
files under a session-writable root — runsetup documents .bmad-loop/policy.toml as a path a
driven session can write. The escalation being closed is a sandboxed session getting the unsandboxed
orchestrator to write to a path of its choosing. A symlinked parent reopens exactly that escalation;
it just costs the attacker a directory swap instead of a file swap.
Affected: every follow_symlinks=False call site — .bmad-loop/decisions.json, the two per-run
sweep writes, .bmad-loop/policy.toml (three writers), story specs (three writers), and the park
records (three writers). The follow_symlinks=True sites have the same parent exposure, but there
the file itself is operator-curated and the threat model is different.
The machinery already exists, and has one consumer
platform_util already ships the correct primitives, added for tui/launch.py's control-window
record:
open_dir_confined(root, target) walks each component O_NOFOLLOW | O_DIRECTORY relative to the
one above it and hands back a descriptor, not a verdict — deliberately, because a boolean
"is this path confined?" is stale the instant it returns.atomic_write_text_at(dir_fd, name, text) then never names a path again: O_CREAT|O_EXCL at
0600 with an unguessable temp name, fsync before the replace, os.replace(..., src_dir_fd=, dst_dir_fd=).
tui/launch.py is currently the only consumer. So this is not "design a hardening story from
scratch" — it is "extend an established one to the rest of the no-follow cohort."
The blocker, and why this is not a small change
DIR_FD_ANCHORED_WRITES is hasattr(os, "O_DIRECTORY") — POSIX only. atomic_write_text_at's
own docstring says a caller reaching it "is on POSIX by construction," and open_dir_confined's says
callers "need a fallback for win32, which has no *at() family to anchor against." Windows has no
equivalent, and this repo treats win32 parity as load-bearing (two Windows CI legs).
So adopting this cannot be a swap inside _atomic_write; it needs a decision about what win32 does:
- Per-caller adoption with a documented win32 fallback, as
tui/launch.py does. Narrow, matches
existing precedent, but repeats the branch at every site.
- A no-follow-anchored variant of the helper (
atomic_write_bytes_at, plus a text sibling that
takes bytes) that callers opt into, with _atomic_write unchanged for everyone else.
- Fold it into
_atomic_write behind follow_symlinks=False — smallest call-site diff, largest
blast radius, and it silently changes behaviour for ~12 existing sites plus win32.
Also unresolved: atomic_write_text_at is text-only and forces UTF-8 with newline="". Three of the
no-follow sites (policy.write_mux_backend, the two frontmatter writers) are byte-verbatim on
purpose, to preserve a CRLF file's line endings, so they need a bytes-anchored variant that does
not exist yet.
Provenance and scope note
Raised by CodeRabbit on #590 (src/bmad_loop/verify.py:1015, consolidated onto verify.py:1412 and
engine.py:2674) and deferred there. #590 does not introduce this and strictly reduces the
surrounding exposure: the four sites it converted from bare write_text/write_bytes previously
followed a symlinked parent and a symlinked final component; they now follow only the parent. The
eight temp-and-replace sites resolved parents by name before and after. Fixing it means changing a
shared helper used well beyond the PR's brief, which AGENTS.md forbids widening into.
Severity
P2 rather than P1: it needs an actively hostile writer under the project root, and the orchestrator
and the driven session normally run as the same user, so the escalation is real only where the CLI
sandboxes session writes to the project directory. It is not P3 because where that sandbox does hold,
this is a write-anywhere escape, and it defeats a protection the codebase has already decided is
worth having.
platform_util._atomic_write— the shared body behindatomic_write_textandatomic_write_bytes—resolves the target's parent directories by name, at both the stage and the publish step:
follow_symlinks=Falseskips the resolve, so the final component is replaced rather than writtenthrough — that part is sound, and its docstring argues it well. But
mkstemp(dir=target.parent)andthe
os.replacedestination are still ordinary path lookups. If any directory above the file is asymlink, both the temp and the published file land wherever that link points, and the no-follow buys
nothing.
Concretely:
follow_symlinks=Falseat.bmad-loop/policy.tomlstops a link planted atpolicy.toml. It does not stop a link planted at.bmad-loop/.mkdir(parents=True, exist_ok=True), which several of these callers run first, succeeds against a symlink-to-a-directory,so a planted parent survives the setup step too.
Why this matters at these specific call sites
The reason
follow_symlinks=Falsewas chosen for them (#379, #590) is that they are machine-mintedfiles under a session-writable root —
runsetupdocuments.bmad-loop/policy.tomlas a path adriven session can write. The escalation being closed is a sandboxed session getting the unsandboxed
orchestrator to write to a path of its choosing. A symlinked parent reopens exactly that escalation;
it just costs the attacker a directory swap instead of a file swap.
Affected: every
follow_symlinks=Falsecall site —.bmad-loop/decisions.json, the two per-runsweep writes,
.bmad-loop/policy.toml(three writers), story specs (three writers), and the parkrecords (three writers). The
follow_symlinks=Truesites have the same parent exposure, but therethe file itself is operator-curated and the threat model is different.
The machinery already exists, and has one consumer
platform_utilalready ships the correct primitives, added fortui/launch.py's control-windowrecord:
open_dir_confined(root, target)walks each componentO_NOFOLLOW | O_DIRECTORYrelative to theone above it and hands back a descriptor, not a verdict — deliberately, because a boolean
"is this path confined?" is stale the instant it returns.
atomic_write_text_at(dir_fd, name, text)then never names a path again:O_CREAT|O_EXCLat0600with an unguessable temp name, fsync before the replace,os.replace(..., src_dir_fd=, dst_dir_fd=).tui/launch.pyis currently the only consumer. So this is not "design a hardening story fromscratch" — it is "extend an established one to the rest of the no-follow cohort."
The blocker, and why this is not a small change
DIR_FD_ANCHORED_WRITESishasattr(os, "O_DIRECTORY")— POSIX only.atomic_write_text_at'sown docstring says a caller reaching it "is on POSIX by construction," and
open_dir_confined's sayscallers "need a fallback for win32, which has no
*at()family to anchor against." Windows has noequivalent, and this repo treats win32 parity as load-bearing (two Windows CI legs).
So adopting this cannot be a swap inside
_atomic_write; it needs a decision about what win32 does:tui/launch.pydoes. Narrow, matchesexisting precedent, but repeats the branch at every site.
atomic_write_bytes_at, plus a text sibling thattakes bytes) that callers opt into, with
_atomic_writeunchanged for everyone else._atomic_writebehindfollow_symlinks=False— smallest call-site diff, largestblast radius, and it silently changes behaviour for ~12 existing sites plus win32.
Also unresolved:
atomic_write_text_atis text-only and forces UTF-8 withnewline="". Three of theno-follow sites (
policy.write_mux_backend, the two frontmatter writers) are byte-verbatim onpurpose, to preserve a CRLF file's line endings, so they need a bytes-anchored variant that does
not exist yet.
Provenance and scope note
Raised by CodeRabbit on #590 (
src/bmad_loop/verify.py:1015, consolidated ontoverify.py:1412andengine.py:2674) and deferred there. #590 does not introduce this and strictly reduces thesurrounding exposure: the four sites it converted from bare
write_text/write_bytespreviouslyfollowed a symlinked parent and a symlinked final component; they now follow only the parent. The
eight temp-and-replace sites resolved parents by name before and after. Fixing it means changing a
shared helper used well beyond the PR's brief, which AGENTS.md forbids widening into.
Severity
P2 rather than P1: it needs an actively hostile writer under the project root, and the orchestrator
and the driven session normally run as the same user, so the escalation is real only where the CLI
sandboxes session writes to the project directory. It is not P3 because where that sandbox does hold,
this is a write-anywhere escape, and it defeats a protection the codebase has already decided is
worth having.