fix(scripts): widen the drive-root litter detector to known temp-sink names

[kyle-sexton]

Closes #3495

Summary

scripts/check-drive-root-litter.sh could not see the C:\tmp residue class — its only matcher was a single-letter drive-root directory naming a mounted drive, so the real C:\tmp\tmp.rSFIkHm5DO artifact observed on 2026-08-30 scored zero. This PR adds a second detection class for known temp-sink names at a drive root.

Fix

• New sink-name class: a known temp-sink name at a drive root (C:\tmp) is reported as litter. Sink vocabulary is reused verbatim from the sibling guard plugins/guardrails/hooks/block-windows-drive-tmp.sh — tmp is the only drive-root sink that guard blocks (/var/tmp and %TEMP% are legitimate and never sit at a volume root), so tmp is the only name here; the two lists grow together. Matching is case-insensitive (Windows filesystems fold case, so C:\TMP is C:\tmp) via drive-root entry enumeration, so the contract holds on the case-sensitive filesystems the test fixtures run on.
• False-positive protection: the same cwd-containment guard as the single-letter class (a candidate containing the cwd is a real checkout, not litter), plus DRIVE_ROOT_LITTER_IGNORE_SINKS (space/comma-separated names, any casing) for an operator who deliberately keeps C:\tmp. An env var rather than a marker file inside the directory, because the detector cannot trust litter's own contents to prove intent. The opt-out is scoped to the sink class only; the single-letter class stays fully armed.
• Preserved contracts: single-letter class behavior unchanged (the shared candidate check was factored into one record_if_litter helper both classes call — the guard body moved verbatim); non-Windows no-op still evaluated first and printed; exit codes 0 clean / 1 litter / 2 usage unchanged; still advisory per ADR 0003 (no measured precision, no promotion to a required gate).
• docs/conventions/windows-path-emit/README.md § "The detection net" (the SSOT) updated to describe both classes, case-insensitivity, and the opt-out.

Verification

• bash scripts/check-drive-root-litter.test.sh: 26/26 assertions pass — 15 pre-existing unchanged, plus new coverage: lowercase and uppercase (TMP) sink detected and named; non-sink drive-root directories not reported; opt-out works in both casings and both directions; opt-out does not bleed into the single-letter class; cwd-containment both directions for the sink class; both classes reported in one run.
• scripts/affected-tests.sh --run: selected the co-located suite, PASS; the doc change is covered by affected-tests-no-suite.txt.
• shellcheck clean on both scripts (local, v0.11-compatible).
• Independently verified twice by fresh-context agents given only the diff and pass/fail criteria (second pass after the case-insensitivity + dedup follow-up): all criteria PASS, no functional bugs found.
• The earlier hygiene job failure (shellcheck=failure, actionlint=failure) was infrastructure, not code: both steps failed downloading their tool binaries from GitHub releases (curl: (22) ... error: 502) before linting anything. The job passes on the current head.

Related

• Refs guardrails: nothing catches a POSIX /tmp path resolving to a Windows drive-root C:\tmp #2594 — the original ask for a drive-root guard against C:\tmp residue, which produced the command-time sibling guard block-windows-drive-tmp.sh; this PR is the same concern pointed at the filesystem after a run.
• docs/adr/0003-verification-guards-earn-default-on-by-measured-precision.md — the doctrine keeping this detector advisory.
• docs/conventions/windows-path-emit/README.md — the owning convention.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Egdf11hXdBch1HTFmjB8FV

[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-08-30T22:04:06.321646Z	7afe36a	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.

[claude]

Claude finished @kyle-sexton's task in 2m 36s —— View job

---

Code review: PR #3489

• Gather context (PR diff, description, changed files)
• Read changed files in full
• Check skip-gate conditions (PR open, non-draft, non-trivial diff — proceeded)
• Review against /review:code-review criteria (correctness, maintainability, CLAUDE.md alignment)
• Post findings (inline comments + summary)

Summary

Reviewed the diff (git diff origin/main...HEAD) covering scripts/check-drive-root-litter.sh, scripts/check-drive-root-litter.test.sh, and docs/conventions/windows-path-emit/README.md.

Correctness: I traced the new temp-sink loop (scripts/check-drive-root-litter.sh#L124-L146) by hand against each new test fixture (sink, sinkcheckout, both, and the opt-out cases) and it produces the expected hits/exit codes in every case, including:

• the sink class firing independently of the single-letter class,
• the cwd-containment guard applying correctly to sink candidates,
• DRIVE_ROOT_LITTER_IGNORE_SINKS suppressing only the sink class and not bleeding into the single-letter class,
• both classes reporting together in one run.

The DRIVE_ROOT_LITTER_IGNORE_SINKS space/comma parsing (" ${ignored_sinks//,/ } " + *" $name "* substring match) is correct and doesn't false-positive on names that merely contain tmp as a substring (e.g. mytmp).

The single-letter loop is confirmed byte-identical to before except for one clarifying comment, as claimed in the PR description.

The docs/conventions/windows-path-emit/README.md update accurately describes both classes and the opt-out, consistent with the script's own header comment.

One maintainability finding posted inline: the new temp-sink loop duplicates the single-letter loop's candidate-build/containment-check/labeling logic almost verbatim. Not a bug, but the containment logic now lives in two places that must be kept in sync by hand — flagged as a non-blocking suggestion to extract a shared helper. See inline comment on scripts/check-drive-root-litter.sh:124-146.

Not independently verified (outside this lane's tool scope — no general Bash execution available, only gh pr diff/view/comment/review, Read, Glob, Grep): the PR's claimed 23/23 test pass count, scripts/affected-tests.sh --run selection, and shellcheck-clean claim. I verified the test logic by static trace instead, which corroborates the claimed behavior but is not the same as having run it.

No blocking issues found.

[claude]

Claude finished @kyle-sexton's task in 56s —— View job

---

Security review

• Gather context (PR diff, changed files)
• Apply skip gate
• Review changed files for security-relevant surface
• Post findings

No security issues found.

This PR touches scripts/check-drive-root-litter.sh, its test suite, and a doc file. It's a read-only, advisory diagnostic scanner (per docs/adr/0003, not wired into any required gate):

• The new temp-sink class (check-drive-root-litter.sh#L124-L146) only probes the filesystem with -d and reads directories under $mount_root/$drive, where $drive is itself filesystem-derived (probed, not attacker-controlled) and $name comes from a fixed literal array (sink_names=(tmp)).
• The new DRIVE_ROOT_LITTER_IGNORE_SINKS env var (L127-L131) is only used in a quoted [[ ... == *...* ]] string comparison — no eval, no command substitution, no injection path.
• No secrets, credentials, network calls, or authorization logic anywhere in the diff.
• The doc change is prose-only, describing the new class and opt-out; no security-relevant content.

Per the skip gate (no security-relevant surface after reading the diff), no findings to report.

[github-actions]

Last security-reviewed head: 9929b5e3dac58735695d352cef64de04d73c84c7. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7afe36acf1

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

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

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

---

Code review: PR #3489 (re-review of 3b2b9d9)

• Gather context (new commit since last review, diff 7afe36a...3b2b9d9)
• Read changed files in full
• Check skip-gate conditions (PR open, non-draft, new commit since last review — proceeded)
• Review against /review:code-review criteria
• Post findings

Summary

This lane last reviewed commit 7afe36a. Since then, 3b2b9d9 made two changes, both correctness fixes, no new maintainability regressions:

1. Extracted record_if_litter() (check-drive-root-litter.sh#L115-L130), shared by both the single-letter and temp-sink loops. This resolves the maintainability finding this lane raised on the prior commit (duplicated candidate-build/containment logic) — confirmed by trace, no behavior change versus the two hand-duplicated copies.
2. Case-insensitive sink matching (check-drive-root-litter.sh#L144-L158): the sink loop now enumerates drive-root entries and lowercases before matching, instead of probing the literal tmp path. Real Windows hosts are case-insensitive so this was moot in production, but it closes a real gap on the test suite's own (case-sensitive) fixture filesystem — verified against the new sinkupper fixture and its two case-direction assertions (sinkupper/sink × TMP/tmp opt-out combinations).

One finding posted inline (scripts/check-drive-root-litter.test.sh:186): Codex's P2 from the prior commit — run() doesn't clear DRIVE_ROOT_LITTER_IGNORE_SINKS before ordinary detection assertions, so a caller's own exported value (now a documented opt-out someone could plausibly set persistently) leaks into the child process and can flip the sink-detection, sinkupper, and "both classes" assertions to false failures. I independently traced this against the current run() definition (lines 30–39) and confirmed it's still present — 3b2b9d9 only added case-insensitivity coverage, it didn't touch this. Not blocking (low-probability environment collision, advisory-only tooling), but straightforward to fix and worth doing given this repo just landed a commit (f8ab565) specifically about eliminating silent false test signals.

Not independently verified (no general Bash execution in this lane — only gh pr diff/view/comment/review, Read, Glob, Grep): the PR's claimed 23/23 test pass count and shellcheck-clean status. Verified by static trace instead of by running the suite.

No other issues found. The doc update (README.md#L171-L178) accurately reflects the case-insensitivity behavior, and cross-checking against the sibling guard (plugins/guardrails/hooks/block-windows-drive-tmp.sh) confirms tmp is indeed its only drive-root sink, supporting the PR's "vocabulary reused, not invented" claim.
· branch fix/drive-root-litter-sink-names

[claude]

Claude finished @kyle-sexton's task in 1m 15s —— View job

---

Security review: PR #3489

• Gather context (PR diff, description, changed files)
• Invoke /review:security-review per custom instructions
• Post findings

Applied the review:security-review skip gate and criteria to the delta since the last-reviewed head (7afe36a → 3b2b9d9, git diff 7afe36a..3b2b9d9), which only touches scripts/check-drive-root-litter.sh, its test suite, and the convention doc.

No security issues found.

The commit makes the temp-sink class match case-insensitively (C:\TMP == C:\tmp) by switching from probing a literal lowercase path to enumerating drive-root entries and folding case in bash, plus extracting the shared candidate-check into record_if_litter() (scripts/check-drive-root-litter.sh#L115-L130). I traced the new enumeration loop (#L148-L158):

• "$mount_root/$drive"/*/ is a standard glob expansion into full, quoted paths — no unquoted expansion of attacker-influenced data.
• name, name_lc are derived via parameter expansion (${entry%/}, ${entry##*/}, ${name,,}) and only ever used inside quoted [[ ... == *"$var"* ]] substring comparisons. The RHS pattern's literal (quoted) segments make glob metacharacters in a directory name inert — no pattern/glob injection.
• DRIVE_ROOT_LITTER_IGNORE_SINKS (still the only env-var input) goes through the same quoted-comparison path (ignored_sinks), never eval'd or passed to a shell that would reinterpret it.
• No new filesystem writes, no privilege/authorization logic, no secrets, no network calls. This remains a read-only, advisory, local diagnostic scanner (per ADR 0003), and the refactor doesn't change that trust boundary.

This confirms the earlier "No security issues found" verdict from the first commit still holds for the full PR as it stands at 3b2b9d9.

[claude]

Claude finished @kyle-sexton's task in 2m 29s —— View job

---

Code review: PR #3489 (/review:code-review, re-review at 1812742)

• Gather context (diff since fork point 0472f07, and delta since last-reviewed head 3b2b9d9)
• Read changed files in full (scripts/check-drive-root-litter.sh, scripts/check-drive-root-litter.test.sh, docs/conventions/windows-path-emit/README.md)
• Check skip-gate conditions (PR open, non-draft, head changed since last review — proceeded)
• Review against /review:code-review criteria (correctness, maintainability, CLAUDE.md/AGENTS.md alignment)
• Post findings (inline comment + summary)

Summary

This lane last reviewed 3b2b9d9. The only commit since then, 1812742, is a comment-only change (documents why the ${var,,} case folds in check-drive-root-litter.sh:143-145 are safe below the Windows host gate) — no functional diff, confirmed by git diff 3b2b9d9..HEAD.

One finding, posted inline (still open — scripts/check-drive-root-litter.test.sh:186): the run() helper (lines 30–39) doesn't clear DRIVE_ROOT_LITTER_IGNORE_SINKS before ordinary-detection invocations, so an operator's own exported value — now a documented, user-facing opt-out someone plausibly sets persistently — leaks into the child process and can silently flip the sink, sinkupper, sinkcheckout, and both assertions to false failures unrelated to the code under test. This is the same issue Codex flagged on 7afe36a and this lane confirmed again on 3b2b9d9; neither of the two commits since then addressed it. Given this repo just landed f8ab565 ("stop two silent false signals"), it's directly on point and worth closing out.

Everything else in this PR checks out:

• The temp-sink detection loop, case-insensitive matching (via drive-root enumeration rather than a literal-path probe), the record_if_litter() extraction, and the DRIVE_ROOT_LITTER_IGNORE_SINKS opt-out (including that it doesn't bleed into the single-letter class) all trace correctly against their fixtures.
• The README update (§"The detection net") accurately reflects the script's two classes, case-insensitivity, and opt-out.
• Cross-checked against the sibling guard plugins/guardrails/hooks/block-windows-drive-tmp.sh: tmp is indeed its only drive-root sink, supporting the "vocabulary reused, not invented" claim.
• No security-relevant surface (read-only filesystem probing, fixed literal sink vocabulary, no eval/injection path) — consistent with the security lane's prior clearance.

Not independently run (this lane has no general Bash execution — only git log/git diff, Read, Glob, Grep): the claimed 26/26 test pass count and shellcheck-clean status. Verified the test-isolation gap above by static trace of run() instead.

No other issues found.
· branch fix/drive-root-litter-sink-names

[claude]

Claude finished @kyle-sexton's task in 50s —— View job

---

Security review: PR #3489 (delta since last-reviewed head 3b2b9d9 → 1812742)

• Gather context (diff since last-reviewed head)
• Invoke /review:security-review and follow its instructions
• Post findings

No security issues found.

The only commit since the last security review (3b2b9d9) is 1812742, which touches a single file, scripts/check-drive-root-litter.sh, and adds three lines to an existing comment block — no code change:

 # case-insensitively by ENUMERATING the drive root's entries - a lowercase
 # probe would rely on the host filesystem folding case, which the test
 # fixtures' filesystems do not. DRIVE_ROOT_LITTER_IGNORE_SINKS exempts a name,
-# any casing.
+# any casing. The ${var,,} case folds below are bash 4.0+; they only ever
+# execute behind the Windows host gate above (Git Bash ships bash 5.x), so a
+# macOS bash 3.2 exits at the gate before reaching them - keep them below it.
 sink_names=" tmp "

Skip gate applies: no security-relevant surface in this delta (comment-only), and the prior review of the full functional diff (through 3b2b9d9) already returned "no security issues found" and still applies — the logic it covered (record_if_litter, the case-insensitive sink enumeration, DRIVE_ROOT_LITTER_IGNORE_SINKS handling) is unchanged by this commit.
· branch fix/drive-root-litter-sink-names

[claude]

Claude finished @kyle-sexton's task in 1m 28s —— View job

---

Code review: PR #3489 (/review:code-review, at c798f0c)

• Gather context (diff vs origin/main, PR description, prior review comments)
• Invoke /review:code-review and apply its skip gate
• Compare current head content against last-reviewed head
• Post result

Skip gate applied

git diff 1812742d21abf0371295a50a76ab32f3ed39ea87 c798f0c3971908002fa60b494ecd3cc21caf4235 -- scripts/check-drive-root-litter.sh scripts/check-drive-root-litter.test.sh docs/conventions/windows-path-emit/README.md returns empty — the three PR files are byte-identical to what this lane already fully reviewed at 1812742 (see the 00:28 UTC review pass above). This synchronize event rehashed the branch (rebase) without changing PR content, matched by identical file-level stats (+193/-28 across the same 3 files).

Per this lane's skip gate ("this head already has a successful review from this lane that still applies"), no new review is performed and no findings are re-posted.

Status carried over from the last full pass

• One open, non-blocking finding remains unaddressed across four review passes (Codex on 7afe36a; this lane on 3b2b9d9 and 1812742): scripts/check-drive-root-litter.test.sh#L186 — run() (lines 30–39) execs bash "$SUT" without clearing DRIVE_ROOT_LITTER_IGNORE_SINKS, so an operator with that documented opt-out exported in their shell would get silent false failures on the sink, sinkupper, sinkcheckout, and both assertions. Not re-posted as a new inline comment since it's identical to what's already on this PR — flagging again here only so it isn't lost in the thread.
• Everything else (temp-sink detection loop, record_if_litter() extraction, case-insensitive matching, DRIVE_ROOT_LITTER_IGNORE_SINKS opt-out scoping, README accuracy) checked out clean on the prior full passes and is unchanged.

No new findings. No inline comments posted this pass (nothing changed to anchor them to).
· branch fix/drive-root-litter-sink-names

[claude]

Claude finished @kyle-sexton's task in 1m 24s —— View job

---

Security review: PR #3489 (/review:security-review, at c798f0c)

• Gather context (diff since last-reviewed head)
• Invoke /review:security-review and follow its instructions
• Post findings

No security issues found.

The branch was rebased onto the current origin/main (a7b52cf, the repo-wide batch-simplify sweep) between the last review and this trigger, which changed the head SHA from 1812742 to c798f0c without changing this PR's content — git diff origin/main...HEAD on the current head is line-for-line identical to the diff already reviewed and cleared across four prior passes on this PR. Skip-gate condition 3 applies (a successful security review of this exact content already exists and still applies), so I re-verified rather than re-deriving from scratch:

• The temp-sink detection loop and the extracted record_if_litter() helper (scripts/check-drive-root-litter.sh#L115-L160) only probe the filesystem with -d/cd/pwd -P. drive and entry/name are filesystem-derived (enumerated, not attacker-supplied), and sink_names is a fixed literal (" tmp ").
• DRIVE_ROOT_LITTER_IGNORE_SINKS (L147-L149) only ever reaches a quoted [[ ... == *"$var"* ]] substring comparison — no eval, no command substitution, no glob/pattern injection path even from an adversarial directory name.
• No workflow files, secrets, network calls, or authorization logic anywhere in this diff — it remains a read-only, advisory-only local diagnostic scanner (per ADR 0003), unchanged in that respect since the first review.
• The doc update is prose-only.

Not a security finding, but noting for the record since a code-review lane may not be running on this trigger: the maintainability comment from prior passes (run() in scripts/check-drive-root-litter.test.sh doesn't clear DRIVE_ROOT_LITTER_IGNORE_SINKS before ordinary-detection assertions, so an inherited env value could flip several assertions to false failures) is still open as of this head — that's a test-isolation concern, not a security one, so it's out of scope for this lane.
· branch fix/drive-root-litter-sink-names

[claude]

Claude finished @kyle-sexton's task in 1m 17s —— View job

---

Code review: PR #3489 (/review:code-review, at 9929b5e)

• Gather context (diff since last-reviewed head c798f0c → 9929b5e)
• Invoke /review:code-review and apply its skip gate
• Review the new commit
• Post findings

Summary

The only commit since the last review (c798f0c) is 9929b5e, which touches only scripts/check-drive-root-litter.test.sh (+18/-4).

This commit fixes the open finding raised across four prior passes on this PR (Codex on 7afe36a; this lane on 3b2b9d9, 1812742, and carried forward at c798f0c): run() previously exec'd bash "$SUT" without clearing DRIVE_ROOT_LITTER_IGNORE_SINKS, so an operator's exported opt-out could silently flip sink-detection assertions to false failures.

I traced the fix against scripts/check-drive-root-litter.test.sh:

• run() (L38-L47) now explicitly sets DRIVE_ROOT_LITTER_IGNORE_SINKS='' on both branches (in addition to DRIVE_ROOT_LITTER_MOUNT_ROOT='' on the - branch, which wasn't set at all before). Confirmed against the SUT (scripts/check-drive-root-litter.sh#L147: ignored_sinks="${DRIVE_ROOT_LITTER_IGNORE_SINKS:-}") that empty and unset are equivalent for this seam, so the empty-string assignment is a correct substitute for clearing it.
• The two ad-hoc cwd invocations (sink-checkout at L183 and L252) that don't go through run() now also set it explicitly.
• The four opt-out-specific invocations (L220-L243) still set a live value per-call, as before — unaffected.
• The new export DRIVE_ROOT_LITTER_IGNORE_SINKS=tmp "poison" line (L53) is a reasonable regression guard: it makes any future invocation that forgets to clear the var fail loudly in CI. I confirmed every bash "$SUT" call site in the file (8 total, across run() and the 6 ad-hoc invocations) now sets the var explicitly, so the poison doesn't leak into any assertion it shouldn't.

This closes out the only outstanding item from prior passes. I did not find any new issues in this commit.

Not independently run (this lane has no general Bash execution — only git log/git diff, Read, Glob, Grep): the commit message's claimed 26/26-both-ways pass count. Verified by static trace instead, which corroborates the claimed behavior.

No findings to post as inline comments this pass.
· branch fix/drive-root-litter-sink-names

[claude]

Claude finished @kyle-sexton's task in 38s —— View job

---

Security review: PR #3489 (/review:security-review, at 9929b5e)

• Gather context (diff since last-reviewed head c798f0c → 9929b5e)
• Invoke /review:security-review and follow its instructions
• Post findings

No security issues found.

Skip gate applied

The only commit since the last-reviewed head (c798f0c) is 9929b5e, which touches a single file — scripts/check-drive-root-litter.test.sh — a test file only, no production code. Confirmed via git diff c798f0c 9929b5e --stat: 18 insertions / 4 deletions, one file.

The change makes run() (lines 30–41) and the two ad-hoc invocations (lines 183, 252) set DRIVE_ROOT_LITTER_IGNORE_SINKS='' explicitly for ordinary-detection assertions, and poisons the outer test-runner environment with export DRIVE_ROOT_LITTER_IGNORE_SINKS=tmp (line 44) as a standing regression guard. This is exactly the test-isolation fix requested across four prior review passes (Codex on 7afe36a; this lane on 3b2b9d9, 1812742, c798f0c) — it's now closed.

Skip-gate condition 2 applies here: no security-relevant surface in this delta. It's a test harness setting explicit env vars (all values are literal quoted strings, no eval, no untrusted input) before invoking the already-reviewed SUT (scripts/check-drive-root-litter.sh, unchanged in this commit). The production script's logic — record_if_litter(), case-insensitive sink enumeration, DRIVE_ROOT_LITTER_IGNORE_SINKS handling — was already cleared in prior passes and is untouched here.

No new findings.
· Job run · branch fix/drive-root-litter-sink-names