fetch: support wildcard and CIDR patterns in domain allow/deny lists#2602
fetch: support wildcard and CIDR patterns in domain allow/deny lists#2602dgageot wants to merge 5 commits intodocker:mainfrom
Conversation
Normalize IPv4-mapped IPv6 addresses (::ffff:a.b.c.d) to their IPv4 form before checking CIDR membership and literal IP matches. Without this, an attacker could bypass an IPv4 deny-list like '169.254.0.0/16' by using the IPv6-mapped form '::ffff:169.254.169.254'. The fix applies net.IP.To4() normalization in two places: 1. CIDR matching: ensures IPv4 CIDRs match IPv4-mapped IPv6 addresses 2. Literal IP matching: ensures '169.254.169.254' matches '::ffff:169.254.169.254' Added regression tests covering: - IPv4-mapped IPv6 matching IPv4 CIDR ranges - IPv4-mapped IPv6 matching literal IPv4 patterns - IPv4 matching IPv4-mapped IPv6 CIDR ranges (edge case)
The fetch domain matcher now normalizes IPv4-mapped IPv6 addresses (::ffff:a.b.c.d) to their IPv4 form, so a deny-list entry for '169.254.0.0/16' will correctly block '::ffff:169.254.169.254'. Update the limitations callout to clarify that IPv4-mapped IPv6 IS normalized, unlike other alternative IP encodings (decimal, hex, octal).
Add tests covering: - Port suffix handling (should not match) - IPv6 zone IDs (should not match) - Empty wildcard prefix - IPv6 case insensitivity - Bracket handling in both host and pattern These tests verify defensive behaviors and edge cases that could potentially be exploited if not handled correctly.
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The CIDR + wildcard domain matching implementation is well-structured. The IPv4-mapped IPv6 normalisation correctly closes the SSRF bypass in both the CIDR path and the string-comparison path. validateDomainPattern cleanly catches malformed CIDRs and invalid wildcard forms at config-load time, and the table-driven test coverage is thorough.
One minor inconsistency was found between the CIDR and non-CIDR normalisation paths (see inline comment). The CIDR path in the changed code uses strings.TrimSuffix(strings.Trim(host, "[]"), ".") (strips brackets and trailing dot), while the non-CIDR normalisation path uses only strings.Trim(host, "[]") (strips brackets only). The practical impact is negligible because url.Hostname() never returns a bracketed-IPv6-with-trailing-dot host, but aligning the two paths would make the code more robust and easier to reason about.
| // Normalize IPv4-mapped IPv6 addresses to their IPv4 form for string | ||
| // comparison. This ensures that "::ffff:169.254.169.254" matches a | ||
| // literal pattern "169.254.169.254" (and vice versa). | ||
| if ip := net.ParseIP(strings.Trim(host, "[]")); ip != nil { |
There was a problem hiding this comment.
[LOW] Inconsistency: non-CIDR IP normalisation path does not strip trailing dot before net.ParseIP
The CIDR path (line 331) correctly chains both bracket-stripping and trailing-dot stripping before calling net.ParseIP:
// CIDR path — correct
ipStr := strings.TrimSuffix(strings.Trim(host, "[]"), ".")
if ip := net.ParseIP(ipStr); ip != nil {The non-CIDR normalisation path (line 355) only strips brackets, not the trailing dot:
// Non-CIDR path — inconsistent
if ip := net.ParseIP(strings.Trim(host, "[]")); ip != nil {For a host like [::ffff:169.254.169.254]. (bracketed IPv6 with FQDN trailing dot), strings.Trim(host, "[]") yields ::ffff:169.254.169.254]. (the trailing . is not in the cutset "[]"), so net.ParseIP returns nil and the host falls through un-normalised to the string comparison stage. The bypass is theoretical in practice since url.Hostname() strips brackets and would not return a host in this form — but the inconsistency between the two paths in the changed code is real.
Suggested fix: Apply the same pattern as the CIDR path:
if ip := net.ParseIP(strings.TrimSuffix(strings.Trim(host, "[]"), ".")); ip != nil {
What
Extend the fetch toolset's
allowed_domains/blocked_domainswith two new pattern shapes on top of the existing bare-host and leading-dot forms:*.example.com.example.com— strict subdomain only (apex excluded).*is allowed only as the leading*.token.169.254.0.0/16,10.0.0.0/8,::1/128,fc00::/7Existing bare (
example.com) and leading-dot (.example.com) patterns are unchanged.Why
Without CIDR support, blocking the cloud-metadata range or a private network meant listing every IP individually. Without
*.glob support, users coming from TLS-cert / nginx / curl conventions had to discover the leading-dot form. Both gaps were visible in real configs — seeexamples/fetch_domain_filtering.yaml.Security fix included
While reviewing the new code I found that
matchesDomaindid not normalise IPv4-mapped IPv6 addresses before CIDR / literal-IP comparison. A URL likehttp://[::ffff:169.254.169.254]/would parse as IPv6, slip past a169.254.0.0/16deny entry, and reach the cloud metadata endpoint anyway. Fixed by canonicalising both sides vianet.IP.To4()before matching. Regression tests added.Validation up-front
validateDomainPatternnow rejects malformed CIDRs (10.0.0.0/33) and any non-leading use of*(foo.*,*.*.example.com, bare*) at config-load time, so silent foot-guns become actionable errors.Files
pkg/tools/builtin/fetch.go—matchesDomainextended (CIDR +*.glob + IPv4-mapped IPv6 normalisation).pkg/config/latest/validate.go—validateDomainPatternsyntax check.pkg/tools/builtin/fetch_test.go,pkg/config/latest/validate_test.go— table-driven coverage for matcher and validator (incl. SSRF regression cases).agent-schema.json,docs/tools/fetch/index.md,examples/fetch_domain_filtering.yaml— descriptions, examples, limitations callout.Tests
mise lint→0 issues.mise test→ all packages pass.