Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use rawv2 for pubnubpublish #2062

Merged
merged 1 commit into from
Nov 1, 2023
Merged

use rawv2 for pubnubpublish #2062

merged 1 commit into from
Nov 1, 2023

Conversation

rosecodym
Copy link
Collaborator

@rosecodym rosecodym commented Oct 30, 2023

Description:

We're seeing secrets of this type flap between verified and unverified, which is expected behavior for multipart secrets without RawV2 defined. This PR adds RawV2 for secrets of this type.

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@@ -55,6 +55,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result
s1 := detectors.Result{
DetectorType: detectorspb.DetectorType_PubNubPublishKey,
Raw: []byte(resMatch),
RawV2: []byte(resMatch + "/" + ressubMatch),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Most multipart secret detectors just splice the two parts together, which seems unwise to me, so I used / as a separator because that's how the secrets are combined in the URL during verification. A few other detectors use : as a separator; if that's a convention we're trying to follow I'm happy to change it.

Copy link
Contributor

Choose a reason for hiding this comment

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

For getting a password hash of the value it doesn't really matter, as long as we include all the parts, but if we ever wanted to parse the field it does. I think the ship has sailed on making this a parseable information. What I wish we did was something like a map[string]string, which could be sorted, serialized, and hashed.

@rosecodym rosecodym merged commit 7197e4b into main Nov 1, 2023
9 checks passed
@rosecodym rosecodym deleted the add-pubnubpublish-rawv2 branch November 1, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants