trufflesecurity / trufflehog

Find, verify, and analyze leaked credentials
https://trufflesecurity.com
GNU Affero General Public License v3.0
15.83k stars 1.65k forks source link

Allow detectors to customize cleaning logic #3233

Closed rosecodym closed 1 month ago

rosecodym commented 1 month ago

Description:

We have identified some cases in which the results "cleaning" logic (the logic that eliminates superfluous results) should not run. In order to allow this, we need to expose the cleaning logic to the engine. This PR does so by doing these things:

Checklist:

rgmz commented 1 month ago

Nothing against this specific change, however, it feels like the number of detector-related interfaces and default implementations is getting out of hand (multi-part creds is another confusing one).

IMO, it would be cleaner ( :sunglasses: ) to use the default CleanResults function unless the detector has a special CustomResultCleaner interface.

rosecodym commented 1 month ago

Nothing against this specific change, however, it feels like the number of detector-related interfaces and default implementations is getting out of hand (multi-part creds is another confusing one).

IMO, it would be cleaner ( 😎 ) to use the default CleanResults function unless the detector has a special CustomResultCleaner interface.

Heh, I will point out that when I did the false positive change that way @mcastorina suggested I instead use an embedded default logic provider (like I did here), which is why I did this. But I actually prefer that solution, so I'll take a pass at it.

@rgmz do you remember any details about that interface-smuggling related bug you found awhile ago? I want to ensure I don't trip over it again.

mcastorina commented 1 month ago

I'm not sure why I suggested that in the past, but I agree with @rgmz's suggestion to use interface smuggling here. I think it makes a lot of sense for detectors to have opt-in behavior for special handling, considering how many detectors we have.

rosecodym commented 1 month ago

3235 is an implementation that uses interface smuggling

rgmz commented 1 month ago

@rgmz do you remember any details about that interface-smuggling related bug you found awhile ago? I want to ensure I don't trip over it again.

The bug was in how the nested structs / interface matching works. https://github.com/trufflesecurity/trufflehog/issues/2960

rosecodym commented 1 month ago

doing #3235 instead