diff --git a/AGENTS.md b/AGENTS.md index b77c7a83e..d587d8230 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,7 @@ Detailed guides for specific subsystems live in `contrib/claude/`: - [`contrib/claude/go-testing.md`](contrib/claude/go-testing.md) — Go test conventions (parallel, require vs assert, naming) - [`contrib/claude/go-service.md`](contrib/claude/go-service.md) — Go service orchestration (Run, graceful shutdown, crash propagation) - [`contrib/claude/go-worker.md`](contrib/claude/go-worker.md) — Go worker pattern (poll-based, bounded concurrency, FOR UPDATE SKIP LOCKED) +- [`contrib/claude/httpclient.md`](contrib/claude/httpclient.md) — HTTP client (kit/httpclient, SSRF protection by default, connector wiring) - [`contrib/claude/gid.md`](contrib/claude/gid.md) — Global identifiers (GID layout, TenantID, entity type registry) - [`contrib/claude/coredata.md`](contrib/claude/coredata.md) — Data access layer (Scoper, SQL patterns, filters, order fields, migrations) - [`contrib/claude/logging.md`](contrib/claude/logging.md) — Structured logging (PII-free rules, field helpers, logger wiring) diff --git a/contrib/claude/httpclient.md b/contrib/claude/httpclient.md new file mode 100644 index 000000000..0801acb30 --- /dev/null +++ b/contrib/claude/httpclient.md @@ -0,0 +1,42 @@ +# HTTP Client + +Use `go.gearno.de/kit/httpclient` for every outbound HTTP call. Never use `http.DefaultClient` or a bare `&http.Client{}`. + +## SSRF protection is the default + +Every call goes through `httpclient.DefaultClient(...)` / `httpclient.DefaultPooledClient(...)` / `httpclient.DefaultPooledTransport(...)` with `httpclient.WithSSRFProtection()` enabled. This applies whenever the destination URL is: + +- Customer-supplied (webhook endpoint, OAuth2 token URL, SCIM bridge URL, connector-provided base URL) +- Reached through a customer-supplied connector (OAuth2/APIKey connection clients) +- A hardcoded third-party provider host (Slack, Linear, GitHub, Google Workspace, Sentry, …) — defense in depth, and public IPs pass the check unchanged + +```go +client := httpclient.DefaultPooledClient( + httpclient.WithLogger(logger), + httpclient.WithSSRFProtection(), +) +``` + +What the option does: + +- Rejects dials to loopback, RFC 1918 private, RFC 6598 CGNAT, link-local, multicast, unspecified, ULA, IPv4-mapped IPv6, and IETF-reserved ranges. Check runs on the resolved peer IP at connect time, defeating DNS rebinding. +- On `DefaultClient` / `DefaultPooledClient`, also refuses redirects whose scheme, host, or port differs from the original. + +## When to omit SSRF protection + +Only when the target is an **internal service you actively intend to reach** (sandbox-local service, sidecar, in-cluster endpoint with a known private IP). These cases are rare in this codebase — confirm the intent in code review. Do not disable it "just to make a test pass." + +For tests that need to hit an `httptest` server on loopback, add `httpclient.WithSSRFAllowLoopback()` on top of `WithSSRFProtection()` (or inject a loopback-friendly client into the component under test). Production callers must not use the loopback exemption. + +## Connector wiring + +`OAuth2Connector.HTTPClient` is a required field for the token-exchange request. Set it via `connector.ApplyProviderDefaults`, which wires an SSRF-protected client. Don't re-introduce a nil fallback in `CompleteWithState` — callers must provide the client explicitly. + +`OAuth2Connection.ClientWithOptions`, `RefreshableClient`, `clientCredentialsClient` and `APIKeyConnection.Client` already append `WithSSRFProtection()` internally; additional caller options layer on top. + +## Summary + +- Customer-reachable URL → `WithSSRFProtection()`, always. +- Third-party SaaS → `WithSSRFProtection()`, always (public IPs pass). +- Local/internal service you meant to dial → document why and omit. +- Test against `httptest` → `WithSSRFProtection()` + `WithSSRFAllowLoopback()`. diff --git a/go.mod b/go.mod index d0bc7b180..6da4b1415 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/vikstrous/dataloadgen v0.0.10 github.com/yuin/goldmark v1.4.13 go.gearno.de/crypto/uuid v0.1.1-0.20251208105319-3f587312a712 - go.gearno.de/kit v0.5.0 + go.gearno.de/kit v0.6.0 go.gearno.de/x/ref v0.0.0-20260216110753-a700c951377c go.opentelemetry.io/otel v1.43.0 go.opentelemetry.io/otel/trace v1.43.0 diff --git a/go.sum b/go.sum index bc0a1737c..96c99da6b 100644 --- a/go.sum +++ b/go.sum @@ -334,8 +334,8 @@ github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= go.gearno.de/crypto/uuid v0.1.1-0.20251208105319-3f587312a712 h1:J5ccbcxFuwxe6Oa9fVi9FqQOo+n17ni4wbl9t4NuEzc= go.gearno.de/crypto/uuid v0.1.1-0.20251208105319-3f587312a712/go.mod h1:fnIIvKO9QnsyLO3ZJLJT3r8KZv/p0FOeT5eZKilYWXg= -go.gearno.de/kit v0.5.0 h1:rtxlR3LPi7Cq0SZurOuv+OgTVZ2/PSTG6PVtlMAndoM= -go.gearno.de/kit v0.5.0/go.mod h1:jWrI/mxd0F4GZApL0HgMextcEQoiy2YA1JVamSA/G0E= +go.gearno.de/kit v0.6.0 h1:2O7Gdi8DCt6sWUKXDq+SJPHjNnV7N+pYHpsBMhfFqmU= +go.gearno.de/kit v0.6.0/go.mod h1:jWrI/mxd0F4GZApL0HgMextcEQoiy2YA1JVamSA/G0E= go.gearno.de/x/panicf v0.1.1 h1:E3Cr9NB8Ry2EsvEG/1eHr7kplP3tEjTf5d56dTX64VQ= go.gearno.de/x/panicf v0.1.1/go.mod h1:VnB8oF0UefMZcYeD4v+Wk4U5Z1uza7PHLlhT2CbNEbU= go.gearno.de/x/ref v0.0.0-20260216110753-a700c951377c h1:rIVWwnNxHYu9aZhHkptXlNYTBJbY4ccaIAYjztVeaDc= diff --git a/pkg/connector/apikey.go b/pkg/connector/apikey.go index 8f8669566..6deb35c71 100644 --- a/pkg/connector/apikey.go +++ b/pkg/connector/apikey.go @@ -40,7 +40,7 @@ func (c *APIKeyConnection) Client(ctx context.Context) (*http.Client, error) { transport := &oauth2Transport{ token: c.APIKey, tokenType: "Bearer", - underlying: httpclient.DefaultPooledTransport(), + underlying: httpclient.DefaultPooledTransport(httpclient.WithSSRFProtection()), } return &http.Client{Transport: transport}, nil } diff --git a/pkg/connector/oauth2.go b/pkg/connector/oauth2.go index e3f10a311..c575d0904 100644 --- a/pkg/connector/oauth2.go +++ b/pkg/connector/oauth2.go @@ -49,6 +49,12 @@ type ( ExtraAuthParams map[string]string // Optional: extra params for auth URL (e.g., access_type=offline for Google) TokenEndpointAuth string // "post-form" (default), "basic-form", or "basic-json" SupportsIncrementalAuth bool + + // HTTPClient is used for the OAuth2 token-exchange request + // issued from CompleteWithState. It must be set by callers; + // ApplyProviderDefaults assigns an SSRF-protected client for + // production use. Tests may inject a loopback-friendly one. + HTTPClient *http.Client } OAuth2State struct { @@ -208,7 +214,7 @@ func (c *OAuth2Connector) CompleteWithState(ctx context.Context, r *http.Request return nil, nil, err } - tokenResp, err := http.DefaultClient.Do(tokenRequest) + tokenResp, err := c.HTTPClient.Do(tokenRequest) if err != nil { return nil, nil, fmt.Errorf("cannot post token URL: %w", err) } @@ -363,7 +369,14 @@ func (c *OAuth2Connection) Client(ctx context.Context) (*http.Client, error) { // ClientWithOptions returns an HTTP client with the given options. // Use this to add logging and tracing to the HTTP client. +// +// SSRF protection is always enabled: the underlying connector URL +// (for example a 1Password SCIM bridge URL) is customer-supplied, +// so dials to private, loopback, or other reserved address ranges +// are refused. Hardcoded provider hosts on public IPs are +// unaffected. func (c *OAuth2Connection) ClientWithOptions(ctx context.Context, opts ...httpclient.Option) (*http.Client, error) { + opts = append(opts, httpclient.WithSSRFProtection()) transport := &oauth2Transport{ token: c.AccessToken, tokenType: c.TokenType, @@ -389,6 +402,11 @@ func (c *OAuth2Connection) RefreshableClient(ctx context.Context, cfg OAuth2Refr return c.ClientWithOptions(ctx, opts...) } + // All HTTP traffic on this path (token refresh + API calls) + // must reject private/loopback/reserved peer IPs because the + // configured TokenURL or API host can be customer-influenced. + opts = append(opts, httpclient.WithSSRFProtection()) + // Determine auth style based on TokenEndpointAuth authStyle := oauth2.AuthStyleInParams switch cfg.TokenEndpointAuth { @@ -462,6 +480,10 @@ func (c *OAuth2Connection) clientCredentialsClient(ctx context.Context, opts ... return c.ClientWithOptions(ctx, opts...) } + // TokenURL is stored from customer-supplied connector settings; + // reject dials to private/loopback/reserved peer IPs. + opts = append(opts, httpclient.WithSSRFProtection()) + formData := url.Values{} formData.Set("grant_type", "client_credentials") if c.Scope != "" { diff --git a/pkg/connector/oauth2_test.go b/pkg/connector/oauth2_test.go index add85517b..044954781 100644 --- a/pkg/connector/oauth2_test.go +++ b/pkg/connector/oauth2_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.gearno.de/kit/httpclient" "go.probo.inc/probo/pkg/gid" "go.probo.inc/probo/pkg/statelesstoken" ) @@ -228,7 +229,9 @@ func TestClientCredentialsClient(t *testing.T) { TokenURL: server.URL, } - client, err := conn.clientCredentialsClient(context.Background()) + // httptest binds to loopback, which the SSRF-protected default + // transport refuses; relax just for this test. + client, err := conn.clientCredentialsClient(context.Background(), httpclient.WithSSRFAllowLoopback()) require.NoError(t, err) require.NotNil(t, client) @@ -518,6 +521,9 @@ func TestCompleteWithState_ScopeFallback(t *testing.T) { RedirectURI: "https://example.com/cb", AuthURL: "https://provider.example.com/authorize", TokenURL: server.URL, + // httptest binds to loopback, which the SSRF-protected + // default client refuses; inject a permissive client. + HTTPClient: httpclient.DefaultClient(httpclient.WithSSRFProtection(), httpclient.WithSSRFAllowLoopback()), } orgID := gid.New(gid.NewTenantID(), 0) diff --git a/pkg/connector/providers.go b/pkg/connector/providers.go index 246f3db24..784dfd48d 100644 --- a/pkg/connector/providers.go +++ b/pkg/connector/providers.go @@ -14,6 +14,8 @@ package connector +import "go.gearno.de/kit/httpclient" + // CallbackPath is the HTTP path for the OAuth2 callback endpoint. const CallbackPath = "/api/console/v1/connectors/complete" @@ -87,9 +89,11 @@ var ( // ApplyProviderDefaults sets the redirect URI and applies static provider // defaults (auth URL, token URL, extra params, token endpoint auth) onto -// an OAuth2Connector. Call this before registering the connector. +// an OAuth2Connector, and wires an SSRF-protected HTTP client for the +// token exchange request. Call this before registering the connector. func ApplyProviderDefaults(provider string, redirectURI string, c *OAuth2Connector) { c.RedirectURI = redirectURI + c.HTTPClient = httpclient.DefaultClient(httpclient.WithSSRFProtection()) if def, ok := providerDefinitions[provider]; ok { c.AuthURL = def.AuthURL diff --git a/pkg/webhook/sender.go b/pkg/webhook/sender.go index 0f298467f..6176f4a71 100644 --- a/pkg/webhook/sender.go +++ b/pkg/webhook/sender.go @@ -88,7 +88,7 @@ func NewSender(pg *pg.Client, logger *log.Logger, cfg Config) *Sender { return &Sender{ pg: pg, logger: logger, - httpClient: httpclient.DefaultPooledClient(httpclient.WithLogger(logger)), + httpClient: httpclient.DefaultPooledClient(httpclient.WithLogger(logger), httpclient.WithSSRFProtection()), encryptionKey: cfg.EncryptionKey, host: cfg.Host, cacheCreatedAt: time.Now(),