diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-SUMMARY.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-SUMMARY.md new file mode 100644 index 0000000..4c1f082 --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-SUMMARY.md @@ -0,0 +1,163 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 04 +subsystem: api +tags: [ssrf, fetchguard, https, dial-control, netip, httptest] + +requires: + - phase: 01-framework-kernel-foundation + provides: compass.Config Lookup/Open layered YAML +provides: + - fetchguard.Fetch SSRF-guarded HTTPS GET with typed Reason errors + - fetchguard.Policy AllowHostsMode and PublicOnlyMode + - Dial-time private/reserved/CGNAT IP block via net.Dialer.Control + - Streaming io.LimitReader byte cap and no-follow redirects +affects: + - phase-12-manual-cover-url + - phase-14-discogs-cover-import + +tech-stack: + added: [] + patterns: + - Dial-time SSRF check on net.Dialer.Control after Addr.Unmap + - Host allow-list is exact or dotted-suffix ("." + allowed), never a raw suffix + - http.ErrUseLastResponse returns 3xx as Result; caller re-invokes Fetch to follow + - io.LimitReader(body, maxBytes+1) for streaming too_large, never post-hoc len() + +key-files: + created: + - fetchguard/fetch.go + - fetchguard/policy.go + - fetchguard/ip.go + - fetchguard/fetch_test.go + - fetchguard/ip_test.go + modified: [] + +key-decisions: + - "Redirects are not errors: Fetch returns the 3xx Result via http.ErrUseLastResponse so the caller must re-invoke Fetch to follow Location (T-06-17)" + - "isReservedOrPrivate does not Unmap; fetch.go's dial hook Unmaps before classifying, closing the v4-mapped-v6 metadata bypass" + - "Guarded Transport disables HTTP_PROXY so Control sees the target address, not a proxy" + - "Unexported Policy.tlsConfig and skipReservedCheck exist only so httptest TLS listeners on 127.0.0.1 can exercise redirect/byte-cap paths; production callers leave both unset" + +patterns-established: + - "SSRF outbound fetch lives in framework package fetchguard; app call sites (ManualCoverUrlFetcher, CoverImporter) arrive in later phases (D-13)" + - "Failure reasons are a closed *Error.Reason set matching PHP: invalid_url, scheme, unresolvable, private_ip, network_error, too_large" + +requirements-completed: [HTTP-07] + +duration: 8min +completed: 2026-09-19 +--- + +# Phase 6 Plan 04: SSRF-guarded outbound fetch helper Summary + +**Dial-time SSRF-guarded `fetchguard.Fetch` with AllowHosts/PublicOnly modes, PHP CIDR table, streaming byte cap, and typed failure reasons — helper and tests only** + +## Performance + +- **Duration:** 8 min +- **Started:** 2026-09-19T16:54:42Z +- **Completed:** 2026-09-19T17:02:28Z +- **Tasks:** 2 +- **Files modified:** 5 + +## Accomplishments + +- Ported `ManualCoverUrlFetcher.php` `PRIVATE_V4_CIDRS` / `PRIVATE_V6_PREFIXES` onto `netip.Prefix`, including CGNAT `100.64.0.0/10` and cloud-metadata `169.254.0.0/16`, with Unmap documented as the dial-hook caller's job. +- Shipped `fetchguard.Fetch`: https-only, host allow-list (exact + dotted-suffix so `evil-discogs.com` cannot match `discogs.com`), always-on private-IP block at `net.Dialer.Control` (closes PHP's admitted DNS-rebinding TOCTOU), no automatic redirects, streaming `io.LimitReader` cap, config defaults 10 MiB / 10s with explicit zero/negative as an error (D-14). +- Zero production call sites — no `ManualCoverUrlFetcher` or `CoverImporter` wiring (D-13). + +## Task Commits + +Each task was committed atomically (TDD RED then GREEN): + +1. **Task 1: Private/reserved IP classification table** + - `f7e7ca7` test(06-04): add failing test for private/reserved IP table + - `d3630e7` feat(06-04): implement private/reserved IP classification table +2. **Task 2: Policy, defaults, and the dial-time-guarded Fetch entry point** + - `bae9de8` test(06-04): add failing tests for SSRF-guarded Fetch + - `7923fe8` feat(06-04): implement dial-time SSRF-guarded Fetch + +**Plan metadata:** (this commit) docs(06-04): complete SSRF-guarded fetch helper plan + +## Files Created/Modified + +- `fetchguard/ip.go` — `isReservedOrPrivate` CIDR table (PHP constants, fail-closed on invalid addr) +- `fetchguard/ip_test.go` — table-driven cases including Unmap, CGNAT, metadata, 172.16/12 boundaries +- `fetchguard/policy.go` — Mode, Reason, Policy, Error, Defaults, DefaultsFromConfig via Lookup +- `fetchguard/fetch.go` — `Fetch` with dial Control, ErrUseLastResponse, LimitReader, typed errors +- `fetchguard/fetch_test.go` — httptest TLS cases for scheme/host/private_ip/redirect/too_large/config + +## Decisions Made + +- Pin redirect behavior to `http.ErrUseLastResponse`: a 3xx is a non-error `Result` so following Location requires a new `Fetch` through the same guard (T-06-17). +- `isReservedOrPrivate` classifies an already-Unmap()'d `netip.Addr`; the dial hook Unmaps, matching Pitfall 6. +- Disable `HTTP_PROXY` on the guarded transport so Control observes the target IP, not a proxy hop. +- Unexported `Policy.tlsConfig` / `skipReservedCheck` let tests talk to loopback httptest servers without weakening production checks. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] Unexported test-only Policy fields for httptest loopback** +- **Found during:** Task 2 (Fetch entry point) +- **Issue:** httptest TLS servers bind 127.0.0.1, which the production dial-time private-IP block correctly rejects. Redirect and streaming-cap tests need a real connection to that listener; mocking `http.RoundTripper` would skip the thing under test for private_ip but would also skip LimitReader/CheckRedirect. +- **Fix:** Unexported `Policy.tlsConfig` and `skipReservedCheck`, set only from `fetch_test.go` via `withTestLoopback`. Private-IP tests leave both unset and assert `ReasonPrivateIP` against the real Control hook. +- **Files modified:** `fetchguard/policy.go`, `fetchguard/fetch.go`, `fetchguard/fetch_test.go` +- **Verification:** `go test ./fetchguard/... -race` — private_ip subtests fail closed; redirect/too_large pass against real TLS listeners +- **Committed in:** `7923fe8` (Task 2 GREEN) + +**2. [Rule 2 - Missing Critical] Disable HTTP_PROXY on the guarded transport** +- **Found during:** Task 2 (Fetch entry point) +- **Issue:** Honoring `HTTP_PROXY` would make Control see the proxy address, not the user-supplied target — an SSRF bypass. +- **Fix:** Set `Transport.Proxy` to nil (no proxy) on the per-call client. +- **Files modified:** `fetchguard/fetch.go` +- **Verification:** private_ip tests still hit Control on 127.0.0.1; `go vet ./...` clean +- **Committed in:** `7923fe8` (Task 2 GREEN) + +--- + +**Total deviations:** 2 auto-fixed (1 blocking, 1 missing-critical) +**Impact on plan:** Both required for correctness/security of the helper and its httptest proof. No call sites added. No scope creep. + +## Issues Encountered + +Grok worktree clone had `.git` as a directory and HEAD on `master`. Created `worktree-agent-01a0ba96` from `ecab09c` without rewinding `master`, then committed only on that branch. + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +- `fetchguard.Fetch` is ready for Phase 12 `ManualCoverUrlFetcher` (PublicOnlyMode) and Phase 14 Discogs `CoverImporter` (AllowHostsMode) call sites. +- Callers must pass an already-resolved `Policy` (or nil cfg to use 10 MiB / 10s). Following a 3xx means a new `Fetch` of Location. +- No blockers. + +## TDD Gate Compliance + +Per-task RED then GREEN commits are present: + +1. `f7e7ca7` test(06-04) (RED Task 1) +2. `d3630e7` feat(06-04) (GREEN Task 1) +3. `bae9de8` test(06-04) (RED Task 2) +4. `7923fe8` feat(06-04) (GREEN Task 2) + +## Self-Check: PASSED + +- FOUND: `fetchguard/ip.go` +- FOUND: `fetchguard/ip_test.go` +- FOUND: `fetchguard/policy.go` +- FOUND: `fetchguard/fetch.go` +- FOUND: `fetchguard/fetch_test.go` +- FOUND: `f7e7ca7` +- FOUND: `d3630e7` +- FOUND: `bae9de8` +- FOUND: `7923fe8` +- `go vet ./...` clean +- `go test ./fetchguard/... -race` pass +- `go test ./... -short` pass + +--- +*Phase: 06-http-routing-auth-groups-and-rate-limiting* +*Completed: 2026-09-19*