* generate probe markdown documentation
Walks the various probes def.yaml files and puts them in a single
markdown document. This doesn't currently include the remediation, but
neither does the existing checks.md document either.
In order to avoid duplicating yaml definitions, this existing ones were
moved to an internal directory so they can be reused.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* fix linter
Signed-off-by: Spencer Schrock <sschrock@google.com>
* add probe doc generation to Makefile
Note: There is no validate-docs step for the probes code, as the
def.yml fields are validated elsewhere currently in the unit tests.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* fix license for new yaml package
Signed-off-by: Spencer Schrock <sschrock@google.com>
---------
Signed-off-by: Spencer Schrock <sschrock@google.com>
* clarify that link leads to specification, not REUSE in general
Signed-off-by: Max Mehl <mail@mehl.mx>
* fix LICENSES directory name
Signed-off-by: Max Mehl <mail@mehl.mx>
* clarify that tool also looks into LICENSES directory
Signed-off-by: Max Mehl <mail@mehl.mx>
* generate checks.md
Signed-off-by: Max Mehl <mail@mehl.mx>
---------
Signed-off-by: Max Mehl <mail@mehl.mx>
* add single probe for dependencyUpdateToolConfigured probe
Signed-off-by: Spencer Schrock <sschrock@google.com>
* delete individual update tool probes
Signed-off-by: Spencer Schrock <sschrock@google.com>
* use new update tool probe in evaluation
Signed-off-by: Spencer Schrock <sschrock@google.com>
* fix dependency update tool tests
The old test names were unclear, and didn't cover all supported tools.
Additionally the warn count changed since there's only one probe now,
instead of 3.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* clarify test name
Signed-off-by: Spencer Schrock <sschrock@google.com>
---------
Signed-off-by: Spencer Schrock <sschrock@google.com>
* check logger counts for SAST tests
previously, we only checked the result score.
test failures with this method dont produce as actionable feedback.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* clarify test names and score constants used
Signed-off-by: Spencer Schrock <sschrock@google.com>
* add generic sastToolConfigured probe
switch over the evaluation code to using the single probe with tool value.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* remove old probes
Signed-off-by: Spencer Schrock <sschrock@google.com>
* add tests
Signed-off-by: Spencer Schrock <sschrock@google.com>
* experiment with one readme
Signed-off-by: Spencer Schrock <sschrock@google.com>
* appease linter
Signed-off-by: Spencer Schrock <sschrock@google.com>
* remove colon from yaml which led to parse errors
Signed-off-by: Spencer Schrock <sschrock@google.com>
* polish documentation details
Signed-off-by: Spencer Schrock <sschrock@google.com>
---------
Signed-off-by: Spencer Schrock <sschrock@google.com>
* feat(branch-protection): consider if project requires PRs prior to make changes
As discussed at the issue #2727, we're adding the "require PRs prior
to make changes" as another requirement to tier 2. In addition to that,
we're changing the weight of the tier 2 requirements so that
"requiring 1 reviewer" has weight 2, while the other tier 2 requirements
have weight 1
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): increment and adapt testing
1. Adapt previous test cases to consider that now we'll have an aditional
Info log telling that the project requires PRs to make changes.
2. Add more cases to test relevant use cases on the tier 2 level of
branch protection
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* docs(branch-protection-check): adapt check description to consider requirement of require PRs to make changes
It adds the new tier 2 requirement, but also specify that the
"require at least 1 reviewer" will have doubled weight.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection-check): avoid duplicate funcions and enhance readability
Made some nice-to-have improvements on project readability,
making it easier easier to understand how the branch-protection
score is computed. Also unified 8 different functions that were
doing basically the same thing.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* feat(branch-protection): standardize values received on evaluation
Previously, at the evaluation part of branch protetion, the
values nil and false or zero were sort of interchangeble. This commit
changes the code to set as nil only the data that could not be retrieved
from github -- all the others would have values as false, zero, true, etc
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(github-client): adapt and add tests to check if nil values are coherent
1. Add new test to evaluate how we're interpreting a rule with all
checkboxes unchecked (most shouldn't be nil)
2. Adapt existent tests to expect non-nil values for unchecked
checkboxes
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* feat(client-github): avoid reusing bool pointers
Changes some pieces of code to prefer using pointers of
bool instantiated independently. If reusing bool pointers, at some piece
of code the value of the bool could inadvertently changed and it would change the
value of all other fields reusing that pointer.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* feat(branch-protection): enhance evaluation if scorecard was run by admin
At the evaluation step we were using some non untrusted fieldds of the
resposte to evaluate if Scorecard was run as admin or not. Now we're
using a field provided directly from the client file.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): adapt testings to say if they have admin info or not
After last commit, the client will tell the evaluation files if
Scorecard was run by administrator or not (i.e., if we have all the
infos). This commit adapts the testings to also provide this info.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(e2e-branch-protection): adapt number of logs after changes
- 2 warns (for 'last push approval' and 'codeowners review' disabled) were added because now those informations come as 'not-nil' at the evaluation part.
- 1 info was added to say that PRs are required to make changes
- 1 debug was removed because it said that we couldn't retrieve 'last push approval' information, but we actually can. It was just incorrectly set as nil
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* Revert the 2 commits with changes around how Scorecard detects admin run
Reverts commit 64c3521d89a6493e0d8c7527aa011f98c3e35719 and commit e2662b7173ef90b44b2d72c37614230440e8a919.
Both had chances around using clients/branch.go scructur to store the
information of whether Scorecard was being run by admin or not. We
decided to not change this structure for this purpose.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): change data structure to use pointer instead of value
At clients.BranchProtectionRule struct, changing
RequiredPullRequestReviews to be a pointer instead of a struct value.
This will allow the usage of the nil value of this structure to mean
that we can't say if the repository requires reviews or not.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* feat(branch-protection): use nil pointer on reviewers struct to mean
we don't know if they require PRs
The nil value of the struct RequiredPullRequestReviews will now mean
that we can't tell whether the project requires PRs to make changes or not.
When we get this case, we're printing a debug informing that we don't have
this data, but also printing a warn saying that they don't require
reviews, because that will be true at this case.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): if we're setting the reviewers struct to nil
when needed
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* doc(branch-protection): add code comment explaining different weight on tier 2 scores
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): avoid duplicate if branches on reviewers num comparation
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* docs(branch-protection): clarify commentings around data structure
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor: clean code on parsing GitHub BP data
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* feat(branch-protection): ressignify the nil PullRequestReviewRule to mean PR not required
Adapt translation of data from GitHub API, now for our internal data
modeling, having a nil PullRequestReviewRule structure will mean that
PRs are not required on the repo (can also mean we don't have data to
ensure that).
It also changes the order of the calls of copyNonAdminSettings and
copyAdminSettings to make the first one be called first. This eases the
code because the PullRequestReviewRule can be always instantiated at
this function.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): ensure we translate GitHub BP data as expected
Ensure we're correctly translating GitHub data from the old Branch
Protection config.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* feat(branch-protection): adapt score evaluation after 2efeee6512
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): adapt testings to changes of last commits
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* docs(branch-protection): add TODO comments pointing refactor opportunities
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* fix: avoid penalyzing non-admin for dismissStaleReview
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* fix(branch-protection): prevent false value from API field to become nil
When translating the API results, if the specific field `DismissesStaleReviews`
had a false value, it was not being initiated in our data model and was
remaining nil.
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor: clarify different weight on first reviewer
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor: enhance clarity of loggings and comments
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): new test to cover different rules affecting same branch
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* docs(branch-protection): change requirements ordering to keep admin ones together
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): simplify auxiliary function
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): fix code format to linter requirements
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): avoid unnecessary initializations and rename function
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): adapt test that was forgotten on commit 6858790a3e
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): use enums to represent tiers
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): remove nil fields of struct initialization when they dont contribute for clarification
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): simplify functions by using generics
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* docs(branch-protection): update docs after generate-docs run
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* fix(branch-protection): fix duplicated line on code
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* fix(branch-protection): stop exporting Tier enum
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* refactor(branch-protection): changing unchanged var to const
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* test(branch-protection): Rename test and adapt it to be consistent with its purpose
I also changed the test to not require PRs, as it's how it is when a new GitHub
Branch Protection config is created. The changes on the loggings numbers are due
to:
1. A warning for not having DismissStaleReviews became a debug
2. Removed the warning we had for not requiring CodeOwners
3. Have a new warning for not requiring PRe
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
---------
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* 🌱 convert Webhook check to probes
Signed-off-by: AdamKorcz <adam@adalogics.com>
* Add test + nits
Signed-off-by: AdamKorcz <adam@adalogics.com>
* replace probe with OutcomeNotApplicable
Signed-off-by: AdamKorcz <adam@adalogics.com>
* return one finding per webhook
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* change wording in def.yml
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* change wording in def.yml and checks.md
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* remove unused struct in test
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* align checks.md with checks.yaml
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* bring back experimental for webhooks
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* change 'token' to 'secret' in probe
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* use checker.MinResultScore instead of 0
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* Change test name
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* use checker.MinResultScore instead of 0
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* fix typo
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* Use checker.MaxResultScore instead of 10
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* rename probe
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* remove the 'totalWebhooks' value from findings
Signed-off-by: Adam Korczynski <adam@adalogics.com>
---------
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
* Remove EnforceAdmins from tier 1.
Scores in some tests either increase to 3, or 4, since EnfroceAdmins no longer keeps them in tier 1.
The number of Debug, Info, and Warn messages will decrease by 1 per branch, since we're no longer logging them.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* move enforce admins to tier 5.
Signed-off-by: Spencer Schrock <sschrock@google.com>
---------
Signed-off-by: Spencer Schrock <sschrock@google.com>
* repo rulesets via v4 api
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* good enough fnmatch implementation.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* good enough rulesMatchingBranch
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* apply matching repo rules to branch protection settings
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* rules: consider admins and require checks
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* non-structural chanages from PR feedback
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* fetch default branch name during repo rules query
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* Testing applyRepoRules
Tests assume a single rule is being applied to a branch, which might be
guarded by a legacy branch protection rule.
I think this logic gets problematic when there are multiple rules
overlaid on the same branch: the "the existing rules does not enforce
for admins, but i do and therefore this branch now does" will give
false-positives.
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* Test_applyRepoRules: builder and standardize names
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* attempt to upgrade/downgrade EnforceAdmins as each rule is applied
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
* simplify enforce admin for now.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* handle merging pull request reviews
Signed-off-by: Spencer Schrock <sschrock@google.com>
* handle merging check rules
Signed-off-by: Spencer Schrock <sschrock@google.com>
* handle last push approval
Signed-off-by: Spencer Schrock <sschrock@google.com>
* handle linear history
Signed-off-by: Spencer Schrock <sschrock@google.com>
* use constants for github rule types.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* add status check test.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* add e2e test for repo rules.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* handle nil branch name data
Signed-off-by: Spencer Schrock <sschrock@google.com>
* add tracking issue.
Signed-off-by: Spencer Schrock <sschrock@google.com>
* fix precedence in if statement
Signed-off-by: Spencer Schrock <sschrock@google.com>
* include repo rules in the check docs.
Signed-off-by: Spencer Schrock <sschrock@google.com>
---------
Signed-off-by: Peter Wagner <1559510+thepwagner@users.noreply.github.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Co-authored-by: Spencer Schrock <sschrock@google.com>
* Forgive all job-level permissions
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Update tests
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Replace magic number
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Rename test
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Test that multiple job-level permissions are forgiven
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Drop unused permissionIsPresent
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Update documentation
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Modify score descriptions
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* Document warning for job-level permissions
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* List job-level permissions that get WARNed
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
---------
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
* ✨ Detect fast-check PBT library for fuzz section
As suggested at https://github.com/ossf/scorecard/issues/2792#issuecomment-1562007596, we add support for the detection of fast-check as a possible fuzzing solution.
I also adapted the documentation related to fuzzing accordingly.
Signed-off-by: Nicolas DUBIEN <github@dubien.org>
* Typo
Signed-off-by: Nicolas DUBIEN <github@dubien.org>
* Update missing md files
Signed-off-by: Nicolas DUBIEN <github@dubien.org>
---------
Signed-off-by: Nicolas DUBIEN <github@dubien.org>
* doc: Updating gitlab support validation status
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* bug: Updated logic for gitlab to prevent exceptions based on releases
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* test: Added initial tests for gitlab branches
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* doc: Updated general README
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* refactor: Cleaned up the query for pipelines to be focused on the commitID
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* feat: Allowed for a non-graphql method of retrieving MRs associated to a commit
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* doc: Updated status for the CI-Tests
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
* bug: Updated the host url for graphql querying. This enabled the removal of the code added for handling empty returns when executing against a non-gitlab.com repository.
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
---------
Signed-off-by: Robison, Jim B <jim.b.robison@lmco.com>
Co-authored-by: Raghav Kaul <8695110+raghavkaul@users.noreply.github.com>
* fix anchor link to code-review in checks.yaml
Signed-off-by: dasfreak <dasfreak@users.noreply.github.com>
Signed-off-by: Marc Ohm <dasfreak@users.noreply.github.com>
* generate checks.md
Signed-off-by: Marc Ohm <dasfreak@users.noreply.github.com>
---------
Signed-off-by: dasfreak <dasfreak@users.noreply.github.com>
Signed-off-by: Marc Ohm <dasfreak@users.noreply.github.com>
This clarifies that AI/ML doesn't count as human code review.
This was earlier done in #2953 but that didn't modify the relevant
.yml file - this does.
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
* Clarify that AI/ML doesn't count as human code review
Add this clarification per the Scorecards Zoom call meeting today
(2023-05-04).
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
* Tweaked per review
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
---------
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
* Tweak Best Practices badge description to clarify things
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
* Provided clearer message when there's no BP badge detected
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
* Remove extra line that shouldn't be there
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
---------
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
On GitHub Branch-Protection configuration there was a rule called "Include administrator", which forced the admins to follow the same rules. It was renamed to "Do not allow bypassing the above settings", we we're updating scorecard accordingly
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
* Improve OSV scanning integration (squashed)
Signed-off-by: Rex P <rexpan@google.com>
* Add support for grouping vulnerabilities and aliases
Signed-off-by: Rex P <rexpan@google.com>
* Updated documentation, spit vulnerability output to multiple warnings
Signed-off-by: Rex P <rexpan@google.com>
* Updated documentation, spit vulnerability output to multiple warnings
Signed-off-by: Rex P <rexpan@google.com>
* Add its own codebase into docs
Signed-off-by: Rex P <rexpan@google.com>
* Update scorecard test to not prevent known vulns
Signed-off-by: Rex P <rexpan@google.com>
Signed-off-by: Rex P <rexpan@google.com>
Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
* Accept Go modules with semantic versions
This also fixes the current parser in two ways:
1. Do not detect `go INSTALL` but only `go install`.
2. Skip flags between `get`/`install` and packages but
record the flag `-insecure`.
Signed-off-by: favonia <favonia@gmail.com>
* Add five more offending cases ("none" is okay)
- v1
- v1.2
- latest
- patch
- upgrade
Signed-off-by: favonia <favonia@gmail.com>
* Add 8 insecure cases to Dockerfile-pkg-managers
RUN ["go", "get", "-insecure", "somerepo.com/org/name@v1.2.3"]
RUN ["go", "get", "-insecure", "somerepo.com/org/name@v1.2.3-semver"]
RUN ["go", "get", "-insecure", "somerepo.com/org/name@v1.2.3-semver+great"]
RUN ["go", "get", "somerepo.com/org/name@v1"]
RUN ["go", "get", "somerepo.com/org/name@v1.2"]
RUN ["go", "install", "somerepo.com/org/name@latest"]
RUN ["go", "install", "somerepo.com/org/name@patch"]
RUN ["go", "install", "somerepo.com/org/name@upgrade"]
Signed-off-by: favonia <favonia@gmail.com>
* Add 8 cases to github-workflow-pkg-managers.yaml
Also change 'go get' to 'go install' for special versions
"none", "patch", "upgrade", and "latest"
Signed-off-by: favonia <favonia@gmail.com>
Signed-off-by: favonia <favonia@gmail.com>
