feat: add Waveapps payment token detector#4791
feat: add Waveapps payment token detector#4791yunior123 wants to merge 5 commits intotrufflesecurity:mainfrom
Conversation
Adds detection for Wave payment tokens with `wave_sn_prod_` and `wave_ci_prod_` prefixes. Includes API verification via Wave's GraphQL endpoint. Closes trufflesecurity#4618
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The manual pb.go edit was incomplete (missing raw descriptor update). Reverting to let maintainers regenerate via `make protos` after the proto/detectors.proto change.
|
Thanks for the catch @cursor[bot]! Reverted the manual Also signed the CLA. |
There was a problem hiding this comment.
Pull request overview
Adds a new TruffleHog detector for Waveapps payment tokens, including optional online verification against Wave’s public GraphQL endpoint, and wires the detector into the default scanner set.
Changes:
- Added
wave_sn_prod_*/wave_ci_prod_*token detection with optional HTTP verification. - Registered the new detector type in protobuf enums and Go bindings.
- Included pattern-matching tests and registered the detector in engine defaults.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| proto/detectors.proto | Adds Waveapps = 1043 to DetectorType. |
| pkg/pb/detectorspb/detectors.pb.go | Manually adds the corresponding Go enum constant and name/value maps. |
| pkg/engine/defaults/defaults.go | Registers waveapps.Scanner in the default detector list. |
| pkg/detectors/waveapps/waveapps.go | Implements token regex matching and optional verification via Wave GraphQL API. |
| pkg/detectors/waveapps/waveapps_test.go | Adds pattern tests for valid/invalid token shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
pkg/pb/detectorspb/detectors.pb.go was updated by manually adding enum constants/maps, but the generated enum descriptor (file_detectors_proto_rawDesc / file_detectors_proto_enumTypes) was not regenerated. Since DetectorType.String() uses protoimpl.X.EnumStringOf(x.Descriptor(), ...), detectorspb.DetectorType_Waveapps.String() will not reliably return "Waveapps" (and code like detector.Type().String() in the engine/metrics will log a numeric/unknown value). Regenerate the protobufs (preferred) or ensure the raw descriptor/enum type metadata is updated consistently so reflection-based stringification and protojson/prototext behave correctly.
pkg/detectors/waveapps/waveapps.go
Outdated
| } | ||
|
|
||
| func verifyWaveapps(ctx context.Context, client *http.Client, token string) (bool, error) { | ||
| // Use a simple introspection query to verify the token is valid. |
There was a problem hiding this comment.
The comment says this is an "introspection query", but the payload is a normal { user { id } } query (GraphQL introspection would use __schema / __type). Please update the comment to match what the code is doing (or change the query if you truly intend introspection), to avoid confusion for future maintainers.
| // Use a simple introspection query to verify the token is valid. | |
| // Use a simple user query to verify the token is valid. |
| if verify { | ||
| client := s.getClient() | ||
| isVerified, verificationErr := verifyWaveapps(ctx, client, resMatch) | ||
| s1.Verified = isVerified | ||
| s1.SetVerificationError(verificationErr, resMatch) | ||
| } |
There was a problem hiding this comment.
The new detector includes a verification path (verifyWaveapps + the verify branch in FromData), but the added tests only cover pattern matching (verify=false). This leaves verification behavior (verified/unverified, timeout/unexpected-status handling, and SetVerificationError behavior) untested. Consider adding a waveapps_integration_test.go (like many other detectors) that injects a stub http.Client (e.g., common.ConstantResponseHttpClient / common.SaneHttpClientTimeOut) to cover success and error cases without needing real Wave tokens.
Addressing Review FeedbackThanks for the thorough reviews! Pushed fixes in 2c1f823. Copilot Review
Cursor Bugbot (CRITICAL flag)
This is incorrect. The original issue #4618 explicitly references CLASigned via cla-assistant ✅ |
| OpenAIAdmin = 1040; | ||
| GoogleGeminiAPIKey = 1041; | ||
| ArtifactoryReferenceToken = 1042; | ||
| Waveapps = 1043; |
There was a problem hiding this comment.
after making any changes to proto, you have to run make protos. See this guide
pkg/detectors/waveapps/waveapps.go
Outdated
| switch res.StatusCode { | ||
| case http.StatusOK: | ||
| // A valid token returns user data; an invalid one returns errors. | ||
| if strings.Contains(string(body), `"id"`) { |
There was a problem hiding this comment.
this is error-prone, let's parse the response if we are dependent on the response to declare the secret as valid
- Fix misleading comment: "introspection query" → "user query" - Add clarifying comment that sn/ci are Waveapps token types, not country codes, with reference link to Waveapps auth docs
- Replace fragile strings.Contains("id") with proper json.Unmarshal
into typed graphQLResponse struct (mirrors railwayapp pattern)
- Add body drain for HTTP connection pooling
- Add integration test file following project conventions
Adds DetectorType_Waveapps = 1043 to the generated Go bindings.
yunior123
force-pushed
the
feat/waveapps-detector
branch
from
2c1f823 to
6771d96
Compare
|
@shahzadhaider1 Thanks for the review! Pushed fixes:
All pattern tests passing. PTAL! |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| // Wave payment tokens have distinct prefixes: wave_sn_prod_ or wave_ci_prod_ | ||
| // These are Waveapps (waveapps.com) API token types, not country codes. | ||
| // Ref: https://developer.waveapps.com/hc/en-us/articles/360018856751-Authentication | ||
| keyPat = regexp.MustCompile(`\b(wave_(?:sn|ci)_prod_[A-Za-z0-9_-]{30,})\b`) |
There was a problem hiding this comment.
Regex \b boundary incompatible with - in character class
Low Severity
The regex \b(wave_(?:sn|ci)_prod_[A-Za-z0-9_-]{30,})\b includes - in the character class but uses \b word boundaries. Since - is not a word character (only [A-Za-z0-9_] are), if a valid token ends with -, the trailing \b will fail to match (non-word to non-word is not a boundary), causing the engine to backtrack and silently truncate the captured token. The truncated value stored in Raw would then fail verification, producing a false-negative or a misleading raw secret value.
Summary
wave_sn_prod_*andwave_ci_prod_*prefixes)gql.waveapps.com/graphql/public)Changes
pkg/detectors/waveapps/waveapps.go— detector implementation with HTTP verificationpkg/detectors/waveapps/waveapps_test.go— pattern-matching tests (5 cases, all passing)proto/detectors.proto— addedWaveapps = 1043enumpkg/pb/detectorspb/detectors.pb.go— manually added constant (maintainers may want to runmake protosto regenerate)pkg/engine/defaults/defaults.go— registered scannerNotes
make protosrequires Docker. Maintainers should runmake protosto regenerate the full file.Test plan
go test ./pkg/detectors/waveapps/ -v— all 5 tests passgo build ./pkg/detectors/waveapps/— compiles cleanlyCloses #4618
Note
Medium Risk
Introduces a new verification network call to an external API, which can affect scan latency and error/timeout behavior, plus updates the shared detector-type enum that must remain consistent across generated protobuf code.
Overview
Adds a new
waveappsdetector that matcheswave_sn_prod_/wave_ci_prod_tokens and (when verification is enabled) validates candidates by calling Wave’s public GraphQL API and interpreting HTTP/GraphQL error responses.Registers the new detector in
defaults.goand introduces theWaveappsprotobufDetectorTypeenum value, along with unit tests, integration tests, and benchmarks covering matching and verification error handling.Written by Cursor Bugbot for commit 6771d96. This will update automatically on new commits. Configure here.