Skip to content
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

⚠️ Add support for commit-based Scorecard #1613

Merged
merged 1 commit into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions checker/check_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type RequestType int
const (
// FileBased request types require checks to run solely on file-content.
FileBased RequestType = iota
// CommitBased request types require checks to run on non-HEAD commit content.
CommitBased
)

// ListUnsupported returns []RequestType not in `supported` and are `required`.
Expand Down
1 change: 1 addition & 0 deletions checks/binary_artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const CheckBinaryArtifacts string = "Binary-Artifacts"
func init() {
var supportedRequestTypes = []checker.RequestType{
checker.FileBased,
checker.CommitBased,
}
if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts, supportedRequestTypes); err != nil {
// this should never happen
Expand Down
2 changes: 1 addition & 1 deletion checks/binary_artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestBinaryArtifacts(t *testing.T) {
ctx := context.Background()

client := localdir.CreateLocalDirClient(ctx, logger)
if err := client.InitRepo(repo); err != nil {
if err := client.InitRepo(repo, "HEAD"); err != nil {
t.Errorf("InitRepo: %v", err)
}

Expand Down
5 changes: 4 additions & 1 deletion checks/ci_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ const (

//nolint:gochecknoinits
func init() {
if err := registerCheck(CheckCITests, CITests, nil); err != nil {
supportedRequestTypes := []checker.RequestType{
checker.CommitBased,
}
if err := registerCheck(CheckCITests, CITests, supportedRequestTypes); err != nil {
// this should never happen
panic(err)
}
Expand Down
5 changes: 4 additions & 1 deletion checks/code_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ const CheckCodeReview = "Code-Review"

//nolint:gochecknoinits
func init() {
if err := registerCheck(CheckCodeReview, CodeReview, nil); err != nil {
supportedRequestTypes := []checker.RequestType{
checker.CommitBased,
}
if err := registerCheck(CheckCodeReview, CodeReview, supportedRequestTypes); err != nil {
// this should never happen
panic(err)
}
Expand Down
1 change: 1 addition & 0 deletions checks/dangerous_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func containsUntrustedContextPattern(variable string) bool {
func init() {
supportedRequestTypes := []checker.RequestType{
checker.FileBased,
checker.CommitBased,
}
if err := registerCheck(CheckDangerousWorkflow, DangerousWorkflow, supportedRequestTypes); err != nil {
// this should never happen
Expand Down
2 changes: 1 addition & 1 deletion checks/license_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestLicenseFileSubdirectory(t *testing.T) {
ctx := context.Background()

client := localdir.CreateLocalDirClient(ctx, logger)
if err := client.InitRepo(repo); err != nil {
if err := client.InitRepo(repo, "HEAD"); err != nil {
t.Errorf("InitRepo: %v", err)
}

Expand Down
1 change: 1 addition & 0 deletions checks/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var permissionsOfInterest = []permission{
func init() {
supportedRequestTypes := []checker.RequestType{
checker.FileBased,
checker.CommitBased,
}
if err := registerCheck(CheckTokenPermissions, TokenPermissions, supportedRequestTypes); err != nil {
// This should never happen.
Expand Down
1 change: 1 addition & 0 deletions checks/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type worklowPinningResult struct {
func init() {
supportedRequestTypes := []checker.RequestType{
checker.FileBased,
checker.CommitBased,
}
if err := registerCheck(CheckPinnedDependencies, PinnedDependencies, supportedRequestTypes); err != nil {
// This should never happen.
Expand Down
2 changes: 1 addition & 1 deletion checks/raw/security_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
Repo: c.Repo.Org(),
}

err = dotGitHub.RepoClient.InitRepo(dotGitHub.Repo)
err = dotGitHub.RepoClient.InitRepo(dotGitHub.Repo, "HEAD")
switch {
case err == nil:
defer dotGitHub.RepoClient.Close()
Expand Down
1 change: 1 addition & 0 deletions checks/sast.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var allowedConclusions = map[string]bool{"success": true, "neutral": true}

//nolint:gochecknoinits
func init() {
// TODO(#575): Check if we can support commit-based requests here.
if err := registerCheck(CheckSAST, SAST, nil); err != nil {
// This should never happen.
panic(err)
Expand Down
5 changes: 4 additions & 1 deletion checks/vulnerabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ const CheckVulnerabilities = "Vulnerabilities"

//nolint:gochecknoinits
func init() {
if err := registerCheck(CheckVulnerabilities, Vulnerabilities, nil); err != nil {
supportedRequestTypes := []checker.RequestType{
checker.CommitBased,
}
if err := registerCheck(CheckVulnerabilities, Vulnerabilities, supportedRequestTypes); err != nil {
// this should never happen
panic(err)
}
Expand Down
5 changes: 2 additions & 3 deletions clients/githubrepo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ type Client struct {
}

// InitRepo sets up the GitHub repo in local storage for improving performance and GitHub token usage efficiency.
func (client *Client) InitRepo(inputRepo clients.Repo) error {
commitSHA := "HEAD"
func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string) error {
ghRepo, ok := inputRepo.(*repoURL)
if !ok {
return fmt.Errorf("%w: %v", errInputRepoType, inputRepo)
Expand Down Expand Up @@ -225,7 +224,7 @@ func CreateOssFuzzRepoClient(ctx context.Context, logger *log.Logger) (clients.R
}

ossFuzzRepoClient := CreateGithubRepoClient(ctx, logger)
if err := ossFuzzRepoClient.InitRepo(ossFuzzRepo); err != nil {
if err := ossFuzzRepoClient.InitRepo(ossFuzzRepo, "HEAD"); err != nil {
return nil, fmt.Errorf("error during InitRepo: %w", err)
}
return ossFuzzRepoClient, nil
Expand Down
2 changes: 1 addition & 1 deletion clients/localdir/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type localDirClient struct {
}

// InitRepo sets up the local repo.
func (client *localDirClient) InitRepo(inputRepo clients.Repo) error {
func (client *localDirClient) InitRepo(inputRepo clients.Repo, commitSHA string) error {
localRepo, ok := inputRepo.(*repoLocal)
if !ok {
return fmt.Errorf("%w: %v", errInputRepoType, inputRepo)
Expand Down
2 changes: 1 addition & 1 deletion clients/localdir/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestClient_CreationAndCaching(t *testing.T) {
}

client := CreateLocalDirClient(ctx, logger)
if err := client.InitRepo(repo); err != nil {
if err := client.InitRepo(repo, "HEAD"); err != nil {
t.Errorf("InitRepo: %v", err)
}

Expand Down
8 changes: 4 additions & 4 deletions clients/mockclients/repo_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion clients/repo_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var ErrUnsupportedFeature = errors.New("unsupported feature")

// RepoClient interface is used by Scorecard checks to access a repo.
type RepoClient interface {
InitRepo(repo Repo) error
InitRepo(repo Repo, commitSHA string) error
URI() string
IsArchived() (bool, error)
ListFiles(predicate func(string) (bool, error)) ([]string, error)
Expand Down
1 change: 1 addition & 0 deletions cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd
var (
flagRepo string
flagLocal string
flagCommit string
flagChecksToRun []string
flagMetadata []string
flagLogLevel string
Expand Down
14 changes: 13 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ var rootCmd = &cobra.Command{
func init() {
rootCmd.Flags().StringVar(&flagRepo, "repo", "", "repository to check")
rootCmd.Flags().StringVar(&flagLocal, "local", "", "local folder to check")
rootCmd.Flags().StringVar(&flagCommit, "commit", "HEAD", "commit to analyze")
rootCmd.Flags().StringVar(
&flagLogLevel,
"verbosity",
Expand Down Expand Up @@ -147,6 +148,9 @@ func scorecardCmd(cmd *cobra.Command, args []string) {
if flagLocal != "" {
requiredRequestTypes = append(requiredRequestTypes, checker.FileBased)
}
if !strings.EqualFold(flagCommit, "HEAD") {
requiredRequestTypes = append(requiredRequestTypes, checker.CommitBased)
}
enabledChecks, err := getEnabledChecks(policy, flagChecksToRun, requiredRequestTypes)
if err != nil {
log.Panic(err)
Expand All @@ -158,7 +162,7 @@ func scorecardCmd(cmd *cobra.Command, args []string) {
}
}

repoResult, err := pkg.RunScorecards(ctx, repoURI, flagFormat == formatRaw, enabledChecks, repoClient,
repoResult, err := pkg.RunScorecards(ctx, repoURI, flagCommit, flagFormat == formatRaw, enabledChecks, repoClient,
ossFuzzRepoClient, ciiClient, vulnsClient)
if err != nil {
log.Panic(err)
Expand Down Expand Up @@ -221,12 +225,20 @@ func validateCmdFlags() {
if flagFormat == formatRaw {
log.Panic("raw option not supported yet")
}
if flagCommit != "HEAD" {
log.Panic("--commit option not supported yet")
}
}

// Validate format.
if !validateFormat(flagFormat) {
log.Panicf("unsupported format '%s'", flagFormat)
}

// Validate `commit` is non-empty.
if flagCommit == "" {
log.Panic("commit should be non-empty")
}
}

func boolSum(bools ...bool) int {
Expand Down
2 changes: 1 addition & 1 deletion cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ var serveCmd = &cobra.Command{
}
defer ossFuzzRepoClient.Close()
ciiClient := clients.DefaultCIIBestPracticesClient()
repoResult, err := pkg.RunScorecards(ctx, repo, false, checks.AllChecks, repoClient,
repoResult, err := pkg.RunScorecards(ctx, repo, "HEAD" /*commitSHA*/, false /*raw*/, checks.AllChecks, repoClient,
ossFuzzRepoClient, ciiClient, vulnsClient)
if err != nil {
logger.Error(err, "running enabled scorecard checks on repo")
Expand Down
2 changes: 1 addition & 1 deletion cron/worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func processRequest(ctx context.Context,
continue
}
repo.AppendMetadata(repo.Metadata()...)
result, err := pkg.RunScorecards(ctx, repo, false, checksToRun,
result, err := pkg.RunScorecards(ctx, repo, "HEAD" /*commitSHA*/, false /*raw*/, checksToRun,
repoClient, ossFuzzRepoClient, ciiClient, vulnsClient)
if errors.Is(err, sce.ErrRepoUnreachable) {
// Not accessible repo - continue.
Expand Down
64 changes: 61 additions & 3 deletions e2e/binary_artifacts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
repo, err := githubrepo.MakeGithubRepo("ossf/scorecard")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())

req := checker.CheckRequest{
Expand All @@ -61,7 +61,38 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-binary-artifacts-e2e")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())

req := checker.CheckRequest{
Ctx: context.Background(),
RepoClient: repoClient,
Repo: repo,
Dlogger: &dl,
}
// TODO: upload real binaries to the repo as well.
expected := scut.TestReturn{
Error: nil,
Score: checker.MinResultScore,
NumberOfWarn: 24,
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.BinaryArtifacts(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
Expect(result.Pass).Should(BeFalse())
// New version.
Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
Expect(repoClient.Close()).Should(BeNil())
})
It("Should return binary artifacts present at commit in source code", func() {
dl := scut.TestDetailLogger{}
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-binary-artifacts-e2e")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo, "0c6e8991781bba24bfaaa0525f69329f672ef7b5")
Expect(err).Should(BeNil())

req := checker.CheckRequest{
Expand Down Expand Up @@ -92,7 +123,34 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-binary-artifacts-e2e-4-binaries")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())

req := checker.CheckRequest{
Ctx: context.Background(),
RepoClient: repoClient,
Repo: repo,
Dlogger: &dl,
}
// TODO: upload real binaries to the repo as well.
expected := scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore - 4,
NumberOfWarn: 4,
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.BinaryArtifacts(&req)
// New version.
Expect(scut.ValidateTestReturn(nil, "binary artifacts", &expected, &result, &dl)).Should(BeTrue())
Expect(repoClient.Close()).Should(BeNil())
})
It("Should return binary artifacts present at commit in source code", func() {
dl := scut.TestDetailLogger{}
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-binary-artifacts-e2e-4-binaries")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo, "d994b3e1a8912283f9958a7c1e0aa480ca24a7ce")
Expect(err).Should(BeNil())

req := checker.CheckRequest{
Expand Down
6 changes: 3 additions & 3 deletions e2e/branch_protection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBranchProtection, func() {
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())
req := checker.CheckRequest{
Ctx: context.Background(),
Expand Down Expand Up @@ -63,7 +63,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBranchProtection, func() {
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e-none")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())
req := checker.CheckRequest{
Ctx: context.Background(),
Expand Down Expand Up @@ -93,7 +93,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBranchProtection, func() {
repo, err := githubrepo.MakeGithubRepo("ossf-tests/scorecard-check-branch-protection-e2e-patch-1")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())
req := checker.CheckRequest{
Ctx: context.Background(),
Expand Down
26 changes: 25 additions & 1 deletion e2e/ci_tests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,31 @@ var _ = Describe("E2E TEST:"+checks.CheckCITests, func() {
repo, err := githubrepo.MakeGithubRepo("ossf-tests/airflow")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo)
err = repoClient.InitRepo(repo, "HEAD")
Expect(err).Should(BeNil())
req := checker.CheckRequest{
Ctx: context.Background(),
RepoClient: repoClient,
Repo: repo,
Dlogger: &dl,
}
expected := scut.TestReturn{
Error: nil,
Score: checker.InconclusiveResultScore,
NumberOfWarn: 0,
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.CITests(&req)
Expect(scut.ValidateTestReturn(nil, "CI tests run", &expected, &result, &dl)).Should(BeTrue())
Expect(repoClient.Close()).Should(BeNil())
})
It("Should return use of CI tests at commit", func() {
dl := scut.TestDetailLogger{}
repo, err := githubrepo.MakeGithubRepo("ossf-tests/airflow")
Expect(err).Should(BeNil())
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
err = repoClient.InitRepo(repo, "0a6850647e531b08f68118ff8ca20577a5b4062c")
Expect(err).Should(BeNil())
req := checker.CheckRequest{
Ctx: context.Background(),
Expand Down
Loading