fix(danger): close H1-H3 classifier gaps (git config, find/rsync deletes, shell rc targets)

Author: jkyberneees

AGENTS.md

@@ -181,6 +181,9 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **Web UI prompt/model validation** (`cmd/odek/serve.go`) — server-side cap on WebSocket prompt size (1 MiB) and validation of model ID length/characters, preventing oversized or unusual payloads from reaching the LLM.
 - **Sub-agent runaway limits** (`cmd/odek/subagent.go`) — `--timeout` is capped at 3600s and `--max-iter` at 100, so a single `odek subagent` invocation cannot run indefinitely.
 - **Secrets.env permission gate** (`internal/config/loader.go`) — refuses to load `~/.odek/secrets.env` when it is group/world-readable, preventing local users from reading API keys injected into the environment.
+- **git config code execution** (`internal/danger/classifier.go`) — `git -c alias.*=!<cmd>`, `git -c core.pager=...`, `git -c core.fsmonitor=...`, `git -c credential.helper=...`, and the `git config` subcommand are classified as `code_execution` because they can inject arbitrary shell commands.
+- **find / rsync destructive flags** (`internal/danger/classifier.go`) — `find -delete` and `rsync --delete` / `--del` / `--remove-source-files` are classified as `destructive`; `find -fprint` / `-fprintf` are classified as `local_write`.
+- **Shell operand path classification** (`internal/danger/classifier.go`) — file operands and redirect targets of shell commands are checked with `ClassifyPath`, so writes to `~/.bashrc`, `~/.zshrc`, `~/.profile`, `~/.ssh`, `~/.odek`, etc. are escalated to `system_write` instead of auto-allowed `local_write`.
 - **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
 
 ### Security findings (`sec_findings.md`)

docs/SECURITY.md

@@ -95,6 +95,9 @@ The classifier is hardened against common evasion tricks (see the package doc in
 - `wipefs`, `blkdiscard`, `sgdisk`/`gdisk`/`cfdisk`/`sfdisk`, `mkswap`, `badblocks`, `cryptsetup`, and the `mkfs.*` family are `destructive`; `shred` is target-aware (local file → `local_write`, raw device / wipe target → `destructive`).
 - `shutdown`, `reboot`, `halt`, `poweroff`, `init 0`/`init 6` — machine power-control commands are `destructive` (deny-by-default) with an accurate label instead of falling through to `unknown`.
 - `env` and `printenv` — a full process-environment dump is classified as `system_write` because it can leak secrets that the redaction scanner does not recognise. `env FOO=bar <cmd>` still classifies the real `<cmd>` normally.
+- `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`.
 
 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.
 

internal/danger/classifier.go

@@ -1572,6 +1572,86 @@ func isSensitiveOdekPath(tok string) bool {
 	return isOdekTrustAnchor(home, abs)
 }
 
+// classifyShellTokenPath expands ~ and common environment-variable shorthands
+// in a shell token, strips prefixes like "of=", and returns the ClassifyPath
+// result for the filesystem path it names. It lets shell commands be checked
+// against the same home-sensitive-dir and rc-file lists used by the file tools,
+// closing the gap where `echo x >> ~/.bashrc` was auto-allowed as local_write.
+func classifyShellTokenPath(tok string) RiskClass {
+	path := tok
+
+	// Strip common key=value prefixes used by dd and similar tools.
+	for _, prefix := range []string{"of=", "if="} {
+		if strings.HasPrefix(strings.ToLower(path), prefix) {
+			path = path[len(prefix):]
+			break
+		}
+	}
+	if path == "" {
+		return Safe
+	}
+
+	// Expand ~ and simple $HOME/${HOME} forms that appear in shell commands.
+	home, _ := os.UserHomeDir()
+	if home != "" {
+		if strings.HasPrefix(path, "~") {
+			path = home + path[1:]
+		} else if path == "$HOME" || strings.HasPrefix(path, "$HOME/") {
+			path = home + path[len("$HOME"):]
+		} else if path == "${HOME}" || strings.HasPrefix(path, "${HOME}/") {
+			path = home + path[len("${HOME}"):]
+		}
+	}
+
+	return ClassifyPath(path)
+}
+
+// shellPathIsSensitive reports whether a shell token names a path that should
+// be treated as system_write or worse. It is used for both write operands and
+// general path arguments so reads of rc files and trust anchors are gated.
+func shellPathIsSensitive(tok string) bool {
+	cls := classifyShellTokenPath(tok)
+	return Rank(cls) >= Rank(SystemWrite)
+}
+
+// shellPathIsHomeSensitive reports whether a shell token names a path under
+// the user's home directory that ClassifyPath considers system_write or worse
+// (e.g. ~/.bashrc, ~/.ssh/id_rsa, ~/.odek/config.json). It is narrower than
+// shellPathIsSensitive so that touching /etc itself does not break the existing
+// classification of commands like `cd /etc`.
+func shellPathIsHomeSensitive(tok string) bool {
+	home, err := os.UserHomeDir()
+	if err != nil || home == "" {
+		return false
+	}
+	path := tok
+	for _, prefix := range []string{"of=", "if="} {
+		if strings.HasPrefix(strings.ToLower(path), prefix) {
+			path = path[len(prefix):]
+			break
+		}
+	}
+	if path == "" {
+		return false
+	}
+	if strings.HasPrefix(path, "~") {
+		path = home + path[1:]
+	} else if path == "$HOME" || strings.HasPrefix(path, "$HOME/") {
+		path = home + path[len("$HOME"):]
+	} else if path == "${HOME}" || strings.HasPrefix(path, "${HOME}/") {
+		path = home + path[len("${HOME}"):]
+	}
+	abs, err := filepath.Abs(path)
+	if err != nil {
+		return false
+	}
+	abs = filepath.Clean(abs)
+	if abs != home && !strings.HasPrefix(abs, home+"/") {
+		return false
+	}
+	return Rank(ClassifyPath(abs)) >= Rank(SystemWrite)
+}
+
 // ── Small token helpers ────────────────────────────────────────────────
 
 // commandName returns the program name from a token, taking the basename of
@@ -1629,6 +1709,21 @@ func hasAny(tokens []string, names ...string) bool {
 	return false
 }
 
+// rsyncDeleteFlags are flags that cause rsync to delete files on the
+// destination or remove them from the source — unrecoverable bulk deletion.
+func hasAnyRsyncDelete(tokens []string) bool {
+	for _, t := range tokens {
+		if strings.HasPrefix(t, "--delete") {
+			return true
+		}
+		switch t {
+		case "--del", "--remove-source-files":
+			return true
+		}
+	}
+	return false
+}
+
 // isAssignment reports whether a token is a NAME=VALUE shell assignment
 // (used to skip `env FOO=bar … cmd`). A leading-slash token like
 // /a=b is a path, not an assignment.
@@ -1854,6 +1949,16 @@ func isDestructive(first string, tokens []string) bool {
 		return false
 	}
 
+	// find -delete removes every matched file; rsync --delete / --remove-source-files
+	// can wipe a directory tree. Both are unrecoverable bulk deletion, equivalent to
+	// rm -rf, so they classify as destructive.
+	if first == "find" && hasAny(tokens, "-delete") {
+		return true
+	}
+	if first == "rsync" && hasAnyRsyncDelete(tokens) {
+		return true
+	}
+
 	if !destructivePrefixes[first] || len(tokens) < 2 {
 		return false
 	}
@@ -1905,17 +2010,17 @@ func isSystemWrite(first string, tokens []string) bool {
 	// write check. Escalate them here so they prompt instead.
 	if writePrefixes[first] || first == "ln" || first == "install" {
 		for _, tok := range tokens[1:] {
-			if isSystemPath(tok) {
+			if shellPathIsSensitive(tok) {
 				return true
 			}
 		}
 	}
-	// Check redirect targets for system paths
+	// Check redirect targets for sensitive paths
 	for _, tok := range tokens {
 		if tok == ">" || tok == ">>" {
 			continue
 		}
-		if isSystemPath(tok) {
+		if shellPathIsSensitive(tok) {
 			// Check if it's a redirect target (token follows > or >>)
 			for i, t := range tokens {
 				if (t == ">" || t == ">>") && i+1 < len(tokens) && tokens[i+1] == tok {
@@ -1924,6 +2029,15 @@ func isSystemWrite(first string, tokens []string) bool {
 			}
 		}
 	}
+	// dd of=... writes its output to an arbitrary path; catch writes to
+	// rc files and trust anchors that would otherwise slip through as safe.
+	if first == "dd" {
+		for _, tok := range tokens {
+			if strings.HasPrefix(strings.ToLower(tok), "of=") && shellPathIsSensitive(tok) {
+				return true
+			}
+		}
+	}
 	return false
 }
 
@@ -1994,6 +2108,10 @@ func isLocalWrite(first string, tokens []string) bool {
 	if writePrefixes[first] {
 		return true
 	}
+	// find -fprint/-fprintf write match lists to a file (arbitrary path)
+	if first == "find" && hasAny(tokens, "-fprint", "-fprintf") {
+		return true
+	}
 	// Any command with > or >> is a write
 	for _, tok := range tokens {
 		if tok == ">" || tok == ">>" {
@@ -2067,7 +2185,72 @@ func isNetworkEgress(first string, tokens []string) bool {
 	return true
 }
 
+// gitCodeExecConfigKeys are git config keys whose values cause arbitrary
+// command execution when set via -c/--config-env or git config.
+var gitCodeExecConfigKeys = map[string]bool{
+	"core.pager":       true,
+	"core.fsmonitor":   true,
+	"credential.helper": true,
+}
+
+// isGitCodeExecution reports whether a git invocation carries a config override
+// or subcommand that can execute arbitrary shell code. It deliberately errs on
+// the side of classification rather than parsing the full git config grammar:
+// aliases whose value starts with "!" run shell commands, and a small set of
+// core/credential keys spawn pagers or helpers.
+func isGitCodeExecution(tokens []string) bool {
+	for i := 0; i < len(tokens); i++ {
+		tok := tokens[i]
+		if tok == "git" {
+			continue
+		}
+
+		var val string
+		consumed := false
+		switch {
+		case tok == "-c" || tok == "--config-env":
+			if i+1 < len(tokens) {
+				val = tokens[i+1]
+				consumed = true
+			}
+		case strings.HasPrefix(tok, "-c"):
+			val = tok[2:]
+		case strings.HasPrefix(tok, "--config-env="):
+			val = tok[len("--config-env="):]
+		case !strings.HasPrefix(tok, "-"):
+			// Once we hit the subcommand, global options are done.
+			if tok == "config" {
+				return true
+			}
+		}
+
+		if consumed {
+			i++
+		}
+		if val == "" {
+			continue
+		}
+
+		key, value, _ := strings.Cut(val, "=")
+		key = strings.ToLower(key)
+		if strings.HasPrefix(key, "alias.") && strings.HasPrefix(value, "!") {
+			return true
+		}
+		if gitCodeExecConfigKeys[key] {
+			return true
+		}
+	}
+	return false
+}
+
 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
+	// can persist the same payloads.
+	if first == "git" && isGitCodeExecution(tokens) {
+		return true
+	}
+
 	// Pipe to shell interpreter
 	for i, tok := range tokens {
 		if tok == "|" && i+1 < len(tokens) && pipedShells[commandName(tokens[i+1])] {
@@ -2317,16 +2500,18 @@ func hasArgAfter(tokens []string, after, target string) bool {
 	return false
 }
 
-// touchesSystemPath reports whether any token names a system path (an
+// touchesSystemPath reports whether any token names a sensitive path (an
 // argument or a redirect target alike). It is intentionally broader than the
 // redirect-only scan in isSystemWrite — it catches reads/args such as
-// `cat /etc/foo` or an unknown tool pointed at /usr — so both checks exist.
+// `cat /etc/foo`, `cat ~/.bashrc`, or an unknown tool pointed at /usr — so both
+// checks exist. It keeps the legacy trailing-slash system-prefix check and
+// additionally uses ClassifyPath for home rc files and trust anchors.
 func touchesSystemPath(tokens []string) bool {
 	for _, tok := range tokens {
 		if tok == ">" || tok == ">>" {
 			continue
 		}
-		if isSystemPath(tok) {
+		if isSystemPath(tok) || shellPathIsHomeSensitive(tok) {
 			return true
 		}
 	}

internal/danger/classifier_test.go

@@ -715,6 +715,97 @@ func TestClassify_RsyncRemote(t *testing.T) {
 	}
 }
 
+// TestClassify_GitConfigCodeExecution verifies H-1: git -c/--config-env and
+// git config can inject arbitrary shell commands through aliases, pager, and
+// credential helpers, so they must classify as code_execution.
+func TestClassify_GitConfigCodeExecution(t *testing.T) {
+	tests := []struct {
+		cmd  string
+		want RiskClass
+	}{
+		{`git -c alias.x='!id' x`, CodeExecution},
+		{`git -c alias.x=!id x`, CodeExecution},
+		{`git -calias.x=!id x`, CodeExecution},
+		{`git -c core.pager='sh -c id' --paginate log`, CodeExecution},
+		{`git -c core.fsmonitor='!pwn' status`, CodeExecution},
+		{`git -c credential.helper='!curl evil' clone`, CodeExecution},
+		{`git config --global alias.pwn '!cmd'`, CodeExecution},
+		{`git config user.email x`, CodeExecution},
+		// Benign config overrides stay in their normal class.
+		{`git -c http.proxy=http://evil fetch origin`, NetworkEgress},
+		{`git -C /repo status`, Safe},
+		{`git status`, Safe},
+	}
+	for _, tt := range tests {
+		t.Run(tt.cmd, func(t *testing.T) {
+			got := Classify(tt.cmd)
+			if got != tt.want {
+				t.Errorf("Classify(%q) = %s, want %s", tt.cmd, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestClassify_FindRsyncDestructive verifies H-2: find -delete and rsync
+// --delete / --remove-source-files can wipe directory trees, so they are
+// destructive; find -fprint/-fprintf are local writes.
+func TestClassify_FindRsyncDestructive(t *testing.T) {
+	tests := []struct {
+		cmd  string
+		want RiskClass
+	}{
+		{"find . -delete", Destructive},
+		{"find ~ -delete", Destructive},
+		{"find . -fprint ~/.bashrc", LocalWrite},
+		{"find . -fprintf /tmp/list '%p\\n'", LocalWrite},
+		{"rsync -a --delete /tmp/empty/ ~", Destructive},
+		{"rsync -av --remove-source-files /a /b", Destructive},
+		{"rsync -av --del /a /b", Destructive},
+		{"rsync -av /src/ /dst/", Safe},
+		{"rsync -av /src/ user@host:/dst/", NetworkEgress},
+	}
+	for _, tt := range tests {
+		t.Run(tt.cmd, func(t *testing.T) {
+			got := Classify(tt.cmd)
+			if got != tt.want {
+				t.Errorf("Classify(%q) = %s, want %s", tt.cmd, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestClassify_ShellRCTargets verifies H-3: shell writes to shell rc files
+// and other home-sensitive paths are escalated to system_write using the same
+// ClassifyPath lists that gate the file tools.
+func TestClassify_ShellRCTargets(t *testing.T) {
+	tests := []struct {
+		cmd  string
+		want RiskClass
+	}{
+		{"echo x >> ~/.bashrc", SystemWrite},
+		{"echo x >> ~/.zshrc", SystemWrite},
+		{"echo x >> ~/.profile", SystemWrite},
+		{"echo x >> $HOME/.bashrc", SystemWrite},
+		{"cp evil ~/.profile", SystemWrite},
+		{"tee -a ~/.zshrc", SystemWrite},
+		{"dd if=evil of=~/.bashrc", SystemWrite},
+		{"cat ~/.bashrc", SystemWrite},
+		{"cat ~/.ssh/id_rsa", SystemWrite},
+		{"cat ~/.odek/config.json", SystemWrite},
+		// Non-rc home paths stay local_write.
+		{"mv evil ~/.local/bin/git", LocalWrite},
+		{"echo x > ~/notes.txt", LocalWrite},
+	}
+	for _, tt := range tests {
+		t.Run(tt.cmd, func(t *testing.T) {
+			got := Classify(tt.cmd)
+			if got != tt.want {
+				t.Errorf("Classify(%q) = %s, want %s", tt.cmd, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestClassify_PythonDashC(t *testing.T) {
 	got := Classify("python -c 'print(1)'")
 	if got != CodeExecution {