diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index 3103289fd..22ed6bac2 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -36,9 +36,13 @@ 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 ;; + # Command/backtick substitution counts as chaining too: a token like + # `$(./wipe-all)/node_modules` ends in a whitelisted suffix while running + # anything inside the substitution. Plain $VAR expansion stays allowed — + # only `$(` triggers. + *';'*|*'|'*|*'&'*|*'$('*|*'`'*|*$'\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 +if [ "$HAS_SEPARATOR" = false ] && printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*[rR][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 @@ -63,8 +67,8 @@ fi 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 +# rm -rf / rm -r / rm -R / rm --recursive (capital -R is BSD/macOS recursive) +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 diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index 5fc70583c..be3c12e34 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -121,6 +121,66 @@ describe('check-careful.sh', () => { expect(output.permissionDecision).toBe('ask'); expect(output.message).toContain('recursive delete'); }); + + // Command substitution is a chaining form: the substitution token can end in + // a whitelisted suffix while running anything inside $(...) or backticks, + // and the safe-exception early exit would skip ALL downstream checks. + test('rm -rf $(./wipe-all)/node_modules warns (command substitution)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf $(./wipe-all)/node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -rf `./wipe-all`/node_modules warns (backtick substitution)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf `./wipe-all`/node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + // Capital -R is the documented recursive flag on BSD rm (macOS) and accepted + // by GNU rm. Both greps previously required a lowercase r, so `rm -R /` + // silently allowed. + test('rm -R / warns (capital -R recursive)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -R /')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -fR /home/user warns (capital R in flag cluster)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -fR /home/user')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -Rf node_modules allows (capital R, single safe target)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -Rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + + // The JSON-escaped-newline separator branch (literal two-char \n surviving + // the grep extraction path) had dedicated code but no test exercising it. + test('newline-chained rm warns (escaped-newline separator branch)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/x\nrm -rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + // Deliberate false positive, pinned: a safe-prefix chain ending in a safe rm + // is indistinguishable from the dangerous-first exploit shape without real + // shell parsing, so warn-on-all-chains is the designed fail-closed direction. + // A future per-segment parser must consciously change this test. + test('cd app && rm -rf node_modules asks (fail-closed on chains, by design)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('cd app && rm -rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); }); // --- SQL destructive commands ---