-
Notifications
You must be signed in to change notification settings - Fork 603
🐛 New check: Secret scanning #4878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4878 +/- ##
==========================================
+ Coverage 66.80% 68.44% +1.64%
==========================================
Files 230 268 +38
Lines 16602 16705 +103
==========================================
+ Hits 11091 11434 +343
+ Misses 4808 4386 -422
- Partials 703 885 +182 🚀 New features to boost your workflow:
|
|
This pull request has been marked stale because it has been open for 10 days with no activity |
|
This pull request has been marked stale because it has been open for 10 days with no activity |
spencerschrock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the groundwork Adam, I've tried to organize my thoughts ahead of the community meeting today.
High level:
I think this should be added, especially with the focus on baseline measurement. .
If the native platform scanning is the most popular, this might be a case similar to the webhooks check where the lack of an admin token may stop this from being on by default.
Mid level
The probes are broken down much too fine-grained in my opinion, to the point where we have N copies of identical stuff, one for each tool/platform. This is similar to how the fuzzing probes were originally before they were combined: #3877
At a high level I see 3 probes (names subject to change/tweak):
hasNativeSecretScanning
hasSecretPushProtection
hasThirdPartySecretScanning
Scoring
If we're thinking of this as an admin check, I would envision full points for preventing pushes, less for only reactively scanning, and none for none. Everything in between would need to be discussed further.
Low level
I didn't really try to go that low in my skim, I saw a few things, but not going to mention them yet until we align on high/mid level.
|
This pull request has been marked stale because it has been open for 10 days with no activity |
What kind of change does this PR introduce?
New check
PR title follows the guidelines defined in our pull request documentation
Tests for the changes have been added (for bug fixes/features)
Which issue(s) this PR fixes
#30
Special notes for your reviewer
This PR adds a new check for secret scanning. A project will score a high score if they have secret scanning and it runs it. The check covers multiple different ways of scanning for secrets:
3.1 gitleaks
3.2 TruffleHog
3.3 detect-secrets
3.4 git-secrets
3.5 ggshield
3.6 sshgit
3.7 repo-supervisor
Of these tools, the Scorecard Secret-Scanning check will both check if the project has either of these workflows - which will give the project a 1 score - and also whether the tools run. The first 5 tools are supposed to run on commits, and the last 2 run periodically. If they also run, the project will score another 7 points. Currently, the motivation to not score 10 in this case is that projects should enable Native GitHub scanning.
This can be tested out with
go run main.go --checks=SecretScanning --repo=github.com/owner/repo. Try for example to enable and disable native GitHub secret scanning in your own repository and see the project score 0 when secret scanning is disabled and 10 when it is enabled.In addition, try to run it on projects with some of the CI tools enabled or use my test repositories - each should score 1 for simply having the workflow and not having native GitHub secret scanning:
Currently, details like the scores are merely suggestions as a starting point for getting these formalized. Modifying them won't be a bit challenge, so if a project should score 3 for having a CI workflow without running it, that is fine.
Does this PR introduce a user-facing change?
Yes