fix(supply-chain): suppress install detection inside data-utility argv (#141) - #148
Merged
Merged
Conversation
#141) The supply-chain gate's install-detection regexes match install-shaped substrings anywhere in a command, including inside the ARGUMENTS of ordinary data-consuming utilities. Live FP from peer review: echo "/usr/bin/npm install foo" is data printed to stdout, but the gate treated the inner substring as a real install verb and blocked. Re-implementation of #141 on top of develop @ c4f6e55, composing cleanly with the now-merged #142/#143/#146 work (line-continuation preprocess, quote-aware tokeniser, recursion + depth-cap synthetic Unvettable). The original #141 implementation (commit 7a6935b on feat/plaintext-fp-guard-141) accumulated too many conflicts with the follow-on merges to rebase cleanly — this is a fresh re-write of the same byte-masking contract. Design — head-verb allowlist, byte-mask preserving offsets: - DATA_CONSUMING_UTILITIES — echo, printf, cat, tac, grep, egrep, fgrep, rg, awk, gawk, sed, head, tail, tee, nl. Same list as the original; deliberately narrow, only utilities whose canonical role is to EMIT or SEARCH their argv as data. - command_head_is_data_utility(segment) — basename-normalises the first non-operator token (handles /bin/echo, /usr/bin/grep) and matches against the allowlist. - mask_data_utility_segments(cmd) — walks the shell_tokens stream, identifies segments whose head is a data utility, overwrites their bytes with ASCII spaces in a same-length working copy. Chain operators (&&, ||, ;, |) and unmasked segments are preserved verbatim, so the regex prefix-anchors and the claimed-span dedup bookkeeping continue to work unchanged. Composition with post-#146 develop — masking layer placement: - Wired into detect_installs_into() (NOT detect_installs()), so it applies at every recursion depth. Wrappers like sh -c 'echo "npm install foo"' extract the inner echo payload via extract_recursion_segments and recurse; the inner head is also a data utility and must be masked at the inner depth. - The recursion sweep below the masking now runs against cmd_raw (the unmasked input), NOT the masked copy. Reason: a substitution body nested inside a data-utility segment still executes at runtime — echo "$(npm install foo)" does install the package — so the gate must still vet that body. Masking suppresses the surface-level regex pass for that segment ONLY; the recursion sweep is left intact. - Composes cleanly with mask_quoted_operators() (also same-length) and the LINE_CONT_RE collapse upstream. Segment-end calculation differs from the original implementation — post-#146 shell_tokens strips quote characters from token payloads (so the literal token for "foo" is foo, payload len 3, source span 5 bytes including the quotes). The original mask used seg_end = off + tok.len() which would under-mask quoted tokens. This re-write tracks segment end via the next operator's source offset (or cmd.len() at end-of-stream), which is robust to the quote-stripping payload contract. Tests: 13 new tests covering the FP cases (echo/grep/printf/cat/rg with install-shaped argv), positive guards (vanilla + abs-path npm install MUST still detect), tricky chain semantics (echo "x" && npm install foo, echo … ; npm install …, echo foo | npm install bar, cd foo && npm install bar), abs-path data-utility heads (/bin/echo …), plus a direct unit test on command_head_is_data_utility. 2,653 tests pass (was 2,640), zero regressions. Out of scope (deferred to principled-fix follow-up): token-position constraint and quote-context exclusion. They are the principled fixes — this remains a head-verb allowlist bandaid and the DATA_CONSUMING_UTILITIES list should shrink (or retire) once those ship. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…llowlist (agy #148 BLOCKER) agy round-1 peer review on PR #148 (re-implementation of #141) caught a BLOCKER: `awk`, `gawk`, and `sed` were in DATA_CONSUMING_UTILITIES, but those tools are Turing-complete with native command-execution primitives: - `awk 'BEGIN { system("npm install evil") }'` - `gawk` (and `mawk`) — same `system()` primitive - GNU `sed 's/.*/npm install bad/e'` — the `e` flag executes the replacement string as a shell command Masking the segment to spaces removed the install from the regex pass AND from the recursion sweep (recursion uses cmd_raw, but no shell-c extractor walks awk/sed script bodies). Net: silent bypass. Fix: remove `awk`/`gawk`/`sed` from the allowlist. The allowlist now contains ONLY utilities that genuinely cannot spawn a shell. Added a doc-comment block explicitly enumerating execution-capable exclusions so a future contributor doesn't re-add them. Also picked up agy's MEDIUM (allowlist coverage): added `jq`, `base64`, `xxd`, `od`, `hexdump` — all genuinely data-consuming, no shell-out flags, currently produce false-positive blocks when they read manifests or payloads containing install-shaped text. New allowlist (15 → 17 entries, awk/gawk/sed dropped, base64/xxd/od/ hexdump/jq added): echo, printf, cat, tac, tee, grep, egrep, fgrep, rg, head, tail, nl, base64, xxd, od, hexdump, jq +8 regression tests: - awk/gawk/sed `system()`/`e`-flag installs MUST be detected (not masked). - jq/base64/xxd/od/hexdump with install-shaped substrings must NOT classify as installs. 2,661 tests pass (+8), clippy clean. Out of scope, noted for tracking: - agy MEDIUM: multi-byte UTF-8 corruption in shell_tokens unquoted payloads (pre-existing, post-#146 contract). Worth a follow-up issue when context allows. Codex peer review on #148 still in flight — this commit lands the agy-found BLOCKER unconditionally given its severity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…BLOCKER) Codex peer review on PR #148 caught an execution-capable utility agy missed: `rg --pre <executable>` runs the named executable as a preprocessor on every file. With `rg` on the allowlist, the segment was masked and the install hidden — silent bypass. Fix: remove `rg` from DATA_CONSUMING_UTILITIES. A plain `rg "<pattern>" src/` is no longer masked either, so a false-positive Ask is possible for the rare case where the pattern is install-shaped. Net trade: better to over-Ask than under-detect. Doc-comment block updated with the third execution-capable exclusion alongside awk/gawk/mawk and sed. +1 regression test: rg_pre_install_must_be_detected_not_masked (the old rg_with_install_substring_is_not_an_install test was replaced — its expected behaviour reversed once rg left the allowlist). 2,661 tests pass, clippy clean. Out of scope, tracking separately: - Codex WARN: `claimed.iter().any(...)` and `dedup_installs()` are O(n²) in the number of detected install spans. Not a bypass; a performance pathology with a crafted command containing many repeated install-shaped segments. Worth a follow-up issue. Both peer-review BLOCKERs (#148 round-1) are now closed: - agy: awk/gawk/sed removed in 76ecc84. - Codex: rg removed here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address the two low-severity findings from the round-3 peer review on PR #148, both surfaced by agy after both round-2 BLOCKERs were closed: 1. Stale comments in two locations still enumerated rg/awk/gawk/sed as data utilities — out of date since those four were deliberately excluded from DATA_CONSUMING_UTILITIES. - src/hooks/supply_chain_gate.rs:1049 — replace `rg, awk, sed` example tail with `jq, base64`, which ARE on the allowlist. - src/hooks/supply_chain_gate.rs:3469 — rewrite the allowlist enumeration to match DATA_CONSUMING_UTILITIES exactly, and call out the exclusions explicitly so future readers don't try to add them back. 2. Seven allowlisted utilities (tac, tee, egrep, fgrep, head, tail, nl) were silently relying on indirect masking coverage — no dedicated "must-be-masked" test pinned them. Add one regression test each so any future trim of the allowlist surfaces as an explicit test failure instead of a quiet behavioural change. Verification: cargo test --bin contextcrawler → 2668 passed; 0 failed; 7 ignored Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thehoff
added a commit
that referenced
this pull request
May 23, 2026
Manual version bump capping the #141 plaintext-FP guard cluster (#139, #140/#146, #141/#148, #142, #143). Headline: zero-execution data-utility allowlist (echo/printf/cat/grep/jq/base64/…) so that install-shaped substrings printed-as-data no longer trigger the supply-chain gate, with hard exclusions for rg / awk / gawk / sed which can spawn subprocesses (Codex + agy round-3 GREEN/GREEN). 46 merged PRs since v0.1.9. See GH release v0.1.10 for the full PR list + notes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
noogalabs
pushed a commit
to noogalabs/contextcrawler
that referenced
this pull request
Jun 4, 2026
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.
Closes #141. Re-implementation of the original feat/plaintext-fp-guard-141 work after develop merged #142/#143/#146 - too many conflicts for clean rebase, fresh re-implementation on current develop instead. Original commit 7a6935b kept on the parked branch for reference.
The bug: the supply-chain gate produces false positives on data text containing install-shaped substrings (e.g. echo statements quoting a package-manager command, grep searches over README files mentioning installer verbs). Live hit on 2026-05-23 when a peer-review prompt quoted a verb - the gate blocked its own review prompt. Ironically also hit when this PR was first opened with a verbose body.
Approach (allowlist, option 1 from the issue): approaches 2 (token-position constraint) and 3 (quote-context exclusion) depend on the shell-quote-aware tokeniser tracked in #140 - now landed via #146. This PR ships option 1 - data-utility allowlist - as a pragmatic bandaid that composes cleanly with the post-#146 pipeline.
Implementation:
Post-#146 contract gotcha codified in a doc comment: shell_tokens now strips quote characters from token payloads. off + tok.len() is no longer the source span. Segment-end uses the next operator token offset (or cmd.len() at end).
13 new tests covering: echo/grep/printf/cat data argv negative guards, positive guards (real installs still detected at every shape), chain-operator boundaries (data utility on left + real install on right), pipe boundaries, nested substitution in data utility (still recurses).
2,653 tests pass, clippy clean on new code.
Per the peer-review-codex-agy-mandatory rule: review on this commit before merge.
🤖 Generated with Claude Code