Add allowlist feature to suppress test secrets

[jporzucek]

Description:

Implements whitelisting #1019

Checklist:

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

[nabeelalam]

Hey @jpozucek, thanks for contributing. I'm going through your PR but I've left a couple comments

[rosecodym]

Thanks for your work on this - the change looks pretty clean and surgical. I'd like to ask how much of a lift it would be to rename "whitelist" to "allowlist," simply because we're already using the latter term in the codebase, and we've historically found a minor but extant cognitive penalty when getting people up to speed on multiple concepts that are the same but have different names. (I'm not requesting changes for this in case it's more of a burden than I'm anticipating.)

[jporzucek]

@rosecodym Changed to allowlist, np!

[nabeelalam]

@jporzucek I tried to test allowlist-secrets with a yaml with a few known verified and unverified secrets, but even though the yaml file is loaded, the result still includes all of the allow listed secrets

[jporzucek]

@nabeelalam ahhh, rookie mistake! Please try again now

[jporzucek]

Also added support for correct parsing with YAML files that don't end with a newline character. The function now automatically appends \n if missing, improving parser compatibility and following POSIX standards.

[nabeelalam]

Thanks for the update @jporzucek I'll test it again in a little bit

[nabeelalam]

The flag is working for me now, thanks @jporzucek. This looks good! If it's not a big change, is it possible to make the flag itself a bit more accurately descriptive? --allowlist-secrets doesn't inherently indicate that it accepts a path to a yaml file. Just off the top of my head --secret-allowlist-path seems a touch more accurate. Perhaps you can think of a better one?

[jporzucek]

@nabeelalam Renamed to --allowlist-secrets-file - hope it's more accurate 🤞

[camgunz]

Thanks for this! The changes I want are using RedactGlobally instead of maskSecret and compiling the regexes up top. It's possible using slices.DeleteFunc doesn't make sense, so if it doesn't, definitely don't rework things to make it make sense.

[jporzucek]

@camgunz All requested changes applied! 🙌

[zricethezav]

I don't want to throw a wrench in this review, but what about introducing this feature in the --config file? https://github.com/trufflesecurity/trufflehog?tab=readme-ov-file#configuration

[jporzucek]

I don't want to throw a wrench in this review, but what about introducing this feature in the --config file? https://github.com/trufflesecurity/trufflehog?tab=readme-ov-file#configuration

You mean as a new field in sources?

[zricethezav]

You mean as a new field in sources?

I was thinking a new top-level key allowlist in the config that could be globally defined like "ignore these secrets if found in any detector" and detector specific allowlists like "ignore these secrets if found in this detector".

This behavior is similar to what is offered in the gitleaks configuration https://github.com/gitleaks/gitleaks?tab=readme-ov-file#configuration.

FWIW, I may be muddying the waters here but I figured since --config supports custom detectors and sources it could also include detector behaviors like allowlists filters.

[jporzucek]

@zricethezav I've added a top-level allowlists so it matches gitleaks naming convention. The behavior now is it merges allowlisted secrets from --config and --allowlist-secrets-file files. I'm not sure what's the expected behavior so let me know if one should have precedence over the other.

[jporzucek]

@zricethezav @nabeelalam @camgunz Can you take a look when you got a chance? 🙏