From 7f0ebd651b26e02c1d84e2c8823f21be406d29f6 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 24 Oct 2026 20:41:51 +0000 Subject: [PATCH] fix(careful): mask a quoted-delimiter heredoc message; any other substitution or heredoc leaves the command unmasked (#1060) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- careful/bin/mask-commit-message.awk | 40 +++++++++++++++++++++++++++-- test/hook-scripts.test.ts | 30 ++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/careful/bin/mask-commit-message.awk b/careful/bin/mask-commit-message.awk index 584d67463..e943f9bc3 100644 --- a/careful/bin/mask-commit-message.awk +++ b/careful/bin/mask-commit-message.awk @@ -11,6 +11,39 @@ # the message on stdout. Quotes and backslashes follow bash: a backslash # escapes the next character outside single quotes, and nothing inside single # quotes. An unterminated quote or escape prints the command unchanged. +# +# A message written as "$(cat <<'EOF' ... EOF )" (a quoted delimiter, so the +# body is never expanded) is masked whole. Any other command substitution, +# backtick, $'...' string or heredoc changes how bash reads the quotes after +# it, so the command is printed unchanged. +# heredoc_msg S N I — S[I] opens a double-quoted "$(cat <<'DELIM' ... DELIM )" +# message: the index after its closing quote, else 0. +function heredoc_msg(s, n, i, p, rest, strip, delim, e, line) { + rest = substr(s, i + 1) + if (!match(rest, /^\$\(cat[ \t]+<<-?[ \t]*/)) return 0 + strip = substr(rest, 1, RLENGTH) ~ /<<-/ + p = i + 1 + RLENGTH + rest = substr(s, p) + if (match(rest, /^'[A-Za-z_][A-Za-z0-9_]*'/) || match(rest, /^"[A-Za-z_][A-Za-z0-9_]*"/)) delim = substr(rest, 2, RLENGTH - 2) + else if (match(rest, /^\\[A-Za-z_][A-Za-z0-9_]*/)) delim = substr(rest, 2, RLENGTH - 1) + else return 0 + p += RLENGTH + while (p <= n && substr(s, p, 1) ~ /[ \t]/) p++ + if (substr(s, p, 1) != "\n") return 0 + p++ + for (;;) { + if (p > n) return 0 + e = index(substr(s, p), "\n") + if (!e) return 0 + line = substr(s, p, e - 1) + if (strip) sub(/^\t+/, "", line) + p += e + if (line == delim) break + } + while (p <= n && substr(s, p, 1) ~ /[ \t\n]/) p++ + if (substr(s, p, 2) != ")\"") return 0 + return p + 2 +} function handle_msg(s, n, i, j, c, safe) { if (i > n) return i if (substr(s, i, 1) == "'") { @@ -21,6 +54,7 @@ function handle_msg(s, n, i, j, c, safe) { return j + 1 } if (substr(s, i, 1) == "\"") { + if ((j = heredoc_msg(s, n, i)) > 0) { result = result "\"\""; return j } j = i + 1; safe = 1 while (j <= n && (c = substr(s, j, 1)) != "\"") { if (c == "$" || c == "`" || c == "\\") safe = 0 @@ -43,16 +77,18 @@ function flag(s, i, rest, m) { END { s = cmd n = length(s); sq = 0; dq = 0; bs = 0; i = 1 - atword = 1; words = 0; git = 0; commit = 0; piped = 0 + atword = 1; words = 0; git = 0; commit = 0; piped = 0; unsure = 0 while (i <= n) { c = substr(s, i, 1) if (bs) { result = result c; bs = 0; i++; continue } if (sq) { if (c == "'") sq = 0; result = result c; i++; continue } + if (c == "`" || substr(s, i, 2) == "$(") unsure = 1 if (dq) { if (c == "\\") bs = 1 else if (c == "\"") dq = 0 result = result c; i++; continue } + if (substr(s, i, 2) == "<<" || substr(s, i, 2) == "$'") unsure = 1 if (c ~ /[;&|()\n]/) { if (c == "|" && commit && substr(s, i + 1, 1) != "|") piped = 1 atword = 1; words = 0; git = 0; commit = 0 @@ -76,5 +112,5 @@ END { else if (c == "\"") dq = 1 result = result c; i++ } - printf "%s", (sq || dq || bs || piped) ? s : result + printf "%s", (sq || dq || bs || piped || unsure) ? s : result } diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index e79ebc44c..7c8db04d9 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -480,6 +480,36 @@ describe('check-careful.sh', () => { expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined(); }); + // The form agents use most: a quoted-delimiter heredoc inside "$( )". Its + // body is never expanded, so it is masked whole. Any other substitution, + // backtick, $'...' or heredoc leaves the command unmasked. + const heredocMessage = (body: string, delimiter = "'EOF'", end = 'EOF') => + `git commit -m "$(cat <<${delimiter}\n${body}\n${end}\n)"`; + test.each([ + heredocMessage('fix: guard the migration that could drop table users\n\nCo-Authored-By: A '), + heredocMessage('docs: why we never truncate "audit" rows (see #12)'), + heredocMessage('chore: git reset --hard is documented', '"MSG"', 'MSG'), + heredocMessage('chore: rm -rf build in the clean script', "-'EOF'", '\tEOF'), + ])('a quoted-delimiter heredoc message is masked: %j', (command) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command)); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined(); + }); + + test.each([ + [heredocMessage('docs: drop table notes') + ' && git push --force', 'force-push'], + [heredocMessage('drop table $(rm -rf build)', 'EOF'), 'recursive delete'], + [`git commit -m "$(echo " -m 'x" ; rm -rf build ; echo ')"`, 'recursive delete'], + ['git commit -m "\`rm -rf build\`"', 'recursive delete'], + ["git commit -F - <<'EOF'\ndocs: drop table notes\nEOF", 'SQL DROP'], + ["git commit -m $'drop table notes\\n'", 'SQL DROP'], + ])('other substitutions and heredocs stay unmasked: %j', (command, reason) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command)); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain(reason); + }); + test('a partial install without the masking program still asks on the message text', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-careful-nomask-')); try {