fix(hooks): the args-heredoc guard judges a split delimiter as written out, also inside a string

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

hooks/scripts/pre-bash-executor.mjs

@@ -302,7 +302,8 @@ const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g,
 // same trim the body lines get, so a CR (a CRLF line), VT, FF or other
 // character trim() strips around the word does not hide it.
 // After a quoted opener that passes, a later opener is skipped only when its
-// word is cut by an unpaired quote and is readable otherwise: the shell meets
+// word is cut by an unpaired quote and is readable otherwise and not split by
+// a backslash-newline (fix round 4.5b2, part A, decision 126): the shell meets
 // the same unpaired quote, stops with a parse error and runs nothing after it,
 // 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
@@ -313,9 +314,12 @@ const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g,
 //     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.
+//     The quote has no partner, so the outer shell never reads the word's
+//     body as a heredoc body; a second shell (bash -c, eval) may, and the
+//     regex reading still reads such a << when the delimiter follows it. An
+//     opener whose raw span holds a backslash-newline has no regex match, so
+//     it is never skipped (fix round 4.5b2, part A, decision 123). Each
+//     opener is read on its own, so skipping one 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, when a quote scan of the text before the run ends
@@ -426,10 +430,6 @@ function argsQuoteScan(text, from, to, openers, state) {
         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;) {
@@ -487,10 +487,18 @@ function argsEscapedLt(text, i, lim, openers) {
   return b % 2 === 1 && argsQuoteScan(text, 0, s, openers, lim.state) === 'none';
 }
 
+// An opener whose raw span holds a backslash-newline. The regex reading cannot
+// see such an opener, so it is never skipped, as string text or as body text
+// (fix round 4.5b2, part A, decisions 123 and 126).
+function argsSplit(text, o) {
+  return text.slice(o.index, o.end).includes('\\\n');
+}
+
 // 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) {
+  if (argsSplit(text, o)) return false;
   const q = text[o.end];
   const lineStart = text.lastIndexOf('\n', o.index) + 1;
   return text.slice(lineStart, o.index).split(q).length % 2 === 0;
@@ -616,8 +624,27 @@ if (command.replace(/\\\n/g, '').includes(ARGS_DELIM)) {
     }
     let bodyFrom = Infinity;
     for (const o of argsShell) {
-      if (o.index >= bodyFrom && o.cutOnly) continue;
+      if (o.index >= bodyFrom && o.cutOnly && !argsSplit(command, o)) continue;
       if (o.cutOnly && argsCutInString(command, o)) continue;
+      // A delimiter split by a backslash-newline whose word has a suffix is
+      // judged as the regex reading judges the same opener written out (fix
+      // round 4.5b2, part A).
+      if (!o.unreadable && argsSplit(command, o)) {
+        const w = o.word.trim();
+        if (w.startsWith(ARGS_DELIM) && w.length > ARGS_DELIM.length) {
+          const m = new RegExp(`^<<-?\\s*(['"\\\\]?)${ARGS_DELIM}(['"]?)`).exec(command.slice(o.index, o.end).replace(/\\\n/g, ''));
+          if (!m) continue;
+          const snl = command.indexOf('\n', o.end);
+          if (snl === -1) continue;
+          const sbody = argsLines.slice(command.slice(0, snl).split('\n').length);
+          if (!sbody.some((line) => line.trim() !== '')) continue;
+          const [, sopen, sclose] = m;
+          const squoted = sopen === '\\' ? sclose === '' : sopen !== '' && sclose === sopen;
+          if (squoted && closesOnceAtEnd) continue;
+          argsVerdict = [!squoted, false];
+          break;
+        }
+      }
       // 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.

tests/hooks/bash-guard-4-5b2-a-known.test.mjs

@@ -0,0 +1,100 @@
+// tests/hooks/bash-guard-4-5b2-a-known.test.mjs
+// [KNA] Fix round 4.5b2, part A (decision 122): the false stops part A keeps.
+// The three commands below are refused by the args-heredoc guard in
+// hooks/scripts/pre-bash-executor.mjs although the shell reads their << as
+// text. Decision 122 dropped their fix; the CHANGELOG lists 943f9841 and
+// a6735ef9 under Known issues. This file holds at e2d6bec too, so it stands in
+// no red wrapper's file list and KNA is no red-proof id. It goes red if a later
+// change lets one of them through; its untagged mutation test shows that each
+// keep-assertion can go red.
+// The helpers bashInput, ARGS_DELIM, ARGS_RULE, expectArgsBlocked and D are
+// copied from tests/hooks/bash-guard-false-stops.test.mjs:26-68, not imported,
+// because importing a test file would run its tests.
+
+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/;
+
+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;
+}
+
+const D = ARGS_DELIM;
+
+// Catalogue K1 (943f9841): a grep string holding << right before the delimiter.
+const K1 = `grep -n '<<${D}' file\necho done`;
+// Catalogue K2 (a6735ef9): the review's commit message.
+const K2 = `git commit -m "don't use '<<' for ${D}" -m "second\nline"`;
+// Catalogue K3 (a6735ef9): the review's escaped-quote grep.
+const K3 = `echo "x\\"y" && grep "<<" f | grep ${D}\nnext`;
+
+test('[KNA] a grep string holding << right before the delimiter is still refused (943f9841)', async () => {
+  await expectArgsBlocked(K1);
+});
+
+test("[KNA] the review's commit message is still refused (a6735ef9)", async () => {
+  await expectArgsBlocked(K2);
+});
+
+test("[KNA] the review's escaped-quote grep is still refused (a6735ef9)", async () => {
+  await expectArgsBlocked(K3);
+});
+
+// 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 whose hook has `find` (asserted to occur exactly once)
+// replaced by `replace`; returns the copy's hook path.
+function mutatedHook(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}`);
+  writeFileSync(hook, src.replace(find, () => replace));
+  return hook;
+}
+
+async function expectPasses(name, command, hook) {
+  const { code, stderr } = await runHook(hook, bashInput(command));
+  assert.doesNotMatch(stderr, ARGS_RULE, `${name} was still refused under the mutation`);
+  assert.strictEqual(code, 0, `${name} did not exit 0 under the mutation: ${stderr}`);
+}
+
+test('part-A known: each keep-assertion goes red when the reading that refuses it lets it through', async () => {
+  // The regex reading off: its loop runs over no matches. K1 is refused only
+  // by the regex reading, so it passes; K2 and K3 stay refused.
+  const regexOff = mutatedHook('    for (const argsOpener of argsOpeners) {', '    for (const argsOpener of []) {');
+  await expectPasses('K1', K1, regexOff);
+  await expectArgsBlocked(K2, regexOff);
+  await expectArgsBlocked(K3, regexOff);
+
+  // The string count off: every cut opener counts as inside a string. K2 and
+  // K3 are refused only by the string count, so they pass; K1 stays refused.
+  const countOff = mutatedHook('  return text.slice(lineStart, o.index).split(q).length % 2 === 0;', '  return true;');
+  await expectPasses('K2', K2, countOff);
+  await expectPasses('K3', K3, countOff);
+  await expectArgsBlocked(K1, countOff);
+});

tests/hooks/bash-guard-4-5b2-a.test.mjs

@@ -1,5 +1,5 @@
 // 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).
+// [RXT] [CNT] [ESR] [SFX] [SIS] Fix round 4.5b2, part A (decisions 91 and 123).
 // 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
@@ -7,7 +7,10 @@
 // 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 escape rule off (CNT). A delimiter split by a backslash-newline with a
+// suffix after it was compared as an exact word, so it passed (SFX). Part 4's
+// string skip skipped a split opener inside a string, which the regex reading
+// cannot see (SIS).
 // The helpers bashInput, ARGS_DELIM, ARGS_RULE, PARSER_LINE,
 // expectArgsBlocked, expectArgsAllowed, UNQUOTED, expectUnquoted,
 // CLOSED_EARLY, expectClosedEarly, D and BODY are copied from
@@ -181,6 +184,65 @@ test('[CNT] a heredoc opener split between its two < still turns the escape rule
   await expectUnquoted(`bash <\\\n<EOF\ncat - \\\\\\<<${D}\n\\$(touch PWNED)\n${D}\nEOF`);
 });
 
+// The delimiter split by a backslash-newline into its two halves; the shell
+// removes the backslash-newline and reads the delimiter written out.
+const SPLIT = 'VOYAGE_ARGS\\\n_EOF_7F3C9E';
+// The body line of the split-suffix forms.
+const SBODY = '\nbody $(echo EXPANDED)';
+
+test('[SFX] a split delimiter with an X suffix is blocked as the same opener written out is', async () => {
+  // Red (catalogue SFX-X): the shell reads the word as the delimiter plus X,
+  // an unquoted heredoc. Measured: /bin/bash and /bin/zsh -f print
+  // body EXPANDED.
+  await expectUnquoted(`cat <<${SPLIT}X${SBODY}`);
+
+  // Control: the same opener written out is blocked as unquoted.
+  await expectUnquoted(`cat <<${D}X${SBODY}`);
+});
+
+test('[SFX] a split delimiter with a backquote suffix is blocked as the same opener written out is', async () => {
+  // Red (catalogue SFX-bq): measured, the form is a parse error in /bin/bash
+  // and /bin/zsh -f; it is blocked conservatively, as written out.
+  await expectUnquoted(`cat <<${SPLIT}\`${SBODY}`);
+
+  // Control: the same opener written out is blocked as unquoted.
+  await expectUnquoted(`cat <<${D}\`${SBODY}`);
+});
+
+test('[SFX] a split delimiter with a . suffix is blocked as the same opener written out is', async () => {
+  // Red (catalogue SFX-dot): the shell reads the word as the delimiter plus
+  // a dot, an unquoted heredoc. Measured: /bin/bash and /bin/zsh -f print
+  // body EXPANDED.
+  await expectUnquoted(`cat <<${SPLIT}.${SBODY}`);
+
+  // Control: the same opener written out is blocked as unquoted.
+  await expectUnquoted(`cat <<${D}.${SBODY}`);
+});
+
+test('[SIS] a split delimiter inside a string is never skipped as text', async () => {
+  // Measured: SIS1-SIS4 and SIS6 create PWNED under /bin/bash and
+  // /bin/zsh -f and pass the hook at e2d6bec and 1731e41. The inner shell
+  // removes the backslash-newline and opens an unquoted heredoc.
+  const RUN = '\n\\$(touch PWNED); touch RAN\n';
+  // Red (catalogue SIS1).
+  await expectUnquoted(`bash -c 'cat <<${SPLIT}'"${RUN}"`);
+
+  // Control (SIS2): a . after the split delimiter.
+  await expectUnquoted(`bash -c 'cat <<${SPLIT}.'"${RUN}"`);
+  // Control (SIS3): a word character after the split delimiter.
+  await expectUnquoted(`bash -c 'cat <<${SPLIT}X'"${RUN}"`);
+  // Control (SIS4): SIS1 with a closing line.
+  await expectUnquoted(`bash -c 'cat <<${SPLIT}'"${RUN}${D}"`);
+  // Control (SIS0): the delimiter written out, blocked already at e2d6bec.
+  await expectUnquoted(`bash -c 'cat <<${D}'"${RUN}"`);
+  // Control (SIS6): SIS1 in the body of a quoted heredoc fed to bash; the
+  // skip after a quoted opener that passes must not skip a split opener.
+  await expectUnquoted(`bash <<'${D}'\nbash -c 'cat <<${SPLIT}'"${RUN}"\n${D}`);
+  // Control that passes (SIS5): a split delimiter inside a grep string with
+  // no line after the command, so no body (decision 87's one-line rule).
+  await expectArgsAllowed(`grep -n '<<${SPLIT}' file`);
+});
+
 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.