Skip to content

Commit 73b0a29

Browse files
authored
fix(ssrf/browser/http_batch): wrap errors, drop internal IPs (L-7)
The SSRF dial guard's refusal message previously included the resolved internal IP, leaking an internal-DNS oracle. browser and http_batch also returned raw network/TLS errors to the model, letting attacker-controlled x509 SAN text escape the untrusted boundary. - Drop the resolved IP from the SSRF refusal message. - Wrap browser fetch errors as untrusted content. - Wrap http_batch request errors as untrusted content. - Add regression tests for IP omission and error wrapping. - Document the change in SECURITY.md and AGENTS.md. Closes L-7 in sec_findings.md.
1 parent 4627b56 commit 73b0a29

8 files changed

Lines changed: 93 additions & 3 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
117117
- **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.<token>` 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.
118118
- **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.
119119
- **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.
120+
- **SSRF error wrapping** (`cmd/odek/ssrf_guard.go`, `cmd/odek/browser_tool.go`, `cmd/odek/perf_tools.go`) — the SSRF refusal message no longer includes the resolved internal IP, and `browser`/`http_batch` network/TLS errors are wrapped as untrusted content so attacker-controlled text in x509 errors cannot reach the model unwrapped.
120121
- **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. Every `/api/*` endpoint additionally requires the per-instance `odek_ws_token` (cookie or `X-Odek-Ws-Token` header) and rejects non-loopback `Host` headers, closing DNS-rebinding reads of `/api/sessions`, `/api/resources`, and `/api/models`.
121122
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.
122123
- **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.

cmd/odek/browser_tool.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,10 @@ func (t *browserTool) doNavigate(rawURL string) (string, error) {
236236

237237
resp, err := t.client.Do(req)
238238
if err != nil {
239-
return jsonError(fmt.Sprintf("cannot fetch %q: %v", rawURL, err))
239+
// Wrap network/TLS errors as untrusted: x509 errors can contain
240+
// attacker-controlled SAN text, and dial errors can expose internal IPs.
241+
msg := fmt.Sprintf("cannot fetch %q: %v", rawURL, err)
242+
return jsonError(wrapUntrusted(t.toolCtx(), rawURL, msg))
240243
}
241244
defer resp.Body.Close()
242245

cmd/odek/browser_tool_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"net/http"
67
"net/http/httptest"
78
"strings"
@@ -452,6 +453,41 @@ func TestBrowser_ExtractsInteractiveElements(t *testing.T) {
452453
}
453454
}
454455

456+
// TestBrowser_Navigate_ErrorWrapped verifies that network/TLS errors from the
457+
// HTTP client are wrapped as untrusted content so attacker-controlled text
458+
// (e.g. x509 SANs) cannot reach the model outside the untrusted boundary.
459+
func TestBrowser_Navigate_ErrorWrapped(t *testing.T) {
460+
allow := "allow"
461+
b := &browserTool{
462+
state: &browserState{nextRef: 1},
463+
dangerousConfig: danger.DangerousConfig{NonInteractive: &allow},
464+
}
465+
b.client = &http.Client{
466+
Transport: roundTripperFunc(func(*http.Request) (*http.Response, error) {
467+
return nil, fmt.Errorf("x509: certificate is valid for attacker-controlled.example.com, not example.com")
468+
}),
469+
}
470+
471+
result := callJSON(t, b, `{"action":"navigate","url":"https://example.com/"}`)
472+
var r struct {
473+
Error string `json:"error"`
474+
}
475+
mustUnmarshal(t, result, &r)
476+
if r.Error == "" {
477+
t.Fatal("expected error")
478+
}
479+
if !strings.HasPrefix(r.Error, "<untrusted_content_") {
480+
t.Errorf("browser error should be wrapped as untrusted, got: %q", r.Error)
481+
}
482+
}
483+
484+
// roundTripperFunc adapts a function to http.RoundTripper.
485+
type roundTripperFunc func(*http.Request) (*http.Response, error)
486+
487+
func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
488+
return f(req)
489+
}
490+
455491
// ── URL Resolution Tests ───────────────────────────────────────────────
456492

457493
func TestResolveURL_RelativeURL(t *testing.T) {

cmd/odek/next_security_vulnerabilities_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,37 @@ func TestParallelShell_CapsOutputSize(t *testing.T) {
590590
}
591591
}
592592

593+
// ── 6a. http_batch must wrap network/TLS errors as untrusted ─────────────
594+
595+
func TestHTTPBatch_WrapsErrors(t *testing.T) {
596+
allow := "allow"
597+
tool := &httpBatchTool{
598+
dangerousConfig: danger.DangerousConfig{NonInteractive: &allow},
599+
}
600+
tool.client = &http.Client{
601+
Transport: roundTripperFunc(func(*http.Request) (*http.Response, error) {
602+
return nil, fmt.Errorf("x509: certificate is valid for attacker.com, not target.com")
603+
}),
604+
}
605+
606+
result := callJSON(t, tool, `{"requests":[{"url":"https://target.com/"}]}`)
607+
var r struct {
608+
Results []struct {
609+
Error string `json:"error"`
610+
} `json:"results"`
611+
}
612+
mustUnmarshal(t, result, &r)
613+
if len(r.Results) != 1 {
614+
t.Fatalf("expected 1 result, got %d", len(r.Results))
615+
}
616+
if r.Results[0].Error == "" {
617+
t.Fatal("expected error")
618+
}
619+
if !strings.HasPrefix(r.Results[0].Error, "<untrusted_content_") {
620+
t.Errorf("http_batch error should be wrapped as untrusted, got: %q", r.Results[0].Error)
621+
}
622+
}
623+
593624
// ── 7. Browser must enforce an HTTP request timeout ──────────────────────
594625

595626
func TestBrowser_NavigateTimeout(t *testing.T) {

cmd/odek/perf_tools.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,9 @@ func (t *httpBatchTool) fetchOne(r httpBatchReq) httpBatchEntry {
705705

706706
resp, err := t.client.Do(httpReq)
707707
if err != nil {
708-
entry.Error = err.Error()
708+
// Wrap network/TLS errors as untrusted so attacker-controlled text in
709+
// x509 / dial errors cannot reach the model outside the untrusted boundary.
710+
entry.Error = wrapUntrusted(t.toolCtx(), r.URL, err.Error())
709711
return entry
710712
}
711713
defer resp.Body.Close()

cmd/odek/ssrf_guard.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ func ssrfGuardedDial(base dialFunc, lookup ipLookupFunc, allowedHosts ...string)
8787
if !hostAllowed {
8888
for _, ipa := range ips {
8989
if danger.IsBlockedIP(ipa.IP) {
90-
return nil, fmt.Errorf("blocked connection to %q: resolves to internal address %s (possible SSRF / DNS rebinding)", host, ipa.IP)
90+
// Do not include the resolved IP in the error: it would leak an
91+
// internal DNS oracle to the model/audit log.
92+
return nil, fmt.Errorf("blocked connection to %q: resolves to an internal address (possible SSRF / DNS rebinding)", host)
9193
}
9294
}
9395
}

cmd/odek/ssrf_guard_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ func TestSSRFGuardedDial_ImplicitlyInternalDialedDirect(t *testing.T) {
5555
}
5656
}
5757

58+
func TestSSRFGuardedDial_ErrorOmitsInternalIP(t *testing.T) {
59+
var dialed []string
60+
guard := ssrfGuardedDial(recordingDial(&dialed), stubLookup("10.0.0.1"))
61+
62+
_, err := guard(context.Background(), "tcp", "evil.example.com:80")
63+
if err == nil {
64+
t.Fatal("expected refusal")
65+
}
66+
if strings.Contains(err.Error(), "10.0.0.1") {
67+
t.Errorf("error must not leak resolved IP: %v", err)
68+
}
69+
}
70+
5871
func TestSSRFGuardedDial_ExternalResolvingInternalRefused(t *testing.T) {
5972
// The SSRF / rebinding core case: a host that presents as external but
6073
// resolves to an internal address must be refused, and no dial attempted.

docs/SECURITY.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,8 @@ There is no user-facing allowlist config field today; the list is derived from e
396396

397397
When `HTTP(S)_PROXY` is set, the transport would dial the proxy address instead of the target, so the dial-time guard would validate only the proxy and the real target could be an internal/rebound address. `ssrfGuardedTransport` detects an active proxy and refuses the request with a clear error rather than silently disabling SSRF protection. Outbound tool traffic therefore requires direct connections.
398398

399+
The SSRF refusal message no longer includes the resolved internal IP, and network/TLS errors from `browser` and `http_batch` are wrapped as untrusted content before reaching the model. This closes two leak channels: an internal-DNS oracle (the resolved IP) and attacker-controlled text inside x509 certificate errors.
400+
399401
### 18b. CGNAT and benchmark IP blocking
400402

401403
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.

0 commit comments

Comments
 (0)