From 1afb04d54a81342e87d9b3adb1f478dbd2c86ddc Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sun, 26 Jul 2026 10:12:15 +0200 Subject: [PATCH] fix(danger): close pipe-fed xargs bypass, gate root-level and git data-loss verbs Fixes three classifier findings: 1. Destructive-command bypass via pipe-fed xargs (HIGH): echo "/" | xargs rm -rf classified as local_write (auto-allow) because piped stdin was never modelled as xargs arguments. When a pipeline's sink is xargs invoking a destructive/system verb, statically determinable upstream payloads (echo/printf args) are now composed onto the inner command before classification; when the payload is not statically determinable and the inner verb is dangerous-capable, the pipeline fails closed (unknown -> deny). 2. Recursive chmod at system roots not treated as system_write (MEDIUM): ClassifyPath("/") fell through to local_write, so chmod -R 777 / and mv / /tmp/x ran without approval while chown -R nobody / prompted. The filesystem root now classifies as system_write, and chattr is handled with the same operand scan as chmod. 3. Irreversible git data-loss verbs classified safe (MEDIUM): git clean -f*, reset --hard, checkout -f/--/, restore (worktree), branch -D / --delete --force, stash drop|clear, and reflog expire now classify as system_write (prompt-by-default) instead of safe. Dry-run and non-destructive forms (clean -n, checkout , branch -d, restore --staged, stash pop) stay safe. Regression tests pin the new classifications and the default-policy actions; docs updated (SECURITY.md, CLI.md, AGENTS.md). --- AGENTS.md | 3 + docs/CLI.md | 4 +- docs/SECURITY.md | 3 + internal/danger/classifier.go | 322 +++++++++++++++++++++++++++++- internal/danger/hardening_test.go | 141 +++++++++++++ 5 files changed, 470 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 59436b3..1b90004 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,6 +116,9 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **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//environ`. - **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`, `cmd/odek/shell.go`, `cmd/odek/perf_tools.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. CLI `shell`/`parallel_shell` reuse a single TTYApprover instance per process so the counter and trust cache persist across prompts. - **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`. +- **Pipe-fed xargs composition** (`internal/danger/classifier.go`) — when a pipeline's sink is `xargs` invoking a destructive/system verb, statically determinable upstream payloads (`echo`/`printf` args) are composed onto the inner command before classification (`echo "/" | xargs rm -rf` → `destructive`, like `rm -rf /`); when the payload is not statically determinable (`cat file | …`, `find … | …`, `$VARS`) and the inner verb is dangerous-capable (`rm`, `shred`, `dd`, `chmod`, `chown`, `mkfs.*`, …), the pipeline fails closed as `unknown` (deny-by-default). +- **Root-level mutation targets** (`internal/danger/classifier.go`) — `ClassifyPath("/")` is `system_write`, so `chmod -R 777 /`, `chattr -R +i /`, and `mv / /tmp/x` prompt instead of falling through to auto-allowed `local_write` (parity with `chown -R`). `chattr` is handled with the same operand scan as `chmod`. +- **Git data-loss verbs gated** (`internal/danger/classifier.go`) — `git clean -f*` (unless `-n`/`--dry-run`), `git reset --hard`, `git checkout -f`/`--`/`./…`, `git restore` (worktree), `git branch -D` / `--delete --force`, `git stash drop`/`clear`, and `git reflog expire` classify as `system_write` (prompt-by-default) instead of `safe`, so an injected payload cannot wipe a working tree with zero friction. - **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.` 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. - **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. - **SSRF guard proxy refusal** (`cmd/odek/ssrf_guard.go`) — when `HTTP(S)_PROXY` is set, the transport would dial the proxy instead of the target and the dial guard would only validate the proxy address. `ssrfGuardedTransport` detects an active proxy, logs a warning, and refuses the request so SSRF protection is not silently voided. diff --git a/docs/CLI.md b/docs/CLI.md index c24d4ad..286797a 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -161,12 +161,12 @@ When running without `--sandbox`, odek classifies every shell command by risk an |-------|---------|----------| | 🟢 safe | allow | `ls`, `cat`, `grep`, `go build` | | 🟡 local_write | allow | `rm file`, `mv`, `echo > file` | -| 🟠 system_write | **prompt** | `sudo`, `apt install`, writes to `/etc/` | +| 🟠 system_write | **prompt** | `sudo`, `apt install`, writes to `/etc/`, `chmod -R 777 /`, `git reset --hard`, `git clean -fdx` | | 🔴 destructive | **deny** | `rm -rf /`, `dd if=/dev/zero`, `mkfs` | | 🔴 network_egress | **prompt** | `curl`, `git push`, `ssh`, `scp` | | 🔴 code_execution | **prompt** | `curl url \| bash`, `eval`, `node -e`, `go run` | | 🟠 install | **prompt** | `npm install`, `pip install`, `go install ` | -| 🔴 unknown | **deny** | any command whose program name isn't recognised; MCP tools (`__`) | +| 🔴 unknown | **deny** | any command whose program name isn't recognised; MCP tools (`__`); pipe-fed `xargs ` whose stdin payload isn't statically determinable | | ⬛ blocked | **deny** | Fork bombs, `dd` to block devices | odek **fails closed**: a command or MCP tool whose name matches no known-safe or known-dangerous diff --git a/docs/SECURITY.md b/docs/SECURITY.md index a5a978c..0c79ff2 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -100,6 +100,9 @@ The classifier is hardened against common evasion tricks (see the package doc in - `git -c alias.x='!id' x`, `git -c core.pager='sh -c id' --paginate log`, `git config --global alias.pwn '!cmd'` — `git -c` / `--config-env` overrides and the `git config` subcommand are `code_execution` because they can define arbitrary shell commands. - `find . -delete`, `rsync -a --delete /empty/ ~`, `rsync --remove-source-files` — bulk-deletion flags are `destructive`; `find -fprint` / `-fprintf` are `local_write` because they write match lists to arbitrary files. - `echo x >> ~/.bashrc`, `cp evil ~/.profile`, `dd if=evil of=~/.bashrc` — shell file operands and redirect targets are run through `ClassifyPath`, so writes to shell rc files, `~/.ssh`, `~/.odek` trust anchors, and other home-sensitive paths are `system_write` instead of auto-allowed `local_write`. Matching is case-insensitive so variants such as `~/.BASHRC` or `~/.odek/CONFIG.JSON` are escalated on case-insensitive filesystems. +- `echo "/" | xargs rm -rf` — a pipeline whose sink is `xargs` composing a destructive/system verb has its upstream literal payload (`echo`/`printf` arguments) composed onto the inner command before classification, so pipe-fed `xargs rm -rf` classifies exactly like `rm -rf /` (`destructive` → deny). When the payload is not statically determinable (`cat file | …`, `find … | …`, `$VARS`) and the inner verb can turn a piped path into destructive/system damage (`rm`, `shred`, `dd`, `chmod`, `chown`, `mkfs.*`, …), the pipeline fails closed as `unknown` (deny-by-default). +- `chmod -R 777 /`, `chattr -R +i /`, `mv / /tmp/x` — the filesystem root itself classifies as `system_write` in `ClassifyPath`, so recursive permission/attribute flips or moves aimed at `/` prompt instead of falling through to auto-allowed `local_write`. `chattr` is now handled with the same operand scan as `chmod`. +- `git clean -fdx`, `git reset --hard`, `git checkout -- .`, `git restore .`, `git branch -D`, `git stash drop`/`clear`, `git reflog expire` — irreversible git data-loss verbs are `system_write` (prompt-by-default) instead of `safe`, so a prompt-injection payload cannot wipe a working tree with zero friction. Dry-run and non-destructive forms (`git clean -n`, `git checkout main`, `git branch -d`, `git stash pop`, `git restore --staged`) stay `safe`. Regression suites (`internal/danger/classifier_bypass_test.go` and `hardening_test.go`) pin these as known-closed evasions. If you find a new bypass, those test files are the place to add it. diff --git a/internal/danger/classifier.go b/internal/danger/classifier.go index 6dd4536..a5e08d3 100644 --- a/internal/danger/classifier.go +++ b/internal/danger/classifier.go @@ -38,7 +38,11 @@ // &&, ||), each segment into pipe stages (on |), and EVERY stage is // classified — not just the head — so `true | dd of=/dev/sda` and // `echo x | sudo rm -rf /home` are seen for what their later stages do. -// The worst class across all parts wins (see rank). +// A stage that pipes INTO xargs running a destructive/system verb has its +// upstream literal payload (echo/printf args) composed onto the inner +// command (`echo "/" | xargs rm -rf` classifies like `rm -rf /`); when +// the payload is not statically determinable the pipeline fails closed +// (unknown → deny). The worst class across all parts wins (see rank). // // 3. Wrapper unwrapping (unwrapWrappers). Leading execution wrappers // (env, xargs, nohup, nice, setsid, timeout, …) are stripped so the @@ -142,6 +146,7 @@ type ToolOperation struct { // // Classification rules (highest wins): // - /boot, /dev, /proc, /sys, /mnt, /media → destructive +// - / (the filesystem root itself) → system_write // - /tmp, $TMPDIR → local_write // - /etc, /root, /var, /run, /lib, /usr → system_write // - $HOME/.ssh, .config, .gnupg, .aws, .kube, .docker, .gitconfig, .env → system_write @@ -170,6 +175,13 @@ func ClassifyPath(path string) RiskClass { abs = strings.TrimPrefix(abs, "/private") } + // The filesystem root itself. A mutation aimed at / (chmod -R 777 /, + // mv / /tmp/x, dd of=/, …) is system-wide damage and must never fall + // through to local_write just because "/" carries no directory prefix. + if abs == string(filepath.Separator) { + return SystemWrite + } + for _, prefix := range []string{"/boot", "/dev", "/proc", "/sys", "/mnt", "/media"} { if strings.HasPrefix(abs, prefix) { return Destructive @@ -774,6 +786,11 @@ var writePrefixes = map[string]bool{ "echo": true, "sed": true, "tee": true, "rm": true, "mv": true, "cp": true, "touch": true, "mkdir": true, "rmdir": true, "chmod": true, "chown": true, + // chattr mutates file attributes (including the immutable flag) the same + // way chmod mutates permissions; recursive use at a system root is + // escalated by the same operand scan, and formerly it fell through to + // Unknown. + "chattr": true, // shred overwrites/removes files like rm. isDestructive escalates it to // destructive when aimed at a block device or catastrophic wipe target; // otherwise a local-file shred is a write (local_write / system_write). @@ -1003,10 +1020,161 @@ func classifyPipeline(tokens []string) RiskClass { for idx, stage := range stages { // idx > 0 means this stage receives piped input from the previous one. worst = worstOf(worst, classifyStage(stage, idx > 0)) + if idx > 0 { + // A pipe-fed xargs turns upstream stdout into command arguments, + // so `echo "/" | xargs rm -rf` executes `rm -rf /` even though no + // stage literally contains that command. Compose the payload. + worst = worstOf(worst, classifyXargsSink(stages[:idx], stage)) + } } return worst } +// classifyXargsSink handles a pipe stage whose command is reached through an +// xargs wrapper while receiving piped stdin. xargs appends each line of stdin +// to the inner command line, so the effective command is `inner …` +// even though the payload never appears as a token of the sink stage. +// +// When the upstream pipeline is a statically determinable literal producer +// (`echo ` / `printf `) the payload tokens are composed onto the +// inner command and the composition is classified — `echo "/" | xargs rm -rf` +// then classifies exactly like `rm -rf /` (destructive). When the payload is +// not statically determinable (`cat file | …`, `find … | …`, variables) and +// the inner verb can turn a piped path into destructive or system-level +// damage, the pipeline fails closed as Unknown (deny-by-default): the same +// treatment an unrecognised verb gets, because the command that will actually +// run is unknowable at classification time. +func classifyXargsSink(upstream [][]string, stage []string) RiskClass { + inner, ok := xargsInnerCommand(stage) + if !ok || len(inner) == 0 { + return Safe + } + if payload, static := staticPipePayload(upstream); static { + composed := make([]string, 0, len(inner)+len(payload)) + composed = append(composed, inner...) + composed = append(composed, payload...) + return classifyStage(composed, false) + } + if xargsDangerousVerb(commandName(inner[0])) { + return Unknown + } + return Safe +} + +// xargsInnerCommand returns the tokens of the command an xargs wrapper in the +// stage's leading wrapper chain will execute (everything after xargs and its +// options). ok reports whether such an xargs wrapper was found; a bare xargs +// (default command is echo) yields an empty inner slice with ok=true. +func xargsInnerCommand(tokens []string) (inner []string, ok bool) { + i := 0 + for i < len(tokens) && isAssignment(tokens[i]) { + i++ // leading VAR=value assignment prefix + } + for i < len(tokens) { + name := commandName(tokens[i]) + if name == "xargs" { + i++ + for i < len(tokens) { + t := tokens[i] + if !strings.HasPrefix(t, "-") || t == "-" { + return tokens[i:], true + } + // Option flags. Value-taking flags consume the next token so + // the value is not mistaken for the inner command. + if xargsValueFlags[t] && i+1 < len(tokens) { + i += 2 + continue + } + i++ + } + return nil, true + } + if !privilegedWrappers[name] && !execWrappers[name] { + return nil, false + } + i++ // consume the wrapper itself + for i < len(tokens) { + t := tokens[i] + switch { + case strings.HasPrefix(t, "-") && t != "-": + i++ // wrapper option flag + case name == "env" && isAssignment(t): + i++ // env VAR=VALUE + case (name == "timeout" || name == "nice" || name == "ionice") && isNumericish(t): + i++ // timeout 5s / nice 10 + default: + goto nextWrapper + } + } + nextWrapper: + } + return nil, false +} + +// xargsValueFlags are xargs options that take a separate value token +// (short and long forms). `--flag=value` spellings need no entry — they are +// a single token and are skipped like any other flag. +var xargsValueFlags = map[string]bool{ + "-I": true, "-L": true, "-n": true, "-P": true, "-s": true, + "-E": true, "-e": true, "-d": true, "-a": true, + "--replace": true, "--max-lines": true, "--max-args": true, + "--max-procs": true, "--max-chars": true, "--eof": true, + "--delimiter": true, "--arg-file": true, +} + +// staticPipePayload returns the literal tokens an upstream pipeline feeds +// into the sink's stdin when they are statically determinable: a single +// producer stage of `echo ` or `printf ` with no shell +// substitutions or variable expansions in its arguments. Anything else +// (file readers, find, command output, $VARS, multi-stage transforms) is +// not statically determinable and reports ok=false. +func staticPipePayload(upstream [][]string) (payload []string, ok bool) { + if len(upstream) != 1 { + return nil, false + } + stage := upstream[0] + if len(stage) == 0 { + return nil, false + } + var args []string + switch commandName(stage[0]) { + case "echo": + args = stage[1:] + for len(args) > 0 && (args[0] == "-n" || args[0] == "-e" || args[0] == "-E") { + args = args[1:] + } + case "printf": + args = stage[1:] + for len(args) > 0 && strings.HasPrefix(args[0], "-") { + args = args[1:] + } + default: + return nil, false + } + for _, a := range args { + // A token containing $ or a backtick expands at runtime, so the real + // payload is not statically determinable. + if strings.ContainsAny(a, "$`") { + return nil, false + } + } + return args, true +} + +// xargsDangerousVerb reports whether a verb invoked through pipe-fed xargs +// can turn an undeterminable piped path into destructive or system-level +// damage. For these the pipeline fails closed (Unknown) when the payload +// cannot be composed statically; benign verbs (grep, wc, …) are unaffected. +func xargsDangerousVerb(name string) bool { + switch name { + case "rm", "shred", "dd", + "chmod", "chown", "chgrp", "chattr", + "mv", "cp", "ln", "install", "tee": + return true + } + return destructivePrefixes[name] +} + // classifyStage classifies a single pipe stage. It first strips leading // execution wrappers (sudo/env/xargs/nohup/timeout/…) so the real command // underneath is the one classified, while privileged wrappers still set a @@ -1834,6 +2002,17 @@ func classifyCommand(tokens []string) RiskClass { return SystemWrite } + // Irreversible git data-loss verbs (clean -f, reset --hard, checkout --, + // restore, branch -D, stash drop/clear, reflog expire) destroy uncommitted + // work or delete history with no undo. They were previously classified + // Safe because git is a "network" command whose non-remote subcommands + // fell through every check — a prompt-injection payload could wipe a + // working tree with zero friction. They now require explicit approval + // (system_write → prompt by default), like other irreversible mutations. + if first == "git" && isGitDataLoss(tokens) { + return SystemWrite + } + // Code execution checks (pipe to shell, eval, -e/-c flags) if isCodeExecution(first, tokens) { return CodeExecution @@ -2317,6 +2496,147 @@ func isGitCodeExecution(tokens []string) bool { return false } +// gitSubcommandAndArgs returns the git subcommand and the tokens that follow +// it, skipping global options. Options that take a separate value token +// (-C, -c, --git-dir, …) consume that token so it is not mistaken for the +// subcommand. +func gitSubcommandAndArgs(tokens []string) (sub string, args []string) { + seenGit := false + skipNext := false + for i, tok := range tokens { + if !seenGit { + if commandName(tok) == "git" { + seenGit = true + } + continue + } + if skipNext { + skipNext = false + continue + } + if strings.HasPrefix(tok, "-") { + switch tok { + case "-C", "-c", "--git-dir", "--work-tree", "--namespace", + "--exec-path", "--super-prefix", "--config-env": + // These consume the following token as their value. + skipNext = true + } + continue + } + return tok, tokens[i+1:] + } + return "", nil +} + +// isGitDataLoss reports whether a git invocation irreversibly destroys +// uncommitted work, branches, stashes, or history: +// +// git clean -f… (unless -n/--dry-run is also given) +// git reset --hard +// git checkout -f | -- | +// git restore (worktree restore; --staged-only is safe) +// git branch -D | -d -f +// git stash drop | clear +// git reflog expire +// +// These are classified system_write (prompt-by-default), not destructive, +// because they are routine recovery verbs with legitimate uses — but they +// must never run silently, since a prompt-injection payload can wipe a +// working tree through them with zero friction. +func isGitDataLoss(tokens []string) bool { + sub, args := gitSubcommandAndArgs(tokens) + switch sub { + case "clean": + // Only -f/--force deletes; -n/--dry-run prints and wins if both given. + force, dryRun := false, false + for _, a := range args { + switch { + case a == "--force": + force = true + case a == "--dry-run": + dryRun = true + case isShortFlagToken(a): + if strings.ContainsRune(a[1:], 'f') { + force = true + } + if strings.ContainsRune(a[1:], 'n') { + dryRun = true + } + } + } + return force && !dryRun + case "reset": + return hasAny(args, "--hard") + case "checkout": + // -f/--force or a pathspec (-- , ".", "./…") discards local + // changes. A bare branch operand (git checkout main) switches, keeps + // the worktree, and is not data loss. + for _, a := range args { + switch { + case a == "--force" || (isShortFlagToken(a) && strings.ContainsRune(a[1:], 'f')): + return true + case a == "--": + return true + case a == "." || strings.HasPrefix(a, "./"): + return true + } + } + return false + case "restore": + // Default restores the worktree from the index, discarding local + // changes; --staged alone only unstages. --source/-s consumes a value. + staged := hasAny(args, "--staged", "-S") + for i := 0; i < len(args); i++ { + a := args[i] + if a == "--source" || a == "-s" { + i++ // skip the source value + continue + } + if a == "--worktree" || a == "-W" { + return true + } + if (a == "--" || !strings.HasPrefix(a, "-")) && !staged { + return true + } + } + return false + case "branch": + // -D, or -d/--delete combined with -f/--force, deletes a branch + // regardless of merge state. + del, force := false, false + for _, a := range args { + switch { + case a == "--delete": + del = true + case a == "--force": + force = true + case isShortFlagToken(a): + if strings.ContainsRune(a[1:], 'D') { + return true + } + if strings.ContainsRune(a[1:], 'd') { + del = true + } + if strings.ContainsRune(a[1:], 'f') { + force = true + } + } + } + return del && force + case "stash": + return hasAny(args, "drop", "clear") + case "reflog": + return hasAny(args, "expire") + } + return false +} + +// isShortFlagToken reports whether a token is a single-dash option cluster +// like -f, -fdx, -Df (as opposed to a --long flag or an operand). +func isShortFlagToken(tok string) bool { + return strings.HasPrefix(tok, "-") && !strings.HasPrefix(tok, "--") && len(tok) > 1 +} + func isCodeExecution(first string, tokens []string) bool { // git -c/--config-env can inject arbitrary shell commands via aliases, // core.pager, core.fsmonitor, credential.helper, etc.; git config writes diff --git a/internal/danger/hardening_test.go b/internal/danger/hardening_test.go index a66d21c..67a5968 100644 --- a/internal/danger/hardening_test.go +++ b/internal/danger/hardening_test.go @@ -305,3 +305,144 @@ func TestHardening_NoRegressionOnBenign(t *testing.T) { } } } + +// TestHardening_XargsPipedPayload pins the fix for the pipe-fed xargs +// bypass: `echo "/" | xargs rm -rf` executes `rm -rf /` but previously +// classified as local_write (auto-allow) because the payload never appears +// as a token of the xargs stage. The classifier now composes statically +// determinable upstream payloads onto the xargs command line and fails +// closed (unknown → deny) when the payload is not determinable and the +// inner verb can turn a piped path into destructive/system damage. +func TestHardening_XargsPipedPayload(t *testing.T) { + cases := []struct { + cmd string + cls RiskClass + }{ + // Statically determinable payloads compose onto the inner command. + {`echo "/" | xargs rm -rf`, Destructive}, + {`echo "/etc" | xargs rm -rf`, Destructive}, + {`echo "/home/user" | xargs rm -rf`, Destructive}, + {`echo "~" | xargs rm -rf`, Destructive}, + {`printf "/\n" | xargs rm -rf`, Destructive}, + {`echo "/dev/sda" | xargs shred`, Destructive}, + {`echo "/" | xargs -0 rm -rf`, Destructive}, + {`echo "/" | xargs -I P rm -rf P`, Destructive}, + {`echo "/" | timeout 5 xargs rm -rf`, Destructive}, + {`echo /etc/passwd | xargs chmod 777`, SystemWrite}, + // Payload not statically determinable + dangerous verb ⇒ fail closed. + {`cat /tmp/paths | xargs rm -rf`, Unknown}, + {`find / -name core | xargs rm`, Unknown}, + {`yes / | xargs rm -rf`, Unknown}, + {`echo "$P" | xargs rm -rf`, Unknown}, + {`echo / | head -1 | xargs rm -rf`, Unknown}, + {`cat files | xargs chmod 600`, Unknown}, + // Benign compositions keep their original classification. + {`echo "./tmpfile" | xargs rm`, LocalWrite}, + {`echo hi | xargs echo`, Safe}, + {`git ls-files | xargs grep foo`, Safe}, + {`find . -name '*.o' | xargs wc -l`, Safe}, + } + for _, tc := range cases { + if got := Classify(tc.cmd); got != tc.cls { + t.Errorf("Classify(%q) = %s, want %s", tc.cmd, got, tc.cls) + } + } + + // The report's core complaint: the pipeline executed a root-destructive + // command with no prompt and no deny under default policy. + cfg := &DangerousConfig{} + if got := cfg.ActionForCommand(`echo "/" | xargs rm -rf`); got != Deny { + t.Errorf("ActionForCommand(echo / | xargs rm -rf) = %s, want deny", got) + } + if got := cfg.ActionForCommand(`cat /tmp/paths | xargs rm -rf`); got != Deny { + t.Errorf("ActionForCommand(cat | xargs rm -rf) = %s, want deny (fail closed)", got) + } +} + +// TestHardening_RootLevelMutationTargets pins the fix for recursive +// permission/attribute flips and moves aimed at the filesystem root: +// ClassifyPath("/") used to fall through to local_write, so `chmod -R 777 /` +// and `mv / /tmp/x` ran without approval while `chown -R nobody /` prompted. +func TestHardening_RootLevelMutationTargets(t *testing.T) { + if got := ClassifyPath("/"); got != SystemWrite { + t.Errorf("ClassifyPath(/) = %s, want system_write", got) + } + cases := []struct { + cmd string + cls RiskClass + }{ + {"chmod -R 777 /", SystemWrite}, + {"chmod 777 /", SystemWrite}, + {"chmod -R 777 /etc", SystemWrite}, + {"chmod -R 777 /usr", SystemWrite}, + {"chattr -R +i /", SystemWrite}, + {"chown -R nobody /", SystemWrite}, + {"mv / /tmp/x", SystemWrite}, + {"mv ~/.ssh /tmp/stolen", SystemWrite}, + // Benign local use keeps its original classification. + {"chmod -R 755 ./dist", LocalWrite}, + {"chmod +x script.sh", LocalWrite}, + {"chattr +i ./file", LocalWrite}, + } + for _, tc := range cases { + if got := Classify(tc.cmd); got != tc.cls { + t.Errorf("Classify(%q) = %s, want %s", tc.cmd, got, tc.cls) + } + } +} + +// TestHardening_GitDataLossVerbs pins the fix for irreversible git verbs +// that were classified Safe (auto-allow): clean -f, reset --hard, +// checkout --, restore, branch -D, stash drop/clear, reflog expire. They +// now require approval (system_write → prompt by default); dry-run and +// non-destructive forms stay Safe. +func TestHardening_GitDataLossVerbs(t *testing.T) { + cases := []struct { + cmd string + cls RiskClass + }{ + {"git clean -fdx", SystemWrite}, + {"git clean -f", SystemWrite}, + {"git clean --force", SystemWrite}, + {"git reset --hard", SystemWrite}, + {"git reset --hard HEAD~3", SystemWrite}, + {"git -C /repo reset --hard", SystemWrite}, + {"git checkout -- .", SystemWrite}, + {"git checkout .", SystemWrite}, + {"git checkout -f main", SystemWrite}, + {"git restore .", SystemWrite}, + {"git restore --worktree src/", SystemWrite}, + {"git branch -D feature", SystemWrite}, + {"git branch --delete --force feature", SystemWrite}, + {"git branch -df feature", SystemWrite}, + {"git stash drop", SystemWrite}, + {"git stash clear", SystemWrite}, + {"git reflog expire --expire=now --all", SystemWrite}, + // Non-destructive forms stay Safe. + {"git clean -n", Safe}, + {"git clean -ndx", Safe}, + {"git clean -fdx -n", Safe}, + {"git reset", Safe}, + {"git reset --soft HEAD~1", Safe}, + {"git checkout main", Safe}, + {"git checkout -b feature", Safe}, + {"git restore --staged file", Safe}, + {"git branch -d feature", Safe}, + {"git stash", Safe}, + {"git stash pop", Safe}, + {"git reflog", Safe}, + {"git status", Safe}, + } + for _, tc := range cases { + if got := Classify(tc.cmd); got != tc.cls { + t.Errorf("Classify(%q) = %s, want %s", tc.cmd, got, tc.cls) + } + } + + // Data-loss verbs require a prompt under default policy instead of + // running silently. + cfg := &DangerousConfig{} + if got := cfg.ActionForCommand("git reset --hard"); got != Prompt { + t.Errorf("ActionForCommand(git reset --hard) = %s, want prompt", got) + } +}