From bdb1ef050e3cb6db4ef4bf7f88cd0380e0dd7f49 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 24 Oct 2026 20:44:03 +0000 Subject: [PATCH] fix(careful): a pipe, process substitution or parameter expansion after the message unmasks it; redirections and comments parse like bash (#1060) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- careful/bin/mask-commit-message.awk | 33 ++++++++++++++++++----------- test/hook-scripts.test.ts | 25 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/careful/bin/mask-commit-message.awk b/careful/bin/mask-commit-message.awk index e943f9bc3..5a1358428 100644 --- a/careful/bin/mask-commit-message.awk +++ b/careful/bin/mask-commit-message.awk @@ -6,16 +6,18 @@ # # Only git commit's own arguments are masked: the scanner splits the command # at unquoted ; & | ( ) and newlines, and masks a message flag only in a -# segment whose first word is git and that has a commit word after it. A -# commit piped into another command is printed unchanged, because git prints -# 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. +# segment whose first word is git and that has a commit word after it. The & +# of a redirection (2>&1, &>) does not split. Any pipe after a masked message +# prints the command unchanged, because git prints the message on stdout. +# Quotes, backslashes and comments follow bash: a backslash escapes the next +# character outside single quotes, and nothing inside single quotes; an +# unquoted # starting a word runs to the end of its line. 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. +# body is never expanded) is masked whole. Any other command, parameter or +# process 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) { @@ -77,20 +79,26 @@ 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; unsure = 0 + atword = 1; words = 0; git = 0; commit = 0; masked = 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 + two = substr(s, i, 2) + if (c == "`" || two == "$(" || two == "${") 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 (two == "<<" || two == "$'" || two == "<(" || two == ">(") unsure = 1 + if (atword && c == "#") { + while (i <= n && substr(s, i, 1) != "\n") { result = result substr(s, i, 1); i++ } + continue + } + if (c == "&" && (substr(s, i - 1, 1) ~ /[<>]/ || substr(s, i + 1, 1) == ">")) { atword = 0; result = result c; i++; continue } if (c ~ /[;&|()\n]/) { - if (c == "|" && commit && substr(s, i + 1, 1) != "|") piped = 1 + if (c == "|" && masked && substr(s, i + 1, 1) != "|" && substr(s, i - 1, 1) != "|") piped = 1 atword = 1; words = 0; git = 0; commit = 0 result = result c; i++; continue } @@ -103,6 +111,7 @@ END { else if (commit && (len = flag(s, i)) > 0) { result = result substr(s, i, len); i += len j = handle_msg(s, n, i) + if (j > i) masked = 1 atword = (j == i); i = j continue } diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index 7c8db04d9..621bf5143 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -510,6 +510,31 @@ describe('check-careful.sh', () => { expect(output.hookSpecificOutput?.permissionDecisionReason).toContain(reason); }); + // git prints the message on stdout, so any pipe after a masked message, + // a process substitution or a parameter expansion leaves it unmasked; a + // redirection's & is not a separator, and a comment's quote is not a quote. + test.each([ + "git commit -m 'rm -rf build' 2>&1 | sh", + "(git commit -m 'rm -rf build') | sh", + "git commit -m 'rm -rf build' > >(sh)", + 'git commit -m "${MSG:-rm -rf build}"', + ])('a message that can reach a shell stays unmasked: %j', (command) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command)); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete'); + }); + + test.each([ + "git commit -m 'drop table notes' 2>&1 && git log -1 --oneline", + "git commit -m 'drop table notes' || echo failed", + "git commit -m 'drop table notes' # it's the docs commit", + ])('redirections, || and comments keep the message masked: %j', (command) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command)); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined(); + }); + 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 {