fix(hooks): the args-heredoc guard reads the quotes before an escaped run, not the whole command

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-11-01 13:04:09 +01:00
commit 1731e41629
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
3 changed files with 357 additions and 24 deletions

View file

@ -318,12 +318,19 @@ const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g,
// skipping it hides no later opener.
// - A < after an odd run of backslashes (counted on the raw text, a line
// break ends the run) starts no opener, in the regex reading and in the
// shell reading alike, but only when no ', " or backquote and no <<
// stands before the run: each of those can make the shell read the
// backslashes once more (inside a backquote, or in an unquoted heredoc
// body fed to a shell, measured), so there the < is read as unescaped.
// The < after the escaped one is read as usual, so a << right after it
// is still checked.
// shell reading alike, when a quote scan of the text before the run ends
// outside every quote and every heredoc body. The scan skips a closed
// '...' or "..." string, a here-string and the body of a heredoc closed
// before the run, so none of these turns the rule off. A quote still open
// before the run, a heredoc not closed before it, and text the scan
// cannot follow (for example $' or $", $( ${ $[, a backquote, a (, a #, a
// continuation, a heredoc word it cannot read, a second < on a heredoc's
// line, a body line of an unquoted heredoc that ends in a backslash) keep
// the rule off: there the shell may read the backslashes once more
// (inside a backquote, or in a heredoc body fed to a shell, measured), so
// the < is read as unescaped. The regex reading takes the shell reading's
// decision for the same <. The < after the escaped one is read as usual,
// so a << right after it is still checked (fix round 4.5b2, part A).
// The shell reading fails closed: an error in it blocks (exit 2), because an
// exit 1 is a non-blocking error and the command would run.
const ARGS_DELIM = 'VOYAGE_ARGS_EOF_7F3C9E';
@ -343,23 +350,141 @@ function blockArgs(unquoted, unreadable) {
process.exit(2);
}
// Where the backslash rule stops applying in text: the first ', " or
// backquote, and the end of the first << (continuations between the two <).
// The quote state at `to` of `text` read from `from` for quotes only: 'none'
// (outside every quote), 'open' (inside '...' or "...", or inside an earlier
// heredoc's body) or 'unknown' (the scan cannot follow the shell here).
// `state`, when given, lets the calls on one text resume where the last stopped.
function argsQuoteScan(text, from, to, openers, state) {
const fresh = (pos) => ({ pos, mode: 'none', pending: null, bodyEnd: -1 });
const st = state ? (state.pos <= to ? state : fresh(0)) : fresh(from);
let p = st.pos;
let mode = st.mode;
let pending = st.pending;
let bodyEnd = st.bodyEnd;
const stop = (result) => {
st.pos = p;
st.mode = mode;
st.pending = pending;
st.bodyEnd = bodyEnd;
return result;
};
const unknown = () => {
mode = 'unknown';
return stop('unknown');
};
for (;;) {
if (mode === 'unknown') return stop('unknown');
if (mode === 'body') {
if (to < bodyEnd) {
p = to;
return stop('open');
}
p = bodyEnd;
mode = 'none';
}
// A token that runs past `to` (a heredoc word, a here-string) is not followed.
if (p > to) return unknown();
if (p === to) return stop(mode === 'none' ? 'none' : 'open');
// The line break that ends a heredoc's line: its body starts after it.
if (pending && p === pending.nl) {
if (mode !== 'none' || pending.bodyEnd === null) return unknown();
mode = 'body';
bodyEnd = pending.bodyEnd;
pending = null;
continue;
}
const c = text[p];
const d = text[p + 1];
if (mode === 'single') {
if (c === "'") mode = 'none';
p += 1;
continue;
}
if (c === '\\') {
// The scan never decides a backslash without the character it escapes.
if (p + 1 >= to) return unknown();
if (mode === 'double' && d === '\n') return unknown();
if (d === '\n') return unknown(); // scan: a continuation cannot be followed
p += 2;
continue;
}
if (mode === 'double') {
if (c === '"') mode = 'none';
else if (c === '`' || (c === '$' && (d === '(' || d === '{' || d === '['))) return unknown();
p += 1;
continue;
}
if (c === "'" || c === '"') {
mode = c === "'" ? 'single' : 'double';
p += 1;
continue;
}
if (c === '`' || c === '(' || c === '#') return unknown();
if (c === '$' && (d === "'" || d === '"' || d === '(' || d === '{' || d === '[')) return unknown();
if (c === '<' && d === '<') {
if (text[p + 2] === '<') {
p += 3; // a here-string is text
continue;
}
if (!openers) {
p += 2;
continue;
}
// The opener record at p (openers are in text order).
let o = null;
for (let lo = 0, hi = openers.length - 1; lo <= hi && !o;) {
const m = (lo + hi) >> 1;
if (openers[m].index < p) lo = m + 1;
else if (openers[m].index > p) hi = m - 1;
else o = openers[m];
}
if (!o || o.unreadable || o.cutOnly || !/^[A-Za-z0-9_]+$/.test(o.word)) return unknown();
let nl = text.indexOf('\n', o.end);
if (nl === -1) nl = text.length;
if (text.slice(o.end, nl).includes('<')) return unknown();
// The closing line: the first line after nl equal to the word (after
// <<- with its leading tabs stripped, nothing else). null: the body
// cannot be followed (an unquoted body line joined to the next).
let close = Infinity;
for (let ls = nl + 1; nl < text.length;) {
const le = text.indexOf('\n', ls);
const line = text.slice(ls, le === -1 ? text.length : le);
if ((o.dash ? line.replace(/^\t+/, '') : line) === o.word) {
close = le === -1 ? text.length : le + 1;
break;
}
if (!o.quoted && line.endsWith('\\')) {
close = null;
break;
}
if (le === -1) break;
ls = le + 1;
}
pending = { nl, bodyEnd: close };
p = o.end;
continue;
}
p += 1;
}
}
// The escape rule cannot apply when no backslash stands directly before a <
// (firstRun is -1: the fast path). The scan state is shared by the escape
// checks of one command, so the text is read once (fix round 4.5b2, part A).
function argsEscapeLimit(text) {
const h = /<(?:\\\n)*</.exec(text);
return { quoteAt: text.search(/['"`]/), heredocEnd: h ? h.index + h[0].length : -1 };
return { firstRun: text.search(/\\</), escaped: new Set(), state: { pos: 0, mode: 'none', pending: null, bodyEnd: -1 } };
}
// A < after an odd run of backslashes in the raw text (a line break ends the
// run) is a literal < to the shell, when no ', " or backquote and no <<
// stands before the run: inside quotes, backquotes or a heredoc body fed to
// a shell, a backslash may be read once more.
function argsEscapedLt(text, i, lim) {
// run) is a literal < to the shell when the quote scan of the text before the
// run ends outside every quote and heredoc body ('none'). `openers` holds
// every opener before i (fix round 4.5b2, part A).
function argsEscapedLt(text, i, lim, openers) {
if (lim.firstRun === -1) return false;
let b = 0;
while (i - 1 - b >= 0 && text[i - 1 - b] === '\\') b += 1;
const s = i - b;
return b % 2 === 1 && (lim.quoteAt === -1 || lim.quoteAt >= s) &&
(lim.heredocEnd === -1 || lim.heredocEnd > s);
return b % 2 === 1 && argsQuoteScan(text, 0, s, openers, lim.state) === 'none';
}
// A cut opener (its quote has no partner after it) whose quote closes a
@ -372,7 +497,9 @@ function argsCutInString(text, o) {
}
// Each heredoc operator in `text`, in text order, as the shell reads its word:
// { index (the operator's first <), end, word, quoted, unreadable, cutOnly }.
// { index (the operator's first <), end, word, quoted, unreadable, cutOnly,
// dash (the - of <<- was read) }. Every < it judges escaped goes into
// lim.escaped, which the regex reading reads.
function argsShellOpeners(text, lim = argsEscapeLimit(text)) {
const n = text.length;
// The index after any run of continuations that starts at k.
@ -383,7 +510,11 @@ function argsShellOpeners(text, lim = argsEscapeLimit(text)) {
const openers = [];
let i = text.indexOf('<');
while (i !== -1) {
if (argsEscapedLt(text, i, lim)) { i = text.indexOf('<', i + 1); continue; }
if (argsEscapedLt(text, i, lim, openers)) {
lim.escaped.add(i);
i = text.indexOf('<', i + 1);
continue;
}
let next = i + 1;
let k = skip(i + 1);
if (text[k] === '<') {
@ -396,7 +527,9 @@ function argsShellOpeners(text, lim = argsEscapeLimit(text)) {
next = k + 1; // a here-string: move past it
} else {
let unreadable = false;
let dash = false;
if (text[k] === '-') {
dash = true;
// After a continuation, bash reads <<- and zsh reads - as the word.
if (k !== second + 1) unreadable = true;
k = skip(k + 1);
@ -446,6 +579,7 @@ function argsShellOpeners(text, lim = argsEscapeLimit(text)) {
quoted,
unreadable: unreadable || isCut,
cutOnly: isCut && !unreadable,
dash,
});
}
}
@ -456,18 +590,23 @@ function argsShellOpeners(text, lim = argsEscapeLimit(text)) {
}
if (command.replace(/\\\n/g, '').includes(ARGS_DELIM)) {
// The regex reading and the shell reading. The shell reading's loop records
// the first failing verdict; an error anywhere in either reading blocks.
// The regex reading and the shell reading. The shell reading's openers are
// read first, so the regex reading takes its escape decisions; the shell
// reading's loop records the first failing verdict; an error anywhere in
// either reading blocks.
let argsVerdict = null;
try {
const argsLim = argsEscapeLimit(command);
const argsShell = argsShellOpeners(command, argsLim);
const argsOpeners = command.matchAll(new RegExp(`<<-?\\s*(['"\\\\]?)${ARGS_DELIM}(['"]?)`, 'g'));
const argsLines = command.split('\n');
const argsClosing = argsLines.flatMap((l, i) => (l.trim() === ARGS_DELIM ? [i] : []));
const closesOnceAtEnd = argsClosing.length === 1 &&
argsLines.slice(argsClosing[0] + 1).every((line) => line.trim() === '');
for (const argsOpener of argsOpeners) {
if (argsEscapedLt(command, argsOpener.index, argsLim)) continue;
// The shell reading's decision for the same <; a < it never reached as
// a start counts as not escaped.
if (argsLim.escaped.has(argsOpener.index)) continue;
// Some line after the opener's line is non-blank (\s is the set trim() strips).
const hasBody = /\n\s*\S/.test(command.slice(argsOpener.index));
if (!hasBody) continue;
@ -476,7 +615,7 @@ if (command.replace(/\\\n/g, '').includes(ARGS_DELIM)) {
if (!quoted || !closesOnceAtEnd) blockArgs(!quoted, false);
}
let bodyFrom = Infinity;
for (const o of argsShellOpeners(command, argsLim)) {
for (const o of argsShell) {
if (o.index >= bodyFrom && o.cutOnly) continue;
if (o.cutOnly && argsCutInString(command, o)) continue;
// The word is compared trimmed, as the body lines are: a word of D plus

View file

@ -0,0 +1,194 @@
// tests/hooks/bash-guard-4-5b2-a.test.mjs
// [RXT] [CNT] [RSK] [QSC] [SFX] [ESR] [EVL] Fix round 4.5b2, part A (decisions 91 and 107).
// Defects: the args-heredoc guard in hooks/scripts/pre-bash-executor.mjs
// turned its escape rule off for the whole command when a ', " or backquote
// or a << stood anywhere before an escaped <, so a here-string, a closed
// heredoc or a closed quote earlier in the command made the guard refuse a
// command in which the shell reads \< as a literal < (ESR). Two branches part
// 4 added had no test: the try that makes an error inside the regex reading's
// escape limit block (RXT), and the continuation-split outer opener that keeps
// the escape rule off (CNT).
// The helpers bashInput, ARGS_DELIM, ARGS_RULE, PARSER_LINE,
// expectArgsBlocked, expectArgsAllowed, UNQUOTED, expectUnquoted,
// CLOSED_EARLY, expectClosedEarly, D and BODY are copied from
// tests/hooks/bash-guard-false-stops.test.mjs:26-68, not imported, because
// importing a test file would run its tests. The guard is only spawned, so
// this file loads at e2d6bec and its red is an assertion.
import { test, after } from 'node:test';
import { strict as assert } from 'node:assert';
import { writeFileSync, rmSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { runHook, runHookWithEnv } from '../helpers/hook-helper.mjs';
import { tmp, pluginCopy } from './guard-helpers.mjs';
const HERE = dirname(fileURLToPath(import.meta.url));
const ROOT = join(HERE, '..', '..');
const PRE_BASH = join(ROOT, 'hooks', 'scripts', 'pre-bash-executor.mjs');
function bashInput(command) {
return { tool_name: 'Bash', tool_input: { command } };
}
const ARGS_DELIM = 'VOYAGE_ARGS_EOF_7F3C9E';
const ARGS_RULE = /args-heredoc-closed-early/;
const PARSER_LINE = 'node /plugin/lib/parsers/arg-parser.mjs --command trekendsession --stdin';
async function expectArgsBlocked(command) {
const { code, stderr } = await runHook(PRE_BASH, bashInput(command));
assert.match(stderr, ARGS_RULE, `the guard did not name args-heredoc-closed-early for: ${JSON.stringify(command)}`);
assert.strictEqual(code, 2, `the guard did not block: ${JSON.stringify(command)}`);
return stderr;
}
async function expectArgsAllowed(command) {
const { code, stderr } = await runHook(PRE_BASH, bashInput(command));
assert.doesNotMatch(stderr, ARGS_RULE, `the guard refused a well-formed heredoc: ${JSON.stringify(command)}`);
assert.strictEqual(code, 0, `the guard refused a well-formed heredoc: ${stderr}`);
}
// The unquoted reason: the guard blocks and says the opener is unquoted.
const UNQUOTED = /opens VOYAGE_ARGS_EOF_7F3C9E unquoted/;
async function expectUnquoted(command) {
const stderr = await expectArgsBlocked(command);
assert.match(stderr, UNQUOTED, `the guard did not give the unquoted reason for: ${JSON.stringify(command)}`);
}
// The closed-early reason: a line equal to the closing line, then more text.
const CLOSED_EARLY = /hold a line equal to the heredoc's closing line/;
async function expectClosedEarly(command) {
const stderr = await expectArgsBlocked(command);
assert.match(stderr, CLOSED_EARLY, `the guard did not give the closed-early reason for: ${JSON.stringify(command)}`);
assert.doesNotMatch(stderr, UNQUOTED, `the guard gave the unquoted reason for: ${JSON.stringify(command)}`);
}
const D = ARGS_DELIM;
// The body line and the closing line of the escape-rule controls.
const BODY = `\nbrief.md $(touch PWNED)\n${D}`;
test('[ESR] a here-string before an escaped < leaves the rule on', async () => {
// Red: the here-string <<< is text to the quote scan, so \< is a literal <
// and the next line runs as a command.
await expectArgsAllowed(`cat <<<x; echo \\<<${D}\nnext`);
// Control: a # comment holding a ' before bash -c "... (catalogue E1): the
// scan cannot follow a comment, so the rule is off and the inner shell
// opens an unquoted heredoc.
await expectUnquoted(`# don't\nbash -c "# it's\ncat - \\\\\\<<${D}\n\\$(touch PWNED)\n${D}"`);
// Control: an arithmetic shift $((1<<X)) before the run (catalogue E2): the
// scan cannot follow $(, so the rule is off.
await expectUnquoted(`echo $((1<<X))\nbash -c ":\nX\ncat - \\\\\\<<${D}\n\\$(touch PWNED)\n${D}"`);
// Control: a continuation before the run: the scan cannot follow it, so the
// rule is off.
await expectUnquoted(`echo a\\\nb && echo \\<<${D}${BODY}`);
});
test("[ESR] a closed quoted heredoc before an escaped < leaves the rule on, and a quote in its body counts for nothing", async () => {
// Red: the heredoc EOF is closed before the run and the ' in its body is
// body text, so \< is a literal < and the next line runs as a command.
await expectArgsAllowed(`cat <<'EOF'\ndon't\nEOF\necho \\<<${D}\nnext`);
// Control: an earlier heredoc not yet closed: the run stands in its body,
// so the rule is off.
await expectUnquoted(`cat <<'EOF'\necho \\<<${D}${BODY}`);
// Control: the would-be closing line is EOF plus a trailing blank, which
// does not close the heredoc, so the run stands in its body.
await expectUnquoted(`cat <<'EOF'\nEOF \necho \\<<${D}${BODY}\nEOF`);
// Control: the would-be closing line is EOF plus a CR, which does not
// close the heredoc, so the run stands in its body.
await expectUnquoted(`cat <<'EOF'\nEOF\r\necho \\<<${D}${BODY}\nEOF`);
// Control: the would-be closing line is a tab plus EOF under << without -,
// which does not close the heredoc, so the run stands in its body.
await expectUnquoted(`cat <<'EOF'\n\tEOF\necho \\<<${D}${BODY}\nEOF`);
// Control: two heredocs on one line: the scan cannot follow them, so the
// rule is off.
await expectUnquoted(`cat <<A <<B\nA\necho \\<<${D}${BODY}\nB`);
// Control: a continuation inside an earlier unquoted heredoc's body
// (catalogue CJ): the shell joins x\ to the next line, so EOF does not
// close the body there and the scan cannot follow it; the rule is off.
await expectUnquoted(`echo \\<x; bash <<EOF\nx\\\nEOF\ncat - \\\\\\<<${D}\n\\$(touch PWNED)\n${D}\nEOF`);
});
test('[ESR] a quote closed before an escaped < leaves the rule on', async () => {
// Red. Moved from tests/hooks/bash-guard-false-stops.test.mjs:117-118
// (decision 106): the ' is closed before the run, so the shell reads \< as a
// literal <.
await expectArgsAllowed(`echo 'x' && echo \\<<${D}${BODY}`);
// Control: a " still open before the run: inside "..." \< keeps its
// backslash, so the rule is off.
await expectUnquoted(`echo "y \\<<${D}${BODY}`);
// Control: a $' string before the run: the scan does not decode it, so the
// rule is off.
await expectUnquoted(`echo $'x' && echo \\<<${D}${BODY}`);
// Control: a closed backquote before the run: the scan cannot follow a
// backquote, so the rule is off.
await expectUnquoted(`x=\`date\`; echo \\<<${D}${BODY}`);
// Control that passes: a first odd run of three. The shell reads \\ and \<
// and then a <D redirection. The run starts two characters before the
// backslash the fast path finds, so this pins the rule against comparing
// that backslash with the run start.
await expectArgsAllowed(`echo \\\\\\<<${D}\nnext`);
});
// The preload makes String.prototype.search throw when it is called from
// inside argsEscapeLimit, so the regex reading's escape limit throws. It is
// written into a temp directory, never under tests/, and only makes the hook
// block.
const PRELOAD = `const orig = String.prototype.search;
String.prototype.search = function (...args) {
if (/argsEscapeLimit/.test(new Error().stack)) {
process.stderr.write('[rxt-preload] thrown\\n');
throw new Error('RXT forced read error');
}
return orig.apply(this, args);
};
`;
const preloadDir = tmp('voyage-rxt-');
after(() => rmSync(preloadDir, { recursive: true, force: true }));
test("[RXT] an error inside the regex reading's escape limit blocks the command (exit 2), never lets it through", async () => {
const file = join(preloadDir, 'rxt-preload.mjs');
writeFileSync(file, PRELOAD);
const forced = { NODE_OPTIONS: '--import=' + pathToFileURL(file).href };
const shipped = `${PARSER_LINE} <<'${D}'\nbrief.md\n${D}`;
// Red, forced: the shipped heredoc under the preload is blocked by the catch.
const { code, stderr } = await runHookWithEnv(PRE_BASH, bashInput(shipped), forced);
assert.ok(stderr.includes('[rxt-preload] thrown'), `the preload did not throw inside the escape limit: ${stderr}`);
assert.strictEqual(code, 2, `the guard did not block when its escape limit threw (exit ${code}): ${stderr}`);
assert.match(stderr, ARGS_RULE, `the guard did not name args-heredoc-closed-early: ${stderr}`);
assert.ok(!stderr.includes('RXT forced read error'), `the error was not caught (an uncaught throw exits 1): ${stderr}`);
// Control: the same command with no preload passes, so the block can come
// only from the catch.
await expectArgsAllowed(shipped);
// Control: a command with no delimiter passes under the preload.
const plain = await runHookWithEnv(PRE_BASH, bashInput('ls -la'), forced);
assert.strictEqual(plain.code, 0, `ls -la was refused under the preload: ${plain.stderr}`);
});
test('[CNT] a heredoc opener split between its two < still turns the escape rule off', async () => {
// Red: the outer opener <\ + line break + <EOF is a heredoc whose body is
// fed to bash, which reads the backslashes once more. Measured: /bin/bash
// 3.2.57 expands the inner body; zsh 5.9 gives a parse error. At e2d6bec the
// base's first-<< search keeps the rule off; on the branch the quote scan's
// continuation check does.
await expectUnquoted(`bash <\\\n<EOF\ncat - \\\\\\<<${D}\n\\$(touch PWNED)\n${D}\nEOF`);
});
test('part-A scan: a long command with many escaped < is read in linear time', async () => {
// About 870 KB: 20000 escaped < before the shipped quoted heredoc. The quote
// scan resumes where its last call stopped, so the text is read once.
const many = ('\\<' + 'x'.repeat(40) + ' ').repeat(20000);
const command = `echo ${many}<<'${D}'\nbrief.md\n${D}`;
const started = Date.now();
const { code, stderr } = await runHook(PRE_BASH, bashInput(command));
const elapsed = Date.now() - started;
assert.strictEqual(code, 0, `the guard did not pass the long command (exit ${code}): ${stderr.slice(0, 300)}`);
assert.ok(elapsed < 15000, `the guard took ${elapsed} ms on ${command.length} characters (limit 15000 ms)`);
});

View file

@ -114,8 +114,8 @@ test('[ESC] a backslash-escaped < starts no opener, and every real opener around
await expectUnquoted(`echo '\\'<<${D}${BODY}`);
// The same, continuation-split.
await expectUnquoted(`echo '\\'<<VOYAGE_ARGS\\\n_EOF_7F3C9E${BODY}`);
// A ' before an odd run: the rule is off.
await expectUnquoted(`echo 'x' && echo \\<<${D}${BODY}`);
// A " still open before an odd run: the rule is off.
await expectUnquoted(`echo 'x' && echo "y \\<<${D}${BODY}`);
// The << after the escaped <.
await expectUnquoted(`echo \\<<<${D}${BODY}`);
// A raw run of two: a line break ends the run, a continuation does not lengthen it.