Skip to content

Commit

Permalink
Add support for commit-based Scorecard
Browse files Browse the repository at this point in the history
  • Loading branch information
azeemsgoogle committed Feb 8, 2022
1 parent 1c95237 commit 1f12364
Show file tree
Hide file tree
Showing 39 changed files with 297 additions and 46 deletions.
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

0 comments on commit 1f12364

Please sign in to comment.