fix(fetch): close redirect/FQDN bypasses in domain filtering

Three issues found while reviewing the domain-filtering feature:

1. SSRF via redirect (critical). The http.Client had no CheckRedirect,

   so an allow-listed origin returning a 3xx to a forbidden host would

   be followed and its body returned to the caller \u2014 a classic bypass.

   Now every redirect target is re-checked against the same lists; a

   regression test against http://169.254.169.254/ (AWS metadata IP)

   demonstrates the fix.

2. FQDN trailing-dot bypass. URLs in FQDN form ("http://host./") kept

   the trailing dot in url.URL.Hostname() and slipped past patterns

   like "host". The matcher now strips trailing dots from both inputs.

3. Empty/whitespace domain entries silently rejected every URL in

   allowed_domains and matched nothing in blocked_domains. Validation

   now rejects them at config-load time with a clear error.

Also documented the IP-encoding limitation (decimal/hex/octal IPv4)

in the user-facing fetch tool docs.

Assisted-By: docker-agent

Author: dgageot

docs/tools/fetch/index.md

@@ -41,9 +41,18 @@ Domain patterns in `allowed_domains` and `blocked_domains` use the following rul
 - **Bare domain** — `example.com` matches the host `example.com` _and_ any subdomain such as `docs.example.com`. It does **not** match unrelated hosts that share a suffix (e.g. `badexample.com`).
 - **Leading dot** — `.example.com` matches **only** strict subdomains (`docs.example.com`, `a.b.example.com`), not the apex `example.com`.
 - **IP literal** — IP addresses are matched exactly (`169.254.169.254`).
+- **Trailing dots** in FQDN-form URLs (`http://example.com./`) are stripped before matching, so they cannot bypass a deny-list entry.
 
 The lists are mutually exclusive: a single fetch toolset may set either `allowed_domains` or `blocked_domains`, but not both.
 
+When a list is configured, every redirect target is re-checked against the same list. A request to an allowed origin that redirects to a forbidden host is rejected before any data is read from the redirect.
+
+<div class="callout callout-warning" markdown="1">
+<div class="callout-title">⚠️ Limitations
+</div>
+  <p>Matching is purely string-based on the URL host. It does <strong>not</strong> perform DNS resolution and does <strong>not</strong> normalise alternative IP encodings (decimal <code>2852039166</code>, hex <code>0xa9.0xfe.0xa9.0xfe</code>, octal, IPv4-mapped IPv6, etc.). If you need to deny access to a specific IP, also list its alternative encodings, or block at the network layer.</p>
+</div>
+
 ### Custom Timeout
 
 ```yaml

pkg/config/latest/validate.go

@@ -88,6 +88,12 @@ func (t *Toolset) validate() error {
 	if len(t.AllowedDomains) > 0 && len(t.BlockedDomains) > 0 {
 		return errors.New("allowed_domains and blocked_domains are mutually exclusive")
 	}
+	if err := validateDomainPatterns("allowed_domains", t.AllowedDomains); err != nil {
+		return err
+	}
+	if err := validateDomainPatterns("blocked_domains", t.BlockedDomains); err != nil {
+		return err
+	}
 	if len(t.Models) > 0 && t.Type != "model_picker" {
 		return errors.New("models can only be used with type 'model_picker'")
 	}
@@ -203,6 +209,18 @@ func (t *Toolset) validate() error {
 	return nil
 }
 
+// validateDomainPatterns rejects empty / whitespace-only entries in a fetch
+// allow- or block-list, since they silently match nothing and turn the list
+// into a foot-gun (e.g. allowed_domains: [""] would reject every URL).
+func validateDomainPatterns(field string, patterns []string) error {
+	for i, p := range patterns {
+		if strings.TrimSpace(p) == "" {
+			return fmt.Errorf("%s[%d] must not be empty", field, i)
+		}
+	}
+	return nil
+}
+
 // isLoopbackHost reports whether host is a loopback address (with or without
 // a port component). It accepts IPv4 loopback, IPv6 loopback, and the literal
 // "localhost".

pkg/config/latest/validate_test.go

@@ -298,6 +298,35 @@ agents:
 `,
 			wantErr: "blocked_domains can only be used with type 'fetch'",
 		},
+		{
+			name: "empty allowed_domains entry is rejected",
+			config: `
+version: "8"
+agents:
+  root:
+    model: "openai/gpt-4"
+    toolsets:
+      - type: fetch
+        allowed_domains:
+          - ""
+          - docker.com
+`,
+			wantErr: "allowed_domains[0] must not be empty",
+		},
+		{
+			name: "whitespace-only blocked_domains entry is rejected",
+			config: `
+version: "8"
+agents:
+  root:
+    model: "openai/gpt-4"
+    toolsets:
+      - type: fetch
+        blocked_domains:
+          - "   "
+`,
+			wantErr: "blocked_domains[0] must not be empty",
+		},
 	}
 
 	for _, tt := range tests {

pkg/tools/builtin/fetch.go

@@ -55,6 +55,15 @@ func (h *fetchHandler) CallTool(ctx context.Context, params FetchToolArgs) (*too
 	client := &http.Client{
 		Timeout:   h.timeout,
 		Transport: remote.NewTransport(ctx),
+		// Re-check the domain allow/deny lists on every redirect: without this,
+		// an allowed origin could redirect into a denied one and bypass the
+		// policy. The 10-redirect cap mirrors the net/http default.
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if len(via) >= 10 {
+				return errors.New("stopped after 10 redirects")
+			}
+			return h.checkDomainAllowed(req.URL)
+		},
 	}
 	if params.Timeout > 0 {
 		client.Timeout = time.Duration(params.Timeout) * time.Second
@@ -291,9 +300,13 @@ func (h *fetchHandler) checkDomainAllowed(u *url.URL) error {
 // ("docs.example.com"); it does NOT match unrelated hosts that share a suffix
 // ("badexample.com"). A pattern with a leading dot (".example.com") matches
 // strict subdomains only — the apex "example.com" is excluded.
+//
+// Trailing dots used in FQDN form ("example.com.") are stripped from both
+// host and pattern before matching, so a URL like http://example.com./ cannot
+// be used to bypass a deny-list entry for example.com.
 func matchesDomain(host, pattern string) bool {
-	host = strings.ToLower(strings.TrimSpace(host))
-	pattern = strings.ToLower(strings.TrimSpace(pattern))
+	host = strings.TrimSuffix(strings.ToLower(strings.TrimSpace(host)), ".")
+	pattern = strings.TrimSuffix(strings.ToLower(strings.TrimSpace(pattern)), ".")
 	if host == "" || pattern == "" || pattern == "." {
 		return false
 	}

pkg/tools/builtin/fetch_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	neturl "net/url"
 	"testing"
 	"time"
 
@@ -433,6 +434,11 @@ func TestMatchesDomain(t *testing.T) {
 		{"only-dot pattern matches nothing", "example.com", ".", false},
 		{"whitespace tolerated", " example.com ", " example.com ", true},
 		{"ip address exact", "169.254.169.254", "169.254.169.254", true},
+		// FQDN trailing dot (regression: must not bypass the matcher).
+		{"trailing dot host matches apex pattern", "example.com.", "example.com", true},
+		{"trailing dot host matches subdomain pattern", "docs.example.com.", "example.com", true},
+		{"trailing dot pattern matches apex host", "example.com", "example.com.", true},
+		{"trailing dot host matches strict-subdomain pattern", "docs.example.com.", ".example.com", true},
 	}
 
 	for _, tc := range tests {
@@ -523,3 +529,59 @@ func TestFetch_BlockedDomains_DeniesIgnoringRobots(t *testing.T) {
 	assert.Contains(t, result.Output, "is blocked by blocked_domains")
 	assert.False(t, robotsRequested, "blocked URLs must not trigger any network call, including robots.txt")
 }
+
+// TestFetch_AllowedDomains_RejectsRedirectToBlockedHost is a regression test for an
+// SSRF-style bypass: an allow-listed origin returning a redirect to a host
+// that is NOT in the allow-list must be rejected before the redirect is
+// followed, otherwise the policy is hollow.
+func TestFetch_AllowedDomains_RejectsRedirectToBlockedHost(t *testing.T) {
+	redirected := false
+	url := runHTTPServer(t, func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/robots.txt" {
+			http.NotFound(w, r)
+			return
+		}
+		redirected = true
+		http.Redirect(w, r, "http://attacker.example.com/secret", http.StatusFound)
+	})
+	parsed, err := neturl.Parse(url)
+	require.NoError(t, err)
+
+	// Allow only the test server's host. The redirect target must be
+	// rejected without any network call to attacker.example.com.
+	tool := NewFetchTool(WithAllowedDomains([]string{parsed.Hostname()}))
+
+	result, err := tool.handler.CallTool(t.Context(), FetchToolArgs{
+		URLs:   []string{url + "/start"},
+		Format: "text",
+	})
+	require.NoError(t, err)
+	assert.True(t, redirected, "the test server should have been hit at least once to issue the redirect")
+	assert.Contains(t, result.Output, "Error fetching")
+	assert.Contains(t, result.Output, "attacker.example.com", "the error should mention the rejected redirect target")
+	assert.Contains(t, result.Output, "is not in allowed_domains")
+}
+
+// TestFetch_BlockedDomains_RejectsRedirectToBlockedHost mirrors the allow-list
+// regression test for the deny-list path: a redirect to a deny-listed host
+// must not be followed.
+func TestFetch_BlockedDomains_RejectsRedirectToBlockedHost(t *testing.T) {
+	url := runHTTPServer(t, func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/robots.txt" {
+			http.NotFound(w, r)
+			return
+		}
+		http.Redirect(w, r, "http://169.254.169.254/metadata", http.StatusFound)
+	})
+
+	tool := NewFetchTool(WithBlockedDomains([]string{"169.254.169.254"}))
+
+	result, err := tool.handler.CallTool(t.Context(), FetchToolArgs{
+		URLs:   []string{url + "/innocent"},
+		Format: "text",
+	})
+	require.NoError(t, err)
+	assert.Contains(t, result.Output, "Error fetching")
+	assert.Contains(t, result.Output, "is blocked by blocked_domains")
+	assert.Contains(t, result.Output, "169.254.169.254")
+}