adding customizable successRanges and rotatedRanges to customDetector#4892
Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
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 be9bfc5. Configure here.
…o that an earlier ranged verifier's rangesInEffect = true can't trigger a spurious SetVerificationError after the legacy verifier already confirmed the secret as live.
kashifkhan0771
approved these changes
May 8, 2026
| } | ||
|
|
||
| if rangesInEffect && !definitive { | ||
| result.SetVerificationError(fmt.Errorf("verification response status code did not match any configured successRanges or rotatedRanges")) |
Contributor
There was a problem hiding this comment.
No format args in fmt.Errorf we can use errors.New.
Contributor
Author
There was a problem hiding this comment.
done, thank you!
| storeResponseBody(resp, result.ExtraData) | ||
| break | ||
| } | ||
| continue |
Contributor
There was a problem hiding this comment.
I think we need to set a definitive as true here before continue for legacy non 200 path.
Scenario that can trigger without this:
Verifier 1: ranged, both configured, returns 500 or any code not in ranges
- rangesInEffect = true
- bothConfigured = true, neither range matches
- loop continues (no break, no definitive)
Verifier 2: legacy (no ranges), returns 401
- hits continue at line 327
- definitive never set
After loop:
- rangesInEffect = true, definitive = false
- SetVerificationError fires
Contributor
Author
There was a problem hiding this comment.
Thank you, just made this change
rosecodym
approved these changes
May 11, 2026
MuneebUllahKhan222
pushed a commit
that referenced
this pull request
May 29, 2026
…#4892) * adding customizable successRanges and rotatedRanges to customDetector *setting definitive = true in the legacy 200 path so that an earlier ranged verifier's rangesInEffect = true can't trigger a spurious SetVerificationError after the legacy verifier already confirmed the secret as live. * adressed review comments
Closed
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This would allow users to add successRanges and rotatedRanges to their customDetectors, for verification purposes, allowing them control/configuration over what is considered live/rotated for these custom detections.
If both successRanges and rotatedRanges are implemented, any response that is not within said ranges is treated as inconclusive. Here is how the change handles edge cases:
Only successRanges set: non-matching responses mean rotated
Only rotatedRanges set: non-matching responses mean live
Neither set: old behavior, only 200 means verified; other statuses are just unverified without a verification error.
Checklist:
make test-community)?make lintthis requires golangci-lint)?Note
Medium Risk
Changes verification behavior for custom detectors by interpreting verifier HTTP status codes via configurable ranges and introducing new "inconclusive" verification errors, which could affect existing workflows if misconfigured. Backward compatibility is preserved when no ranges are provided, but verification outcomes and error reporting now depend on additional config.
Overview
Adds configurable HTTP status handling for custom detector webhook verification by introducing
rotatedRangesalongside existingsuccessRanges, allowing verifiers to explicitly mark secrets as live, rotated, or inconclusive based on response codes.Updates the verifier loop to evaluate these ranges (including single-range fallbacks and legacy no-range behavior), records truncated response bodies via a shared helper, and emits a verification error when ranges are configured but no verifier returns a definitive match.
Extends the protobuf schema (
VerifierConfig) and YAML parsing/validation to supportrotatedRanges, addsStatusCodeMatchesRanges, and introduces focused unit tests covering parsing, range matching, mixed legacy+ranged verifiers, and edge-case status handling.Reviewed by Cursor Bugbot for commit d12d4d9. Bugbot is set up for automated code reviews on this repo. Configure here.