fix: Check source project access for GitLab PRs

- Added a check to ensure the GitLab token has read access to the source
repository when processing pull requests.
- This addresses an issue where builds could fail if the token lacked the
necessary scope to access private source repositories.

https://issues.redhat.com/browse/SRVKP-8902

Co-Authored-By: Claude <[email protected]>
Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: 2 people

pkg/provider/gitlab/gitlab.go

@@ -195,9 +195,11 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
 	}
 	v.apiURL = apiURL
 
-	v.gitlabClient, err = gitlab.NewClient(runevent.Provider.Token, gitlab.WithBaseURL(apiURL))
-	if err != nil {
-		return err
+	if v.gitlabClient == nil {
+		v.gitlabClient, err = gitlab.NewClient(runevent.Provider.Token, gitlab.WithBaseURL(apiURL))
+		if err != nil {
+			return err
+		}
 	}
 	v.Token = &runevent.Provider.Token
 
@@ -211,6 +213,19 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
 		v.sourceProjectID = runevent.SourceProjectID
 	}
 
+	// check that we have access to the source project if it's a private repo, this should only occur on Merge Requests
+	if runevent.TriggerTarget == triggertype.PullRequest {
+		_, resp, err := v.Client().Projects.GetProject(runevent.SourceProjectID, &gitlab.GetProjectOptions{})
+		errmsg := fmt.Sprintf("failed to access GitLab source repository ID %d: please ensure token has 'read_repository' scope on that repository",
+			runevent.SourceProjectID)
+		if resp != nil && resp.StatusCode == http.StatusNotFound {
+			return fmt.Errorf("%s", errmsg)
+		}
+		if err != nil {
+			return fmt.Errorf("%s: %w", errmsg, err)
+		}
+	}
+
 	// if we don't have sourceProjectID (ie: incoming-webhook) then try to set
 	// it ASAP if we can.
 	if v.sourceProjectID == 0 && runevent.Organization != "" && runevent.Repository != "" {
@@ -280,27 +295,35 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts
 		Context:     gitlab.Ptr(contextName),
 	}
 
-	// setCommitStatus attempts to set the commit status for a given SHA, handling GitLab's permission model.
-	// First tries the source project (fork), then falls back to the target project (upstream repo).
-	// This fallback is necessary because:
-	// - In fork/MR scenarios, the GitLab token may not have write access to the contributor's fork
-	// - This ensures commit status can be displayed somewhere useful regardless
-	// of permission differences
-	// Returns nil if status is set on either project, logs error if both attempts fail.
+	// In case we have access, set the status. Typically, on a Merge Request (MR)
+	// from a fork in an upstream repository, the token needs to have write access
+	// to the fork repository in order to create a status. However, the token set on the
+	// Repository CR usually doesn't have such broad access, preventing from creating
+	// a status comment on it.
+	// This would work on a push or an MR from a branch within the same repo.
+	// Ignoring errors because of the write access issues,
 	_, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt)
-	if err == nil {
-		v.Logger.Debugf("created commit status on source project ID %d", event.SourceProjectID)
+	if err != nil {
+		v.Logger.Debugf("cannot set status with the GitLab token on the source project: %v", err)
+	} else {
+		// we managed to set the status on the source repo, all good we are done
+		v.Logger.Debugf("created commit status on source project ID %d", event.TargetProjectID)
 		return nil
 	}
 	if _, _, err2 := v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt); err2 == nil {
 		v.Logger.Debugf("created commit status on target project ID %d", event.TargetProjectID)
+		// we managed to set the status on the target repo, all good we are done
 		return nil
 	}
-	v.Logger.Debugf(
-		"Failed to create commit status: source project ID %d, target project ID %d. "+
-			"If you want Gitlab Pipeline Status update, ensure your GitLab token has access "+
-			"to the source repository. Error: %v",
-		event.SourceProjectID, event.TargetProjectID, err)
+	v.Logger.Debugf("cannot set status with the GitLab token on the target project: %v", err)
+	// we only show the first error as it's likely something the user has more control to fix
+	// the second err is cryptic as it needs a dummy gitlab pipeline to start
+	// with and will only give more confusion in the event namespace
+	v.eventEmitter.EmitMessage(v.repo, zap.InfoLevel, "FailedToSetCommitStatus",
+		fmt.Sprintf("failed to create commit status: source project ID %d, target project ID %d. "+
+			"If you want Gitlab Pipeline Status update, ensure your GitLab token giving it access "+
+			"to the source repository. %v",
+			event.SourceProjectID, event.TargetProjectID, err))
 
 	eventType := triggertype.IsPullRequestType(event.EventType)
 	// When a GitOps command is sent on a pushed commit, it mistakenly treats it as a pull_request

pkg/provider/gitlab/gitlab_test.go

@@ -396,6 +396,99 @@ func TestSetClient(t *testing.T) {
 	assert.Equal(t, expected, logs[0].Message)
 }
 
+func TestSetClientRepositoryAccessCheck(t *testing.T) {
+	ctx, _ := rtesting.SetupFakeContext(t)
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	fakelogger := zap.New(observer).Sugar()
+	run := &params.Run{
+		Clients: clients.Clients{
+			Log: fakelogger,
+		},
+	}
+
+	tests := []struct {
+		name              string
+		triggerTarget     triggertype.Trigger
+		sourceProjectID   int
+		setupMockResponse func(*http.ServeMux, int)
+		expectedError     string
+	}{
+		{
+			name:            "Non-pull request trigger should skip access check",
+			triggerTarget:   triggertype.Push,
+			sourceProjectID: 123,
+			setupMockResponse: func(_ *http.ServeMux, _ int) {
+				// No mock needed - should not make the call
+			},
+			expectedError: "",
+		},
+		{
+			name:            "Pull request with successful access",
+			triggerTarget:   triggertype.PullRequest,
+			sourceProjectID: 123,
+			setupMockResponse: func(mux *http.ServeMux, projectID int) {
+				path := fmt.Sprintf("/projects/%d", projectID)
+				mux.HandleFunc(path, func(rw http.ResponseWriter, r *http.Request) {
+					if r.Method == http.MethodGet {
+						rw.WriteHeader(http.StatusOK)
+						fmt.Fprint(rw, `{"id": 123, "name": "test-repo"}`)
+					}
+				})
+			},
+			expectedError: "",
+		},
+		{
+			name:            "Pull request with not found should return specific error",
+			triggerTarget:   triggertype.PullRequest,
+			sourceProjectID: 456,
+			setupMockResponse: func(mux *http.ServeMux, projectID int) {
+				path := fmt.Sprintf("/projects/%d", projectID)
+				mux.HandleFunc(path, func(rw http.ResponseWriter, r *http.Request) {
+					if r.Method == http.MethodGet {
+						// Return 404 without error to test the status code check
+						rw.WriteHeader(http.StatusNotFound)
+						fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
+					}
+				})
+			},
+			expectedError: "failed to access GitLab source repository ID 456: please ensure token has 'read_repository' scope on that repository",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockClient, mux, tearDown := thelp.Setup(t)
+			defer tearDown()
+
+			// Setup the mock for the repository access check API call
+			if tt.setupMockResponse != nil {
+				tt.setupMockResponse(mux, tt.sourceProjectID)
+			}
+
+			v := &Provider{gitlabClient: mockClient}
+			event := &info.Event{
+				Provider: &info.Provider{
+					Token: "test-token",
+				},
+				Organization:    "test-org",
+				Repository:      "test-repo",
+				TriggerTarget:   tt.triggerTarget,
+				SourceProjectID: tt.sourceProjectID,
+				TargetProjectID: 123,
+			}
+
+			err := v.SetClient(ctx, run, event, nil, nil)
+
+			if tt.expectedError != "" {
+				assert.Assert(t, err != nil, "expected error but got none")
+				assert.ErrorContains(t, err, tt.expectedError)
+			} else {
+				assert.NilError(t, err, "unexpected error: %v", err)
+			}
+		})
+	}
+}
+
 func TestSetClientDetectAPIURL(t *testing.T) {
 	ctx, _ := rtesting.SetupFakeContext(t)
 	observer, _ := zapobserver.New(zap.InfoLevel)