Skip to content

Conversation

@gg-jonathangriffe
Copy link
Contributor

@gg-jonathangriffe gg-jonathangriffe commented Oct 22, 2024

Context

In the GitGuardian vscode extension, we want to be able to avoid scanning files that are in the .gitignore.

The secret scan path command has an use-gitignore option that could do this, however it currently only applies to directories scanned recursively and not single files.

This MR proposes to also apply it to single files, which would allow the use of this argument in the vscode extension to avoid scanning gitignored files. This would also match the behavior of ignored paths in the .gitguardian.yaml config, for which an error is raised if an ignored directory is passed to the secret scan path command.

Validation

  • In a repo, put a file in the .gitignore
  • run ggshield secret scan path --use-gitignore <your_file>
    No file should be scanned.
    -->

PR check list

  • As much as possible, the changes include tests (unit and/or functional)
  • If the changes affect the end user (new feature, behavior change, bug fix) then the PR has a changelog entry (see doc/dev/getting-started.md). If the changes do not affect the end user, then the skip-changelog label has been added to the PR.

@gg-jonathangriffe gg-jonathangriffe self-assigned this Oct 22, 2024
@codecov
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.58%. Comparing base (118ca6d) to head (8647d93).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #988   +/-   ##
=======================================
  Coverage   91.58%   91.58%           
=======================================
  Files         180      180           
  Lines        7605     7607    +2     
=======================================
+ Hits         6965     6967    +2     
  Misses        640      640           
Flag Coverage Δ
unittests 91.58% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@salome-voltz salome-voltz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

Copy link
Collaborator

@agateau-gg agateau-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. I am just a bit worried about performances in the case where someone calls secret scan path with dozens of file names, because with these changes we run quite a few git commands for each file 😅.

But that's a corner case, just something to keep in mind.

@gg-jonathangriffe gg-jonathangriffe merged commit 635ad8c into main Oct 23, 2024
28 checks passed
@gg-jonathangriffe gg-jonathangriffe deleted the jgriffe/SCRT-4922/gitignore-paths branch October 23, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants