Skip to content

Commit 81b3804

Browse files
authored
✨ Skip checks that don't apply to the current repo type (#5000)
* ✨ Skip checks that don't apply to the current repo type 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> * Fix repo-type filtering: cache docs, case-insensitive lookup, pass type 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> * Skip docs parsing when repoType is empty, use O(1) lookup map - 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> * Cache parsed checks.yaml with sync.Once in docs.Read() 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> * Fix lint: rename cachedErr to errCachedRead, fix var block ordering Signed-off-by: Jamie Magee <jamie.magee@gmail.com> --------- Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
1 parent 56e538e commit 81b3804

File tree

15 files changed

+162
-27
lines changed

15 files changed

+162
-27
lines changed

clients/azuredevopsrepo/repo.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ func (r *Repo) Metadata() []string {
100100
return r.metadata
101101
}
102102

103+
// Type implements Repo.Type.
104+
func (r *Repo) Type() clients.RepoType {
105+
return clients.RepoTypeAzureDevOps
106+
}
107+
103108
// Path() implements RepoClient.Path.
104109
func (r *Repo) Path() string {
105110
return fmt.Sprintf("%s/%s/%s/%s", r.organization, r.project, "_git", r.name)

clients/githubrepo/repo.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@ func (r *Repo) Metadata() []string {
112112
return r.metadata
113113
}
114114

115+
// Type implements Repo.Type.
116+
func (r *Repo) Type() clients.RepoType {
117+
return clients.RepoTypeGitHub
118+
}
119+
115120
func (r *Repo) commitExpression() string {
116121
if strings.EqualFold(r.commitSHA, clients.HeadSHA) {
117122
// TODO(#575): Confirm that this works as expected.

clients/gitlabrepo/repo.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,11 @@ func (r *Repo) Metadata() []string {
166166
return r.metadata
167167
}
168168

169+
// Type implements Repo.Type.
170+
func (r *Repo) Type() clients.RepoType {
171+
return clients.RepoTypeGitLab
172+
}
173+
169174
// Path() implements RepoClient.Path.
170175
func (r *Repo) Path() string {
171176
return fmt.Sprintf("%s/%s", r.owner, r.project)

clients/localdir/repo.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ func (r *Repo) AppendMetadata(m ...string) {
6969
r.metadata = append(r.metadata, m...)
7070
}
7171

72+
// Type implements Repo.Type.
73+
func (r *Repo) Type() clients.RepoType {
74+
return clients.RepoTypeLocal
75+
}
76+
7277
// Path() implements RepoClient.Path.
7378
func (r *Repo) Path() string {
7479
return r.path

clients/mockclients/repo.go

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

clients/repo.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,20 @@
1414

1515
package clients
1616

17+
// RepoType identifies the hosting platform of a repository.
18+
type RepoType string
19+
20+
const (
21+
// RepoTypeGitHub represents a GitHub-hosted repository.
22+
RepoTypeGitHub RepoType = "GitHub"
23+
// RepoTypeGitLab represents a GitLab-hosted repository.
24+
RepoTypeGitLab RepoType = "GitLab"
25+
// RepoTypeAzureDevOps represents an Azure DevOps-hosted repository.
26+
RepoTypeAzureDevOps RepoType = "Azure DevOps"
27+
// RepoTypeLocal represents a local directory.
28+
RepoTypeLocal RepoType = "local"
29+
)
30+
1731
// Repo interface uniquely identifies a repo.
1832
type Repo interface {
1933
// Path returns the specifier of the repository within its forge
@@ -29,4 +43,6 @@ type Repo interface {
2943
IsValid() error
3044
Metadata() []string
3145
AppendMetadata(metadata ...string)
46+
// Type returns the hosting platform type.
47+
Type() RepoType
3248
}

cmd/root.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func rootCmd(o *options.Options) error {
160160
// this call to policy is different from the one in scorecard.Run
161161
// this one is concerned with a policy file, while the scorecard.Run call is
162162
// more concerned with the supported request types
163-
enabledChecks, err := policy.GetEnabled(pol, o.Checks(), requiredRequestTypes)
163+
enabledChecks, err := policy.GetEnabled(pol, o.Checks(), requiredRequestTypes, "")
164164
if err != nil {
165165
return fmt.Errorf("GetEnabled: %w", err)
166166
}

cmd/serve.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (s *server) handleScorecard(w http.ResponseWriter, r *http.Request) {
154154
if !strings.EqualFold(opts.Commit, clients.HeadSHA) {
155155
requiredRequestTypes = append(requiredRequestTypes, checker.CommitBased)
156156
}
157-
enabledChecks, err := policy.GetEnabled(nil, opts.Checks(), requiredRequestTypes)
157+
enabledChecks, err := policy.GetEnabled(nil, opts.Checks(), requiredRequestTypes, repo.Type())
158158
if err != nil {
159159
http.Error(w, fmt.Sprintf("GetEnabled: %v", err), http.StatusInternalServerError)
160160
return

cron/internal/worker/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ func processRequest(ctx context.Context,
211211
commitSHA = repoReq.GetCommit()
212212
requiredRequestType = append(requiredRequestType, checker.CommitBased)
213213
}
214-
checksToRun, err := policy.GetEnabled(nil /*policy*/, nil /*checks*/, requiredRequestType)
214+
checksToRun, err := policy.GetEnabled(nil /*policy*/, nil /*checks*/, requiredRequestType, repo.Type())
215215
if err != nil {
216216
return fmt.Errorf("error during policy.GetEnabled: %w", err)
217217
}

docs/checks/impl.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"errors"
2020
"fmt"
2121
"strings"
22+
"sync"
2223

2324
"github.com/ossf/scorecard/v5/docs/checks/internal"
2425
sce "github.com/ossf/scorecard/v5/errors"
@@ -28,6 +29,12 @@ var errCheckNotExist = errors.New("check does not exist")
2829

2930
const docURL = "https://github.com/ossf/scorecard/blob/%s/docs/checks.md"
3031

32+
var (
33+
readOnce sync.Once
34+
errCachedRead error
35+
cachedDoc Doc
36+
)
37+
3138
// DocImpl implements `Doc` interface and
3239
// contains checks' documentation.
3340
//
@@ -37,15 +44,18 @@ type DocImpl struct {
3744
}
3845

3946
// Read loads the checks' documentation.
47+
// The embedded YAML is parsed once and cached for subsequent calls.
4048
func Read() (Doc, error) {
41-
m, e := internal.ReadDoc()
42-
if e != nil {
43-
d := DocImpl{}
44-
return &d, fmt.Errorf("internal.ReadDoc: %w", e)
45-
}
46-
47-
d := DocImpl{internaldoc: m}
48-
return &d, nil
49+
readOnce.Do(func() {
50+
m, e := internal.ReadDoc()
51+
if e != nil {
52+
cachedDoc = &DocImpl{}
53+
errCachedRead = fmt.Errorf("internal.ReadDoc: %w", e)
54+
return
55+
}
56+
cachedDoc = &DocImpl{internaldoc: m}
57+
})
58+
return cachedDoc, errCachedRead
4959
}
5060

5161
// GetCheck returns the information for check `name`.

0 commit comments

Comments
 (0)