fix(hooks): the args-heredoc guard lets through a heredoc operator the shell reads as text

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-31 16:42:30 +01:00
commit 9f7ba6503f
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
3 changed files with 417 additions and 15 deletions

View file

@ -307,6 +307,23 @@ const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g,
// so a typed argument such as fix "<<" handling passes. Every other later
// opener is checked, because the accepted opener may sit inside a string and
// must not hide a real opener after it.
// Two kinds of << are text to the shell and are skipped (fix round 4.5b,
// part 4):
// - A cut word whose quote closes a string opened earlier on its line (an
// odd count of that quote from the line start to the opener's first <),
// as in grep -n '<<' or a commit message holding "<<". A cut word with
// no string open before it stays unreadable and is blocked as unquoted.
// The skip is safe: the quote has no partner, so the word's body is
// never read as a heredoc body, and each opener is read on its own, so
// 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.
// 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';
@ -326,6 +343,34 @@ 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 <).
function argsEscapeLimit(text) {
const h = /<(?:\\\n)*</.exec(text);
return { quoteAt: text.search(/['"`]/), heredocEnd: h ? h.index + h[0].length : -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) {
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);
}
// A cut opener (its quote has no partner after it) whose quote closes a
// string opened earlier on the raw line holding the opener's first <: an
// odd count of that quote character from the line start to the <.
function argsCutInString(text, o) {
const q = text[o.end];
const lineStart = text.lastIndexOf('\n', o.index) + 1;
return text.slice(lineStart, o.index).split(q).length % 2 === 0;
}
// Each heredoc operator in `text`, in text order, as the shell reads its word:
// { index (the operator's first <), end, word, quoted, unreadable, cutOnly }.
function argsShellOpeners(text) {
@ -336,8 +381,10 @@ function argsShellOpeners(text) {
return k;
};
const openers = [];
const lim = argsEscapeLimit(text);
let i = text.indexOf('<');
while (i !== -1) {
if (argsEscapedLt(text, i, lim)) { i = text.indexOf('<', i + 1); continue; }
let next = i + 1;
let k = skip(i + 1);
if (text[k] === '<') {
@ -410,26 +457,29 @@ function argsShellOpeners(text) {
}
if (command.replace(/\\\n/g, '').includes(ARGS_DELIM)) {
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) {
// 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;
const [, open, close] = argsOpener;
const quoted = open === '\\' ? close === '' : open !== '' && close === open;
if (!quoted || !closesOnceAtEnd) blockArgs(!quoted, false);
}
// The shell reading. The loop records the first failing verdict; an error
// anywhere in it blocks.
// The regex reading and the shell reading. 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 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;
// 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;
const [, open, close] = argsOpener;
const quoted = open === '\\' ? close === '' : open !== '' && close === open;
if (!quoted || !closesOnceAtEnd) blockArgs(!quoted, false);
}
let bodyFrom = Infinity;
for (const o of argsShellOpeners(command)) {
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
// characters trim() strips (a CR from a CRLF line, a VT, an FF, a
// no-break space) is closed by a line equal to it, which trims to D.

View file

@ -0,0 +1,219 @@
#!/usr/bin/env bash
# tests/e2e-fix-round-4-5b-part-4.sh
# End to end for fix round 4-5b, part 4 (plan
# .claude/projects/2026-10-31-fix-round-4-5b-part-4, End-to-end:, decision
# 90): the Bash hook (hooks/scripts/pre-bash-executor.mjs) reads commands the
# way a session sends them, and /bin/bash shows in a temp directory that each
# block is real and each pass is text to the shell.
# 1. CSG: a continuation-split unquoted opener with a body and no closing
# line is blocked as unquoted; /bin/bash expands its body.
# 2. SLT: a grep pattern '<<' on a line that names the delimiter, and a
# string holding << before the shipped argument heredoc, pass.
# 3. ESC: a backslash-escaped < starts no opener: the hook passes it, and
# /bin/bash runs the next line as a command.
# 4. ESC, the heredoc fed to a shell: an odd run of backslashes after a <<
# is still blocked as unquoted; /bin/bash expands the inner body.
# 5. The shipped form still passes: the parser block of
# commands/trekreview.md (## Phase 1), filled with the repo root and the
# typed argument fix "<<" handling.
# Prints one "ok - <case>" or "FAIL - <case>: <why>" line per case and exits 1
# when any case failed, else 0. Starts nothing but node and /bin/bash (and the
# usual file tools). bash 3.2 compatible, ASCII only, no arrays.
set -u
cd "$(dirname "$0")/.." || { echo "FAIL - setup: cannot cd to the repo root"; exit 1; }
ROOT="$(pwd)"
HOOK="$ROOT/hooks/scripts/pre-bash-executor.mjs"
unset CLAUDE_CODE_SESSION_ID
TMP="$(mktemp -d "${TMPDIR:-/tmp}/e2e-fix-round-4-5b-part-4.XXXXXX")" || { echo "FAIL - setup: mktemp"; exit 1; }
trap 'rm -rf "$TMP"' EXIT
TMP="$(cd "$TMP" && pwd -P)" || { echo "FAIL - setup: cannot enter the temp directory"; exit 1; }
export LC_ALL=C
export LANG=C
FAILED=0
pass() { echo "ok - $1"; }
bad() { echo "FAIL - $1"; FAILED=$((FAILED + 1)); }
# hookrun <out> <line>...: the Bash hook with a Bash tool input whose command
# is the given lines joined by newlines; stderr goes to <out>. Returns the
# hook's exit status.
hookrun() {
out="$1"
shift
node -e 'process.stdout.write(JSON.stringify({ tool_name: "Bash", tool_input: { command: process.argv.slice(1).join("\n") } }));' "$@" \
| node "$HOOK" > /dev/null 2> "$out"
}
# hookfile <out> <file>: the Bash hook with the file's text as the command.
hookfile() {
node -e 'process.stdout.write(JSON.stringify({ tool_name: "Bash", tool_input: { command: require("fs").readFileSync(process.argv[1], "utf8") } }));' "$2" \
| node "$HOOK" > /dev/null 2> "$1"
}
D=VOYAGE_ARGS_EOF_7F3C9E
PARSER_LINE='node /plugin/lib/parsers/arg-parser.mjs --command trekendsession --stdin'
# blocked <name> <out> <line>...: the hook exits 2 and names args-heredoc-closed-early.
blocked() {
bname="$1"
bout="$2"
shift 2
hookrun "$bout" "$@"
c=$?
if [ "$c" -ne 2 ]; then
bad "$bname: the hook exited $c, not 2: $(cat "$bout")"
return 1
elif ! grep -q 'args-heredoc-closed-early' "$bout"; then
bad "$bname: the hook did not name args-heredoc-closed-early: $(cat "$bout")"
return 1
fi
return 0
}
# unquoted <name> <out> <line>...: blocked, with the unquoted reason.
unquoted() {
qname="$1"
uout="$2"
shift 2
blocked "$qname" "$uout" "$@" || return 1
if ! grep -q "opens $D unquoted" "$uout"; then
bad "$qname: the hook did not give the unquoted reason: $(cat "$uout")"
return 1
fi
return 0
}
# allowed <name> <out> <line>...: the hook exits 0.
allowed() {
aname="$1"
aout="$2"
shift 2
hookrun "$aout" "$@"
c=$?
if [ "$c" -ne 0 ]; then
bad "$aname: the hook exited $c: $(cat "$aout")"
return 1
fi
return 0
}
# premise <dir> <script> <out> <line>...: writes the lines to <script> and
# runs it with /bin/bash in <dir>; stdout and stderr go to <out>.
premise() {
pdir="$1"
pscript="$2"
pout="$3"
shift 3
mkdir -p "$pdir" || return 1
printf '%s\n' "$@" > "$pscript" || return 1
( cd "$pdir" && /bin/bash "$pscript" ) > "$pout" 2>&1 < /dev/null
return 0
}
# ---- 1. CSG ----
case_csg() {
name='CSG: a continuation-split unquoted opener with a body and no closing line is blocked as unquoted'
L1='cat <<VOYAGE_ARGS\'
L2='_EOF_7F3C9E'
L3='$(touch "'"$TMP"'/ran-csg")'
unquoted "$name" "$TMP/csg.err" "$L1" "$L2" "$L3" || return
premise "$TMP/csg" "$TMP/csg.sh" "$TMP/csg.out" "$L1" "$L2" "$L3" \
|| { bad "$name: could not write the premise script"; return; }
if [ ! -e "$TMP/ran-csg" ]; then
bad "$name: /bin/bash did not expand the body: $(cat "$TMP/csg.out")"
else
pass "$name (exit 2; /bin/bash expands the body, so the block is not a false positive)"
fi
}
# ---- 2. SLT ----
case_slt() {
name="SLT: a grep pattern '<<' that names the delimiter, and a string holding << before the shipped heredoc, pass"
allowed "$name (grep)" "$TMP/slt-1.err" "grep -n '<<' file.md | grep $D" 'echo done' || return
allowed "$name (string)" "$TMP/slt-2.err" "echo \"a<<b\" && $PARSER_LINE <<'$D'" 'brief.md' "$D" || return
pass "$name (exit 0 for each)"
}
# ---- 3. ESC ----
case_esc() {
name='ESC: a backslash-escaped < starts no opener'
L1='echo \<<'"$D"
L2='echo next'
allowed "$name" "$TMP/esc.err" "$L1" "$L2" || return
premise "$TMP/esc" "$TMP/esc.sh" "$TMP/esc.out" "$L1" "$L2" \
|| { bad "$name: could not write the premise script"; return; }
if ! grep -qx 'next' "$TMP/esc.out"; then
bad "$name: /bin/bash did not run the second line as a command: $(cat "$TMP/esc.out")"
else
pass "$name (exit 0; /bin/bash prints next, so no heredoc took the second line)"
fi
}
# ---- 4. ESC, the heredoc fed to a shell ----
case_esc_fed() {
name='ESC: an odd run of backslashes after a << in a heredoc fed to a shell is still blocked as unquoted'
L1='/bin/bash <<EOF'
L2='cat - \\\<<'"$D"
L3='\$(touch "'"$TMP"'/ran-esc")'
L4="$D"
L5='EOF'
unquoted "$name" "$TMP/esc-fed.err" "$L1" "$L2" "$L3" "$L4" "$L5" || return
premise "$TMP/esc-fed" "$TMP/esc-fed.sh" "$TMP/esc-fed.out" "$L1" "$L2" "$L3" "$L4" "$L5" \
|| { bad "$name: could not write the premise script"; return; }
if [ ! -e "$TMP/ran-esc" ]; then
bad "$name: /bin/bash did not expand the inner body: $(cat "$TMP/esc-fed.out")"
else
pass "$name (exit 2; /bin/bash expands the inner body, so the block is not a false positive)"
fi
}
# ---- 5. The shipped form still passes ----
case_shipped() {
name='shipped: the trekreview Phase 1 parser block with the typed argument fix "<<" handling passes'
# The block as written, with ${CLAUDE_PLUGIN_ROOT} and $ARGUMENTS filled in
# the way Claude Code fills them.
node -e '
const fs = require("fs");
const [md, root, args] = process.argv.slice(1);
const text = fs.readFileSync(md, "utf8");
const s = text.indexOf("## Phase 1 ");
if (s === -1) process.exit(3);
const rest = text.slice(s);
const e = rest.search(/\n## /);
const phase = e === -1 ? rest : rest.slice(0, e);
const blocks = phase.split("\n```bash\n").slice(1).map((b) => b.split("\n```")[0]).filter((b) => b.includes("<<" + String.fromCharCode(39) + "VOYAGE_ARGS_EOF_7F3C9E" + String.fromCharCode(39)));
if (blocks.length !== 1) { console.error("expected one Phase 1 parser block, found " + blocks.length); process.exit(3); }
process.stdout.write(blocks[0].split("${CLAUDE_PLUGIN_ROOT}").join(root).split("$ARGUMENTS").join(args) + "\n");
' "$ROOT/commands/trekreview.md" "$ROOT" 'fix "<<" handling' > "$TMP/phase1.sh" \
|| { bad "$name: could not read the Phase 1 block"; return; }
hookfile "$TMP/shipped.err" "$TMP/phase1.sh"
c=$?
if [ "$c" -ne 0 ]; then
bad "$name: the hook exited $c: $(cat "$TMP/shipped.err")"
else
pass "$name (exit 0)"
fi
}
case_csg
case_slt
case_esc
case_esc_fed
case_shipped
if [ "$FAILED" -ne 0 ]; then
echo "e2e: $FAILED case(s) failed"
exit 1
fi
echo "e2e: ok"
exit 0

View file

@ -0,0 +1,133 @@
// tests/hooks/bash-guard-false-stops.test.mjs
// [SLT] [ESC] Fix round 4.5b, part 4 (decision 90).
// Defects: the args-heredoc guard in hooks/scripts/pre-bash-executor.mjs
// refused commands in which the shell reads a << as text. A << inside a
// string, whose cutting quote closes that string, was read as an unreadable
// opener and blocked (SLT). A backslash-escaped < was read as the start of an
// opener (ESC); and the shell reading took an escaped < plus the real << for a
// here-string, so a real opener after it was not checked. The guard must skip
// both, and still check every real opener around them.
// The helpers ARGS_DELIM, ARGS_RULE, PARSER_LINE, bashInput,
// expectArgsBlocked, expectArgsAllowed, UNQUOTED and expectUnquoted are copied
// from tests/hooks/bash-guard-args-gate.test.mjs:27-54, not imported, because
// importing a test file would run its tests. The guard is only spawned, so
// this file loads at 12bca8b and its red is an assertion.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { runHook } from '../helpers/hook-helper.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 of the forms with a << after an escaped <.
const HDBODY = 'HDBODY #$(touch PWNED1)';
// The body line and the closing line of the ESC controls.
const BODY = `\nbrief.md $(touch PWNED)\n${D}`;
test('[SLT] a commit message whose string holds "<<" passes', async () => {
// Red: the << sits in a "..." string; its cutting " closes that string.
await expectArgsAllowed(`git commit -F - <<'EOF'\nfix: let fix "<<" handling pass for ${D}\n\nbody\nEOF`);
// Control: no string is open before the cut quote (the regex reading blocks it too).
await expectUnquoted(`${PARSER_LINE} <<"${D}\nbrief.md`);
// Control: an even, non-zero count of " before the <, a spelling the regex
// reading cannot see, stays blocked.
await expectUnquoted(`echo "x" && ${PARSER_LINE} <<V"OYAGE_ARGS_EOF_7F3C9E\nbrief.md\n${D}`);
});
test("[SLT] a grep pattern '<<' on a line that names the delimiter passes", async () => {
// Red: the << sits in a '...' string; its cutting ' closes that string.
await expectArgsAllowed(`grep -n '<<' file.md | grep ${D}\necho done`);
});
test('[SLT] a file write whose body holds "<<" in a string passes', async () => {
// Red: a heredoc body line holds "<<" in a string.
await expectArgsAllowed(`cat <<'EOF' > t.mjs\nconst s = "<<";\nconst d = '${D}';\nEOF`);
});
test('[SLT] a string holding << before the shipped argument heredoc passes, and hides no later opener', async () => {
// Red: the string's << is text; the shipped heredoc after it passes.
await expectArgsAllowed(`echo "a<<b" && ${PARSER_LINE} <<'${D}'\nbrief.md\n${D}`);
// Control: the skipped string hides no continuation-split unquoted opener after it.
await expectUnquoted(`echo "a<<b" && ${PARSER_LINE} <<VOYAGE_ARGS\\\n_EOF_7F3C9E\nbrief.md $(touch PWNED)\n${D}`);
});
test('[ESC] a backslash-escaped < starts no opener, and every real opener around it is still checked', async () => {
// Red: \<< is a literal < and a lone <; the next line runs as a command.
await expectArgsAllowed(`echo \\<<${D}\necho next`);
// Red: the << after an escaped < is read as usual, so each is blocked.
await expectUnquoted(`cat \\<<<VOYAGE_ARGS\\\n_EOF_7F3C9E\n${HDBODY}`);
await expectUnquoted(`cat \\<\\\n<<VOYAGE_ARGS\\\n_EOF_7F3C9E\n${HDBODY}`);
await expectUnquoted(`cat \\<<\\\n<${D}\n${HDBODY}\n${D}`);
await expectUnquoted(`cat \\<<<VOYAGE_ARGS\\\n_EOF\\\n_7F3C9E\n${HDBODY}\n${D}`);
await expectClosedEarly(`cat \\<<<V"OYAGE_ARGS_EOF_7F3C9E"\nx\n${D}\ntouch PWNED2`);
// Control: the opener line, the body line and the closing line.
// An even run of backslashes: the < is not escaped.
await expectUnquoted(`echo \\\\<<${D}${BODY}`);
// A quote right before the <, no backslash run.
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}`);
// 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.
await expectUnquoted(`echo a\\\\\n\\\\<<${D}${BODY}`);
// Control: exactly the lines given.
// A " before an odd run of three: inside "..." \< keeps its backslash, so
// the inner shell opens an unquoted heredoc.
await expectUnquoted(`bash -c "cat \\\\\\<<${D}\n\\$(touch PWNED)\n${D}"`);
// A backquote before an odd run of three.
await expectUnquoted(`x=\`cat - \\\\\\<<${D}\n$(echo EXPANDED)\n${D}\``);
// A heredoc fed to a shell: the outer heredoc strips one backslash level
// (operator decision 2026-10-31).
await expectUnquoted(`bash <<EOF\ncat - \\\\\\<<${D}\n\\$(touch PWNED)\n${D}\nEOF`);
});