Fix Shopify detector: add missing shpca_ and shptka_ token patterns#4946
Fix Shopify detector: add missing shpca_ and shptka_ token patterns#4946asivaprasad09 wants to merge 5 commits intotrufflesecurity:mainfrom
Conversation
Adds a detector for Together AI API keys (tgp_v1_ format). Verifies keys via GET /v1/models endpoint.
…us codes, deduplicate - Previously tokens with no co-located domain were silently dropped. Now an unverified result is always emitted so the token is never lost. - Switch to common.SaneHttpClient() consistent with all other detectors. - Properly drain response body with io.Copy(io.Discard) to avoid connection leaks. - Treat 403 as determinately invalid (was falling into error bucket). - Deduplicate keys and domains before processing to avoid duplicate results. - Extract verification into verifyGrafanaServiceAccount() helper.
…d when no domain
- Tighten regex from cmVmdGtu[a-zA-Z0-9]{54,60} to cmVmdGtuOj[a-zA-Z0-9]{54}
Reference tokens always decode to reftkn:01:<expiry>:<random> making
the full prefix cmVmdGtuOj (base64 of 'reftkn:') and total length
always exactly 64 chars. Variable length range was incorrect.
- Update keyword from cmVmdGtu to cmVmdGtuOj for more specific
pre-filtering and fewer false positives.
- Emit unverified result when no jfrog.io domain is found in the chunk.
Previously tokens without a co-located domain were silently dropped.
The detector only covered shppa_ (partner app) and shpat_ (private app). Two token types were silently missed: - shpca_ (custom app tokens) - shptka_ (token app tokens) Updated regex and keywords to cover all 4 Shopify token formats.
- Token alone with no myshopify.com domain was silently dropped. Now emits unverified result so token is always flagged. - Fix status code handling: explicit 401/403 = determinately invalid, other non-200 = indeterminate error (was silently ignored before). - Drain response body with io.Copy(io.Discard) to avoid connection leaks. - Extract verifyShopify() helper consistent with detector conventions. - Switch to common.SaneHttpClient(), deduplicate keys and domains.
|
Akshara Sivaprasad seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Reviewed by Cursor Bugbot for commit ba42427. Configure here.
|
|
||
| var ( | ||
| client = detectors.DetectorHttpClientWithNoLocalAddresses | ||
| defaultClient = common.SaneHttpClient() |
There was a problem hiding this comment.
SSRF protection removed from user-controlled domain requests
Medium Severity
The defaultClient was changed from detectors.DetectorHttpClientWithNoLocalAddresses to common.SaneHttpClient(). Both the Shopify and Grafana detectors make verification HTTP requests to domains extracted from scanned data (user-controlled input). The previous client included WithNoLocalIP() protection that blocks connections to loopback, link-local, and private IP addresses, preventing SSRF. common.SaneHttpClient() lacks this safeguard. While the domain regexes constrain targets to *.myshopify.com and *.grafana.net, this still removes an intentional defense-in-depth layer against DNS rebinding or similar attacks.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ba42427. Configure here.
| if err != nil { | ||
| return false, err | ||
| } | ||
| defer res.Body.Close() |
There was a problem hiding this comment.
Response body not drained before close
Low Severity
In verifyTogetherAI, the response body is closed with a bare defer res.Body.Close() without first draining it via io.Copy(io.Discard, res.Body). The other verification functions added in this same PR (verifyShopify, verifyGrafanaServiceAccount) correctly drain before closing. Not draining the body prevents the underlying TCP connection from being reused by the connection pool, which can lead to connection churn under load.
Reviewed by Cursor Bugbot for commit ba42427. Configure here.
amanfcp
left a comment
amanfcp
left a comment
There was a problem hiding this comment.
I'd suggest you to create separate PRs for each detector enhancements.
Regarding the change mentioned in description, how were you able to test sptka and shpca? When I last worked on it, it said that new credentials of these types cannot be created anymore, however the existing ones work. It's been deferred since we cannot merge a change we cannot test.
3 participants
Summary
Problem
The existing Shopify detector only covered two of the four Shopify access token formats:
shppa_— Partner app tokens ✅shpat_— Private app tokens ✅shpca_— Custom app tokens ❌ missingshptka_— Token app tokens ❌ missingAny
shpca_orshptka_token in scanned content was silently missed.Changes
(shppa_|shpat_)→shp(?:ca|at|tka|pa)_covering all 4 formatsshpca_andshptka_to the pre-filter list[0-9A-Fa-f]to[a-f0-9](Shopify tokens use lowercase hex only)Before vs After
shppa_Partner appshpat_Private appshpca_Custom appshptka_Token appTest plan
*.myshopify.comdomaingo build ./pkg/detectors/shopify/...passes cleanlyNote
Medium Risk
Adds a new detector with live HTTP verification and adjusts multiple existing detectors’ matching/verification behavior, which could affect scan results and outbound request volume. Regex tightening and new “emit unverified when no domain found” paths may change false-positive/false-negative rates.
Overview
Improves secret detection coverage and robustness across several detectors.
Expands the Shopify detector to recognize additional token prefixes (now covering
shpca_,shpat_,shptka_,shppa_), updates keyword prefiltering accordingly, and refactors verification into a helper using a shared “sane” HTTP client with safer response body draining.Updates Grafana service account detection to a stricter token format, dedupes matches, uses the shared HTTP client, and emits findings as unverified when no
*.grafana.netdomain is present instead of dropping them.Adjusts Artifactory reference token matching to a more specific base64 prefix/length and similarly emits unverified results when no
*.jfrog.iohost is found.Adds a new
TogetherAIdetector (regex + optional verification againstGET /v1/models), wires it into default detector lists, and registers the newDetectorType_TogetherAIin the protobuf enum (with tests/benchmarks for the new detector).Reviewed by Cursor Bugbot for commit ba42427. Bugbot is set up for automated code reviews on this repo. Configure here.