fix(hook-utils): stop treating the OS temp tree as project content

[kyle-sexton]

Summary

• Root cause. hook::read_file_path (lib/hook-utils.sh) decides project membership by
  prefix-matching CLAUDE_PROJECT_DIR. When the project directory is the user's home — the shape a
  session started outside any checkout takes — the OS temp root sits under it, so Claude Code's own
  per-session scratchpad passes the membership test. Every file-scoped hook that scopes on this guard
  (12 of them) then treats scratch files as project content. In the reported case typos-format
  autocorrected a shell variable in a throwaway script from typos' built-in dictionary, in a location
  with no repository and therefore no typos config to allow-list the token with, silently breaking
  the script. This is the failure the repository's own hook-precision rule 5 already names, and
  typos-format.sh's header wrongly claimed rule 5 was N/A for it.
• Fix (shared lib). New hook::under_temp_root in lib/hook-utils.sh; hook::read_file_path
  now rejects a file inside the OS temp tree when the project root is outside it. The exemption is
  deliberate and load-bearing: when the project root itself lives under temp — a mktemp -d fixture
  checkout, which is how this repository's own hook suites run — its files are still project content
  and are accepted. Temp roots come from TMPDIR / TMP / TEMP plus the POSIX defaults, resolved
  through the same hook::physical_path + hook::normalize_path pipeline the membership comparison
  already uses (both spellings matter: on Git Bash TMPDIR=/tmp while TEMP carries the Windows
  form of the identical directory, and realpath resolves one but not the other). Cost: every
  in-project file now pays one candidate-resolution pass — after dedup, about two realpath
  spawns — before the guard returns; the second call (on the project root) only runs when the file
  matched, so the common case pays one pass, not two.
• Two scope caveats, stated plainly. The gate lives inside the existing
  if [[ -n "${CLAUDE_PROJECT_DIR:-}" ]] branch, so a session with CLAUDE_PROJECT_DIR unset
  (some headless -p sessions, per bash-format's README) still processes temp-tree files — the
  reported case had it set, so this closes the report, but the temp tree is not universally excluded.
  And a Git worktree living under the OS temp root, reached from a home-shaped project dir, is now
  skipped by every file-scoped hook; a git-worktree escape hatch was considered and dropped as a
  second heuristic layer. Working inside such a worktree makes it the project root, which the
  temp-rooted exemption already covers, so the exposure is narrow — noted because Git worktrees are being created in system temp and dead-session scratchpad paths, leaving orphaned registrations #1774 concerns
  worktrees in system temp.
• Scope. Canonical source edited, scripts/sync-hook-utils.sh run: all 16 carrying plugins
  synced byte-identical, version-bumped, and changelogged. bash-format's README and setup SKILL
  state this guard's contract (they already claimed temp/scratchpad files were skipped — true only
  when the project dir was a checkout) and are corrected to state the refined rule.
• typos-format disclosure. The applied-rewrite guidance pointed only at
  extend-words / extend-identifiers. It now also names extend-ignore-re for a region quoted
  verbatim (a Markdown code fence, a transcript, a signature block), which typos'
  reference documents for exactly
  that case and supports across lines via (?s).
• Not fixed here, by design. The issue's defects 2 (correction applied to a token in identifier
  position) and 3 (correction applied inside a Markdown code fence) are typos-cli behavior, not
  hook behavior — declining by syntactic position would mean re-implementing typos' tokenizer. Both
  remain live inside a repository, where the consuming repo's typos config is the seam
  (extend-identifiers / extend-ignore-identifiers-re, extend-ignore-re). Suggested-fix item 4
  ("fail loudly rather than rewrite silently") is a plugin posture change, not a defect, and is left
  for a human decision; typos_format_write_changes = false already gives a report-only mode.
  Analysis is on the issue.

Test plan

Repro-first, per docs/conventions/hook-precision/README.md: each new stay-quiet case fails against
the unmodified guard and passes after. Run on Windows 11 / Git Bash.

• The real production shape, end to end — the committed hook driven against a file inside the
  actual harness session scratchpad (%LOCALAPPDATA%\Temp\claude\<project>\<session>\scratchpad\)
  with CLAUDE_PROJECT_DIR='C:\Users\<user>' and no temp-root overrides, so nothing about the
  fixture is synthetic:

  • pristine hook: this has the typo → this has the typo, hook reported REWROTE 1 word(s) —
    the reported defect, reproduced in place
  • fixed hook: file byte-identical afterward, hook stdout empty
  • This also proves the candidate list matches the backslash spelling the environment actually
    uses (TEMP=C:\Users\KYLESE~1\AppData\Local\Temp, while TMPDIR=/tmp resolves to a different
    spelling that realpath leaves alone). Test 12c pins that spelling with its own case.

• Black-box hook contract — plugins/typos-format/hooks/typos-format.test.sh, new Case 7b (a
  temp-tree file under a home-shaped project dir must be left untouched and silent):

  • before the sync, against the unmodified plugin copy: PASS=75 FAIL=2 — temp-tree file not silent (rc=0 out={"hookSpecificOutput"... "typos-format REWROTE 1 word(s) in inventory.txt"...})
    and temp-tree file was rewritten: this has the typo
  • after: PASS=77 FAIL=0

• Shared lib — lib/hook-utils.test.sh, new Test 12c (four cases: the regression, the
  temp-rooted-project exemption, an in-project control, and the Windows backslash spelling of the
  temp root). Driven against the pristine copy of the lib and then the fixed one:

  • before: FAIL: temp-tree file admitted under a non-temp project root (the exemption and control
    cases are non-regression guards, green either way)
  • after: all 4 pass
  • full suite: 4 runs of the fixed lib gave PASS=120..121 with 0–2 failures, all in the
    buffer_stdin timing class (trickle, stall overshoot, late-EOF). 5 runs of the pristine
    baseline on the same machine gave PASS=116..118 with 0–2 failures in the same class — that
    class is load-sensitive and pre-existing (fix(hook-utils): expand Windows 8.3 short names in the membership guard #1692's test plan documents it too). under_temp_root
    has no call path into buffer_stdin.

• Whole plugin corpus — scripts/run-plugin-tests.sh: all plugin contract tests pass.

• Gates — scripts/sync-hook-utils.sh --check (all 16 copies match),
  scripts/check-changelog-parity.sh --check, scripts/check-silent-skips.sh,
  scripts/check-shell-portability.sh --paths on the changed shell files, shellcheck --rcfile .shellcheckrc -x, shfmt -d — all clean.

Cleanup guidance for anyone affected

This change stops future rewrites; it cannot repair a file already corrupted. A scratch file written
during a session whose CLAUDE_PROJECT_DIR was a home directory may carry silent dictionary
corrections. There is no way to enumerate them after the fact — the hook's disclosure was emitted at
the time, in the transcript. Re-generate rather than trust any scratch script from such a session.

Related

• Follow-on to fix(hook-utils): expand Windows 8.3 short names in the membership guard #1692, which made short-form (KYLESE~1) scratchpad paths pass the membership guard;
  correct for in-project files, and it is what exposed this guard's over-broad membership predicate.
• docs/conventions/hook-precision/README.md rule 5 and its repro-first discipline.

Fixes #1769

🤖 Generated with Claude Code

https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C

[claude]

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

---

PR Review

• Gather context from PR description and changed files
• Review core fix in lib/hook-utils.sh
• Review test coverage in lib/hook-utils.test.sh
• Review typos-format hook and test changes
• Check plugin sync consistency
• Review documentation/README changes
• Compile and post findings

---

Summary

The fix is correct and the approach is sound. The hook::under_temp_root predicate correctly implements the intended logic; the exemption for temp-rooted fixture checkouts is load-bearing and preserved; the cross-platform candidate list (env vars + POSIX defaults, all canonicalized through the existing physical_path → normalize_path pipeline) handles the Windows backslash-form case documented in the test plan. Four findings below — one is a test correctness issue, the rest are minor.

---

Finding 1 — Test correctness: Case 7b is missing the HOME-under-temp guard

Severity: Medium (test failure on exotic CI; the production fix is unaffected)

plugins/typos-format/hooks/typos-format.test.sh, line 683–708

Test 12c in hook-utils.test.sh correctly guards before running its assertions:

if [[ -n "$PROJ12C" ]] && ! hook::under_temp_root "$(hook::normalize_path "$(hook::physical_path "$PROJ12C")")"; then

Case 7b only checks if [[ -n "$SCRATCH_HOME" ]]. On a CI host where $HOME is itself inside the OS temp tree (e.g. Docker with HOME=/tmp/...), mktemp -d "$HOME/.typos-format-test.XXXXXX" creates SCRATCH_HOME under /tmp. Then:

• CLAUDE_PROJECT_DIR="$SCRATCH_HOME" → under the real /tmp
• TMPDIR/TMP/TEMP="$SCRATCH_HOME/scratchpad" → also under /tmp
• Test file is under $SCRATCH_HOME/scratchpad → under /tmp

hook::read_file_path sees both the file and the project root under temp, so the exemption fires — the file is not skipped, and typos rewrites it. The assertion [[ "$(cat ...) == "$BEFORE_SP" ]] then fails, reporting a regression where none exists.

The fix is a one-line guard matching Test 12c:

if [[ -n "$SCRATCH_HOME" ]] && ! hook::under_temp_root "$(hook::normalize_path "$(hook::physical_path "$SCRATCH_HOME")")"; then

Fix this →

---

Finding 2 — Remediation guidance asymmetry: extend-ignore-re missing from user channel

Severity: Low (informational gap for users)

plugins/typos-format/hooks/typos-format.sh, line 394 vs. line 401

The CTX (agent channel) correctly names extend-ignore-re for verbatim-quoted regions:

…for a region quoted verbatim — a Markdown code fence, a transcript, a signature block — use an extend-ignore-re pattern instead…

But SYSMSG (user channel, the message the person whose file was rewritten sees) only says:

Add any wrong rewrite to extend-words / extend-identifiers in your typos config, or set the typos_format_write_changes option to false for report-only mode.

The user is the one who needs to act on the disclosure, and they get shorter guidance than the agent. This is the channel where extend-ignore-re matters most — the user is staring at an unexpectedly rewritten file. Brevity is reasonable, but omitting the verbatim-region escape hatch entirely makes the system message's remediation incomplete for one of the named failure modes.

---

Finding 3 — hook::under_temp_root argument convention is implicit

Severity: Low (documentation / future-proofing)

lib/hook-utils.sh, line 243–262

The function comment says <normalized-path> but does not document that the caller must pre-normalize. The candidates are normalized inside the function; the target is not. The current caller (hook::read_file_path) pre-normalizes correctly, but a future caller passing a raw path would get silent wrong results (a symlink target that resolves outside temp would be admitted). A one-line note in the comment suffices: # <target> must already be canonicalized by the caller (hook::normalize_path ∘ hook::physical_path).

---

Finding 4 — Pipe character edge case in dedup string

Severity: Informational (theoretical only)

lib/hook-utils.sh, line 248–250

The seen dedup uses *"|$cand|"* pattern matching. A temp directory path containing a literal | character would defeat the deduplicate check — both occurrences would resolve and be compared, just not deduped. In practice, operating system temp directories never contain |, and even without dedup the correctness of return 0 is unaffected (the function would just resolve the same candidate twice). Documenting this residual is sufficient; no code change needed.

---

Other observations (non-blocking)

• /var/tmp path: This hardcoded POSIX default is iterated but never directly tested — the test fixture uses env-var overrides to point the candidates at a synthetic scratchpad. This is acceptable; creating test artifacts under the real /var/tmp is not portable in CI.
• Plugin sync: The 16 plugin copies are declared byte-identical via scripts/sync-hook-utils.sh --check. The CHANGELOG entries, version bumps, and README corrections are consistent across the changed plugins.
• Rule 5 attribution in typos-format.sh: The header's correction from "N/A" to the actual rule-5 clause is accurate and matches the implementation.
• bash-format README and SKILL.md: The updated scope language precisely describes the refined rule — including the temp-root exemption and the unset-CLAUDE_PROJECT_DIR caveat. No gaps found.

---

Overall: The core fix is correct; no changes are required to merge. Finding 1 is the most practical: it would cause a spurious test failure on Docker-based CI with a non-standard $HOME. Findings 2 and 3 are quality improvements worth considering in a follow-up.

[claude]

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

---

Security Review — PR #1812

• Gather context
• Read core changes (lib/hook-utils.sh, hook-utils.test.sh)
• Read typos-format.sh and test changes
• Analyze for injection surfaces, logic flaws, trust boundary issues
• Post findings

---

Summary: No critical or important security issues found. This PR is a targeted, defense-in-depth improvement. One low-impact edge case is noted below.

---

Findings

SUGGESTION — Pipe character in seen dedup sentinel can cause a false-duplicate skip | Confidence: low

File: lib/hook-utils.sh lines 248–252

hook::under_temp_root deduplicates candidates with a pipe-delimited sentinel:

case "$seen" in
*"|$cand|"*) continue ;;
esac
seen="$seen|$cand|"

$cand is the raw value of TMPDIR, TMP, or TEMP. On Linux, | is a legal character in directory names, so if one of these env vars contains a | (e.g. TMPDIR=/tmp/a|b), the seen string becomes |/tmp/a|b|. A subsequent candidate /tmp/a then hits the pattern *"|/tmp/a|"* — which matches — and is silently skipped as a false duplicate. The affected candidate is never resolved or compared, meaning files under it would not be recognized as temp-tree files and could pass through the hook guard.

In practice this requires TMPDIR/TMP/TEMP to contain a | — unusual for a temp-directory path, but not impossible on POSIX hosts. Exploiting it also requires environment-variable control in the Claude Code session, which already implies broader access than this guard provides. The impact is a partial relaxation back toward the pre-fix behavior for that specific temp root, not a privilege escalation.

A simple fix would be to key the dedup on the resolved $norm rather than the raw $cand, or use a null-byte separator (which can't appear in bash string values). The raw-$cand approach does catch the trivially common case (same env var string appearing twice), but misses cross-form aliases anyway (e.g. TMPDIR=/tmp and the literal /tmp candidate are not deduped, so they both resolve — harmlessly, but intentionally as noted in the comment).

---

Not-findings (rationale)

• Command injection via $cand — hook::physical_path calls realpath -- "$1" with -- to stop option processing, and all expansions are double-quoted throughout. No injection surface.
• Symlink escape — Both the file path and the temp-root candidates are passed through hook::physical_path (which calls realpath), so a symlink pointing from the project tree into the temp tree is resolved before the comparison. The fix is symlink-safe.
• Glob metacharacters in [[ ]] — [[ "$target" == "$norm"/* ]] treats the RHS as a glob. $norm is realpath output and cannot contain *, ?, or [ in practice.
• TMPDIR/TMP/TEMP set to a non-existent path — The [[ -d "$cand" ]] guard prevents non-existent candidates from being compared.
• TMPDIR pointed at the project directory to disable the guard — This causes hook::under_temp_root "$norm_project" to return true, disabling the new guard for that project. But this returns behavior to the pre-fix state (not worse), and requires session-level environment control. It's within the stated caveats in the PR body.
• No new trust boundaries or network egress — The function reads only filesystem metadata and three well-known environment variables. No new external data sources.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 942bf24566

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

[claude]

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

---

PR Review — post-synchronize pass

• Read style commit (12a1ef5) — comment hygiene only, no logic change
• Read core fix in lib/hook-utils.sh — hook::under_temp_root + guard in hook::read_file_path
• Read lib/hook-utils.test.sh — Test 12c and its ! hook::under_temp_root guard
• Read typos-format.test.sh Case 7b — checked guard vs. Test 12c
• Confirmed Codex P2 finding (trailing slash on /)
• Checked status of all prior review findings

---

What the synchronize commit adds

The latest commit (12a1ef5 style(hook-utils): drop a Windows user path from a test comment) is correct and appropriate. It replaces a literal C:\Users\...\Temp path in a comment in lib/hook-utils.test.sh with a portable description ("a drive-letter path with backslash separators"), which silences the machine-specific-paths hygiene check. No logic changes.

---

Finding still open from prior review — actionable

Case 7b missing the HOME-under-temp guard · Medium

plugins/typos-format/hooks/typos-format.test.sh, line 687

lib/hook-utils.test.sh Test 12c correctly skips itself when $HOME is inside the OS temp tree:

# lib/hook-utils.test.sh line 529
if [[ -n "$PROJ12C" ]] && ! hook::under_temp_root "$(hook::normalize_path "$(hook::physical_path "$PROJ12C")")"; then

Case 7b in typos-format.test.sh does not apply the same guard:

# typos-format.test.sh line 687 — guard as-is
if [[ -n "$SCRATCH_HOME" ]]; then

On a Docker CI host where $HOME is under /tmp (a common configuration), SCRATCH_HOME=$(mktemp -d "$HOME/.typos-format-test.XXXXXX") lands under /tmp. At that point CLAUDE_PROJECT_DIR="$SCRATCH_HOME" is a temp-rooted project dir, so hook::under_temp_root "$norm_project" returns true — the exemption fires, the file is not skipped, typos rewrites it, and the assertion [[ "$(cat …)" == "$BEFORE_SP" ]] fails. This would be reported as a regression where none exists.

The fix is a one-line change matching Test 12c:

if [[ -n "$SCRATCH_HOME" ]] && ! hook::under_temp_root "$(hook::normalize_path "$(hook::physical_path "$SCRATCH_HOME")")"; then

Note that the else branch must also be updated to emit a SKIP message instead of the ok pass it currently prints — Case 7b's current else branch says "temp-tree scoping SKIPPED (no writable HOME…)" which is the right text; only the guard condition needs updating.

Fix this →

---

Codex P2 finding — confirmed, low impact

TMPDIR=/ silently dropped · Low

lib/hook-utils.sh, line 254–255

norm="${norm%/}"
[[ -n "$norm" ]] || continue

If TMPDIR=/, realpath / returns /, the trailing-slash strip yields "", and [[ -n "$norm" ]] || continue drops it from the candidate list. Files under / would then only match the POSIX hardcoded candidates (/tmp, /var/tmp). TMPDIR=/ is a misconfiguration (everything is temp), and the POSIX defaults still cover the practically relevant paths — this is not a blocker. A brief comment acknowledging the edge case is sufficient; no logic change needed.

---

Other findings from prior review — unchanged

• Finding 2 (Low): SYSMSG in typos-format.sh at line 402 still omits extend-ignore-re from the user-facing remediation text. CTX (agent channel, line 394) mentions it correctly. Low priority; the user can find it from the typos reference link, but completeness favors adding it here too.
• Finding 3 (Low): hook::under_temp_root's comment at line 232 doesn't explicitly document that <target> must already be canonicalized by the caller. A one-line note (e.g. # <target> must already be canonical: hook::normalize_path ∘ hook::physical_path) would protect future callers.
• Finding 4 (Informational): Pipe-char dedup edge case — acknowledged, no change needed.

---

Summary

The core fix is correct; the style commit is a clean hygiene pass. One actionable item remains: Case 7b in typos-format.test.sh needs the same ! hook::under_temp_root guard that Test 12c already uses, or it will produce a spurious CI failure on any host where $HOME sits under /tmp. Everything else is low-priority or informational.

[claude]

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

---

Security Review — PR #1812

• Read core changes (lib/hook-utils.sh, hook-utils.test.sh)
• Read typos-format.sh and test changes
• Analyze injection surfaces, logic flaws, trust boundary issues
• Post findings

---

No CRITICAL or IMPORTANT security findings. The new hook::under_temp_root function and its integration into hook::read_file_path are sound. Two suggestions are noted below; both confirm findings already raised in the prior security review.

---

SUGGESTION — norm="${norm%/}" silently drops a candidate when TMPDIR resolves to / | Confidence: low

File: lib/hook-utils.sh line 254

norm="${norm%/}"
[[ -n "$norm" ]] || continue

norm%/ strips a single trailing slash from the resolved candidate. When a TMPDIR/TMP/TEMP value resolves (via realpath) to the filesystem root /, this produces an empty string that the [[ -n "$norm" ]] guard then silently drops. The hardcoded /tmp and /var/tmp candidates still cover the standard POSIX temp trees, so common configurations are unaffected. The exposure is narrow: a host where the actual session temp dir is neither under /tmp nor /var/tmp, the relevant TMPDIR/TMP/TEMP env var resolves to /, and CLAUDE_PROJECT_DIR sits outside the temp tree. That combination requires a misconfigured environment (TMPDIR=/ is meaningless) and session-level env var control, which already implies broader access than this guard provides. Confirms the Codex review comment on line 255.

---

SUGGESTION — case "$seen" in *"|$cand|"* interprets raw $cand as a glob | Confidence: low

File: lib/hook-utils.sh lines 248–249

case "$seen" in
*"|$cand|"*) continue ;;

In bash case patterns, variable expansions are subject to glob interpretation: [, ], *, and ? in the expanded value of $cand act as pattern metacharacters. If TMPDIR were set to a path like /tmp/test[1] (a valid POSIX directory name), the dedup pattern would become *"|/tmp/test[1]|"*, matching |/tmp/test1| rather than the literal bracket form. The practical effect is incorrect deduplication — a candidate either being checked twice (efficiency, benign) or a distinct candidate being mistakenly skipped (unlikely but causes a double-resolve on a different candidate). The correctness of return 0 is unaffected either way: even a doubly-resolved candidate produces a correct comparison result. Requires TMPDIR/TMP/TEMP to contain glob metacharacters, which is unusual for operating-system temp directories. Confirms the finding in the prior security review.

---

Not-findings (rationale)

• Command injection via $cand — hook::physical_path calls realpath -- "$1" (the -- blocks option injection) and all argument expansions are double-quoted throughout. No injection surface.
• Symlink escape — Both the file path and each temp-root candidate pass through hook::physical_path (realpath), so a symlink pointing from the project tree into the temp tree is resolved before comparison. The guard is symlink-safe.
• [[ "$target" == "$norm"/* ]] RHS glob — $norm is the output of realpath, which cannot produce *, ?, or [ in practice. The failure direction is conservative (over-rejection, not under-rejection), so safety is preserved even if glob chars somehow appeared.
• TMPDIR set to the project directory — causes hook::under_temp_root "$norm_project" to return true, disabling the new guard for that project. This reverts to pre-fix behavior, not worse. Requires session-level env-var control.
• CLAUDE_PROJECT_DIR unset — acknowledged in the PR body as a stated caveat; the gate sits inside the existing if [[ -n "${CLAUDE_PROJECT_DIR:-}" ]] branch, so headless sessions without it are out of scope for this fix, not regressed.
• No new trust boundaries or network egress — the function reads filesystem metadata and three well-known env vars only. No external data sources, no credential handling, no PII.

[claude]

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

---

Security Review — PR #1812 (post-synchronize)

• Diff against origin/main (confirms the file set matches the PR description; the merge of 78f0040 brought in no changes overlapping this PR's scope)
• Review the delta since the last full security pass — commit fd6eab7 (root-temp-candidate fix) and the mechanical 12a1ef5 style commit
• Verify all 16 plugin hook-utils.sh copies stay byte-identical to lib/hook-utils.sh after the new commit
• Re-confirm prior findings still hold at HEAD (cd2bead)

---

No CRITICAL or IMPORTANT security findings. Everything reachable since the last full review (commits 12a1ef5, fd6eab7, and the merge cd2bead) has been covered; nothing new changes the risk posture.

---

New since the last pass

fd6eab7 — root (/) as a temp candidate now short-circuits correctly

lib/hook-utils.sh#L254-L262

norm=$(hook::normalize_path "$(hook::physical_path "$cand")")
[[ "$norm" == / ]] && return 0
norm="${norm%/}"
[[ -n "$norm" ]] || continue

This resolves the Codex P2 finding from the prior review pass (TMPDIR=/ used to trim to an empty string and get silently dropped, under-recognizing temp-tree files). The fix is correct: a temp candidate of / legitimately contains every absolute path, so short-circuiting to return 0 is the right verdict, not an approximation.

One consequence worth naming explicitly (not a new vulnerability, just tracing the logic through): if TMPDIR=/, then hook::under_temp_root also returns true for the project root itself, which fires the temp-rooted-project exemption (lib/hook-utils.sh#L302) and disables the new guard entirely — every file looks "temp-rooted," including the project. This is the same class already dispositioned in the prior review ("TMPDIR pointed at the project directory to disable the guard"): it degrades back to pre-fix behavior, not below it, and requires session-level TMPDIR control, which already implies broader access than this guard defends against. TMPDIR=/ is also a degenerate misconfiguration on any real host. No action needed.

The accompanying regression test (Test 12d in lib/hook-utils.test.sh) correctly scopes the override to a subshell (SC2030/SC2031 disabled deliberately, with a comment explaining why), so it can't leak TMPDIR=/ into the rest of the suite.

Sync integrity check — md5sum across lib/hook-utils.sh and all 16 plugins/*/hooks/hook-utils.sh copies at HEAD confirms they're byte-identical (e40b8e39c219ba670607c211d699931d everywhere). No plugin is left running the pre-fix logic while others have the patch — an inconsistency here would itself have been a security-relevant gap, since a hook is only as safe as its copy.

Merge commit cd2bead — merges 78f0040 (fix(repo-fleet-hygiene): ...) from main. git diff origin/main...HEAD shows the exact same 38-file set as the PR description with no unrelated content pulled in; the merge introduced no new surface for this review.

---

Carried forward from prior passes (unchanged, still low-severity)

• case "$seen" in *"|$cand|"*) dedups on the raw, unnormalized $cand — a path containing | or glob metacharacters ([, *, ?) could defeat the dedup or hit unintended matches. Effect is at most a redundant resolve/compare, not a wrong verdict — return 0's correctness doesn't depend on dedup succeeding.
• hook::under_temp_root's doc comment doesn't state that <target> must already be canonicalized by the caller — informational, the one current caller (hook::read_file_path) does this correctly.
• typos-format.sh's SYSMSG (user-facing channel) still omits extend-ignore-re from remediation guidance that CTX (agent channel) includes — a completeness gap, not a security issue.

Not-findings (reconfirmed at HEAD)

• No command injection: realpath -- "$1" blocks option-injection via --, all expansions quoted throughout.
• Symlink-safe: both sides of every comparison go through hook::physical_path → realpath before comparison.
• No new trust boundaries, secrets handling, or network egress — the function reads only filesystem metadata and three well-known env vars.

[claude]

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

---

Security Review — PR #1812 (post-synchronize, commit 8ef265f)

• Diff the new commit (8ef265f) against the previously-reviewed HEAD (cd2bead)
• Confirm it touches no security-relevant code
• Post findings

---

No CRITICAL or IMPORTANT security findings. The new commit does not change any risk posture.

What changed

8ef265f chore(plugins): bump the 16 hook-utils carriers past the versions main shipped touches only:

• plugins/*/.claude-plugin/plugin.json — a version-string bump (e.g. 0.7.6 → 0.7.7) in each of the 16 carrying plugins, needed because #1785 landed the same version numbers on main first, making this branch's lib/hook-utils.sh change version-invisible to consumers.
• plugins/*/CHANGELOG.md — a corresponding changelog entry per plugin.

No hook-utils.sh copy, no hook logic, no workflow file, and no permissions/config surface changed in this commit. git show 8ef265f --stat confirms the file set is limited to plugin.json and CHANGELOG.md pairs across the same 16 plugins already covered in prior passes.

Carried forward from prior reviews (unchanged, still low-severity, no new action)

• case "$seen" in *"|$cand|"*) in lib/hook-utils.sh dedups on the raw, unnormalized $cand — a temp-root env var containing | or glob metacharacters ([, *, ?) could defeat the dedup or hit an unintended match. Effect is at most a redundant resolve/compare; return 0's correctness doesn't depend on dedup succeeding.
• hook::under_temp_root's doc comment doesn't explicitly state <target> must already be canonicalized by the caller — informational; the one current caller (hook::read_file_path) does this correctly.
• typos-format.sh's SYSMSG (user-facing channel) omits extend-ignore-re from remediation guidance that CTX (agent channel) includes — a completeness gap, not a security issue.

Not-findings (reconfirmed)

• No command injection: realpath -- "$1" blocks option-injection via --, all expansions quoted throughout.
• Symlink-safe: both sides of every comparison go through hook::physical_path → realpath before comparison.
• No new trust boundaries, secrets handling, or network egress anywhere in this PR's changes.
• All 16 plugin hook-utils.sh copies remain byte-identical to lib/hook-utils.sh (unchanged by this commit; last verified at cd2bead).

Summary

This commit is a pure version/changelog bump with no code or config surface change. No new security review is warranted beyond the prior pass at cd2bead; all previously-identified items remain low-severity and non-blocking.

Branch: fix/hook-utils-scratchpad-scope-1769

[claude]

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

---

PR Review (code-review lane) — post-synchronize pass

• Diff since last full review: only new commit is 8ef265f (version-bump/changelog only)
• Verify 8ef265f version bump correctness across all 16 carrier plugins
• Re-check previously-flagged findings against current HEAD
• Spot-check test coverage added by fd6eab7 (Test 12d)
• Compile findings

---

What changed since the last full pass

The only new commit is 8ef265f chore(plugins): bump the 16 hook-utils carriers past the versions main shipped. It's a mechanical changelog/version fix: main independently shipped the same version numbers via #1785's carrier bump, so this branch re-bumps each of the 16 plugins one patch version further and splits the previously-merged changelog entry back into two (the new temp-tree fix at the new version, the pre-existing #1503 fix retained at the old one).

I verified this commit directly rather than trusting the description:

• All 16 changed plugin.json versions match their CHANGELOG.md top entry (e.g. bash-format → 0.6.9, source-control → 0.42.2, typos-format → 0.4.4, etc. — checked all 16).
• Changelog version ordering is descending with no duplicates in every touched file (spot-checked bash-format and actionlint in full, headers in the rest).
• All 16 plugins/*/hooks/hook-utils.sh copies remain byte-identical to lib/hook-utils.sh (diffed directly).

No correctness issues in this commit.

---

Status of prior findings at HEAD (8ef265f)

Re-read the actual files rather than assuming the version bump touched them — it didn't (git diff cd2bead..8ef265f touches only plugin.json/CHANGELOG.md files).

• Still open — Finding 1 (Medium): Case 7b missing the HOME-under-temp guard. plugins/typos-format/hooks/typos-format.test.sh#L687 still reads if [[ -n "$SCRATCH_HOME" ]]; then, with no ! hook::under_temp_root ... guard, while lib/hook-utils.test.sh Test 12c (line 529) applies exactly that guard. On any CI host where $HOME is itself under the OS temp tree (common in containerized runners), SCRATCH_HOME lands under temp, the temp-rooted-project exemption fires, typos rewrites the file, and Case 7b fails with a false "regression." This is a real portability gap in the test suite, not yet addressed.
• Still open — Finding 2 (Low): SYSMSG remediation asymmetry. plugins/typos-format/hooks/typos-format.sh#L402 (user-facing channel) still omits extend-ignore-re for verbatim-quoted regions, which the CTX agent-facing channel at line 394 already names. Informational-gap only.
• Still open — Finding 3 (Low): hook::under_temp_root doc comment doesn't state the caller must pre-normalize <target>. lib/hook-utils.sh#L232. No functional risk today (the sole caller normalizes correctly); worth a one-line comment for future callers.
• Resolved since the last pass — root-candidate handling (fd6eab7). lib/hook-utils.sh#L254-L262 now short-circuits TMPDIR=/ correctly instead of trimming it to an empty (and discarded) candidate. Test 12d (lib/hook-utils.test.sh#L589-L601) covers it, and correctly scopes the TMPDIR=/ override to a subshell with a shellcheck disable=SC2030,SC2031 comment explaining why — good test hygiene, doesn't leak into the rest of the suite.

---

Summary

Architecture and error handling in hook::under_temp_root / hook::read_file_path remain sound; test coverage is strong overall (Test 12c/12d are well-isolated). The one actionable maintainability gap carried forward from earlier passes is Finding 1 — Case 7b's guard doesn't match the pattern its own sibling test (12c) already established for the identical hazard, so it can produce a false failure on HOME-under-temp CI hosts. Recommend applying the one-line fix before merge; Findings 2 and 3 are fine to leave for a follow-up.

Fix this →

[github-actions]

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

[claude]

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

---

Security Review — PR #1812 (post-synchronize, commit 3c384b8)

• Diff the new commit (3c384b8) against the previously-reviewed HEAD (8ef265f)
• Confirm it touches no security-relevant code
• Post findings

---

No CRITICAL or IMPORTANT security findings. The new commit does not change any risk posture.

What changed

3c384b8 style(hook-utils): widen the Test 12c subshell directive to SC2030 touches only lib/hook-utils.test.sh, one line:

-        # shellcheck disable=SC2031 # subshell-local by design
+        # shellcheck disable=SC2030,SC2031 # subshell-local by design
         export CLAUDE_PROJECT_DIR="$PROJ12C" TMP="$win_tmp" TEMP="$win_tmp"
         unset TMPDIR

This widens a ShellCheck suppression comment on Test 12c's deliberate subshell-local export/unset (Test 12d's sibling subshell changed ShellCheck's dataflow analysis so the same pattern now also trips SC2030). No executable logic changed — the export/unset/hook::read_file_path call sequence is byte-identical; only the disable directive's scope widened. No hook-utils.sh copy, hook logic, workflow file, or permissions/config surface is touched.

Carried forward from prior reviews (unchanged, still low-severity, no new action)

• case "$seen" in *"|$cand|"*) in lib/hook-utils.sh dedups on the raw, unnormalized $cand — a temp-root env var containing | or glob metacharacters ([, *, ?) could defeat the dedup or hit an unintended match. Effect is at most a redundant resolve/compare; return 0's correctness doesn't depend on dedup succeeding.
• hook::under_temp_root's doc comment doesn't explicitly state <target> must already be canonicalized by the caller — informational; the one current caller (hook::read_file_path) does this correctly.
• typos-format.sh's SYSMSG (user-facing channel) omits extend-ignore-re from remediation guidance that CTX (agent channel) includes — a completeness gap, not a security issue.

Not-findings (reconfirmed)

• No command injection: realpath -- "$1" blocks option-injection via --, all expansions quoted throughout.
• Symlink-safe: both sides of every comparison go through hook::physical_path → realpath before comparison.
• No new trust boundaries, secrets handling, or network egress anywhere in this PR's changes.
• All 16 plugin hook-utils.sh copies remain byte-identical to lib/hook-utils.sh (unchanged by this commit; last verified at cd2bead).

Summary

This commit is a pure ShellCheck-directive fix in a test file with no behavioral or security-relevant change. No new security review is warranted beyond the prior pass at 8ef265f; all previously-identified items remain low-severity and non-blocking.

Branch: fix/hook-utils-scratchpad-scope-1769

[claude]

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

---

PR Review (code-review lane) — post-synchronize pass

• Diff since last full code-review pass (last covered 8ef265f; new commit is 3c384b8)
• Review the new commit for correctness/maintainability
• Re-check status of previously open findings (Case 7b guard, SYSMSG asymmetry, doc comment)
• Post findings

---

What changed since the last pass

The only new commit is 3c384b8 style(hook-utils): widen the Test 12c subshell directive to SC2030, touching a single line in lib/hook-utils.test.sh#L569:

-        # shellcheck disable=SC2031 # subshell-local by design
+        # shellcheck disable=SC2030,SC2031 # subshell-local by design
         export CLAUDE_PROJECT_DIR="$PROJ12C" TMP="$win_tmp" TEMP="$win_tmp"
         unset TMPDIR

This is correct and expected: Test 12d's sibling subshell (added in fd6eab7) changed ShellCheck's dataflow analysis so the same deliberate subshell-local export/unset pattern in Test 12c now also trips SC2030 ("var is modified in this subshell but the change is only local"), in addition to the pre-existing SC2031 suppression. Widening the directive to cover both is the right fix — the underlying export/unset/hook::read_file_path call sequence is byte-identical, this only silences a lint false-positive on an intentional pattern that's already commented as such. No behavioral or architectural change.

---

Status of previously-flagged findings at HEAD (3c384b8)

Re-verified directly against the current files (not assumed from prior passes):

• Still open — Case 7b missing the HOME-under-temp guard (Medium). plugins/typos-format/hooks/typos-format.test.sh#L687 still reads if [[ -n "$SCRATCH_HOME" ]]; then, with no ! hook::under_temp_root ... guard, while lib/hook-utils.test.sh Test 12c applies exactly that guard for the identical hazard. On a CI host where $HOME is itself under the OS temp tree, SCRATCH_HOME lands under temp, the temp-rooted-project exemption fires, typos rewrites the fixture file, and Case 7b fails with a false "regression." This is the one actionable, unresolved item from earlier passes and is unaffected by this commit.

  Fix this →

• Still open — SYSMSG remediation asymmetry (Low). plugins/typos-format/hooks/typos-format.sh#L402 (user-facing channel) still omits extend-ignore-re for verbatim-quoted regions, which the CTX agent-facing channel at line 394 already names. Informational gap, fine as a follow-up.

• Still open — hook::under_temp_root doc comment doesn't state pre-normalization contract (Low). lib/hook-utils.sh#L232. No functional risk today; the sole caller (hook::read_file_path) normalizes correctly.

No new maintainability or test-coverage issues introduced by 3c384b8.

---

Summary

3c384b8 is a clean, correct ShellCheck-directive widening with no behavioral change — nothing to flag. The single actionable item carried forward from earlier passes remains Case 7b's missing ! hook::under_temp_root guard, which would produce a spurious test failure on any CI host where $HOME sits under the OS temp tree. Recommend applying that one-line fix before merge; the SYSMSG and doc-comment items are fine to leave for a follow-up.

Branch: fix/hook-utils-scratchpad-scope-1769