From 96b593f460badf7a3da2ad3487e0e4986243ef03 Mon Sep 17 00:00:00 2001 From: Ahsan-Sarbaz Date: Wed, 12 Aug 2026 18:50:11 +0500 Subject: [PATCH] fix(jiratoken): stop 202 reverification retry loop (#5200) --- pkg/detectors/jiratoken/v1/jiratoken.go | 122 +++++++++++-- .../v1/jiratoken_integration_test.go | 10 +- pkg/detectors/jiratoken/v1/jiratoken_test.go | 165 +++++++++++++++++- pkg/detectors/jiratoken/v2/jiratoken_v2.go | 24 ++- .../v2/jiratoken_v2_integration_test.go | 6 + .../jiratoken/v2/jiratoken_v2_test.go | 3 + 6 files changed, 304 insertions(+), 26 deletions(-) diff --git a/pkg/detectors/jiratoken/v1/jiratoken.go b/pkg/detectors/jiratoken/v1/jiratoken.go index b63b132f3..65027e261 100644 --- a/pkg/detectors/jiratoken/v1/jiratoken.go +++ b/pkg/detectors/jiratoken/v1/jiratoken.go @@ -8,6 +8,9 @@ import ( "fmt" "io" "net/http" + "net/url" + "slices" + "sort" "strings" regexp "github.com/wasilibs/go-re2" @@ -20,15 +23,25 @@ import ( type Scanner struct { detectors.DefaultMultiPartCredentialProvider + detectors.EndpointSetter client *http.Client } // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) var _ detectors.Versioner = (*Scanner)(nil) +var _ detectors.EndpointCustomizer = (*Scanner)(nil) +var _ detectors.CloudProvider = (*Scanner)(nil) func (Scanner) Version() int { return 1 } +// CloudHost is used whenever the scanned data carries no Atlassian host of its own. +// reason: https://community.atlassian.com/forums/Jira-Product-Discovery-questions/Authorization-issues-with-GRAPHQL/qaq-p/2640943 +// The graphql API answers on this host as long as the credentials are valid. +const CloudHost = "api.atlassian.com" + +func (Scanner) CloudEndpoint() string { return "https://" + CloudHost } + var ( defaultClient = detectors.DetectorHttpClientWithLocalAddresses @@ -40,8 +53,79 @@ var ( invalidHosts = simple.NewCache[struct{}]() errNoHost = errors.New("no such host") + + // atlassianDomains are the only hosts that can answer the verification request. + // domainPat matches any domain-shaped string near an atlassian/jira/confluence keyword, so without + // this filter the detector would send the email and token to unrelated hosts found in the same chunk. + atlassianDomains = []string{"atlassian.net", "atlassian.com", "jira.com"} ) +// IsAtlassianHost reports whether host is owned by Atlassian and can therefore be used for verification. +func IsAtlassianHost(host string) bool { + host = strings.ToLower(strings.TrimSuffix(host, ".")) + for _, domain := range atlassianDomains { + if host == domain || strings.HasSuffix(host, "."+domain) { + return true + } + } + return false +} + +// EndpointHost reduces an endpoint to its bare host. Found domains arrive without a scheme while cloud and +// user-configured endpoints carry one, and the analyzer expects SecretParts["domain"] to be a bare host. +func EndpointHost(endpoint string) string { + endpoint = strings.TrimSuffix(strings.TrimSpace(endpoint), "/") + if u, err := url.Parse(endpoint); err == nil && u.Host != "" { + return u.Host + } + return endpoint +} + +// FoundHosts returns the Atlassian hosts among the domains matched in the data, sorted so results are +// deterministic. domainPat matches any domain-shaped string, so everything else is dropped rather than +// being sent the email and token. +func FoundHosts(domains map[string]struct{}) []string { + hosts := make([]string, 0, len(domains)) + for domain := range domains { + if IsAtlassianHost(domain) { + hosts = append(hosts, domain) + } + } + sort.Strings(hosts) + return hosts +} + +// VerificationHosts reduces endpoints to the bare hosts to verify against, dropping duplicates. The +// analyzer expects SecretParts["domain"] to be a bare host, and a user-configured endpoint can repeat a +// found one. +// +// CloudHost is only a fallback for data that named no Atlassian host, and EndpointSetter places it ahead +// of the found hosts, so it is dropped when the data named hosts of its own. Data that named CloudHost +// itself keeps it, since then it is a found host rather than the fallback. +func VerificationHosts(endpoints, found []string) []string { + seen := make(map[string]struct{}, len(endpoints)) + hosts := make([]string, 0, len(endpoints)) + dropCloud := len(found) > 0 && !slices.Contains(found, CloudHost) + + for _, endpoint := range endpoints { + host := EndpointHost(endpoint) + if host == "" || (dropCloud && host == CloudHost) { + continue + } + if _, ok := seen[host]; ok { + continue + } + seen[host] = struct{}{} + hosts = append(hosts, host) + } + return hosts +} + +// verificationURL builds the graphql URL for a bare host such as "acme.atlassian.net". +func verificationURL(host string) string { + return "https://" + host + "/gateway/api/graphql" +} + type JIRAGraphQLResponse struct { Data struct { Me struct { @@ -83,24 +167,20 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result uniqueEmails[strings.ToLower(email[1])] = struct{}{} } - if len(uniqueDomains) == 0 { - // reason: https://community.atlassian.com/forums/Jira-Product-Discovery-questions/Authorization-issues-with-GRAPHQL/qaq-p/2640943 - // In case we don't find any domain matches we can use this as the graphql API works with this domain if our authentication is valid - uniqueDomains["api.atlassian.com"] = struct{}{} - } + found := FoundHosts(uniqueDomains) + hosts := VerificationHosts(s.Endpoints(found...), found) for email := range uniqueEmails { for token := range uniqueTokens { - for domain := range uniqueDomains { + for _, domain := range hosts { if invalidHosts.Exists(domain) { - delete(uniqueDomains, domain) continue } s1 := detectors.Result{ DetectorType: detector_typepb.DetectorType_JiraToken, Raw: []byte(token), - RawV2: []byte(fmt.Sprintf("%s:%s:%s", email, token, domain)), + RawV2: fmt.Appendf(nil, "%s:%s:%s", email, token, domain), ExtraData: map[string]string{ "rotation_guide": "https://howtorotate.com/docs/tutorials/atlassian/", "version": fmt.Sprintf("%d", s.Version()), @@ -123,6 +203,13 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result s1.SetVerificationError(verificationErr, token) } + + // The credential is the same across endpoints, so once one of them verifies it there + // is nothing to learn from the rest. + if s1.Verified { + results = append(results, s1) + break + } } results = append(results, s1) @@ -146,7 +233,7 @@ func VerifyJiraToken(ctx context.Context, client *http.Client, email, domain, to } // api docs: https://developer.atlassian.com/platform/atlassian-graphql-api/graphql/#authentication - req, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://"+domain+"/gateway/api/graphql", bytes.NewBuffer(jsonBody)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, verificationURL(domain), bytes.NewBuffer(jsonBody)) if err != nil { return false, err } @@ -171,8 +258,8 @@ func VerifyJiraToken(ctx context.Context, client *http.Client, email, domain, to }() // the API returns 200 if the token is valid - switch resp.StatusCode { - case http.StatusOK: + switch { + case resp.StatusCode == http.StatusOK: var jiraResp JIRAGraphQLResponse if err := json.NewDecoder(resp.Body).Decode(&jiraResp); err != nil { return false, nil // can't decode response in case of 200 OK = not valid JIRA domain @@ -181,10 +268,19 @@ func VerifyJiraToken(ctx context.Context, client *http.Client, email, domain, to // A 200 on its own isn't enough - unrelated hosts can also return JSON and get flagged as verified. // Only trust it when the body actually carries the authenticated user's name. return jiraResp.Data.Me.User.Name != "", nil - case http.StatusUnauthorized: + case resp.StatusCode == http.StatusUnauthorized, resp.StatusCode == http.StatusForbidden: + // The credentials were rejected. Nothing to retry. return false, nil - default: + case resp.StatusCode == http.StatusTooManyRequests, resp.StatusCode >= 500: + // Rate limiting and server errors are genuinely transient, so keep them indeterminate + // and let the secret be retried. return false, fmt.Errorf("unexpected status code: %d", resp.StatusCode) + default: + // Anything else (202, 3xx, 404, ...) is not an Atlassian authentication response at all. + // domainPat matches any domain-shaped string near a jira/atlassian/confluence keyword, so this + // request may well have gone to an unrelated host that happily accepts POSTs. Retrying will + // return the same status forever, so treat it as a terminal "not verified" instead of an error. + return false, nil } } diff --git a/pkg/detectors/jiratoken/v1/jiratoken_integration_test.go b/pkg/detectors/jiratoken/v1/jiratoken_integration_test.go index 8eb9252d7..50cc391fa 100644 --- a/pkg/detectors/jiratoken/v1/jiratoken_integration_test.go +++ b/pkg/detectors/jiratoken/v1/jiratoken_integration_test.go @@ -118,7 +118,7 @@ func TestJiraToken_FromChunk(t *testing.T) { wantVerificationErr: true, }, { - name: "found, verified but unexpected api surface", + name: "found, unexpected api surface is terminal and not an error", s: Scanner{client: common.ConstantResponseHttpClient(404, "")}, args: args{ ctx: context.Background(), @@ -136,11 +136,14 @@ func TestJiraToken_FromChunk(t *testing.T) { }, }, wantErr: false, - wantVerificationErr: true, + wantVerificationErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + tt.s.SetCloudEndpoint(tt.s.CloudEndpoint()) + tt.s.UseCloudEndpoint(true) + tt.s.UseFoundEndpoints(true) got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) if (err != nil) != tt.wantErr { t.Errorf("JiraToken.FromData() error = %v, wantErr %v", err, tt.wantErr) @@ -165,6 +168,9 @@ func TestJiraToken_FromChunk(t *testing.T) { func BenchmarkFromData(benchmark *testing.B) { ctx := context.Background() s := Scanner{} + s.SetCloudEndpoint(s.CloudEndpoint()) + s.UseCloudEndpoint(true) + s.UseFoundEndpoints(true) for name, data := range detectors.MustGetBenchmarkData() { benchmark.Run(name, func(b *testing.B) { b.ResetTimer() diff --git a/pkg/detectors/jiratoken/v1/jiratoken_test.go b/pkg/detectors/jiratoken/v1/jiratoken_test.go index c61fd8572..cba80e7e2 100644 --- a/pkg/detectors/jiratoken/v1/jiratoken_test.go +++ b/pkg/detectors/jiratoken/v1/jiratoken_test.go @@ -15,7 +15,9 @@ import ( var ( validTokenPattern = "Z7VoIYJ0K4rFWLBfkhOsLAWX" invalidTokenPattern = "Z7VoI?J0K4rF#LBfkhO&LAWX" - validDomainPattern = "hereisavalidsubdomain.heresalongdomain.com" + validDomainPattern = "hereisavalidsubdomain.atlassian.net" + foreignDomainPattern = "hereisavalidsubdomain.heresalongdomain.com" + cloudDomain = "api.atlassian.com" invalidDomainPattern = "?y4r3fs1ewqec12v1e3tl.5Hcsrcehic89saXd.ro@" validEmailPattern = "xfkf_bz7@grum.com" invalidEmailPattern = "xfKF_BZq7/grum.com" @@ -24,6 +26,9 @@ var ( func TestJiraToken_Pattern(t *testing.T) { d := Scanner{} + d.SetCloudEndpoint(d.CloudEndpoint()) + d.UseCloudEndpoint(true) + d.UseFoundEndpoints(true) ahoCorasickCore := ahocorasick.NewAhoCorasickCore([]detectors.Detector{d}) tests := []struct { name string @@ -33,7 +38,16 @@ func TestJiraToken_Pattern(t *testing.T) { { name: "valid pattern - with keyword jira", input: fmt.Sprintf("%s %s \n%s %s\n%s %s", keyword, validTokenPattern, keyword, validDomainPattern, keyword, validEmailPattern), - want: []string{validEmailPattern + ":" + validTokenPattern + ":" + validDomainPattern}, + // The tenant host found in the data is used on its own. The cloud endpoint must not shadow it, + // or SecretParts["domain"] would stop being the host the analyzer needs. + want: []string{validEmailPattern + ":" + validTokenPattern + ":" + validDomainPattern}, + }, + { + // A domain that isn't Atlassian-owned can't answer the verification request, so it is dropped + // and only the cloud endpoint remains. + name: "non-atlassian domain falls back to the cloud endpoint", + input: fmt.Sprintf("%s %s \n%s %s\n%s %s", keyword, validTokenPattern, keyword, foreignDomainPattern, keyword, validEmailPattern), + want: []string{validEmailPattern + ":" + validTokenPattern + ":" + cloudDomain}, }, { name: "valid pattern - key out of prefix range", @@ -90,12 +104,121 @@ func TestJiraToken_Pattern(t *testing.T) { } } +func TestIsAtlassianHost(t *testing.T) { + tests := map[string]bool{ + "example.atlassian.net": true, + "atlassian.net": true, + "api.atlassian.com": true, + "acme.jira.com": true, + "EXAMPLE.Atlassian.Net": true, + "example.atlassian.net.": true, + // Suffix lookalikes must not pass, or the filter would be trivially bypassed. + "notatlassian.net": false, + "atlassian.net.evil.com": false, + "example.com": false, + "hereisavalidsubdomain.co.uk": false, + } + + for host, want := range tests { + t.Run(host, func(t *testing.T) { + if got := IsAtlassianHost(host); got != want { + t.Errorf("IsAtlassianHost(%q) = %v, want %v", host, got, want) + } + }) + } +} + +func TestFoundHosts(t *testing.T) { + tests := []struct { + name string + domains []string + want []string + }{ + { + name: "non-atlassian domains are dropped", + domains: []string{"tenant.atlassian.net", "hooks.example.com"}, + want: []string{"tenant.atlassian.net"}, + }, + { + name: "nothing atlassian found", + domains: []string{"hooks.example.com"}, + want: []string{}, + }, + { + // The cloud host is a normal Atlassian host when it shows up in the data. + name: "cloud host in the data is kept", + domains: []string{CloudHost}, + want: []string{CloudHost}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + domains := make(map[string]struct{}, len(test.domains)) + for _, d := range test.domains { + domains[d] = struct{}{} + } + if diff := cmp.Diff(test.want, FoundHosts(domains)); diff != "" { + t.Errorf("FoundHosts() diff: (-want +got)\n%s", diff) + } + }) + } +} + +func TestVerificationHosts(t *testing.T) { + const cloudEndpoint = "https://" + CloudHost + + tests := []struct { + name string + endpoints []string + found []string + want []string + }{ + { + // EndpointSetter puts the cloud endpoint ahead of the found ones, so leaving it in would let it + // win the break-on-verified and report a domain the analyzer cannot use. + name: "cloud dropped when the data named a host", + endpoints: []string{cloudEndpoint, "tenant.atlassian.net"}, + found: []string{"tenant.atlassian.net"}, + want: []string{"tenant.atlassian.net"}, + }, + { + name: "cloud kept when the data named nothing", + endpoints: []string{cloudEndpoint}, + found: nil, + want: []string{CloudHost}, + }, + { + // Dropping the cloud host here would leave no hosts at all and produce no results. + name: "cloud kept when the data named it", + endpoints: []string{cloudEndpoint, CloudHost}, + found: []string{CloudHost}, + want: []string{CloudHost}, + }, + { + name: "configured endpoints are reduced to hosts and deduped", + endpoints: []string{"https://jira.example.com:8443/", cloudEndpoint, "tenant.atlassian.net", "https://tenant.atlassian.net", ""}, + found: []string{"tenant.atlassian.net"}, + want: []string{"jira.example.com:8443", "tenant.atlassian.net"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if diff := cmp.Diff(test.want, VerificationHosts(test.endpoints, test.found)); diff != "" { + t.Errorf("VerificationHosts() diff: (-want +got)\n%s", diff) + } + }) + } +} + func TestJiraToken_Verify(t *testing.T) { tests := []struct { name string statusCode int body string wantVerified bool + wantErr bool }{ { name: "authenticated user in body is verified", @@ -116,14 +239,48 @@ func TestJiraToken_Verify(t *testing.T) { body: `{"code":401,"message":"Unauthorized"}`, wantVerified: false, }, + { + name: "forbidden is not verified", + statusCode: 403, + body: `{"code":403,"message":"Forbidden"}`, + wantVerified: false, + }, + { + // Atlassian never answers 202 here. It comes from unrelated hosts that accept the POST, and it + // would repeat on every retry, so it has to be terminal rather than an indeterminate error. + name: "accepted is not verified and not an error", + statusCode: 202, + body: `{"status":"accepted"}`, + wantVerified: false, + }, + { + name: "not found is not verified and not an error", + statusCode: 404, + body: `{"code":404,"message":"Not Found"}`, + wantVerified: false, + }, + { + name: "rate limited is indeterminate", + statusCode: 429, + body: `{"code":429,"message":"Too Many Requests"}`, + wantVerified: false, + wantErr: true, + }, + { + name: "server error is indeterminate", + statusCode: 500, + body: `{"code":500,"message":"Internal Server Error"}`, + wantVerified: false, + wantErr: true, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { client := common.ConstantResponseHttpClient(test.statusCode, test.body) verified, err := VerifyJiraToken(context.Background(), client, "user@example.com", "example.atlassian.net", validTokenPattern) - if err != nil { - t.Fatalf("unexpected error: %v", err) + if (err != nil) != test.wantErr { + t.Fatalf("got error = %v, wantErr %v", err, test.wantErr) } if verified != test.wantVerified { t.Errorf("got verified = %v, want %v", verified, test.wantVerified) diff --git a/pkg/detectors/jiratoken/v2/jiratoken_v2.go b/pkg/detectors/jiratoken/v2/jiratoken_v2.go index 6e802e0c9..90e275d7e 100644 --- a/pkg/detectors/jiratoken/v2/jiratoken_v2.go +++ b/pkg/detectors/jiratoken/v2/jiratoken_v2.go @@ -17,14 +17,19 @@ import ( type Scanner struct { client *http.Client detectors.DefaultMultiPartCredentialProvider + detectors.EndpointSetter } // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) var _ detectors.Versioner = (*Scanner)(nil) +var _ detectors.EndpointCustomizer = (*Scanner)(nil) +var _ detectors.CloudProvider = (*Scanner)(nil) func (Scanner) Version() int { return 2 } +func (Scanner) CloudEndpoint() string { return "https://" + v1.CloudHost } + var ( defaultClient = detectors.DetectorHttpClientWithLocalAddresses @@ -66,19 +71,17 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result uniqueEmails[strings.ToLower(email[1])] = struct{}{} } - if len(uniqueDomains) == 0 { - // reason: https://community.atlassian.com/forums/Jira-Product-Discovery-questions/Authorization-issues-with-GRAPHQL/qaq-p/2640943 - // In case we don't find any domain matches we can use this as the graphql API works with this domain if our authentication is valid - uniqueDomains["api.atlassian.com"] = struct{}{} - } + // domainPat here is not even keyword-gated, so it captures every domain in the chunk. + found := v1.FoundHosts(uniqueDomains) + hosts := v1.VerificationHosts(s.Endpoints(found...), found) for email := range uniqueEmails { for token := range uniqueTokens { - for domain := range uniqueDomains { + for _, domain := range hosts { s1 := detectors.Result{ DetectorType: detector_typepb.DetectorType_JiraToken, Raw: []byte(token), - RawV2: []byte(fmt.Sprintf("%s:%s:%s", email, token, domain)), + RawV2: fmt.Appendf(nil, "%s:%s:%s", email, token, domain), ExtraData: map[string]string{ "rotation_guide": "https://howtorotate.com/docs/tutorials/atlassian/", "version": fmt.Sprintf("%d", s.Version()), @@ -95,6 +98,13 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result isVerified, verificationErr := v1.VerifyJiraToken(ctx, client, email, domain, token) s1.Verified = isVerified s1.SetVerificationError(verificationErr, token) + + // The credential is the same across endpoints, so once one of them verifies it there + // is nothing to learn from the rest. + if s1.Verified { + results = append(results, s1) + break + } } results = append(results, s1) diff --git a/pkg/detectors/jiratoken/v2/jiratoken_v2_integration_test.go b/pkg/detectors/jiratoken/v2/jiratoken_v2_integration_test.go index d72a7f331..1dca3b25b 100644 --- a/pkg/detectors/jiratoken/v2/jiratoken_v2_integration_test.go +++ b/pkg/detectors/jiratoken/v2/jiratoken_v2_integration_test.go @@ -141,6 +141,9 @@ func TestJiraToken_FromChunk(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + tt.s.SetCloudEndpoint(tt.s.CloudEndpoint()) + tt.s.UseCloudEndpoint(true) + tt.s.UseFoundEndpoints(true) got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) if (err != nil) != tt.wantErr { t.Errorf("JiraToken.FromData() error = %v, wantErr %v", err, tt.wantErr) @@ -165,6 +168,9 @@ func TestJiraToken_FromChunk(t *testing.T) { func BenchmarkFromData(benchmark *testing.B) { ctx := context.Background() s := Scanner{} + s.SetCloudEndpoint(s.CloudEndpoint()) + s.UseCloudEndpoint(true) + s.UseFoundEndpoints(true) for name, data := range detectors.MustGetBenchmarkData() { benchmark.Run(name, func(b *testing.B) { b.ResetTimer() diff --git a/pkg/detectors/jiratoken/v2/jiratoken_v2_test.go b/pkg/detectors/jiratoken/v2/jiratoken_v2_test.go index 4249d15b5..40c5b8a42 100644 --- a/pkg/detectors/jiratoken/v2/jiratoken_v2_test.go +++ b/pkg/detectors/jiratoken/v2/jiratoken_v2_test.go @@ -12,6 +12,9 @@ import ( func TestJiraToken_Pattern(t *testing.T) { d := Scanner{} + d.SetCloudEndpoint(d.CloudEndpoint()) + d.UseCloudEndpoint(true) + d.UseFoundEndpoints(true) ahoCorasickCore := ahocorasick.NewAhoCorasickCore([]detectors.Detector{d}) tests := []struct { name string