Skip to content

fix(bin): allocate per-task temporary roots as claimed, task-owned random paths - #3483

Open
jleemcf wants to merge 11 commits into
kunchenguid:mainfrom
jleemcf:fm/fm-tasktmp-untrusted-root-t7
Open

fix(bin): allocate per-task temporary roots as claimed, task-owned random paths#3483
jleemcf wants to merge 11 commits into
kunchenguid:mainfrom
jleemcf:fm/fm-tasktmp-untrusted-root-t7

Conversation

@jleemcf

@jleemcf jleemcf commented Sep 2, 2026

Copy link
Copy Markdown

Intent

Make every per-task temporary root safely and unpredictably created and task-owned before GOTMPDIR, launchers, metadata, endpoint creation, or cleanup can use it, implementing the captain-approved design in data/fm-tasktmp-plan-p1/report.md without redesign. Use one shared tasktmp owner library. Publish a durable private claim naming each random candidate before anything exists in public temporary space, create the root and gotmp child privately, and classify every recorded path into the explicit named states trusted, absent, partial, or hostile so every caller agrees: partial own-private roots are recoverable by recreating gotmp on reuse and removing on cleanup, while foreign-owned or group/world-writable roots remain hostile and untouched. Every entropy, claim, collision, stat, ownership, mode, or validation failure must refuse loudly and must never fall back to predictable /tmp/fm-, chmod, enter, canonicalize, traverse, adopt, or delete an unsafe path. Relaunch must reuse the exact recorded trusted or partial random or grandfathered legacy root, allocate anew only for an absent field or recognized missing root, and refuse an unsafe root before stopping the prior worker. Teardown must revalidate immediately before process scanning and removal, preserve absent-field and missing-directory compatibility, refuse to touch an unsafe root while still completing all unrelated cleanup, and preserve a durable re-reportable record of the refused path. Ordinary pre-publication failure must clean the root and claim; locked startup must reconcile crash remnants under the spawn lock while read-only startup only reports and does not flag a live spawn as abandoned; published metadata, committed spawn state, and Orca recovery must transfer claim ownership safely, with exact matching validated metadata completing a pending transfer and ambiguous metadata never authorizing deletion. The candidate nonce must not leak through a temporary state filename before public-root creation. Preserve Orca success/failure recovery behavior and all backend/harness GOTMPDIR ordering. PR 3220 is not merged, so do not introduce or redesign Chrome launcher behavior; the current security surface is Go temporary-directory poisoning and unsafe cleanup. Prove behavior through executable interfaces, especially a hostile predictable path through real fm-spawn.sh with fake pane and real isolated worktree, asserting the hostile symlink, inode, permissions, and sentinel stay untouched while metadata and GOTMPDIR use the random root and no PATH export appears. State the exact old two-line mutation in the test and mutation-check that it turns this passing hostile test red. Cover distinct allocations, no-fallback failures, collision refusal, pre-publication cleanup, locked/read-only startup, unsafe live-root audit, claim transfer, named-state behavior, random and trusted-legacy relaunch reuse, pre-stop unsafe relaunch refusal, Orca cleanup success/failure, teardown accept/refuse compatibility and durable refused-root reporting, inactive receipt stability, backend metadata-derived exports, and random-root test cleanup. Keep Firstmate shared material one-sentence-per-line Markdown, plain dashes, shellcheck-clean scripts, behavior-based colocated tests, no agent co-author, and no Herdr lifecycle commands. Implement, validate, push, open the project PR, and wait for real CI to pass; do not merge.

What Changed

  • Added bin/fm-tasktmp-lib.sh as the single owner of per-task temporary roots: it publishes a durable private state/<id>.tasktmp-claim naming a random candidate before anything exists in the public temporary parent, creates the root and its gotmp/ child privately, and classifies every recorded path as trusted, incomplete, unsafe, or absent so no caller can reduce trust to a boolean. Entropy, claim, collision, stat, ownership, and mode failures refuse loudly instead of falling back to the predictable /tmp/fm-<id>, and an unsafe path is never chmodded, entered, canonicalized, traversed, adopted, or deleted.
  • Rewired the lifecycle onto that library: fm-spawn.sh claims and creates the root before any endpoint, launcher, or metadata exists, transfers claim ownership to published metadata (including the Orca recovery path), reclaims superseded roots after publication, and reuses the exact recorded trusted, incomplete, or grandfathered legacy root on relaunch — allocating anew only for an absent field or a recognized missing directory. fm-control.sh refuses an unsafe recorded root before stopping the prior worker; fm-teardown.sh revalidates immediately before process scanning and again before removal, keeps absent-field and missing-directory compatibility, and on refusal preserves the path, completes all unrelated cleanup, and writes a durable state/<id>.tasktmp-refused record surfaced in the completion line. fm-bootstrap.sh gains a seventh mutating sweep that reconciles crash remnants under the per-task spawn lock, with a report-only pass in detect-only sessions.
  • Extended the test suites to cover the new behavior end to end — including a hostile predictable /tmp/fm-<id> path driven through real fm-spawn.sh that asserts the hostile symlink, inode, permissions, and sentinel stay untouched while metadata and GOTMPDIR use the random root — plus distinct allocations, no-fallback and collision refusals, pre-publication cleanup, locked and read-only startup, claim transfer, relaunch reuse and pre-stop refusal, Orca cleanup, teardown accept/refuse compatibility, backend GOTMPDIR ordering, and random-root fixture cleanup. Docs (AGENTS.md, docs/configuration.md, docs/agent-control.md, docs/scripts.md, bootstrap-diagnostics) record the new state files, the TASKTMP_RECONCILE: diagnostic, and the seven-sweep bootstrap count.

Risk Assessment

⚠️ Medium: The security-critical invariants the intent requires are implemented and covered by real executable tests, and the prior rounds' error-level lock and durability defects are genuinely closed; the remaining issues are a re-reportability gap and a narrow leak on the new superseded-root reclaim path, both safe to address as follow-ups.

Testing

Beyond the suites, I drove the real fm-spawn.sh against a planted hostile /tmp/fm-<id> symlink and captured an operator-level transcript showing metadata and the pane's GOTMPDIR both pointing at a fresh unpredictable 0700 task-owned root while the attacker's symlink inode, target inode, 777 mode, and sentinel stayed byte-identical and no PATH export was emitted; I then applied the documented two-line legacy allocator mutation and confirmed that hostile case turns red before restoring the source. A second transcript drives real fm-teardown.sh and real fm-bootstrap.sh to show an unsafe recorded root refused untouched while the rest of teardown completes, a durable 0600 refusal record written, both read-only and locked startups re-reporting it, and the report retiring once the path is gone. The targeted suites for every touched surface (gotmp, kimi harness, control relaunch, teardown, backend, Orca, inactive reconcile, fixture cleanup, remote secondmate trace) all pass; the sole failure was fm-session-start's digest-ordering case, which I proved is an environment artifact of node being installed on this machine's system PATH by re-running it green against a node-free base PATH.

The captured results showed hostile paths remaining untouched while trusted private roots were allocated and reused, teardown refusals were durably re-reported, the targeted suites passed, and restoring the old allocator made the hostile-path regression test fail.

  • Outcome: ⚠️ 1 info across 1 run (14m32s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 3 issues (1 warning, 2 infos)
  • ⚠️ bin/fm-teardown.sh:2973 - Intent requires teardown to "refuse to touch an unsafe root while still completing all unrelated cleanup, and preserve a durable re-reportable record of the refused path." The refusal is only reported transiently: the REFUSED stderr lines (2757, 2762, 2918) and the completion line at 2973-2974 are process output, and teardown then removes state/<id>.meta (fm_backlog_atomic_transition remove at 2956), which is the only durable carrier of tasktmp=. Concrete sequence: /tmp/fm-<id> is a symlink to an attacker directory -> teardown prints REFUSED plus "...was preserved untouched for an operator" and exits 0 with the meta deleted -> the startup re-report path fm_tasktmp_audit_meta (bin/fm-tasktmp-lib.sh:496, driven by the state/*.meta loop at 566) has no record to scan, so no later fm-bootstrap.sh run ever mentions the path again. The hostile path stays on disk permanently with no re-reportable record. tests/fm-gotmp.test.sh:355 asserts the meta is removed, so this is a deliberate choice that needs the author's call: either the completion line was intended to satisfy the criterion, or teardown needs to leave a durable artifact (e.g. a state/<id>.tasktmp-refused record that fm_tasktmp_startup re-reports).
  • ⚠️ bin/fm-spawn.sh:2984 - On the relaunch path a claim-bookkeeping failure is a hard exit 1 after the prior worker has already been stopped (fm-control.sh do_relaunch calls do_exit before invoking fm-spawn --relaunch) and after the replacement metadata has been published at 2972. fm_tasktmp_claim_mark_committed writes state/.<id>.tasktmp-claim.commit.<pid> and renames it (bin/fm-tasktmp-lib.sh:322-328), so an ENOSPC/EROFS/EIO in the state directory aborts the relaunch before any launch delivery, leaving the task with a durable published record, an armed replacement busy generation, and no agent. The equivalent failure on the fresh path at 3198-3202 is only a warning, with the correct justification already written there: "The published task record still owns the exact root, so retaining the claim is safe and lets locked startup complete the transfer." That reasoning holds identically at 2980 - the metadata published two statements earlier records exactly TASK_TMP - so this path can degrade to a warning and let fm_tasktmp_startup locked finish the transfer. Separately, the guard [ -e &#34;$STATE/$ID.tasktmp-claim&#34; ] fires for every relaunch, including one that reused a trusted recorded root and minted no claim, so a leftover claim naming a different path also lands in this hard-exit branch even though it is not this relaunch's claim (the comment at 2977 assumes otherwise).
  • ℹ️ bin/fm-tasktmp-lib.sh:117 - fm_tasktmp_directory_stat_valid forks id -u on every call and fm_tasktmp_classify_path forks a subshell for fm_tasktmp_parent (line 63: cd -- /tmp &amp;&amp; pwd -P) on every call. A single fm_tasktmp_validate costs two id -u forks plus one parent-resolution subshell plus two stat forks. fm_tasktmp_startup's state/*.meta audit loop (566-572) runs that per task record on every locked and every read-only session start, and teardown runs it three times per task. Both the effective uid and the resolved trusted parent are process-invariant; caching each in a module-level variable on first use removes roughly half the forks with no behavior change.

🔧 Fix: fix(tasktmp): record refused roots durably, scope claim transfer
3 issues (1 error, 1 warning, 1 info) still open:

  • 🚨 bin/fm-spawn.sh:869 - The EXIT-trap claim reconciliation runs on paths that never held the per-task spawn lock, letting a second spawn delete a live spawn's temporary root. trap spawn_abort_cleanup EXIT is installed at bin/fm-spawn.sh:899, ID is assigned at 999, and SPAWN_TASK_LOCK is only acquired (with SPAWN_TASK_LOCK_HELD=1) at 1106-1111. Every exit in between - most concretely the exit 1 at 1109 for "another spawn is already creating task $ID", plus the backend-validation exits at 1093-1103 - reaches the block at 869 with the lock unheld, and its only guards are [ -n &#34;${ID:-}&#34; ] and the claim's existence. Concrete sequence: spawn A holds the lock and has published state/<id>.tasktmp-claim (phase=pending) plus its root at fm-spawn.sh:2112, but has not yet published state/<id>.meta; spawn B for the same id starts, fails fm_lock_try_acquire at 1107, exits, and its trap calls fm_tasktmp_claim_reconcile_one (871). In fm-tasktmp-lib.sh:422 the meta is not a regular file, so control falls to fm_tasktmp_claim_cleanup_root (446), which trusts the root (root scope), sees a valid gotmp child, and rm -rfs A's live root (410), then retires A's claim (447). A then silently skips its own transfer because spawn_tasktmp_claim_is_ours (2098) reads an absent claim, so A launches and records tasktmp= for a directory that no longer exists, and export GOTMPDIR=$TASK_TMP/gotmp (3109) points at a freed, publicly listable /tmp name any local user can now create - the exact Go temp-directory poisoning this change exists to close. The invariant the library header states ("Callers must hold state/.spawn-<id>.lock while creating or reconciling a claim") is violated by its own caller. The main flow holds the task lock all the way through the EXIT trap (there is no release before 876), so gating both the Orca commit block at 857 and the reconcile block at 869 on [ &#34;$SPAWN_TASK_LOCK_HELD&#34; = 1 ] is behavior-preserving for every path that can legitimately own a claim, and leaves an unowned leftover for locked startup exactly as the warning text at 3216 already describes.
  • ⚠️ bin/fm-teardown.sh:2984 - The completion line unconditionally tells the operator the refused root "was preserved untouched for an operator and recorded at <path> for re-reporting", but refuse_task_tmp (bin/fm-teardown.sh:761) tolerates a failed fm_tasktmp_refusal_record and only emits a separate stderr line saying "only this run reports it". Failure sequence: teardown refuses an unsafe root, the state directory is full or read-only so the record's printf/chmod/mv chain in fm_tasktmp_refusal_record (bin/fm-tasktmp-lib.sh:549-559) fails, the error goes to stderr, teardown still exits 0, and the stdout completion line names a state/&lt;id&gt;.tasktmp-refused path that does not exist. An operator or log reader acting on the success line believes a later startup will re-report the hostile path when nothing will, because teardown has already removed state/<id>.meta, the only other carrier of tasktmp=. Have refuse_task_tmp record whether the write succeeded (e.g. a TASK_TMP_REFUSED_RECORDED flag) and have line 2984 select between the "recorded at ..." wording and a "could not be recorded" wording.
  • ℹ️ bin/fm-tasktmp-lib.sh:655 - Locked startup reports an ordinary in-flight spawn as an actionable diagnostic while read-only startup reports the identical fact as informational. When fm_lock_try_acquire fails at line 654 the holder is provably live (fm-wake-lib.sh:823-830 steals every stale lock), which is the same condition fm_tasktmp_spawn_lock_live detects for the read-only branch at line 640, where it prints BOOTSTRAP_INFO: and does not set result=1. The locked branch instead prints TASKTMP_RECONCILE: and sets result=1, so any session start that happens while another session's spawn is mid-flight emits a line AGENTS.md and the bootstrap-diagnostics skill both classify as actionable, forcing a skill load for a non-issue. The skill text added in this change (.agents/skills/bootstrap-diagnostics/SKILL.md) documents this case as "needs no operator", so the classification looks deliberate - confirming whether that is intended, or whether the locked branch should emit BOOTSTRAP_INFO: like its read-only sibling, is the author's call.

🔧 Fix: fix(tasktmp): reconcile claims only under the spawn lock
3 issues (2 warnings, 1 info) still open:

  • ⚠️ bin/fm-spawn.sh:2119 - A non---relaunch spawn over an existing task record allocates a brand-new random root and abandons the one the record already named, leaking a /tmp directory per incarnation. Concrete path: bin/fm-remote-secondmate-control.sh:175 launches a remote secondmate through plain fm-spawn.sh &lt;id&gt; ... --secondmate (never --relaunch) with FM_STATE_OVERRIDE=$TARGET_HOME/state/parent-route; cmd_launch kills a dead endpoint first (fm-remote-secondmate-control.sh:167-170) and then re-runs that same fresh spawn against an existing parent-route/<id>.meta. RELAUNCH is 0, so line 2119 calls fm_tasktmp_claim_create and the metadata block at 2924 overwrites tasktmp= with the new root. The previous root has no claim (it was retired by fm_tasktmp_claim_transfer at first launch) and is no longer named by any record, so none of the three recovery paths can ever reach it: fm_tasktmp_claim_create's reconcile (bin/fm-tasktmp-lib.sh:470) needs a claim, fm_tasktmp_startup's claim and meta loops (628, 703) only see the current tasktmp=, and teardown only removes the last recorded value. Before this change the same re-launch reused the single /tmp/fm-<id> via mkdir -p, so nothing accumulated. The leak is already visible in the change's own fixtures: tests/fm-remote-secondmate-trace-context.test.sh:42-54 adds TRACE_TASKTMP_RECORDS plus remember_recorded_task_roots specifically because "a later incarnation of the same task id overwrites the tasktmp= record that names the previous one", and calls it after each of the three ios spawns. Bootstrap's secondmate liveness sweep relaunches dead or missing secondmates at every session start, so a flaky secondmate accumulates one 0700 /tmp/fm-<id>.<nonce> root per start with no bound. Either reuse the recorded root through fm_tasktmp_recorded_prepare on the fresh path when $STATE/$ID.meta already records one, or fm_tasktmp_remove the previously recorded root before publishing the replacement. This changes allocation semantics the author appears to have accepted deliberately, so it needs their call.
  • ⚠️ bin/fm-tasktmp-lib.sh:628 - fm_tasktmp_startup globs only the top level of the state directory (&#34;$state&#34;/*.tasktmp-claim at 628, &#34;$state&#34;/*.tasktmp-refused at 672, &#34;$state&#34;/*.meta at 703), but remote secondmates run their whole tasktmp lifecycle one level down. bin/fm-remote-secondmate-control.sh:41 sets CONTROL_STATE=$TARGET_HOME/state/parent-route and passes it as FM_STATE_OVERRIDE to fm-spawn.sh (175) and to fm-teardown.sh (316, 321), while bin/fm-bootstrap.sh:1379 and :1527 call fm_tasktmp_startup with STATE=${FM_STATE_OVERRIDE:-$FM_HOME/state} - the parent directory. Concrete failure: a remote-secondmate teardown refuses an untrusted recorded root, refuse_task_tmp (bin/fm-teardown.sh:761) writes $TARGET_HOME/state/parent-route/<id>.tasktmp-refused and removes the meta, and no later session start on that host ever names the path again, because the refused-record loop never descends into parent-route. That directly contradicts the intent's "preserve a durable re-reportable record of the refused path" and the library header's own promise at bin/fm-tasktmp-lib.sh:21-23 that "every later startup re-reports it until the refused path is gone". The same blind spot means a crash remnant claim in a route directory is never reconciled under the spawn lock by startup (it only self-heals if that exact secondmate is launched again). The test-side sweep in tests/lib.sh:86-101 already walks nested state/<route>/<id>.meta for exactly this reason, so the nesting was known; the production sweep was not extended to match. Deciding whether startup should also scan state/*/ route directories is the author's call.
  • ℹ️ bin/fm-tasktmp-lib.sh:85 - Round 1's accepted fork-cost cleanup was only half applied. fm_tasktmp_euid (69) now memoizes into FM_TASKTMP_EUID as asked, but fm_tasktmp_parent still runs $(CDPATH=&#39;&#39; cd -- /tmp 2&gt;/dev/null &amp;&amp; pwd -P) in a fresh subshell on every call, and fm_tasktmp_classify_path (160) calls it unconditionally - so every fm_tasktmp_trust, fm_tasktmp_validate, fm_tasktmp_validate_root, fm_tasktmp_claim_read, and fm_tasktmp_claim_create pays one subshell plus a cd. fm_tasktmp_startup's meta audit loop (703-709) runs that once per task record on every locked and every read-only session start, and fm-teardown validates three times per task. The resolved trusted parent is process-invariant exactly like the uid, so memoizing it in a module-level variable on first use (and returning the cached value) removes the remaining per-call fork with no behavior change.

🔧 Fix: fix(tasktmp): reclaim superseded roots, sweep route state dirs
3 issues (1 warning, 2 infos) still open:

  • ⚠️ bin/fm-spawn.sh:2985 - When the new superseded-root reclaim refuses an unsafe path it emits only a transient stderr warning, even though the record that named the path was destroyed two statements earlier, so the hostile path becomes permanently unnameable. Sequence: a fresh spawn over an existing record captures SPAWN_SUPERSEDED_TASK_TMP at 2125 (say the grandfathered legacy /tmp/fm-<id>, which an attacker has since replaced with a symlink to a directory they own); publication at 2975 overwrites state/<id>.meta so tasktmp= now names the new random root; the reclaim at 2984 calls fm_tasktmp_remove, whose fm_tasktmp_trust -> fm_tasktmp_validate -> fm_tasktmp_directory_stat_valid reports 'is not a real directory' (bin/fm-tasktmp-lib.sh:150) and yields FM_TASKTMP_TRUST=unsafe, so nothing is touched - correct - but the only durable carrier of that path is gone. Neither of the three startup recovery loops can ever name it again: the claim loop (bin/fm-tasktmp-lib.sh:641) needs a claim, the refused-record loop (:685) needs state/<id>.tasktmp-refused, and the meta audit (:716) reads the new tasktmp= value. This is the exact durability gap round 1 identified for teardown, and the library already ships the fix teardown uses: refuse_task_tmp (bin/fm-teardown.sh:761) calls fm_tasktmp_refusal_record so every later startup re-reports the path until it is gone, and the library header at bin/fm-tasktmp-lib.sh:21-23 states that contract unconditionally. The reachable remote-secondmate path is the same one this reclaim exists for: bin/fm-remote-secondmate-control.sh:167-175 kills a dead endpoint and re-runs a plain fresh spawn over the existing parent-route/<id>.meta. Fix: in the failure branch at 2985, when FM_TASKTMP_TRUST is unsafe, also call fm_tasktmp_refusal_record "$STATE" "$ID" "$SPAWN_SUPERSEDED_TASK_TMP" "$FM_TASKTMP_ERROR" and say in the warning whether the record was written, mirroring bin/fm-teardown.sh:761-773.
  • ℹ️ bin/fm-spawn.sh:2983 - The superseded-root reclaim runs only on the successful publication path, but two abort paths destroy the prior record without reaching it, leaving the superseded root with no record naming it - the same leak class the reclaim was added to close. (a) Orca recovery: with an existing meta naming S and a fresh spawn under way, a failing fm_backend_remove_worktree publishes the recovery metadata at 826-847 over state/<id>.meta with tasktmp=$TASK_TMP (the NEW root), so the old record naming S is replaced; the claim blocks at 863-882 then retire the claim for the new root and exit, and S is never reclaimed. (b) Fresh-commit rollback: SPAWN_FRESH_COMMIT_PENDING is set at 2910 before the publish at 2975, so an abort in that window runs spawn_fresh_commit_rollback -> fm_backlog_dispatch_rollback, which fm_backlog_record_remove's state/<id>.meta (bin/fm-backlog-transition-lib.sh:431) - the OLD record naming S - while reconcile_one only cleans the new claimed root. Both windows are narrow and the leaked root is mode 0700 and unpredictably named, so this is a bounded resource leak rather than a security hole, but it is mechanically closable: factor the reclaim at 2983-2988 into a helper and also call it from spawn_abort_cleanup after the claim reconciliation, guarded on SPAWN_SUPERSEDED_TASK_TMP being non-empty and no longer equal to the current fm_meta_get "$STATE/$ID.meta" tasktmp value - that guard leaves S alone in the ordinary abort where the old record survives intact.
  • ℹ️ bin/fm-spawn.sh:3240 - The relaunch and fresh claim-ownership blocks are byte-identical apart from one word in the warning. bin/fm-spawn.sh:3021-3025 and 3238-3242 both run fm_tasktmp_claim_mark_committed &#34;$STATE&#34; &#34;$ID&#34; || fm_tasktmp_claim_transfer &#34;$STATE&#34; &#34;$ID&#34; &#34;$STATE/$ID.meta&#34; behind spawn_tasktmp_claim_is_ours and warn 'task $ID was relaunched/launched, but its temporary-root claim remains for locked startup reconciliation: $FM_TASKTMP_ERROR'. A single helper next to spawn_tasktmp_claim_is_ours (bin/fm-spawn.sh:2108) taking the verb as its argument would collapse both call sites to one line each and keep the two transfer points from drifting apart, which matters because they are the only places a published record takes ownership of a claimed root.
⚠️ **Test** - 1 info
  • ℹ️ tests/fm-session-start.test.sh:994 - tests/fm-session-start.test.sh:994 (test_output_ordering_diagnostics_lead) fails on this machine for an environmental reason unrelated to the change: the fixture forces a MISSING: node diagnostic by deleting node from its fake bin, but node is installed at /usr/bin/node, which is on the suite's fallback base PATH (/usr/bin:/bin:/usr/sbin:/sbin), so bootstrap still finds it. Re-running the same suite with a node-free FM_TEST_BASE_PATH passed every case, and this change touches neither the ordering assertions nor bootstrap's tool detection. CI, which runs without a system node on PATH, is unaffected.
  • bin/fm-test-run.sh tests/fm-gotmp.test.sh tests/fm-kimi-harness.test.sh tests/fm-control-relaunch.test.sh (95 assertions, 0 failures)
  • bin/fm-test-run.sh tests/fm-teardown.test.sh
  • bin/fm-test-run.sh tests/fm-backend.test.sh tests/fm-backend-orca.test.sh tests/fm-test-fixture-cleanup.test.sh
  • bin/fm-test-run.sh tests/fm-inactive-reconcile.test.sh tests/fm-remote-secondmate-trace-context.test.sh
  • bin/fm-test-run.sh tests/fm-session-start.test.sh (failed on this box) then FM_TEST_BASE_PATH=&lt;node-free shim&gt; bin/fm-test-run.sh tests/fm-session-start.test.sh (passed) to prove the failure is environmental
  • Mutation check: replaced the fm-spawn.sh allocator block with the exact old two lines TASK_TMP=&#34;/tmp/fm-$ID&#34; / mkdir -p &#34;$TASK_TMP/gotmp&#34;, re-ran tests/fm-kimi-harness.test.sh (red at kimi meta adopted the hostile predictable root), then restored the file and confirmed git status --porcelain is clean
  • Manual evidence run: real bin/fm-spawn.sh against a planted hostile /tmp/fm-&lt;id&gt; -> 0777 symlink with sentinel, using the kimi harness fixture's fake pane and real isolated worktree; captured metadata tasktmp=, pane export GOTMPDIR= line, PATH-export count, before/after lstat dev:inode and mode of the symlink and target, sentinel contents, and fm_tasktmp_trust classification of both paths
  • Manual evidence run: real bin/fm-teardown.sh on a task whose recorded root is attacker-controlled, then real bin/fm-bootstrap.sh in read-only (FM_BOOTSTRAP_DETECT_ONLY=1) and locked modes, then again after the operator removes the hostile path, capturing the refusal message, the 0600 .tasktmp-refused record contents, the repeated TASKTMP_RECONCILE reports, and the retirement BOOTSTRAP_INFO line
  • Leak check: confirmed no stray /tmp/fm-*.&lt;nonce&gt; random roots and no tasktmp-bearing fixture metadata remained after the runs
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T00:37:25.464762Z db392a3 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge because no unacknowledged blocking failure remains.

No blocking failure remains.

Reviews (2): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: waiting on the author (CI just approved and still running; one intent-gap follow-up worth closing). Not waiting on the captain yet.

Attestation MATCH. Body no-mistakes-pipeline-attestation binds db392a3e717f5d9ac012b87da9a4457fd6e30cf0; HEAD is the same. MERGEABLE/UNSTABLE vs main 7d4b5177b4ed999db46ca3570af2d776bc39b0ea (compare: ahead 10 / behind 5; no rebase this pass). workflow-zero (no .github/workflows changes). Greptile SUCCESS 5/5.

Fork CI approved this pass after a real diff review (no workflow RCE surface; security-hardening scripts/tests/docs only): CI 33575543571 (in_progress after approve) and Require no-mistakes 33575543605 (SUCCESS).

Contract-class: new-default. Main still allocates the predictable /tmp/fm-<id> (bin/fm-spawn.sh two-line TASK_TMP="/tmp/fm-$ID"; mkdir -p …/gotmp) and tears it down with unvalidated rm -rf. This PR always-on replaces that for every unconfigured spawn with claimed random /tmp/fm-<id>.<nonce>, new state/<id>.tasktmp-claim / .tasktmp-refused artifacts, a seventh bootstrap mutating sweep, and TASKTMP_RECONCILE: diagnostics. A "fix" title and the real Go-temp poisoning close do not make that restore: main never promised random claimed roots, and changing how temp roots work for all unconfigured runs is a default-behavior widen. Claimed captain-approved design at data/fm-tasktmp-plan-p1/report.md is not present on main or in this PR's files, so it is not treated as captain consent from this pass. No auto-merge.

Security (via gh pr diff / HEAD sources, no clone): claim-before-create under the per-task spawn lock; /dev/urandom nonce; no fallback to predictable /tmp/fm-<id>; lstat without following; euid + private-mode checks; unsafe paths never chmod/enter/canonicalize/traverse/adopt/delete; EXIT-trap reconcile gated on SPAWN_TASK_LOCK_HELD=1; teardown / control pre-stop refuse; durable .tasktmp-refused for teardown refusals. Residual (already in the PR body's open NM review): when a fresh spawn's superseded-root reclaim refuses an unsafe prior path, only a transient warning is emitted after tasktmp= was overwritten — the hostile path becomes unnameable to later startup (same durability class teardown already closed). Narrow abort-path superseded-root leak is mode-0700/unpredictable (resource, not poisoning). Not a CI-approval blocker; please close the reclaim refusal-record gap (or explicitly accept it as a tracked follow-up) before asking for captain default-behavior.

Please:

  1. Prefer writing fm_tasktmp_refusal_record on the superseded-reclaim unsafe branch (mirror teardown's refuse_task_tmp), matching the intent's durable re-report contract.
  2. Let CI finish green on this HEAD (or re-attest if you push).
  3. After that, this still needs an explicit captain default-behavior decision — I will not Firstmate-flag until it is otherwise ready except that decision.

VISION.md per-rule

  • One captain, one interface — aligns as motive (unsafe roots stay reportable; hostile predictable paths no longer adopted).
  • Authority is explicit and never inferred — does not align as shipped: always-on random claimed roots + new bootstrap sweep are not captain opt-in.
  • Scripts own the mechanics, agents own the judgment — aligns (fm-tasktmp-lib.sh owns exact trust/claim mechanics).
  • A restart is a non-event — aligns (claims, refusal records, locked/read-only startup reconcile).
  • Delegation with a spine — aligns (hostile predictable-root executable proof + mutation check).
  • The fleet outlives any vendor — aligns (GOTMPDIR ordering preserved; no Chrome/PR 3220 redesign).
  • Scope — aligns as command-layer temp-root safety; resisted as a default-behavior widen until captain word.

Do not merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants