From d421f539b74c3ee67f0d1778ba8f9354655a70cd Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 15:05:25 +0200 Subject: [PATCH 1/6] fix(safety): flag-order variants for docker system prune --volumes and compose down -v Destructive-pattern matching is contiguous: 'docker system prune --volumes' (HIGH) does not match 'docker system prune -f --volumes' because -f sits between the two anchor tokens, so the command falls back to bare 'docker system prune' (MEDIUM) and under-reports the blast radius when the --volumes flag is actually present. Same story on the compose side: 'docker compose down --volumes' (HIGH) does not cover 'docker compose down --remove-orphans -v' or 'docker compose down -v --remove-orphans', which land on bare 'docker compose down' (LOW-MEDIUM) even though named volumes are wiped. Add explicit variants for every observed ordering of -f, -a, -af, -fa, -v, --volumes, and --remove-orphans, plus regression tests. --- pkg/safety/safety_patterns.json | 10 ++++++++++ pkg/safety/safety_test.go | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/pkg/safety/safety_patterns.json b/pkg/safety/safety_patterns.json index 42d44db6ce..82d11b193c 100644 --- a/pkg/safety/safety_patterns.json +++ b/pkg/safety/safety_patterns.json @@ -94,7 +94,14 @@ { "pattern": "docker system prune -a", "blast_radius": "MEDIUM-HIGH", "category": "dk-multi-del", "notes": "adds all unused images" }, { "pattern": "docker system prune --all", "blast_radius": "MEDIUM-HIGH", "category": "dk-multi-del", "notes": "long form of -a" }, { "pattern": "docker system prune --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "adds named-volume deletion" }, + { "pattern": "docker system prune -f --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: force between prune and --volumes" }, + { "pattern": "docker system prune -a --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: -a between prune and --volumes" }, + { "pattern": "docker system prune --volumes -f", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: --volumes precedes -f" }, + { "pattern": "docker system prune --volumes -a", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: --volumes precedes -a" }, + { "pattern": "docker system prune --volumes -af", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant of -af --volumes" }, + { "pattern": "docker system prune --volumes -fa", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant of -af --volumes" }, { "pattern": "docker system prune -af --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "the YOLO combination" }, + { "pattern": "docker system prune -fa --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant of -af --volumes" }, { "pattern": "docker buildx prune", "blast_radius": "MEDIUM", "category": "dk-build-cache", "notes": "rebuild cost only" }, { "pattern": "docker buildx prune --all", "blast_radius": "MEDIUM-HIGH", "category": "dk-build-cache", "notes": "every cached layer" }, { "pattern": "docker buildx prune -a", "blast_radius": "MEDIUM-HIGH", "category": "dk-build-cache", "notes": "short form of --all" }, @@ -105,6 +112,9 @@ { "pattern": "docker compose down -v", "blast_radius": "HIGH", "category": "dk-compose", "notes": "drops named volumes" }, { "pattern": "docker compose down --volumes", "blast_radius": "HIGH", "category": "dk-compose", "notes": "long form of -v" }, { "pattern": "docker compose down --volumes --remove-orphans", "blast_radius": "HIGH", "category": "dk-compose", "notes": "aggressive cleanup" }, + { "pattern": "docker compose down --remove-orphans -v", "blast_radius": "HIGH", "category": "dk-compose", "notes": "flag-order variant: -v after --remove-orphans still drops named volumes" }, + { "pattern": "docker compose down --remove-orphans --volumes", "blast_radius": "HIGH", "category": "dk-compose", "notes": "flag-order variant of --volumes --remove-orphans" }, + { "pattern": "docker compose down -v --remove-orphans", "blast_radius": "HIGH", "category": "dk-compose", "notes": "short-flag ordering with --remove-orphans" }, { "pattern": "docker compose rm -v ", "blast_radius": "MEDIUM-HIGH", "category": "dk-compose", "notes": "volume removal per service" }, { "pattern": "docker compose rm -f", "blast_radius": "MEDIUM", "category": "dk-compose", "notes": "bypasses removal prompt" }, { "pattern": "docker context rm ", "blast_radius": "LOW", "category": "dk-context", "notes": "reversible if endpoint known" }, diff --git a/pkg/safety/safety_test.go b/pkg/safety/safety_test.go index d063ae3b63..daccf2c558 100644 --- a/pkg/safety/safety_test.go +++ b/pkg/safety/safety_test.go @@ -21,6 +21,13 @@ func TestClassifyCommand_DestructivePatterns(t *testing.T) { {"docker rm -f web", "high"}, {"docker rm -vf web", "high"}, {"docker stop web", "low"}, + {"docker system prune -f --volumes", "high"}, + {"docker system prune --volumes -f", "high"}, + {"docker system prune -a --volumes", "high"}, + {"docker system prune -fa --volumes", "high"}, + {"docker compose down --remove-orphans -v", "high"}, + {"docker compose down -v --remove-orphans", "high"}, + {"docker compose down --remove-orphans --volumes", "high"}, {"docker volume prune --all", "high"}, {"docker volume prune -af", "high"}, {"docker builder prune", "medium"}, From 267b33403bd9661d9a710b472fd7d8473d066fd0 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 15:07:18 +0200 Subject: [PATCH 2/6] feat(safety): PowerShell Remove-Item and Clear-Content patterns The destructive taxonomy is Unix-only; Windows users running the shell tool in PowerShell can execute destructive commands (Remove-Item, Clear-Content) that the classifier reports as unknown, so the UI cannot label them and permissive safety modes lose the chance to warn on blast radius. Add PowerShell equivalents for the fs-delete and fs-overwrite categories, covering: - Remove-Item ... -Recurse -Force / -Force -Recurse (HIGH, rm -rf equivalent) - Remove-Item ... -Recurse (HIGH, still irreversible on non-tty) - Remove-Item ... -Force (MEDIUM, suppresses confirmation) - Remove-Item (LOW, prompts on tty) - Clear-Content (MEDIUM, in-place truncate) Positional, -Path, and trailing-path forms are enumerated so the observed argument orderings all match. Argument matching is already case-insensitive via normalizeCommand, so 'remove-item' and 'REMOVE-ITEM' also match. --- pkg/safety/safety_patterns.json | 18 ++++++++++++++++++ pkg/safety/safety_test.go | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/pkg/safety/safety_patterns.json b/pkg/safety/safety_patterns.json index 82d11b193c..634740015c 100644 --- a/pkg/safety/safety_patterns.json +++ b/pkg/safety/safety_patterns.json @@ -37,6 +37,24 @@ { "pattern": "chmod -R 777 ", "blast_radius": "LOW", "category": "fs-permissions", "notes": "security regression, reversible" }, { "pattern": "sed -i", "blast_radius": "MEDIUM", "category": "fs-modify", "notes": "in-place file edit; -i.bak variants also match" } ], + "powershell": [ + { "pattern": "Remove-Item -Path -Recurse -Force", "blast_radius": "HIGH", "category": "fs-delete", "notes": "PowerShell recursive force delete; equivalent to `rm -rf`" }, + { "pattern": "Remove-Item -Path -Force -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "PowerShell flag-order variant of -Recurse -Force" }, + { "pattern": "Remove-Item -Recurse -Force", "blast_radius": "HIGH", "category": "fs-delete", "notes": "positional path form of recursive force delete" }, + { "pattern": "Remove-Item -Force -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "flag-order variant of positional recursive force delete" }, + { "pattern": "Remove-Item -Recurse -Force ", "blast_radius": "HIGH", "category": "fs-delete", "notes": "trailing-path form of recursive force delete" }, + { "pattern": "Remove-Item -Force -Recurse ", "blast_radius": "HIGH", "category": "fs-delete", "notes": "flag-order variant of trailing-path recursive force delete" }, + { "pattern": "Remove-Item -Path -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "PowerShell recursive delete without -Force; still irreversible on non-tty" }, + { "pattern": "Remove-Item -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "positional path form of recursive delete" }, + { "pattern": "Remove-Item -Recurse ", "blast_radius": "HIGH", "category": "fs-delete", "notes": "trailing-path form of recursive delete" }, + { "pattern": "Remove-Item -Path -Force", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "PowerShell force delete of a single item; suppresses confirmation" }, + { "pattern": "Remove-Item -Force", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "positional form of PowerShell force delete" }, + { "pattern": "Remove-Item -Force ", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "flag-first form of PowerShell force delete" }, + { "pattern": "Remove-Item -Path ", "blast_radius": "LOW", "category": "fs-delete", "notes": "PowerShell delete of a single item; prompts on tty" }, + { "pattern": "Remove-Item ", "blast_radius": "LOW", "category": "fs-delete", "notes": "positional form of PowerShell delete" }, + { "pattern": "Clear-Content ", "blast_radius": "MEDIUM", "category": "fs-overwrite", "notes": "PowerShell truncate; empties file contents in place" }, + { "pattern": "Clear-Content -Path ", "blast_radius": "MEDIUM", "category": "fs-overwrite", "notes": "PowerShell truncate via -Path" } + ], "git": [ { "pattern": "git reset --hard", "blast_radius": "HIGH", "category": "git-discard", "notes": "discards uncommitted work; unrecoverable" }, { "pattern": "git checkout -- ", "blast_radius": "MEDIUM-HIGH", "category": "git-discard", "notes": "overwrites local modifications" }, diff --git a/pkg/safety/safety_test.go b/pkg/safety/safety_test.go index daccf2c558..8420c53db2 100644 --- a/pkg/safety/safety_test.go +++ b/pkg/safety/safety_test.go @@ -41,6 +41,14 @@ func TestClassifyCommand_DestructivePatterns(t *testing.T) { {"git push --force origin main", "medium"}, {"git push --force-with-lease origin main", "medium"}, {"git stash clear", "high"}, + {`Remove-Item -Path ".\config\cache\*" -Recurse -Force`, "high"}, + {`Remove-Item -Path ".\config\cache\*" -Force -Recurse`, "high"}, + {`Remove-Item "C:\tmp\foo" -Recurse -Force`, "high"}, + {`Remove-Item -Recurse -Force C:\tmp\foo`, "high"}, + {`Remove-Item -Path ".\file.txt" -Force`, "medium"}, + {`Remove-Item .\file.txt -Force`, "medium"}, + {`Remove-Item -Path ".\file.txt"`, "low"}, + {`Clear-Content -Path .\log.txt`, "medium"}, } for _, tt := range tests { t.Run(tt.command, func(t *testing.T) { From 3c6862cc616d2c9b7cc505feb5d02f8b7f4b71fc Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 15:09:06 +0200 Subject: [PATCH 3/6] feat(safety): destructive SQL, app-CLI, and key-value-store patterns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Destructive intent frequently arrives via a wrapper — 'docker exec …', 'kubectl exec …', an SSH login — that hides the dangerous verb inside the inner command. The current taxonomy only inspects the outer shell, so real data-loss commands come back as unknown and permissive safety modes can't tell the UI what the blast radius actually is. Add pattern groups that fire on the destructive verb wherever it appears in the command line, so wrapping through docker exec / kubectl exec / ssh inherits the classification automatically: - SQL DDL/DML (drop database, drop table, drop schema, truncate table, delete from) — matches across mysql, mariadb, psql, sqlite3, and the -e or -c one-shot forms these clients accept. - App-framework reset verbs (n8n db:reset and user-management:reset, Frappe bench new-site --force / drop-site, Rails db:drop / db:reset / db:schema:load, Django manage.py flush, Prisma migrate reset, Sequelize db:drop) — one-shot commands that wipe or replace application state. - Key-value flushes (redis-cli flushall / flushdb). Regression tests cover the docker-exec-wrapped forms as well as the bare client invocations. --- pkg/safety/safety_patterns.json | 26 ++++++++++++++++++++++++++ pkg/safety/safety_test.go | 13 +++++++++++++ 2 files changed, 39 insertions(+) diff --git a/pkg/safety/safety_patterns.json b/pkg/safety/safety_patterns.json index 634740015c..f831ec3c78 100644 --- a/pkg/safety/safety_patterns.json +++ b/pkg/safety/safety_patterns.json @@ -143,6 +143,32 @@ { "pattern": "docker exec rm -rf ", "blast_radius": "HIGH", "category": "dk-exec-host", "notes": "filesystem op via bind mount hits host" }, { "pattern": "docker run --rm -v $(pwd):/x rm -rf /x", "blast_radius": "HIGH", "category": "dk-bind-mount", "notes": "host-bind YOLO pattern" }, { "pattern": "docker run --rm -v /:/host ", "blast_radius": "HIGH", "category": "dk-bind-mount", "notes": "mounts host root — almost always wrong" } + ], + "sql": [ + { "pattern": "drop database ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "destroys entire database and all data within; matches across mysql/psql/sqlite/mariadb clients and their docker-exec invocations" }, + { "pattern": "drop database if exists ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "idempotent form still destroys the database if present" }, + { "pattern": "drop schema ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "destroys schema and its objects" }, + { "pattern": "drop schema if exists ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "idempotent form" }, + { "pattern": "drop table ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "destroys table schema and rows" }, + { "pattern": "drop table if exists ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "idempotent form" }, + { "pattern": "truncate table ", "blast_radius": "MEDIUM-HIGH", "category": "sql-truncate", "notes": "removes all rows; typically non-transactional" }, + { "pattern": "delete from ", "blast_radius": "MEDIUM", "category": "sql-delete", "notes": "unqualified DELETE would remove every row; presence of WHERE cannot be checked from the command shape" } + ], + "app-cli": [ + { "pattern": "n8n db:reset", "blast_radius": "HIGH", "category": "app-reset", "notes": "wipes the n8n instance database" }, + { "pattern": "n8n user-management:reset", "blast_radius": "MEDIUM", "category": "app-reset", "notes": "resets the n8n owner account and API keys" }, + { "pattern": "bench new-site --force ", "blast_radius": "HIGH", "category": "app-reset", "notes": "Frappe/ERPNext: --force overwrites an existing site's database in place" }, + { "pattern": "bench drop-site ", "blast_radius": "HIGH", "category": "app-reset", "notes": "Frappe/ERPNext: drops the site's database and archive" }, + { "pattern": "rails db:drop", "blast_radius": "HIGH", "category": "app-reset", "notes": "Rails: drops the database for the current env" }, + { "pattern": "rails db:reset", "blast_radius": "HIGH", "category": "app-reset", "notes": "Rails: drop + create + migrate + seed" }, + { "pattern": "rails db:schema:load", "blast_radius": "HIGH", "category": "app-reset", "notes": "Rails: replaces the schema, wiping rows" }, + { "pattern": "manage.py flush", "blast_radius": "HIGH", "category": "app-reset", "notes": "Django: removes all data from the database" }, + { "pattern": "prisma migrate reset", "blast_radius": "HIGH", "category": "app-reset", "notes": "Prisma: drops schema and reruns migrations" }, + { "pattern": "sequelize db:drop", "blast_radius": "HIGH", "category": "app-reset", "notes": "Sequelize CLI: drops the database" } + ], + "key-value-store": [ + { "pattern": "redis-cli flushall", "blast_radius": "HIGH", "category": "kv-flush", "notes": "removes every key from every Redis database" }, + { "pattern": "redis-cli flushdb", "blast_radius": "HIGH", "category": "kv-flush", "notes": "removes every key from the current Redis database" } ] }, "safe": { diff --git a/pkg/safety/safety_test.go b/pkg/safety/safety_test.go index 8420c53db2..3b1918423e 100644 --- a/pkg/safety/safety_test.go +++ b/pkg/safety/safety_test.go @@ -49,6 +49,19 @@ func TestClassifyCommand_DestructivePatterns(t *testing.T) { {`Remove-Item .\file.txt -Force`, "medium"}, {`Remove-Item -Path ".\file.txt"`, "low"}, {`Clear-Content -Path .\log.txt`, "medium"}, + {`docker exec mariadb-1 mariadb -uroot -proot -e "DROP DATABASE IF EXISTS azfulfillment_db;"`, "high"}, + {`psql -c "DROP TABLE users"`, "high"}, + {`sqlite3 /tmp/x.db "DROP SCHEMA public CASCADE"`, "high"}, + {`mysql -e "TRUNCATE TABLE sessions"`, "high"}, + {`docker exec pg psql -c "DELETE FROM audit_log"`, "medium"}, + {"docker exec n8n n8n db:reset", "high"}, + {"docker exec n8n n8n user-management:reset", "medium"}, + {"docker exec frappe bench new-site --force site1.local", "high"}, + {"docker exec redis redis-cli FLUSHALL", "high"}, + {"docker exec redis redis-cli FLUSHDB", "high"}, + {"docker exec app rails db:reset", "high"}, + {"docker exec app python manage.py flush --noinput", "high"}, + {"docker exec app npx prisma migrate reset --force", "high"}, } for _, tt := range tests { t.Run(tt.command, func(t *testing.T) { From 05937b1890859a22aff28d0b12f65433abefc0f0 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 15:09:53 +0200 Subject: [PATCH 4/6] fix(safety): anchor shell-metachar patterns on whitespace, not word boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The destructive-pattern regex prefix is (?:^|.*\b), which fires on any word boundary in the normalised command. That's the right anchor for alphanumeric verbs like "rm" or "docker", but wrong for patterns whose leading token is itself a shell metacharacter — ">", "<", "|", "&", ";". A word boundary sits between any word char and the metachar, so: - "--email --firstName Admin" matches the truncate pattern "> " via the l> boundary inside the redacted placeholder. - 'git commit -m "feat: 1>0 check"' matches the same pattern via 1>0. - Any placeholder or in-string comparison that happens to sit between two word characters spuriously escalates to destructive/fs-overwrite. Anchor shell-metacharacter patterns on \s instead: a real redirect, pipe, or chain operator is preceded by whitespace (or the start of the line) after normalizeCommand collapses runs to single spaces. Verbal patterns keep the \b anchor so 'cd /tmp && rm -rf foo' still resolves its embedded 'rm -rf' correctly. --- pkg/safety/classify.go | 28 +++++++++++++++++++++++++++- pkg/safety/safety_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/pkg/safety/classify.go b/pkg/safety/classify.go index f8738ea66f..a216867c23 100644 --- a/pkg/safety/classify.go +++ b/pkg/safety/classify.go @@ -204,6 +204,20 @@ func carriesDenyFlag(normalized string, flags []string) bool { return false } +// isShellMetacharAnchor reports whether c can start a pattern that +// only makes sense as a shell operator (redirect, pipe, chain, +// separator). Anchoring such patterns on `\b` is too weak — a word +// boundary fires between any word char and the operator, so a +// letter-adjacent `>` (e.g. inside `` or a literal `"a>b"`) +// would spuriously match `> `. +func isShellMetacharAnchor(c byte) bool { + switch c { + case '>', '<', '|', '&', ';': + return true + } + return false +} + // containsShellMetacharacter returns true when the command contains a // character that can chain (`;`, `&`), pipe (`|`), redirect (`<`, `>`), // or substitute (backticks, `$(`) commands — with or without @@ -300,9 +314,21 @@ func stringSlice(value any) []string { // matches anywhere in the normalised command. Destructive intent is // the priority — a destructive pattern hidden inside a larger // command (e.g. `cd /tmp && rm -rf foo`) should still match. +// +// A pattern that begins with a shell metacharacter (`>`, `<`, `|`, +// `&`, `;`) is anchored on whitespace rather than a word boundary: +// `>` between two word characters isn't a shell redirect, so a +// redacted placeholder like `` or an inline comparison like +// `"1 > 0"` must not match the `> ` truncate pattern. The +// `\b` prefix would have fired on either. func patternToRegexp(pattern string) string { var b strings.Builder - b.WriteString(`(?i)(?:^|.*\b)`) + b.WriteString(`(?i)`) + if pattern != "" && isShellMetacharAnchor(pattern[0]) { + b.WriteString(`(?:^|.*\s)`) + } else { + b.WriteString(`(?:^|.*\b)`) + } for i := 0; i < len(pattern); { switch pattern[i] { case '<': diff --git a/pkg/safety/safety_test.go b/pkg/safety/safety_test.go index 3b1918423e..86a5c7fc6f 100644 --- a/pkg/safety/safety_test.go +++ b/pkg/safety/safety_test.go @@ -152,6 +152,34 @@ func TestClassifyCommand_DenyFlagsAreNeverSafe(t *testing.T) { assert.Equal(t, ClassSafe, ClassifyCommand("git log --oneline -5").Class) } +// The truncate-redirect pattern `> ` must not fire when `>` sits +// between two word characters — it isn't a shell redirect there, just a +// placeholder or comparison operator. Anchoring on `\b` alone was too +// permissive. +func TestClassifyCommand_TruncateRedirectRequiresWhitespaceAnchor(t *testing.T) { + notDestructive := []string{ + `docker exec n8n n8n user:create --email --firstName Admin`, + `git commit -m "feat: 1>0 check"`, + `echo "a>b"`, + } + for _, command := range notDestructive { + t.Run(command, func(t *testing.T) { + assert.NotEqual(t, ClassDestructive, ClassifyCommand(command).Class, + "letter-adjacent > must not match the > truncate pattern") + }) + } + + // Real redirects — `>` preceded by whitespace — must still match. + for _, command := range []string{ + `echo hi > /etc/passwd`, + `cat data > /tmp/out.txt`, + } { + t.Run(command, func(t *testing.T) { + assert.Equal(t, ClassDestructive, ClassifyCommand(command).Class) + }) + } +} + func TestClassifyCommand_UnknownCommand(t *testing.T) { label := ClassifyCommand("./deploy.sh --prod") assert.Equal(t, ClassUnknown, label.Class) From 747151af45f9c3f75284cf8b8f37fa46df3af4f9 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 16:03:02 +0200 Subject: [PATCH 5/6] fix(safety): use wildcards for flag-order and quoted-path gaps; narrow SQL patterns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #4081: - Docker: the classifier's compiled patterns already have a trailing '\b.*' suffix, so 'docker system prune --volumes' already covers '--volumes -f/-a/-af/-fa' on master; enumerating those orderings was redundant. Drop them and add wildcard entries 'docker system prune ...--volumes', 'docker compose down ...-v', and 'docker compose down ...--volumes' to close the only genuine gaps ('-f --volumes', '-fa --volumes', and '--remove-orphans -v/--volumes'). - PowerShell: '' compiles to '\S+' and cannot span a quoted path with spaces ('C:\Program Files\…', 'OneDrive - Corp\…'), so the enumerated -Recurse/-Force orderings fell through to LOW on every Windows path that contains a space. Collapse the 14 Remove- Item entries to two wildcard forms plus a positional base case. - Redis / Frappe: connection flags on 'redis-cli' ('-h', '-p', '-n') and the site-first ordering on 'bench new-site … --force' broke contiguity and reported unknown. Wildcards fix both. - SQL: 'drop database' / 'drop table' / 'delete from' matched inside the quoted arguments of safe-listed search commands, flipping 'grep "drop table users" -r .' from safe to destructive/high. Under 'restricted' this changes ALLOW to DENY on a common workflow. Anchor every SQL DDL/DML pattern on a known client executable (mysql / mariadb / psql / sqlite3); the leading '.*\b' anchor keeps wrapper commands (docker exec, kubectl exec, ssh) matching. Add regression tests both for the safe cases and for the accepted precedence caveat (destructive wins when the client name itself appears in the search argument). --- pkg/safety/safety_patterns.json | 72 +++++++++++++++------------------ pkg/safety/safety_test.go | 29 +++++++++++++ 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/pkg/safety/safety_patterns.json b/pkg/safety/safety_patterns.json index f831ec3c78..b674e5b126 100644 --- a/pkg/safety/safety_patterns.json +++ b/pkg/safety/safety_patterns.json @@ -38,22 +38,10 @@ { "pattern": "sed -i", "blast_radius": "MEDIUM", "category": "fs-modify", "notes": "in-place file edit; -i.bak variants also match" } ], "powershell": [ - { "pattern": "Remove-Item -Path -Recurse -Force", "blast_radius": "HIGH", "category": "fs-delete", "notes": "PowerShell recursive force delete; equivalent to `rm -rf`" }, - { "pattern": "Remove-Item -Path -Force -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "PowerShell flag-order variant of -Recurse -Force" }, - { "pattern": "Remove-Item -Recurse -Force", "blast_radius": "HIGH", "category": "fs-delete", "notes": "positional path form of recursive force delete" }, - { "pattern": "Remove-Item -Force -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "flag-order variant of positional recursive force delete" }, - { "pattern": "Remove-Item -Recurse -Force ", "blast_radius": "HIGH", "category": "fs-delete", "notes": "trailing-path form of recursive force delete" }, - { "pattern": "Remove-Item -Force -Recurse ", "blast_radius": "HIGH", "category": "fs-delete", "notes": "flag-order variant of trailing-path recursive force delete" }, - { "pattern": "Remove-Item -Path -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "PowerShell recursive delete without -Force; still irreversible on non-tty" }, - { "pattern": "Remove-Item -Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "positional path form of recursive delete" }, - { "pattern": "Remove-Item -Recurse ", "blast_radius": "HIGH", "category": "fs-delete", "notes": "trailing-path form of recursive delete" }, - { "pattern": "Remove-Item -Path -Force", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "PowerShell force delete of a single item; suppresses confirmation" }, - { "pattern": "Remove-Item -Force", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "positional form of PowerShell force delete" }, - { "pattern": "Remove-Item -Force ", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "flag-first form of PowerShell force delete" }, - { "pattern": "Remove-Item -Path ", "blast_radius": "LOW", "category": "fs-delete", "notes": "PowerShell delete of a single item; prompts on tty" }, - { "pattern": "Remove-Item ", "blast_radius": "LOW", "category": "fs-delete", "notes": "positional form of PowerShell delete" }, - { "pattern": "Clear-Content ", "blast_radius": "MEDIUM", "category": "fs-overwrite", "notes": "PowerShell truncate; empties file contents in place" }, - { "pattern": "Clear-Content -Path ", "blast_radius": "MEDIUM", "category": "fs-overwrite", "notes": "PowerShell truncate via -Path" } + { "pattern": "Remove-Item ...-Recurse", "blast_radius": "HIGH", "category": "fs-delete", "notes": "wildcard spans quoted paths with spaces (`C:\\Program Files\\…`) that `` (=\\S+) could not" }, + { "pattern": "Remove-Item ...-Force", "blast_radius": "MEDIUM", "category": "fs-delete", "notes": "when -Recurse is also present, the HIGH entry above wins on severity" }, + { "pattern": "Remove-Item ", "blast_radius": "LOW", "category": "fs-delete", "notes": "prompts on tty" }, + { "pattern": "Clear-Content ...", "blast_radius": "MEDIUM", "category": "fs-overwrite", "notes": "empties file contents in place, positional or -Path form" } ], "git": [ { "pattern": "git reset --hard", "blast_radius": "HIGH", "category": "git-discard", "notes": "discards uncommitted work; unrecoverable" }, @@ -111,15 +99,8 @@ { "pattern": "docker system prune", "blast_radius": "MEDIUM", "category": "dk-multi-del", "notes": "containers + images + networks, NO volumes" }, { "pattern": "docker system prune -a", "blast_radius": "MEDIUM-HIGH", "category": "dk-multi-del", "notes": "adds all unused images" }, { "pattern": "docker system prune --all", "blast_radius": "MEDIUM-HIGH", "category": "dk-multi-del", "notes": "long form of -a" }, - { "pattern": "docker system prune --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "adds named-volume deletion" }, - { "pattern": "docker system prune -f --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: force between prune and --volumes" }, - { "pattern": "docker system prune -a --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: -a between prune and --volumes" }, - { "pattern": "docker system prune --volumes -f", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: --volumes precedes -f" }, - { "pattern": "docker system prune --volumes -a", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant: --volumes precedes -a" }, - { "pattern": "docker system prune --volumes -af", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant of -af --volumes" }, - { "pattern": "docker system prune --volumes -fa", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant of -af --volumes" }, - { "pattern": "docker system prune -af --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "the YOLO combination" }, - { "pattern": "docker system prune -fa --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "flag-order variant of -af --volumes" }, + { "pattern": "docker system prune --volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "adds named-volume deletion; --volumes trailed by other flags inherits via the compiled `\\b.*` suffix" }, + { "pattern": "docker system prune ...--volumes", "blast_radius": "HIGH", "category": "dk-multi-del", "notes": "covers -f/-fa (or future flags) between `prune` and `--volumes`" }, { "pattern": "docker buildx prune", "blast_radius": "MEDIUM", "category": "dk-build-cache", "notes": "rebuild cost only" }, { "pattern": "docker buildx prune --all", "blast_radius": "MEDIUM-HIGH", "category": "dk-build-cache", "notes": "every cached layer" }, { "pattern": "docker buildx prune -a", "blast_radius": "MEDIUM-HIGH", "category": "dk-build-cache", "notes": "short form of --all" }, @@ -130,9 +111,8 @@ { "pattern": "docker compose down -v", "blast_radius": "HIGH", "category": "dk-compose", "notes": "drops named volumes" }, { "pattern": "docker compose down --volumes", "blast_radius": "HIGH", "category": "dk-compose", "notes": "long form of -v" }, { "pattern": "docker compose down --volumes --remove-orphans", "blast_radius": "HIGH", "category": "dk-compose", "notes": "aggressive cleanup" }, - { "pattern": "docker compose down --remove-orphans -v", "blast_radius": "HIGH", "category": "dk-compose", "notes": "flag-order variant: -v after --remove-orphans still drops named volumes" }, - { "pattern": "docker compose down --remove-orphans --volumes", "blast_radius": "HIGH", "category": "dk-compose", "notes": "flag-order variant of --volumes --remove-orphans" }, - { "pattern": "docker compose down -v --remove-orphans", "blast_radius": "HIGH", "category": "dk-compose", "notes": "short-flag ordering with --remove-orphans" }, + { "pattern": "docker compose down ...-v", "blast_radius": "HIGH", "category": "dk-compose", "notes": "covers --remove-orphans (or future flags) between `down` and `-v`" }, + { "pattern": "docker compose down ...--volumes", "blast_radius": "HIGH", "category": "dk-compose", "notes": "long-flag variant" }, { "pattern": "docker compose rm -v ", "blast_radius": "MEDIUM-HIGH", "category": "dk-compose", "notes": "volume removal per service" }, { "pattern": "docker compose rm -f", "blast_radius": "MEDIUM", "category": "dk-compose", "notes": "bypasses removal prompt" }, { "pattern": "docker context rm ", "blast_radius": "LOW", "category": "dk-context", "notes": "reversible if endpoint known" }, @@ -145,20 +125,32 @@ { "pattern": "docker run --rm -v /:/host ", "blast_radius": "HIGH", "category": "dk-bind-mount", "notes": "mounts host root — almost always wrong" } ], "sql": [ - { "pattern": "drop database ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "destroys entire database and all data within; matches across mysql/psql/sqlite/mariadb clients and their docker-exec invocations" }, - { "pattern": "drop database if exists ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "idempotent form still destroys the database if present" }, - { "pattern": "drop schema ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "destroys schema and its objects" }, - { "pattern": "drop schema if exists ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "idempotent form" }, - { "pattern": "drop table ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "destroys table schema and rows" }, - { "pattern": "drop table if exists ", "blast_radius": "HIGH", "category": "sql-drop", "notes": "idempotent form" }, - { "pattern": "truncate table ", "blast_radius": "MEDIUM-HIGH", "category": "sql-truncate", "notes": "removes all rows; typically non-transactional" }, - { "pattern": "delete from
", "blast_radius": "MEDIUM", "category": "sql-delete", "notes": "unqualified DELETE would remove every row; presence of WHERE cannot be checked from the command shape" } + { "pattern": "mysql ...drop database", "blast_radius": "HIGH", "category": "sql-drop", "notes": "anchored on the client so `grep 'drop database' -r .` stays safe; docker/kubectl exec wrappers still match via the leading `.*\\b` anchor" }, + { "pattern": "mysql ...drop schema", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "mysql ...drop table", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "mysql ...truncate table", "blast_radius": "MEDIUM-HIGH", "category": "sql-truncate" }, + { "pattern": "mysql ...delete from", "blast_radius": "MEDIUM", "category": "sql-delete" }, + { "pattern": "mariadb ...drop database", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "mariadb ...drop schema", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "mariadb ...drop table", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "mariadb ...truncate table", "blast_radius": "MEDIUM-HIGH", "category": "sql-truncate" }, + { "pattern": "mariadb ...delete from", "blast_radius": "MEDIUM", "category": "sql-delete" }, + { "pattern": "psql ...drop database", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "psql ...drop schema", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "psql ...drop table", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "psql ...truncate table", "blast_radius": "MEDIUM-HIGH", "category": "sql-truncate" }, + { "pattern": "psql ...delete from", "blast_radius": "MEDIUM", "category": "sql-delete" }, + { "pattern": "sqlite3 ...drop database", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "sqlite3 ...drop schema", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "sqlite3 ...drop table", "blast_radius": "HIGH", "category": "sql-drop" }, + { "pattern": "sqlite3 ...truncate table", "blast_radius": "MEDIUM-HIGH", "category": "sql-truncate" }, + { "pattern": "sqlite3 ...delete from", "blast_radius": "MEDIUM", "category": "sql-delete" } ], "app-cli": [ { "pattern": "n8n db:reset", "blast_radius": "HIGH", "category": "app-reset", "notes": "wipes the n8n instance database" }, { "pattern": "n8n user-management:reset", "blast_radius": "MEDIUM", "category": "app-reset", "notes": "resets the n8n owner account and API keys" }, - { "pattern": "bench new-site --force ", "blast_radius": "HIGH", "category": "app-reset", "notes": "Frappe/ERPNext: --force overwrites an existing site's database in place" }, - { "pattern": "bench drop-site ", "blast_radius": "HIGH", "category": "app-reset", "notes": "Frappe/ERPNext: drops the site's database and archive" }, + { "pattern": "bench new-site ...--force", "blast_radius": "HIGH", "category": "app-reset", "notes": "Frappe/ERPNext: --force overwrites an existing site's database; wildcard matches either flag ordering" }, + { "pattern": "bench drop-site", "blast_radius": "HIGH", "category": "app-reset", "notes": "Frappe/ERPNext: drops the site's database and archive" }, { "pattern": "rails db:drop", "blast_radius": "HIGH", "category": "app-reset", "notes": "Rails: drops the database for the current env" }, { "pattern": "rails db:reset", "blast_radius": "HIGH", "category": "app-reset", "notes": "Rails: drop + create + migrate + seed" }, { "pattern": "rails db:schema:load", "blast_radius": "HIGH", "category": "app-reset", "notes": "Rails: replaces the schema, wiping rows" }, @@ -167,8 +159,8 @@ { "pattern": "sequelize db:drop", "blast_radius": "HIGH", "category": "app-reset", "notes": "Sequelize CLI: drops the database" } ], "key-value-store": [ - { "pattern": "redis-cli flushall", "blast_radius": "HIGH", "category": "kv-flush", "notes": "removes every key from every Redis database" }, - { "pattern": "redis-cli flushdb", "blast_radius": "HIGH", "category": "kv-flush", "notes": "removes every key from the current Redis database" } + { "pattern": "redis-cli ...flushall", "blast_radius": "HIGH", "category": "kv-flush", "notes": "wildcard spans connection flags (-h/-p/-n); removes every key from every Redis database" }, + { "pattern": "redis-cli ...flushdb", "blast_radius": "HIGH", "category": "kv-flush", "notes": "as above, current database only" } ] }, "safe": { diff --git a/pkg/safety/safety_test.go b/pkg/safety/safety_test.go index 86a5c7fc6f..3be3c1c128 100644 --- a/pkg/safety/safety_test.go +++ b/pkg/safety/safety_test.go @@ -45,6 +45,10 @@ func TestClassifyCommand_DestructivePatterns(t *testing.T) { {`Remove-Item -Path ".\config\cache\*" -Force -Recurse`, "high"}, {`Remove-Item "C:\tmp\foo" -Recurse -Force`, "high"}, {`Remove-Item -Recurse -Force C:\tmp\foo`, "high"}, + {`Remove-Item "C:\Program Files\MyApp\cache" -Recurse -Force`, "high"}, + {`Remove-Item -Path "C:\Users\Foo\OneDrive - Corp\cache" -Recurse -Force`, "high"}, + {`Remove-Item -Force -Recurse "C:\Program Files\MyApp"`, "high"}, + {`Remove-Item "C:\tmp\foo" -Recurse`, "high"}, {`Remove-Item -Path ".\file.txt" -Force`, "medium"}, {`Remove-Item .\file.txt -Force`, "medium"}, {`Remove-Item -Path ".\file.txt"`, "low"}, @@ -59,6 +63,10 @@ func TestClassifyCommand_DestructivePatterns(t *testing.T) { {"docker exec frappe bench new-site --force site1.local", "high"}, {"docker exec redis redis-cli FLUSHALL", "high"}, {"docker exec redis redis-cli FLUSHDB", "high"}, + {"redis-cli -h prod.example.com -p 6379 FLUSHALL", "high"}, + {"redis-cli -n 2 flushdb", "high"}, + {"bench new-site site1.local --force", "high"}, + {"bench new-site --force site1.local", "high"}, {"docker exec app rails db:reset", "high"}, {"docker exec app python manage.py flush --noinput", "high"}, {"docker exec app npx prisma migrate reset --force", "high"}, @@ -152,6 +160,27 @@ func TestClassifyCommand_DenyFlagsAreNeverSafe(t *testing.T) { assert.Equal(t, ClassSafe, ClassifyCommand("git log --oneline -5").Class) } +// SQL patterns are client-anchored so text searches for SQL verbs stay +// safe; drift here silently flips ALLOW→DENY under `restricted`. +func TestClassifyCommand_SqlPatternsDoNotShadowTextSearch(t *testing.T) { + for _, command := range []string{ + `grep "drop table users" -r .`, + `grep -rn "delete from cart" .`, + `rg "DROP DATABASE"`, + `rg "DELETE FROM users"`, + `git log --grep "delete from cart"`, + } { + t.Run(command, func(t *testing.T) { + assert.Equal(t, ClassSafe, ClassifyCommand(command).Class) + }) + } + + // Accepted trade: destructive wins when the client name itself + // appears in the search argument. + label := ClassifyCommand(`grep "mysql -e 'drop database foo'" file`) + assert.Equal(t, ClassDestructive, label.Class) +} + // The truncate-redirect pattern `> ` must not fire when `>` sits // between two word characters — it isn't a shell redirect there, just a // placeholder or comparison operator. Anchoring on `\b` alone was too From b876c1982951225e3e0246d37173b16d336b90d3 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 16:04:57 +0200 Subject: [PATCH 6/6] docs(safety): note fd-redirect trade in shell-metachar anchor comment --- pkg/safety/classify.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/safety/classify.go b/pkg/safety/classify.go index a216867c23..d2d3ecbf53 100644 --- a/pkg/safety/classify.go +++ b/pkg/safety/classify.go @@ -318,9 +318,12 @@ func stringSlice(value any) []string { // A pattern that begins with a shell metacharacter (`>`, `<`, `|`, // `&`, `;`) is anchored on whitespace rather than a word boundary: // `>` between two word characters isn't a shell redirect, so a -// redacted placeholder like `` or an inline comparison like -// `"1 > 0"` must not match the `> ` truncate pattern. The -// `\b` prefix would have fired on either. +// placeholder like `` or an inline `"a > b"` inside a quoted +// argument must not match the `> ` truncate pattern. Trade-off: +// fd-number redirects (`1> file`, `2> file`) no longer match either, +// which drops the `2> /dev/null` false positive as a bonus. Gating +// treats destructive and unknown identically, so this only affects +// the label attached to the confirmation dialog. func patternToRegexp(pattern string) string { var b strings.Builder b.WriteString(`(?i)`)