Update sqlserver redaction, deduplication, and URI redaction #1369
Conversation
| continue | ||
| } | ||
|
|
||
| detected := detectors.Result{ | ||
| DetectorType: detectorspb.DetectorType_SQLServer, | ||
| Raw: []byte(params.Password), | ||
| Raw: []byte(paramsUnsafe.Password), | ||
| RawV2: []byte(paramsUnsafe.URL().String()), |
There was a problem hiding this comment.
question: Will this potentially lead to duplicate results similar to to the changes we made earlier with the other detectors. Since Raw and RawV2 are used as part of the lookup during deduplication we would create a new secret with potentially the same Raw value.
There was a problem hiding this comment.
Possibly. I need to sync with @trufflesteeeve on this since the whole point of introducing RawV2 was to start properly deduping detectors that had an incorrect raw field.
There was a problem hiding this comment.
It's been a while since that was added. I'll have to read the code to see what should be happening. We did see a secret duplication issue when RawV2 was added before, though.
Also I'm not 100% sure how this interacts with SecretsV2.
There was a problem hiding this comment.
So I think I understand where the hashv2 duplication came from. We've always been adding the hashv2 to the secrets - https://github.com/trufflesecurity/thog/blob/46fe1c8d6d5b13737c603e20bafded703fc066c9/scanner/pkg/secrets/secret.go#L81
But if there was no rawv2, then that's just a hash of empty string. But it still gives it a hashv2 value. So when we look it up when we actually have a hash v2 value, it's different. I think we need to adjust our hashv2 lookup logic to include the default hash of empty string as well, and treat it as if there is no hashv2.
There was a problem hiding this comment.
Okay, let's adjust that before merging this or any more changes w/ rawv2
There was a problem hiding this comment.
I have the PR for the fix here, nearly done - https://github.com/trufflesecurity/thog/pull/1065
There was a problem hiding this comment.
I'm also noticing that with secretsv2 saving, changing the HashV2 will always create a new secret.
There was a problem hiding this comment.
I've fully updated but not yet tested the PR above to allow HashV2 to be fine to update. I'm going to try and finish that up early tomorrow.
SQLServer wasn't being properly deduplicated and URI wasn't being properly redacted in some cases