From 4e8fb16fc274420194d630dea484efd23f960f4b Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Wed, 13 May 2026 01:30:47 +0000 Subject: [PATCH] =?UTF-8?q?fix(oidc):=20test=20seam=20for=20jwksProbeClien?= =?UTF-8?q?t=20=E2=80=94=20closes=20the=20B5=20R6=20httptest=20regression?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI break diagnosed from go-build-and-test on 47da13e+596e675: TestTestDiscovery_HappyPath_AgainstMockIdP + TestTestDiscovery_JWKSFetchFails fail with "refusing to dial reserved address 127.0.0.1" because my Bundle 5 R6 closure wrapped jwksReachable in validation.SafeHTTPDialContext — which is exactly what the production guard is supposed to refuse for httptest.NewServer's 127.0.0.1 bind. Same shape as the Slack/Teams test-seam fix in 596e675: factor the http.Client construction into a package-level var (`jwksProbeClient`), default to the SSRF-safe transport in production, override to http.DefaultTransport in test-only `setup_test.go::init()`. Production code never reassigns the var. The audit R6 closure stands — the production jwksReachable still uses validation.SafeHTTPDialContext. Verification (sandbox, Go 1.25.10): go test -short -count=1 -run 'TestTestDiscovery_HappyPath|TestTestDiscovery_JWKSFetchFails' ./internal/auth/oidc # PASS (1.1s) go test -short -count=1 ./internal/auth/oidc # PASS (21.8s) gofmt -l # clean go vet ./internal/auth/oidc # clean --- internal/auth/oidc/setup_test.go | 32 ++++++++++++++++++++++++++++ internal/auth/oidc/test_discovery.go | 30 ++++++++++++++++---------- 2 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 internal/auth/oidc/setup_test.go diff --git a/internal/auth/oidc/setup_test.go b/internal/auth/oidc/setup_test.go new file mode 100644 index 0000000..79cb9fe --- /dev/null +++ b/internal/auth/oidc/setup_test.go @@ -0,0 +1,32 @@ +// Test-only setup for the internal/auth/oidc package. +// +// Bundle 5 closure (audit R6) wrapped the package's jwks reachability +// probe in validation.SafeHTTPDialContext so production OIDC config +// dry-runs can't be pivoted into reserved-address ranges via DNS +// rebinding. The test suite uses httptest.NewServer which binds to +// 127.0.0.1 — that's exactly the reserved-address case the production +// guard refuses to dial, so the package-level jwksProbeClient is +// replaced here with an SSRF-guard-bypassed http.Client for the +// duration of every test in this package. +// +// Mirrors the internal/connector/notifier/webhook + slack + teams +// test-seam pattern (newForTest constructor). The production code +// never reassigns jwksProbeClient — only this _test.go file does, so +// the test seam can't leak into a real deployment. + +package oidc + +import ( + "net/http" + "time" +) + +func init() { + // Replace the SSRF-safe transport with one that has no + // DialContext override. http.DefaultTransport handles 127.0.0.1 + // without complaint, which is what httptest.NewServer needs. + jwksProbeClient = &http.Client{ + Timeout: 10 * time.Second, + Transport: http.DefaultTransport, + } +} diff --git a/internal/auth/oidc/test_discovery.go b/internal/auth/oidc/test_discovery.go index 14e2e5a..454c2f2 100644 --- a/internal/auth/oidc/test_discovery.go +++ b/internal/auth/oidc/test_discovery.go @@ -157,22 +157,30 @@ func (s *Service) TestDiscovery(ctx context.Context, issuerURL string) (*TestDis // 10-second timeout matches the package-wide oidcOutboundTimeout // budget so token endpoint + JWKS + userinfo probes share the same // wall-clock horizon. +// jwksProbeClient is the *http.Client used by jwksReachable. Package- +// level var so the test suite can swap it for an SSRF-guard-bypassed +// client when exercising jwksReachable against httptest.NewServer +// (which binds to 127.0.0.1 and would otherwise be refused by +// validation.SafeHTTPDialContext). Mirrors the +// internal/connector/notifier/webhook + slack + teams test-seam +// pattern. Production code never reassigns this var. +var jwksProbeClient = &http.Client{ + Timeout: oidcOutboundTimeout, + Transport: &http.Transport{ + DialContext: validation.SafeHTTPDialContext(oidcOutboundTimeout), + MaxIdleConns: 10, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + }, +} + var jwksReachable = func(ctx context.Context, jwksURI string) (bool, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, jwksURI, nil) if err != nil { return false, err } - client := &http.Client{ - Timeout: oidcOutboundTimeout, - Transport: &http.Transport{ - DialContext: validation.SafeHTTPDialContext(oidcOutboundTimeout), - MaxIdleConns: 10, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - }, - } - resp, err := client.Do(req) + resp, err := jwksProbeClient.Do(req) if err != nil { return false, err }