fix(careful): three bypasses in check-careful.sh (quoted args, rm -R, ${IFS})

careful/bin/check-careful.sh

@@ -7,16 +7,47 @@ set -euo pipefail
 # Read stdin (JSON with tool_input)
 INPUT=$(cat)
 
-# Extract the "command" field value from tool_input
-# Try grep/sed first (handles 99% of cases), fall back to Python for escaped quotes
-CMD=$(printf '%s' "$INPUT" | grep -o '"command"[[:space:]]*:[[:space:]]*"[^"]*"' | head -1 | sed 's/.*:[[:space:]]*"//;s/"$//' || true)
-
-# Python fallback if grep returned empty (e.g., escaped quotes in command)
-if [ -z "$CMD" ]; then
-  CMD=$(printf '%s' "$INPUT" | python3 -c 'import sys,json; print(json.loads(sys.stdin.read()).get("tool_input",{}).get("command",""))' 2>/dev/null || true)
+# Extract the "command" field value from tool_input with a real JSON parser.
+#
+# The previous extractor was
+#   grep -o '"command"[[:space:]]*:[[:space:]]*"[^"]*"'
+# whose [^"]* stops at the first escaped quote in the JSON string value. Any
+# destructive command preceded by a quoted argument was therefore truncated
+# away before the pattern checks ever ran:
+#
+#   git commit -m "wip" && rm -rf /   ->  CMD='git commit -m \'   -> allowed
+#   bash -c "rm -rf /"                ->  CMD='bash -c \'         -> allowed
+#   echo "x"; rm -rf ~                ->  CMD='echo \'            -> allowed
+#
+# The python3 fallback never rescued these because CMD was non-empty, so the
+# `[ -z "$CMD" ]` guard did not fire. Parse the payload properly instead, and
+# fail CLOSED when it cannot be parsed at all — a hook that gates destructive
+# commands must not allow-by-default on unreadable input.
+#
+# python3 is tried first because it ships with macOS and most Linux distros and
+# is reliably on PATH in a hook environment; node is the fallback.
+extract_cmd() {
+  if command -v python3 >/dev/null 2>&1; then
+    printf '%s' "$INPUT" | python3 -c 'import sys,json; d=json.loads(sys.stdin.read()); c=d.get("tool_input",{}).get("command",""); sys.stdout.write(c if isinstance(c,str) else "")' 2>/dev/null && return 0
+  fi
+  if command -v node >/dev/null 2>&1; then
+    printf '%s' "$INPUT" | node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{try{const j=JSON.parse(s);const c=(j&&j.tool_input&&j.tool_input.command)||"";process.stdout.write(typeof c==="string"?c:"")}catch(e){process.exit(3)}})' 2>/dev/null && return 0
+  fi
+  return 1
+}
+
+set +e
+CMD=$(extract_cmd)
+EXTRACT_RC=$?
+set -e
+
+# No parser available, or the payload is not parseable JSON. Fail closed.
+if [ "$EXTRACT_RC" -ne 0 ] && [ -n "$INPUT" ]; then
+  printf '{"permissionDecision":"ask","message":"[careful] Could not parse the tool payload to safety-check this command. Approve only if you know what it does."}\n'
+  exit 0
 fi
 
-# If we still couldn't extract a command, allow
+# Parsed fine, but there is genuinely no command field (non-Bash payload) — allow.
 if [ -z "$CMD" ]; then
   echo '{}'
   exit 0
@@ -25,12 +56,29 @@ fi
 # Normalize: lowercase for case-insensitive SQL matching
 CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]')
 
+# --- Shell-obfuscation tripwire ---
+# Every check below inspects the command as a STRING, but bash executes what the
+# string MEANS after expansion. ${IFS} holds the default field separator and
+# contains no literal whitespace, so
+#
+#   rm${IFS}-rf${IFS}/
+#
+# matches none of the `rm\s+` patterns while executing as a full recursive
+# delete. The same holds for a command assembled by a base64 decode piped to a
+# shell. Rather than try to out-parse bash, treat these splitting/decoding
+# primitives as a reason to ask: they are vanishingly rare in commands a human
+# actually means to run unattended.
+if printf '%s' "$CMD" | grep -qE '\$\{IFS\}|\$IFS|\$\(echo[^)]*base64[^)]*\)|base64[[:space:]]+(-d|--decode)[^|]*\|[[:space:]]*(sh|bash)' 2>/dev/null; then
+  printf '{"permissionDecision":"ask","message":"[careful] Shell obfuscation detected (IFS word-splitting or base64-to-shell). Read the command carefully before approving."}\n'
+  exit 0
+fi
+
 # --- Check for safe exceptions (one standalone rm of build artifacts) ---
 # Match the complete command. Parsing only the last rm is unsafe because shell
 # syntax or comments can hide an earlier destructive command, for example:
 #   rm -rf / # rm -rf node_modules
 # Unknown syntax fails closed and falls through to the destructive checks.
-if printf '%s' "$CMD" | grep -qE '^[[:space:]]*rm[[:space:]]+(-[a-zA-Z]*r[a-zA-Z]*[[:space:]]+|--recursive[[:space:]]+)(([^[:space:];&|#]*/)?(node_modules|\.next|dist|__pycache__|\.cache|build|\.turbo|coverage)[[:space:]]*)+$' 2>/dev/null; then
+if printf '%s' "$CMD" | grep -qE '^[[:space:]]*rm[[:space:]]+(-[a-zA-Z]*[rR][a-zA-Z]*[[:space:]]+|--recursive[[:space:]]+)(([^[:space:];&|#]*/)?(node_modules|\.next|dist|__pycache__|\.cache|build|\.turbo|coverage)[[:space:]]*)+$' 2>/dev/null; then
   echo '{}'
   exit 0
 fi
@@ -40,7 +88,9 @@ WARN=""
 PATTERN=""
 
 # rm -rf / rm -r / rm --recursive
-if printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*r|--recursive)' 2>/dev/null; then
+# [rR] because rm accepts -R as an equal synonym for -r (and -fR, -Rf); matching
+# only lowercase let `rm -R /` through as a non-recursive command.
+if printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*[rR]|--recursive)' 2>/dev/null; then
   WARN="Destructive: recursive delete (rm -r). This permanently removes files."
   PATTERN="rm_recursive"
 fi

test/hook-scripts.test.ts

@@ -111,13 +111,93 @@ describe('check-careful.sh', () => {
       expect(output.permissionDecision).toBe('ask');
       expect(output.message).toContain('recursive delete');
     });
+
+    test.each([
+      'rm -R /',
+      'rm -Rf ~',
+      'rm -fR /var/data',
+    ])('uppercase -R is recursive too: %s', (command) => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('recursive delete');
+    });
+
+    test('rm -Rf node_modules still allows (safe exception accepts -R)', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -Rf node_modules'));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBeUndefined();
+    });
+
+    test.each([
+      'git commit -m "wip" && rm -rf /',
+      'bash -c "rm -rf /"',
+      'echo "x"; rm -rf ~',
+      'npm run build --msg "done" && rm -rf /',
+    ])('a quoted argument cannot hide a later destructive command: %s', (command) => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('recursive delete');
+    });
+  });
+
+  // --- Shell obfuscation ---
+
+  describe('shell obfuscation', () => {
+    test.each([
+      'rm${IFS}-rf${IFS}/',
+      'rm$IFS-rf$IFS/',
+      'echo cm0gLXJmIC8= | base64 -d | sh',
+    ])('asks when the command hides its shape behind expansion: %s', (command) => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('obfuscation');
+    });
+
+    test('ordinary commands are unaffected', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('cat file.b64 | base64 -d > out.bin'));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBeUndefined();
+    });
+  });
+
+  // --- JSON payload extraction ---
+
+  describe('command extraction', () => {
+    test('fails closed when the payload is not valid JSON', () => {
+      const { exitCode, output } = runHookRaw(CAREFUL_SCRIPT, 'this is not json');
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+      expect(output.message).toContain('parse');
+    });
+
+    test('allows a well-formed payload with no command field', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, { tool_input: { file_path: '/tmp/x' } });
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBeUndefined();
+    });
+
+    test('allows when command is present but not a string', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, { tool_input: { command: 42 } });
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBeUndefined();
+    });
+
+    test('preserves escaped quotes in the extracted command', () => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('echo "hello world"'));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBeUndefined();
+    });
   });
 
   // --- SQL destructive commands ---
-  // Note: SQL commands that contain embedded double quotes (e.g., psql -c "DROP TABLE")
-  // get their command value truncated by the grep-based JSON extractor because \"
-  // terminates the [^"]* match. We use commands WITHOUT embedded quotes so the grep
-  // extraction works and the SQL keywords are visible to the pattern matcher.
+  // Embedded double quotes are now safe to use here. They previously truncated the
+  // extracted command (the grep-based extractor stopped at the first \"), which hid
+  // the SQL keyword from the pattern matcher — so these tests had to be written
+  // without quotes, in a shape no one actually types. The JSON-parser extraction
+  // fixed that, and the quoted forms below are the realistic ones.
 
   describe('SQL destructive commands', () => {
     test('psql DROP TABLE warns with DROP in message', () => {
@@ -127,6 +207,16 @@ describe('check-careful.sh', () => {
       expect(output.message).toContain('DROP');
     });
 
+    test.each([
+      'psql -c "DROP TABLE users"',
+      'psql -c "TRUNCATE orders"',
+      'mysql -e "DROP DATABASE prod"',
+    ])('a quoted SQL statement is still inspected: %s', (command) => {
+      const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command));
+      expect(exitCode).toBe(0);
+      expect(output.permissionDecision).toBe('ask');
+    });
+
     test('mysql drop database warns (case insensitive)', () => {
       const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('mysql -e drop database mydb'));
       expect(exitCode).toBe(0);