docs: document allow_private_ips option and SSRF protection in fetch tool#2833
Conversation
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
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.
| allowed_domains: | ||
| - localhost | ||
| - 127.0.0.1 | ||
| - 10.0.0.0/8 # internal corporate range |
There was a problem hiding this comment.
[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 servicesto 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.
aheritier
added
area/tools
3 participants
Adds documentation for the existing
allow_private_ipsfetch-toolset option and clarifies how the fetch tool's default SSRF protection behaves. Reported by users hitting "connection refused" when trying to fetchlocalhostURLs (see #2832).What changed
docs/tools/fetch/index.md:allow_private_ips(defaultfalse), with a link to the new section.169.254.169.254, IPv4-mapped IPv6, multicast, unspecified);allow_private_ips: true, with a YAML example.allowed_domainswheneverallow_private_ips: trueis set, plus a note explaining the timing difference:allowed_domainsis 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 unlessallow_private_ips: trueis 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), andagent-schema.json. Only the public docs were lacking.Closes #2832.
Validation
task lint— ✅ cleantask test— ✅ all relevant suites pass (pkg/tools/builtin/fetch,pkg/httpclient,pkg/config/latest, …); the only failures are pre-existingpkg/teamloaderTestLoadExamplessubtests that try to pull a real model via Docker Model Runner, unrelated to this change.