harden(supply-chain): close abs/relative-path install bypass - #139
Merged
Merged
Conversation
Empirically observed on 2026-05-23 during Pi.dev install. The Tirith
supply-chain gate failed to fire on:
/Users/thehoff/.nvm/versions/node/v25.0.0/bin/npm install \
--ignore-scripts @earendil-works/pi-coding-agent
— a package well inside the gate's 3-day RecentRelease cooldown that
should have blocked. Root cause was a pair of identical assumptions in
the detector:
1. The install-detect regexes (NPM_RE, PNPM_RE, YARN_RE, PIP_RE, UV_RE,
POETRY_RE, PIPX_RE) anchored the verb on `(?:^|\s|;|&&|\|\|)`. A
path separator (`/` POSIX or `\` Windows) before the verb is not in
that set, so `/bin/npm install …` and `./bin/pnpm i …` never matched.
2. `detect_bare_lockfile_installs` matched the head token with a
literal `match tok { "npm" | "pnpm" | "yarn" => … }`. An absolute or
relative path token (`"/Users/.../bin/npm"`) is not the literal
string `"npm"`, so the bare-install variant also slipped past.
Fix (surgical, source-level only):
- Prefix anchor in all 7 regexes → `(?:^|[\s;/\\]|&&|\|\|)`. Treats `/`
and `\` as command-start delimiters alongside whitespace and shell
operators. Same behaviour as before for non-path invocations.
- New `installer_basename(tok)` helper strips the last path component
(POSIX + Windows). `detect_bare_lockfile_installs` now compares the
basename of the head token, not the raw token.
Regression tests (10 new, all previously red, now green):
- abs_path_{npm,pnpm,yarn,pip,uv,poetry,pipx}_*_detected — every
ecosystem with an abs-path invocation.
- abs_path_bare_npm_install_detected — bare lockfile install via path.
- relative_path_npm_install_detected — `./node_modules/.bin/npm i …`.
- abs_path_does_not_double_match_mid_path_substring — negative guard:
a path that merely mentions `install` (no verb) must produce zero
hits.
Out of scope (separate finding, follow-up): the `CONTEXTCRAWLER_SUPPLY_CHAIN=off`
env-var override did not propagate from a Claude Code Bash tool call to
the contextcrawler-hook process. The gate code at line 993 reads the var
correctly; the breakage is in env-passing from the PreToolUse hook
contract. Worth investigating in its own pass.
2,582 tests pass (+18 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
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
…oetry/uv bypasses Three tight follow-ups from the two-model peer review of thehoff#139 (Codex + Antigravity / Gemini). The shell-quote tokeniser refactor and the plaintext-data false-positive guard are tracked in thehoff#140 and thehoff#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: thehoff#140 — shell-quote-aware tokeniser (closes the HIGH bypass class: `sh -c '<install>'`, `$(<install>)`, backticks, quoted heads, quoted operators in flag values). thehoff#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>
noogalabs
pushed a commit
to noogalabs/contextcrawler
that referenced
this pull request
Jun 4, 2026
…pass (thehoff#140) PR thehoff#139 review (Codex + Antigravity) flagged a HIGH bypass class: the naive shell_tokens() did not honour shell quoting, so several install shapes slipped past the gate. 1. sh -c '<install>' / bash -c '<install>' wrapped installs 2. $(<install>) / backtick <install> command-substitution installs 3. 'npm' install pkg / "pip" install pkg quoted-head installs 4. operator inside a quoted flag value (--description="install & test") truncated [^|;&<>]+ and dropped packages after the flag Implementation: - shell_tokens() walks bytes with quote-state for single, double, and ANSI-C $'…' quotes. Token text is the literal payload with quote chars stripped, so a quoted head matches npm/pip/etc. - mask_quoted_operators() produces a same-length copy where operator bytes inside quoted regions become spaces. Package-list regexes run against this; the captured arg span is re-sliced from the original command to keep real characters in package names. - extract_recursion_segments() pulls sh -c arg / bash -c arg / $(…) / backtick bodies out as flat strings. detect_installs_into() recurses on each (depth-capped at MAX_RECURSION_DEPTH = 6). - Regex anchors now treat ' and " as command-start delimiters so 'npm' install lodash is caught by the outer pass. - Structural dedup collapses duplicates when the outer regex + recursion both fire on the same install. Adversarial fixtures (TDD, all real shell forms): - quoted_sh_c_install_detected - quoted_bash_c_install_detected - command_substitution_install_detected - backtick_substitution_install_detected - quoted_head_install_detected - quoted_operator_in_flag_value_does_not_truncate_scan - ansi_c_quoted_install_detected - quoted_install_as_data_argument_still_flagged (echo "$(npm install foo)" — conservatively flagged: the install runs before echo prints, so the side-effect already happened) - nested_sh_c_install_detected — defence in depth - unmatched_quote_falls_back_safely — no panic on malformed input - shell_tokens unit tests for quote stripping + operator-in-quote Out of scope (documented in code comments): heredocs (<<EOF … EOF), eval with variable expansion, process substitution <(…). These would need shell-style word expansion, not just tokenisation. Test count: 2582 -> 2596 (+14). All 2596 pass, 0 failures. Clippy: 49 warnings before, 49 after — no new warnings on changed code. Pre-existing type_complexity on parse_package_args left untouched per task constraints. Closes thehoff#140. 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.
Empirically observed bypass on 2026-05-23, while installing Pi.dev for the new harness integration. The Tirith supply-chain gate failed to fire on:
```
/Users/thehoff/.nvm/versions/node/v25.0.0/bin/npm install --ignore-scripts @earendil-works/pi-coding-agent
```
— a package well inside the gate's 3-day RecentRelease cooldown that should have blocked. Same shape works for
pnpm,yarn,pip,uv,poetry,pipx.Root cause
Two parallel assumptions in
src/hooks/supply_chain_gate.rs:(?:^|\s|;|&&|\|\|). A path separator (/POSIX,\Windows) before the verb is not in that set, so/bin/npm install …and./bin/pnpm i …never matched.detect_bare_lockfile_installsmatched the head token with a literalmatch tok { "npm" | "pnpm" | "yarn" => … }. An absolute or relative path token (e.g."/Users/.../bin/npm") is not the literal string"npm", so the bare variant slipped past too.Fix (surgical)
(?:^|[\s;/\\]|&&|\|\|)./and\are now command-start delimiters alongside whitespace and shell operators. Behaviour unchanged for non-path invocations.installer_basename(tok)helper strips the last POSIX/Windows path component. The bare-install token check compares the basename, not the raw token.Regression tests (10 new, were red, now green)
abs_path_{npm,pnpm,yarn,pip,uv,poetry,pipx}_*_detected— every ecosystem via absolute path.abs_path_bare_npm_install_detected— bare lockfile install via abs path.relative_path_npm_install_detected—./node_modules/.bin/npm i ….abs_path_does_not_double_match_mid_path_substring— negative guard: a path that merely contains the wordinstall(no verb) must produce zero hits.Out of scope (separate finding for follow-up)
The
CONTEXTCRAWLER_SUPPLY_CHAIN=offenv-var override did not propagate from a Claude Code Bash tool call to the spawned contextcrawler process. The gate code at line 993 reads the var correctly; the break is in env-passing from the PreToolUse hook contract. Worth its own investigation.2,582 tests pass (+18 from develop), clippy clean on changed file.
🤖 Generated with Claude Code