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.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user