Define multiple global allowlists #1777

rgmz · 2025-03-01T01:53:32Z

Description:

This is a follow-up to #1496. It allows defining multiple global allowlists with independent AND/OR conditions.

Multiple Allowlists

Supports multiple top-level allowlists with AND/OR conditions, similar to multiple rule allowlists added in #1496.

[[allowlists]]
regexes = ["^changeit$"]
[[allowlists]]
condition = "AND"
paths = ["^node_modules/.*"]
stopwords = ["mock"]

Targeted / Reuable Allowlists

Instead of copying & pasting allowlists between multiples rules, the top-level allowlists now have targetRules which will be assigned to the rules at runtime. Allowlists that have targetRules are not added to the global allowlists. (Inspired by Finatext/gls.)

Example: #1777 (comment).

Miscellaneous Changes

config.go
- Include title in the parsed config (inconsequential other than Create command to print config #1654 I think)
- Unified parsing of allowlists, instead of duplicating the logic.
- Replaced instances of logging.Fatal() with returning an error to simplify tests
- Extended config's global allowlists are appended, instead of being merged in
detect.go
- Unified allowlist logic with checkCommitOrPathAllowed and checkFindingAllowed functions, rather than large amounts of duplicated code
Tests
- Added new test cases for the feature
- Refactored/reorganized existing test cases to clearly differentiate what is being tested, and whether it's a valid/invalid config

Checklist:

Does your PR pass tests?
Have you written new tests for your changes?
Have you lint your code locally prior to submission?

rgmz · 2025-03-29T15:23:17Z

@zricethezav @bplaxco What are your thoughts on this feature (conceptually, ignoring the current ugly code)? It feels quite useful, especially for large configs that have duplicated allowlists that may not be suitable for the global allowlist (e.g., leaktk/patterns#64).

The following are equivalent:

Before

[[rules]]
id = "github-app-token"
regex = '''(?:ghu|ghs)_[0-9a-zA-Z]{36}'''
[[rules.allowlists]]
paths = [
    '''(?:^|/)@octokit/auth-token/README\.md$''',
]

[[rules]]
id = "github-oauth"
regex = '''gho_[0-9a-zA-Z]{36}'''

[[rules]]
id = "github-pat"
regex = '''ghp_[0-9a-zA-Z]{36}'''
[[rules.allowlists]]
paths = [
    '''(?:^|/)@octokit/auth-token/README\.md$''',
]

After

[[rules]]
id = "github-app-token"
regex = '''(?:ghu|ghs)_[0-9a-zA-Z]{36}'''

[[rules]]
id = "github-oauth"
regex = '''gho_[0-9a-zA-Z]{36}'''

[[rules]]
id = "github-pat"
regex = '''ghp_[0-9a-zA-Z]{36}'''

[[allowlists]]
ruleIDs = ["github-app-token", "github-pat"]
paths = [
    '''(?:^|/)@octokit/auth-token/README\.md$''',
]

bplaxco · 2025-03-29T17:03:21Z

@rgmz multiple global allowlists makes sense to me. It mirrors what's on the rules, and I like the idea of having the ability to have a global allowlist with an AND condition apart from the other normal large global allowlist. It'd probably also simplify the code (if this hasn't already been done) for extending existing allowlist config. You'd just append the extended allowlist rather than mutating the existing global one.

bplaxco · 2025-03-29T17:08:55Z

(e.g., leaktk/patterns#64).

Heh aside: in that specific MR I also added support for a # pragma :include: "filename" macro for scripts/compile so I could reference snippets over and over again instead of having to update the same list in a lot of different places. I could have probably added a # pragma :include: "common_password_hash_allowlist" for those.

All that to say, I don't think I'd want gitleaks to allow variables or anything like that for me in the config—I like gitleaks taking a simple static config, but for writing large configs, having a dynamic source is really handy.

rgmz · 2025-04-02T12:59:02Z

It'd probably also simplify the code (if this hasn't already been done) for extending existing allowlist config. You'd just append the extended allowlist rather than mutating the existing global one.

Good point. I've updated the logic to simplify extending:

gitleaks/config/config.go

Lines 441 to 444 in e7beb14

    
           // append allowlists, not attempting to merge 
        
           for _, a := range extensionConfig.Allowlists { 
        
           	c.Allowlists = append(c.Allowlists, a) 
        
           }

zricethezav · 2025-04-02T14:00:03Z

@zricethezav @bplaxco What are your thoughts on this feature (conceptually, ignoring the current ugly code)? It feels quite useful, especially for large configs that have duplicated allowlists that may not be suitable for the global allowlist (e.g., leaktk/patterns#64).

Conceptually, I agree with this proposal. What do you need from me to get this PR from a draft state to a ready state?

rgmz · 2025-04-06T01:27:21Z

What do you need from me to get this PR from a draft state to a ready state?

After a second pass, I'm much happier with how this looks. I think it's ready for review now.

Forgive the large number of files moved. I did shuffle around a bunch of tests in config_test.go and detect_test.go to get a better sense of the cases.

zricethezav · 2025-04-14T19:43:37Z

config/gitleaks.toml

@@ -13,23 +13,9 @@

 title = "gitleaks config"

+# TODO: change to [[allowlists]]


@rgmz in a fast follow release?

Yeah, one or two minor versions later to avoid the compatibility issue from last time. In the future, something like #1754 could catch such issues.

README.md

zricethezav · 2025-04-14T20:00:20Z

@rgmz could you update the description to be a little more ... descriptive :P, namely targetRules being added and some of the other quality of life improvements you added like moving the tests around and creating some helper functions for config assembly?

zricethezav · 2025-04-14T20:12:06Z

config/config.go

@@ -60,18 +62,22 @@ type viperRuleAllowlist struct {
 	StopWords   []string
 }

+type viperGlobalAllowlist struct {
+	TargetRules        []string
+	viperRuleAllowlist `mapstructure:",squash"`


huh, TIL https://pkg.go.dev/github.com/mitchellh/mapstructure#hdr-Embedded_Structs_and_Squashing

I went a bit crazy at first trying to get parseAllowlist to work.

rgmz · 2025-04-14T20:56:18Z

@rgmz could you update the description to be a little more ... descriptive :P, namely targetRules being added and some of the other quality of life improvements you added like moving the tests around and creating some helper functions for config assembly?

Great point -- I've updated the PR description. LMK if you feel there's anything I've missed.

rgmz · 2025-04-29T15:11:08Z

FYI @taiki45.

Updates gitleaks to v8.26.0. Gitleaks added the ability to have multiple configured allow lists with gitleaks/gitleaks#1777 , so this also adjusts how we add the configured allow list to the detector's config. Signed-off-by: Mikhail Swift <mikhail@testifysec.com>

rgmz force-pushed the feat/multiple-global-allowlists branch 4 times, most recently from 6a3a2b0 to 2b4f451 Compare March 1, 2025 03:59

rgmz force-pushed the feat/multiple-global-allowlists branch 9 times, most recently from c914c1d to 72583b8 Compare March 29, 2025 15:14

rgmz force-pushed the feat/multiple-global-allowlists branch 3 times, most recently from ff2ce76 to e7beb14 Compare April 2, 2025 12:57

rgmz force-pushed the feat/multiple-global-allowlists branch 2 times, most recently from bbc0c7c to 06b0164 Compare April 2, 2025 13:11

rgmz force-pushed the feat/multiple-global-allowlists branch 7 times, most recently from f00162e to eb3676b Compare April 5, 2025 17:42

rgmz force-pushed the feat/multiple-global-allowlists branch 5 times, most recently from 21f7201 to af8e1b4 Compare April 6, 2025 01:06

rgmz marked this pull request as ready for review April 6, 2025 01:24

rgmz force-pushed the feat/multiple-global-allowlists branch 2 times, most recently from bbc408d to f9b1dea Compare April 12, 2025 16:13

zricethezav reviewed Apr 14, 2025

View reviewed changes

README.md Show resolved Hide resolved

zricethezav reviewed Apr 14, 2025

View reviewed changes

README.md Show resolved Hide resolved

zricethezav reviewed Apr 14, 2025

View reviewed changes

rgmz force-pushed the feat/multiple-global-allowlists branch from f9b1dea to e52b2cf Compare April 14, 2025 20:36

feat(config): define multiple global allowlists

bb47e30

rgmz force-pushed the feat/multiple-global-allowlists branch from e52b2cf to bb47e30 Compare April 14, 2025 20:54

zricethezav approved these changes Apr 29, 2025

View reviewed changes

zricethezav merged commit 4451b45 into gitleaks:master Apr 29, 2025
2 checks passed

rgmz deleted the feat/multiple-global-allowlists branch April 29, 2025 15:09

This was referenced Apr 30, 2025

Fix gitleaks Enterprise-CMCS/macpro-mdct-mcr#12140

Merged

pin gitleaks version to prevent config errors Enterprise-CMCS/macpro-mdct-mfp#921

Merged

rocio-desantiago mentioned this pull request Apr 30, 2025

[QMR] Fix gitleaks Enterprise-CMCS/macpro-mdct-qmr#2595

Merged

gmrabian mentioned this pull request May 7, 2025

Fix gitleaks Enterprise-CMCS/macpro-mdct-carts#139931

Merged

mikhailswift mentioned this pull request May 13, 2025

fix: update gitleaks and fix config usage in-toto/go-witness#499

Merged

5 tasks

alayne222 pushed a commit to alayne222/gitleaks that referenced this pull request May 28, 2025

feat(config): define multiple global allowlists (gitleaks#1777)

43728d5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Define multiple global allowlists #1777

Define multiple global allowlists #1777

Uh oh!

rgmz commented Mar 1, 2025 •

edited

Loading

Uh oh!

rgmz commented Mar 29, 2025

Uh oh!

bplaxco commented Mar 29, 2025

Uh oh!

bplaxco commented Mar 29, 2025

Uh oh!

rgmz commented Apr 2, 2025

Uh oh!

zricethezav commented Apr 2, 2025

Uh oh!

rgmz commented Apr 6, 2025

Uh oh!

zricethezav Apr 14, 2025

Uh oh!

rgmz Apr 14, 2025

Uh oh!

Uh oh!

Uh oh!

zricethezav commented Apr 14, 2025

Uh oh!

zricethezav Apr 14, 2025

Uh oh!

rgmz Apr 14, 2025

Uh oh!

rgmz commented Apr 14, 2025

Uh oh!

Uh oh!

rgmz commented Apr 29, 2025

Uh oh!

Uh oh!

		@@ -13,23 +13,9 @@

		title = "gitleaks config"

		# TODO: change to [[allowlists]]

Uh oh!

Define multiple global allowlists #1777

Define multiple global allowlists #1777

Uh oh!

Conversation

rgmz commented Mar 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Uh oh!

rgmz commented Mar 29, 2025

Uh oh!

bplaxco commented Mar 29, 2025

Uh oh!

bplaxco commented Mar 29, 2025

Uh oh!

rgmz commented Apr 2, 2025

Uh oh!

zricethezav commented Apr 2, 2025

Uh oh!

rgmz commented Apr 6, 2025

Uh oh!

zricethezav Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

rgmz Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zricethezav commented Apr 14, 2025

Uh oh!

zricethezav Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

rgmz Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

rgmz commented Apr 14, 2025

Uh oh!

Uh oh!

rgmz commented Apr 29, 2025

Uh oh!

Uh oh!

rgmz commented Mar 1, 2025 •

edited

Loading