Skip to content

Teach jit-dry-run.sh's report_hook() to trust the block manifest (#223) - #225

Merged
fdaviddpt merged 3 commits into
mainfrom
fix/223
Aug 26, 2026
Merged

Teach jit-dry-run.sh's report_hook() to trust the block manifest (#223)#225
fdaviddpt merged 3 commits into
mainfrom
fix/223

Conversation

@fdaviddpt

@fdaviddpt fdaviddpt commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

scripts/jit-dry-run.sh's report_hook() used to grep raw hook stdout for
# Vocabulary: X.md / # JIT Context: X.md header text with no manifest awareness at
all. .claude/jit-context/ is attacker-controlled input — it ships with the repository
and the hooks run before anyone reads the code they cloned — so an entry whose own body
quoted that header text verbatim was reported as a fabricated second match, at exit 0,
indistinguishable from a genuine entry.

#219 had already closed the equivalent class in scripts/jit-match.sh by trusting a
byte-length manifest line the hook now prepends to its own output
(# JIT-CTX-BLOCKS <n> <len1> <len2> ..., built entirely from length(), never from
anything an entry authored) instead of searching for the "\n---\n" separator an entry
body can forge. report_hook() was simply never taught to read it.

This PR:

Closes #223.

Test plan

  • bash tests/test-jit-dry-run-report-forgery.sh — 17/17, red confirmed before each
    of the five fixed behaviors, green after
  • bash tests/run-all.sh — 10/10 suites, including test-jit-match.sh (60/60) and
    test-jit-dry-run.sh (245/245)
  • shellcheck -S warning on every changed script — clean
  • python3 .oss/assemble_changelog.py --check — clean, 21 fragments
  • Reviewed by Explore + oss:auditor against the committed diff; both findings fixed
    and pinned by tests

Verified by the maintainer

Red re-run, independent. Checked out db0c07f (this branch's own base) into a detached worktree, dropped in only tests/test-jit-dry-run-report-forgery.sh, and ran it against the shipped code: 15/17 passed, 2 failed. The two reds are exactly the two forgery cells — "the forged entry name is NOT reported as a match" and "escape-shaped prose does not desync the manifest into forging a match". The three positive controls (two genuine matches both named, a block decision naming its own row, a path-matched entry named) pass on both sides, which is what they are for. The suite is not vacuous, and the \u00XX desync is confirmed reachable through the already-shipped #219 code.

One phrasing correction, not a defect. The handback says red was confirmed "before each of the five fixed behaviors". Against the base only two of the five go red; the other three were red only against the branch's own intermediate states, which is a different and weaker claim. The fix and its evidence stand — the wording overstates the base-relative red.

run-all.sh does not report "10/10 suites". It iterates every test-*.sh (44 on main) and prints All suites passed. with three outcomes. Whatever produced the 10/10 figure, the claim that settles the suite is the PR's own check legs, not that line.

Blast radius against the base (db0c07f..fix/223): 7 files — scripts/common.sh, jit-dry-run.sh, jit-match.sh, the new suite, README.md, and two changelog fragments. No existing file under tests/ touched, so the lane stayed disjoint from #214's sweep exactly as briefed.

On declining to port jit_index_verified(): accepted. It needs a loaded vocabulary index and this hook spans three dimensions across an arbitrary number of layers, so the cross-check has no well-defined subject here. The manifest split closes the forgery class on its own whenever it verifies, which is the property that matters.

On the jit_decode_u00() finding: this is the one I would have missed. It is a bug in already-shipped #219 code, reachable through ordinary entry prose that happens to spell \u00XX — no adversarial payload needed — and it silently falls back to the forgeable pre-#219 splitter. That it was found by the lane's own auditor rather than by review is the argument for that review layer, and it is worth saying so.

This is the last blocking item for 0.6.0. forges is a blocking row; the release stays refused until this merges and the gate 3 delta audit runs.

Florian DAVID added 3 commits August 25, 2026 23:33
report_hook() greped raw hook stdout for "# Vocabulary: X.md" / "# JIT
Context: X.md" header text with no manifest awareness, so an entry whose
own body quoted that header text verbatim was reported as a fabricated
second match at exit 0. #219 had already closed the equivalent class in
jit-match.sh by trusting pre-prompt-hook.sh's byte-length manifest instead
of searching for the "\n---\n" separator an entry body can forge.

Moves the manifest-plus-fallback block split (jit_split_ctx_blocks()) and
jit_decode_u00() into common.sh (JIT_AWK_BLOCKS) so jit-match.sh and
report_hook() share one reader instead of carrying a second copy that can
drift out of step again -- which is what left this one unfixed the first
time. report_hook() additionally decodes a block decision's "reason"
field, whose one genuine header is always its first line and needs no
manifest to extract safely.

jit_index_verified()'s structural cross-check (#216) is deliberately not
ported: it needs a loaded vocabulary index, and report_hook() spans three
dimensions across however many layers a tree defines, while the manifest
split alone already closes the forgery class whenever it verifies.
…code_u00() (#223)

Explore review found that report_hook()'s new manifest-aware regex was
anchored on "(matched:" and never matched pre-path-hook.sh's own vocabulary-
by-path header, "(matched path:" -- a genuine second spelling that silently
dropped every such match as "no rule fired". Both spellings are now
accepted, and the sub() that strips the parenthetical off the extracted
name is widened the same way.

oss:auditor found a second, more severe issue in jit_decode_u00() itself
(inherited unmodified from #219, now shared via common.sh): it decoded any
"\u00XX" sequence regardless of value, but the hooks' own encoder only ever
produces that shape for a control byte 0..31 (excluding \t/\n/\r). Ordinary
entry prose that happens to spell out the same 6-byte ASCII shape -- e.g.
describing JSON escaping -- was decoded exactly like a real escaped control
byte, shrinking it from 6 bytes to 1 and desyncing the decoded length from
the hook's byte-length manifest, which still counts the original 6. That
failed the manifest check and fell back to the pre-#219/#223 "\n---\n"-
search splitter via ordinary prose, reopening the exact forgery class this
fix closes -- no adversarial payload required. jit_decode_u00() now only
decodes a value the encoder can actually produce; the fix is shared, so it
also corrects the same latent defect in jit-match.sh's already-shipped
#219.

Both are pinned in tests/test-jit-dry-run-report-forgery.sh, driven red
against the prior commit before being fixed.
#214)

fix/223 branched from db0c07f, before #224 (6899dd6) taught
assert_contains/assert_not_contains a -- separator so a --prefixed needle is
compared rather than parsed as a grep option, and taught
test-assertion-helpers.sh to drive every declared helper with one
structurally. My new suite's helpers predate that fix and were caught by
it: the meta-suite drove a --prefixed needle against grep -qF "$expected"
with no separator and found the composition defect between two
individually-correct changes.

Rebased fix/223 onto origin/main (6899dd6) -- no conflicts, since #224
never touched a file this branch created or modified -- and added the same
-- separator #224 already applied to the other 16 suites, matching its
convention exactly rather than inventing a variant.

bash tests/run-all.sh prints "All suites passed." across all 45
tests/test-*.sh files, including test-assertion-helpers.sh (315/315) and
this suite's own 8 driven checks in both directions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

jit-dry-run.sh's report_hook() still greps the hook stream for block headers, so a forged entry body makes it print a fabricated match

1 participant