From 630e3d3a28a10362ce7163059b99d39e62a541d2 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 14 Jul 2026 18:38:33 -0700 Subject: [PATCH] fix(careful): parse hook commands as JSON first --- careful/bin/check-careful.sh | 27 +++++++++++++++++++-------- test/hook-scripts.test.ts | 28 +++++++++++++++------------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index 22bf8b922..adde07510 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -7,13 +7,24 @@ 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) +# Parse JSON before inspecting the command. Regex extraction cannot distinguish +# escaped quotes from the end of a JSON string and can hide a destructive tail. +# A malformed hook payload is itself unsafe, so fail closed and ask. +if ! CMD=$(printf '%s' "$INPUT" | python3 -c ' +import json, sys +data = json.load(sys.stdin) +if not isinstance(data, dict): + raise ValueError("hook payload must be an object") +tool_input = data.get("tool_input", {}) +if not isinstance(tool_input, dict): + raise ValueError("tool_input must be an object") +command = tool_input.get("command", "") +if not isinstance(command, str): + raise ValueError("command must be a string") +sys.stdout.write(command) +' 2>/dev/null); then + echo '{"permissionDecision":"ask","message":"[careful] Could not parse the command payload safely. Review it before proceeding."}' + exit 0 fi # If we still couldn't extract a command, allow @@ -30,7 +41,7 @@ CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]') # 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 [[ "$CMD" != *$'\n'* ]] && 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 echo '{}' exit 0 fi diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index db2e7629f..8402139f5 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -114,11 +114,6 @@ describe('check-careful.sh', () => { }); // --- 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. - describe('SQL destructive commands', () => { test('psql DROP TABLE warns with DROP in message', () => { const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('psql -c DROP TABLE users;')); @@ -140,6 +135,16 @@ describe('check-careful.sh', () => { expect(output.permissionDecision).toBe('ask'); expect(output.message).toContain('TRUNCATE'); }); + + test('escaped quotes cannot hide DROP TABLE from JSON parsing', () => { + const { exitCode, output } = runHook( + CAREFUL_SCRIPT, + carefulInput('psql -c "DROP TABLE users"'), + ); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('DROP'); + }); }); // --- Git destructive commands --- @@ -241,17 +246,14 @@ describe('check-careful.sh', () => { expect(output.permissionDecision).toBeUndefined(); }); - test('malformed JSON input allows gracefully (exit 0, output {})', () => { - const { exitCode, raw } = runHookRaw(CAREFUL_SCRIPT, 'this is not json at all{{{{'); + test('malformed JSON input fails closed', () => { + const { exitCode, output } = runHookRaw(CAREFUL_SCRIPT, 'this is not json at all{{{{'); expect(exitCode).toBe(0); - expect(raw).toBe('{}'); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('parse'); }); - test('Python fallback: grep fails on multiline JSON, Python parses it', () => { - // Construct JSON where "command": and the value are on separate lines. - // grep works line-by-line, so it cannot match "command"..."value" across lines. - // This forces CMD to be empty, triggering the Python fallback which handles - // the full JSON correctly. + test('JSON parser handles a command value on the next line', () => { const rawJson = '{"tool_input":{"command":\n"rm -rf /tmp/important"}}'; const { exitCode, output } = runHookRaw(CAREFUL_SCRIPT, rawJson); expect(exitCode).toBe(0);