fix(arg-parser): take the arguments as stdin text, and block a heredoc that closes early

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-26 15:40:21 +01:00
commit f1704f0800
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
4 changed files with 287 additions and 16 deletions

View file

@ -258,6 +258,43 @@ const deobfuscated = normalizeBashExpansion(command);
const normalized = normalizeCommand(deobfuscated); const normalized = normalizeCommand(deobfuscated);
const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g, '')); const commandView = commandPositionView(deobfuscated.replace(/\x1B\[[0-9;]*m/g, ''));
// BEGIN args-heredoc-closed-early
// The commands hand $ARGUMENTS to lib/parsers/arg-parser.mjs (--stdin) through
// a quoted heredoc closed by VOYAGE_ARGS_EOF_7F3C9E. A typed line equal to that
// 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). Openers <<'D', <<"D" and <<\D are quoted
// (the body is not expanded); any other opener of the delimiter is treated as
// unquoted and blocked, since its body would expand. A quoted opener passes
// only when exactly one line, trimmed, equals the delimiter and every line
// after it is empty.
const ARGS_DELIM = 'VOYAGE_ARGS_EOF_7F3C9E';
const argsOpener = /<<-?\s*(['"\\]?)VOYAGE_ARGS_EOF_7F3C9E(['"]?)/.exec(command);
if (argsOpener) {
const [, open, close] = argsOpener;
const quoted = (open === '\\' && close === '') || (open !== '' && open !== '\\' && close === open);
const lines = command.split('\n');
const closing = [];
lines.forEach((line, i) => { if (line.trim() === ARGS_DELIM) closing.push(i); });
const closesOnceAtEnd = closing.length === 1 &&
lines.slice(closing[0] + 1).every((line) => line.trim() === '');
if (!quoted || !closesOnceAtEnd) {
const why = !quoted
? `The heredoc that hands the arguments to the arg parser opens ${ARGS_DELIM} unquoted, so the shell would expand its body. ` +
`Way forward: open it as <<'${ARGS_DELIM}'.`
: `The arguments hold a line equal to the heredoc's closing line ${ARGS_DELIM}, or text follows that closing line, ` +
'so the shell would run what follows as commands. Way forward: take that line out of the arguments and run the command again.';
process.stderr.write(
'[voyage] BLOCKED: args-heredoc-closed-early\n' +
` Command: ${normalized.slice(0, 200)}${normalized.length > 200 ? '...' : ''}\n` +
` ${why}\n`
);
process.exit(2);
}
}
// END args-heredoc-closed-early
// Check BLOCK rules first // Check BLOCK rules first
for (const rule of BLOCK_RULES) { for (const rule of BLOCK_RULES) {
if (rule.pattern.test(rule.commandView ? commandView : normalized)) { if (rule.pattern.test(rule.commandView ? commandView : normalized)) {

View file

@ -3,7 +3,14 @@
// //
// Each command has its own valid-flag set; passing flags from another command // Each command has its own valid-flag set; passing flags from another command
// produces an `unknown_flags` array but does not error — the caller decides. // produces an `unknown_flags` array but does not error — the caller decides.
//
// The commands hand $ARGUMENTS to the CLI as stdin text through a quoted
// heredoc (`--stdin`), never as shell words, so nothing in the arguments is
// expanded: no `~`, no `$VAR` or `$HOME`, no glob, no command substitution,
// and `;` is an ordinary character. Write a path out in full; `~/x` stays the
// three characters `~/x`. Quoting is the parser's own (see `tokenize`).
import { readFileSync } from 'node:fs';
import { isMain } from '../util/is-main.mjs'; import { isMain } from '../util/is-main.mjs';
const FLAG_SCHEMA = { const FLAG_SCHEMA = {
@ -54,9 +61,14 @@ const FLAG_SCHEMA = {
* positional: string[], * positional: string[],
* unknown: string[], * unknown: string[],
* errors: Array<{code: string, message: string}>, * errors: Array<{code: string, message: string}>,
* tail: string,
* }} * }}
* `tail` is the raw text after the first token, trimmed, with one balanced
* quote pair around the whole of it removed ('' when there is none): a
* label read whole, with its own spaces and apostrophes.
*/ */
export function parseArgs(argString, command) { export function parseArgs(argString, command) {
const tail = tailOf(argString);
const schema = FLAG_SCHEMA[command]; const schema = FLAG_SCHEMA[command];
if (!schema) { if (!schema) {
return { return {
@ -65,10 +77,11 @@ export function parseArgs(argString, command) {
positional: [], positional: [],
unknown: [], unknown: [],
errors: [{ code: 'ARG_UNKNOWN_COMMAND', message: `Unknown command: ${command}` }], errors: [{ code: 'ARG_UNKNOWN_COMMAND', message: `Unknown command: ${command}` }],
tail,
}; };
} }
const tokens = tokenize(argString); const tokens = tokenize(argString).map((t) => t.value);
const flags = {}; const flags = {};
const positional = []; const positional = [];
const unknown = []; const unknown = [];
@ -115,22 +128,61 @@ export function parseArgs(argString, command) {
unknown.push(tok); unknown.push(tok);
} }
return { command, flags, positional, unknown, errors }; return { command, flags, positional, unknown, errors, tail };
} }
function tokenize(s) { const isSpace = (c) => /\s/.test(c);
if (typeof s !== 'string') return [];
const trimmed = s.trim(); /**
if (trimmed === '') return []; * Split argument text into words. Words are separated by whitespace. A word
* that starts with `'` or `"` and has the same quote again at the end of a
* later word is a balanced pair: the pair groups the words between them into
* one value and is removed. A quote with no such partner is a literal
* character (`user's` stays `user's`). Nothing is expanded.
*
* @param {string} text
* @returns {Array<{ value: string, start: number, end: number }>}
* `start` / `end` are offsets into `text` (end exclusive), quotes included.
*/
export function tokenize(text) {
if (typeof text !== 'string') return [];
const out = []; const out = [];
const re = /"([^"]*)"|'([^']*)'|(\S+)/g; const n = text.length;
let m; let i = 0;
while ((m = re.exec(trimmed)) !== null) { while (i < n) {
out.push(m[1] !== undefined ? m[1] : m[2] !== undefined ? m[2] : m[3]); while (i < n && isSpace(text[i])) i++;
if (i >= n) break;
const start = i;
const q = text[i];
if (q === '"' || q === "'") {
let j = text.indexOf(q, i + 1);
while (j !== -1 && j + 1 < n && !isSpace(text[j + 1])) j = text.indexOf(q, j + 1);
if (j !== -1) {
out.push({ value: text.slice(i + 1, j), start, end: j + 1 });
i = j + 1;
continue;
}
}
while (i < n && !isSpace(text[i])) i++;
out.push({ value: text.slice(start, i), start, end: i });
} }
return out; return out;
} }
// The raw text after the first token, trimmed; one balanced quote pair that
// encloses the whole of it is removed. '' when there is no second token.
function tailOf(text) {
const tokens = tokenize(text);
if (tokens.length === 0) return '';
const rest = text.slice(tokens[0].end).trim();
const inner = tokenize(rest);
if (inner.length === 1 && inner[0].start === 0 && inner[0].end === rest.length
&& (rest[0] === '"' || rest[0] === "'") && inner[0].value.length === rest.length - 2) {
return inner[0].value;
}
return rest;
}
export { FLAG_SCHEMA }; export { FLAG_SCHEMA };
// Flags a command cannot run without, and its usage line. The CLI below emits // Flags a command cannot run without, and its usage line. The CLI below emits
@ -144,16 +196,41 @@ const REQUIRED = {
}, },
}; };
// CLI: node lib/parsers/arg-parser.mjs --command <command> -- $ARGUMENTS // CLI: node lib/parsers/arg-parser.mjs --command <command> --stdin <<'VOYAGE_ARGS_EOF_7F3C9E'
// stdout: the parse result as JSON. exit 0, or exit 1 with each error as // $ARGUMENTS
// "Error: <message>" plus the usage line on stderr. // VOYAGE_ARGS_EOF_7F3C9E
// reads the whole stdin as the argument text (one trailing newline
// removed). The quoted heredoc keeps the shell from expanding anything; the
// Bash guard (hooks/scripts/pre-bash-executor.mjs, args-heredoc-closed-early)
// refuses a command in which that heredoc would close early.
// CLI: node lib/parsers/arg-parser.mjs --command <command> -- <word>...
// the older argv form, kept: the words after `--` are the arguments.
// stdout: the parse result as JSON (with `tail`). exit 0, or exit 1 with
// each error as "Error: <message>" plus the usage line on stderr.
if (isMain(import.meta.url)) { if (isMain(import.meta.url)) {
const argv = process.argv.slice(2); const argv = process.argv.slice(2);
const ci = argv.indexOf('--command'); const ci = argv.indexOf('--command');
const command = ci >= 0 ? argv[ci + 1] : undefined; const command = ci >= 0 ? argv[ci + 1] : undefined;
const dd = argv.indexOf('--'); const dd = argv.indexOf('--');
const rest = dd >= 0 ? argv.slice(dd + 1) : []; // `--stdin` counts only before `--`: after it, it is an argument word.
const r = parseArgs(rest.map((t) => (/\s/.test(t) ? `"${t}"` : t)).join(' '), command); const options = dd >= 0 ? argv.slice(0, dd) : argv;
let r;
if (options.includes('--stdin')) {
let text = '';
let readError = null;
try {
text = readFileSync(0, 'utf8');
} catch (e) {
readError = e;
}
r = parseArgs(text.replace(/\r?\n$/, ''), command);
if (readError) {
r.errors.push({ code: 'ARG_STDIN_UNREADABLE', message: `could not read the arguments from stdin (${readError.code || readError.message})` });
}
} else {
const rest = dd >= 0 ? argv.slice(dd + 1) : [];
r = parseArgs(rest.map((t) => (/\s/.test(t) ? `"${t}"` : t)).join(' '), command);
}
const req = REQUIRED[command]; const req = REQUIRED[command];
if (req) { if (req) {
for (const [flag, metavar] of req.flags) { for (const [flag, metavar] of req.flags) {

View file

@ -9,6 +9,7 @@
import { test } from 'node:test'; import { test } from 'node:test';
import { strict as assert } from 'node:assert'; import { strict as assert } from 'node:assert';
import { readFileSync } from 'node:fs';
import { dirname, join } from 'node:path'; import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import { runHook } from '../helpers/hook-helper.mjs'; import { runHook } from '../helpers/hook-helper.mjs';
@ -314,3 +315,70 @@ test('pre-bash-executor fails open on malformed JSON', async () => {
const { code } = await runHook(PRE_BASH, 'not-json'); const { code } = await runHook(PRE_BASH, 'not-json');
assert.strictEqual(code, 0); assert.strictEqual(code, 0);
}); });
// -----------------------------------------------------------------------
// ARG / R6 (fix round 4.5b) -- the parser heredoc must not close early.
// The commands hand $ARGUMENTS to the arg parser through a quoted heredoc
// closed by VOYAGE_ARGS_EOF_7F3C9E. A typed line equal to that delimiter
// would end the heredoc there, and the shell would run every line after it
// as a command. Each blocking case asserts the rule's name on stderr, so it
// is red on its own assertion where the rule does not exist.
// -----------------------------------------------------------------------
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)}`);
}
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}`);
}
test('[ARG] the guard blocks a parser heredoc whose closing line is followed by a command', async () => {
await expectArgsBlocked(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md\n${ARGS_DELIM}\ntouch PWNED`);
await expectArgsBlocked(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n`);
});
test('[ARG] the guard blocks a parser heredoc with the closing line twice', async () => {
await expectArgsBlocked(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n${ARGS_DELIM}`);
// A typed line with spaces around the delimiter still counts as a closing line.
await expectArgsBlocked(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md\n ${ARGS_DELIM} \ntouch PWNED\n${ARGS_DELIM}\n`);
});
test('[ARG] the guard allows the parser heredoc as the commands write it', async () => {
await expectArgsAllowed(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md Session 2 of 3\n${ARGS_DELIM}`);
await expectArgsAllowed(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md Session 2 of 3\n${ARGS_DELIM}\n`);
await expectArgsAllowed(`${PARSER_LINE} <<'${ARGS_DELIM}'\n\n${ARGS_DELIM}\n\n`);
await expectArgsAllowed(`${PARSER_LINE} <<'${ARGS_DELIM}'\nbrief.md ; touch PWNED $(touch PWNED2) *\n${ARGS_DELIM}`);
});
test('[ARG] the guard blocks an unquoted or spaced opener whose body closes early', async () => {
await expectArgsBlocked(`${PARSER_LINE} << '${ARGS_DELIM}'\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n${ARGS_DELIM}`);
await expectArgsBlocked(`${PARSER_LINE} <<"${ARGS_DELIM}"\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n${ARGS_DELIM}`);
await expectArgsBlocked(`${PARSER_LINE} <<-'${ARGS_DELIM}'\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n${ARGS_DELIM}`);
await expectArgsBlocked(`${PARSER_LINE} <<${ARGS_DELIM}\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n${ARGS_DELIM}`);
// Unquoted, the body would expand $( ) even when it closes where it should.
await expectArgsBlocked(`${PARSER_LINE} <<${ARGS_DELIM}\nbrief.md $(touch PWNED)\n${ARGS_DELIM}`);
});
test('[ARG] the guard checks a backslash-quoted <<\\VOYAGE_ARGS_EOF_7F3C9E opener', async () => {
await expectArgsBlocked(`${PARSER_LINE} <<\\${ARGS_DELIM}\nbrief.md\n${ARGS_DELIM}\ntouch PWNED\n${ARGS_DELIM}`);
await expectArgsBlocked(`${PARSER_LINE} << \\${ARGS_DELIM}\nbrief.md\n${ARGS_DELIM}\ntouch PWNED`);
await expectArgsAllowed(`${PARSER_LINE} <<\\${ARGS_DELIM}\nbrief.md Session 2 of 3\n${ARGS_DELIM}`);
});
test("[ARG] the guard's added lines are ASCII", () => {
const src = readFileSync(PRE_BASH, 'utf8');
const from = src.indexOf('// BEGIN args-heredoc-closed-early');
const to = src.indexOf('// END args-heredoc-closed-early');
assert.ok(from >= 0 && to > from, 'the guard marks its args-heredoc-closed-early block with BEGIN/END comments');
const block = src.slice(from, to);
assert.match(block, /VOYAGE_ARGS_EOF_7F3C9E/);
assert.doesNotMatch(block, /[^\x00-\x7f]/, 'a hook script is ASCII (~/.claude/CLAUDE.md, Miljo)');
});

View file

@ -1,11 +1,15 @@
import { test } from 'node:test'; import { test } from 'node:test';
import { strict as assert } from 'node:assert'; import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process'; import { spawnSync } from 'node:child_process';
import { existsSync, mkdtempSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path'; import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url'; import { fileURLToPath, pathToFileURL } from 'node:url';
import { parseArgs } from '../../lib/parsers/arg-parser.mjs'; import { parseArgs } from '../../lib/parsers/arg-parser.mjs';
import { childEnv } from '../helpers/child-env.mjs';
const PARSER = join(dirname(fileURLToPath(import.meta.url)), '..', '..', 'lib', 'parsers', 'arg-parser.mjs'); const PARSER = join(dirname(fileURLToPath(import.meta.url)), '..', '..', 'lib', 'parsers', 'arg-parser.mjs');
const PARSER_URL = pathToFileURL(PARSER).href;
test('trekbrief — empty args', () => { test('trekbrief — empty args', () => {
const r = parseArgs('', 'trekbrief'); const r = parseArgs('', 'trekbrief');
@ -200,3 +204,88 @@ test('trekendsession — -h stays a positional (no alias resolution)', () => {
assert.deepEqual(r.errors, []); assert.deepEqual(r.errors, []);
assert.equal(r.flags['--help'], undefined); assert.equal(r.flags['--help'], undefined);
}); });
// --- ARG / M10 (fix round 4.5b) — the arguments reach the parser as stdin
// text, never as shell words, and the label is read whole from `tail` ---
function runStdin(command, input, cwd) {
return spawnSync(process.execPath, [PARSER, '--command', command, '--stdin'], {
input, cwd, encoding: 'utf8', env: childEnv(),
});
}
test('[ARG] --stdin keeps ; $( ) and * as literal characters and creates no file', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.parseArgs, 'function', 'arg-parser.mjs exports parseArgs');
const cwd = mkdtempSync(join(tmpdir(), 'arg-parser-stdin-'));
try {
const r = runStdin('trekendsession', 'brief.md ; touch PWNED $(touch PWNED2) * ~/x $HOME\n', cwd);
assert.equal(r.status, 0, `the parser CLI refused --stdin: ${r.stdout}${r.stderr}`);
const out = JSON.parse(r.stdout);
assert.deepEqual(out.positional,
['brief.md', ';', 'touch', 'PWNED', '$(touch', 'PWNED2)', '*', '~/x', '$HOME']);
assert.equal(out.tail, '; touch PWNED $(touch PWNED2) * ~/x $HOME');
assert.equal(existsSync(join(cwd, 'PWNED')), false, 'no PWNED file was created');
assert.equal(existsSync(join(cwd, 'PWNED2')), false, 'no PWNED2 file was created');
} finally {
rmSync(cwd, { recursive: true, force: true });
}
});
test('[ARG] a balanced quote groups words', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.tokenize, 'function', 'arg-parser.mjs exports tokenize');
assert.deepEqual(mod.tokenize(`a "b c" 'd e' f`), [
{ value: 'a', start: 0, end: 1 },
{ value: 'b c', start: 2, end: 7 },
{ value: 'd e', start: 8, end: 13 },
{ value: 'f', start: 14, end: 15 },
]);
assert.deepEqual(mod.tokenize(`brief.md "Fix the user's login"`).map((t) => t.value),
['brief.md', "Fix the user's login"]);
});
test('[ARG] an unbalanced quote stays a literal character', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.tokenize, 'function', 'arg-parser.mjs exports tokenize');
assert.deepEqual(mod.tokenize(`Fix the user's login`).map((t) => t.value),
['Fix', 'the', "user's", 'login']);
assert.deepEqual(mod.tokenize('say "hi there').map((t) => t.value),
['say', '"hi', 'there']);
assert.deepEqual(mod.parseArgs(`brief.md it's`, 'trekendsession').positional,
['brief.md', "it's"]);
});
test('[ARG] empty stdin gives no positional, an empty tail and exit 0', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.parseArgs, 'function', 'arg-parser.mjs exports parseArgs');
for (const input of ['', '\n']) {
const r = runStdin('trekendsession', input);
assert.equal(r.status, 0, `the parser CLI refused empty stdin: ${r.stdout}${r.stderr}`);
const out = JSON.parse(r.stdout);
assert.deepEqual(out.positional, []);
assert.deepEqual(out.errors, []);
assert.equal(out.tail, '', `tail for ${JSON.stringify(input)}`);
}
});
test('[M10] tail keeps a multi-word label whole', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.parseArgs, 'function', 'arg-parser.mjs exports parseArgs');
const r = mod.parseArgs('brief.md Session 2 of 3', 'trekendsession');
assert.equal(r.tail, 'Session 2 of 3');
assert.equal(r.positional[0], 'brief.md');
});
test('[M10] one outer quote pair around the label is removed', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.parseArgs, 'function', 'arg-parser.mjs exports parseArgs');
assert.equal(mod.parseArgs(`brief.md 'Session 2 of 3'`, 'trekendsession').tail, 'Session 2 of 3');
assert.equal(mod.parseArgs(`brief.md "Session 2 of 3"`, 'trekendsession').tail, 'Session 2 of 3');
});
test('[M10] an inner apostrophe stays', async () => {
const mod = await import(PARSER_URL);
assert.equal(typeof mod.parseArgs, 'function', 'arg-parser.mjs exports parseArgs');
assert.equal(mod.parseArgs(`brief.md Fix the user's login`, 'trekendsession').tail, "Fix the user's login");
});