chore: tighten SSRF client detection with forbidgo

GitOrigin-RevId: dfeb7424d9ad1b80f9abf215fd8390ea04301f7e

Author: aeneasr

.golangci.yml

@@ -8,6 +8,7 @@ linters:
     - ineffassign
     - staticcheck
     - unused
+    - forbidigo
   disable:
     - bodyclose # too many false negatives
 
@@ -19,8 +20,20 @@ linters:
         - G306
         - G704
         - G705
+    forbidigo:
+      forbid:
+        - pattern: "http\\.DefaultClient"
+          msg: "Use reg.HTTPClient(ctx) or httpx.NewResilientClient() — http.DefaultClient bypasses SSRF protection and retry logic."
+        - pattern: "retryablehttp\\.NewClient\\b"
+          msg: "Use httpx.NewResilientClient() — retryablehttp.NewClient() bypasses SSRF protection and registry configuration."
   exclusions:
     rules:
       - linters:
           - staticcheck
         text: "SA1019" # we do use deprecated APIs on purpose sometimes
+      - linters:
+          - forbidigo
+        path: "pkg/(httpclient|client-go)/"
+      - linters:
+          - forbidigo
+        path: "_test\\.go$"

cmd/cliclient/client.go

@@ -12,13 +12,12 @@ import (
 
 	"github.com/pkg/errors"
 
-	"github.com/hashicorp/go-retryablehttp"
-
 	"github.com/spf13/cobra"
 
 	"github.com/spf13/pflag"
 
 	kratos "github.com/ory/kratos/pkg/httpclient"
+	"github.com/ory/x/httpx"
 )
 
 const (
@@ -71,8 +70,9 @@ func NewClient(cmd *cobra.Command) (*kratos.APIClient, error) {
 	}
 
 	conf := kratos.NewConfiguration()
-	conf.HTTPClient = retryablehttp.NewClient().StandardClient()
-	conf.HTTPClient.Timeout = time.Second * 10
+	conf.HTTPClient = httpx.NewResilientClient(
+		httpx.ResilientClientWithConnectionTimeout(10 * time.Second),
+	).StandardClient()
 	conf.Servers = kratos.ServerConfigurations{{URL: u.String()}}
 	return kratos.NewAPIClient(conf), nil
 }

selfservice/strategy/password/validator.go

@@ -58,7 +58,6 @@ var (
 // password has been breached in a previous data leak using k-anonymity.
 type DefaultPasswordValidator struct {
 	reg    validatorDependencies
-	Client *retryablehttp.Client
 	hashes *ristretto.Cache[string, int64]
 
 	minIdentifierPasswordDist            int
@@ -67,6 +66,7 @@ type DefaultPasswordValidator struct {
 
 type validatorDependencies interface {
 	config.Provider
+	httpx.ClientProvider
 }
 
 func NewDefaultPasswordValidatorStrategy(reg validatorDependencies) (*DefaultPasswordValidator, error) {
@@ -81,9 +81,6 @@ func NewDefaultPasswordValidatorStrategy(reg validatorDependencies) (*DefaultPas
 		return nil, errors.Wrap(err, "error while setting up validator cache")
 	}
 	return &DefaultPasswordValidator{
-		Client: httpx.NewResilientClient(
-			httpx.ResilientClientWithConnectionTimeout(time.Second),
-		),
 		reg:                       reg,
 		hashes:                    cache,
 		minIdentifierPasswordDist: 5, maxIdentifierPasswordSubstrThreshold: 0.5,
@@ -123,7 +120,7 @@ func (s *DefaultPasswordValidator) fetch(ctx context.Context, hpw []byte, apiDNS
 	if err != nil {
 		return 0, err
 	}
-	res, err := s.Client.Do(req)
+	res, err := s.reg.HTTPClient(ctx, httpx.ResilientClientWithConnectionTimeout(time.Second)).Do(req)
 	if err != nil {
 		return 0, errors.Wrapf(ErrNetworkFailure, "%s", err)
 	}

selfservice/strategy/password/validator_test.go

@@ -17,10 +17,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-retryablehttp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ory/herodot"
+	"github.com/ory/kratos/driver"
 	"github.com/ory/kratos/driver/config"
 	"github.com/ory/kratos/pkg"
 	"github.com/ory/kratos/selfservice/strategy/password"
@@ -30,6 +32,32 @@ import (
 	"github.com/ory/x/httpx"
 )
 
+// testRegistry embeds RegistryDefault and overrides HTTPClient to inject a
+// controllable HTTP client for HIBP-related tests.
+type testRegistry struct {
+	*driver.RegistryDefault
+	cl *retryablehttp.Client
+}
+
+func (r *testRegistry) HTTPClient(_ context.Context, _ ...httpx.ResilientOptions) *retryablehttp.Client {
+	return r.cl
+}
+
+// newTestValidator wires base with a fake HIBP HTTP client and returns the
+// validator along with the fake client for request assertions and stubbing.
+func newTestValidator(t *testing.T, base *driver.RegistryDefault, opts ...httpx.ResilientOptions) (*password.DefaultPasswordValidator, *fakeHttpClient) {
+	t.Helper()
+	fakeClient := NewFakeHTTPClient()
+	cl := httpx.NewResilientClient(append([]httpx.ResilientOptions{
+		httpx.ResilientClientWithMaxRetry(1),
+		httpx.ResilientClientWithConnectionTimeout(time.Millisecond),
+	}, opts...)...)
+	cl.HTTPClient = &fakeClient.Client
+	s, err := password.NewDefaultPasswordValidatorStrategy(&testRegistry{RegistryDefault: base, cl: cl})
+	require.NoError(t, err)
+	return s, fakeClient
+}
+
 func TestDefaultPasswordValidationStrategy(t *testing.T) {
 	t.Parallel()
 
@@ -93,16 +121,8 @@ func TestDefaultPasswordValidationStrategy(t *testing.T) {
 	t.Run("failure cases", func(t *testing.T) {
 		t.Parallel()
 
-		_, reg := pkg.NewFastRegistryWithMocks(t)
-		s, err := password.NewDefaultPasswordValidatorStrategy(reg)
-		require.NoError(t, err)
-
-		fakeClient := NewFakeHTTPClient()
-		s.Client = httpx.NewResilientClient(
-			httpx.ResilientClientWithMaxRetry(1),
-			httpx.ResilientClientWithConnectionTimeout(time.Millisecond),
-			httpx.ResilientClientWithMaxRetryWait(time.Millisecond))
-		s.Client.HTTPClient = &fakeClient.Client
+		_, base := pkg.NewFastRegistryWithMocks(t)
+		s, fakeClient := newTestValidator(t, base, httpx.ResilientClientWithMaxRetryWait(time.Millisecond))
 
 		t.Run("case=should send request to pwnedpasswords.com", func(t *testing.T) {
 			ctx := contextx.WithConfigValue(t.Context(), config.ViperKeyIgnoreNetworkErrors, false)
@@ -139,12 +159,10 @@ func TestDefaultPasswordValidationStrategy(t *testing.T) {
 		t.Parallel()
 
 		const maxBreaches = 5
-		_, reg := pkg.NewFastRegistryWithMocks(t, configx.WithValue(config.ViperKeyPasswordMaxBreaches, maxBreaches))
-		s, err := password.NewDefaultPasswordValidatorStrategy(reg)
-		require.NoError(t, err)
+		_, base := pkg.NewFastRegistryWithMocks(t, configx.WithValue(config.ViperKeyPasswordMaxBreaches, maxBreaches))
+		s, fakeClient := newTestValidator(t, base)
 
 		hibpResp := make(chan string, 1)
-		fakeClient := NewFakeHTTPClient()
 		fakeClient.responder = func(req *http.Request) (*http.Response, error) {
 			buffer := bytes.NewBufferString(<-hibpResp)
 			return &http.Response{
@@ -154,8 +172,6 @@ func TestDefaultPasswordValidationStrategy(t *testing.T) {
 				Request:       req,
 			}, nil
 		}
-		s.Client = httpx.NewResilientClient(httpx.ResilientClientWithMaxRetry(1), httpx.ResilientClientWithConnectionTimeout(time.Millisecond))
-		s.Client.HTTPClient = &fakeClient.Client
 
 		hashPw := func(t *testing.T, pw string) string {
 			//#nosec G401 -- sha1 is used for k-anonymity
@@ -271,13 +287,8 @@ func TestChangeHaveIBeenPwnedValidationHost(t *testing.T) {
 	testServerURL, err := url.Parse(testServer.URL)
 	require.NoError(t, err)
 
-	_, reg := pkg.NewFastRegistryWithMocks(t, configx.WithValue(config.ViperKeyPasswordHaveIBeenPwnedHost, testServerURL.Host))
-	s, err := password.NewDefaultPasswordValidatorStrategy(reg)
-	require.NoError(t, err)
-
-	fakeClient := NewFakeHTTPClient()
-	s.Client = httpx.NewResilientClient(httpx.ResilientClientWithMaxRetry(1), httpx.ResilientClientWithConnectionTimeout(time.Millisecond))
-	s.Client.HTTPClient = &fakeClient.Client
+	_, base := pkg.NewFastRegistryWithMocks(t, configx.WithValue(config.ViperKeyPasswordHaveIBeenPwnedHost, testServerURL.Host))
+	s, fakeClient := newTestValidator(t, base)
 
 	testServerExpectedCallURL := fmt.Sprintf("https://%s/range/BCBA9", testServerURL.Host)
 
@@ -291,13 +302,8 @@ func TestChangeHaveIBeenPwnedValidationHost(t *testing.T) {
 func TestDisableHaveIBeenPwnedValidationHost(t *testing.T) {
 	t.Parallel()
 
-	_, reg := pkg.NewFastRegistryWithMocks(t, configx.WithValue(config.ViperKeyPasswordHaveIBeenPwnedEnabled, false))
-	s, err := password.NewDefaultPasswordValidatorStrategy(reg)
-	require.NoError(t, err)
-
-	fakeClient := NewFakeHTTPClient()
-	s.Client = httpx.NewResilientClient(httpx.ResilientClientWithMaxRetry(1), httpx.ResilientClientWithConnectionTimeout(time.Millisecond))
-	s.Client.HTTPClient = &fakeClient.Client
+	_, base := pkg.NewFastRegistryWithMocks(t, configx.WithValue(config.ViperKeyPasswordHaveIBeenPwnedEnabled, false))
+	s, fakeClient := newTestValidator(t, base)
 
 	t.Run("case=should not send request to test server", func(t *testing.T) {
 		require.NoError(t, s.Validate(t.Context(), "mohutdesub", "damrumukuh"))