SSRF: fetch guard is literal-only — a hostname resolving to a blocked IP bypasses it

[spartan8806]

Hi! Really enjoying wigolo. Following the "open an issue first for non-trivial changes" guidance before proposing anything.

The gap: guardFetchUrl / guardServeTarget validate only the literal hostname — which the code already notes ("we never actually resolve here… a follow-up tier could re-check at fetch time"). Since the fetch layer resolves DNS itself, a public hostname whose A/AAAA record points at a blocked address — cloud metadata (169.254.169.254), an RFC-1918 host, or a loopback service in serve mode — passes the guard and is only reached after resolution. That defeats the "always block metadata" and serve-mode loopback protections.

Proposed approach: add guardResolvedHost(host, field, { allowPrivate, lookup }) that resolves the host and runs every resolved IP back through your existing guardFetchUrl (as an http:/// literal) — so the resolved-IP policy is identical to the literal one, no drift. Wire it into the fetch path (http-client redirect loop first; then the dispatch / firecrawl / browser seams). lookup is injectable so it's unit-testable without real DNS.

Scope: this closes the static-record bypass (the metadata case). Full rebinding (TOCTOU) safety also wants the socket pinned to the validated IP (a lookup hook / custom dispatcher) — I'd suggest the resolved-IP re-check first (small, high-value), pinning as a follow-up.

I've got a focused branch + tests ready to PR if this direction works for you — happy to adjust first. Does this approach sound right, or would you rather pin at the connector level from the start?

[KnockOutEZ]

Thanks for the writeup, and for opening the issue first. You're right, and the code says so itself: guardFetchUrl (src/watch/ssrf.ts) and guardServeTarget (src/daemon/rest/target-guard.ts) check parsed.hostname as a literal and never resolve. A public name whose A/AAAA record points at 169.254.169.254, an RFC-1918 host, or (in serve mode) loopback passes the guard and is only reached once the fetch layer resolves. It's every tool behind guardFetchUrl (fetch, crawl, extract, agent, find_similar), the whole serve surface behind guardServeTarget including the Firecrawl shim, and every redirect re-guard (http-client, router, tls-tier, escape-hatch), which are all literal too, so a 302 to a resolving name has the same hole.

The sharp edge is serve mode. Local stdio needs the agent talked into the bad URL first; a non-loopback serve bind lets a remote caller reach the box's metadata endpoint or internal services by name, which is exactly the loopback refusal guardServeTarget is supposed to enforce.

Your approach is the one I want: resolve, then run each IP back through guardFetchUrl as an http://<ip>/ literal, so the resolved-IP policy is identical to the literal one and the existing SSRF_CODES keep the REST status mapping working. Injectable lookup for tests, yes. Four things to settle in the PR:

1. Keep it in src/watch/ssrf.ts next to guardFetchUrl, so one file owns the policy.
2. Thread allowPrivate from each call site's existing opts rather than re-defaulting it, or a home user's WIGOLO_FETCH_ALLOW_PRIVATE=1 192.168 NAS (fetched by name) breaks.
3. Serve mode needs the resolved-IP version of its loopback refusal, not just the base guard, or a name resolving to 127.0.0.1 still probes local services under a non-loopback bind. For the plain fetch tool a name resolving to 127.0.0.1 staying allowed is correct (the local-dev promise); reusing guardFetchUrl gives you both behaviors for free.
4. guardResolvedHost is async now and lookup can NXDOMAIN or time out. A failed or empty resolution isn't a bypass (no IP to connect to), so I'd let it fall through to the real fetch and surface the natural DNS error instead of faking a private-block. Say what you chose so we can talk it over.

On phasing: re-check first, pin later. Agreed, and I don't want to gate this on connector-level pinning. The re-check fully closes what you reported (static records pointing at metadata, RFC-1918, or loopback). Active rebinding, where the record flips between check and connect, is the TOCTOU residual, and the only airtight fix is pinning the socket to the validated IP. Pinning is uneven across our tiers: the HTTP client tier takes a lookup/connect hook cleanly, the TLS tier only if its stack exposes one, and the browser tier is effectively unpinnable since the browser does its own DNS (best case is pre-nav resolve-and-validate, or a validating proxy). So "pin everywhere first" either stalls on the browser or ships uneven coverage. Resolve-and-validate at every seam lands uniform, testable coverage now; pinning is a per-tier follow-up where the stack allows, with the browser residual documented honestly. Please open a follow-up issue for the pin so the TOCTOU gap is on record.

Seams to wire so nothing slips: http-client (input plus each redirect hop), router, the tls-tier redirect loop, escape-hatch hops, dispatch and firecrawl-compat, and the browser seam (pre-nav, check-only, with a comment saying so).

Tests, all via injected lookup: public name to metadata IP (blocked, METADATA), to RFC-1918 (blocked; allowed under allowPrivate), to loopback (blocked under a serve non-loopback bind, allowed for plain fetch), to an ordinary public IP (allowed); a 302 whose target resolves to metadata (blocked); and a negative test that a normal public site still fetches, so the guard can't silently over-block.

PR away on the phase-1 re-check. One note: opening a PR means agreeing to the CLA in CONTRIBUTING.md (same license plus the relicense grant), flagging it so it's no surprise. Get npm test and npm run lint green first. Thanks again, this is a real one.