Repository navigation
fix(bin): namespace per-task temp root by uid - #5439
Closed
Thoughts-One wants to merge 4 commits into
Closed
Thoughts-One wants to merge 4 commits into
Thoughts-One wants to merge 4 commits into
Conversation
A stale /tmp/fm-<id> directory left behind by another local account could block every task with that id, since the temp root's path did not depend on which account was spawning. Fold the spawning account's numeric uid into the path (/tmp/fm-<uid>-<id>) so two accounts can never compute the same root, keeping every existing safety guarantee (private 0700 root, safe reuse only of a real directory owned by this user and writable by nobody else, refusal of anything else) intact. Update fm-teardown.sh's comments and every test that hardcoded the old path to match, and add a regression test asserting a stale directory at the old account-agnostic path no longer blocks or is touched by spawn.
add_ship_task and add_herdr_ship_task still registered /tmp/fm-$id for cleanup after relaunch, while fm-spawn now creates /tmp/fm-<uid>-$id, leaking a temp directory on every relaunch case run.
Thoughts-One
force-pushed
the
fm/up-tmp-dir-root
branch
from
September 27, 2026 20:27
9f918d2 to
975d209
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix a defect: a stale /tmp/fm- directory owned by another local account blocks every later spawn of that task id. Namespace the per-task temp root by uid so each account gets its own root.
The task temp root becomes /tmp/fm--, with the existing 0700, owner, and not-writable-by-others checks unchanged.
What Changed
fm-spawn.shnow creates the per-task temp root at/tmp/fm-<uid>-<id>/(the numeric uid fromid -u). Before this change it used/tmp/fm-<id>/. That path was shared by all accounts, so a stale/tmp/fm-<id>that another local account owned blocked every later spawn of that id. The existing 0700, owner, and not-writable-by-others checks on reuse did not change. The comments infm-teardown.shnow give the new path. Teardown still removes the path that is recorded intasktmp=./tmp/fm-<id>path. It then checks that spawn succeeds, records and creates its own private (0700) uid-namespaced root, and does not change the old directory.Related: #3457 (closed stale), #3483 (closed).
Risk Assessment
✅ Low: The change is a narrow one-line path change: TASK_TMP becomes /tmp/fm-$(id -u)-$ID. The existing 0700, owner, and not-writable-by-others checks are unchanged. Teardown reads the recorded tasktmp= value, so it follows the new path with no other change. The fix round did what was asked: it removed the uid guard and updated the promoted-scout fixture path and its cleanup registration. The new regression test runs a real spawn against a world-writable legacy /tmp/fm- directory. The old code refused that spawn and the new code accepts it.
Testing
I drove the real fm-spawn and fm-teardown in a throwaway fm-lab Herdr session, first at base and then at target. The blocker was a /tmp/fm-<id> owned by another uid (524288). Base reproduces the defect: fm-spawn refuses with "task temp root ... is not a private directory owned by this user". Target spawns cleanly: meta records tasktmp=/tmp/fm-1000-<id>, the root is 0700 and owned by uid 1000, the pane prints GOTMPDIR=/tmp/fm-1000-<id>/gotmp, and the foreign directory is not changed. Adversarial checks still hold: a 0777 directory or a symlink at /tmp/fm-<uid>-<id> is refused as before. Teardown removes the uid-namespaced root and leaves the other account's directory. The targeted suites fm-kimi-harness (31 ok) and fm-gotmp (3 ok) pass. fm-control-relaunch gets past the step where base fails on a real uid-1001 /tmp/fm-sm3 on this host. It then fails on one unrelated case because the installed tasks-axi is 0.2.5 and that case needs 0.2.6 or newer. Before this check, a refused spawn has already created a Herdr tab, and this ordering is the same at base. Both lab sessions were torn down, the default session was not touched, and the worktree is clean.Evidence: Live Herdr-lab e2e script (foreign-uid blocker, podman unshare)
Source: Live Herdr-lab e2e script (foreign-uid blocker, podman unshare)
Evidence: Base 9296f9b reproduces the defect: spawn refused by foreign-owned /tmp/fm-<id>
Source: Base 9296f9b reproduces the defect: spawn refused by foreign-owned /tmp/fm-<id>
[base] planted: uid=524288 mode=755 /tmp/fm-uidtmpA713795 (this account uid=1000) [base] fm-spawn rc=1 stderr: error: task temp root /tmp/fm-uidtmpA713795 already exists and is not a private directory owned by this user; refusing to stage the launch command thereEvidence: Target f09098b: spawn succeeds, 0700 uid root, GOTMPDIR in pane, teardown cleans, unsafe roots still refused
Source: Target f09098b: spawn succeeds, 0700 uid root, GOTMPDIR in pane, teardown cleans, unsafe roots still refused
[target] planted: uid=524288 mode=755 /tmp/fm-uidtmpA742672 [target] fm-spawn rc=0 [target] meta: tasktmp=/tmp/fm-1000-uidtmpA742672 [target] root: uid=1000 mode=700 /tmp/fm-1000-uidtmpA742672 [target] legacy after spawn: uid=524288 mode=755 /tmp/fm-uidtmpA742672 [target] pane output: GOTMPDIR=/tmp/fm-1000-uidtmpA742672/gotmp [target] fm-teardown rc=0 [target] /tmp/fm-1000-uidtmpA742672 removed [target] planted: uid=1000 mode=777 /tmp/fm-1000-uidtmpB742672 [target] fm-spawn rc=1 (refused: not a private directory owned by this user) [target] symlink variant: fm-spawn rc=1 (refused)Evidence: fm-kimi-harness test run
Source: fm-kimi-harness test run
Evidence: fm-gotmp test run
Source: fm-gotmp test run
Evidence: fm-control-relaunch on target (one failure: tasks-axi 0.2.5 < 0.2.6 required)
Source: fm-control-relaunch on target (one failure: tasks-axi 0.2.5 < 0.2.6 required)
Evidence: fm-control-relaunch on base: fails on the defect itself (/tmp/fm-sm3 owned by uid 1001 on this host)
Source: fm-control-relaunch on base: fails on the defect itself (/tmp/fm-sm3 owned by uid 1001 on this host)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
bin/fm-spawn.sh:4011- The change adds a new failure path: ifid -ufails or prints a non-numeric value, fm-spawn refuses to spawn. The intent only asks for the temp root to become /tmp/fm-<uid>-<id>, with the existing 0700, owner, and not-writable-by-others checks unchanged.id -uis POSIX and effectively never fails, and the existing ownership checks still protect a degraded path like /tmp/fm--<id>. So this guard goes beyond the narrow PR the intent asks for. The smallest honest remedy is to remove it and useTASK_TMP="/tmp/fm-$(id -u)-$ID"directly. Keep the guard only if you want it as defense in depth.bin/fm-spawn.sh:4018- Transition edge case: a task spawned before this change hastasktmp=/tmp/fm-<id>in its meta. Onfm-spawn --relaunch, TASK_TMP is recomputed, and preserve_relaunch_meta rewritestasktmp=to the new /tmp/fm-<uid>-<id>. After that, fm-teardown removes only the new root, and the old root, owned by this user, is left in /tmp. The leak is small, one-time, and limited to tasks that were live during the upgrade. Fixing it would mean extra migration logic, which goes beyond the narrow scope the intent asks for. It is noted here only for awareness.tests/fm-control-relaunch.test.sh:1048- The promoted-scout relaunch fixture still writestasktmp=/tmp/fm-$id, but the other fixtures in this file were moved to/tmp/fm-$(id -u)-$id. This loop also runsrun_spawn ... --relaunch, which creates/tmp/fm-$(id -u)-rl-promoted-<mode>, and it never adds that path to TASK_TMPS, so each test run leaves three directories in /tmp. The leak existed before this change under the old path, but the fixture is now inconsistent with the new format. Fix: update the fixture line to/tmp/fm-$(id -u)-$idand addTASK_TMPS+=("/tmp/fm-$(id -u)-$id")in the loop.🔧 Fix applied.
✅ Re-checked - no issues remain.
tests/fm-control-relaunch.test.sh- tests/fm-control-relaunch.test.sh fails one case locally ('a relaunch must not re-run a transition the row already reflects') because the installed tasks-axi is 0.2.5 and that case needs 0.2.6 or newer. The case is not related to the temp root. On base, the same suite fails earlier: a real uid-1001 /tmp/fm-sm3 on this host blocks it, which is the defect this change fixes. Remote CI owns the full run.live-foreign-tmp-e2e.sh <base-9296f9b-tree> base: real bin/fm-spawn.sh --backend herdr on a new isolated fm-lab-uidtmp-* session provisioned and torn down through bin/fm-herdr-lab.sh, with /tmp/fm-<id> set to uid 524288 usingpodman unshare chownlive-foreign-tmp-e2e.sh <worktree> target: the same foreign-owned blocker, then fm-spawn, a check of the meta, directory mode and owner, and the pane's GOTMPDIR, then fm-teardown, then an unsafe 0777 directory and a symlink at /tmp/fm-<uid>-<id>bash tests/fm-kimi-harness.test.sh(includes the new test_kimi_spawn_ignores_stale_legacy_shared_temp_root)bash tests/fm-gotmp.test.shbash tests/fm-control-relaunch.test.shon target and on the extracted base tree, to compare the results✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.