Allow list bugs (#484)

* fix allowlist bug

config/allowlist.go

@@ -47,7 +47,7 @@ func (a *AllowList) RepoAllowed(repo string) bool {
 
 // IgnoreDotGit appends a `.git$` rule to ignore all .git paths. This is used for --no-git scans
 func (a *AllowList) IgnoreDotGit() error {
-	re, err := regexp.Compile(".git$")
+	re, err := regexp.Compile(".git")
 	if err != nil {
 		return err
 	}

config/default.go

@@ -159,6 +159,6 @@ title = "gitleaks config"
 [allowlist]
 	description = "Allowlisted files"
 	files = ['''^\.?gitleaks.toml$''',
-	'''(.*?)(jpg|gif|doc|pdf|bin)$''',
+	'''(.*?)(png|jpg|gif|doc|docx|pdf|bin|xls|pyc|zip)$''',
 	'''(go.mod|go.sum)$''']
 `

config/rule.go

@@ -79,6 +79,9 @@ func (r *Rule) HasFileOrPathLeakOnly(filePath string) bool {
 	if len(r.Entropies) != 0 {
 		return false
 	}
+	if r.AllowList.FileAllowed(filepath.Base(filePath)) || r.AllowList.PathAllowed(filePath) {
+		return false
+	}
 	return r.HasFileLeak(filepath.Base(filePath)) || r.HasFilePathLeak(filePath)
 }
 

go.mod

@@ -2,7 +2,7 @@ module github.com/zricethezav/gitleaks/v7
 
 go 1.15
 
-replace github.com/go-git/go-git/v5 => github.com/zricethezav/go-git/v5 v5.2.1
+replace github.com/go-git/go-git/v5 => github.com/zricethezav/go-git/v5 v5.2.2
 
 require (
 	github.com/BurntSushi/toml v0.3.1

go.sum

@@ -68,6 +68,8 @@ github.com/xanzy/ssh-agent v0.2.1 h1:TCbipTQL2JiiCprBWx9frJ2eJlCYT00NmctrHxVAr70
 github.com/xanzy/ssh-agent v0.2.1/go.mod h1:mLlQY/MoOhWBj+gOGMQkOeiEvkx+8pJSI+0Bx9h2kr4=
 github.com/zricethezav/go-git/v5 v5.2.1 h1:snaoKv8ksDSz7NfBRXsBr9Yr8IKEKWRWf1zdhFmcFvI=
 github.com/zricethezav/go-git/v5 v5.2.1/go.mod h1:kh02eMX+wdqqxgNMEyq8YgwlIOsDOa9homkUq1PoTMs=
+github.com/zricethezav/go-git/v5 v5.2.2 h1:VOVijF5OpIiHvFZXX94AJezdJ0b0kTyRyVNZ4yaqCRU=
+github.com/zricethezav/go-git/v5 v5.2.2/go.mod h1:kh02eMX+wdqqxgNMEyq8YgwlIOsDOa9homkUq1PoTMs=
 golang.org/x/crypto v0.0.0-20190219172222-a4c6cb3142f2/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073 h1:xMPOj6Pz6UipU1wXLkrtqpHbR0AVFnyPEQq/wRWz9lM=

scan/commit.go

@@ -103,14 +103,17 @@ func (cs *CommitScanner) Scan() (Report, error) {
 				// Check the actual content
 				for _, line := range strings.Split(chunk.Content(), "\n") {
 					for _, rule := range cs.cfg.Rules {
+						if rule.AllowList.FileAllowed(filepath.Base(to.Path())) ||
+							rule.AllowList.PathAllowed(to.Path()) ||
+							rule.AllowList.CommitAllowed(cs.commit.Hash.String()) {
+							continue
+						}
 						offender := rule.Inspect(line)
 						if offender == "" {
 							continue
 						}
-						if cs.cfg.Allowlist.RegexAllowed(line) ||
-							rule.AllowList.FileAllowed(filepath.Base(to.Path())) ||
-							rule.AllowList.PathAllowed(to.Path()) ||
-							rule.AllowList.CommitAllowed(cs.commit.Hash.String()) {
+
+						if cs.cfg.Allowlist.RegexAllowed(line) {
 							continue
 						}
 

scan/filesatcommit.go

@@ -81,15 +81,19 @@ func (fs *FilesAtCommitScanner) Scan() (Report, error) {
 
 		for i, line := range strings.Split(content, "\n") {
 			for _, rule := range fs.cfg.Rules {
+				if rule.AllowList.FileAllowed(filepath.Base(f.Name)) ||
+					rule.AllowList.PathAllowed(f.Name) ||
+					rule.AllowList.CommitAllowed(fs.commit.Hash.String()) {
+					continue
+				}
+
 				offender := rule.Inspect(line)
 
 				if offender == "" {
 					continue
 				}
-				if fs.cfg.Allowlist.RegexAllowed(line) ||
-					rule.AllowList.FileAllowed(filepath.Base(f.Name)) ||
-					rule.AllowList.PathAllowed(f.Name) ||
-					rule.AllowList.CommitAllowed(fs.commit.Hash.String()) {
+
+				if fs.cfg.Allowlist.RegexAllowed(line) {
 					continue
 				}
 

scan/nogit.go

@@ -7,11 +7,11 @@ import (
 	"path/filepath"
 	"strings"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/zricethezav/gitleaks/v7/config"
 	"github.com/zricethezav/gitleaks/v7/options"
 
-	log "github.com/sirupsen/logrus"
-
 	"golang.org/x/sync/errgroup"
 )
 
@@ -36,7 +36,6 @@ func NewNoGitScanner(opts options.Options, cfg config.Config) *NoGitScanner {
 		log.Error(err)
 		return nil
 	}
-
 	return ngs
 }
 
@@ -95,13 +94,17 @@ func (ngs *NoGitScanner) Scan() (Report, error) {
 				lineNumber++
 				for _, rule := range ngs.cfg.Rules {
 					line := scanner.Text()
+
+					if rule.AllowList.FileAllowed(filepath.Base(p)) ||
+						rule.AllowList.PathAllowed(p) {
+						continue
+					}
+
 					offender := rule.Inspect(line)
 					if offender == "" {
 						continue
 					}
-					if ngs.cfg.Allowlist.RegexAllowed(line) ||
-						rule.AllowList.FileAllowed(filepath.Base(p)) ||
-						rule.AllowList.PathAllowed(p) {
+					if ngs.cfg.Allowlist.RegexAllowed(line) {
 						continue
 					}
 

scan/scan_test.go

@@ -15,7 +15,10 @@ import (
 const testRepoBase = "../test_data/test_repos/"
 
 func TestScan(t *testing.T) {
-	moveDotGit("dotGit", ".git")
+	err := moveDotGit("dotGit", ".git")
+	if err != nil {
+		t.Fatal(err)
+	}
 	defer moveDotGit(".git", "dotGit")
 	tests := []struct {
 		description string
@@ -32,6 +35,7 @@ func TestScan(t *testing.T) {
 				Path:         "../test_data/test_repos/test_repo_1",
 				Report:       "../test_data/test_local_repo_one_aws_leak.json.got",
 				ReportFormat: "json",
+				Threads:      runtime.GOMAXPROCS(0),
 			},
 			wantPath: "../test_data/test_local_repo_one_aws_leak.json",
 		},
@@ -120,6 +124,7 @@ func TestScan(t *testing.T) {
 				Report:       "../test_data/test_local_repo_two_leaks_file_commit_range.json.got",
 				ReportFormat: "json",
 				CommitsFile:  "../test_data/test_options/test_local_repo_commits.txt",
+				Threads:      runtime.GOMAXPROCS(0),
 			},
 			wantPath: "../test_data/test_local_repo_two_leaks_file_commit_range.json",
 		},
@@ -219,6 +224,7 @@ func TestScan(t *testing.T) {
 				Report:       "../test_data/test_regex_entropy.json.got",
 				ConfigPath:   "../test_data/test_configs/regex_entropy.toml",
 				ReportFormat: "json",
+				Threads:      runtime.GOMAXPROCS(0),
 			},
 			wantPath: "../test_data/test_regex_entropy.json",
 		},
@@ -425,6 +431,38 @@ func TestScan(t *testing.T) {
 			},
 			wantPath: "../test_data/test_only_files_no_git.json",
 		},
+		{
+			description: "test allowlist files",
+			opts: options.Options{
+				Path:         "../test_data/test_repos/test_repo_10",
+				Report:       "../test_data/test_allow_list_file.json.got",
+				ReportFormat: "json",
+				ConfigPath:   "../test_data/test_configs/allowlist_files.toml",
+			},
+			wantPath: "../test_data/test_allow_list_file.json",
+		},
+		{
+			description: "test allowlist files no-git",
+			opts: options.Options{
+				Path:         "../test_data/test_repos/test_repo_10",
+				Report:       "../test_data/test_allow_list_file_no_git.json.got",
+				ReportFormat: "json",
+				ConfigPath:   "../test_data/test_configs/allowlist_files.toml",
+				NoGit:        true,
+			},
+			wantPath: "../test_data/test_allow_list_file_no_git.json",
+		},
+		{
+			description: "test allowlist docx no-git",
+			opts: options.Options{
+				Path:         "../test_data/test_repos/test_repo_10",
+				Report:       "../test_data/test_allow_list_docx_no_git.json.got",
+				ReportFormat: "json",
+				ConfigPath:   "../test_data/test_configs/allowlist_docx.toml",
+				NoGit:        true,
+			},
+			wantPath: "../test_data/test_allow_list_docx_no_git.json",
+		},
 	}
 
 	for _, test := range tests {
@@ -708,6 +746,14 @@ func moveDotGit(from, to string) error {
 		return err
 	}
 	for _, dir := range repoDirs {
+		if to == ".git" {
+			_, err := os.Stat(fmt.Sprintf("%s/%s/%s", testRepoBase, dir.Name(), "dotGit"))
+			if os.IsNotExist(err) {
+				// dont want to delete the only copy of .git accidentally
+				continue
+			}
+			os.RemoveAll(fmt.Sprintf("%s/%s/%s", testRepoBase, dir.Name(), ".git"))
+		}
 		if !dir.IsDir() {
 			continue
 		}
@@ -721,6 +767,7 @@ func moveDotGit(from, to string) error {
 		if err != nil {
 			return err
 		}
+		// fmt.Println("RENAMED")
 	}
 	return nil
 }

test_data/test_allow_list_docx_no_git.json

@@ -0,0 +1,18 @@
+[
+ {
+  "line": "",
+  "lineNumber": 1,
+  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_10/bad.zip",
+  "commit": "",
+  "repo": "",
+  "repoURL": "",
+  "leakURL": "",
+  "rule": "Block dangerous filetypes",
+  "commitMessage": "",
+  "author": "",
+  "email": "",
+  "file": "../test_data/test_repos/test_repo_10/bad.zip",
+  "date": "0001-01-01T00:00:00Z",
+  "tags": "key, extensions"
+ }
+]

test_data/test_allow_list_file.json

@@ -0,0 +1,18 @@
+[
+ {
+  "line": "",
+  "lineNumber": 1,
+  "offender": "Filename or path offender: tmp/bad.docx",
+  "commit": "b0f9b62dfe12e4e10de180359c6b9276472494f8",
+  "repo": "test_repo_10",
+  "repoURL": "",
+  "leakURL": "",
+  "rule": "Block dangerous filetypes",
+  "commitMessage": "Create bad.docx",
+  "author": "Zachary Rice",
+  "email": "zricer@protonmail.com",
+  "file": "tmp/bad.docx",
+  "date": "2020-12-09T11:02:10-05:00",
+  "tags": "key, extensions"
+ }
+]

test_data/test_allow_list_file_no_git.json

@@ -0,0 +1,18 @@
+[
+ {
+  "line": "",
+  "lineNumber": 1,
+  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_10/tmp/bad.docx",
+  "commit": "",
+  "repo": "",
+  "repoURL": "",
+  "leakURL": "",
+  "rule": "Block dangerous filetypes",
+  "commitMessage": "",
+  "author": "",
+  "email": "",
+  "file": "../test_data/test_repos/test_repo_10/tmp/bad.docx",
+  "date": "0001-01-01T00:00:00Z",
+  "tags": "key, extensions"
+ }
+]

test_data/test_configs/allowlist_docx.toml

@@ -0,0 +1,11 @@
+[[rules]]
+	description = "Block dangerous filetypes"
+	file = '''(.*?)(pdf|doc|docx|zip|xls|tfplan|tfstate|tfvars|vault_pass|vagrant|pyc|key|cache)$'''
+	tags = ["key", "extensions"]
+	[rules.allowlist]
+		paths = ['''.docx''']
+		description = "ignore known locations and files"
+
+#[allowlist]
+#	description = "Allowlisted files"
+#	paths = ['''.zip''']

test_data/test_configs/allowlist_files.toml

@@ -0,0 +1,11 @@
+[[rules]]
+	description = "Block dangerous filetypes"
+	file = '''(.*?)(pdf|doc|docx|zip|xls|tfplan|tfstate|tfvars|vault_pass|vagrant|pyc|key|cache)$'''
+	tags = ["key", "extensions"]
+	[rules.allowlist]
+		paths = ['''.zip''']
+		description = "ignore known locations and files"
+#
+#[allowlist]
+#	description = "Allowlisted files"
+#	paths = ['''.zip''']

test_data/test_only_files_no_git.json

@@ -2,7 +2,7 @@
  {
   "line": "",
   "lineNumber": 1,
-  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_2/no_secrets.md",
+  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_10/README.md",
   "commit": "",
   "repo": "",
   "repoURL": "",
@@ -11,7 +11,7 @@
   "commitMessage": "",
   "author": "",
   "email": "",
-  "file": "../test_data/test_repos/test_repo_2/no_secrets.md",
+  "file": "../test_data/test_repos/test_repo_10/README.md",
   "date": "0001-01-01T00:00:00Z",
   "tags": "key, extensions"
  },
@@ -31,6 +31,22 @@
   "date": "0001-01-01T00:00:00Z",
   "tags": "key, extensions"
  },
+ {
+  "line": "",
+  "lineNumber": 1,
+  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_2/no_secrets.md",
+  "commit": "",
+  "repo": "",
+  "repoURL": "",
+  "leakURL": "",
+  "rule": "flag go",
+  "commitMessage": "",
+  "author": "",
+  "email": "",
+  "file": "../test_data/test_repos/test_repo_2/no_secrets.md",
+  "date": "0001-01-01T00:00:00Z",
+  "tags": "key, extensions"
+ },
  {
   "line": "",
   "lineNumber": 1,
@@ -66,7 +82,7 @@
  {
   "line": "",
   "lineNumber": 1,
-  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_4/no_secrets.md",
+  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_4/secrets.md",
   "commit": "",
   "repo": "",
   "repoURL": "",
@@ -75,14 +91,14 @@
   "commitMessage": "",
   "author": "",
   "email": "",
-  "file": "../test_data/test_repos/test_repo_4/no_secrets.md",
+  "file": "../test_data/test_repos/test_repo_4/secrets.md",
   "date": "0001-01-01T00:00:00Z",
   "tags": "key, extensions"
  },
  {
   "line": "",
   "lineNumber": 1,
-  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_4/secrets.md",
+  "offender": "Filename or path offender: ../test_data/test_repos/test_repo_4/no_secrets.md",
   "commit": "",
   "repo": "",
   "repoURL": "",
@@ -91,7 +107,7 @@
   "commitMessage": "",
   "author": "",
   "email": "",
-  "file": "../test_data/test_repos/test_repo_4/secrets.md",
+  "file": "../test_data/test_repos/test_repo_4/no_secrets.md",
   "date": "0001-01-01T00:00:00Z",
   "tags": "key, extensions"
  },

test_data/test_repos/test_repo_10/.gitleaks.toml

@@ -0,0 +1,11 @@
+[[rules]]
+	description = "Block dangerous filetypes"
+	file = '''(.*?)(pdf|doc|docx|zip|xls|tfplan|tfstate|tfvars|vault_pass|vagrant|pyc|key|cache)$'''
+	tags = ["key", "extensions"]
+	[rules.allowlist]
+		paths = ['''.git''', '''.zip''']
+		description = "ignore known locations and files"
+
+[allowlist]
+	description = "Allowlisted files"
+	paths = ['''.zip''']

test_data/test_repos/test_repo_10/README.md

@@ -0,0 +1 @@
+# test_repo_paths

test_data/test_repos/test_repo_10/bad.zip

@@ -0,0 +1 @@
+this is a bad .zip

test_data/test_repos/test_repo_10/dotGit/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/main

test_data/test_repos/test_repo_10/dotGit/config

@@ -0,0 +1,13 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true
+	precomposeunicode = true
+[remote "origin"]
+	url = git@github.com:zricethezav/test_repo_paths.git
+	fetch = +refs/heads/*:refs/remotes/origin/*
+[branch "main"]
+	remote = origin
+	merge = refs/heads/main

test_data/test_repos/test_repo_10/dotGit/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

test_data/test_repos/test_repo_10/dotGit/info/exclude

@@ -0,0 +1,6 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~

test_data/test_repos/test_repo_10/dotGit/logs/HEAD

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 5cd9652a24771400b4ce76beadbb2496a73341b8 Zach Rice <zrice@gitlab.com> 1607529853 -0500	clone: from github.com:zricethezav/test_repo_paths.git

test_data/test_repos/test_repo_10/dotGit/logs/refs/heads/main

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 5cd9652a24771400b4ce76beadbb2496a73341b8 Zach Rice <zrice@gitlab.com> 1607529853 -0500	clone: from github.com:zricethezav/test_repo_paths.git

test_data/test_repos/test_repo_10/dotGit/logs/refs/remotes/origin/HEAD

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 5cd9652a24771400b4ce76beadbb2496a73341b8 Zach Rice <zrice@gitlab.com> 1607529853 -0500	clone: from github.com:zricethezav/test_repo_paths.git

test_data/test_repos/test_repo_10/dotGit/packed-refs

@@ -0,0 +1,2 @@
+# pack-refs with: peeled fully-peeled sorted 
+5cd9652a24771400b4ce76beadbb2496a73341b8 refs/remotes/origin/main

test_data/test_repos/test_repo_10/dotGit/refs/heads/main

@@ -0,0 +1 @@
+5cd9652a24771400b4ce76beadbb2496a73341b8

test_data/test_repos/test_repo_10/dotGit/refs/remotes/origin/HEAD

@@ -0,0 +1 @@
+ref: refs/remotes/origin/main

test_data/test_repos/test_repo_10/tmp/bad.docx

@@ -0,0 +1 @@
+bad .docx