From 93875dd11c90171f5a01a959e96a1ab6496959c8 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Tue, 22 Aug 2023 17:48:21 -0700 Subject: [PATCH 1/2] :bug: Fix loop aliasing errors causing linter to fail. (#3414) Signed-off-by: Spencer Schrock --- checks/evaluation/permissions/permissions.go | 14 ++++++++++++-- clients/git/client_test.go | 4 ++-- clients/gitlabrepo/issues_test.go | 4 ---- clients/gitlabrepo/workflows.go | 7 ++++++- pkg/json_raw_results.go | 6 +++--- 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/checks/evaluation/permissions/permissions.go b/checks/evaluation/permissions/permissions.go index 8f0c1be5d2b..b8ca06796aa 100644 --- a/checks/evaluation/permissions/permissions.go +++ b/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) } } diff --git a/clients/git/client_test.go b/clients/git/client_test.go index a51c5445120..98e132b9c9f 100644 --- a/clients/git/client_test.go +++ b/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 diff --git a/clients/gitlabrepo/issues_test.go b/clients/gitlabrepo/issues_test.go index 09008842827..f61ee63fb0d 100644 --- a/clients/gitlabrepo/issues_test.go +++ b/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 } diff --git a/clients/gitlabrepo/workflows.go b/clients/gitlabrepo/workflows.go index e90ef5e86d0..ff558b56d36 100644 --- a/clients/gitlabrepo/workflows.go +++ b/clients/gitlabrepo/workflows.go @@ -44,6 +44,11 @@ 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 { @@ -51,7 +56,7 @@ func workflowsRunsFrom(data []*gitlab.Job, filename string) []clients.WorkflowRu 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 diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 4be011aabd7..629bee123b9 100644 --- a/pkg/json_raw_results.go +++ b/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{ From 475d975cc25b228781efe21d2ef57b2f5aaec3c0 Mon Sep 17 00:00:00 2001 From: Raghav Kaul <8695110+raghavkaul@users.noreply.github.com> Date: Wed, 23 Aug 2023 13:03:20 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20Licenses:=20Get=20License=20?= =?UTF-8?q?SPDXId=20from=20GitLab=20API=20(#3413)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix licenses check * Update repoclient * Get SPDXId from `key` field in GitLab Projects API * Update e2etest repos Signed-off-by: Raghav Kaul * add test Signed-off-by: Raghav Kaul * stricter regex Signed-off-by: Raghav Kaul --------- Signed-off-by: Raghav Kaul --- clients/gitlabrepo/client.go | 5 +++-- clients/gitlabrepo/licenses.go | 18 ++++++---------- e2e/license_test.go | 38 +++++++++++++++++++++++++++------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go index 2e0e054eaec..f79e502aa56 100644 --- a/clients/gitlabrepo/client.go +++ b/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 diff --git a/clients/gitlabrepo/licenses.go b/clients/gitlabrepo/licenses.go index 6ef7d827ecd..7b76963dde6 100644 --- a/clients/gitlabrepo/licenses.go +++ b/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, }, ) diff --git a/e2e/license_test.go b/e2e/license_test.go index e437496845a..61cd12ce910 100644 --- a/e2e/license_test.go +++ b/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)