Merge branch 'main' into fix/licenses-folder

checks/binary_artifact.go

@@ -24,10 +24,11 @@ import (
 // CheckBinaryArtifacts is the exported name for Binary-Artifacts check.
 const CheckBinaryArtifacts string = "Binary-Artifacts"
 
-//nolint
+//nolint:gochecknoinits
 func init() {
 	supportedRequestTypes := []checker.RequestType{
 		checker.CommitBased,
+		checker.FileBased,
 	}
 	if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts, supportedRequestTypes); err != nil {
 		// this should never happen

checks/evaluation/permissions/permissions.go

@@ -63,6 +63,16 @@ func TokenPermissions(name string, c *checker.CheckRequest, r *checker.TokenPerm
 		"GitHub workflow tokens follow principle of least privilege")
 }
 
+// avoid memory aliasing by returning a new copy.
+func newUint(u uint) *uint {
+	return &u
+}
+
+// avoid memory aliasing by returning a new copy.
+func newStr(s string) *string {
+	return &s
+}
+
 func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckRequest) (int, error) {
 	// See list https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/.
 	// Note: there are legitimate reasons to use some of the permissions like checks, deployments, etc.
@@ -83,10 +93,10 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq
 			loc = &finding.Location{
 				Type:      r.File.Type,
 				Path:      r.File.Path,
-				LineStart: &r.File.Offset,
+				LineStart: newUint(r.File.Offset),
 			}
 			if r.File.Snippet != "" {
-				loc.Snippet = &r.File.Snippet
+				loc.Snippet = newStr(r.File.Snippet)
 			}
 		}
 

checks/raw/binary_artifact.go

@@ -15,6 +15,7 @@
 package raw
 
 import (
+	"errors"
 	"fmt"
 	"path/filepath"
 	"regexp"
@@ -201,26 +202,35 @@ func gradleWrapperValidated(c clients.RepoClient) (bool, error) {
 	if err != nil {
 		return false, fmt.Errorf("%w", err)
 	}
-	if gradleWrapperValidatingWorkflowFile != "" {
-		// If validated, check that latest commit has a relevant successful run
-		runs, err := c.ListSuccessfulWorkflowRuns(gradleWrapperValidatingWorkflowFile)
-		if err != nil {
-			return false, fmt.Errorf("failure listing workflow runs: %w", err)
-		}
-		commits, err := c.ListCommits()
-		if err != nil {
-			return false, fmt.Errorf("failure listing commits: %w", err)
-		}
-		if len(commits) < 1 || len(runs) < 1 {
+	// no matching files, validation failed
+	if gradleWrapperValidatingWorkflowFile == "" {
+		return false, nil
+	}
+
+	// If validated, check that latest commit has a relevant successful run
+	runs, err := c.ListSuccessfulWorkflowRuns(gradleWrapperValidatingWorkflowFile)
+	if err != nil {
+		// some clients, such as the local file client, don't support this feature
+		// claim unvalidated, so that other parts of the check can still be used.
+		if errors.Is(err, clients.ErrUnsupportedFeature) {
 			return false, nil
 		}
-		for _, r := range runs {
-			if *r.HeadSHA == commits[0].SHA {
-				// Commit has corresponding successful run!
-				return true, nil
-			}
+		return false, fmt.Errorf("failure listing workflow runs: %w", err)
+	}
+	commits, err := c.ListCommits()
+	if err != nil {
+		return false, fmt.Errorf("failure listing commits: %w", err)
+	}
+	if len(commits) < 1 || len(runs) < 1 {
+		return false, nil
+	}
+	for _, r := range runs {
+		if *r.HeadSHA == commits[0].SHA {
+			// Commit has corresponding successful run!
+			return true, nil
 		}
 	}
+
 	return false, nil
 }
 

checks/raw/binary_artifact_test.go

@@ -256,3 +256,35 @@ func TestBinaryArtifacts(t *testing.T) {
 		})
 	}
 }
+
+func TestBinaryArtifacts_workflow_runs_unsupported(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	mockRepoClient := mockrepo.NewMockRepoClient(ctrl)
+	const jarFile = "gradle-wrapper.jar"
+	const verifyWorkflow = ".github/workflows/verify.yaml"
+	files := []string{jarFile, verifyWorkflow}
+	mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil).AnyTimes()
+	mockRepoClient.EXPECT().GetFileContent(jarFile).DoAndReturn(func(file string) ([]byte, error) {
+		content, err := os.ReadFile("../testdata/binaryartifacts/jars/gradle-wrapper.jar")
+		if err != nil {
+			return nil, fmt.Errorf("%w", err)
+		}
+		return content, nil
+	}).AnyTimes()
+	mockRepoClient.EXPECT().GetFileContent(verifyWorkflow).DoAndReturn(func(file string) ([]byte, error) {
+		content, err := os.ReadFile("../testdata/binaryartifacts/workflows/verify.yaml")
+		if err != nil {
+			return nil, fmt.Errorf("%w", err)
+		}
+		return content, nil
+	}).AnyTimes()
+
+	mockRepoClient.EXPECT().ListSuccessfulWorkflowRuns(gomock.Any()).Return(nil, clients.ErrUnsupportedFeature).AnyTimes()
+	got, err := BinaryArtifacts(mockRepoClient)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(got.Files) != 1 {
+		t.Errorf("expected 1 file, got %d", len(got.Files))
+	}
+}

clients/git/client_test.go

@@ -77,7 +77,7 @@ func createTestRepo(t *testing.T) (path string) {
 	return dir
 }
 
-//nolint:testparallel
+//nolint:paralleltest
 func TestInitRepo(t *testing.T) {
 	tests := []struct { //nolint:govet
 		name        string
@@ -147,7 +147,7 @@ func TestListCommits(t *testing.T) {
 	}
 }
 
-//nolint:testparallel
+//nolint:paralleltest
 func TestSearch(t *testing.T) {
 	testCases := []struct {
 		name     string

clients/gitlabrepo/client.go

@@ -81,7 +81,8 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD
 
 	// Sanity check.
 	proj := fmt.Sprintf("%s/%s", glRepo.owner, glRepo.project)
-	repo, _, err := client.glClient.Projects.GetProject(proj, &gitlab.GetProjectOptions{})
+	license := true // Get project license information. Used for licenses client.
+	repo, _, err := client.glClient.Projects.GetProject(proj, &gitlab.GetProjectOptions{License: &license})
 	if err != nil {
 		return sce.WithMessage(sce.ErrRepoUnreachable, proj+"\t"+err.Error())
 	}
@@ -107,7 +108,7 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD
 	}
 
 	if repo.Owner != nil {
-		client.repourl.owner = repo.Owner.Name
+		client.repourl.owner = repo.Owner.Username
 	}
 
 	// Init contributorsHandler

clients/gitlabrepo/issues_test.go

@@ -50,10 +50,6 @@ func (s suffixStubTripper) RoundTrip(r *http.Request) (*http.Response, error) {
 	}, nil
 }
 
-func strptr(s string) *string {
-	return &s
-}
-
 func associationptr(r clients.RepoAssociation) *clients.RepoAssociation {
 	return &r
 }

clients/gitlabrepo/licenses.go

@@ -44,18 +44,11 @@ var errLicenseURLParse = errors.New("couldn't parse gitlab repo license url")
 
 func (handler *licensesHandler) setup() error {
 	handler.once.Do(func() {
-		licenseMap := []clients.License{}
-		if len(licenseMap) == 0 {
-			// TODO: handler.errSetup = fmt.Errorf("request for repo licenses failed with %w", err)
-			handler.errSetup = fmt.Errorf("%w: ListLicenses not yet supported for gitlab", clients.ErrUnsupportedFeature)
-			return
-		}
-
 		l := handler.glProject.License
 
-		ptn, err := regexp.Compile(fmt.Sprintf("%s/~/blob/master/(.*)", handler.repourl.URI()))
+		ptn, err := regexp.Compile(fmt.Sprintf("%s/-/blob/(?:\\w+)/(.*)", handler.repourl.URI()))
 		if err != nil {
-			handler.errSetup = fmt.Errorf("couldn't parse License URL: %w", err)
+			handler.errSetup = fmt.Errorf("couldn't parse license url: %w", err)
 			return
 		}
 
@@ -68,9 +61,10 @@ func (handler *licensesHandler) setup() error {
 
 		handler.licenses = append(handler.licenses,
 			clients.License{
-				Key:  l.Key,
-				Name: l.Name,
-				Path: path,
+				Key:    l.Key,
+				Name:   l.Name,
+				Path:   path,
+				SPDXId: l.Key,
 			},
 		)
 

clients/gitlabrepo/workflows.go

@@ -44,14 +44,19 @@ func (handler *workflowsHandler) listSuccessfulWorkflowRuns(filename string) ([]
 	return workflowsRunsFrom(jobs, filename), nil
 }
 
+// avoid memory aliasing by returning a new copy.
+func strptr(s string) *string {
+	return &s
+}
+
 func workflowsRunsFrom(data []*gitlab.Job, filename string) []clients.WorkflowRun {
 	var workflowRuns []clients.WorkflowRun
 	for _, job := range data {
 		// Find a better way to do this.
 		for _, artifact := range job.Artifacts {
 			if strings.EqualFold(artifact.Filename, filename) {
 				workflowRuns = append(workflowRuns, clients.WorkflowRun{
-					HeadSHA: &job.Pipeline.Sha,
+					HeadSHA: strptr(job.Pipeline.Sha),
 					URL:     job.WebURL,
 				})
 				continue

e2e/license_test.go

@@ -123,7 +123,33 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
 
 			dl := scut.TestDetailLogger{}
-			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-license-e2e")
+			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-license-e2e")
+			Expect(err).Should(BeNil())
+			repoClient, err := gitlabrepo.CreateGitlabClient(context.Background(), repo.Host())
+			Expect(err).Should(BeNil())
+			err = repoClient.InitRepo(repo, clients.HeadSHA, 0)
+			Expect(err).Should(BeNil())
+			req := checker.CheckRequest{
+				Ctx:        context.Background(),
+				RepoClient: repoClient,
+				Repo:       repo,
+				Dlogger:    &dl,
+			}
+			expected := scut.TestReturn{
+				Error:        nil,
+				Score:        10,
+				NumberOfInfo: 2,
+			}
+			result := checks.License(&req)
+
+			Expect(scut.ValidateTestReturn(nil, "license found", &expected, &result,
+				&dl)).Should(BeTrue())
+		})
+		It("Should return license check works for unrecognized license type - GitLab", func() {
+			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
+
+			dl := scut.TestDetailLogger{}
+			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-license-e2e-unrecognized-license-type")
 			Expect(err).Should(BeNil())
 			repoClient, err := gitlabrepo.CreateGitlabClient(context.Background(), repo.Host())
 			Expect(err).Should(BeNil())
@@ -151,7 +177,7 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 			skipIfTokenIsNot(gitlabPATTokenType, "GitLab only")
 
 			dl := scut.TestDetailLogger{}
-			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/scorecard-check-license-e2e")
+			repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/ossf-test/scorecard-check-license-e2e")
 			Expect(err).Should(BeNil())
 			repoClient, err := gitlabrepo.CreateGitlabClient(context.Background(), repo.Host())
 			Expect(err).Should(BeNil())
@@ -164,11 +190,9 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
 				Dlogger:    &dl,
 			}
 			expected := scut.TestReturn{
-				Error:         nil,
-				Score:         9,
-				NumberOfWarn:  1,
-				NumberOfInfo:  1,
-				NumberOfDebug: 0,
+				Error:        nil,
+				Score:        10,
+				NumberOfInfo: 2,
 			}
 			result := checks.License(&req)
 

pkg/json_raw_results.go

@@ -331,7 +331,7 @@ func (r *jsonScorecardRawResult) addTokenPermissionsRawResults(tp *checker.Token
 				Offset: t.File.Offset,
 			}
 			if t.File.Snippet != "" {
-				p.File.Snippet = &t.File.Snippet
+				p.File.Snippet = asPointer(t.File.Snippet)
 			}
 		}
 
@@ -361,7 +361,7 @@ func (r *jsonScorecardRawResult) addPackagingRawResults(pk *checker.PackagingDat
 		}
 
 		if p.File.Snippet != "" {
-			jpk.File.Snippet = &p.File.Snippet
+			jpk.File.Snippet = asPointer(p.File.Snippet)
 		}
 
 		for _, run := range p.Runs {
@@ -419,7 +419,7 @@ func (r *jsonScorecardRawResult) addDangerousWorkflowRawResults(df *checker.Dang
 			Type: string(e.Type),
 		}
 		if e.File.Snippet != "" {
-			v.File.Snippet = &e.File.Snippet
+			v.File.Snippet = asPointer(e.File.Snippet)
 		}
 		if e.Job != nil {
 			v.Job = &jsonWorkflowJob{