[DO NOT MERGE — awaiting D-1] feat(effects): M1 — request-aware proxy/direct Net transport

[sunholo-voight-kampff]

DRAFT — HELD, not ready to merge. Milestone M1 of m-net-effect-proxy-boundary, implemented to the reviewed design doc's letter. It is opened as a draft because implementing the design faithfully breaks four existing SSRF tests on CI's own poisoned-proxy legs, and the decision that resolves that (D-1) is still awaiting human ratification.

What landed

A package-private request-aware netProxyRoundTripper (internal/effects/net_proxy.go, 150 lines) with separate direct and proxy transport paths:

• no proxy → resolveAndValidateIP exactly once, dial the returned IP with no hostname re-resolution (anti-DNS-rebinding pinning preserved)
• proxy → ordinary proxy dialing, zero local target resolution/validation; the proxy address never enters the target-IP substitution closure

All three Net constructors routed through it (net.go:96/:212/:587); the three preflight resolveAndValidateIP calls (:85/:201/:565) and the redirect-validator call (:317) removed; public error categories preserved via a typed internal error unwrapped through *url.Error at both enumerated sites (:567, :631). 781 lines of tests.

Acceptance — AC-M1.1 (controller-run, then independently reproduced by the evaluator)

rc=0, === RUN count 17 (≥4), all four top-level --- PASS.
The same command on the pristine tree is rc=0 with === RUN count 0 / [no tests to run] — vacuously green — so the count assertion is the load-bearing part, not the exit code.

⚠ Why this is held — B1

HTTP_PROXY=http://127.0.0.1:9 HTTPS_PROXY=http://127.0.0.1:9 NO_PROXY=localhost,127.0.0.1 \
  go test -count=1 ./internal/effects -run TestNetIPValidation

tree	result
this branch	rc=1 — 4 of 7 subtests FAIL
pristine dev (79caa15b3)	rc=0 — 7 PASS

The only variable is this change. ci.yml sets exactly that poison on its own test legs (:89-91, :350-352), so this will red CI.

Mechanism, and it is D-1 by construction: IP validation now happens only on the direct route, so a target that is a literal private IP (https://10.0.0.1/…) is handed to the proxy unvalidated. The loopback subtests still pass precisely because NO_PROXY routes them direct — which corroborates the mechanism.

The decision, with a third option the design doc does not currently carry

resolveAndValidateIP already has a // Special case: raw IP address (skip DNS) branch that calls validateIP with zero network I/O. So for a literal-IP target there is no resolution, no TOCTOU and no rebinding risk — the rationale for skipping validation on proxied routes simply does not apply to that case.

• (a) Ratify D-1 as written, and update TestNetIPValidation to expect E_NET_REQUEST_FAILED for literal private IPs when a proxy is configured. This narrows the shipped security guarantee.
• (b) Apply the literal-IP narrowing: validate when net.ParseIP(host) != nil, even on the proxy route. Restores parity for the unambiguous case and shrinks D-1 to hostnames only.

The sprint evaluator (independent, sonnet) reviewed this at HOLD 71/100 and recommends (b).

Verification

• Controller mutation D (neuter the production http.ProxyFromEnvironment default) → gate rc=1, but reds exactly one subtest — the production default has a single point of coverage (NB1). sha256 restored byte-identical.
• Evaluator ran three further, distinct mutations (bypass direct validation; reverse routing; re-add preflight resolution to the proxy path); each reddened the expected subtests. All restorations sha256-verified.
• Gates green: scoped go build, go vet, gofmt, make check-file-sizes / check-boundaries / check-changelog / fmt-check, and go test ./internal/effects ./internal/executor/... ./internal/testutil (rc=0).
• No behavioural drift: the original transports were bare http.Transport{} with only a custom DialContext; so are the new ones. No goroutine leak measured across 20 sequential / 50 concurrent requests.

🤖 Generated with Claude Code