diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index 3080e10c0..45fc98a44 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -224,6 +224,21 @@ 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. +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) \ + && CMD_MEDIUM="$_cmd_masked" || true + CMD_MEDIUM_LOWER=$(printf '%s' "$CMD_MEDIUM" | tr '[:upper:]' '[:lower:]') +fi + # --- PowerShell and cmd table (#3067) --- # Runs on the PowerShell tool's command, and on the part of any command from a # nested `powershell`/`pwsh`/`cmd` launcher onward (`pwsh -c "..."`, @@ -303,50 +318,50 @@ if [ -n "$_WIN_SCAN" ]; then fi # rm -rf / rm -r / rm -R / rm --recursive (capital -R is BSD/macOS recursive) -if [ -z "$WARN" ] && grep -qE 'rm\s+(-[a-zA-Z]*[rR]|--recursive)' <<< "$CMD" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'rm\s+(-[a-zA-Z]*[rR]|--recursive)' <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Destructive: recursive delete (rm -r). This permanently removes files." PATTERN="rm_recursive" fi # DROP TABLE / DROP DATABASE -if [ -z "$WARN" ] && grep -qE 'drop\s+(table|database)' <<< "$CMD_LOWER" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'drop\s+(table|database)' <<< "$CMD_MEDIUM_LOWER" 2>/dev/null; then WARN="Destructive: SQL DROP detected. This permanently deletes database objects." PATTERN="drop_table" fi # TRUNCATE -if [ -z "$WARN" ] && grep -qE '\btruncate\b' <<< "$CMD_LOWER" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE '\btruncate\b' <<< "$CMD_MEDIUM_LOWER" 2>/dev/null; then WARN="Destructive: SQL TRUNCATE detected. This deletes all rows from a table." PATTERN="truncate" fi # git push --force / git push -f / plus-refspec force (git push origin +ref) -if [ -z "$WARN" ] && grep -qE 'git\s+push\s' <<< "$CMD" 2>/dev/null \ - && grep -qE '(-f\b|--force|(^|[[:space:]])\+[^[:space:]])' <<< "$CMD" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'git\s+push\s' <<< "$CMD_MEDIUM" 2>/dev/null \ + && grep -qE '(-f\b|--force|(^|[[:space:]])\+[^[:space:]])' <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Destructive: git force-push rewrites remote history. Other contributors may lose work." PATTERN="git_force_push" fi # git reset --hard -if [ -z "$WARN" ] && grep -qE 'git\s+reset\s+--hard' <<< "$CMD" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'git\s+reset\s+--hard' <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Destructive: git reset --hard discards all uncommitted changes." PATTERN="git_reset_hard" fi # git checkout . / git restore . -if [ -z "$WARN" ] && grep -qE 'git\s+(checkout|restore)\s+\.' <<< "$CMD" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'git\s+(checkout|restore)\s+\.' <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Destructive: discards all uncommitted changes in the working tree." PATTERN="git_discard" fi # kubectl delete -if [ -z "$WARN" ] && grep -qE 'kubectl\s+delete' <<< "$CMD" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'kubectl\s+delete' <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Destructive: kubectl delete removes Kubernetes resources. May impact production." PATTERN="kubectl_delete" fi # docker rm -f / docker system prune -if [ -z "$WARN" ] && grep -qE 'docker\s+(rm\s+-f|system\s+prune)' <<< "$CMD" 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'docker\s+(rm\s+-f|system\s+prune)' <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Destructive: Docker force-remove or prune. May delete running containers or cached images." PATTERN="docker_destructive" fi @@ -386,7 +401,7 @@ $_GSTACK_HOME_DIR/projects/$SLUG/careful-patterns.txt" _PAT_RC=0 printf '' | grep -qE -- "$_PAT" 2>/dev/null || _PAT_RC=$? [ "$_PAT_RC" -eq 2 ] && continue # invalid ERE — skip the line - if grep -qE -- "$_PAT" <<< "$CMD" 2>/dev/null; then + if grep -qE -- "$_PAT" <<< "$CMD_MEDIUM" 2>/dev/null; then WARN="Project rule matched: $_PAT" PATTERN="project_rule" break diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index 44ac80437..894c30d37 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -342,6 +342,43 @@ describe('check-careful.sh', () => { expect(output.hookSpecificOutput?.permissionDecisionReason).toContain(reason); }); + // --- Commit message false positives (#1060) --- + + describe('commit message false positives (#1060)', () => { + // Quoted -m content describes destructive patterns but does not run them. + test.each([ + 'git commit -m "fix: prevent accidental drop table in migration"', + 'git commit -m "docs: explain truncate vs delete semantics"', + 'git commit -m "feat: add guard for recursive delete patterns"', + 'git add . && git commit -m "fix: handle drop database edge case"', + ])('does not warn on descriptive text in commit message: %s', (command) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command)); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined(); + }); + + // Real destructive commands chained with a commit still warn. + test('warns when a real DROP TABLE follows git commit in a chain', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -m "release" && psql -c "DROP TABLE staging"'), + ); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('SQL DROP'); + }); + + test('warns when a real TRUNCATE follows git commit in a chain', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -m "cleanup" && psql -c "TRUNCATE audit_log"'), + ); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('SQL TRUNCATE'); + }); + }); + // --- Shell obfuscation --- describe('shell obfuscation', () => {