✨ Skip checks that don't apply to the current repo type#5000
✨ Skip checks that don't apply to the current repo type#5000jeffmendoza merged 5 commits intoossf:mainfrom
Conversation
c2d5a81 to
3f7c0dc
Compare
3f7c0dc to
cc85a86
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5000 +/- ##
==========================================
+ Coverage 66.80% 69.78% +2.98%
==========================================
Files 230 252 +22
Lines 16602 15736 -866
==========================================
- Hits 11091 10982 -109
+ Misses 4808 3873 -935
- Partials 703 881 +178 🚀 New features to boost your workflow:
|
cc85a86 to
3403c1a
Compare
3403c1a to
9b476d5
Compare
There was a problem hiding this comment.
Pull request overview
This PR makes check selection aware of the repository’s hosting platform by introducing a Repo.Type() API and filtering enabled checks based on the repos field in docs/checks/internal/checks.yaml. This reduces misleading or error-prone results from forge-specific checks (notably for Azure DevOps repos).
Changes:
- Add
clients.RepoTypeand a newType() RepoTypemethod to theclients.Repointerface, implemented by GitHub/GitLab/Azure DevOps/local repos. - Extend
policy.GetEnabledto accept a repo type and skip checks whosechecks.yamlreposfield doesn’t include that type. - Update
checks.yamlrepo-type tags and add/adjust tests and call sites to pass repo type where available.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| policy/policy.go | Adds repo-type-aware filtering via isSupportedRepoType during check enablement. |
| policy/policy_test.go | Adds coverage for repo-type filtering behavior (Azure DevOps + permissive empty type). |
| pkg/scorecard/scorecard.go | Passes repo.Type() into policy.GetEnabled during a run. |
| pkg/scorecard/scorecard_test.go | Updates mocks to expect Repo.Type() calls. |
| docs/checks/internal/checks.yaml | Makes repos field load-bearing and tags additional checks for Azure DevOps. |
| cron/internal/worker/main.go | Passes repo.Type() into policy.GetEnabled for cron execution. |
| cmd/serve.go | Updates policy.GetEnabled call signature (currently passes empty repo type). |
| cmd/root.go | Updates policy.GetEnabled call signature (currently passes empty repo type). |
| clients/repo.go | Introduces RepoType enum and adds Type() to the repo interface. |
| clients/mockclients/repo.go | Updates generated mock to include Type(). |
| clients/localdir/repo.go | Implements Type() returning local. |
| clients/gitlabrepo/repo.go | Implements Type() returning GitLab. |
| clients/githubrepo/repo.go | Implements Type() returning GitHub. |
| clients/azuredevopsrepo/repo.go | Implements Type() returning Azure DevOps. |
Comments suppressed due to low confidence (2)
docs/checks/internal/checks.yaml:370
reposnow includes Azure DevOps, but the description states the check is limited to GitHub and relies on GitHub user profileCompany. Please reconcile this (update the description to match Azure DevOps behavior, or remove Azure DevOps fromrepos) so users aren’t misled and repo-type filtering is correct.
repos: GitHub, Azure DevOps
short: Determines if the project has a set of contributors from multiple organizations (e.g., companies).
description: |
Risk: `Low` (lower number of trusted code reviewers)
This check tries to determine if the project has recent contributors from
multiple organizations (e.g., companies). It is currently limited to
repositories hosted on GitHub, and does not support other source hosting
repositories (i.e., Forges).
docs/checks/internal/checks.yaml:543
reposnow includes Azure DevOps, but the description explicitly says the check is limited to GitHub and only detects GitHub apps / GitHub workflow usage. Please update the description to reflect Azure DevOps support (what signals are checked there), or drop Azure DevOps fromreposif support is not complete.
repos: GitHub, Azure DevOps
short: Determines if the project uses static code analysis.
description: |
Risk: `Medium` (possible unknown bugs)
This check tries to determine if the project uses Static Application Security
Testing (SAST), also known as [static code analysis](https://owasp.org/www-community/controls/Static_Code_Analysis).
It is currently limited to repositories hosted on GitHub, and does not support
other source hosting repositories (i.e., Forges).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@JamieMagee Can you take a look at the three unresolved copilot comments and consider if they are appropriate? The first one looks good (caching the parsed data). |
There was a problem hiding this comment.
Pull request overview
This PR makes check selection platform-aware by filtering enabled checks based on the repository’s hosting platform (as declared in docs/checks/internal/checks.yaml), preventing GitHub-specific checks from running on non-GitHub repos (notably Azure DevOps).
Changes:
- Extend
policy.GetEnabledto accept arepoTypeand skip checks whosereposlist inchecks.yamldoesn’t include that platform. - Add
Repo.Type() clients.RepoTypeto theclients.Repointerface and implement it across repo types and mocks; wirerepo.Type()into key call sites. - Update
checks.yamlto declare Azure DevOps support for 11 checks and add unit tests for repo-type filtering.
Show a summary per file
| File | Description |
|---|---|
| policy/policy.go | Adds repo-type-based filtering using docs-derived repos metadata. |
| policy/policy_test.go | Adds coverage for Azure DevOps filtering and permissive behavior when repo type is empty. |
| pkg/scorecard/scorecard.go | Passes repo.Type() into policy.GetEnabled during runs. |
| pkg/scorecard/scorecard_test.go | Updates mocks to expect Repo.Type() calls. |
| docs/checks/internal/checks.yaml | Updates repos: declarations to include Azure DevOps for multiple checks. |
| cron/internal/worker/main.go | Filters enabled checks by repo.Type() for cron worker execution. |
| cmd/serve.go | Filters enabled checks by repo.Type() for server execution path. |
| cmd/root.go | Updates GetEnabled call signature (still passes empty repo type). |
| clients/repo.go | Introduces RepoType and adds Type() RepoType to clients.Repo. |
| clients/mockclients/repo.go | Updates generated mock to include Type(). |
| clients/localdir/repo.go | Implements Type() as local. |
| clients/gitlabrepo/repo.go | Implements Type() as GitLab. |
| clients/githubrepo/repo.go | Implements Type() as GitHub. |
| clients/azuredevopsrepo/repo.go | Implements Type() as Azure DevOps. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (5)
policy/policy.go:160
docs.Read()errors are silently ignored when buildingrepoTypeLookup. If the embedded checks docs can't be read/unmarshaled, repo-type filtering is effectively disabled (all checks treated as supported), which can reintroduce the misleading results this change is trying to prevent. Consider returning an internal error whenrepoType != ""anddocs.Read()fails (or at least surfacing/logging it) instead of failing open.
if repoType != "" {
if d, err := docs.Read(); err == nil {
repoTypeLookup = make(map[string][]string)
for _, c := range d.GetChecks() {
repoTypeLookup[strings.ToLower(c.GetName())] = c.GetSupportedRepoTypes()
}
}
docs/checks/internal/checks.yaml:150
Branch-Protectionis now marked as supportingAzure DevOps, but the short/description text still specifically references "GitHub's branch protection" and links to GitHub-only settings. Either update the documentation to be forge-agnostic / describe Azure DevOps equivalents, or removeAzure DevOpsfromreposif the check isn't actually applicable there.
This issue also appears in the following locations of the same file:
- line 289
- line 362
- line 535
repos: GitHub, GitLab, Azure DevOps
short: Determines if the default and release branches are protected with GitHub's branch protection settings.
description: |
Risk: `High` (vulnerable to intentional malicious code injection)
This check determines whether a project's default and release branches are
protected with GitHub's [branch protection](https://docs.github.com/github/administering-a-repository/defining-the-mergeability-of-pull-requests/about-protected-branches)
or [repository rules](https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/about-rulesets) settings.
docs/checks/internal/checks.yaml:307
Code-Reviewis now marked as supportingAzure DevOps, but the description explicitly says it checks for "an approval on GitHub". Also, the implementation detects GitHub reviews viaAssociatedMergeRequest.Reviews/MergedBy(seechecks/raw/code_review.go), which Azure DevOps commits don't appear to populate, so enabling this on Azure risks incorrect scores. Either update the check+docs to correctly support Azure DevOps reviews, or removeAzure DevOpsfromreposfor this check.
repos: GitHub, Azure DevOps
short: Determines if the project requires human code review before pull requests (aka merge requests) are merged.
description: |
Risk: `High` (unintentional vulnerabilities or possible injection of malicious
code)
This check determines whether the project requires human code review
before pull requests (merge requests) are merged.
Reviews detect various unintentional problems, including vulnerabilities that
can be fixed immediately before they are merged, which improves the quality of
the code. Reviews may also detect or deter an attacker trying to insert
malicious code (either as a malicious contributor or as an attacker who has
subverted a contributor's account), because a reviewer might detect the
subversion.
The check determines whether the most recent changes (over the last ~30 commits) have
an approval on GitHub
or if the merger is different from the committer (implicit review). It also
docs/checks/internal/checks.yaml:372
Contributorsis now marked as supportingAzure DevOps, but the description still says it is "currently limited to repositories hosted on GitHub" and describes using the GitHub user profileCompanyfield. With repo-type filtering now enforced, this documentation needs to be updated to reflect Azure DevOps behavior (or removeAzure DevOpsfromreposif support isn't intended).
repos: GitHub, Azure DevOps
short: Determines if the project has a set of contributors from multiple organizations (e.g., companies).
description: |
Risk: `Low` (lower number of trusted code reviewers)
This check tries to determine if the project has recent contributors from
multiple organizations (e.g., companies). It is currently limited to
repositories hosted on GitHub, and does not support other source hosting
repositories (i.e., Forges).
The check looks at the `Company` field on the GitHub user profile for authors of
docs/checks/internal/checks.yaml:543
SASTis now marked as supportingAzure DevOps, but the description still says it is "currently limited to repositories hosted on GitHub" and the implementation looks for GitHub check runs and.github/workflows/*GitHub Actions usage. With repo-type filtering now enforced, either update the docs+implementation to properly support Azure DevOps pipelines, or dropAzure DevOpsfromreposto avoid misleading Azure results.
repos: GitHub, Azure DevOps
short: Determines if the project uses static code analysis.
description: |
Risk: `Medium` (possible unknown bugs)
This check tries to determine if the project uses Static Application Security
Testing (SAST), also known as [static code analysis](https://owasp.org/www-community/controls/Static_Code_Analysis).
It is currently limited to repositories hosted on GitHub, and does not support
other source hosting repositories (i.e., Forges).
- Files reviewed: 14/14 changed files
- Comments generated: 1
There was a problem hiding this comment.
Pull request overview
This PR makes check enablement forge-aware by filtering checks based on the repos field in docs/checks/internal/checks.yaml, so GitHub-specific checks don’t run (and mis-score/error) on other repo types like Azure DevOps.
Changes:
- Extend
clients.Repowith aType() RepoTypemethod and thread repo type intopolicy.GetEnabledto filter checks by supported platforms. - Cache
docs/checks.Read()viasync.Onceto avoid repeated embedded-YAML unmarshalling, and build a per-call lookup map for repo-type support. - Update
checks.yamlto mark additional checks as supporting Azure DevOps and add/adjust tests for filtering behavior.
Show a summary per file
| File | Description |
|---|---|
| policy/policy.go | Adds repo-type-aware filtering to enabled-check selection using check docs lookup. |
| policy/policy_test.go | Adds test cases covering Azure DevOps filtering and permissive behavior for empty repo type. |
| pkg/scorecard/scorecard.go | Passes repo.Type() into policy.GetEnabled during runs. |
| pkg/scorecard/scorecard_test.go | Updates mocks to expect Repo.Type() calls. |
| docs/checks/internal/checks.yaml | Expands repos declarations to include Azure DevOps for 11 checks. |
| docs/checks/impl.go | Caches parsed check docs using sync.Once to avoid repeated YAML parsing. |
| cron/internal/worker/main.go | Passes repo.Type() into policy.GetEnabled for cron runs. |
| cmd/serve.go | Passes repo.Type() into policy.GetEnabled for server requests. |
| cmd/root.go | Updates GetEnabled call signature (passes empty repo type). |
| clients/repo.go | Introduces RepoType and adds Type() to the exported clients.Repo interface. |
| clients/mockclients/repo.go | Regenerates mock to include Type() method. |
| clients/localdir/repo.go | Implements Repo.Type() returning local. |
| clients/gitlabrepo/repo.go | Implements Repo.Type() returning GitLab. |
| clients/githubrepo/repo.go | Implements Repo.Type() returning GitHub. |
| clients/azuredevopsrepo/repo.go | Implements Repo.Type() returning Azure DevOps. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (4)
docs/checks/internal/checks.yaml:236
- The
CI-Testsentry now lists GitLab/Azure DevOps inrepos, but the description text below still states the check is “currently limited to repositories hosted on GitHub” and “does not support other source hosting repositories”. Please update the documentation to match the declared supported repo types (or adjustreposif support is still GitHub-only).
This issue also appears in the following locations of the same file:
- line 359
- line 466
- line 532
CI-Tests:
risk: Low
tags: supply-chain, testing
repos: GitHub, GitLab, Azure DevOps
short: Determines if the project runs tests before pull requests are merged.
description: |
Risk: `Low` (possible unknown vulnerabilities)
This check tries to determine if the project runs tests before pull requests are
merged. It is currently limited to repositories hosted on GitHub, and does not
support other source hosting repositories (i.e., Forges). This check only
docs/checks/internal/checks.yaml:370
- The
Contributorsentry now lists Azure DevOps inrepos, but the description text below still says the check is “currently limited to repositories hosted on GitHub” and doesn’t support other forges. Please update the documentation to match the declared supported repo types (or adjustreposaccordingly).
Contributors:
risk: Low
tags: source-code
repos: GitHub, Azure DevOps
short: Determines if the project has a set of contributors from multiple organizations (e.g., companies).
description: |
Risk: `Low` (lower number of trusted code reviewers)
This check tries to determine if the project has recent contributors from
multiple organizations (e.g., companies). It is currently limited to
repositories hosted on GitHub, and does not support other source hosting
repositories (i.e., Forges).
docs/checks/internal/checks.yaml:478
- The
Pinned-Dependenciesentry now lists Azure DevOps inrepos, but the description text below still states the check is “currently limited to repositories hosted on GitHub” and doesn’t support other forges. Please update the documentation to match the declared supported repo types (or adjustreposaccordingly).
Pinned-Dependencies:
risk: Medium
tags: supply-chain, security, dependencies
repos: GitHub, Azure DevOps, local
short: Determines if the project has declared and pinned the dependencies of its build process.
description: |
Risk: `Medium` (possible compromised dependencies)
This check tries to determine if the project pins dependencies used during its build and release process.
A "pinned dependency" is a dependency that is explicitly set to a specific hash instead of
allowing a mutable version or range of versions. It
is currently limited to repositories hosted on GitHub, and does not support
other source hosting repositories (i.e., Forges).
docs/checks/internal/checks.yaml:543
- The
SASTentry now lists Azure DevOps inrepos, but the description text below still states the check is “currently limited to repositories hosted on GitHub” and doesn’t support other forges. Please update the documentation to match the declared supported repo types (or adjustreposaccordingly).
SAST:
risk: Medium
tags: supply-chain, security, testing
repos: GitHub, Azure DevOps
short: Determines if the project uses static code analysis.
description: |
Risk: `Medium` (possible unknown bugs)
This check tries to determine if the project uses Static Application Security
Testing (SAST), also known as [static code analysis](https://owasp.org/www-community/controls/Static_Code_Analysis).
It is currently limited to repositories hosted on GitHub, and does not support
other source hosting repositories (i.e., Forges).
- Files reviewed: 15/15 changed files
- Comments generated: 2
4e2ba30 to
f1c228c
Compare
Scorecard's Azure DevOps support runs every check regardless of whether it makes sense for the platform. Checks like Dangerous-Workflow and Token-Permissions look exclusively at GitHub Actions files and produce misleading results on non-GitHub repos. The repos field in checks.yaml already listed supported platforms per check, but nothing enforced it at runtime. Now GetEnabled reads that field and drops checks that don't list the repo's platform. - Add RepoType to the Repo interface (GitHub, GitLab, Azure DevOps, local) with implementations on all four concrete types and the mock - Pass RepoType through to policy.GetEnabled, which filters checks against checks.yaml's repos field - Tag 11 checks as supporting Azure DevOps based on which client methods are actually implemented Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
…pe in serve - Parse checks.yaml once per GetEnabled call instead of per-check - Use case-insensitive check name lookup so CLI args like 'binary-artifacts' don't bypass the repo-type filter - Pass repo.Type() in serve.go where the repo is already available Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
- Only call docs.Read() when repoType is non-empty - Build a lowercased name -> supported types map once per GetEnabled call instead of O(N) GetChecks() scan per check Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
The embedded YAML is immutable at runtime, so parsing it once and reusing the result is safe. Avoids repeated unmarshalling across GetEnabled calls in cron/multi-repo paths. Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
Head branch was pushed to by a user without write access
f1c228c to
58d8e69
Compare
What kind of change does this PR introduce?
Feature.
What is the current behavior?
All checks run on every repo regardless of platform. Checks like Dangerous-Workflow and Token-Permissions scan
.github/workflows/*and parse them withactionlint. On Azure DevOps repos these checks find nothing useful but still emit results, sometimes errors, sometimes bogus scores.What is the new behavior (if this is a feature change)?
GetEnabledreads thereposfield from checks.yaml and drops checks that don't list the repo's platform. TheRepointerface has a newType()method returningGitHub,GitLab,Azure DevOps, orlocal.11 checks are tagged as supporting Azure DevOps based on which client methods are implemented.
Which issue(s) this PR fixes
NONE
Special notes for your reviewer
The
reposfield in checks.yaml was documentation-only until now. This makes it load-bearing: if a check is missing a platform in itsreposfield, it won't run there.Does this PR introduce a user-facing change?
Yes. Azure DevOps users will see fewer irrelevant checks in their results.