Skip to content

fix(detectors/ngrok): match API keys that do not start with 2 - #5152

Open
mattbrady-1 wants to merge 3 commits into
mainfrom
fix/ngrok-detector-api-key-pattern
Open

fix(detectors/ngrok): match API keys that do not start with 2#5152
mattbrady-1 wants to merge 3 commits into
mainfrom
fix/ngrok-detector-api-key-pattern

Conversation

@mattbrady-1

@mattbrady-1 mattbrady-1 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Broaden the ngrok detector prefix from 2[a-zA-Z0-9]{26} to [a-zA-Z0-9]{27} so valid bearer tokens are detected regardless of first character.
  • Allow suffixes of 20-21 alphanumeric chars with any leading character. Real authtokens (verified via ERR_NGROK_206) exist with 20-char and letter-leading suffixes, so a digit-leading 21-char suffix constraint drops valid secrets.
  • Add tests for API keys not starting with 2, letter-leading and 20-char suffixes, and ak_ resource IDs, which are not secrets.

Test plan

  • go test ./pkg/detectors/ngrok/ -run TestNgrok_Pattern
  • Scan fixture containing a valid ngrok API bearer token and confirm verified detection

Made with Cursor

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>
@mattbrady-1
mattbrady-1 requested a review from a team July 23, 2026 16:15
@mattbrady-1
mattbrady-1 requested a review from a team as a code owner July 23, 2026 16:15
Comment thread pkg/detectors/ngrok/ngrok.go Outdated
@github-actions

Copy link
Copy Markdown

Corpora Test Results

Scans 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: ngrok

Status Detector Unique matches (main) Unique matches (PR) New Removed
ngrok 0 0 0 0
  • 🔴 regression: >5 new, >20% increase over main, or any removed
  • ⚠️ warning: 1–5 new and ≤20% increase over main
  • ✅ clean
  • 🆕 new detector (no baseline)

The prefix broadening accidentally allowed any alphanumeric suffix start, which would increase false positives.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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 3fa132a. Configure here.

Comment thread pkg/detectors/ngrok/ngrok.go Outdated
Real authtokens verified against the ngrok API can have 20-char and
letter-leading suffixes, so the digit-leading 21-char suffix constraint
drops valid secrets.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants