BUG: CI-Tests: internal error: internal error: Client.Repositories.ListStatuses: error getting commit statuses

[adam-moss]

Describe the bug
A clear and concise description of what the bug is.

Error: check runtime error: CI-Tests: internal error: internal error: Client.Repositories.ListStatuses: error getting commit statuses: GET https://gitlab.com/api/v4/projects/<redact>/repository/commits/statuses: 404 {message: 404 Commit Not Found}
2023/11/28 21:12:32 error during command execution: check runtime error: CI-Tests: internal error: internal error: Client.Repositories.ListStatuses: error getting commit statuses: GET https://gitlab.com/api/v4/projects/<redact>/repository/commits/statuses: 404 {message: 404 Commit Not Found}

Reproduction steps
Steps to reproduce the behavior:

1. Create a private repository on gitlab.com
2. Run scorecard --repo gitlab.com/path/to/repo

Expected behavior
A clear and concise description of what you expected to happen.

scorecard completes successfully with no errors reported.

Additional context
I have built scorecard from source with the latest commit on the main branch.

[naveensrinivasan]

@adam-moss I tried to recreate and I wasn't able to.

Can you please add some specifics as to what of repository? Did it have any code? Did it have any commits?

Thanks

[spencerschrock]

@adam-moss I tried to recreate and I wasn't able to.

This may be relevant #3581 (comment)

[aarongoldenthal]

I hit this today with Scorecard 4.13.1 at https://gitlab.com/gitlab-ci-utils/gitlab-pmd-cpd (and most of the others I tried in the same group, which are all public).

The error was under CI-Tests:

internal error: internal error: Client.Repositories.ListStatuses: https://github.com/ossf/scorecard/blob/49c0eed3a423f00c872b5c3c9f1bbca9e8aae799/docs/checks.md#ci-tests
error getting commit statuses: GET
https://gitlab.com/api/v4/projects/22151381/repository/commits/statuses
404 {message: 404 Commit Not Found}

The listed URL at repository/commits/statuses is not a valid GitLab API endpoint. I believe it should be repository/commits/:sha/statuses.

[spencerschrock]

We don't actually set the endpoint ourselves, we use a library.

scorecard/clients/gitlabrepo/statuses.go

Lines 35 to 43 in b3ad602

	func (handler *statusesHandler) listStatuses(ref string) ([]clients.Status, error) {
	commitStatuses, _, err := handler.glClient.Commits.GetCommitStatuses(
	handler.repourl.projectID, ref, &gitlab.GetCommitStatusesOptions{})
	if err != nil {
	return nil, fmt.Errorf("error getting commit statuses: %w", err)
	}
	return statusFromData(commitStatuses), nil
	}

The problem seems to be that pr.HeadSHA isn't set (correctly) here:

scorecard/checks/raw/ci_tests.go

Lines 61 to 66 in b3ad602

	statuses, err := c.ListStatuses(pr.HeadSHA)
	if err != nil {
	return checker.CITestData{}, sce.WithMessage(
	sce.ErrScorecardInternal,
	fmt.Sprintf("Client.Repositories.ListStatuses: %v", err),
	)

Based on the graphQL documentation: https://docs.gitlab.com/ee/api/graphql/reference/#mergerequest:
In the graphQL query we fetch mergeCommitSha which is "SHA of the merge request commit (set once merged)."

scorecard/clients/gitlabrepo/graphql.go

Lines 64 to 96 in b3ad602

	type graphqlMergeRequestNode struct {
	ID GitlabGID `graphql:"id"`
	IID string `graphql:"iid"`
	MergedAt time.Time `graphql:"mergedAt"`
	Author struct {
	Username string `graphql:"username"`
	ID GitlabGID `graphql:"id"`
	} `graphql:"author"`
	MergedBy struct {
	Username string `graphql:"username"`
	ID GitlabGID `graphql:"id"`
	} `graphql:"mergeUser"`
	Commits struct {
	Nodes []struct {
	SHA string `graphql:"sha"`
	} `graphql:"nodes"`
	} `graphql:"commits"`
	Reviewers struct {
	Nodes []struct {
	Username string `graphql:"username"`
	ID GitlabGID `graphql:"id"`
	MergeRequestInteraction struct {
	ReviewState string `graphql:"reviewState"`
	} `graphql:"mergeRequestInteraction"`
	} `graphql:"nodes"`
	} `graphql:"reviewers"`
	Approvers struct {
	Nodes []struct {
	Username string `graphql:"username"`
	ID GitlabGID `graphql:"id"`
	} `graphql:"nodes"`
	} `graphql:"approvedBy"`
	MergeCommitSHA string `graphql:"mergeCommitSha"`

I assume we actually want diffHeadSha which is "Diff head SHA of the merge request."

[aarongoldenthal]

After doing a few tests with the GraphQL API, yes diffHeadSha looks to be the correct field. The mergeCommitSha field returns null for the following cases:

• MR is open
• MR is closed
• MR was merged, but the merge method was fast-forward merge (so no merge commit was created)

[spencerschrock]

yes diffHeadSha looks to be the correct field. The mergeCommitSha field returns null for the following cases:

• MR is open
• MR is closed
• MR was merged, but the merge method was fast-forward merge (so no merge commit was created)

Thanks for the info, we've run into issues with different merge styles before, so knowing about the fast-forward merge for GitLab is helpful. Aside from being null or not, I think it's also the wrong semantic. We want the head SHA from while it was still a PR so we can determine if CI-Tests ran before it was merged. So diffHeadSha would help match the semantics our GitHub client already does.

After doing a few tests with the GraphQL API

If this experimentation was using the Scorecard codebase, or you're interested, feel free to send a PR. Or someone else can get to it