From 5dacc814b880c5a3814a057bbbf52c8f167d7272 Mon Sep 17 00:00:00 2001 From: jacob-wang Date: Tue, 21 Apr 2026 05:04:19 +0800 Subject: [PATCH 1/2] fix(careful): skip false positives in text-output command segments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /careful hook was triggering on patterns inside git commit -m messages, echo arguments, and other text-output commands — even though those strings are not executed as destructive operations. Split command on shell operators (&&, ||, ;) and skip segments that are purely text-output commands (git commit -m, echo, printf, cat). Only check the remaining executable segments for destructive patterns. Fixes #1060. --- careful/bin/check-careful.sh | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index c8bc2c7ab..f5e00f527 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -22,6 +22,38 @@ if [ -z "$CMD" ]; then exit 0 fi +# Split command on shell operators and filter out text-only segments. +# Segments that start with git commit, echo, cat, or printf produce text output, +# not destructive actions — skip their content to avoid false positives. +FILTERED_CMD="" +IFS_SAVE="$IFS" +IFS="$(printf '\n\t')" +for segment in $(printf '%s' "$CMD" | sed 's/&&/\n/g; s/||/\n/g; s/;/ /g'); do + trimmed=$(printf '%s' "$segment" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') + # Skip segments that are purely text-output commands (their args may mention + # dangerous patterns without actually executing them) + case "$trimmed" in + git\ commit\ -m\ *|git\ commit\ --message\ *|echo\ *|printf\ *|cat\ <*) + continue + ;; + *) + if [ -n "$FILTERED_CMD" ]; then + FILTERED_CMD="$FILTERED_CMD && " + fi + FILTERED_CMD="${FILTERED_CMD}${trimmed}" + ;; + esac +done +IFS="$IFS_SAVE" + +# Use filtered command for pattern checks; if all segments were filtered out, allow +if [ -z "$FILTERED_CMD" ]; then + echo '{}' + exit 0 +fi + +CMD="$FILTERED_CMD" + # Normalize: lowercase for case-insensitive SQL matching CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]') From b3fb044482456f536e97a112e27b8273026d8fa8 Mon Sep 17 00:00:00 2001 From: jacob-wang Date: Tue, 21 Apr 2026 06:08:33 +0800 Subject: [PATCH 2/2] test(hook-scripts): add false positive prevention tests for text-output commands --- test/hook-scripts.test.ts | 60 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index 850b5b983..7d6caf2bc 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -229,6 +229,66 @@ describe('check-careful.sh', () => { } }); + // --- False positive prevention: patterns in text-output command arguments --- + + describe('false positive prevention: text-output command segments', () => { + test('git commit -m with "rm -rf" in message allows', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -m "fix: remove old cache files with rm -rf ~/.cache"'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + + test('git commit --message with DROP TABLE in message allows', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit --message "chore: drop old analytics tables DROP TABLE events"'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + + test('echo with dangerous pattern in argument allows', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('echo "About to run: rm -rf /tmp/stale-cache"'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + + test('printf with dangerous pattern in format string allows', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('printf "Deleting with: rm -rf %s\\n" "/tmp/old-build"'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + + test('chained command where git commit -m is filtered but rm follows allows the rm warning', () => { + // git commit -m gets filtered, but the `rm -rf /tmp/data` after && should still warn + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('git commit -m "cleanup rm -rf /tmp/data" && rm -rf /tmp/data'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('cat with dangerous pattern in filename allows', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('cat /tmp/notes-about-rm-rf-commands.txt'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + }); + // --- Edge cases --- describe('edge cases', () => {