fix(careful): mask only git commit's own message; a piped commit or another command's -m stays visible (#1060)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

careful/bin/check-careful.sh

@@ -233,7 +233,8 @@ PATTERN=""
 CMD_MEDIUM="$CMD"
 CMD_MEDIUM_LOWER="$CMD_LOWER"
 _MASK_AWK="$_HOOK_DIR/mask-commit-message.awk"
-if [ -f "$_MASK_AWK" ] && printf '%s' "$CMD" | grep -qE '(^|[;&|][[:space:]]*)git[[:space:]]+commit[[:space:]]' 2>/dev/null; then
+case "$CMD" in *git*commit*) _MAYBE_COMMIT=1 ;; *) _MAYBE_COMMIT=0 ;; esac
+if [ "$_MAYBE_COMMIT" -eq 1 ] && [ -f "$_MASK_AWK" ]; then
   if _cmd_masked=$(printf '%s' "$CMD" | awk -f "$_MASK_AWK" 2>/dev/null); then
     CMD_MEDIUM="$_cmd_masked"
     CMD_MEDIUM_LOWER=$(printf '%s' "$CMD_MEDIUM" | tr '[:upper:]' '[:lower:]')

careful/bin/mask-commit-message.awk

@@ -3,61 +3,78 @@
 # (-m, -am, --message) replaced by an empty quoted string. The command is
 # scanned as one string: newlines are kept, since bash runs each line as its
 # own command, and quote state carries across lines.
-function handle_msg(s, n, i,    j, safe) {
+#
+# 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.
+function handle_msg(s, n, i,    j, c, safe) {
   if (i > n) return i
   if (substr(s, i, 1) == "'") {
-    result = result "'"
-    i++
-    while (i <= n && substr(s, i, 1) != "'") i++
-    result = result "'"
-    if (i <= n) i++
-  } else if (substr(s, i, 1) == "\"") {
-    j = i+1; safe = 1
-    while (j <= n && substr(s, j, 1) != "\"") {
-      if (substr(s,j,1) == "$" || substr(s,j,1) == "`" || substr(s,j,1) == "\\") safe = 0
+    j = i + 1
+    while (j <= n && substr(s, j, 1) != "'") j++
+    if (j > n) return i
+    result = result "''"
+    return j + 1
+  }
+  if (substr(s, i, 1) == "\"") {
+    j = i + 1; safe = 1
+    while (j <= n && (c = substr(s, j, 1)) != "\"") {
+      if (c == "$" || c == "`" || c == "\\") safe = 0
       j++
     }
-    if (safe) { result = result "\"\""; i = j+1 }
-    else {
-      result = result substr(s, i, j-i)
-      if (j <= n) result = result "\""
-      i = j+1
-    }
+    if (!safe || j > n) return i
+    result = result "\"\""
+    return j + 1
   }
   return i
 }
+# flag S I — length of the message flag (with its trailing blanks) at S[I], or 0
+function flag(s, i,    rest, m) {
+  rest = substr(s, i)
+  if (rest ~ /^--message=/) return 10
+  if (match(rest, /^(--message|-am|-m)[ \t]+/)) return RLENGTH
+  return 0
+}
 { cmd = (NR > 1) ? cmd "\n" $0 : $0 }
 END {
   s = cmd
   n = length(s); sq = 0; dq = 0; bs = 0; i = 1
+  atword = 1; words = 0; git = 0; commit = 0; piped = 0
   while (i <= n) {
     c = substr(s, i, 1)
     if (bs) { result = result c; bs = 0; i++; continue }
-    if (!sq && dq && c == "\\") { result = result c; bs = 1; i++; continue }
-    if (!dq && c == "'") { sq = !sq; result = result c; i++; continue }
-    if (!sq && c == "\"") { dq = !dq; result = result c; i++; continue }
-    if (sq || dq) { result = result c; i++; continue }
-    rest = substr(s, i)
-    if (rest ~ /^--message=/) {
-      result = result "--message="; i += 10
-      i = handle_msg(s, n, i); continue
+    if (sq) { if (c == "'") sq = 0; result = result c; i++; continue }
+    if (dq) {
+      if (c == "\\") bs = 1
+      else if (c == "\"") dq = 0
+      result = result c; i++; continue
     }
-    if (rest ~ /^--message([ \t]|$)/) {
-      result = result "--message"; i += 9
-      while (i <= n && substr(s,i,1) ~ /[ \t]/) { result = result substr(s,i,1); i++ }
-      i = handle_msg(s, n, i); continue
+    if (c ~ /[;&|()\n]/) {
+      if (c == "|" && commit && substr(s, i + 1, 1) != "|") piped = 1
+      atword = 1; words = 0; git = 0; commit = 0
+      result = result c; i++; continue
     }
-    if (rest ~ /^-am([ \t]|$)/) {
-      result = result "-am"; i += 3
-      while (i <= n && substr(s,i,1) ~ /[ \t]/) { result = result substr(s,i,1); i++ }
-      i = handle_msg(s, n, i); continue
-    }
-    if (rest ~ /^-m([ \t]|$)/) {
-      result = result "-m"; i += 2
-      while (i <= n && substr(s,i,1) ~ /[ \t]/) { result = result substr(s,i,1); i++ }
-      i = handle_msg(s, n, i); continue
+    if (c == " " || c == "\t") { atword = 1; result = result c; i++; continue }
+    if (atword) {
+      atword = 0; words++
+      rest = substr(s, i)
+      if (words == 1 && rest ~ /^git([ \t\n;&|()]|$)/) git = 1
+      else if (git && !commit && rest ~ /^commit([ \t\n;&|()]|$)/) commit = 1
+      else if (commit && (len = flag(s, i)) > 0) {
+        result = result substr(s, i, len); i += len
+        j = handle_msg(s, n, i)
+        atword = (j == i); i = j
+        continue
+      }
     }
+    if (c == "\\") bs = 1
+    else if (c == "'") sq = 1
+    else if (c == "\"") dq = 1
     result = result c; i++
   }
-  printf "%s", result
+  printf "%s", (sq || dq || bs || piped) ? s : result
 }

test/hook-scripts.test.ts

@@ -454,6 +454,32 @@ describe('check-careful.sh', () => {
       expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
     });
 
+    // Only git commit's own message is masked: a -m argument of another
+    // command in the chain, a commit whose output is piped into a shell, and
+    // an escaped quote inside the message all stay visible.
+    test.each([
+      ["git commit -m a && ls | parallel -m 'rm -rf build'", 'recursive delete'],
+      ["git commit -m 'rm -rf build' | sh", 'recursive delete'],
+      ["echo git commit -m 'drop table users'", 'SQL DROP'],
+      ['git commit -m "a\\"b" && rm -rf build', 'recursive delete'],
+      ["git commit -m wip \\\n&& rm -rf build", 'recursive delete'],
+    ])('text outside commit\'s own message still asks: %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.each([
+      'git -C repo commit --message="docs: truncate the changelog intro"',
+      "git commit --message 'chore: drop database fixtures from the seed'",
+      "git commit --amend -m 'fix: git reset --hard in the docs'",
+    ])('commit\'s own message is masked in every flag form: %s', (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 {