fix(sandbox): close the PAT exposures md-notebook's carve-out opens

[bolichen97]

This PR was re-scoped. It opened as the fix for #8762 (the Notes backend masked
from its own state files). That issue was fixed and closed by #8794, which landed a
generic carve-out on main. What remains here is the HARDENING that carve-out needs and
did not ship with — so this PR no longer closes #8762 and no longer carries a carve-out
of its own. It now builds on main's _APP_BACKEND_OWNED_LEAVES /
app_backend_visible_targets, and reuses #8965's foreign-mask refusal rather than
reimplementing it.

Problem / Motivation

Giving the md-notebook backend its own hidden state leaves back made
workspace/md-notebook/{pat,vaults.json,settings.json} writable on a sandboxed host for
the first time. Two paths then expose the GitHub PAT to a same-uid sandboxed agent. Both
are reachable on main today.

A — an absent leaf gets no mask at all. mount(2) cannot target a path that does not
exist, and the launcher's hiding loops are guarded on existence, so a leaf that has never
been written is silently skipped:

• sandbox.py:3584 if os.path.isdir(target): and sandbox.py:3674 if os.path.isfile(target): — an absent path matches neither loop.
• _CREW_PRECREATE_HIDDEN_DIR_LEAVES (sandbox.py:598) is the only hidden-leaf materialiser input on main, and it holds ("aws-control-staging",) — nothing for the md-notebook FILE leaves.
• main's own comment above _CREW_PRECREATE_READONLY_DIR_LEAVES already names this: "The same mount(2) limit leaves the SENSITIVE_DIRS / SENSITIVE_FILES mask loops skipping their own absent targets. That is a real sibling gap … each leaf needs its own argument."

So on a default install an agent namespace spawned before the first vault attach has
no mask at the pat name. The namespace outlives that: the backend saves a PAT later, and
the still-running agent reads it at the real path.

B — the staged temp sits outside the mask. The mask covers exactly three names
(sandbox.py:217-219), while the writers published through atomic_write, which stages
next to the target:

• atomic_write.py:980 fd, tmp = tempfile.mkstemp(dir=str(path.parent), suffix=".tmp")
• server.py:429 atomic_write(target, pat, fsync=True, restrict_to_owner=True, …)

That temp holds the real PAT bytes at a name no mask covers. A SIGKILL between write and
rename leaves it readable forever. Related, same mechanism: server.py:1447 cleared the
PAT with os.unlink(_pat_file()), deleting the mask's own mount target.

Why it matters

The exposed value is a live GitHub token with push rights to the user's notes vaults. The
reader is any same-uid process in an agent namespace — which is exactly the population
_CREW_HIDDEN_LEAVES exists to fence, so both paths defeat the control rather than
sitting outside it. Path A needs no crash and no race: a fresh install, one long-lived
agent session, then a first vault attach.

What changed (motivation → approach → change)

Hole A is an absent mount target, so the fix is to stop the target being absent.
_materialize_md_notebook_mask_targets() publishes each leaf's absent-equivalent document
before launch, at the same call site as _materialize_sealable_ceilings() /
_materialize_maskable_dirs(). It is the NESTED counterpart to the latter, which
deliberately handles only direct children of the data home with a plain mkdir; these
leaves sit under an agent-writable workspace/md-notebook/, so every chain it walks first
goes through the #4381 planted-link refusal, before any mkdir (a mkdir follows a
planted link). _MD_NOTEBOOK_PRECREATE_CONTENT carries the per-leaf absent-equivalence
argument main's gap note asks for: pat → empty, vaults.json → [], settings.json →
{}, each already what its reader returns for absent.

Hole B is a temp outside the mask, so the fix is to move the temp inside one. All three
state writers now stage in a TOP-LEVEL md-notebook-staging directory, masked as a whole DIRECTORY
so every temp name — present, future, and orphaned — is covered, and added to the app's
owned leaves so the backend's own spawn still gets it back.
atomic_write.refuse_linked_parent is the existing #4381 guard made public for a stager
that cannot route through atomic_write itself; it is not a second implementation.

Four riders, each closing something the above would otherwise open. A leaf is accepted only
as a REGULAR file, judged by lstat, both when it already exists and when it wins the
publish race — one check covering a link (a mount resolves its target, so a resolving link
would mask the referent while the lexical name stayed agent-replaceable) and a
FIFO/socket/device node (matches neither isdir nor isfile, so its mask is skipped in
silence). Clearing the PAT writes an empty file instead of unlinking, keeping the mount
target. And the md-notebook spawn starts with -I + runpy and an explicit import root, so
sitecustomize / usercustomize / user-site .pth from an agent-writable directory cannot
execute in the one namespace where the PAT is unmasked; that is scoped to this spawn, and the
other module builtins keep the bare python -m.

The staging directory is TOP-LEVEL rather than a child of the state dir, for the reason the
sibling aws-control-staging leaf already records: a mask covers the leaf, not its ancestors,
so under the agent-writable workspace/md-notebook it could be renamed out from under its own
mask. Being a direct child of the data home also means the shared _materialize_maskable_dirs
creates it, under its own documented rule.

A link in the state chain degrades the app, not the host. Materialisation cannot verify
which directory a write to that name would reach, so the leaf is SKIPPED and the spawn
proceeds. Refusing it would let one optional app's on-disk layout stop every sandboxed process
on the host — an operator who symlinks workspace/ to another disk would find no agent could
start, over a Notes file they may never have created. Skipping is safe only because it is keyed
off the SAME predicate that withholds the carve-out (carveout_chain_has_planted_link, applied
in app_backend_visible_targets alongside #8965's foreign-mask filter): while a link is in the
chain the backend cannot write this state at all, so an unmaterialised — and therefore unmasked
— leaf has nothing to expose. The two are never decided separately, and a test pins them
together because separating them reopens exactly the hole this PR closes.

Two further items close the same exposures on paths materialising forward cannot reach.
Pre-upgrade orphans are swept: an older writer staged beside the target, so a crash in
that window can leave a PAT-bearing *.tmp at a name no mask covers, and exposure B would
otherwise be closed for new installs while an upgraded host kept those bytes readable
forever. Every *.tmp DIRECT child of the state dir is such an orphan by construction —
the writers now stage in the top-level staging dir, and note temps live beside their note under
vaults/<id>/ — so the sweep removes them (regular files only, judged by lstat, sparing another spawn's
in-flight ceiling temp), refuses the spawn if one cannot be removed, and logs each removal as
a token-exposure event so the operator knows to rotate. It runs on BOTH launch paths and
across BOTH crew-home spellings, unlike materialising, which is Linux- and live-home-only:
a Seatbelt deny and a live-home stub cannot reach an orphan already on disk under a legacy
home. A drift gate pins the fact that this hardening
is md-notebook-literal while _APP_BACKEND_OWNED_LEAVES is a generic table: an app joining
it would get its leaves unmasked for its own spawn and silently reopen both holes, neither
visible at runtime, so a test fails until that app either ships its own materialisation and
startup-isolation decision or is added to the coverage table.

Tests

test/test_sandbox_md_notebook_carveout.py (new, POSIX-only by collection):

• TestTheStagingDirectoryIsMaskedAndCarvedBack — md-notebook-staging is masked in all three tiers and both home spellings, is returned to the backend spawn, is lifted from the launcher's hidden list for it, has no agent-writable ancestor, is named identically by the writer and the mask, and is absent from the precreate table (the two tables must not drift). Also that the sweep runs on BOTH launch paths.
• TestAbsentStateFilesAreMaterializedSoTheMaskCanMount — every leaf is created with its absent-equivalent document; those documents read as absent to the backend; existing state is left byte-for-byte alone; an absent data home is not created; and the spawn is REFUSED on a failed publish, a planted intermediate symlink, a resolving leaf symlink, a dangling leaf symlink, a special file at a leaf, and one raced in during the publish window. Plus the legacy sweep: pre-upgrade sibling temps in all three shapes are removed; the state files and vault clones survive; a non-regular *.tmp is left alone; an unremovable orphan refuses the spawn; and the sweep refuses to delete through a planted parent link or a linked state directory (the chain guard and the O_NOFOLLOW open are each mutation-pinned). Plus: namespace_argv materialises before the launcher script is built.
• TestANewOwnedLeavesAppCannotSilentlySkipMaterialization — the drift gate: an app added to _APP_BACKEND_OWNED_LEAVES without materialisation coverage fails, with the message naming both things it must decide.
• TestStateWritersStageInsideTheMask — a crashed publish leaves its temp inside the mask; each writer publishes to its target with no residue; a planted parent link and a planted staging link each refuse the write; and the PAT clear keeps the mount target, asserted both at the writer and through api_pat's clear branch.

test/test_app_backend.py — the shipped md-notebook spawn starts isolated (-I, runpy,
explicit root) while other module builtins keep -m and get no carve-out, and a
third-party app wearing the name gets neither.

docs/system-specs/modules/md-notebook.md records the state layout (including the staging dir
and settings.json, both previously undocumented) and a new Sandbox section.

Mutation-checked: every guard was reverted in turn and the owning test asserted to go
red — 27 mutations, 27 caught, 0 survivors, re-run in full AFTER the rebase onto
cbdd4a569 rather than before, because the staging leaf moving to the data-home root and the
sweep being hoisted onto both launch paths invalidated the earlier run.

Three of those mutants were only caught because the run exposed a weakness and it was fixed
rather than explained away: a dedicated leaf-link helper turned out to be unreachable
(lstat already refuses a link at both the existing-target and race-winner checks) so it was
deleted as dead code; the PAT-clear test called the writer directly and so never pinned
api_pat's branch, which now has a route-level test; and the sweep's returned list was
bookkeeping no caller consumed, so it is now asserted as the operator-visible record of which
credentials were exposed. Stated precisely: the dir_fd PINNING of lstat/unlink is
defence-in-depth against a TOCTOU race and is NOT mutation-pinned — in any non-racing test the
chain guard fires first — while both link guards themselves are.

Manual verification

N/A — unit coverage sufficient. Both holes are on the sandbox launch path, and the tests
drive the real _materialize_md_notebook_mask_targets(), the real writers, and the real
namespace_argv / launcher-script builders against a temp data home; the refusals are
asserted by planting the actual on-disk shapes (symlinks, FIFOs, a raced mkfifo).

Related Issues

Context, deliberately NOT closing keywords:

• Notes (md-notebook): backend cannot write its own vaults.json under the OS sandbox — attach/clone always fails with EPERM #8762 — the original defect, already closed by fix(sandbox): unmask an app backend's own state leaves for its spawn #8794. This PR hardens that fix; it does not re-fix it.
• fix(sandbox): refuse carve-outs shadowed by a foreign mask (#8795) #8965 — the foreign-mask shadow refusal, merged. Reused via carveout_shadowed_by_foreign_mask; the staging spelling this PR adds is filtered by it for free.
• md-notebook: migrate state files into one masked .state/ directory, retiring the leaf-mask apparatus #8797 — retiring the leaf-mask apparatus for a single masked .state/ directory. That would subsume the staging and leaf-link work here; it needs a migration of live user state and is deliberately not attempted in this PR.
• Builtin provenance is path-only: consider integrity-verified package code for credential-sensitive spawns #8796 — integrity-verified provenance for credential-sensitive spawns, the architecture-level ask extracted from an earlier round of this PR.

Pattern harvest

Rule candidate: semgrep
Pattern: a mask/deny entry naming a path that no materialiser list covers, where the
launcher applies that entry under an existence predicate (isdir / isfile / exists).
Both halves are statically visible — the guarded application and the absent target — and
together they are the whole bug class, so detecting it needs no runtime probe.

Rule candidate: semgrep
Pattern: a secret writer opening its temp with mkstemp(dir=…) /
NamedTemporaryFile(dir=…) pointed at the TARGET's parent, while that parent is covered
by leaf-granular rather than directory-granular masking. The temp name is by construction
outside a leaf mask.

Not generalizable: which empty document is absent-equivalent for a given leaf. That is a
per-reader semantic judgement (b"" vs b"[]" vs b"{}" here), so the table carries a
written argument per leaf rather than a lint.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of dfe4e76f0fe262d89e59dfd12499381e16cb606b — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Both exposure paths are closed at the enforcing layer (mount targets, mask-covered staging), fail-closed only where launch would expose, root-cause fix tracked in #8797.

[DESIGN-REVIEWED] dfe4e76

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @bolichen97 overrides the GPT 5.6 finding for dfe4e76f0fe262d89e59dfd12499381e16cb606b; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt dfe4e76f0fe262d89e59dfd12499381e16cb606b: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of dfe4e76f0fe262d89e59dfd12499381e16cb606b — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All premises verified against the repo: atomic_write stages beside its target (atomic_write.py:999), the launcher's mask loops guard on isdir/isfile (sandbox.py:4167, 4257), the Seatbelt profile denies only named literal/subpath paths (so an arbitrary *.tmp sibling is genuinely uncovered on macOS), main's own sibling-gap note requires per-leaf arguments, fsync parity in the settings writer is preserved, readers' absent-equivalence is real, and is_builtin_app is a real provenance gate. The claimed prior mechanisms (_APP_BACKEND_OWNED_LEAVES, foreign-mask filter, aws-control-staging precedent) all exist; the atomic_write fork is recorded with a named follow-up (#8797) rather than presented as neutral. No pins were deleted; the counted new public surface (refuse_linked_parent: 4 call sites, carveout_chain_has_planted_link: 2) has real consumers.

First-Principles-Verdict: PASS

Verify on a real pip install --user host that the -I + runpy md-notebook launch still imports its deps through the single inserted root — a .pth-based editable install will not.

What this change ships

Inventory (10 items) — 10 justified

Intent: close the two ways the md-notebook carve-out (landed on main) exposes the GitHub PAT to same-uid sandboxed agents — a FIX, with each rider closing a named adjacent exposure of the same carve-out.

1. Absent md-notebook state files are created empty-equivalent before every Linux spawn, so masks always mount — justified
2. New top-level masked md-notebook-staging dir; all three state writers stage temps there, not beside the target — justified
3. Clearing the PAT now writes an empty file instead of deleting the mask's mount target — justified
4. Pre-upgrade *.tmp orphans beside the state files are swept at every launch (both OSes, both home spellings), logged as token exposure — justified
5. The shipped md-notebook backend launches with python -I + runpy instead of bare python -m — justified
6. A carve-out is withheld when the leaf's parent chain contains a planted link, coupled to the materialiser's skip by one predicate — justified
7. A link or special file at a state-leaf name refuses the spawn (lstat-judged, including the publish race) — justified
8. atomic_write.refuse_linked_parent becomes public (4 counted call sites: server.py ×2, sandbox.py ×2) — justified
9. The staging dir is also fenced from agent file tools (_CREW_SECRET_LEAVES) — justified (mandated by the pinned disposition-union invariant)
10. A drift gate fails CI if an app joins _APP_BACKEND_OWNED_LEAVES without materialisation and an isolation decision — justified

More than 10 quiet differences exist; the remainder are plumbing for the above (_publish_empty_ceiling gains a content param, _CEILING_TEMP_PREFIX named, MD_NOTEBOOK_APP_NAME exported).

[FIRST-PRINCIPLES-REVIEWED] dfe4e76

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed dfe4e76f0fe262d89e59dfd12499381e16cb606b — this comment is updated in place on each push.

Review details

Both candidates are self-rated low-confidence, and each dies under falsification:

Candidate 1 (sweep deletes any *.tmp direct child): The swept directory workspace/md-notebook/ has no legitimate *.tmp direct child by construction — state files aren't .tmp, the staging dir is now top-level, and note temps live under vaults/<id>/ (a subdirectory, not swept). The "concrete input" of an operator/tool placing an unrelated foo.tmp there is speculative, and the fail-closed SandboxCeilingUnsealable-on-unremovable behavior is the deliberate, tested design (test_an_unremovable_legacy_temp_refuses_the_spawn). No grounded (a)/(c).

Candidate 2 (blocking syscalls on the spawn path): The sibling _materialize_maskable_dirs() already performs blocking FS I/O at the identical call site pre-PR, so this is a pre-existing accepted pattern, not a regression this diff introduces — and the stall is conditioned on a hypothetical stalled network FS. Speculative; not 80+.

No new Step-2 finding grounds to the bar: no import cycle (atomic_write doesn't import sandbox), the _write_state_staged_sync failure path correctly closes the fd and unlinks the staged temp, and the PAT-clear route writes empty rather than unlinking (preserving the mask mount target, pinned by tests).

No findings.

[OPUS-REVIEWED] dfe4e76

Verdict parsed from the review's SHA-scoped output markers for commit dfe4e76f0fe262d89e59dfd12499381e16cb606b.

False positive or not applicable? A repository writer can comment:
/ai-review override fable dfe4e76f0fe262d89e59dfd12499381e16cb606b: <one-sentence reason>

[bolichen97]

Disposition: FIXED — the BLOCKING finding on f77d3d0 (absent state files bypass the Linux masks) was verified real and is addressed at head aa79d95. (span=0a32be773082)

Finding: mount(2) cannot target an absent path and the launcher's hiding loops guard on existence, so a leaf that does not exist yet gets no mask in the spawned namespace; this PR makes the three md-notebook state files creatable on a sandboxed host for the first time, so an agent namespace spawned before the first vault attach could read the PAT saved after it.

Fix shape is exactly the suggested one: materialise absent-equivalent state files before the Linux sandbox launch, fail closed when a mask target cannot be published. _materialize_md_notebook_mask_targets() runs at the _materialize_sealable_ceilings call site in namespace_argv, raising SandboxCeilingUnsealable on a failed publish or a dangling-symlink squat.
Per-leaf absent-equivalence argument (the bar _CREW_PRECREATE_READONLY_FILE_LEAVES sets): vaults.json → [] (_read_vaults_sync returns [] for absent and [] alike), settings.json → {} (_read_settings_sync reads {} as every-field-default), pat → empty (_read_pat_sync maps absent and empty both to None). Pinned by a test that calls the backend's own readers against the materialised documents rather than restating their behavior.
Stale-read direction is safe here: the agent-side view is the pinned empty mask file either way — that is the mask's intent, not a degradation, so the concern that excludes most hidden leaves from materialisation does not arise.
macOS needs nothing: Seatbelt denies are path rules that hold for names that do not exist yet.
Mutation-verified: removing the call site and corrupting an absent-equivalent document each go red on a distinct new test (8 mutations total across the PR).

[bolichen97]

Disposition: FIXED — F1 on aa79d95 (leaf-only masks expose atomic-write temporary files) was verified real and is addressed at head edd0b02, in the direction the finding suggested (a dedicated masked staging location for the shipped backend). (span=fa7ab943e399)

F1: the mask covers only the three exact leaf names; the PAT save staged a sibling temp holding the real PAT bytes at a name no mask covers, and a SIGKILL between write and rename left that unmasked sibling readable by a same-uid sandboxed agent forever.

All three state writers (_write_pat_sync, _write_vaults_sync, _write_settings_sync) now stage through one helper that mkstemps under workspace/md-notebook/.staging and renames into place (same filesystem, atomicity preserved; temp 0600 from mkstemp on POSIX, owner-only DACL on Windows before any payload byte).
.staging is masked as a WHOLE directory (like whatsapp): every temp name, present and future, is covered — the in-flight window AND the SIGKILL orphan. Added to BOTH gates: sandbox._CREW_HIDDEN_LEAVES (OS mask, every tier) and security._CREW_SECRET_LEAVES (agent file-tool gate), keeping the governance-mask union equation intact.
The materialiser creates the directory before every Linux launch so its mask has a mount target (an absent dir is skipped by the hiding loops — the same class as the round-1 finding), refusing the spawn on failure.
The backend carve-out includes .staging, so the one provenance-checked spawn can still write; note saves are deliberately untouched (they stage beside the note inside the vault, possibly a different filesystem, and hold no secret).
Pinned by tests: a simulated crash at publish time leaves the temp inside .staging and nothing beside the target; a successful write leaves zero residue; the default launcher masks .staging; mutations (stage-beside-target revert, staging-mask removal) each go red.

[bolichen97]

Disposition: ACCEPTED-AND-APPLIED — the CONCERNS subtraction (drop the deprecated ~/.kirocrew spellings from the carve-out) is implemented at head edd0b02.

Watch item: the legacy-prefix carve-out was justified by "an unmigrated host still keeps the real bytes at ~/.kirocrew", but no resolver returns that home, so the entries only exposed a stale legacy PAT copy for no functional gain.

• Verified the premise independently: config/paths.py has no legacy fallback and server._crew_data_home() resolves through config_dir() with a ~/.kiro/crew fallback — the backend can never read ~/.kirocrew.
• md_notebook_backend_state_paths() now names only the LIVE spellings (default home prefix + _relocated_crew_targets); the ~/.kirocrew mask entries stay. Pinned by a test asserting the legacy spelling is never carved and its launcher/seatbelt rules survive the carve-out.
• The sibling check's conclusion (0 spawned-backend siblings among the other app-owned masked leaves) matches my own audit; no remainder issue needed there.

[bolichen97]

Disposition: FIXED — F1 on edd0b02 (state writes follow pre-planted parent links) was verified real — a guard regression my round-2 fix introduced — and is addressed at head c3f37c9 in exactly the suggested direction. (span=ef1d1a0f6644)

F1: the old PAT write went through atomic_write(restrict_to_owner=True), whose path calls _refuse_linked_parent (#4381); the bespoke staged writer mkdir'd/mkstemp'd with no linked-parent refusal, so a pre-planted parent link would land the token outside the sensitive-path fence.

• atomic_write.refuse_linked_parent is now the PUBLIC form of the atomic_write: secret writes (restrict_to_owner=True) follow a pre-planted parent symlink/junction — add refusal in the shared helper #4381 refusal, documented for exactly this caller class (a secret writer that must stage outside the target's parent and cannot route through atomic_write itself). No second implementation: it delegates to the same _refuse_linked_parent.
• _write_state_staged_sync calls it on BOTH chains the write walks — the target and the .staging probe chain — BEFORE the mkdirs (which walk through a planted link and would build the tree under its target), matching atomic_write's own ordering.
• Pinned by two tests: a planted state-dir link and a planted .staging link each refuse with OSError and publish nothing at the link target; a mutation removing the two calls goes red.

[bolichen97]

Disposition: FIXED (the Watch item) / DECLARED — item 8 (undeclared shed of the #4381 planted-link guard) was real and is restored at head c3f37c9; the PR body and commit message now declare both the original shed and the restoration.

Watch: _write_state_staged_sync replaces the PAT's atomic_write(restrict_to_owner=True) call, and restrict_to_owner implied _refuse_linked_parent; the new writer followed a pre-planted parent link, undeclared.

• Restored via a public atomic_write.refuse_linked_parent (same implementation, no fork), called on both chains before the mkdirs; two refusal tests + a mutation pin it.
• Items 1-7, 9-10 of the review's ledger match my own accounting; nothing else in the delta is undeclared.

[bolichen97]

Disposition: FIXED — F1 on c3f37c9 (settings write lost its durability guard) verified real and fixed at head e9c7126, exactly as suggested. (span=ef1d1a0f6644)

F1: _write_settings_sync called the staged writer with the default fsync_file=False, while the previous path (_atomic_write_text_sync → _stage_note_text_sync) fsync'd before publish.

• Verified against the old path on-source: _stage_note_text_sync's docstring and body fsync the staged temp — the shed was real, introduced by my staging redirect.
• _write_settings_sync now passes fsync_file=True, with a comment naming why settings earn it (the autoSync authorization bit and lastSync stamp; a rename published from an unflushed page cache can discard an acknowledged toggle on power loss).
• The adjudicator's FLAG record (low, recoverable) is fair, but the restore costs one rare-path fsync and removes the guard-removal class entirely, so fixing beats rebutting.

[bolichen97]

Disposition: FIXED — F1 on e9c7126 (PAT clearing can reopen the mask race) verified real and fixed at head 6fe6ab8, exactly as suggested. (span=fa7ab943e399)

F1: api_pat's clear branch did os.unlink(_pat_file()), deleting the mask's mount target; a clear landing between the launcher's materialize and mount steps leaves that namespace maskless, and a later PAT save becomes readable inside it.

• The clear branch now atomically REPLACES the file with empty content via the same staged writer as the save (_write_pat_sync("")), so all guards match (masked staging, linked-parent refusal, owner-only, fsync) and the inode-at-the-name never disappears. The empty file is the reader's absent-equivalent (_read_pat_sync maps "" to None) — the same argument the materializer's per-leaf table already documents.
• The existing API contract holds: the hasPat: false clear test still passes unchanged.
• Pinned by a new test: after a clear the mount target still exists, holds zero bytes, and reads as absent.
• Class note: this was the last remaining writer/deleter of any masked md-notebook leaf — grep -n "unlink\|remove" server.py over the state paths shows no other site touches them outside the staged writer.

[bolichen97]

Disposition: FIXED — F1 on 6fe6ab8 (Python startup hooks inherit the credential carve-out) verified real and fixed at head 5c90d25 in the suggested direction (isolated startup for the module backend). (span=0a32be773082)

F1: the module builtin spawned via bare python -m under sys.executable, so sitecustomize/usercustomize/user-site .pth run at interpreter startup — and the default user site is an agent-writable, gateway-independent injection path that would execute inside the one namespace where the PAT is unmasked.

• The module-style builtin branch now launches with -I and runs the module via runpy.run_module after explicitly inserting the one import root kiro_crew itself was imported from (backend.py lives in that package, so its own tree root IS that root). repr() keeps both injected strings inert literals.
• Why runpy instead of bare -I -m: -I implies -E/-s and drops cwd-on-sys.path, so the import universe must be restated explicitly. The chosen root is correct across all three install layouts — venv (site-packages, harmless duplicate), --user (the user-site dir re-admitted as a PLAIN path entry, which never imports usercustomize and never processes .pth), and a source tree (the repo src root). Smoke-verified both the venv-site and explicit-root resolutions under -I on this host.
• Applies to every module-style builtin spawn, not only md-notebook — the injection surface is the same wherever the interpreter starts fresh.
• Pinned by the builtin spawn test (asserts -I + runpy shape rides with the carve-out) and a mutation dropping -I goes red; 714 tests across the app-backend/spawn-audit/bridges suites pass.

[bolichen97]

• Disposition: FIXED — the sweep now carries both link guards, at head d79df2c. (span=fa7ab943e399)

BLOCKING -- src/kiro_crew/sandbox.py:1137 -- Seatbelt sweep follows a linked state directory. Agent-created state-dir symlink -> sandbox_exec_argv sweep -> unlink follows the linked directory -> unrelated *.tmp files are deleted. Fix: refuse linked parent components before listing and require state_dir itself to be a real directory.

Correct, and the adjudication's framing is the part I have no answer to: the sibling materialiser's comment — which I wrote — states that the intermediate components are agent-writable and that a resolving link planted at one would land its writes under an attacker-chosen tree, which is exactly why it routes every chain through refuse_linked_parent. I then wrote a function directly beside it that DELETES, and omitted the same guard. Create-only was the weaker case; unlink is irreversible.

Fixed as suggested, with one addition. The chain goes through refuse_linked_parent before anything is listed. The directory is then opened O_NOFOLLOW | O_DIRECTORY, and every lstat and unlink is issued against that pinned descriptor rather than by path — O_NOFOLLOW alone only judges the final component, and the descriptor also closes the swap-between-check-and-delete window that a by-path unlink leaves open.

A root failing either guard is SKIPPED, not escalated to a spawn refusal, and that choice is deliberate rather than lenient: skipping removes the deletion hazard completely, while refusing would fail EVERY agent spawn on a host that merely symlinks its legacy ~/.kirocrew at the new home — a layout that is unusual, not hostile. What skipping costs is that an orphan under that root stays unswept, which is that root's pre-existing state rather than a new exposure, and on Linux the materialiser refuses the same chain anyway.

Two tests pin it: a planted link at workspace/ and a linked state directory, each with a bystander *.tmp in the victim tree that must survive. Both mutation-checked (dropping the chain refusal, and dropping O_NOFOLLOW, each turns one red). Stated precisely, because I would rather under-claim: the dir_fd PINNING of lstat/unlink is defence-in-depth against a TOCTOU race and is NOT mutation-pinned — in any non-racing test the chain guard fires first, so a deterministic test cannot distinguish it. The two link guards are pinned; the race hardening rests on the argument above.

Also fixed here, found by running the full suite rather than by review: adding the staging leaf to _CREW_PRECREATE_HIDDEN_DIR_LEAVES broke test_sandbox_absent_ceiling_seal.py's "an existing directory is left alone and not reported", which pre-created only aws-control-staging and asserted an empty result. It now iterates the tuple like its two sibling tests already do, so it stays true as leaves are added. Full suite re-run on this exact code: 83 failures, every one inside the 84 pre-existing host-environment failures a clean origin/main tree produces, zero introduced.

[bolichen97]

• Disposition: REBUTTED (the fork stands) — with the calcification risk agreed and now tracked.

The PAT writer forked off atomic_write […] the single credential-write chokepoint now silently excludes the most sensitive credential in the tree. Acceptable only while #8797 […] remains a live option — that decision should be made before this fork calcifies.

I accept the characterisation, including that the excluded credential is the most sensitive one, and I am not softening it — the spec says the same thing in the PR's own words. What I decline is closing the fork inside this PR, for the mechanical reason already recorded: atomic_write's pinned-parent path opens its temp through a directory descriptor so the location cannot be re-resolved, and a staging_dir= would have to bypass or duplicate that fence on a primitive with many callers.

The "before it calcifies" half is the actionable part and I have acted on it rather than agreeing in a thread: the calcification trigger is now recorded on #8797 itself, so the cost this PR documents informs that issue's priority instead of living only here. The one guard whose loss would actually matter is not left to vigilance either — refuse_linked_parent is the same helper made public, and a mutation dropping either call site turns a test red.

[bolichen97]

• Disposition: ACKNOWLEDGED — agreed, and the trigger you name is now on md-notebook: migrate state files into one masked .state/ directory, retiring the leaf-mask apparatus #8797.

The drift gate makes joining _APP_BACKEND_OWNED_LEAVES a bespoke security-design exercise […] a second app joining is the trigger point where deferring #8797 stops being cheap.

Agreed without qualification, and your framing is sharper than mine was: I had described the gate as making the omission loud, which is true but understates it — the gate makes the generic table stop scaling generically, and that is a real design cost rather than a neutral safeguard.

No code change, because the gate is the correct stopgap and the alternative (a generic materialiser) would have to invent a per-leaf absent-equivalence argument for a leaf it has never seen. What I have added is the missing link: #8797 now carries "a second app joining _APP_BACKEND_OWNED_LEAVES is the point where deferring this stops being cheap" as an explicit priority trigger, so the compounding cost is visible where the decision gets made.

[bolichen97]

• Disposition: FIXED — the trigger is recorded on md-notebook: migrate state files into one masked .state/ directory, retiring the leaf-mask apparatus #8797.

Record the "decide #8797 before a second owned-leaves app" trigger on #8797 itself, so the compounding cost this PR documents drives that issue's priority.

Done, as a comment on #8797. It names both costs this PR accepted contingent on that issue staying live — the forked secret-write chokepoint and the app-literal machinery behind a generic table — with the specific mechanics of each, and states the trigger: a second app joining _APP_BACKEND_OWNED_LEAVES.

Worth saying why this was the right ask rather than a nicety: every other record in this thread argues why a cost is acceptable, and all of them are conditional on #8797 being decided. That condition existed only in this PR's review history, where the person prioritising #8797 would never see it. It is now on the issue.

[bolichen97]

• Disposition: NEEDS-A-DECISION — verified, reproduced, and worse than I had assumed; the confidentiality-vs-availability trade is yours to sign off, so I have not changed it unilaterally.

New whole-host availability coupling: _materialize_md_notebook_mask_targets() runs on every namespace spawn and fails closed on "a link at any component" — so an operator who symlinks workspace/ (a common relocate-to-bigger-disk layout) has every sandboxed spawn refused over Notes-app state they may never use. […] a degraded mode that disables the md-notebook carve-out instead of refusing spawn would confine the blast radius to the app that owns the state.

Reproduced against this head: a data home whose workspace/ is a symlink to another disk, with no md-notebook state present at all and the app never used, makes _materialize_md_notebook_mask_targets() raise SandboxCeilingUnsealable — i.e. every sandboxed agent spawn on that host is refused, over a file that does not exist. The sweep, on the identical planted-link condition, skips quietly. So your comparison is exact.

One thing I want to correct rather than let stand, because "the sweep resolved the identical condition the other way" reads as inconsistency and the asymmetry is actually principled. The two failure modes differ in kind: if the SWEEP skips, a legacy orphan stays readable — but it was already readable before this PR, so skipping preserves the status quo and creates nothing. If the MATERIALISER proceeded, the mask would be vacuous for a leaf the carve-out has just made writable, so this PR would be creating the exposure it exists to close. Fail-closed is right there in a way it is not for the sweep.

What that argument does NOT justify is the blast radius, which is your actual point and which I accept. Two candidate remedies, neither of which I am taking without your ruling:

Narrow the trigger. Refuse only when there is something to protect — md-notebook state present, or the app installed — and otherwise skip materialisation. That answers "state they may never use" exactly, is a small change, and keeps fail-closed for every host that has a PAT to lose. It does not help a host that DOES use Notes under a symlinked workspace/.

Your degraded mode. Deny the backend its carve-out instead of refusing the spawn: the backend then cannot write, so no new PAT is created, and only the Notes app breaks. This confines the radius as you say, but it needs cross-module coordination (the materialiser runs in namespace_argv, the carve-out is granted in apps/backend.py) and it has a residue I would not want to hide — a PAT written BEFORE the link was planted stays unmaskable and readable, so degraded mode is safe for future writes but not retroactively.

My own lean is the narrowing, because it is the smallest change that removes the case you named without weakening any host that has a credential at stake. I am not shipping it on my own judgement: you flagged this as needing explicit sign-off and I agree that is the right call for a trade of this shape.

[bolichen97]

• Disposition: ACKNOWLEDGED — accurate, and the accretion trigger is already recorded on md-notebook: migrate state files into one masked .state/ directory, retiring the leaf-mask apparatus #8797.

The PR concedes #8797 […] subsumes the staging dir, leaf materialisation, and the atomic_write fork whose cost the spec itself records […] the drift gate institutionalizes bespoke per-app apparatus: each new _APP_BACKEND_OWNED_LEAVES entry must hand-build its own materialise/sweep/startup story. If #8797 slips, this pattern accretes.

Agreed, and I am not going to argue the framing down — "institutionalizes" is fairer than my earlier description of the gate as merely making an omission loud. The gate stops a silent regression; it does not stop the pattern from spreading, and those are different properties.

No code change here, because the alternative (a generic per-app materialiser) would have to invent a per-leaf absent-equivalence argument for a leaf it has never seen, which is the judgement that must not be automated. What exists instead is the accretion trigger, recorded on #8797 last round at your lane's own request: it names both costs this PR accepted contingent on that issue staying live, and states that a SECOND app joining _APP_BACKEND_OWNED_LEAVES is the point where deferring stops being cheap.

[bolichen97]

• Disposition: ACCEPTED — already actioned, and the sequencing argument is now on the issue itself.

Sequence #8797 promptly behind this: it deletes most of the ~900 md-notebook-literal lines added here and retires the atomic_write fork before a second carve-out app copies the pattern.

Accepted, and done to the extent I can do it: I cannot set another issue's priority, but I can make sure the cost driving that priority is visible where the decision happens. #8797 now carries a comment naming what this PR spends to work around its absence — the forked secret-write chokepoint with the guard that will not reach it, and the app-literal materialise/sweep/startup apparatus — plus the trigger condition.

Worth stating why that mattered more than agreeing in this thread: every "acceptable" argument in this PR's review history is conditional on #8797 remaining a live option, and that condition existed only inside this review. Whoever prioritises #8797 would never have seen it.

[bolichen97]

@bolichen97 Audit note on this PR, not a request to close it.

Already on main: #8794 landed the enabling half this PR hardens. src/kiro_crew/sandbox.py now carries _APP_BACKEND_OWNED_LEAVES and app_backend_visible_targets, so the Notes backend gets its three state leaves back and the original goal from issue #8762 is covered there.

Still missing on main: everything this PR now carries. A grep of main for _MD_NOTEBOOK_STAGING_LEAF, _MD_NOTEBOOK_PRECREATE_CONTENT, _materialize_md_notebook_mask_targets, _sweep_legacy_md_notebook_temps and md-notebook-staging returns zero hits, and both exposures are still live: the precreate tuple in src/kiro_crew/sandbox.py holds only aws-control-staging, and src/kiro_crew/apps/builtins/md_notebook/server.py still stages beside the target and unlinks the PAT. So this is not superseded.

Three things block it, none a duplicate:

1. The GPT lane BLOCKING F1 is unanswered at this head; the state leaves stay under the agent-writable workspace/md-notebook ancestor, which only md-notebook: migrate state files into one masked .state/ directory, retiring the leaf-mask apparatus #8797 fully fixes.
2. The design lane NEEDS-A-DECISION availability trade (a symlinked workspace/ refuses every sandboxed spawn) is parked on a human ruling.
3. It needs a rebase. refactor(security): split security.py into a package and drop path regex #9183 deleted src/kiro_crew/security.py, so the 10-line secret-leaf hunk must move to src/kiro_crew/security/paths.py. The PR is 84 commits behind and currently conflicting.

Please narrow the PR to that remaining scope and rebase. Also coordinate _CREW_PRECREATE_HIDDEN_DIR_LEAVES with #9010, #8468 and #9162, which each add their own leaf to the same tuple, and point #6307 at the public refuse_linked_parent this PR adds instead of the private helper.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

[bolichen97]

• Disposition: FIXED — a link in the state chain now degrades the app instead of the host, at head b974c1e.

New whole-host availability coupling: _materialize_md_notebook_mask_targets() runs on every namespace spawn and fails closed on "a link at any component" — so an operator who symlinks workspace/ […] has every sandboxed spawn refused over Notes-app state they may never use. […] a degraded mode that disables the md-notebook carve-out instead of refusing spawn would confine the blast radius to the app that owns the state.

Taken as you proposed, and the maintainer signed off on the trade you escalated. I had first argued for a narrower change — refuse only on hosts that actually have state or the app installed — and dropped it after checking the premise: md-notebook is an auto-discovered builtin with defaultEnabled, so "has the app" is true on essentially every default host and that narrowing would have bought nothing. Your shape is the one that works.

Implemented as one predicate with two responses rather than a special case. carveout_chain_has_planted_link() is the boolean form of the #4381 refusal; app_backend_visible_targets now filters on it beside #8965's carveout_shadowed_by_foreign_mask, so it is generic — ANY app's leaf whose chain is attacker-controlled is withheld, not just md-notebook's — and the materialiser skips that leaf instead of raising.

The safety argument is the coupling, so I want it stated rather than implied: skipping leaves a leaf unmasked, which is harmless ONLY while nothing can write it, and that is guaranteed by the carve-out being withheld under the same predicate. Separating the two would reopen exactly hole A. test_skipping_materialisation_and_withholding_the_carveout_are_one_decision pins it, and three mutations are caught — restoring the refusal, dropping the carve-out filter, and making the predicate fail toward "safe" instead of "unsafe".

One correction to my earlier reasoning, since I used it to argue against your suggestion: I claimed degraded mode leaves a PAT written BEFORE the link was planted unmaskable. That was not load-bearing — with the chain unsafe the mask binds exactly as it does on main today, so this PR neither improves nor worsens that case, and the withheld carve-out means no NEW state can be written while the condition holds. The objection did not survive checking.

What this costs, plainly: on a host that symlinks workspace/, Notes now fails on its own state rather than working. That is the app's pre-#8794 behaviour, it is confined to the app, and every other spawn on the host is unaffected — which is the trade you asked to have signed off.

[bolichen97]

/ai-review override gpt dfe4e76: the md-notebook state files already sit under the agent-writable workspace/md-notebook on main today, so this hardening PR neither introduces nor worsens the ancestor exposure, relocating that live state is tracked as #8797, and blocking here would leave the three PAT paths this PR does close open on main.

[github-actions]

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for dfe4e76f0fe262d89e59dfd12499381e16cb606b.

the md-notebook state files already sit under the agent-writable workspace/md-notebook on main today, so this hardening PR neither introduces nor worsens the ancestor exposure, relocating that live state is tracked as #8797, and blocking here would leave the three PAT paths this PR does close open on main.

This decision applies only to this commit. A new push requires a new judgment.