diff --git a/docker/.env b/docker/.env index 532f35706f..d33ff98f99 100644 --- a/docker/.env +++ b/docker/.env @@ -307,8 +307,9 @@ ADMIN_DEFAULT_PASSWORD=admin # Development-only: set to 1 to allow private database and connector hosts # (MySQL, PostgreSQL, IMAP, test_db_connection, ExeSQL), including -# host.docker.internal. The dial stays pinned to the resolved address. -# Agent URL and Invoke checks stay in force. Do not enable in production. +# host.docker.internal, and HTTP request (Invoke) target and proxy URLs. +# Dials stay pinned to the resolved address; HTTP/HTTPS validation still applies. +# Other Agent URL checks stay in force. Do not enable in production. ALLOW_ANY_HOST=0 # The RAGFlow Docker image to download. v0.22+ doesn't include embedding models. diff --git a/internal/agent/component/invoke.go b/internal/agent/component/invoke.go index 5ded3578b1..df3c0bb38a 100644 --- a/internal/agent/component/invoke.go +++ b/internal/agent/component/invoke.go @@ -24,12 +24,13 @@ // traceparent headers. // // SSRF guard (PR #15426): every outbound URL is validated against -// the shared common.AssertURLSafe before any network I/O. Both the -// target URL and an optional proxy URL are checked — the proxy +// the shared common.AssertURLSafe before any network I/O, unless +// ALLOW_ANY_HOST permits private hosts. Both the target URL and +// an optional proxy URL are checked — the proxy // vector matters because the Go transport hands the request to the // proxy host, which would otherwise re-resolve the original host // and re-open the rebinding window the SSRF guard just closed. To -// defeat DNS rebinding the transport dials the validated public IP +// defeat DNS rebinding the transport dials the validated IP // directly (utility.PinnedHTTPClient) and we disable redirect // following so a 30x to a private host cannot bypass the guard. package component @@ -93,12 +94,12 @@ func (i *InvokeComponent) Name() string { return i.name } // param // // SSRF flow (PR #15426): -// 1. Validate the target URL via common.AssertURLSafe (loopback / -// link-local / RFC1918 / metadata / unresolvable are rejected). +// 1. Validate the target URL (private addresses are rejected unless +// ALLOW_ANY_HOST is enabled; scheme and DNS checks always apply). // 2. Validate the optional proxy URL the same way (the proxy // re-resolves the target host; an unsafe proxy would defeat // step 1). -// 3. Use utility.PinnedHTTPClient to dial the validated public IP +// 3. Use utility.PinnedHTTPClient to dial the validated IP // for the target host — closing the TOCTOU window between // validation and connect. // 4. Disable redirect following so a 30x to a private host cannot @@ -140,8 +141,8 @@ func (i *InvokeComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st } // Step 1: SSRF guard for the target URL. The validated - // hostname + resolved public IP are reused for DNS pinning. - host, pinnedIP, err := common.AssertURLSafe(rawURL) + // hostname + resolved IP are reused for DNS pinning. + host, pinnedIP, err := validateInvokeURL(rawURL) if err != nil { return invokeSSRFError("url", rawURL, err), nil } @@ -180,7 +181,7 @@ func (i *InvokeComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st return invokeSSRFError("url", rawURL, fmt.Errorf("Invoke: proxy mode requires a literal-IP target URL (hostnames are unsafe because the proxy re-resolves them)")), nil } - ph, pip, perr := common.AssertURLSafe(proxyStr) + ph, pip, perr := validateInvokeURL(proxyStr) if perr != nil { return invokeSSRFError("proxy", proxyStr, perr), nil } @@ -247,7 +248,7 @@ func (i *InvokeComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { // When the transport dials the proxy, addr is // ":". Replace the - // host with the validated public IP while + // host with the validated IP while // keeping the original port. Any other dial // (e.g. a redirect hop the no-redirect policy // would have blocked) falls through to the @@ -266,7 +267,7 @@ func (i *InvokeComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st CheckRedirect: noRedirects, } } else { - // Direct path: pin to the validated public IP, disable + // Direct path: pin to the validated IP, disable // redirects, and apply the OTel transport. PinnedHTTPClient // sets its own timeout; we re-wrap with otelhttp so the // request gets a child span + W3C traceparent injected. @@ -379,6 +380,22 @@ func invokeHeaders(raw any) (map[string]any, error) { return headers, nil } +func validateInvokeURL(rawURL string) (hostname, resolvedIP string, err error) { + if !common.AllowConfiguredPrivateHost() { + return common.AssertURLSafe(rawURL) + } + if err := common.AssertURLSchemeSafe(rawURL); err != nil { + return "", "", err + } + parsed, err := url.Parse(strings.TrimSpace(rawURL)) + if err != nil { + return "", "", err + } + hostname = parsed.Hostname() + resolvedIP, err = common.ResolveHostPin(hostname) + return hostname, resolvedIP, err +} + // invokeSSRFError builds the _ERROR output the canvas uses to route // around a refused URL. We mirror the Python message verbatim // ("URL not valid") so downstream consumers that key on the string diff --git a/internal/agent/component/invoke_test.go b/internal/agent/component/invoke_test.go index 3735a11715..f759ff7c6f 100644 --- a/internal/agent/component/invoke_test.go +++ b/internal/agent/component/invoke_test.go @@ -34,19 +34,11 @@ import ( "go.uber.org/zap/zaptest/observer" ) -// Test setup: enable the test-only SSRF bypass for tests in this -// file (the happy-path httptest server lives on 127.0.0.1, which -// the production guard rejects). The bypass is now a process- -// memory boolean (common.AllowAnyHostForTest) instead of an -// env var — the previous form (ALLOW_ANY_HOST env) was a live -// runtime toggle any operator could flip to disable the guard -// globally. PR review round 6, Major #3. -// -// Each test that wants the production guard back resets it to -// false in its body; we don't blanket-disable here because some -// tests below rely on the bypass being on. +// Reset the environment opt-in so tests select private-host access explicitly +// through either ALLOW_ANY_HOST or the in-memory test hook. func setupAllowAnyHost(t *testing.T, enabled bool) { t.Helper() + t.Setenv(common.EnvAllowAnyHost, "") prev := common.AllowAnyHostForTest common.AllowAnyHostForTest = enabled t.Cleanup(func() { common.AllowAnyHostForTest = prev }) @@ -254,6 +246,85 @@ func TestInvoke_MissingURL(t *testing.T) { } } +func TestInvoke_AllowAnyHost(t *testing.T) { + setupAllowAnyHost(t, false) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.Host, "local.test.invalid:") || r.URL.Path != "/logo.svg" { + t.Errorf("request = %s%s, want local.test.invalid:/logo.svg", r.Host, r.URL.Path) + } + _, _ = w.Write([]byte("local response")) + })) + defer srv.Close() + _, port, err := net.SplitHostPort(strings.TrimPrefix(srv.URL, "http://")) + if err != nil { + t.Fatal(err) + } + + originalLookup := common.LookupHost + common.LookupHost = func(host string) ([]string, error) { + if host != "local.test.invalid" { + return nil, fmt.Errorf("unexpected hostname %q", host) + } + return []string{"127.0.0.1"}, nil + } + t.Cleanup(func() { common.LookupHost = originalLookup }) + + for _, value := range []string{"", "0", "1"} { + t.Run("ALLOW_ANY_HOST="+value, func(t *testing.T) { + t.Setenv(common.EnvAllowAnyHost, value) + c, err := NewInvokeComponent(nil) + if err != nil { + t.Fatal(err) + } + out, err := c.Invoke(t.Context(), nil, map[string]any{ + "method": "GET", + "url": "http://local.test.invalid:" + port + "/logo.svg", + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + if value == "1" { + if out["result"] != "local response" || out["_ERROR"] != nil { + t.Fatalf("output = %#v, want local response", out) + } + } else if out["_ERROR"] != "URL not valid" || out["result"] != nil { + t.Fatalf("output = %#v, want blocked URL", out) + } + }) + } +} + +func TestInvoke_AllowAnyHostRejectsInvalidURLs(t *testing.T) { + setupAllowAnyHost(t, false) + t.Setenv(common.EnvAllowAnyHost, "1") + originalLookup := common.LookupHost + common.LookupHost = func(host string) ([]string, error) { + return nil, fmt.Errorf("no such host: %s", host) + } + t.Cleanup(func() { common.LookupHost = originalLookup }) + + for _, kind := range []string{"url", "proxy"} { + for _, target := range []string{"file://127.0.0.1/logo.svg", "http:///logo.svg", "http://missing.test.invalid/logo.svg"} { + t.Run(kind+"/"+target, func(t *testing.T) { + c, err := NewInvokeComponent(nil) + if err != nil { + t.Fatal(err) + } + inputs := map[string]any{"method": "GET", "url": "http://192.168.1.5/logo.svg"} + inputs[kind] = target + out, err := c.Invoke(t.Context(), nil, inputs) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + if out["_ERROR"] != "URL not valid" || out["result"] != nil { + t.Fatalf("output = %#v, want blocked URL", out) + } + }) + } + } +} + // TestInvoke_SSRFGuard_BlocksLoopback mirrors PR #15426: when // AllowAnyHostForTest is unset (production shape), the Invoke // component must reject loopback / link-local / RFC1918 URLs @@ -360,8 +431,8 @@ func TestInvoke_SSRFGuard_BlocksProxy(t *testing.T) { // not followed. This closes the bypass window where a public // host could 302-redirect to a private one. func TestInvoke_NoRedirects_NotFollowed(t *testing.T) { - // Need the bypass to talk to the local httptest server. - setupAllowAnyHost(t, true) + setupAllowAnyHost(t, false) + t.Setenv(common.EnvAllowAnyHost, "1") srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Location", "http://127.0.0.1:1/secret") @@ -402,9 +473,8 @@ func TestInvoke_NoRedirects_NotFollowed(t *testing.T) { // proxy URL itself is unreachable on the test network, so // the dial will fail — but with an error that mentions // the IP we dialled, not the original hostname. That -// proves the pinning path is active. We un-set -// ALLOW_ANY_HOST so the SSRF guard accepts a public-IP -// URL but the dial still happens through our code path. +// proves the pinning path is active. ALLOW_ANY_HOST permits +// the local proxy while the dial stays pinned. // // Target host is a literal IP (8.8.8.8) — proxy mode // fail-closes for hostname targets because the proxy @@ -412,7 +482,8 @@ func TestInvoke_NoRedirects_NotFollowed(t *testing.T) { // would re-open the rebinding window. PR review round 5, // Major #3. func TestInvoke_ProxyDNSPin(t *testing.T) { - setupAllowAnyHost(t, true) + setupAllowAnyHost(t, false) + t.Setenv(common.EnvAllowAnyHost, "1") proxyHit := make(chan struct{}, 1) proxySrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/common/allow_private_host.go b/internal/common/allow_private_host.go index 5106187559..d683db2bb7 100644 --- a/internal/common/allow_private_host.go +++ b/internal/common/allow_private_host.go @@ -31,8 +31,8 @@ import ( // The flag is the development opt-in documented in docker/.env. It applies // to host-based database and connector dials (MySQL, PostgreSQL, IMAP, // test_db_connection, ExeSQL), including host.docker.internal, and configured -// MCP servers. Other URL and Invoke guards do not call this. DNS pinning -// remains enabled. Default is off. +// MCP servers, and Invoke target and proxy URLs. Other URL guards do not +// call this. DNS pinning remains enabled. Default is off. func AllowConfiguredPrivateHost() bool { switch strings.ToLower(strings.TrimSpace(GetEnv(EnvAllowAnyHost))) { case "1", "true", "yes", "on": diff --git a/internal/common/http.go b/internal/common/http.go index bb1990b708..3d40bfa19d 100644 --- a/internal/common/http.go +++ b/internal/common/http.go @@ -477,17 +477,9 @@ var LookupHost = net.LookupHost // talk to a local httptest server flip it on and reset it in // t.Cleanup. // -// The previous form (env-var ALLOW_ANY_HOST) was a live runtime -// toggle that any operator could flip to disable the SSRF guard -// globally — including the DNS pinning that the Invoke component -// relies on. PR review round 6, Major #3: this variable lives in -// process memory only, so it cannot be enabled by an env var or -// a deployment mistake. The explicit "_ForTest" suffix is the -// signal that production code must never touch it. -// -// Host-based database and connector dials and configured MCP servers are a -// separate opt-in. AllowConfiguredPrivateHost reads ALLOW_ANY_HOST for those -// dials only. This function stays limited to the in-memory test hook. +// Host-based database and connector dials, configured MCP servers, and Invoke +// URLs have a separate ALLOW_ANY_HOST opt-in via AllowConfiguredPrivateHost. +// AssertURLSafe and AssertHostSafe stay limited to the in-memory test hook. var AllowAnyHostForTest = false // allowAnyHost reads the test-only override. Kept as a private @@ -500,8 +492,8 @@ func allowAnyHost() bool { // AssertURLSchemeSafe is a lenient SSRF guard for drivers that may legitimately // target private networks or loopback addresses (e.g. self-hosted Ollama, vLLM, // Xinference). It only rejects dangerous schemes and empty hosts; it does not -// resolve DNS and does not require public routability. Use this ONLY for -// local-inference model drivers — cloud-hosted drivers must use AssertURLSafe. +// resolve DNS and does not require public routability. Use this only where +// private hosts are explicitly permitted; cloud-hosted drivers must use AssertURLSafe. var AssertURLSchemeSafe = func(rawURL string) error { parsed, err := url.Parse(strings.TrimSpace(rawURL)) if err != nil {