mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-18 19:02:18 +02:00
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>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
11de390be1
commit
b22f7a66e1
@@ -26,7 +26,19 @@ fi
|
|||||||
CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]')
|
CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]')
|
||||||
|
|
||||||
# --- Check for safe exceptions (rm -rf of build artifacts) ---
|
# --- 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
|
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
|
||||||
|
|||||||
@@ -96,6 +96,31 @@ 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');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// 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 ---
|
// --- SQL destructive commands ---
|
||||||
|
|||||||
Reference in New Issue
Block a user