Commit d3cce3e
fix(plugins): close 13 sentinel-hook allow bypasses
Three independent adversarial reviewers over two passes found thirteen false-ALLOW bypasses in sentinel-read-allow.js — five introduced by the comment-skip and traversal changes that prompted the review, eight pre-existing and surviving from the earlier review round. All are closed and pinned by regression cases. Every PoC was run through the hook and its shell effect confirmed in real bash.
Arbitrary command execution, three distinct routes:
A function definition with a subshell body — `cat () ( touch x ); cat` — has no top-level separator inside the body, so segmentation saw one segment whose first token was the safe name `cat` and never vetted the body; the following call ran it. The `$(`/`<(`/`>(` guards all require a sigil, so bare parens slipped past every one. Now rejected, tested on comment-stripped text so that prose parens in a `# Reload X (Check 41)` line stay inert.
maskQuotes ate a backslash-newline, merging a `# comment \` line with the line after it, so the comment skip swallowed a payload the shell still ran — bash ends a comment at the physical newline and does not continue it. Escaped newlines now survive as newlines in both the plain and double-quote branches.
Setting a sensitive variable had three routes that never produce the literal `NAME=` the assignment pattern looks for: `printf -v PATH` uses the builtin's variable-write flag, `read PATH < file` writes its target name through the deliberately-permitted input redirect, and `export PA${X}H=` is expanded only after the text has been inspected. Each hijacked PATH end to end, with a planted binary running as a whitelisted read-only token. The control now gates the variable write itself rather than the assignment syntax.
Command-boundary desync: bash unescapes `\'` inside `$'...'`, so the string ends at a different quote than the masker believes and the toggle count drifts — a `;` and a following command were masked into a single `echo` segment. No blueprint idiom uses ANSI-C quoting, so it is refused outright.
File write via a whitelisted token: `uniq IN OUT` writes its second positional operand, truncating a two-line file to zero bytes. Same class as the already-excluded `sort -o`, but positional rather than a flag. Removed from SAFE_TOKENS.
Write redirect via prefix match: the redirect stripper consumed the `>/dev/null` prefix of `>/dev/nullpwned`, leaving no `>` for the check that follows. Each stripped form is now anchored to a token boundary.
Traversal, two ways: an allow-list of neighbour characters let every unlisted separator through, so `cat {a,../etc/passwd}` brace-expanded into a parent-directory read. The check is now default-reject, exempting only a `..` beside a word character or dot. It also tests a backslash-collapsed copy, since `cat \.\./etc/passwd` carries no literal `..` bytes yet the shell resolves the parent directory.
Anchor forgery: the anchor test ran against the raw command, so a commented-out sentinel read matched READ_FORM and vouched for a live command with no blueprint idiom in it. stripComments gates the anchor test only; every other check still inspects the full command, so a comment can cost an allow but never grant one.
Assignment-operator and name coverage: the patterns matched only `=` and `+=`, so bash's `${VAR:=word}` form assigned a sensitive variable — and kept an existing export attribute, so a child process inherits it — without ever producing `VAR=`. Both patterns now cover `:=`, and NONEMPTY_IFS gains `{` in its boundary class, which `${IFS=x}` had been using to evade it. Quoting and escaping the name no longer helps either: the guard tests a quote-stripped copy alongside the raw text, with empty quotes preserved so the blueprint's own `IFS=""` stays empty. Added LD_AUDIT, the four missing DYLD siblings and PROMPT_COMMAND; dropped BASH_FUNC, which could never match its own real form and only gave false coverage.
The two changes that prompted the review stay: whole-line comments no longer reject an otherwise read-only block, and `...` ellipsis and `v1.2..v1.3` are no longer read as traversal.
Coverage held at 46 of 790 blueprint blocks through every fix — none of the hardening cost an allow. The one guard that did cost coverage, rejecting bare parens, was narrowed to live text after measuring 14 lost blocks. Hook suite 44 to 137 tests. Documented and left unfixed: the traversal guard sees only literal text, so dots the shell materialises at runtime stay invisible; every such case lands inside the read-only disclosure residual the header already accepts.
READMEs synced across the four plugins; patch bumps on each.
---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>1 parent a8c91d1 commit d3cce3e
17 files changed
Lines changed: 1189 additions & 123 deletions
File tree
- plugins
- cc_develop
- .claude-plugin
- hooks
- cc_foundry
- .claude-plugin
- hooks
- tests
- cc_oss
- .claude-plugin
- hooks
- cc_research
- .claude-plugin
- hooks
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
640 | 640 | | |
641 | 641 | | |
642 | 642 | | |
643 | | - | |
| 643 | + | |
644 | 644 | | |
645 | 645 | | |
646 | 646 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3001 | 3001 | | |
3002 | 3002 | | |
3003 | 3003 | | |
3004 | | - | |
| 3004 | + | |
3005 | 3005 | | |
3006 | 3006 | | |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
19 | 19 | | |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5573 | 5573 | | |
5574 | 5574 | | |
5575 | 5575 | | |
5576 | | - | |
| 5576 | + | |
5577 | 5577 | | |
5578 | 5578 | | |
0 commit comments