fix(detectors/ngrok): match API keys that do not start with 2 - #5152
fix(detectors/ngrok): match API keys that do not start with 2#5152mattbrady-1 wants to merge 1 commit into
Conversation
The ngrok bearer token pattern is 27 alphanumerics, an underscore, and 21 alphanumerics. Requiring a leading 2 missed valid API keys and ak_ resource IDs were never secrets. Co-authored-by: Cursor <cursoragent@cursor.com>
![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 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7aa6716. Configure here.
| keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"ngrok"}) + `\b(2[a-zA-Z0-9]{26}_\d[a-zA-Z0-9]{20})\b`) | ||
| // ngrok API keys and authtokens are {prefix}_{suffix} with a digit-leading suffix. | ||
| // API keys are 27+21 chars; authtokens vary but share the same charset constraints. | ||
| keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"ngrok"}) + `\b([a-zA-Z0-9]{27}_[a-zA-Z0-9]{21})\b`) |
There was a problem hiding this comment.
Regex accidentally drops digit-leading suffix constraint
Medium Severity
The comment on line 41 describes "a digit-leading suffix" but the new regex uses [a-zA-Z0-9]{21} for the suffix, which allows it to start with any alphanumeric character. The old regex enforced \d[a-zA-Z0-9]{20} for the suffix portion. The PR intended only to broaden the prefix (removing the 2 requirement), but the suffix constraint was also accidentally dropped, which could increase false positive detections.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7aa6716. Configure here.
There was a problem hiding this comment.
The bugbot comment looks correct.
One caveat on the suffix: I created four authtokens, and one came back 20 chars with a non-digit leading char (verified real via agent_ingresses : ERR_NGROK_206). So v2 can't assume 21 chars or a digit-leading suffix, it needs to allow {20,21} and letter-or-digit start.
Corpora Test ResultsScans a corpus of real-world public code against only the detectors changed in this PR, then compares unique match counts between the PR build and the main baseline to catch regex regressions. Verification is disabled — each detector's regex is measured independently. 0 new · 1 clean | Scoped to:
|
2 participants
Summary
2[a-zA-Z0-9]{26}_...to[a-zA-Z0-9]{27}_...so valid bearer tokens are detected regardless of first character.2and forak_resource IDs, which are not secrets.Test plan
go test ./pkg/detectors/ngrok/ -run TestNgrok_PatternMade with Cursor