Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
3 changes: 3 additions & 0 deletions docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
197 changes: 191 additions & 6 deletions internal/danger/classifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down Expand Up @@ -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 == ">>" {
Expand Down Expand Up @@ -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])] {
Expand Down Expand Up @@ -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
}
}
Expand Down
91 changes: 91 additions & 0 deletions internal/danger/classifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading