fix: rotation note misreports on bad max-bytes, awk -v mangles a backslash CLAUDE_PROJECT_DIR (#423, #424) - #425
Merged
Conversation
…ths misreported on two bad inputs (#423, #424) #423: session-start-hook.sh's size-watch note formatted a malformed JIT_CONTEXT_LOG_MAX_BYTES through `awk '{ printf "%.1f", $1 / 1000000 }'` with no validation, so a non-numeric, negative, or leading-zero value silently read as 0 (or -0) and the note claimed rotation was on ("rotates automatically past 0.0 MB") while jit_log_rotate() had actually refused the value and done nothing. The note now validates the value with the same case statement jit_log_rotate() uses and names the refused value instead. tests/test-log-rotation-406.sh's own A9 never reached this branch (its fixture log sits under jit-misses.sh's 10MB size-threshold default); C4/C5/C6 pad the log past that threshold to actually exercise it, red before the fix and green after. #424: pre-tool-hook.sh built tools_base/vocab_base in bash and passed them to the decisive awk as `-v` values -- but awk's -v processes backslash escapes in the value it receives, the same defect #402/#378 already fixed for JIT_WORKTREE_NOTE by routing it through ENVIRON instead. A CLAUDE_PROJECT_DIR containing a backslash escape sequence mangled JIT_BASE before any rule lookup ran, so the hook silently answered "{}" for a rule that fires correctly on the identical tree under a plain path. Both now read JIT_BASE via ENVIRON, already exported for exactly this reason (common.sh), the same way jit_transclude_resolve() already does. tests/test-pre-tool-hook.sh adds a new #424 section building two identical trees, one with a literal backslash in its path, and asserts they answer identically -- red before the fix (the backslash tree answered "{}"), green after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…sibling -v-escape site oss:auditor found that #423's own new note branch embedded the refused JIT_CONTEXT_LOG_MAX_BYTES value verbatim into the JSON systemMessage payload, with no escaping -- and that value never passed jit_load_config()'s validation (it can be exported straight into the environment), so a value containing a literal '"' broke the JSON structure outright. Escaped it the same way LOG_FILE is already escaped into JIT_LOG_ESC. New C7 test in tests/test-log-rotation-406.sh parses the hook's own stdout as JSON with python3 rather than grepping a substring, since a grep still finds the raw text even inside now-broken JSON -- red before this fix (JSONDecodeError), green after. Same auditor pass found that pre-tool-hook.sh's `home`/`project` awk variables were still built via `-v $HOME`/`-v ${CLAUDE_PROJECT_DIR:-.}`, the identical escape- processing defect #424 fixed for tools_base/vocab_base in the same file, on the exact variable (CLAUDE_PROJECT_DIR) the whole PR is about. Moved both onto ENVIRON too, for the same reason and by the same mechanism. New test in tests/test-pre-tool-hook.sh isolates $HOME (deliberately rooted outside $TEST_DIR, so CLAUDE_PROJECT_DIR's own unaffected strip cannot mask the result) and confirms the project-prefix strip survives a backslash-bearing $HOME -- red before this fix (the log tail kept the unstripped absolute path), green after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…423/#424 jit-misses.sh (generic-word file lookup) and rebuild-tsv.sh (layerdir/generic-word classify pass) both carry the identical CLAUDE_PROJECT_DIR-derived-path-through-awk- -v defect #424 fixed for pre-tool-hook.sh's tools_base/vocab_base -- found by Explore while reviewing that fix, not touched by it (different files, different subsystem, non-blocking per the ranking table's `fails-to-preserve`/`misreports` row). Logged to trap.d/ rather than filed, per that row's routing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
What changed
#423 --
scripts/session-start-hook.sh's log-rotation size-watch note formatted the rawJIT_CONTEXT_LOG_MAX_BYTESenv var (exportable directly, bypassingjit_load_config()'s own validation) through awk with no validation of its own, so a malformed value (abc,-5,010) silently formatted as0.0/-0.0MB and the note claimed rotation was on whilejit_log_rotate()(scripts/common.sh) had actually refused the value via its own case-statement validation and rotated nothing. Fixed by mirroring that same case-statement validation in the note branch; on a refused value it now says rotation did NOT run and names the (escaped) refused value.#424 --
scripts/pre-tool-hook.shbuilttools_base/vocab_baseawk variables by concatenating$JIT_BASEpaths in bash and passing them via-v, which processes backslash escapes -- so aCLAUDE_PROJECT_DIRcontaining a literal backslash (a Windows-shaped path) silently mangled the path before any rule lookup ran, and the hook answered{}for every rule undertools//vocabulary/, indistinguishable from "no rule matched". Same defect class as #402/#378. Fixed by removing the-vargs and building the identical strings inline viaENVIRON["JIT_BASE"]instead.Self-review round -- two fixes beyond the two issues' own literal scope
Both
Exploreandoss:auditorwere spawned against the first commit's diff. Two real defects the auditor found were fixed directly rather than filed, since both were low-blast-radius, same-file, same-mechanism, and pinnable with a red-first test:JIT_CONTEXT_LOG_MAX_BYTESvalue UNESCAPED into the JSONsystemMessagepayload -- a value containing a literal"broke the JSON outright (verified viapython3 json.loads). Escaped it the same way$LOG_FILEis already escaped two lines above.tools_base/vocab_base):scripts/pre-tool-hook.sh'shome/projectawk variables were STILL built via-v $HOME/-v ${CLAUDE_PROJECT_DIR:-.}-- the identical-vescape-processing defect, on the SAME variable (CLAUDE_PROJECT_DIR) this whole PR is about, in the SAME file. Verified empirically across one-true-awk/gawk/mawk that the corruption is present and INCONSISTENT across engines. Moved ontoENVIRONtoo.Both new tests are red-before/green-after; see the report for exact commands and counts.
Adjacent findings, not fixed here (logged to trap.d instead)
Explore found the identical
-v-escape defect class LIVE in two sibling files this PR never touches --scripts/jit-misses.sh(a real functional break: generic-word filtering silently degrades to "off" on a backslash-bearing path) andscripts/rebuild-tsv.sh(two reporting-only breaks in maintainer tooling). Ranked non-blocking (fails-to-preserve/misreports), logged astrap.d/424.jit-misses-generic-words-v-escape.mdandtrap.d/424.rebuild-tsv-v-escape.mdrather than fixed here -- different files, different subsystems, each needs its own fixture.Tests
tests/test-log-rotation-406.sh: 80/80 (new C4/C5/C6 for session-start-hook.sh's rotation note misreports when JIT_CONTEXT_LOG_MAX_BYTES is malformed #423, C7 for the JSON-escaping self-review fix)tests/test-pre-tool-hook.sh: 331/331 (new section for pre-tool-hook.sh silently answers {} when CLAUDE_PROJECT_DIR contains a backslash escape sequence #424's backslash path, plus the isolated-$HOME test for the home/project self-review fix)Full local suite (
bash tests/run-all.sh) intentionally not run this round -- CI covers three platforms and is the source of truth; only the two narrowed suites the diff touches were run, repeatedly, through red/green cycles.Tree-mutation receipt
could-not-compare: the before-snapshot (
scripts/tree_snapshot.py) was deleted by an unrelated scratch-file cleanup before the compare step ran. Corroborated instead by repeatedgit status --porcelainchecks through the session, which never showed an unintended change; both spawns carried explicit no-edit instructions.Closes #423, closes #424.
[AI-generated]