diff --git a/AGENTS.md b/AGENTS.md index ad9194b..f197eca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,7 +115,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **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. - **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`. - **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`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs, 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 / 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. - **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. - **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls. - **Browser element cap** (`cmd/odek/browser_tool.go`) — the number of interactive elements extracted per page is capped at 500 so a hostile page cannot OOM the agent with thousands of links or buttons. diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 7c6d574..a527e52 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -391,6 +391,12 @@ client := &http.Client{ There is no user-facing allowlist config field today; the list is derived from each tool's own operator-controlled `base_url`. If you need a broader or user-editable allowlist, add a `dangerous.ssrf_allowed_hosts` (or `network.allowed_hosts`) array to the config and merge it into the set passed to `ssrfGuardedTransport`. +### 18b. CGNAT and benchmark IP blocking + +Go's `net.IP.IsPrivate()` covers RFC1918 and RFC4193 private ranges, but it does not cover RFC 6598 CGNAT (`100.64.0.0/10`) or the RFC 2544 benchmark-testing range (`198.18.0.0/15`). Tailscale and similar overlay networks use `100.64/10` addresses, so an attacker who could steer the agent to `http://100.x.x.x` (or a hostname that rebinding resolves to such an address) could reach unauthenticated internal services that the operator expected to be unreachable from the agent. + +`internal/danger.IsBlockedIP` now blocks both ranges in addition to loopback, RFC1918, RFC4193, link-local, and unspecified addresses. Because this is the single source of truth used by `ClassifyURL` and the dial-time SSRF guard, the policy gate and the transport stay in sync. + ### 19. MCP server environment sanitisation MCP server subprocesses no longer inherit the full odek process environment. They receive only a minimal allowlist of safe variables (e.g. `PATH`, `HOME`, `LANG`, `TMPDIR`) plus any explicit `env` overrides from the server config. Keys matching secret patterns — `*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_CREDENTIAL`, `*_PRIVATE_KEY`, etc. — are stripped even when listed in `env`. This prevents a compromised or malicious MCP server from reading secrets loaded from `~/.odek/secrets.env` or other provider keys that were present in the parent environment. diff --git a/internal/danger/classifier.go b/internal/danger/classifier.go index cfc08d5..0910c4d 100644 --- a/internal/danger/classifier.go +++ b/internal/danger/classifier.go @@ -330,10 +330,27 @@ func ClassifyURL(rawURL string) RiskClass { return NetworkEgress } +// extraBlockedNets contains IPv4 ranges that net.IP.IsPrivate does not cover +// but which must still be unreachable to the agent's web tools: +// - 100.64.0.0/10 RFC 6598 CGNAT (includes Tailscale) +// - 198.18.0.0/15 RFC 2544 benchmark testing +var extraBlockedNets []*net.IPNet + +func init() { + for _, cidr := range []string{"100.64.0.0/10", "198.18.0.0/15"} { + _, n, err := net.ParseCIDR(cidr) + if err != nil { + panic(fmt.Sprintf("danger: invalid blocked CIDR %q: %v", cidr, err)) + } + extraBlockedNets = append(extraBlockedNets, n) + } +} + // IsBlockedIP reports whether ip falls in a range that the agent's web tools // must never reach: loopback (127/8, ::1), RFC1918 / RFC4193 private (incl. // IPv6 ULA fc00::/7), link-local (169.254/16 — which covers the -// 169.254.169.254 cloud-metadata endpoint — and fe80::/10), or the unspecified +// 169.254.169.254 cloud-metadata endpoint — and fe80::/10), RFC 6598 CGNAT +// (100.64/10), RFC 2544 benchmark testing (198.18/15), or the unspecified // address (0.0.0.0, ::). It is the single source of truth shared by both // ClassifyURL's literal-host gate and the dial-time SSRF guard, so the two // cannot drift apart. A nil IP is treated as blocked (fail closed). @@ -341,9 +358,17 @@ func IsBlockedIP(ip net.IP) bool { if ip == nil { return true } - return ip.IsLoopback() || ip.IsPrivate() || + if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || - ip.IsUnspecified() + ip.IsUnspecified() { + return true + } + for _, n := range extraBlockedNets { + if n.Contains(ip) { + return true + } + } + return false } // hostnameIsInternal reports whether a non-IP hostname denotes a well-known @@ -2196,8 +2221,8 @@ func isNetworkEgress(first string, tokens []string) bool { // 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, + "core.pager": true, + "core.fsmonitor": true, "credential.helper": true, } diff --git a/internal/danger/classifier_test.go b/internal/danger/classifier_test.go index 6c2dea9..d62a659 100644 --- a/internal/danger/classifier_test.go +++ b/internal/danger/classifier_test.go @@ -1499,9 +1499,12 @@ func TestIsBlockedIP(t *testing.T) { {"1.1.1.1", false}, {"93.184.216.34", false}, {"2606:4700:4700::1111", false}, - // CGNAT 100.64/10 is not covered by Go's IsPrivate — documents the - // current (allowed) behavior so a future tightening is a conscious change. - {"100.64.0.1", false}, + // CGNAT 100.64/10 (RFC 6598) and RFC 2544 benchmark testing + // 198.18/15 are not covered by Go's IsPrivate. + {"100.64.0.1", true}, + {"100.127.255.255", true}, + {"198.18.0.1", true}, + {"198.19.255.255", true}, } for _, tt := range tests { t.Run(tt.ip, func(t *testing.T) {