From 110e352273a18837c65ed01aa7b26da1de802051 Mon Sep 17 00:00:00 2001 From: raghavkaul <8695110+raghavkaul@users.noreply.github.com> Date: Mon, 13 Mar 2023 11:13:50 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Gitlab=20support:=20RepoClient=20(#?= =?UTF-8?q?2655)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add make targets and E2E test target for GitLab only Signed-off-by: Raghav Kaul * Add GitLab support to RepoClient Signed-off-by: Raghav Kaul * Build * Make target for e2e-gitlab-token * Only run Gitlab tests in CI that don't require a token Signed-off-by: Raghav Kaul * Add tests Signed-off-by: Raghav Kaul * Remove spurious printf Signed-off-by: Raghav Kaul * 🐛 Check OSS Fuzz build file for Fuzzing check (#2719) * Check OSS-Fuzz using project list Signed-off-by: Spencer Schrock * Use clients.RepoClient interface to perform the new OSS Fuzz check Signed-off-by: Spencer Schrock * wip: add eager client for better repeated lookup of projects Signed-off-by: Spencer Schrock * Split lazy and eager behavior into different implementations. Signed-off-by: Spencer Schrock * Add tests and benchmarks Signed-off-by: Spencer Schrock * Switch to always parsing JSON to determine if a project is present. The other approach of looking for a substring match would lead to false positives. Signed-off-by: Spencer Schrock * Add eager constructor to surface status file errors sooner. Signed-off-by: Spencer Schrock * Switch existing users to new OSS Fuzz client Signed-off-by: Spencer Schrock * Mark old method as deprecated in the godoc Signed-off-by: Spencer Schrock * remove unused comment. Signed-off-by: Spencer Schrock * Use new OSS Fuzz client in e2e test. Signed-off-by: Spencer Schrock * fix typo. Signed-off-by: Spencer Schrock * Fix potential path bug with test server. Signed-off-by: Spencer Schrock * Force include the two JSON files which were being ignored by .gitignore Signed-off-by: Spencer Schrock * trim the status json file Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --------- Signed-off-by: Raghav Kaul Signed-off-by: Spencer Schrock Co-authored-by: Spencer Schrock --- .github/workflows/integration.yml | 14 +++ Makefile | 6 + checker/client.go | 54 +++++++-- clients/githubrepo/repo.go | 4 + clients/githubrepo/repo_test.go | 4 + clients/gitlabrepo/branches.go | 18 +-- clients/gitlabrepo/checkruns.go | 2 +- clients/gitlabrepo/client.go | 31 +++-- clients/gitlabrepo/commits.go | 4 +- clients/gitlabrepo/contributors.go | 2 +- clients/gitlabrepo/issues.go | 4 +- clients/gitlabrepo/languages.go | 2 +- clients/gitlabrepo/project.go | 2 +- clients/gitlabrepo/releases.go | 2 +- clients/gitlabrepo/repo.go | 94 ++++++++++----- clients/gitlabrepo/repo_test.go | 143 +++++++++++++---------- clients/gitlabrepo/search.go | 4 +- clients/gitlabrepo/searchCommits.go | 4 +- clients/gitlabrepo/searchCommits_test.go | 8 +- clients/gitlabrepo/search_test.go | 24 ++-- clients/gitlabrepo/statuses.go | 2 +- clients/gitlabrepo/webhook.go | 2 +- clients/gitlabrepo/workflows.go | 2 +- clients/localdir/repo.go | 4 + clients/mockclients/repo.go | 8 ++ clients/repo.go | 1 + e2e/attestor_policy_test.go | 2 - 27 files changed, 289 insertions(+), 158 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index b3583631247..1eeca00f1cb 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -71,3 +71,17 @@ jobs: with: files: ./e2e-coverage.out verbose: true + + - name: Run GitLab E2E #using retry because the GitHub token is being throttled. + uses: nick-invision/retry@943e742917ac94714d2f408a0e8320f2d1fcafcd + with: + max_attempts: 3 + retry_on: error + timeout_minutes: 30 + command: make e2e-gitlab + + - name: codecov + uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # 2.1.0 + with: + files: ./e2e-coverage.out + verbose: true diff --git a/Makefile b/Makefile index 25d5e2370e2..b9d5e4c8509 100644 --- a/Makefile +++ b/Makefile @@ -334,6 +334,12 @@ e2e-gh-token: build-scorecard check-env | $(GINKGO) # Run e2e tests. GITHUB_AUTH_TOKEN set to secrets.GITHUB_TOKEN must be used to run this. TOKEN_TYPE="GITHUB_TOKEN" $(GINKGO) --race -p -v -cover -coverprofile=e2e-coverage.out --keep-separate-coverprofiles ./... +e2e-gitlab-token: ## Runs e2e tests that require a GITLAB_TOKEN + TOKEN_TYPE="GITLAB_PAT" $(GINKGO) --race -p -vv --focus '.*GitLab Token' ./... + +e2e-gitlab: ## Runs e2e tests for GitLab only. TOKEN_TYPE is not used (since these are public APIs), but must be set to something + TOKEN_TYPE="GITLAB_PAT" $(GINKGO) --race -p -vv --focus '.*GitLab' ./... + e2e-attestor: ## Runs e2e tests for scorecard-attestor cd attestor/e2e; go test -covermode=atomic -coverprofile=e2e-coverage.out; cd ../.. diff --git a/checker/client.go b/checker/client.go index 1fd646fec28..b6931a7dec1 100644 --- a/checker/client.go +++ b/checker/client.go @@ -17,9 +17,11 @@ package checker import ( "context" "fmt" + "os" "github.com/ossf/scorecard/v4/clients" ghrepo "github.com/ossf/scorecard/v4/clients/githubrepo" + glrepo "github.com/ossf/scorecard/v4/clients/gitlabrepo" "github.com/ossf/scorecard/v4/clients/localdir" "github.com/ossf/scorecard/v4/clients/ossfuzz" "github.com/ossf/scorecard/v4/log" @@ -35,7 +37,9 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge clients.VulnerabilitiesClient, // vulnClient error, ) { - var githubRepo clients.Repo + var repo clients.Repo + var makeRepoError error + if localURI != "" { localRepo, errLocal := localdir.MakeLocalDirRepo(localURI) var retErr error @@ -50,18 +54,46 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge retErr } - githubRepo, errGitHub := ghrepo.MakeGithubRepo(repoURI) - if errGitHub != nil { - return githubRepo, - nil, - nil, - nil, - nil, - fmt.Errorf("getting local directory client: %w", errGitHub) + _, experimental := os.LookupEnv("SCORECARD_EXPERIMENTAL") + var repoClient clients.RepoClient + + //nolint:nestif + if experimental && glrepo.DetectGitLab(repoURI) { + repo, makeRepoError = glrepo.MakeGitlabRepo(repoURI) + if makeRepoError != nil { + return repo, + nil, + nil, + nil, + nil, + fmt.Errorf("getting local directory client: %w", makeRepoError) + } + + var err error + repoClient, err = glrepo.CreateGitlabClientWithToken(ctx, os.Getenv("GITLAB_AUTH_TOKEN"), repo) + if err != nil { + return repo, + nil, + nil, + nil, + nil, + fmt.Errorf("error creating gitlab client: %w", err) + } + } else { + repo, makeRepoError = ghrepo.MakeGithubRepo(repoURI) + if makeRepoError != nil { + return repo, + nil, + nil, + nil, + nil, + fmt.Errorf("getting local directory client: %w", makeRepoError) + } + repoClient = ghrepo.CreateGithubRepoClient(ctx, logger) } - return githubRepo, /*repo*/ - ghrepo.CreateGithubRepoClient(ctx, logger), /*repoClient*/ + return repo, /*repo*/ + repoClient, /*repoClient*/ ossfuzz.CreateOSSFuzzClient(ossfuzz.StatusURL), /*ossFuzzClient*/ clients.DefaultCIIBestPracticesClient(), /*ciiClient*/ clients.DefaultVulnerabilitiesClient(), /*vulnClient*/ diff --git a/clients/githubrepo/repo.go b/clients/githubrepo/repo.go index 0f288cf7205..295ee21bfa1 100644 --- a/clients/githubrepo/repo.go +++ b/clients/githubrepo/repo.go @@ -76,6 +76,10 @@ func (r *repoURL) URI() string { return fmt.Sprintf("%s/%s/%s", r.host, r.owner, r.repo) } +func (r *repoURL) Host() string { + return r.host +} + // String implements Repo.String. func (r *repoURL) String() string { return fmt.Sprintf("%s-%s-%s", r.host, r.owner, r.repo) diff --git a/clients/githubrepo/repo_test.go b/clients/githubrepo/repo_test.go index 85a507d2c7c..10b2c7a5e5a 100644 --- a/clients/githubrepo/repo_test.go +++ b/clients/githubrepo/repo_test.go @@ -97,6 +97,10 @@ func TestRepoURL_IsValid(t *testing.T) { if !tt.wantErr && !cmp.Equal(tt.expected, r, cmp.AllowUnexported(repoURL{})) { t.Errorf("Got diff: %s", cmp.Diff(tt.expected, r)) } + + if !cmp.Equal(r.Host(), tt.expected.host) { + t.Errorf("%s expected host: %s got host %s", tt.inputURL, tt.expected.host, r.Host()) + } }) } } diff --git a/clients/gitlabrepo/branches.go b/clients/gitlabrepo/branches.go index 7ca6189cc98..79aeaafbb2a 100644 --- a/clients/gitlabrepo/branches.go +++ b/clients/gitlabrepo/branches.go @@ -46,13 +46,13 @@ func (handler *branchesHandler) setup() error { return } - proj, _, err := handler.glClient.Projects.GetProject(handler.repourl.projectID, &gitlab.GetProjectOptions{}) + proj, _, err := handler.glClient.Projects.GetProject(handler.repourl.project, &gitlab.GetProjectOptions{}) if err != nil { handler.errSetup = fmt.Errorf("requirest for project failed with error %w", err) return } - branch, _, err := handler.glClient.Branches.GetBranch(handler.repourl.projectID, proj.DefaultBranch) + branch, _, err := handler.glClient.Branches.GetBranch(handler.repourl.project, proj.DefaultBranch) if err != nil { handler.errSetup = fmt.Errorf("request for default branch failed with error %w", err) return @@ -60,7 +60,7 @@ func (handler *branchesHandler) setup() error { if branch.Protected { protectedBranch, resp, err := handler.glClient.ProtectedBranches.GetProtectedBranch( - handler.repourl.projectID, branch.Name) + handler.repourl.project, branch.Name) if err != nil && resp.StatusCode != 403 { handler.errSetup = fmt.Errorf("request for protected branch failed with error %w", err) return @@ -70,13 +70,13 @@ func (handler *branchesHandler) setup() error { } projectStatusChecks, resp, err := handler.glClient.ExternalStatusChecks.ListProjectStatusChecks( - handler.repourl.projectID, &gitlab.ListOptions{}) + handler.repourl.project, &gitlab.ListOptions{}) if err != nil && resp.StatusCode != 404 { handler.errSetup = fmt.Errorf("request for external status checks failed with error %w", err) return } - projectApprovalRule, resp, err := handler.glClient.Projects.GetApprovalConfiguration(handler.repourl.projectID) + projectApprovalRule, resp, err := handler.glClient.Projects.GetApprovalConfiguration(handler.repourl.project) if err != nil && resp.StatusCode != 404 { handler.errSetup = fmt.Errorf("request for project approval rule failed with %w", err) return @@ -105,24 +105,24 @@ func (handler *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { } func (handler *branchesHandler) getBranch(branch string) (*clients.BranchRef, error) { - bran, _, err := handler.glClient.Branches.GetBranch(handler.repourl.projectID, branch) + bran, _, err := handler.glClient.Branches.GetBranch(handler.repourl.project, branch) if err != nil { return nil, fmt.Errorf("error getting branch in branchsHandler.getBranch: %w", err) } if bran.Protected { - protectedBranch, _, err := handler.glClient.ProtectedBranches.GetProtectedBranch(handler.repourl.projectID, bran.Name) + protectedBranch, _, err := handler.glClient.ProtectedBranches.GetProtectedBranch(handler.repourl.project, bran.Name) if err != nil { return nil, fmt.Errorf("request for protected branch failed with error %w", err) } projectStatusChecks, resp, err := handler.glClient.ExternalStatusChecks.ListProjectStatusChecks( - handler.repourl.projectID, &gitlab.ListOptions{}) + handler.repourl.project, &gitlab.ListOptions{}) if err != nil && resp.StatusCode != 404 { return nil, fmt.Errorf("request for external status checks failed with error %w", err) } - projectApprovalRule, resp, err := handler.glClient.Projects.GetApprovalConfiguration(handler.repourl.projectID) + projectApprovalRule, resp, err := handler.glClient.Projects.GetApprovalConfiguration(handler.repourl.project) if err != nil && resp.StatusCode != 404 { return nil, fmt.Errorf("request for project approval rule failed with %w", err) } diff --git a/clients/gitlabrepo/checkruns.go b/clients/gitlabrepo/checkruns.go index a666dd03a26..3508f4d3247 100644 --- a/clients/gitlabrepo/checkruns.go +++ b/clients/gitlabrepo/checkruns.go @@ -34,7 +34,7 @@ func (handler *checkrunsHandler) init(repourl *repoURL) { func (handler *checkrunsHandler) listCheckRunsForRef(ref string) ([]clients.CheckRun, error) { pipelines, _, err := handler.glClient.Pipelines.ListProjectPipelines( - handler.repourl.projectID, &gitlab.ListProjectPipelinesOptions{}) + handler.repourl.project, &gitlab.ListProjectPipelinesOptions{}) if err != nil { return nil, fmt.Errorf("request for pipelines returned error: %w", err) } diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go index a64164c63c5..e58aca0f7c7 100644 --- a/clients/gitlabrepo/client.go +++ b/clients/gitlabrepo/client.go @@ -52,8 +52,7 @@ type Client struct { languages *languagesHandler licenses *licensesHandler ctx context.Context - // tarball tarballHandler - commitDepth int + commitDepth int } // InitRepo sets up the GitLab project in local storage for improving performance and GitLab token usage efficiency. @@ -64,9 +63,10 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD } // Sanity check. - repo, _, err := client.glClient.Projects.GetProject(glRepo.projectID, &gitlab.GetProjectOptions{}) + proj := fmt.Sprintf("%s/%s", glRepo.owner, glRepo.project) + repo, _, err := client.glClient.Projects.GetProject(proj, &gitlab.GetProjectOptions{}) if err != nil { - return sce.WithMessage(sce.ErrRepoUnreachable, err.Error()) + return sce.WithMessage(sce.ErrRepoUnreachable, proj+"\t"+err.Error()) } if commitDepth <= 0 { client.commitDepth = 30 // default @@ -75,8 +75,9 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD } client.repo = repo client.repourl = &repoURL{ - hostname: inputRepo.URI(), - projectID: fmt.Sprint(repo.ID), + scheme: glRepo.scheme, + host: glRepo.host, + project: fmt.Sprint(repo.ID), defaultBranch: repo.DefaultBranch, commitSHA: commitSHA, } @@ -127,13 +128,11 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitD // Init languagesHandler client.licenses.init(client.repourl) - // Init tarballHandler. - // client.tarball.init(client.ctx, client.repourl, client.repo, commitSHA) return nil } func (client *Client) URI() string { - return fmt.Sprintf("%s/%s/%s", client.repourl.hostname, client.repourl.owner, client.repourl.projectID) + return fmt.Sprintf("%s/%s/%s", client.repourl.host, client.repourl.owner, client.repourl.project) } func (client *Client) LocalPath() (string, error) { @@ -222,7 +221,7 @@ func (client *Client) Close() error { } func CreateGitlabClientWithToken(ctx context.Context, token string, repo clients.Repo) (clients.RepoClient, error) { - client, err := gitlab.NewClient(token, gitlab.WithBaseURL(repo.URI())) + client, err := gitlab.NewClient(token, gitlab.WithBaseURL(repo.Host())) if err != nil { return nil, fmt.Errorf("could not create gitlab client with error: %w", err) } @@ -269,6 +268,7 @@ func CreateGitlabClientWithToken(ctx context.Context, token string, repo clients languages: &languagesHandler{ glClient: client, }, + licenses: &licensesHandler{}, }, nil } @@ -276,3 +276,14 @@ func CreateGitlabClientWithToken(ctx context.Context, token string, repo clients func CreateOssFuzzRepoClient(ctx context.Context, logger *log.Logger) (clients.RepoClient, error) { return nil, fmt.Errorf("%w, oss fuzz currently only supported for github repos", clients.ErrUnsupportedFeature) } + +// DetectGitLab: check whether the repoURI is a GitLab URI +// Makes HTTP request to GitLab API. +func DetectGitLab(repoURI string) bool { + var repo repoURL + if err := repo.parse(repoURI); err != nil { + return false + } + + return repo.IsValid() == nil +} diff --git a/clients/gitlabrepo/commits.go b/clients/gitlabrepo/commits.go index 505e932588c..92956dda745 100644 --- a/clients/gitlabrepo/commits.go +++ b/clients/gitlabrepo/commits.go @@ -41,7 +41,7 @@ func (handler *commitsHandler) init(repourl *repoURL) { // nolint: gocognit func (handler *commitsHandler) setup() error { handler.once.Do(func() { - commits, _, err := handler.glClient.Commits.ListCommits(handler.repourl.projectID, &gitlab.ListCommitsOptions{}) + commits, _, err := handler.glClient.Commits.ListCommits(handler.repourl.project, &gitlab.ListCommitsOptions{}) if err != nil { handler.errSetup = fmt.Errorf("request for commits failed with %w", err) return @@ -76,7 +76,7 @@ func (handler *commitsHandler) setup() error { // Commits are able to be a part of multiple merge requests, but the only one that will be important // here is the earliest one. - mergeRequests, _, err := handler.glClient.Commits.ListMergeRequestsByCommit(handler.repourl.projectID, commit.ID) + mergeRequests, _, err := handler.glClient.Commits.ListMergeRequestsByCommit(handler.repourl.project, commit.ID) if err != nil { handler.errSetup = fmt.Errorf("unable to find merge requests associated with commit: %w", err) return diff --git a/clients/gitlabrepo/contributors.go b/clients/gitlabrepo/contributors.go index 0b36594790c..f52afd4d7c9 100644 --- a/clients/gitlabrepo/contributors.go +++ b/clients/gitlabrepo/contributors.go @@ -46,7 +46,7 @@ func (handler *contributorsHandler) setup() error { return } contribs, _, err := handler.glClient.Repositories.Contributors( - handler.repourl.projectID, &gitlab.ListContributorsOptions{}) + handler.repourl.project, &gitlab.ListContributorsOptions{}) if err != nil { handler.errSetup = fmt.Errorf("error during ListContributors: %w", err) return diff --git a/clients/gitlabrepo/issues.go b/clients/gitlabrepo/issues.go index d47b2b3af08..1902bcd0a3f 100644 --- a/clients/gitlabrepo/issues.go +++ b/clients/gitlabrepo/issues.go @@ -40,7 +40,7 @@ func (handler *issuesHandler) init(repourl *repoURL) { func (handler *issuesHandler) setup() error { handler.once.Do(func() { issues, _, err := handler.glClient.Issues.ListProjectIssues( - handler.repourl.projectID, &gitlab.ListProjectIssuesOptions{}) + handler.repourl.project, &gitlab.ListProjectIssuesOptions{}) if err != nil { handler.errSetup = fmt.Errorf("unable to find issues associated with the project id: %w", err) return @@ -49,7 +49,7 @@ func (handler *issuesHandler) setup() error { // There doesn't seem to be a good way to get user access_levels in gitlab so the following way may seem incredibly // barberic, however I couldn't find a better way in the docs. projectAccessTokens, resp, err := handler.glClient.ProjectAccessTokens.ListProjectAccessTokens( - handler.repourl.projectID, &gitlab.ListProjectAccessTokensOptions{}) + handler.repourl.project, &gitlab.ListProjectAccessTokensOptions{}) if err != nil && resp.StatusCode != 401 { handler.errSetup = fmt.Errorf("unable to find access tokens associated with the project id: %w", err) return diff --git a/clients/gitlabrepo/languages.go b/clients/gitlabrepo/languages.go index 1346fabe012..57b9a652ca8 100644 --- a/clients/gitlabrepo/languages.go +++ b/clients/gitlabrepo/languages.go @@ -40,7 +40,7 @@ func (handler *languagesHandler) init(repourl *repoURL) { func (handler *languagesHandler) setup() error { handler.once.Do(func() { client := handler.glClient - languageMap, _, err := client.Projects.GetProjectLanguages(handler.repourl.projectID) + languageMap, _, err := client.Projects.GetProjectLanguages(handler.repourl.project) if err != nil || languageMap == nil { handler.errSetup = fmt.Errorf("request for repo languages failed with %w", err) return diff --git a/clients/gitlabrepo/project.go b/clients/gitlabrepo/project.go index 595f83a5224..a36256e7e7e 100644 --- a/clients/gitlabrepo/project.go +++ b/clients/gitlabrepo/project.go @@ -39,7 +39,7 @@ func (handler *projectHandler) init(repourl *repoURL) { func (handler *projectHandler) setup() error { handler.once.Do(func() { - proj, _, err := handler.glClient.Projects.GetProject(handler.repourl.projectID, &gitlab.GetProjectOptions{}) + proj, _, err := handler.glClient.Projects.GetProject(handler.repourl.project, &gitlab.GetProjectOptions{}) if err != nil { handler.errSetup = fmt.Errorf("request for project failed with error %w", err) return diff --git a/clients/gitlabrepo/releases.go b/clients/gitlabrepo/releases.go index 016af359d86..6aca7adbc24 100644 --- a/clients/gitlabrepo/releases.go +++ b/clients/gitlabrepo/releases.go @@ -44,7 +44,7 @@ func (handler *releasesHandler) setup() error { handler.errSetup = fmt.Errorf("%w: ListReleases only supported for HEAD queries", clients.ErrUnsupportedFeature) return } - releases, _, err := handler.glClient.Releases.ListReleases(handler.repourl.projectID, &gitlab.ListReleasesOptions{}) + releases, _, err := handler.glClient.Releases.ListReleases(handler.repourl.project, &gitlab.ListReleasesOptions{}) if err != nil { handler.errSetup = fmt.Errorf("%w: ListReleases failed", err) return diff --git a/clients/gitlabrepo/repo.go b/clients/gitlabrepo/repo.go index df97ae925ab..5c223ac12af 100644 --- a/clients/gitlabrepo/repo.go +++ b/clients/gitlabrepo/repo.go @@ -18,9 +18,11 @@ package gitlabrepo import ( "fmt" - "regexp" + "net/url" "strings" + "github.com/xanzy/go-gitlab" + "github.com/ossf/scorecard/v4/clients" sce "github.com/ossf/scorecard/v4/errors" ) @@ -30,9 +32,10 @@ const ( ) type repoURL struct { - hostname string + scheme string + host string owner string - projectID string + project string defaultBranch string commitSHA string metadata []string @@ -41,64 +44,91 @@ type repoURL struct { // Parses input string into repoURL struct /* * Accepted input string formats are as follows: - * "gitlab..com//" - * "https://gitlab..com//" + * "gitlab..com//" + * "https://gitlab..com//" + +The following input format is not supported: + * https://gitlab..com/projects/ */ func (r *repoURL) parse(input string) error { - switch { - case strings.Contains(input, "https://"): - input = strings.TrimPrefix(input, "https://") - case strings.Contains(input, "http://"): - input = strings.TrimPrefix(input, "http://") - case strings.Contains(input, "://"): - return sce.WithMessage(sce.ErrScorecardInternal, "unknown input format") + var t string + c := strings.Split(input, "/") + switch l := len(c); { + // owner/repo format is not supported for gitlab, it's github-only + case l == 2: + return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("gitlab repo must specify host: %s", input)) + case l >= 3: + t = input } - stringParts := strings.Split(input, "/") + // Allow skipping scheme for ease-of-use, default to https. + if !strings.Contains(t, "://") { + t = "https://" + t + } - stringParts[2] = strings.TrimSuffix(stringParts[2], "/") + u, e := url.Parse(t) + if e != nil { + return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("url.Parse: %v", e)) + } - r.hostname, r.owner, r.projectID = stringParts[0], stringParts[1], stringParts[2] + const splitLen = 2 + split := strings.SplitN(strings.Trim(u.Path, "/"), "/", splitLen) + if len(split) != splitLen { + return sce.WithMessage(sce.ErrorInvalidURL, fmt.Sprintf("%v. Expected full repository url", input)) + } + + r.scheme, r.host, r.owner, r.project = u.Scheme, u.Host, split[0], split[1] return nil } // URI implements Repo.URI(). -// TODO: there may be a reason the string was originally in format "%s/%s/%s", hostname, owner, projectID, -// however I changed it to be more "userful". func (r *repoURL) URI() string { - return fmt.Sprintf("https://%s", r.hostname) + return fmt.Sprintf("%s/%s/%s", r.host, r.owner, r.project) +} + +func (r *repoURL) Host() string { + return fmt.Sprintf("%s://%s", r.scheme, r.host) } // String implements Repo.String. func (r *repoURL) String() string { - return fmt.Sprintf("%s-%s_%s", r.hostname, r.owner, r.projectID) + return fmt.Sprintf("%s-%s_%s", r.host, r.owner, r.project) } func (r *repoURL) Org() clients.Repo { return &repoURL{ - hostname: r.hostname, - owner: r.owner, - projectID: gitlabOrgProj, + host: r.host, + owner: r.owner, + project: gitlabOrgProj, } } // IsValid implements Repo.IsValid. func (r *repoURL) IsValid() error { - hostMatched, err := regexp.MatchString("gitlab.*com", r.hostname) - if err != nil { - return fmt.Errorf("error processing regex: %w", err) + if strings.Contains(r.host, "gitlab.") { + return nil } - if !hostMatched { - return sce.WithMessage(sce.ErrorInvalidURL, "non gitlab repository found") + + client, err := gitlab.NewClient("", gitlab.WithBaseURL(fmt.Sprintf("%s://%s", r.scheme, r.host))) + if err != nil { + return sce.WithMessage(err, + fmt.Sprintf("couldn't create gitlab client for %s", r.host), + ) } - isNotDigit := func(c rune) bool { return c < '0' || c > '9' } - b := strings.IndexFunc(r.projectID, isNotDigit) == -1 - if !b { - return sce.WithMessage(sce.ErrorInvalidURL, "incorrect format for projectID") + _, resp, err := client.Projects.ListProjects(&gitlab.ListProjectsOptions{}) + if resp == nil || resp.StatusCode != 200 { + return sce.WithMessage(sce.ErrRepoUnreachable, + fmt.Sprintf("couldn't reach gitlab instance at %s", r.host), + ) + } + if err != nil { + return sce.WithMessage(err, + fmt.Sprintf("error when connecting to gitlab instance at %s", r.host), + ) } - if strings.TrimSpace(r.owner) == "" || strings.TrimSpace(r.projectID) == "" { + if strings.TrimSpace(r.owner) == "" || strings.TrimSpace(r.project) == "" { return sce.WithMessage(sce.ErrorInvalidURL, fmt.Sprintf("%v. Expected the full project url", r.URI())) } diff --git a/clients/gitlabrepo/repo_test.go b/clients/gitlabrepo/repo_test.go index 6892bfb5f03..de99f02b1e1 100644 --- a/clients/gitlabrepo/repo_test.go +++ b/clients/gitlabrepo/repo_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" ) func TestRepoURL_IsValid(t *testing.T) { @@ -30,83 +31,58 @@ func TestRepoURL_IsValid(t *testing.T) { wantErr bool }{ { - name: "valid http address", + name: "github repository", expected: repoURL{ - hostname: "gitlab.example.com", - owner: "foo", - projectID: "1234", + scheme: "https", + host: "https://github.com", + owner: "ossf", + project: "scorecard", }, - inputURL: "http://gitlab.example.com/foo/1234", - wantErr: false, + inputURL: "https://github.com/ossf/scorecard", + wantErr: true, }, { - name: "valid https address", + name: "GitHub project with 'gitlab.' in the title", expected: repoURL{ - hostname: "gitlab.example.com", - owner: "foo", - projectID: "1234", + scheme: "http", + host: "http://github.com", + owner: "foo", + project: "gitlab.test", }, - inputURL: "https://gitlab.example.com/foo/1234", - wantErr: false, + inputURL: "http://github.com/foo/gitlab.test", + wantErr: true, }, { - name: "valid http address with trailing slash", + name: "valid gitlab project", expected: repoURL{ - hostname: "gitlab.example.com", - owner: "foo", - projectID: "1234", + host: "https://gitlab.com", + owner: "ossf-test", + project: "scorecard-check-binary-artifacts-e2e", }, - inputURL: "http://gitlab.example.com/foo/1234/", + inputURL: "https://gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e", wantErr: false, }, { name: "valid https address with trailing slash", expected: repoURL{ - hostname: "gitlab.example.com", - owner: "foo", - projectID: "1234", + scheme: "https", + host: "https://gitlab.com", + owner: "ossf-test", + project: "scorecard-check-binary-artifacts-e2e", }, - inputURL: "https://gitlab.example.com/foo/1234/", + inputURL: "https://gitlab.com/ossf-test/scorecard-check-binary-artifacts-e2e/", wantErr: false, }, + { - name: "non gitlab repository", - expected: repoURL{ - hostname: "github.com", - owner: "foo", - projectID: "1234", - }, - inputURL: "https://github.com/foo/1234", - wantErr: true, - }, - { - name: "GitLab project with wrong projectID", - expected: repoURL{ - hostname: "gitlab.example.com", - owner: "foo", - projectID: "bar", - }, - inputURL: "https://gitlab.example.com/foo/bar", - wantErr: true, - }, - { - name: "GitHub project with 'gitlab.' in the title", - expected: repoURL{ - hostname: "github.com", - owner: "foo", - projectID: "gitlab.test", - }, - inputURL: "http://github.com/foo/gitlab.test", - wantErr: true, - }, - { - name: "valid gitlab project without http or https", + name: "valid hosted gitlab project", expected: repoURL{ - hostname: "gitlab.example.com", - owner: "foo", - projectID: "1234", + scheme: "https", + host: "https://salsa.debian.org", + owner: "webmaster-team", + project: "webml", }, - inputURL: "gitlab.example.com/foo/1234", + inputURL: "https://salsa.debian.org/webmaster-team/webwml", wantErr: false, }, } @@ -115,9 +91,9 @@ func TestRepoURL_IsValid(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() r := repoURL{ - hostname: tt.expected.hostname, - owner: tt.expected.owner, - projectID: tt.expected.projectID, + host: tt.expected.host, + owner: tt.expected.owner, + project: tt.expected.project, } if err := r.parse(tt.inputURL); err != nil { t.Errorf("repoURL.parse() error = %v", err) @@ -125,12 +101,55 @@ func TestRepoURL_IsValid(t *testing.T) { if err := r.IsValid(); (err != nil) != tt.wantErr { t.Errorf("repoURL.IsValid() error = %v, wantErr %v", err, tt.wantErr) } - if !tt.wantErr && !cmp.Equal(tt.expected, r, cmp.AllowUnexported(repoURL{})) { - fmt.Println("expected: " + tt.expected.hostname + " GOT: " + r.hostname) + if !tt.wantErr && !cmp.Equal(tt.expected, r, cmpopts.IgnoreUnexported(repoURL{})) { + fmt.Println("expected: " + tt.expected.host + " GOT: " + r.host) fmt.Println("expected: " + tt.expected.owner + " GOT: " + r.owner) - fmt.Println("expected: " + tt.expected.projectID + " GOT: " + r.projectID) + fmt.Println("expected: " + tt.expected.project + " GOT: " + r.project) t.Errorf("Got diff: %s", cmp.Diff(tt.expected, r)) } + if !cmp.Equal(r.Host(), tt.expected.host) { + t.Errorf("%s expected host: %s got host %s", tt.inputURL, tt.expected.host, r.Host()) + } }) } } + +func TestRepoURL_DetectGitlab(t *testing.T) { + tests := []struct { + repouri string + expected bool + }{ + { + repouri: "github.com/ossf/scorecard", + expected: false, + }, + { + repouri: "ossf/scorecard", + expected: false, + }, + { + repouri: "https://github.com/ossf/scorecard", + expected: false, + }, + { + repouri: "gitlab.com/gitlab-org/gitlab", + expected: true, + }, + { + repouri: "https://salsa.debian.org/webmaster-team/webml", + expected: true, + }, + { + // Invalid repo + repouri: "https://abcdef.foo.txt/nilproj/nilrepo", + expected: false, + }, + } + + for _, tt := range tests { + g := DetectGitLab(tt.repouri) + if g != tt.expected { + t.Errorf("got %s isgitlab: %t expected %t", tt.repouri, g, tt.expected) + } + } +} diff --git a/clients/gitlabrepo/search.go b/clients/gitlabrepo/search.go index cb1a2f5cbf1..2c4d94245b1 100644 --- a/clients/gitlabrepo/search.go +++ b/clients/gitlabrepo/search.go @@ -45,7 +45,7 @@ func (handler *searchHandler) search(request clients.SearchRequest) (clients.Sea return clients.SearchResponse{}, fmt.Errorf("handler.buildQuery: %w", err) } - blobs, _, err := handler.glClient.Search.BlobsByProject(handler.repourl.projectID, query, &gitlab.SearchOptions{}) + blobs, _, err := handler.glClient.Search.BlobsByProject(handler.repourl.project, query, &gitlab.SearchOptions{}) if err != nil { return clients.SearchResponse{}, fmt.Errorf("Search.BlobsByProject: %w", err) } @@ -60,7 +60,7 @@ func (handler *searchHandler) buildQuery(request clients.SearchRequest) (string, if _, err := queryBuilder.WriteString( fmt.Sprintf("%s project:%s/%s", strings.ReplaceAll(request.Query, "/", " "), - handler.repourl.owner, handler.repourl.projectID)); err != nil { + handler.repourl.owner, handler.repourl.project)); err != nil { return "", fmt.Errorf("WriteString: %w", err) } if request.Filename != "" { diff --git a/clients/gitlabrepo/searchCommits.go b/clients/gitlabrepo/searchCommits.go index 1fa1ea9f727..6de69701d0e 100644 --- a/clients/gitlabrepo/searchCommits.go +++ b/clients/gitlabrepo/searchCommits.go @@ -41,7 +41,7 @@ func (handler *searchCommitsHandler) search(request clients.SearchCommitsOptions return nil, fmt.Errorf("handler.buildQuiery: %w", err) } - commits, _, err := handler.glClient.Search.CommitsByProject(handler.repourl.projectID, query, &gitlab.SearchOptions{}) + commits, _, err := handler.glClient.Search.CommitsByProject(handler.repourl.project, query, &gitlab.SearchOptions{}) if err != nil { return nil, fmt.Errorf("Search.Commits: %w", err) } @@ -75,7 +75,7 @@ func (handler *searchCommitsHandler) buildQuery(request clients.SearchCommitsOpt var queryBuilder strings.Builder if _, err := queryBuilder.WriteString( fmt.Sprintf("project:%s/%s author:%s", - handler.repourl.owner, handler.repourl.projectID, + handler.repourl.owner, handler.repourl.project, request.Author)); err != nil { return "", fmt.Errorf("writestring: %w", err) } diff --git a/clients/gitlabrepo/searchCommits_test.go b/clients/gitlabrepo/searchCommits_test.go index a509d62543e..b7ad84d2173 100644 --- a/clients/gitlabrepo/searchCommits_test.go +++ b/clients/gitlabrepo/searchCommits_test.go @@ -34,8 +34,8 @@ func TestSearchCommitsBuildQuery(t *testing.T) { { name: "Basic", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchCommitsOptions{ Author: "testAuthor", @@ -45,8 +45,8 @@ func TestSearchCommitsBuildQuery(t *testing.T) { { name: "EmptyQuery:", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchCommitsOptions{}, hasError: true, diff --git a/clients/gitlabrepo/search_test.go b/clients/gitlabrepo/search_test.go index 2cd7e65edca..284fa23cbee 100644 --- a/clients/gitlabrepo/search_test.go +++ b/clients/gitlabrepo/search_test.go @@ -34,8 +34,8 @@ func TestBuildQuery(t *testing.T) { { name: "Basic", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchRequest{ Query: "testquery", @@ -45,8 +45,8 @@ func TestBuildQuery(t *testing.T) { { name: "EmptyQuery", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchRequest{}, hasError: true, @@ -55,8 +55,8 @@ func TestBuildQuery(t *testing.T) { { name: "WithFilename", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchRequest{ Query: "testquery", @@ -67,8 +67,8 @@ func TestBuildQuery(t *testing.T) { { name: "WithPath", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchRequest{ Query: "testquery", @@ -79,8 +79,8 @@ func TestBuildQuery(t *testing.T) { { name: "WithFilenameAndPath", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchRequest{ Query: "testquery", @@ -92,8 +92,8 @@ func TestBuildQuery(t *testing.T) { { name: "WithFilenameAndPathWithSeperator", repourl: &repoURL{ - owner: "testowner", - projectID: "1234", + owner: "testowner", + project: "1234", }, searchReq: clients.SearchRequest{ Query: "testquery/query", diff --git a/clients/gitlabrepo/statuses.go b/clients/gitlabrepo/statuses.go index bce2e198dde..317df54db87 100644 --- a/clients/gitlabrepo/statuses.go +++ b/clients/gitlabrepo/statuses.go @@ -34,7 +34,7 @@ func (handler *statusesHandler) init(repourl *repoURL) { // for gitlab this only works if ref is SHA. func (handler *statusesHandler) listStatuses(ref string) ([]clients.Status, error) { commitStatuses, _, err := handler.glClient.Commits.GetCommitStatuses( - handler.repourl.projectID, ref, &gitlab.GetCommitStatusesOptions{}) + handler.repourl.project, ref, &gitlab.GetCommitStatusesOptions{}) if err != nil { return nil, fmt.Errorf("error getting commit statuses: %w", err) } diff --git a/clients/gitlabrepo/webhook.go b/clients/gitlabrepo/webhook.go index 2d2970fd678..9c9d6c49277 100644 --- a/clients/gitlabrepo/webhook.go +++ b/clients/gitlabrepo/webhook.go @@ -40,7 +40,7 @@ func (handler *webhookHandler) init(repourl *repoURL) { func (handler *webhookHandler) setup() error { handler.once.Do(func() { projectHooks, _, err := handler.glClient.Projects.ListProjectHooks( - handler.repourl.projectID, &gitlab.ListProjectHooksOptions{}) + handler.repourl.project, &gitlab.ListProjectHooksOptions{}) if err != nil { handler.errSetup = fmt.Errorf("request for project hooks failed with %w", err) return diff --git a/clients/gitlabrepo/workflows.go b/clients/gitlabrepo/workflows.go index e90ef5e86d0..cba2cef59ad 100644 --- a/clients/gitlabrepo/workflows.go +++ b/clients/gitlabrepo/workflows.go @@ -35,7 +35,7 @@ func (handler *workflowsHandler) init(repourl *repoURL) { func (handler *workflowsHandler) listSuccessfulWorkflowRuns(filename string) ([]clients.WorkflowRun, error) { var buildStates []gitlab.BuildStateValue buildStates = append(buildStates, gitlab.Success) - jobs, _, err := handler.glClient.Jobs.ListProjectJobs(handler.repourl.projectID, + jobs, _, err := handler.glClient.Jobs.ListProjectJobs(handler.repourl.project, &gitlab.ListJobsOptions{Scope: &buildStates}) if err != nil { return nil, fmt.Errorf("error getting project jobs: %w", err) diff --git a/clients/localdir/repo.go b/clients/localdir/repo.go index 01dfaf6a531..c3426e566af 100644 --- a/clients/localdir/repo.go +++ b/clients/localdir/repo.go @@ -37,6 +37,10 @@ func (r *repoLocal) URI() string { return fmt.Sprintf("file://%s", r.path) } +func (r *repoLocal) Host() string { + return "" +} + // String implements Repo.String. func (r *repoLocal) String() string { return r.URI() diff --git a/clients/mockclients/repo.go b/clients/mockclients/repo.go index 5a5c1e0ba27..a2812268cf1 100644 --- a/clients/mockclients/repo.go +++ b/clients/mockclients/repo.go @@ -129,6 +129,14 @@ func (m *MockRepo) URI() string { return ret0 } +// URI mocks base method. +func (m *MockRepo) Host() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Host") + ret0, _ := ret[0].(string) + return ret0 +} + // URI indicates an expected call of URI. func (mr *MockRepoMockRecorder) URI() *gomock.Call { mr.mock.ctrl.T.Helper() diff --git a/clients/repo.go b/clients/repo.go index eac20dd0683..6d4c70f7671 100644 --- a/clients/repo.go +++ b/clients/repo.go @@ -17,6 +17,7 @@ package clients // Repo interface uniquely identifies a repo. type Repo interface { URI() string + Host() string String() string Org() Repo IsValid() error diff --git a/e2e/attestor_policy_test.go b/e2e/attestor_policy_test.go index 7de6a87aa9d..56c7ec5691e 100644 --- a/e2e/attestor_policy_test.go +++ b/e2e/attestor_policy_test.go @@ -15,7 +15,6 @@ package e2e import ( - "fmt" "os" "strings" @@ -185,7 +184,6 @@ var _ = Describe("E2E TEST PAT: scorecard-attestor policy", func() { } for _, tc := range tt { - fmt.Printf("attestor_policy_test.go: %s\n", tc.name) f, err := os.CreateTemp("/tmp", strings.ReplaceAll(tc.name, " ", "-")) Expect(err).Should(BeNil()) defer os.Remove(f.Name())