mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-22 04:40:44 +02:00
harden(careful): substitution separators + capital -R recursive flag (#2039)
Two residual fail-opens in the same guard PR #2040 hardened, both verified by executing the script pre-fix: - rm -rf $(./wipe-all)/node_modules silently allowed: the substitution token ends in a whitelisted suffix and the safe-exception early exit skipped ALL downstream checks. $( and backtick now count as chain separators; plain $VAR expansion stays allowed. - rm -R / silently allowed: both greps required a lowercase r in the flag cluster; capital -R is the documented BSD/macOS recursive flag. Both greps now match -[a-zA-Z]*[rR]. Six new tests: substitution x2 -> ask, capital-R x2 -> ask, rm -Rf node_modules single-command -> still allowed, escaped-newline branch (existing code, previously untested), and a pinned deliberate FP (cd app && rm -rf node_modules -> ask) documenting the fail-closed direction on chains. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
b22f7a66e1
commit
5d23ccab56
@@ -36,9 +36,13 @@ HAS_SEPARATOR=false
|
|||||||
case "$CMD" in
|
case "$CMD" in
|
||||||
# Real separators, plus JSON-escaped newlines (\n / \r) which survive the
|
# Real separators, plus JSON-escaped newlines (\n / \r) which survive the
|
||||||
# grep extraction path as literal two-char sequences and still mark a chain.
|
# 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
|
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
|
SAFE_ONLY=true
|
||||||
RM_ARGS=$(printf '%s' "$CMD" | sed -E 's/.*rm[[:space:]]+(-[a-zA-Z]+[[:space:]]+)*//;s/--recursive[[:space:]]*//')
|
RM_ARGS=$(printf '%s' "$CMD" | sed -E 's/.*rm[[:space:]]+(-[a-zA-Z]+[[:space:]]+)*//;s/--recursive[[:space:]]*//')
|
||||||
for target in $RM_ARGS; do
|
for target in $RM_ARGS; do
|
||||||
@@ -63,8 +67,8 @@ fi
|
|||||||
WARN=""
|
WARN=""
|
||||||
PATTERN=""
|
PATTERN=""
|
||||||
|
|
||||||
# rm -rf / rm -r / rm --recursive
|
# 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]*r|--recursive)' 2>/dev/null; then
|
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."
|
WARN="Destructive: recursive delete (rm -r). This permanently removes files."
|
||||||
PATTERN="rm_recursive"
|
PATTERN="rm_recursive"
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -121,6 +121,66 @@ describe('check-careful.sh', () => {
|
|||||||
expect(output.permissionDecision).toBe('ask');
|
expect(output.permissionDecision).toBe('ask');
|
||||||
expect(output.message).toContain('recursive delete');
|
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 ---
|
// --- SQL destructive commands ---
|
||||||
|
|||||||
Reference in New Issue
Block a user