fix(hooks): the args-heredoc guard checks a delimiter split by a backslash-newline, and its catch branch is tested

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

hooks/scripts/pre-bash-executor.mjs

@@ -264,8 +264,10 @@ const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g,
 // delimiter would close the heredoc there, and the shell would run every line
 // after it as a command (fix round 4.5b, R6). This check reads the raw
 // command: `normalized` and `commandView` collapse lines and drop heredoc
-// bodies (hooks/lib/bash-view.mjs). A command that never names the delimiter
-// skips the whole check.
+// bodies (hooks/lib/bash-view.mjs). The check runs when the command names the
+// delimiter once every backslash-newline is removed: the shell removes a
+// continuation inside the opener's word, and an unquoted body with no closing
+// line is still expanded (fix round 4.5b, part 4).
 // Every opener in the command is read in two ways, and the command is blocked
 // when either reading blocks it (the union):
 //   1. A regex finds <<D, <<'D', <<"D" and <<\D, with an optional - and
@@ -407,7 +409,7 @@ function argsShellOpeners(text) {
   return openers;
 }
 
-if (command.includes(ARGS_DELIM)) {
+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] : []));

tests/hooks/bash-guard-args-gate.test.mjs

@@ -0,0 +1,106 @@
+// tests/hooks/bash-guard-args-gate.test.mjs
+// [CSG] [CTB] Fix round 4.5b, part 4 (decision 90).
+// Defects: the args-heredoc guard in hooks/scripts/pre-bash-executor.mjs ran
+// its whole check only when the command held the delimiter literally, so an
+// unquoted opener whose delimiter is split by a backslash-newline, with a body
+// and no closing line, was never read: the guard exited 0 and the shell
+// expanded the body (CSG). And the branch that makes the guard fail closed
+// when its own shell reading throws had no test (CTB).
+// The helpers ARGS_DELIM, ARGS_RULE, PARSER_LINE, bashInput,
+// expectArgsBlocked, expectArgsAllowed, UNQUOTED and expectUnquoted are copied
+// from tests/hooks/bash-guard-opener-word.test.mjs:22-53, not imported,
+// because importing a test file would run its tests. The guard is only
+// spawned, so this file loads at 591f3c4 and its red is an assertion.
+
+import { test, after } from 'node:test';
+import { strict as assert } from 'node:assert';
+import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { dirname, join } from 'node:path';
+import { fileURLToPath, pathToFileURL } from 'node:url';
+import { runHook, runHookWithEnv } 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)}`);
+}
+
+const D = ARGS_DELIM;
+
+test('[CSG] the guard checks an opener whose delimiter is split by a backslash-newline, with a body and no closing line', async () => {
+  // Red: the command never holds the delimiter literally; the shell removes
+  // the continuation, reads the delimiter as the word and expands the body.
+  await expectUnquoted(`${PARSER_LINE} <<VOYAGE_ARGS\\\n_EOF_7F3C9E\nbrief.md $(touch PWNED)`);
+  await expectUnquoted(`${PARSER_LINE} <<VOYAGE_ARGS_EOF\\\n_7F3C9E\nbrief.md $(touch PWNED)`);
+
+  // Control: an ordinary unquoted heredoc that names no delimiter passes.
+  const { code, stderr } = await runHook(PRE_BASH, bashInput('cat <<EOF\n$(date)\nEOF'));
+  assert.doesNotMatch(stderr, ARGS_RULE, `the guard refused an ordinary heredoc: ${stderr}`);
+  assert.strictEqual(code, 0, `the guard refused an ordinary heredoc: ${stderr}`);
+});
+
+// The preload makes String.prototype.indexOf throw when it is called with '<'
+// from inside argsShellOpeners, so the shell reading throws. It is written
+// into a temp directory, never under tests/, and only makes the hook block.
+const PRELOAD = `const orig = String.prototype.indexOf;
+String.prototype.indexOf = function (search, ...rest) {
+  if (search === '<' && /argsShellOpeners/.test(new Error().stack)) {
+    process.stderr.write('[ctb-preload] thrown\\n');
+    throw new Error('CTB forced read error');
+  }
+  return orig.call(this, search, ...rest);
+};
+`;
+
+const preloadDir = mkdtempSync(join(tmpdir(), 'voyage-ctb-'));
+after(() => rmSync(preloadDir, { recursive: true, force: true }));
+
+test('[CTB] an error inside the shell reading blocks the command (exit 2), never lets it through', async () => {
+  const file = join(preloadDir, 'ctb-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('[ctb-preload] thrown'), `the preload did not throw inside the shell reading: ${stderr}`);
+  assert.strictEqual(code, 2, `the guard did not block when its shell reading threw (exit ${code}): ${stderr}`);
+  assert.match(stderr, ARGS_RULE, `the guard did not name args-heredoc-closed-early: ${stderr}`);
+  assert.ok(stderr.includes("The guard could not read the opener's word"), `the could-not-read suffix is missing: ${stderr}`);
+  assert.ok(!stderr.includes('CTB 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}`);
+});

tests/hooks/bash-guard-reader-pins.test.mjs

@@ -0,0 +1,193 @@
+// tests/hooks/bash-guard-reader-pins.test.mjs
+// Fix round 4.5b, part 4 (decision 90): part 3's five fix-round reader cases
+// (tests/e2e-fix-round-4-5b-part-3.sh:277-305) as pins under npm test, so a
+// later edit of the args-heredoc guard in hooks/scripts/pre-bash-executor.mjs
+// cannot drop them unseen. Untagged: they pass at 591f3c4, and no red
+// wrapper lists this file.
+// Each case is built from the same lines as the e2e case, joined with \n.
+// Three more tests prove the pins can fail: each mutates one line of a copy
+// of the hook (pluginCopy, tests/hooks/guard-helpers.mjs) and sees the pins it
+// guards flip. A later edit of a mutated line makes its test fail with
+// "matched 0 times", never pass silently.
+// The helpers ARGS_DELIM, ARGS_RULE, PARSER_LINE, bashInput,
+// expectArgsBlocked, expectArgsAllowed, UNQUOTED and expectUnquoted are copied
+// from tests/hooks/bash-guard-opener-word.test.mjs:22-53, not imported,
+// because importing a test file would run its tests; here they take the hook
+// to run as a second argument.
+
+import { test, after } from 'node:test';
+import { strict as assert } from 'node:assert';
+import { readFileSync, writeFileSync, rmSync } from 'node:fs';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { runHook } from '../helpers/hook-helper.mjs';
+import { 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, hook = PRE_BASH) {
+  const { code, stderr } = await runHook(hook, 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, hook = PRE_BASH) {
+  const { code, stderr } = await runHook(hook, 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, hook = PRE_BASH) {
+  const stderr = await expectArgsBlocked(command, hook);
+  assert.match(stderr, UNQUOTED, `the guard did not give the unquoted reason for: ${JSON.stringify(command)}`);
+}
+
+const D = ARGS_DELIM;
+const CR = '\r';
+const VT = '\x0b';
+const PARTLY = `${PARSER_LINE} <<"VOYAGE"_ARGS_EOF_7F3C9E`;
+
+// The five cases, numbered as the plan numbers them. `expect` is the normal
+// result; `cr` and `vt` are the bytes the case intends.
+const PINS = [
+  {
+    n: 1,
+    name: `Bash hook: <<$ + backslash-newline + '${D}' that closes early is blocked as unquoted`,
+    lines: [`${PARSER_LINE} <<$\\`, `'${D}'`, 'brief.md', D, 'touch PWNED'],
+    expect: 'unquoted',
+    cr: 0,
+    vt: 0,
+  },
+  {
+    n: 2,
+    name: "Bash hook: <<VOYAGE$ + backslash-newline + '_ARGS_EOF_7F3C9E' that closes early is blocked",
+    lines: [`${PARSER_LINE} <<VOYAGE$\\`, "'_ARGS_EOF_7F3C9E'", 'brief.md', D, 'touch PWNED'],
+    expect: 'blocked',
+    cr: 0,
+    vt: 0,
+  },
+  {
+    n: 3,
+    name: 'Bash hook: a partly quoted opener with CRLF lines that closes early is blocked',
+    lines: [`${PARTLY}${CR}`, `brief.md${CR}`, `${D}${CR}`, `touch PWNED${CR}`],
+    expect: 'blocked',
+    cr: 4,
+    vt: 0,
+  },
+  {
+    n: 4,
+    name: 'Bash hook: a partly quoted opener whose word ends in a VT, closed by D + VT, is blocked',
+    lines: [`${PARTLY}${VT}`, 'brief.md', `${D}${VT}`, 'touch PWNED'],
+    expect: 'blocked',
+    cr: 0,
+    vt: 2,
+  },
+  {
+    n: 5,
+    name: 'Bash hook: a partly quoted opener with CRLF lines and the closing line last passes',
+    lines: [`${PARTLY}${CR}`, `brief.md${CR}`, `${D}${CR}`],
+    expect: 'allowed',
+    cr: 3,
+    vt: 0,
+  },
+];
+
+const commandOf = (pin) => pin.lines.join('\n');
+const pin = (n) => PINS.find((p) => p.n === n);
+
+async function expectNormal(p, hook = PRE_BASH) {
+  const command = commandOf(p);
+  if (p.expect === 'unquoted') await expectUnquoted(command, hook);
+  else if (p.expect === 'blocked') await expectArgsBlocked(command, hook);
+  else await expectArgsAllowed(command, hook);
+}
+
+for (const p of PINS) {
+  test(`part-4 reader pins: ${p.name}`, async () => {
+    await expectNormal(p);
+  });
+}
+
+test('part-4 reader pins: each built command holds exactly the CR and VT bytes its case intends', () => {
+  for (const p of PINS) {
+    const command = commandOf(p);
+    assert.strictEqual(command.split(CR).length - 1, p.cr, `pin ${p.n} holds the wrong number of CR bytes`);
+    assert.strictEqual(command.split(VT).length - 1, p.vt, `pin ${p.n} holds the wrong number of VT bytes`);
+  }
+});
+
+// The copies, removed after the file's tests.
+const copies = [];
+after(() => {
+  for (const dir of copies) rmSync(dir, { recursive: true, force: true });
+});
+
+// A copy of the plugin and its hook path; `find` must occur exactly once in
+// the copy's hook. `mutate()` then writes `replace` in its place, so the same
+// hook is run before and after the mutation.
+function mutatedCopy(find, replace) {
+  const root = pluginCopy();
+  copies.push(root);
+  const hook = join(root, 'hooks', 'scripts', 'pre-bash-executor.mjs');
+  const src = readFileSync(hook, 'utf8');
+  const count = src.split(find).length - 1;
+  assert.strictEqual(count, 1, `the mutation text matched ${count} times, not once: ${find}`);
+  return { hook, mutate: () => writeFileSync(hook, src.replace(find, () => replace)) };
+}
+
+async function expectPasses(p, hook) {
+  const { code, stderr } = await runHook(hook, bashInput(commandOf(p)));
+  assert.doesNotMatch(stderr, ARGS_RULE, `pin ${p.n} was still blocked under the mutation`);
+  assert.strictEqual(code, 0, `pin ${p.n} did not exit 0 under the mutation: ${stderr}`);
+}
+
+const MUTATIONS = [
+  {
+    name: 'pins 1 and 2 go red under const q1 = k + 1',
+    find: "const q1 = c === '$' ? skip(k + 1) : k + 1;",
+    replace: 'const q1 = k + 1;',
+    pins: [1, 2],
+    flipped: async (p, hook) => expectPasses(p, hook),
+  },
+  {
+    name: 'pins 3 and 4 go red under the untrimmed word comparison',
+    find: 'o.word.trim() !== ARGS_DELIM',
+    replace: 'o.word !== ARGS_DELIM',
+    pins: [3, 4],
+    flipped: async (p, hook) => expectPasses(p, hook),
+  },
+  {
+    name: 'pin 5 goes red under the untrimmed closing-line count',
+    find: 'body.flatMap((l, i) => (l.trim() === ARGS_DELIM',
+    replace: 'body.flatMap((l, i) => (l === ARGS_DELIM',
+    pins: [5],
+    flipped: async (p, hook) => {
+      await expectArgsBlocked(commandOf(p), hook);
+    },
+  },
+];
+
+for (const m of MUTATIONS) {
+  test(`part-4 reader pins: ${m.name}`, async () => {
+    const { hook, mutate } = mutatedCopy(m.find, m.replace);
+    // The unmutated copy gives each pin its normal result, so a broken copy
+    // cannot pass for a flip.
+    for (const n of m.pins) await expectNormal(pin(n), hook);
+    mutate();
+    for (const n of m.pins) await m.flipped(pin(n), hook);
+  });
+}