From b22f7a66e112dd643e628c0e8ae53975060cc990 Mon Sep 17 00:00:00 2001
From: Jayesh Betala <jayesh.betala7@gmail.com>
Date: Thu, 18 Jun 2026 14:48:12 +0530
Subject: [PATCH] fix(careful): warn on chained rm even when the last target is
 safe

The safe-exception block whitelisted rm -rf of build artifacts by
extracting targets with a single greedy match (.*rm ...), which only ever
inspects the LAST rm in the command. A chain like 'rm -rf /; rm -rf
node_modules' was therefore judged solely by its trailing safe target and
allowed without warning, waving through the destructive 'rm -rf /'.

Gate the shortcut to single rm invocations: when any shell separator
(; | & newline, incl. JSON-escaped \n/\r from the grep extraction path)
is present, fall through to the destructive-pattern check, which warns on
any recursive rm. Single-command artifact cleanups still allow.

Adds 3 regression tests covering semicolon and && chains in both orders.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 careful/bin/check-careful.sh | 14 +++++++++++++-
 test/hook-scripts.test.ts    | 25 +++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh
index d9c39e48c..3103289fd 100755
--- a/careful/bin/check-careful.sh
+++ b/careful/bin/check-careful.sh
@@ -26,7 +26,19 @@ fi
 CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]')
 
 # --- Check for safe exceptions (rm -rf of build artifacts) ---
-if printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*r[a-zA-Z]*\s+|--recursive\s+)' 2>/dev/null; then
+# Only whitelist when the command is a SINGLE rm invocation. The target
+# extraction below greedily matches the LAST `rm ` in the string, so a chained
+# command like `rm -rf /; rm -rf node_modules` would be judged solely by its
+# final (safe) targets and wave through the destructive earlier `rm -rf /`.
+# When any shell separator is present, skip the shortcut and fall through to the
+# destructive-pattern check, which warns on any recursive rm.
+HAS_SEPARATOR=false
+case "$CMD" in
+  # Real separators, plus JSON-escaped newlines (\n / \r) which survive the
+  # grep extraction path as literal two-char sequences and still mark a chain.
+  *';'*|*'|'*|*'&'*|*$'\n'*|*$'\r'*|*'\n'*|*'\r'*) HAS_SEPARATOR=true ;;
+esac
+if [ "$HAS_SEPARATOR" = false ] && printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*r[a-zA-Z]*\s+|--recursive\s+)' 2>/dev/null; then
   SAFE_ONLY=true
   RM_ARGS=$(printf '%s' "$CMD" | sed -E 's/.*rm[[:space:]]+(-[a-zA-Z]+[[:space:]]+)*//;s/--recursive[[:space:]]*//')
   for target in $RM_ARGS; do
diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts
index f1ffe1239..5fc70583c 100644
--- a/test/hook-scripts.test.ts
+++ b/test/hook-scripts.test.ts
@@ -96,6 +96,31 @@ describe('check-careful.sh', () => {
       expect(output.permissionDecision).toBe('ask');
       expect(output.message).toContain('recursive delete');
     });
+
+    // Regression: the safe-exception extracts targets from only the LAST `rm` in
+    // the command (greedy match), so a chain that ends in a safe target must not
+    // wave through a destructive earlier rm. The shortcut only applies to a
+    // single rm invocation; any shell separator falls through to the warning.
+    test('rm -rf /; rm -rf node_modules warns (semicolon chain, dangerous first)', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /; rm -rf node_modules'));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('recursive delete');
+    });
+
+    test('rm -rf /etc/data && rm -rf dist warns (&& chain, dangerous first)', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/data && rm -rf dist'));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('recursive delete');
+    });
+
+    test('rm -rf node_modules; rm -rf /home/user/data warns (safe first, dangerous last)', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf node_modules; rm -rf /home/user/data'));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('recursive delete');
+    });
   });
 
   // --- SQL destructive commands ---