Skip to content

docs: document allow_private_ips option and SSRF protection in fetch tool#2833

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:board/5a7a5be7054fd72b
May 20, 2026
Merged

docs: document allow_private_ips option and SSRF protection in fetch tool#2833
dgageot merged 1 commit into
docker:mainfrom
dgageot:board/5a7a5be7054fd72b

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 20, 2026

Adds documentation for the existing allow_private_ips fetch-toolset option and clarifies how the fetch tool's default SSRF protection behaves. Reported by users hitting "connection refused" when trying to fetch localhost URLs (see #2832).

What changed

docs/tools/fetch/index.md:

  • New row in the Options table for allow_private_ips (default false), with a link to the new section.
  • New "SSRF protection and reaching localhost" section explaining:
    • which IP ranges the dial-time block refuses by default (loopback, RFC1918, link-local incl. 169.254.169.254, IPv4-mapped IPv6, multicast, unspecified);
    • that the check happens after DNS resolution, so DNS rebinding is also blocked;
    • why this is the default (LLM-driven fetches are an SSRF vector);
    • how to opt in with allow_private_ips: true, with a YAML example.
  • New "Already blocked by default" info callout next to the existing "Block sensitive hosts" example, so readers don't think that example is what protects them.
  • New "Pair with an allow-list" warning callout recommending allowed_domains whenever allow_private_ips: true is set, plus a note explaining the timing difference: allowed_domains is checked pre-DNS on the hostname, while the SSRF block runs post-DNS on the resolved IP — so a public hostname that resolves to a private IP is still refused unless allow_private_ips: true is set.

No code, schema, or example YAML changes were needed — the option already exists in pkg/tools/builtin/fetch/fetch.go (WithAllowPrivateIPs), pkg/config/latest/types.go (AllowPrivateIPs), and agent-schema.json. Only the public docs were lacking.

Closes #2832.

Validation

  • task lint — ✅ clean
  • task test — ✅ all relevant suites pass (pkg/tools/builtin/fetch, pkg/httpclient, pkg/config/latest, …); the only failures are pre-existing pkg/teamloader TestLoadExamples subtests that try to pull a real model via Docker Model Runner, unrelated to this change.

@dgageot dgageot requested a review from a team as a code owner May 20, 2026 10:09
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

Documentation PR adding allow_private_ips option docs and SSRF protection section. Overall the content is accurate and well-structured. One finding about a potentially misleading comment in the YAML example was confirmed as likely to cause user confusion.

Comment thread docs/tools/fetch/index.md
allowed_domains:
- localhost
- 127.0.0.1
- 10.0.0.0/8 # internal corporate range
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] CIDR 10.0.0.0/8 in allowed_domains only matches literal-IP URLs, not hostname-based internal services

The comment # internal corporate range implies this entry will allow access to all internal services by hostname (e.g., http://internal.corp/). However, as documented in the Domain matching section above, "Hostname hosts never match a CIDR pattern" — this CIDR only matches when the URL host is already a literal IP address, such as http://10.0.0.5/.

A user following this example to grant an agent access to their internal hostname-based services would find the CIDR silently ineffective for those URLs. Consider:

  • Replacing the comment with something like # only matches literal-IP URLs such as http://10.x.x.x/ — use hostnames for named services to set correct expectations, or
  • Adding a note that hostname-based internal services must be listed individually (e.g., - internal.corp) rather than covered by a CIDR.

@dgageot dgageot merged commit 4121f5b into docker:main May 20, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to fetch on localhost

3 participants