Feat/Passing multiple arguments to dir #1750

recreator66 · 2025-02-11T20:41:35Z

Description:

Checklist:

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

recreator66 · 2025-02-11T20:47:25Z

PR is currently WIP. There is still some testing necessary if the changes have any impact on the core functionalities (e.g. passing config file path, gitleaks-ignore-path etc.)

I am happy for any volunteer that reviews the changes and tests Detector functionalities.

rgmz

This is a good base.

I have a few suggestions to streamline the approach a bit. You can apply this with git apply patch.txt: patch.txt

rgmz · 2025-02-16T19:36:57Z

cmd/directory.go

+	var allFindings []report.Finding
+
+	start := time.Now()
+	findingsMap := make(map[string]report.Finding)


What's the purpose of this map? Wouldn't it technically change the behaviour of how things are reported?

From my point of view the behavior itself would not change. The only reason for creating the map is to remove redundant findings by overriding them with the same fingerprint. Otherwise the report would not be accurate anymore when e.g. scanning a root directory with a child directory.

You can test this by scanning the ./testdata directory with ./testdata/repos/nogit as additional argument.

rgmz · 2025-02-16T19:38:47Z

cmd/directory.go

+		for _, finding := range findings {
+			findingsMap[finding.Fingerprint] = finding
+		}
+		detector = det


This is an unfortunate consequence of configuring report output under Detector. I don't know if there's a better way.

I see your point. In general, more testing is required from my end as new Detector structs will be initialized for each directory. This basically has an impact on all cli flag that you pass in.

Will think about a better way, ideally there is only one Detector initialized that stores all flags with e.g. absolute paths for config files etc

rgmz · 2025-03-11T02:37:13Z

Can you elaborate? It sounds like the results are the same?

Result of gitleaks detect --no-git testdata/expected/git -v and gitleaks detect --no-git . -v is the same.
...
I would have expected that the result ... is equal..

recreator66 · 2025-03-12T21:44:27Z

Update

Test Case 1

Compile both gitleaks binaries from source and target branch of this MR.
Execute following command with gitleaks binary from master branch
- gitleaks dir testdata/repos/staged - reports 2 leaks
- gitleaks dir testdata/repos/nogit - reports 1 leak
Execute following command with gitleaks binary from recreator66:feat/dir-multiple-args branch
- gitleaks dir testdata/repos/staged testdata/repos/nogit - reports 3 leaks

Test Case 2

Compile both gitleaks binaries from source and target branch of this MR.
Create a custom configuration toml file e.g. gitleaks-toml.zip
Execute following command with gitleaks binary from master branch
- gitleaks dir testdata/config -c gitleaks-test.toml - reports 1 leak
- gitleaks dir testdata/repos -c gitleaks-test.toml - reports 1 leak
Execute following command with gitleaks binary from recreator66:feat/dir-multiple-args branch
- gitleaks dir testdata/config testdata/repos -c gitleaks-test.toml - reports 2 leaks

Even with passing a custom toml I haven't noticed any issues with the changes.
The detailed reports can be compared with verbose mode/ flag.

Since the value of config flag is the only one used to setup the gitleaks config I assume that the changes made within this MR do not have an impact on how the detector is initialized.

To-Do

Test and adjust the logging while initializing new detectors. You can see e.g. in debug mode that gitleaks is logging the construction of additional detectors from L39. This logging needs to be suppressed.

Output from scanning 2 directories:

gitleaks dir testdata/config testdata/repos -c gitleaks-test.toml -l debug

    ○
    │╲
    │ ○
    ○ ░
    ░    gitleaks

10:38PM DBG using github.com/wasilibs/go-re2 regex engine
10:38PM DBG using gitleaks config gitleaks-test.toml from `--config`
10:38PM DBG found .gitleaksignore file: .gitleaksignore
10:38PM DBG using github.com/wasilibs/go-re2 regex engine
10:38PM DBG using gitleaks config gitleaks-test.toml from `--config`
10:38PM DBG found .gitleaksignore file: .gitleaksignore
10:38PM DBG Skipping symlink path=testdata/repos/symlinks/file_symlink/symlinked_id_ed25519
10:38PM INF scanned ~17007 bytes (17.01 KB) in 52.9ms
10:38PM WRN leaks found: 2

recreator66 · 2025-03-17T21:38:10Z

@rgmz I have adopted your approach to initialize the config once while iterating over the args (= directories). In addition, I enhanced the logging within the gitleaks directory command.

PR is ready for review

recreator66 · 2025-04-15T07:41:34Z

@rgmz is this PR still relevant?

UnknownPlatypus · 2025-04-19T14:53:01Z

Does the current state of the pr allows doing gitleaks dir file1 file2 ... ?

If so it's still very relevant to me to be able to fix the pre-commit integration (see #1409 (comment))

pre-commit run tools on a list of files (usually the one your are trying to commit) so I really need this PR behavior to make the pre-commit integration work properly.

recreator66 · 2025-04-23T21:42:12Z

@UnknownPlatypus yes the current state allows doing gitleaks dir file1 file2 ... and reports unique findings.

Holzhaus · 2025-07-22T17:43:28Z

@recreator66 Can you resolve the merge conflicts? Thanks!

@rgmz Anything else missing here?

I really would like to get #1409 fixed, and both this PR as well as #1918 are needed to fix the issue without introducing a major performance regression.

recreator66 · 2025-08-24T20:52:23Z

@Holzhaus this PR was already in a ready state back in March, but I haven’t received any feedback or review since then. I'm open to resolving the merge conflicts, but I’d prefer to avoid spending more time on it unless there's a clear intent to review and potentially merge it.

Let me know @rgmz

Holzhaus

LGTM, thank you!

Holzhaus · 2025-08-25T09:53:45Z

cmd/directory.go

+			logging.Err(scanErr).
+				Str("path", arg).
+				Msg("failed scan path")
+			err = scanErr


If multiple paths raise an error, only the last one will be kept in err, correct? If so, I guess it's not a problem because we still have them logged anyway.

Yes, your assumption is correct. Only the last error will be detected and passed to findingSummaryAndExit.

Anyway, since we log it the output looks like this (without verbose mode)

./gitleaks dir testdata/config123 testdata123 testdata/repos -c .gitleaks-test.toml ○ │╲ │ ○ ○ ░ ░ gitleaks 9:09PM WRN skipping error="lstat testdata/config123: no such file or directory" path=testdata/config123 9:09PM WRN skipping error="lstat testdata123: no such file or directory" path=testdata123 9:09PM INF scanned ~0 bytes (0) in 27.3ms 9:09PM WRN leaks found: 1

Holzhaus · 2025-08-25T09:56:42Z

@recreator66 Maybe you can consider using the "Request review" function in the upper right corner to request a review from @rgmz and/or @zricethezav.

recreator66 added 3 commits February 11, 2025 21:38

chore: prevent printing banner multiple times

3f55b13

feat: add logic to handle multiple arguments

b896585

chore: go fmt

08bddf3

recreator66 marked this pull request as draft February 11, 2025 20:43

rgmz reviewed Feb 16, 2025

View reviewed changes

UnknownPlatypus mentioned this pull request Feb 22, 2025

gitleaks does not scan correct files with pre-commit run --files #1409

Open

recreator66 added 2 commits March 12, 2025 20:56

refactor: apply patch.txt

9b8be67

refactor: apply patch.txt & unpack findings

4d791ff

perf: initialize config only once

2462fdb

recreator66 marked this pull request as ready for review March 17, 2025 21:35

jrdnbradford mentioned this pull request May 10, 2025

Set pass_filenames to false for Docker hook #1850

Merged

3 tasks

danielsparing mentioned this pull request May 24, 2025

Feature Request: Support for gitleaks (or trufflehog) nbQA-dev/nbQA#885

Open

UnknownPlatypus mentioned this pull request Jul 18, 2025

Fix pre-commit hook by use git-agnostic file-based checking #1918

Open

3 tasks

Holzhaus approved these changes Aug 25, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into feat/dir-multiple-args

81fc668

Uh oh!

Feat/Passing multiple arguments to dir #1750

Are you sure you want to change the base?

Feat/Passing multiple arguments to dir #1750

Uh oh!

Conversation

recreator66 commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Uh oh!

recreator66 commented Feb 11, 2025

Uh oh!

rgmz left a comment

Choose a reason for hiding this comment

Uh oh!

rgmz Feb 16, 2025

Choose a reason for hiding this comment

Uh oh!

recreator66 Feb 19, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

recreator66 Feb 19, 2025

Choose a reason for hiding this comment

Uh oh!

rgmz commented Mar 11, 2025

Uh oh!

recreator66 commented Mar 12, 2025

Uh oh!

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

Uh oh!

Uh oh!

recreator66 commented Apr 15, 2025

Uh oh!

UnknownPlatypus commented Apr 19, 2025

Uh oh!

recreator66 commented Apr 23, 2025

Uh oh!

Holzhaus commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

recreator66 commented Aug 24, 2025

Uh oh!

Holzhaus left a comment

Choose a reason for hiding this comment

Uh oh!

Holzhaus Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

recreator66 Aug 31, 2025

Choose a reason for hiding this comment

Uh oh!

Holzhaus commented Aug 25, 2025

Uh oh!

Uh oh!

recreator66 commented Feb 11, 2025 •

edited

Loading

rgmz Feb 16, 2025 •

edited

Loading

recreator66 commented Mar 17, 2025 •

edited

Loading

Holzhaus commented Jul 22, 2025 •

edited

Loading