diff --git a/AGENTS.md b/AGENTS.md index af5e307..94d0ff6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,6 +123,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **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. +- **`gh` classified like `git`** (`internal/danger/classifier.go`) — the GitHub CLI is in `networkPrefixes`; every real subcommand (`gh pr`, `gh api`, `gh repo delete`, …) contacts the GitHub API and classifies as `network_egress`, while meta invocations (`gh --version`, `gh help`, `gh completion`, bare `gh`) stay `safe`. `-R`/`--repo` consumes its value token so it is not mistaken for the subcommand (parity with `git -C`). - **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/internal/danger/classifier.go b/internal/danger/classifier.go index 5b61bac..57d03f3 100644 --- a/internal/danger/classifier.go +++ b/internal/danger/classifier.go @@ -836,6 +836,9 @@ var networkPrefixes = map[string]bool{ "curl": true, "wget": true, "scp": true, "rsync": true, "nc": true, "ncat": true, "ssh": true, "sftp": true, "ftp": true, "tftp": true, "telnet": true, "git": true, + // gh is the GitHub CLI: like git's remote-contacting subcommands, every + // real gh subcommand talks to the GitHub API (see isNetworkEgress). + "gh": true, // reverse-shell / tunnelling relays "socat": true, "rclone": true, // DNS lookups double as exfiltration channels @@ -2437,6 +2440,34 @@ func isNetworkEgress(first string, tokens []string) bool { } return false } + // gh subcommands inherently contact the GitHub API — the same class as + // git's remote-contacting subcommands. Only meta invocations (help, + // completion, version queries) stay local and fall through to Safe. + if first == "gh" { + skipNext := false + for _, tok := range tokens[1:] { + if skipNext { + skipNext = false + continue + } + if strings.HasPrefix(tok, "-") { + // -R/--repo consumes the following token as its value; it must + // not be mistaken for the subcommand (parity with git -C). + if tok == "-R" || tok == "--repo" { + skipNext = true + } + continue + } + // First non-flag token is the subcommand. + switch tok { + case "help", "completion", "version": + return false + } + return true + } + // Bare gh or flags only (e.g. gh --version, gh --help). + return false + } // rsync with remote target (contains :) if first == "rsync" { for _, tok := range tokens[1:] { diff --git a/internal/danger/classifier_test.go b/internal/danger/classifier_test.go index df801f7..41ec53c 100644 --- a/internal/danger/classifier_test.go +++ b/internal/danger/classifier_test.go @@ -198,6 +198,44 @@ func TestClassify_NetworkEgress_GitPushNeedsRemote(t *testing.T) { } } +func TestClassify_NetworkEgress_Gh(t *testing.T) { + // gh gets the same classification as git: every real subcommand contacts + // the GitHub API (network egress); meta invocations stay local (safe). + tests := []struct { + cmd string + cls RiskClass + }{ + {"gh pr view 123", NetworkEgress}, + {"gh pr merge 125 --squash", NetworkEgress}, + {"gh repo clone owner/repo", NetworkEgress}, + {"gh repo delete owner/repo --yes", NetworkEgress}, + {"gh api /user", NetworkEgress}, + {"gh auth login", NetworkEgress}, + {"gh release delete v1.0.0 --yes", NetworkEgress}, + {"/usr/local/bin/gh pr checks", NetworkEgress}, + // -R/--repo consumes the following token as its value; it must not be + // mistaken for the subcommand (parity with git -C). + {"gh -R owner/repo pr list", NetworkEgress}, + {"gh --repo owner/repo pr list", NetworkEgress}, + + {"gh", Safe}, + {"gh --version", Safe}, + {"gh --help", Safe}, + {"gh help", Safe}, + {"gh help pr", Safe}, + {"gh completion -s bash", Safe}, + {"gh -R owner/repo help", Safe}, + } + for _, tt := range tests { + t.Run(tt.cmd, func(t *testing.T) { + got := Classify(tt.cmd) + if got != tt.cls { + t.Errorf("Classify(%q) = %s, want %s", tt.cmd, got, tt.cls) + } + }) + } +} + func TestClassify_CodeExecution_Commands(t *testing.T) { tests := []struct { cmd string