diff --git a/clients/azuredevopsrepo/repo.go b/clients/azuredevopsrepo/repo.go index f3fca4ef453..99362c67e04 100644 --- a/clients/azuredevopsrepo/repo.go +++ b/clients/azuredevopsrepo/repo.go @@ -100,6 +100,11 @@ func (r *Repo) Metadata() []string { return r.metadata } +// Type implements Repo.Type. +func (r *Repo) Type() clients.RepoType { + return clients.RepoTypeAzureDevOps +} + // Path() implements RepoClient.Path. func (r *Repo) Path() string { return fmt.Sprintf("%s/%s/%s/%s", r.organization, r.project, "_git", r.name) diff --git a/clients/githubrepo/repo.go b/clients/githubrepo/repo.go index da58b93a1d8..4cfde8f422e 100644 --- a/clients/githubrepo/repo.go +++ b/clients/githubrepo/repo.go @@ -112,6 +112,11 @@ func (r *Repo) Metadata() []string { return r.metadata } +// Type implements Repo.Type. +func (r *Repo) Type() clients.RepoType { + return clients.RepoTypeGitHub +} + func (r *Repo) commitExpression() string { if strings.EqualFold(r.commitSHA, clients.HeadSHA) { // TODO(#575): Confirm that this works as expected. diff --git a/clients/gitlabrepo/repo.go b/clients/gitlabrepo/repo.go index a0cc0bcd54d..338d6f6bf3d 100644 --- a/clients/gitlabrepo/repo.go +++ b/clients/gitlabrepo/repo.go @@ -166,6 +166,11 @@ func (r *Repo) Metadata() []string { return r.metadata } +// Type implements Repo.Type. +func (r *Repo) Type() clients.RepoType { + return clients.RepoTypeGitLab +} + // Path() implements RepoClient.Path. func (r *Repo) Path() string { return fmt.Sprintf("%s/%s", r.owner, r.project) diff --git a/clients/localdir/repo.go b/clients/localdir/repo.go index 925b2442a76..de01cf16024 100644 --- a/clients/localdir/repo.go +++ b/clients/localdir/repo.go @@ -69,6 +69,11 @@ func (r *Repo) AppendMetadata(m ...string) { r.metadata = append(r.metadata, m...) } +// Type implements Repo.Type. +func (r *Repo) Type() clients.RepoType { + return clients.RepoTypeLocal +} + // Path() implements RepoClient.Path. func (r *Repo) Path() string { return r.path diff --git a/clients/mockclients/repo.go b/clients/mockclients/repo.go index aeef513252c..d7b00b252c3 100644 --- a/clients/mockclients/repo.go +++ b/clients/mockclients/repo.go @@ -28,6 +28,8 @@ import ( reflect "reflect" gomock "go.uber.org/mock/gomock" + + clients "github.com/ossf/scorecard/v5/clients" ) // MockRepo is a mock of Repo interface. @@ -153,3 +155,17 @@ func (mr *MockRepoMockRecorder) URI() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "URI", reflect.TypeOf((*MockRepo)(nil).URI)) } + +// Type mocks base method. +func (m *MockRepo) Type() clients.RepoType { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Type") + ret0, _ := ret[0].(clients.RepoType) + return ret0 +} + +// Type indicates an expected call of Type. +func (mr *MockRepoMockRecorder) Type() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Type", reflect.TypeOf((*MockRepo)(nil).Type)) +} diff --git a/clients/repo.go b/clients/repo.go index 120841a1a6e..e7b0f346b25 100644 --- a/clients/repo.go +++ b/clients/repo.go @@ -14,6 +14,20 @@ package clients +// RepoType identifies the hosting platform of a repository. +type RepoType string + +const ( + // RepoTypeGitHub represents a GitHub-hosted repository. + RepoTypeGitHub RepoType = "GitHub" + // RepoTypeGitLab represents a GitLab-hosted repository. + RepoTypeGitLab RepoType = "GitLab" + // RepoTypeAzureDevOps represents an Azure DevOps-hosted repository. + RepoTypeAzureDevOps RepoType = "Azure DevOps" + // RepoTypeLocal represents a local directory. + RepoTypeLocal RepoType = "local" +) + // Repo interface uniquely identifies a repo. type Repo interface { // Path returns the specifier of the repository within its forge @@ -29,4 +43,6 @@ type Repo interface { IsValid() error Metadata() []string AppendMetadata(metadata ...string) + // Type returns the hosting platform type. + Type() RepoType } diff --git a/cmd/root.go b/cmd/root.go index cde5b8012b1..32724d4db46 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -160,7 +160,7 @@ func rootCmd(o *options.Options) error { // this call to policy is different from the one in scorecard.Run // this one is concerned with a policy file, while the scorecard.Run call is // more concerned with the supported request types - enabledChecks, err := policy.GetEnabled(pol, o.Checks(), requiredRequestTypes) + enabledChecks, err := policy.GetEnabled(pol, o.Checks(), requiredRequestTypes, "") if err != nil { return fmt.Errorf("GetEnabled: %w", err) } diff --git a/cmd/serve.go b/cmd/serve.go index d301e15d47e..f8ba1c34049 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -154,7 +154,7 @@ func (s *server) handleScorecard(w http.ResponseWriter, r *http.Request) { if !strings.EqualFold(opts.Commit, clients.HeadSHA) { requiredRequestTypes = append(requiredRequestTypes, checker.CommitBased) } - enabledChecks, err := policy.GetEnabled(nil, opts.Checks(), requiredRequestTypes) + enabledChecks, err := policy.GetEnabled(nil, opts.Checks(), requiredRequestTypes, repo.Type()) if err != nil { http.Error(w, fmt.Sprintf("GetEnabled: %v", err), http.StatusInternalServerError) return diff --git a/cron/internal/worker/main.go b/cron/internal/worker/main.go index 756b48eaf2c..b0e8225fb79 100644 --- a/cron/internal/worker/main.go +++ b/cron/internal/worker/main.go @@ -211,7 +211,7 @@ func processRequest(ctx context.Context, commitSHA = repoReq.GetCommit() requiredRequestType = append(requiredRequestType, checker.CommitBased) } - checksToRun, err := policy.GetEnabled(nil /*policy*/, nil /*checks*/, requiredRequestType) + checksToRun, err := policy.GetEnabled(nil /*policy*/, nil /*checks*/, requiredRequestType, repo.Type()) if err != nil { return fmt.Errorf("error during policy.GetEnabled: %w", err) } diff --git a/docs/checks/impl.go b/docs/checks/impl.go index 45461d9b20a..5403d106b4d 100644 --- a/docs/checks/impl.go +++ b/docs/checks/impl.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "strings" + "sync" "github.com/ossf/scorecard/v5/docs/checks/internal" sce "github.com/ossf/scorecard/v5/errors" @@ -28,6 +29,12 @@ var errCheckNotExist = errors.New("check does not exist") const docURL = "https://github.com/ossf/scorecard/blob/%s/docs/checks.md" +var ( + readOnce sync.Once + errCachedRead error + cachedDoc Doc +) + // DocImpl implements `Doc` interface and // contains checks' documentation. // @@ -37,15 +44,18 @@ type DocImpl struct { } // Read loads the checks' documentation. +// The embedded YAML is parsed once and cached for subsequent calls. func Read() (Doc, error) { - m, e := internal.ReadDoc() - if e != nil { - d := DocImpl{} - return &d, fmt.Errorf("internal.ReadDoc: %w", e) - } - - d := DocImpl{internaldoc: m} - return &d, nil + readOnce.Do(func() { + m, e := internal.ReadDoc() + if e != nil { + cachedDoc = &DocImpl{} + errCachedRead = fmt.Errorf("internal.ReadDoc: %w", e) + return + } + cachedDoc = &DocImpl{internaldoc: m} + }) + return cachedDoc, errCachedRead } // GetCheck returns the information for check `name`. diff --git a/docs/checks/internal/checks.yaml b/docs/checks/internal/checks.yaml index bab6591a16f..cd2b2aa4048 100644 --- a/docs/checks/internal/checks.yaml +++ b/docs/checks/internal/checks.yaml @@ -51,7 +51,7 @@ checks: Dependency-Update-Tool: risk: High tags: supply-chain, security, dependencies - repos: GitHub, local + repos: GitHub, Azure DevOps, local short: Determines if the project uses a dependency update tool. description: | Risk: `High` (possibly vulnerable to attacks on known flaws) @@ -89,7 +89,7 @@ checks: Binary-Artifacts: risk: High tags: supply-chain, security, dependencies - repos: GitHub, GitLab, local + repos: GitHub, GitLab, Azure DevOps, local short: Determines if the project has generated executable (binary) artifacts in the source repository. description: | Risk: `High` (non-reviewable code) @@ -140,7 +140,7 @@ checks: Branch-Protection: risk: High tags: supply-chain, security, source-code, code-reviews - repos: GitHub, GitLab + 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) @@ -226,7 +226,7 @@ checks: CI-Tests: risk: Low tags: supply-chain, testing - repos: GitHub, GitLab + repos: GitHub, GitLab, Azure DevOps short: Determines if the project runs tests before pull requests are merged. description: | Risk: `Low` (possible unknown vulnerabilities) @@ -286,7 +286,7 @@ checks: Code-Review: risk: High tags: supply-chain, security, source-code, code-reviews - repos: GitHub + 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 @@ -359,7 +359,7 @@ checks: Contributors: risk: Low tags: source-code - repos: GitHub + 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) @@ -466,7 +466,7 @@ checks: Pinned-Dependencies: risk: Medium tags: supply-chain, security, dependencies - repos: GitHub, local + 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) @@ -532,7 +532,7 @@ checks: SAST: risk: Medium tags: supply-chain, security, testing - repos: GitHub + repos: GitHub, Azure DevOps short: Determines if the project uses static code analysis. description: | Risk: `Medium` (possible unknown bugs) @@ -603,7 +603,7 @@ checks: Security-Policy: risk: Medium short: Determines if the project has published a security policy. - repos: GitHub + repos: GitHub, Azure DevOps tags: supply-chain, security, policy description: | Risk: `Medium` (possible insecure reporting of vulnerabilities) @@ -746,7 +746,7 @@ checks: Vulnerabilities: risk: High tags: supply-chain, security, vulnerabilities - repos: GitHub + repos: GitHub, Azure DevOps short: Determines if the project has open, known unfixed vulnerabilities. description: | Risk: `High` (known vulnerabilities) @@ -810,7 +810,7 @@ checks: License: risk: Low tags: license - repos: GitHub, local + repos: GitHub, Azure DevOps, local short: Determines if the project has defined a license. description: | Risk: `Low` (possible impediment to security review) diff --git a/pkg/scorecard/scorecard.go b/pkg/scorecard/scorecard.go index 3c2528f884b..86a70cf8a02 100644 --- a/pkg/scorecard/scorecard.go +++ b/pkg/scorecard/scorecard.go @@ -425,7 +425,7 @@ func Run(ctx context.Context, repo clients.Repo, opts ...Option) (Result, error) requiredRequestTypes = append(requiredRequestTypes, checker.CommitBased) } - checksToRun, err := policy.GetEnabled(nil, c.checks, requiredRequestTypes) + checksToRun, err := policy.GetEnabled(nil, c.checks, requiredRequestTypes, repo.Type()) if err != nil { return Result{}, fmt.Errorf("getting enabled checks: %w", err) } diff --git a/pkg/scorecard/scorecard_test.go b/pkg/scorecard/scorecard_test.go index 8228d9bfe15..d2fe1a7870a 100644 --- a/pkg/scorecard/scorecard_test.go +++ b/pkg/scorecard/scorecard_test.go @@ -166,6 +166,7 @@ func TestRun(t *testing.T) { repo := mockrepo.NewMockRepo(ctrl) repo.EXPECT().URI().Return(tt.args.uri).AnyTimes() + repo.EXPECT().Type().Return(clients.RepoTypeGitHub).AnyTimes() mockRepoClient.EXPECT().InitRepo(repo, tt.args.commitSHA, 0).Return(nil) @@ -281,6 +282,7 @@ func TestRun_WithProbes(t *testing.T) { repo.EXPECT().URI().Return(tt.args.uri).AnyTimes() repo.EXPECT().Host().Return("github.com").AnyTimes() + repo.EXPECT().Type().Return(clients.RepoTypeGitHub).AnyTimes() mockRepoClient.EXPECT().InitRepo(repo, tt.args.commitSHA, 0).Return(nil) mockRepoClient.EXPECT().URI().Return(repo.URI()).AnyTimes() diff --git a/policy/policy.go b/policy/policy.go index d088e6943e8..dfe2d9cb00b 100644 --- a/policy/policy.go +++ b/policy/policy.go @@ -25,6 +25,8 @@ import ( "github.com/ossf/scorecard/v5/checker" "github.com/ossf/scorecard/v5/checks" + "github.com/ossf/scorecard/v5/clients" + docs "github.com/ossf/scorecard/v5/docs/checks" sce "github.com/ossf/scorecard/v5/errors" ) @@ -143,9 +145,21 @@ func GetEnabled( sp *ScorecardPolicy, argsChecks []string, requiredRequestTypes []checker.RequestType, + repoType clients.RepoType, ) (checker.CheckNameToFnMap, error) { enabledChecks := checker.CheckNameToFnMap{} + // Build a case-insensitive repo-type lookup map once, only when needed. + var repoTypeLookup map[string][]string + 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() + } + } + } + switch { case len(argsChecks) != 0: // Populate checks to run with the `--repo` CLI argument. @@ -156,6 +170,9 @@ func GetEnabled( fmt.Sprintf("Unsupported RequestType %s by check: %s", fmt.Sprint(requiredRequestTypes), checkName)) } + if !isSupportedRepoType(repoTypeLookup, checkName, repoType) { + continue + } if !enableCheck(checkName, &enabledChecks) { return enabledChecks, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("invalid check: %s", checkName)) @@ -165,9 +182,9 @@ func GetEnabled( // Populate checks to run with policy file. for checkName := range sp.GetPolicies() { if !isSupportedCheck(checkName, requiredRequestTypes) { - // We silently ignore the check, like we do - // for the default case when no argsChecks - // or policy are present. + continue + } + if !isSupportedRepoType(repoTypeLookup, checkName, repoType) { continue } @@ -182,6 +199,9 @@ func GetEnabled( if !isSupportedCheck(checkName, requiredRequestTypes) { continue } + if !isSupportedRepoType(repoTypeLookup, checkName, repoType) { + continue + } if !enableCheck(checkName, &enabledChecks) { return enabledChecks, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("invalid check: %s", checkName)) @@ -216,6 +236,27 @@ func isSupportedCheck(checkName string, requiredRequestTypes []checker.RequestTy return len(unsupported) == 0 } +// isSupportedRepoType checks if a check supports the given repo type +// using a pre-built lookup map. If the map is nil (docs unavailable or +// repoType empty) or the check is not found, it defaults to supported. +func isSupportedRepoType(repoTypeLookup map[string][]string, checkName string, repoType clients.RepoType) bool { + if repoTypeLookup == nil { + return true + } + + supportedTypes, exists := repoTypeLookup[strings.ToLower(checkName)] + if !exists { + return true + } + + for _, t := range supportedTypes { + if strings.EqualFold(t, string(repoType)) { + return true + } + } + return false +} + // Enables checks by name. func enableCheck(checkName string, enabledChecks *checker.CheckNameToFnMap) bool { if enabledChecks != nil { diff --git a/policy/policy_test.go b/policy/policy_test.go index 42c482991ab..c7ba899cea6 100644 --- a/policy/policy_test.go +++ b/policy/policy_test.go @@ -23,6 +23,7 @@ import ( "github.com/ossf/scorecard/v5/checker" "github.com/ossf/scorecard/v5/checks" + "github.com/ossf/scorecard/v5/clients" sce "github.com/ossf/scorecard/v5/errors" ) @@ -312,6 +313,7 @@ func TestGetEnabled(t *testing.T) { policyFile string argsChecks []string requiredRequestTypes []checker.RequestType + repoType clients.RepoType expectedEnabledChecks int expectedError bool }{ @@ -319,6 +321,7 @@ func TestGetEnabled(t *testing.T) { name: "checks limited to those specified by checks arg", argsChecks: []string{"Binary-Artifacts"}, requiredRequestTypes: []checker.RequestType{checker.FileBased}, + repoType: clients.RepoTypeGitHub, expectedEnabledChecks: 1, expectedError: false, }, @@ -326,6 +329,7 @@ func TestGetEnabled(t *testing.T) { name: "mix of supported and unsupported checks", argsChecks: []string{"Binary-Artifacts", "UnsupportedCheck"}, requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased}, + repoType: clients.RepoTypeGitHub, expectedEnabledChecks: 1, expectedError: true, }, @@ -333,6 +337,7 @@ func TestGetEnabled(t *testing.T) { name: "request types limit enabled checks", argsChecks: []string{}, requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased}, + repoType: clients.RepoTypeGitHub, expectedEnabledChecks: 7, // All checks which are FileBased and CommitBased expectedError: false, }, @@ -341,9 +346,34 @@ func TestGetEnabled(t *testing.T) { policyFile: "testdata/policy-ok.yaml", argsChecks: []string{}, requiredRequestTypes: []checker.RequestType{}, + repoType: clients.RepoTypeGitHub, expectedEnabledChecks: 3, expectedError: false, }, + { + name: "azure devops repo type filters GitHub-only checks", + argsChecks: []string{"Binary-Artifacts", "Dangerous-Workflow"}, + requiredRequestTypes: []checker.RequestType{}, + repoType: clients.RepoTypeAzureDevOps, + expectedEnabledChecks: 1, // Only Binary-Artifacts supports Azure DevOps + expectedError: false, + }, + { + name: "empty repo type is permissive", + argsChecks: []string{"Binary-Artifacts", "Dangerous-Workflow"}, + requiredRequestTypes: []checker.RequestType{}, + repoType: "", + expectedEnabledChecks: 2, // Both enabled when repo type is empty + expectedError: false, + }, + { + name: "default checks filtered by azure devops repo type", + argsChecks: []string{}, + requiredRequestTypes: []checker.RequestType{}, + repoType: clients.RepoTypeAzureDevOps, + expectedEnabledChecks: 11, // Only checks with Azure DevOps in repos field + expectedError: false, + }, } for _, tt := range tests { @@ -362,7 +392,7 @@ func TestGetEnabled(t *testing.T) { sp = pol } - enabledChecks, err := GetEnabled(sp, tt.argsChecks, tt.requiredRequestTypes) + enabledChecks, err := GetEnabled(sp, tt.argsChecks, tt.requiredRequestTypes, tt.repoType) if len(enabledChecks) != tt.expectedEnabledChecks { t.Errorf("Unexpected number of enabled checks: got %v, want %v", len(enabledChecks), tt.expectedEnabledChecks)