SSRF-safe recipe fetcher #65

Member

Maestro task: f4414e41-bd99-498d-a613-a90053963249 (Feature 10 - Recipe Grounding and Photos).

Done-condition

internal/recipes Fetcher retrieves https only; resolves and dials only public addresses (refusing loopback, RFC1918, link-local, CGNAT 100.64/10, multicast, unspecified, IPv6 ULA/link-local and IPv4-mapped equivalents) via a custom DialContext so the checked IP is the dialed IP; <=3 redirects re-checked per hop; 2 MB body cap; 10 s timeout; descriptive User-Agent; per-host rate limit; URL-keyed response cache. Stub-resolver/httptest TLS tests prove each blocked range, redirect-to-private refusal, DNS switch between lookups, the size cap, the timeout and http:// rejection. Go quality floor passes.

Evidence

  • New internal/recipes fetcher + tests; docs/ARCHITECTURE.md updated.
  • gofmt/vet/go test -race/build pass; recipes tests run 10x under -race; no new dependency.
Maestro task: f4414e41-bd99-498d-a613-a90053963249 (Feature 10 - Recipe Grounding and Photos). ## Done-condition internal/recipes Fetcher retrieves https only; resolves and dials only public addresses (refusing loopback, RFC1918, link-local, CGNAT 100.64/10, multicast, unspecified, IPv6 ULA/link-local and IPv4-mapped equivalents) via a custom DialContext so the checked IP is the dialed IP; <=3 redirects re-checked per hop; 2 MB body cap; 10 s timeout; descriptive User-Agent; per-host rate limit; URL-keyed response cache. Stub-resolver/httptest TLS tests prove each blocked range, redirect-to-private refusal, DNS switch between lookups, the size cap, the timeout and http:// rejection. Go quality floor passes. ## Evidence - New internal/recipes fetcher + tests; docs/ARCHITECTURE.md updated. - gofmt/vet/go test -race/build pass; recipes tests run 10x under -race; no new dependency.
dev_agent added 1 commit 2026-10-05 11:53:19 -04:00
Add SSRF-safe recipe fetcher
CI / ci (push) Successful in 1m36s
CI / ci (pull_request) Successful in 1m35s
e7dbcf3d2c
internal/recipes.Fetcher fetches https:// URLs only. Its DialContext
resolves the host once, refuses unless every answer is public (loopback,
RFC 1918, link-local, CGNAT 100.64/10, multicast, unspecified, reserved,
IPv6 ULA/link-local/multicast and IPv4-mapped or -embedding forms are
blocked) and dials the address it checked, so a rebinding DNS answer
cannot swap in a private IP. Keep-alives and environment proxies are off,
so every request and redirect hop dials and checks afresh.

At most 3 redirects, each re-checked for scheme and address; bodies over
2 MB are an error rather than truncated; 10 s for the whole fetch; a
descriptive User-Agent; requests to one host at least 1 s apart; 2xx
pages cached by URL for an hour.

Tests use a stub resolver and httptest TLS servers.

Maestro task: f4414e41-bd99-498d-a613-a90053963249

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
overseer added 1 commit 2026-10-05 11:53:20 -04:00
Add SSRF-safe recipe fetcher
CI / ci (push) Successful in 1m36s
CI / ci (pull_request) Successful in 1m35s
e7dbcf3d2c
internal/recipes.Fetcher fetches https:// URLs only. Its DialContext
resolves the host once, refuses unless every answer is public (loopback,
RFC 1918, link-local, CGNAT 100.64/10, multicast, unspecified, reserved,
IPv6 ULA/link-local/multicast and IPv4-mapped or -embedding forms are
blocked) and dials the address it checked, so a rebinding DNS answer
cannot swap in a private IP. Keep-alives and environment proxies are off,
so every request and redirect hop dials and checks afresh.

At most 3 redirects, each re-checked for scheme and address; bodies over
2 MB are an error rather than truncated; 10 s for the whole fetch; a
descriptive User-Agent; requests to one host at least 1 s apart; 2xx
pages cached by URL for an hour.

Tests use a stub resolver and httptest TLS servers.

Maestro task: f4414e41-bd99-498d-a613-a90053963249

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent requested changes 2026-10-05 11:58:20 -04:00
Dismissed
review_agent left a comment
Member

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.
overseer added 1 commit 2026-10-05 12:04:29 -04:00
Harden recipe fetcher: public-only check, cache copies, rate-limit slots
CI / ci (push) Successful in 1m35s
CI / ci (pull_request) Successful in 1m33s
177b2dd2e7
- IsPublic now refuses every non-globally-reachable IPv4 special-purpose
  range (adds 192.0.0.0/24, TEST-NET-1/2/3, 192.88.99.0/24, benchmarking
  198.18.0.0/15) and admits IPv6 only from global unicast 2000::/3, minus
  its documentation, 6to4, Teredo and IETF protocol-assignment ranges.
  Test fixtures move off TEST-NET-3 to public addresses; Fetch-level tests
  cover the new blocked ranges and the public IPv6 path.
- The cache stores and returns copies of Page.Body, so callers cannot
  alter cached bytes or race with other readers.
- The per-host rate limiter advances only when a request is let through;
  a wait that ends with its context holds no slot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent requested changes 2026-10-05 12:08:32 -04:00
Dismissed
review_agent left a comment
Member

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.
overseer added 1 commit 2026-10-05 12:15:15 -04:00
Hand back rate-limit slots of requests that are never sent
CI / ci (push) Successful in 1m36s
CI / ci (pull_request) Successful in 1m36s
b013b542e3
waitTurn now checks the context before taking a host's slot, so a request
canceled before Fetch takes none. The request let through takes the slot
and gets a release; limitedTransport calls it when the round trip fails
before the request headers are written (traced with httptrace), restoring
the host's previous send time and waking requests waiting for their turn.

Adds TestFetchUnsentRequestHoldsNoSlot (canceled before Fetch and failed
dial on a host with no previous request; the next request does not wait)
and TestReleaseWakesWaiter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent requested changes 2026-10-05 12:21:50 -04:00
Dismissed
review_agent left a comment
Member

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.

Independent Codex gpt-6-sol review: CHANGES_REQUIRED for this exact commit.
overseer added 1 commit 2026-10-05 12:29:57 -04:00
Rate-limit recipe fetches when the request is written
CI / ci (push) Successful in 1m33s
CI / ci (pull_request) Successful in 1m34s
317e69c6ce
The per-host slot was taken in RoundTrip, before the transport resolved,
dialed and completed TLS, so a request stalled in setup could be sent
less than HostInterval after a later request to the same host.

The transport now dials through dialTLS, which resolves, dials and
handshakes under the request's context and returns a turnConn. The
turnConn's first Write, which is the start of the request, waits for the
host's slot under the Fetcher's mutex; a first Write that sends nothing
hands the slot back. ALPN offers only HTTP/1.1 and keep-alives stay off,
so each connection carries exactly one request.

TestFetchRateLimitsSendsNotSetup stalls one request in DNS while a
second is sent, and checks the sends are a full interval apart on a fake
clock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review_agent left a comment
Member

Independent Codex gpt-6-sol review: APPROVED for this exact commit.

Independent Codex gpt-6-sol review: APPROVED for this exact commit.
approver_agent merged commit 24eee8bc61 into feature/cebed295-8c05-453b-807b-8078dcd8d250-recipe-grounding 2026-10-05 12:38:58 -04:00
Sign in to join this conversation.
No Reviewers
No labels
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#65