Add explicit warning when an overlap is detected #2929
Conversation
|
|
rgmz
force-pushed
the
feat/overlap-notify
branch
5 times, most recently
from
f334a37 to
503266e
Compare
| @@ -752,7 +779,20 @@ func (e *Engine) verificationOverlapWorker(ctx context.Context) { | |||
| continue | |||
| } | |||
|
|
|||
| if likelyDuplicate(ctx, key, chunkSecrets) { | |||
| if ok, t := likelyDuplicate(ctx, key, chunkSecrets); ok { | |||
There was a problem hiding this comment.
Personally, I don't understand the threat model for this feature. It silently skips verification if a result matches more than one detector, which happens frequently during normal operation and can result in valid secrets being skipped.
The notion of a "malicious" detector is confusing — is Truffle adding malicious detectors to TruffleHog? Either this check needs a whitelist of common overlaps (e.g., Azure-related, URL and FTP), or it should only trigger between built-in and custom detectors (IMO).
| @@ -633,7 +633,7 @@ func TestLikelyDuplicate(t *testing.T) { | |||
| name: "empty strings", | |||
| val: chunkSecretKey{"", detectorA.Key}, | |||
| dupes: map[chunkSecretKey]struct{}{{"", detectorB.Key}: {}}, | |||
| expected: true, | |||
| expected: false, | |||
There was a problem hiding this comment.
Should empty strings be considered duplicates? I guess it makes sense, but I'd initially tweaked the logic for likelyDuplicate to ignore anything smaller than 3 characters.
rgmz
force-pushed
the
feat/overlap-notify
branch
3 times, most recently
from
adb2b14 to
e9988fa
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
2 times, most recently
from
ae3db9c to
582444e
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
from
582444e to
c3e49e8
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
from
c3e49e8 to
7021992
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
3 times, most recently
from
5ca7755 to
9dd4a83
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
from
9dd4a83 to
1841161
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
from
1841161 to
099544a
Compare
rgmz
force-pushed
the
feat/overlap-notify
branch
from
099544a to
030ca35
Compare
Description:
Similar to #2922, the goal of this change is to provide clear and actionable information if a detection is impacted.
The problem is currently that "verification overlap" doesn't provide useful feedback. Even if you use
--results=unknownto see the error, it doesn't tell you which detectors overlapped.Checklist:
make test-community)?make lintthis requires golangci-lint)?