diff --git a/pkg/custom_detectors/custom_detectors.go b/pkg/custom_detectors/custom_detectors.go index 0d11996b0..3e866c59d 100644 --- a/pkg/custom_detectors/custom_detectors.go +++ b/pkg/custom_detectors/custom_detectors.go @@ -9,6 +9,7 @@ import ( "regexp" //nolint:depguard // used instead of github.com/wasilibs/go-re2 due to differences in utf-8 handling "slices" "strings" + "time" "golang.org/x/oauth2" "golang.org/x/sync/errgroup" @@ -25,6 +26,12 @@ import ( // for poorly defined regexps. const maxTotalMatches = 100 +// defaultTokenLifetime is applied when the IdP omits expires_in from +// the token response. Five minutes is conservative enough to avoid +// sending stale tokens during long scans while keeping token endpoint +// traffic reasonable. +const defaultTokenLifetime = 5 * time.Minute + // ─── OAuth2 token acquisition ──────────────────────────────────────── // ropcTokenSource implements oauth2.TokenSource for the Resource Owner @@ -61,10 +68,23 @@ func (s *ropcTokenSource) Token() (*oauth2.Token, error) { ) return nil, err } + + // RFC 6749 marks expires_in as "RECOMMENDED", not required. When + // the IdP omits it, oauth2 leaves Expiry as zero — which + // ReuseTokenSource treats as "never expired," caching the token + // for the lifetime of the process. Force a conservative default + // so stale tokens get refreshed during long scans. + if tok.Expiry.IsZero() { + tok.Expiry = time.Now().Add(defaultTokenLifetime) + logger.Info("IdP did not return expires_in; using default token lifetime", + "default_lifetime", defaultTokenLifetime, + ) + } + logger.Info("ROPC token acquired successfully", "token_endpoint", s.conf.Endpoint.TokenURL, "token_type", tok.TokenType, - "expires_in", tok.Expiry, + "expires_at", tok.Expiry, ) return tok, nil } @@ -523,9 +543,10 @@ func (c *CustomRegexWebhook) createResults(ctx context.Context, match map[string if outcome.RespBody != "" { result.ExtraData["response"] = outcome.RespBody } - if !outcome.Definitive && outcome.RangesInEffect { - // At least one endpoint used ranges but none matched. - result.SetVerificationError(errors.New("verification response status code did not match any configured successRanges or rotatedRanges")) + if outcome.Attempted && !outcome.Definitive { + // Verification was configured but no endpoint gave a conclusive + // answer — either all transports failed or no range matched. + result.SetVerificationError(errors.New("verification attempted but no endpoint gave a definitive answer")) } select { diff --git a/pkg/custom_detectors/custom_detectors_test.go b/pkg/custom_detectors/custom_detectors_test.go index 36c0a3f76..cc76ed090 100644 --- a/pkg/custom_detectors/custom_detectors_test.go +++ b/pkg/custom_detectors/custom_detectors_test.go @@ -3,10 +3,12 @@ package custom_detectors import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -1045,6 +1047,44 @@ func TestBuildTokenSource_ROPC(t *testing.T) { assert.NotEmpty(t, traced.Trace, "trace ID should be set") } +func TestROPCTokenSource_DefaultExpiryWhenIdPOmitsIt(t *testing.T) { + t.Parallel() + + // Simulate an IdP that returns a token without expires_in. + // The ropcTokenSource should set a default expiry so + // ReuseTokenSource doesn't cache it forever. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Token response with no expires_in field. + _, _ = fmt.Fprint(w, `{"access_token":"tok123","token_type":"Bearer"}`) + })) + defer srv.Close() + + auth := &custom_detectorspb.VerifierAuth{ + AuthConfig: &custom_detectorspb.VerifierAuth_Oauth2{ + Oauth2: &custom_detectorspb.OAuth2Config{ + TokenEndpoint: srv.URL, + GrantConfig: &custom_detectorspb.OAuth2Config_Ropc{ + Ropc: &custom_detectorspb.ROPCConfig{ + Username: "user", + Password: "pass", + ClientId: "c", + ClientSecret: "s", + }, + }, + }, + }, + } + ts, err := BuildTokenSource(auth) + require.NoError(t, err) + + tok, err := ts.Token() + require.NoError(t, err) + assert.Equal(t, "tok123", tok.AccessToken) + assert.False(t, tok.Expiry.IsZero(), "expiry should be set even when IdP omits expires_in") + assert.WithinDuration(t, time.Now().Add(defaultTokenLifetime), tok.Expiry, 30*time.Second) +} + // ─── Auth validation in NewWebhookCustomRegex ──────────────────────── func TestNewWebhookCustomRegex_RejectsUnusableAuth(t *testing.T) { diff --git a/pkg/custom_detectors/verify.go b/pkg/custom_detectors/verify.go index d9fa697ff..fe61f5b06 100644 --- a/pkg/custom_detectors/verify.go +++ b/pkg/custom_detectors/verify.go @@ -10,6 +10,7 @@ package custom_detectors import ( "bytes" "context" + "encoding/json" "io" "net/http" "strings" @@ -46,6 +47,12 @@ type VerifyOutcome struct { // answer (verified or not). False means no endpoint matched any // configured range — the caller should treat this as an error. Definitive bool + // Attempted is true when at least one endpoint was tried. When + // Attempted is true but Definitive is false, verification was + // configured but every endpoint failed (transport error, token + // acquisition failure, etc.). Callers should surface this as a + // verification error rather than silently reporting "unverified." + Attempted bool // RangesInEffect is true when at least one endpoint had // successRanges or rotatedRanges configured. When true and // Definitive is false, it means a range-based endpoint failed @@ -100,6 +107,8 @@ func VerifyCredential( return outcome } + outcome.Attempted = true + // Per-endpoint context enrichment: if this endpoint has an // OAuth2 token source carrying a trace, set it on the context // so all downstream log messages inherit the correlation ID. @@ -143,6 +152,11 @@ func VerifyCredential( } // Apply customer-defined headers, then default Content-Type. + // When no explicit Content-Type is set, unmarshal the body to + // distinguish structured JSON (objects/arrays) from raw values. + // Bare primitives like quoted strings or numbers are technically + // valid JSON but not what verification endpoints expect as a + // JSON payload, so they get text/plain. for _, h := range ep.Headers { key, value, found := strings.Cut(h, ":") if !found { @@ -151,7 +165,17 @@ func VerifyCredential( req.Header.Add(key, strings.TrimLeft(value, "\t\n\v\f\r ")) } if req.Header.Get("Content-Type") == "" { - req.Header.Set("Content-Type", "application/json") + var structured interface{} + if json.Unmarshal(body, &structured) == nil { + switch structured.(type) { + case map[string]interface{}, []interface{}: + req.Header.Set("Content-Type", "application/json") + default: + req.Header.Set("Content-Type", "text/plain") + } + } else { + req.Header.Set("Content-Type", "text/plain") + } } // If this endpoint has OAuth2 auth, wrap the base client with @@ -237,9 +261,10 @@ func VerifyCredential( // lifecycle (token acquisition → verify → result) is visible. if traced && lastCtx != nil { logger := lastCtx.Logger() - if rangesInEffect && !outcome.Definitive { - logger.Error(nil, "OAuth2 verification inconclusive; no status range matched", + if outcome.Attempted && !outcome.Definitive { + logger.Error(nil, "OAuth2 verification inconclusive; no endpoint gave a definitive answer", "status_code", outcome.StatusCode, + "ranges_in_effect", rangesInEffect, ) } else if outcome.Verified { logger.Info("OAuth2 verification succeeded", diff --git a/pkg/detectors/endpoint_customizer.go b/pkg/detectors/endpoint_customizer.go index 2b447f7a1..c76468718 100644 --- a/pkg/detectors/endpoint_customizer.go +++ b/pkg/detectors/endpoint_customizer.go @@ -57,7 +57,18 @@ func (e *EndpointSetter) Endpoints(foundEndpoints ...string) []string { // SetOAuth2TokenSource configures an OAuth2 token source for this // detector's custom verifier. When set, the engine uses OAuthVerifier-based // verification instead of the detector's built-in verification logic. -func (e *EndpointSetter) SetOAuth2TokenSource(ts OAuth2TokenSource) { e.oauth2Source = ts } +// +// Setting a non-nil token source disables cloud and found endpoints as +// a safety measure: OAuth2-authenticated requests carry a Bearer token +// and must only go to explicitly configured verification endpoints, +// never to native cloud APIs where the token could leak. +func (e *EndpointSetter) SetOAuth2TokenSource(ts OAuth2TokenSource) { + e.oauth2Source = ts + if ts != nil { + e.useCloudEndpoint = false + e.useFoundEndpoints = false + } +} // HasOAuth2 reports whether OAuth2 auth is configured for this // detector's custom verifier endpoint. diff --git a/pkg/detectors/endpoint_customizer_test.go b/pkg/detectors/endpoint_customizer_test.go index 3467a77e8..52a43af42 100644 --- a/pkg/detectors/endpoint_customizer_test.go +++ b/pkg/detectors/endpoint_customizer_test.go @@ -88,6 +88,27 @@ func TestEndpointSetter_OAuth2(t *testing.T) { }) } +func TestEndpointSetter_OAuth2_DisablesCloudAndFound(t *testing.T) { + t.Parallel() + + var s EndpointSetter + s.useCloudEndpoint = true + s.cloudEndpoint = "https://api.example.com" + s.useFoundEndpoints = true + + // Before setting OAuth2, cloud and found endpoints are included. + assert.NoError(t, s.SetConfiguredEndpoints("https://verify.example.com")) + assert.Equal(t, []string{"https://verify.example.com", "https://api.example.com", "https://found.example.com"}, + s.Endpoints("https://found.example.com")) + + // Setting a token source disables cloud and found endpoints to + // prevent credential leakage to native cloud APIs. + s.SetOAuth2TokenSource(&fakeTokenSource{}) + assert.Equal(t, []string{"https://verify.example.com"}, s.Endpoints("https://found.example.com")) + assert.False(t, s.useCloudEndpoint) + assert.False(t, s.useFoundEndpoints) +} + // fakeTokenSource satisfies oauth2.TokenSource for tests. type fakeTokenSource struct{} diff --git a/pkg/engine/oauth_verifier.go b/pkg/engine/oauth_verifier.go index fe45fd246..b091ec07c 100644 --- a/pkg/engine/oauth_verifier.go +++ b/pkg/engine/oauth_verifier.go @@ -82,8 +82,8 @@ func OAuthVerify( outcome := custom_detectors.VerifyCredential(ctx, baseClient, endpoints, result.Raw, vars) - if !outcome.Definitive && outcome.RangesInEffect { - return false, fmt.Errorf("all verification endpoints exhausted; no status code matched configured ranges") + if outcome.Attempted && !outcome.Definitive { + return false, fmt.Errorf("verification attempted but no endpoint gave a definitive answer") } return outcome.Verified, nil } diff --git a/pkg/engine/oauth_verifier_test.go b/pkg/engine/oauth_verifier_test.go index 3480c5172..d461e8e1d 100644 --- a/pkg/engine/oauth_verifier_test.go +++ b/pkg/engine/oauth_verifier_test.go @@ -309,3 +309,100 @@ func TestOAuthVerify_CustomHeaders(t *testing.T) { require.NoError(t, err) assert.True(t, verified) } + +// ─── Transport failure tests (Attempted field) ────────────────────── + +func TestOAuthVerify_AllEndpointsFail_ReturnsError(t *testing.T) { + t.Parallel() + + // Point at a listener that immediately closes connections. + // Every endpoint fails at the transport level, so Attempted is + // true but Definitive is false — OAuthVerify should return an error. + ts := &staticTokenSource{token: "test-token"} + configs := cfgs("http://127.0.0.1:1") + verified, err := OAuthVerify(context.Background(), nil, ts, configs, testResult()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no endpoint gave a definitive answer") + assert.False(t, verified) +} + +// ─── Content-Type sniff tests ─────────────────────────────────────── + +func TestOAuthVerify_DefaultBody_ContentTypePlainText(t *testing.T) { + t.Parallel() + + // Without a body template, raw secret bytes are sent. When the + // raw secret isn't valid JSON, Content-Type defaults to text/plain. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "text/plain", r.Header.Get("Content-Type")) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + ts := &staticTokenSource{token: "test-token"} + verified, err := OAuthVerify(context.Background(), nil, ts, cfgs(srv.URL), testResult()) + require.NoError(t, err) + assert.True(t, verified) +} + +func TestOAuthVerify_TemplateBody_ContentTypeJSON(t *testing.T) { + t.Parallel() + + // With a body template, ResolveRequestBody produces JSON. + // Content-Type should default to application/json. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + ts := &staticTokenSource{token: "test-token"} + configs := []OAuthVerifyConfig{{ + Endpoint: srv.URL, + RequestBody: map[string]string{"secret": "$secret"}, + }} + verified, err := OAuthVerify(context.Background(), nil, ts, configs, testResult()) + require.NoError(t, err) + assert.True(t, verified) +} + +func TestOAuthVerify_ExplicitContentType_Honored(t *testing.T) { + t.Parallel() + + // An explicit Content-Type header takes priority over the sniff. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "application/xml", r.Header.Get("Content-Type")) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + ts := &staticTokenSource{token: "test-token"} + configs := []OAuthVerifyConfig{{ + Endpoint: srv.URL, + Headers: []string{"Content-Type: application/xml"}, + }} + verified, err := OAuthVerify(context.Background(), nil, ts, configs, testResult()) + require.NoError(t, err) + assert.True(t, verified) +} + +func TestOAuthVerify_JSONPrimitive_ContentTypePlainText(t *testing.T) { + t.Parallel() + + // A raw secret that is a valid JSON primitive (e.g. a quoted + // string) should still get text/plain, not application/json. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "text/plain", r.Header.Get("Content-Type")) + body, _ := io.ReadAll(r.Body) + assert.Equal(t, `"some-api-key"`, string(body)) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + ts := &staticTokenSource{token: "test-token"} + r := testResult() + r.Raw = []byte(`"some-api-key"`) + verified, err := OAuthVerify(context.Background(), nil, ts, cfgs(srv.URL), r) + require.NoError(t, err) + assert.True(t, verified) +}