fix(disk-hygiene): trust the no-interpreter notice marker only in an owned directory

[kyle-sexton]

Requested by Kyle · project thread

Closes #6350

Summary

Before: with no Python, disk-hygiene showed its no-interpreter notice once per session, latched by a marker it trusted wherever it found one. When --marker-root was absent or unwritable, the marker lived in ${TMPDIR:-/tmp}/disk-hygiene-guard-no-interpreter/. Another local user could create that directory first, or a symlink in its place, with a marker for any session, and so silence the notice for everyone. The guard's denies were never affected.

After: run-python-hook.sh reads or writes the notice marker only in a real directory this user owns ([[ -d && ! -L && -O ]], all builtins). A directory that fails the test is skipped, so the notice shows again (over-reporting, the safe direction this notice already takes).

Fix

• plugins/disk-hygiene/hooks/run-python-hook.sh: new _owned_marker_dir, used by _no_python_noticed before it trusts an existing marker and before it writes one. It does not create through an existing symlink or a foreign directory.
• plugins/disk-hygiene/hooks/run-python-hook.test.sh: a symlinked marker directory holding this session's marker still notices, nothing is written through the symlink, and the notice never latches. A directory owned by nobody still notices; that case needs root to set up and prints SKIP otherwise. A stay-quiet case shows that this user's own tmp directory still latches the notice once per session.
• Changelog fragment for disk-hygiene (patch, Security).

Defaults I picked where the issue left a detail open:

• The check applies to both marker candidates, the data root and the tmp fallback. It costs only builtins and changes nothing for a data root this user owns.
• On Windows under Git Bash, -O relies on MSYS's owner mapping. If a directory reads as not owned, the notice repeats rather than going silent.

Verification

• bash plugins/disk-hygiene/hooks/run-python-hook.test.sh: pass, including the new cases. The foreign-owner case ran as root here.
• The same suite against origin/main's run-python-hook.sh fails on the new symlink case: FAIL: a symlinked marker directory holding this session's marker is not trusted.
• shellcheck on both files: clean. shfmt is not installed here, so formatting was not machine-checked.
• scripts/check-changelog-fragments.sh --check and --check-required origin/main, scripts/check-changelog-parity.sh --check: pass. markdownlint on the fragment: 0 issues.
• scripts/affected-tests.sh --base origin/main --run was stopped early when the overnight run wrapped up. 26 of the 142 selected suites had finished, all passing, including every disk-hygiene suite. The rest were not run.

Related

• docs(conventions): minimal, user-visible model-facing text for hooks and mods, then sweep #6225, fix(disk-hygiene): cut guard deny text to the mismatch and a pointer to /disk-hygiene:clean #6311: the work whose security review found this.
• Not changed here: the .launched skip marker (--launch-marker/--skip-unless-marker) uses the same tmp fallback shape. A directory planted there could make the Stop monitor skip. Worth a follow-up with the same check.
• perf(autonomy, disk-hygiene): skip idle Stop rows in the launcher without starting bash #6437 (open) edits run-python-hook.test.sh and hooks.json in disk-hygiene. git merge-tree against its branch refused with "unrelated histories", so the overlap is unchecked.

🤖 Generated with Claude Code

https://claude.ai/code/session_01L8KdG7fWg2YrErVYWtohmR

---

Generated by Claude Code

[kyle-sexton]

ci-status is red because this PR is a draft, not because of the change. On a draft every lint and test lane skips (lint-repo, test-bash, test-node, test-python, check-skills, check-plugins), and the aggregate counts a skip as not passing. See .github/workflows/pr-require-checks.yml:507-513.

The lanes run once the PR is marked ready. That flip waits for Kyle, because it also starts the review lanes. Until then, the local runs listed under Verification in the PR body are the signal.

---

Generated by Claude Code

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-13T13:08:59.631657Z	9aaef90	Draft marked ready

ℹ️ 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.

[claude]

Claude finished @kyle-sexton's task in 3m 49s —— View job

---

Security review: no issues found

• Gather context (PR diff, changed files)
• Review run-python-hook.sh marker-ownership logic for security issues
• Review test file changes
• Check instruction-surface deletions (none — this PR tightens, not loosens, a check; no standing instruction text was removed)
• Post findings

Scope: plugins/disk-hygiene/hooks/run-python-hook.sh (new _owned_marker_dir / updated _no_python_noticed), its test file, and the changelog fragment.

Analysis: The new _owned_marker_dir gate ([[ -d && ! -L && -O ]]) is checked before the marker directory is trusted for either read or write, on both marker candidates (data root and tmp fallback). I checked for a TOCTOU gap between that check and the subsequent [[ -f "$path" ]] / : >"$path" use:

• An attacker cannot forge the -O ownership test without being root, so they cannot fabricate a real, non-symlink directory that passes the check as this user — the original "plant a directory/symlink first" attack from fix(disk-hygiene): check ownership of the no-interpreter notice marker before trusting it #6350 stays closed even under a race.
• Swapping the directory entry after the check passes requires unlink/rename rights on the parent ($TMPDIR), which the standard sticky bit on /tmp denies to a non-owner regardless of timing. Without a sticky bit (non-default), an attacker could still only replace the entry with something they own or a symlink, both of which the next check correctly rejects — so the failure mode stays "notice shows again," the safe direction this code already commits to.
• mkdir -p against a pre-existing dangling or live symlink at $dir is a no-op (the -e || -L guard short-circuits it), so no directory is ever created through a planted symlink.

No path lets another local user silence the notice or redirect the write to attacker-chosen content. Quoting throughout is correct ("$dir", "$path"), and $safe (session-id sanitization) is unchanged from existing code. Test additions (symlinked dir, foreign-owner dir gated on root, own-tmp latch-once case) exercise exactly these paths and match the implementation.

The one related gap — the .launched skip marker sharing the same unguarded tmp-fallback shape — is unchanged by this diff and is explicitly called out by the author in the PR's Related section as a disclosed follow-up, not a silent gap in this change.

No CRITICAL, IMPORTANT, or SUGGESTION findings.

claude-lane-summary: lane=claude-security-review sha=9aaef9088499a1d765156108a50dc82fdbda9b70 run=38402283697

[claude]

Claude finished @kyle-sexton's task in 3m 19s —— View job

---

Code review

• Gather context (PR body, diff, REVIEW.md, standards contract)
• Read run-python-hook.sh ownership-check logic and run-python-hook.test.sh new cases
• Trace _marker_candidates / _MARKER_PATHS call sites to confirm scope
• Attempt to run the test suite
• Check related marker code (guard_launch_monitor.py) for scope correctness
• Post findings

Findings: 0.

Traced _owned_marker_dir (-d && ! -L && -O, all builtins) and its two call sites in
_no_python_noticed
(run-python-hook.sh#L580-L601):
the read loop only trusts a marker whose parent directory is owned and not a symlink before
testing -f, and the write loop only creates/writes through a directory that passes the same
check, preferring the data root over the tmp fallback and breaking after the first successful
write. That matches the stated intent and the new test cases (symlinked dir rejected and nothing
written through it, foreign-owner dir rejected, own tmp dir still latches once per session). The
-e || -L guard before mkdir -p correctly avoids running mkdir against an existing
non-directory or a dangling symlink, and _owned_marker_dir catches the symlink case either way.
No correctness gap found in the diff.

Noted but out of scope for this lane: guard_launch_monitor.py's own once-per-session .warned
marker (_already_warned/_write_marker,
guard_launch_monitor.py#L178-L196)
uses the same tmp-fallback shape with no ownership check, in a file this PR doesn't touch. That's
a pre-existing issue on untouched lines and the same security-finding class this PR fixes, so per
REVIEW.md's lane split it's left to the security lane (and to a possible follow-up alongside the
.launched marker the PR body already flags) rather than reported here.

Verification note: I could not execute run-python-hook.test.sh or shellcheck in this
lane's sandbox (command execution requires an approval this environment doesn't grant), so I
reviewed the logic statically rather than confirming the author's Verification section myself —
that section's pass/fail claims are author-reported and unverified by me.
· branch fix/6350-marker-ownership

claude-lane-summary: lane=claude-review sha=9aaef9088499a1d765156108a50dc82fdbda9b70 run=38402284162

[github-actions]

claude-review has reviewed this pull request through 9aaef90; a later push is reviewed from there.

[github-actions]

claude-security-review has reviewed this pull request through 9aaef90; a later push is reviewed from there.