harden(supply-chain): newline-chain + Windows launcher + bare poetry/uv bypasses - #142
Merged
Merged
Conversation
…oetry/uv bypasses Three tight follow-ups from the two-model peer review of #139 (Codex + Antigravity / Gemini). The shell-quote tokeniser refactor and the plaintext-data false-positive guard are tracked in #140 and #141 — both bigger work, deliberately not stuffed here. 1. NEWLINE CHAIN (Antigravity BLOCKER). The arg-capture group `[^|;&<>]+` in all 7 install-detect regexes matched newlines, so a multi-line script like `npm install x\npip install y` chain-swallowed the second line and suppressed downstream detection. Adds `\r\n` to the negated class. 2. WINDOWS LAUNCHER (Codex MEDIUM). `npm.cmd`, `pip.exe`, `yarn.cmd` slipped past both the package-bearing regexes and the bare-install token match. Two-line fix: regex verb now accepts an optional `(?:\.(?:cmd|exe|bat))?` suffix; `installer_basename` strips those same suffixes after path-separator splitting. 3. POETRY / UV BARE LOCKFILE INSTALL (Antigravity HIGH). `detect_bare_lockfile_installs` only covered npm/pnpm/yarn, so `poetry install` and `uv sync` (resolving the full dep tree from pyproject.toml / poetry.lock / uv.lock with no package args) returned a silent Skip. Extended the match block to recognise both verbs and surface them as Pypi-ecosystem unvettable installs. Reshapes the `is_bare_install_verb: bool` thread to carry `Ecosystem` directly so the push records the right ecosystem (was hardcoded `Ecosystem::Npm`). The unvettable lockfile-source string also forks per ecosystem. 12 new regression tests: - newline_chain_both_installs_detected (`\n`) - newline_chain_carriage_return_also_caught (`\r\n`) - windows_launcher_{npm_cmd,pip_exe,abs_path_npm_cmd,bare_npm_cmd}_detected - poetry_install_bare_lockfile_detected - poetry_install_with_flags_still_bare_lockfile - poetry_add_still_handled_by_regex (sanity: package-bearing path) - uv_sync_bare_lockfile_detected - uv_pip_sync_bare_lockfile_detected - uv_abs_path_sync_detected Out of scope, tracked: #140 — shell-quote-aware tokeniser (closes the HIGH bypass class: `sh -c '<install>'`, `$(<install>)`, backticks, quoted heads, quoted operators in flag values). #141 — plaintext-data false-positive guard (closes the MEDIUM regression risk: `echo "/usr/bin/npm install foo"` is data, not an install — hit live during this very peer review when agy received a prompt that quoted the verb). 2,594 tests pass (+12 from develop), clippy clean on changed file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 23, 2026
Closed
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
…t bypasses Round-two peer review of thehoff#142 by Codex + agy converged on a BLOCKER and three HIGHs. All four landed here. Both reviewers verdict on thehoff#142 was "hold"; thehoff#142 merged on the strength of what it DID close (per Hoff direction), with this branch carving the remainder. 1. BLOCKER (Codex + agy AGREED). Value-consuming flag bypass on the new bare-lockfile surface. `poetry install --with dev`, `uv sync --extra dev`, `uv sync --group dev` — `--with`/`--extra`/`--group` consume the next token, but the `has_package` scan treated that value as a package and silently dropped the install. Fix: thread `accepts_packages: bool` through the per-verb match. `poetry install`, `uv sync`, `uv pip sync`, `npm ci`, `yarn install`, and bare `yarn` never take a positional package — for them the package scan is skipped, always-bare. `npm install` / `pnpm install` / `pnpm i` keep the scan as before (they do take a positional, and the named-install case is normally claimed by NPM_RE / PNPM_RE upstream). 2. HIGH (Codex + agy AGREED). Windows launcher case sensitivity. `NPM.CMD install foo`, `Npm.Cmd install foo`, `PIP.EXE install x` slipped past both the regex verb check and the bare-install token compare. Fix: - `installer_basename` strips `.cmd`/`.exe`/`.bat` case-insensitively in a loop (so chained extensions `npm.cmd.exe` also collapse). - The bare-install token match lowercases the basename before comparing. - All 7 install-detect regexes use `(?i:…)` scoped flags on the verb and the suffix groups: `(?i:npm)(?:\.(?i:cmd|exe|bat))*\s+(?i:i|install|add)`. - The optional suffix group changed from `?` to `*` so the regex matches chained extensions (`npm.cmd.exe install foo`). 3. HIGH (Codex only). Versioned pip launchers. `pip3.12.exe install x` stripped `.exe` to `pip3.12`, which didn't match `pip|pip3`. The PIP_RE verb is now `(?i:pip\d*(?:\.\d+)*)`, matching `pip`, `pip3`, `pip3.12`, `pip3.12.0`, etc., paired with the chained-suffix group. 4. HIGH (agy only). Line-continuation bypass — my own thehoff#142 regression. `\r\n` in the arg-capture's negated class truncated `npm install \<nl> foo` at the trailing backslash, so packages on the continuation line went undetected. Fix: a `LINE_CONT_RE` (`\\\r?\n\s*`) preprocess at the top of `detect_installs` collapses backslash-newline-indent sequences to a single space BEFORE detection runs. The newline guard from thehoff#142 is preserved for genuine multi-command newline separation (test `line_continuation_does_not_merge_distinct_commands`). Also folds in agy MEDIUM (double-extension `npm.cmd.exe`) since the loop strip + `*` regex quantifier address it for free. 18 new regression tests: - poetry_install_with_{value_consuming,only,without_and_extras}_flag_still_bare - uv_sync_with_{extra,group}_value_still_bare - npm_ci_with_value_consuming_flag_still_bare - yarn_install_with_value_consuming_flag_still_bare - windows_launcher_{uppercase_npm_cmd,mixed_case,full_uppercase_no_suffix,pip_exe_uppercase,double_extension}_detected - versioned_pip_{3_12,3_12_exe,abs_path}_detected - line_continuation_{multiline,crlf}_install_caught - line_continuation_does_not_merge_distinct_commands 2,612 tests pass (+18 from develop's 2,594), clippy clean on the changed file (the line 688 warning is the pre-existing `parse_package_args` type_complexity, unchanged). Per the new peer-review-mandatory rule in memory, this PR will be reviewed by Codex + agy before the merge question is opened. 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
thehoff#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 thehoff#141 on top of develop @ c4f6e55, composing cleanly with the now-merged thehoff#142/thehoff#143/thehoff#146 work (line-continuation preprocess, quote-aware tokeniser, recursion + depth-cap synthetic Unvettable). The original thehoff#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-thehoff#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-thehoff#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>
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.
Three tight follow-ups from the two-model peer review of #139 (Codex + Antigravity/Gemini). Each addresses a distinct bypass observed during review of the absolute/relative-path fix.
What lands here
1. Newline chain (Antigravity BLOCKER)
The arg-capture group `[^|;&<>]+` in all 7 install-detect regexes matched newlines, so a multi-line script chain-swallowed the next line and suppressed downstream detection. Adds `\r\n` to the negated class.
```
Before this PR: 1 detection (greedy first match swallows the rest)
After: 2 detections
echo PKG1
echo PKG2
```
2. Windows launcher (Codex MEDIUM)
`npm.cmd`, `pip.exe`, `yarn.cmd` slipped past both the package-bearing regexes (which required `npm\s+`) and the bare-install token match (`match tok { "npm" => … }`). Two-line fix:
3. Bare poetry / uv lockfile install (Antigravity HIGH)
`detect_bare_lockfile_installs` only covered npm/pnpm/yarn, so `poetry install` and `uv sync` (resolving the full dep tree from pyproject.toml / poetry.lock / uv.lock with no package args) returned a silent `Skip`. Extended to recognise both verbs and surface them as Pypi-ecosystem unvettable installs. Reshapes the internal `is_bare_install_verb: bool` thread into `Option` so the push records the right ecosystem (was hardcoded `Ecosystem::Npm`); unvettable lockfile-source string now forks per ecosystem.
Out of scope, tracked
Tests
12 new adversarial cases:
```
newline_chain_both_installs_detected // \n
newline_chain_carriage_return_also_caught // \r\n
windows_launcher_npm_cmd_detected
windows_launcher_pip_exe_detected
windows_launcher_abs_path_npm_cmd_detected // C:\Tools\node\npm.cmd …
windows_launcher_bare_npm_cmd_detected
poetry_install_bare_lockfile_detected
poetry_install_with_flags_still_bare_lockfile
poetry_add_still_handled_by_regex // sanity: package-bearing
uv_sync_bare_lockfile_detected
uv_pip_sync_bare_lockfile_detected
uv_abs_path_sync_detected
```
2,594 tests pass (+12 from develop), clippy clean on changed file.
Peer-review provenance
Findings consolidated from Codex (verdict: needs-work) and Antigravity/Gemini (verdict: needs-work). Both reviewers converged on the regression risk (#141) and the shell-quoting bypass class (#140). Antigravity flagged the newline-chain BLOCKER which Codex missed; Codex flagged the Windows launcher MEDIUM which Antigravity missed. Multi-model peer review is paying its keep.
🤖 Generated with Claude Code