From 8e6328c9ce29b4456898c8068cdc102d49d325e1 Mon Sep 17 00:00:00 2001 From: Bhumika-1432006 Date: Thu, 22 Oct 2026 00:05:19 +0530 Subject: [PATCH] fix(careful): replace sed masking with quote-aware awk scanner (#1060) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sed-based -m content masker had two bypass paths that capy-ai identified: 1. Single-quoted message containing a double-quote boundary: git commit -m 'note -m "' && rm -rf build && echo '"' The double-quote pass matched '-m "' inside the single-quoted string, swallowing '&& rm -rf build && echo '; the rm vanished from CMD_MEDIUM. 2. Double-quoted message with a command substitution: git commit -m "$(rm -rf build)" sed replaced the content including the $(...), so the substitution — which executes in the shell — was masked away. Replace the sed substitution with a quote-aware awk scanner that: - Walks the command string tracking single-quote, double-quote, and backslash state, so it only treats a delimiter as a message boundary when the shell would. - Masks single-quoted messages (always literal) and double-quoted messages that contain no $, backtick, or backslash (safe to mask — no expansion). - Leaves double-quoted messages with $, backtick, or backslash unmasked so command substitutions stay visible to the MEDIUM checks. - Handles -m, -am, --message, and --message= forms. Four new tests added as requested (alongside the original six): - single-quoted message with embedded double-quote + chained rm → warns - commit message with $(rm -rf build) → warns - git push --force chained after commit → warns - git commit -am "drop table notes" → silent (message masked) --- careful/bin/check-careful.sh | 75 ++++++++++++++++++++++++++++++++---- test/hook-scripts.test.ts | 46 ++++++++++++++++++++++ 2 files changed, 113 insertions(+), 8 deletions(-) diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index 45fc98a44..680c5974c 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -224,17 +224,76 @@ esac WARN="" PATTERN="" -# Mask quoted -m argument content in git commit commands so that descriptive -# commit messages (e.g. "fix: prevent drop table in migration") do not trigger -# false positives. Real destructive commands outside the quoted string still -# match because the rest of the command is preserved. Fall back to $CMD when -# sed is unavailable. +# Mask quoted -m/-am/--message argument content in git commit commands so that +# descriptive commit messages do not trigger false positives. Uses a +# quote-aware awk scanner that tracks shell quoting state so it cannot swallow +# real commands that follow a commit via &&/; (e.g. `git commit -m 'note -m "' +# && rm -rf build` keeps the rm visible). Double-quoted messages containing $, +# backtick or backslash are left unmasked so command substitutions remain +# visible to the MEDIUM checks. Falls back to $CMD when awk is unavailable. CMD_MEDIUM="$CMD" CMD_MEDIUM_LOWER="$CMD_LOWER" if printf '%s' "$CMD" | grep -qE '(^|[;&|][[:space:]]*)git[[:space:]]+commit[[:space:]]' 2>/dev/null; then - _cmd_masked=$(printf '%s' "$CMD" \ - | sed 's/-m[[:space:]]*"[^"]*"/-m ""/g - s/-m[[:space:]]*'"'"'[^'"'"']*'"'"'/-m '"'"''"'"'/g' 2>/dev/null) \ + _AWK_MASK_PROG=$(cat <<'_AWKEOF' +function handle_msg(s, n, i, j, 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++ + } + if (safe) { result = result "\"\""; i = j+1 } + else { + result = result substr(s, i, j-i) + if (j <= n) result = result "\"" + i = j+1 + } + } + return i +} +{ + n = length($0); sq = 0; dq = 0; bs = 0; i = 1 + while (i <= n) { + c = substr($0, 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($0, i) + if (rest ~ /^--message=/) { + result = result "--message="; i += 10 + i = handle_msg($0, n, i); continue + } + if (rest ~ /^--message([ \t]|$)/) { + result = result "--message"; i += 9 + while (i <= n && substr($0,i,1) ~ /[ \t]/) { result = result substr($0,i,1); i++ } + i = handle_msg($0, n, i); continue + } + if (rest ~ /^-am([ \t]|$)/) { + result = result "-am"; i += 3 + while (i <= n && substr($0,i,1) ~ /[ \t]/) { result = result substr($0,i,1); i++ } + i = handle_msg($0, n, i); continue + } + if (rest ~ /^-m([ \t]|$)/) { + result = result "-m"; i += 2 + while (i <= n && substr($0,i,1) ~ /[ \t]/) { result = result substr($0,i,1); i++ } + i = handle_msg($0, n, i); continue + } + result = result c; i++ + } + printf "%s", result +} +_AWKEOF +) + _cmd_masked=$(printf '%s' "$CMD" | awk "$_AWK_MASK_PROG" 2>/dev/null) \ && CMD_MEDIUM="$_cmd_masked" || true CMD_MEDIUM_LOWER=$(printf '%s' "$CMD_MEDIUM" | tr '[:upper:]' '[:lower:]') fi diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index 894c30d37..0a1834f52 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -377,6 +377,52 @@ describe('check-careful.sh', () => { expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('SQL TRUNCATE'); }); + + // Quote-aware awk scanner: a single-quoted message whose content looks like + // a flag boundary must not swallow real commands that follow via &&. + test('warns when rm follows a single-quoted message that embeds a double-quote', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput(`git commit -m 'note -m "' && rm -rf build && echo '"'`), + ); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete'); + }); + + // A double-quoted message containing $(...) is not masked — the substitution + // content remains visible to the MEDIUM checks. + test('warns when a commit message contains a command substitution with rm', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -m "$(rm -rf build)"'), + ); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete'); + }); + + // A force-push chained after a commit must still warn even after the message is masked. + test('warns on force-push chained after git commit', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -m "x" && git push --force'), + ); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('force-push'); + }); + + // -am shorthand: the message is masked, so "drop table" in the message text + // does not trigger the SQL DROP warning. + test('does not warn on drop-table text in -am commit message', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -am "drop table notes"'), + ); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined(); + }); }); // --- Shell obfuscation ---