Skip to content

Commit 5173302

Browse files
chmouelclaude
authored andcommitted
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 <noreply@anthropic.com> Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent 542f1a0 commit 5173302

File tree

2 files changed

+132
-6
lines changed

2 files changed

+132
-6
lines changed

pkg/provider/gitlab/gitlab.go

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,11 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
195195
}
196196
v.apiURL = apiURL
197197

198-
v.gitlabClient, err = gitlab.NewClient(runevent.Provider.Token, gitlab.WithBaseURL(apiURL))
199-
if err != nil {
200-
return err
198+
if v.gitlabClient == nil {
199+
v.gitlabClient, err = gitlab.NewClient(runevent.Provider.Token, gitlab.WithBaseURL(apiURL))
200+
if err != nil {
201+
return err
202+
}
201203
}
202204
v.Token = &runevent.Provider.Token
203205

@@ -211,6 +213,19 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
211213
v.sourceProjectID = runevent.SourceProjectID
212214
}
213215

216+
// check that we have access to the source project if it's a private repo, this should only occur on Merge Requests
217+
if runevent.TriggerTarget == triggertype.PullRequest {
218+
_, resp, err := v.Client().Projects.GetProject(runevent.SourceProjectID, &gitlab.GetProjectOptions{})
219+
errmsg := fmt.Sprintf("failed to access GitLab source repository ID %d: please ensure token has 'read_repository' scope on that repository",
220+
runevent.SourceProjectID)
221+
if resp != nil && resp.StatusCode == http.StatusNotFound {
222+
return fmt.Errorf("%s", errmsg)
223+
}
224+
if err != nil {
225+
return fmt.Errorf("%s: %w", errmsg, err)
226+
}
227+
}
228+
214229
// if we don't have sourceProjectID (ie: incoming-webhook) then try to set
215230
// it ASAP if we can.
216231
if v.sourceProjectID == 0 && runevent.Organization != "" && runevent.Repository != "" {
@@ -287,10 +302,28 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts
287302
// a status comment on it.
288303
// This would work on a push or an MR from a branch within the same repo.
289304
// Ignoring errors because of the write access issues,
290-
if _, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt); err != nil {
291-
v.eventEmitter.EmitMessage(v.repo, zap.ErrorLevel, "FailedToSetCommitStatus",
292-
"cannot set status with the GitLab token because of: "+err.Error())
305+
_, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt)
306+
if err != nil {
307+
v.Logger.Debugf("cannot set status with the GitLab token on the source project: %v", err)
308+
} else {
309+
// we managed to set the status on the source repo, all good we are done
310+
v.Logger.Debugf("created commit status on source project ID %d", event.TargetProjectID)
311+
return nil
312+
}
313+
if _, _, err2 := v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt); err2 == nil {
314+
v.Logger.Debugf("created commit status on target project ID %d", event.TargetProjectID)
315+
// we managed to set the status on the target repo, all good we are done
316+
return nil
293317
}
318+
v.Logger.Debugf("cannot set status with the GitLab token on the target project: %v", err)
319+
// we only show the first error as it's likely something the user has more control to fix
320+
// the second err is cryptic as it needs a dummy gitlab pipeline to start
321+
// with and will only give more confusion in the event namespace
322+
v.eventEmitter.EmitMessage(v.repo, zap.InfoLevel, "FailedToSetCommitStatus",
323+
fmt.Sprintf("failed to create commit status: source project ID %d, target project ID %d. "+
324+
"If you want Gitlab Pipeline Status update, ensure your GitLab token giving it access "+
325+
"to the source repository. %v",
326+
event.SourceProjectID, event.TargetProjectID, err))
294327

295328
eventType := triggertype.IsPullRequestType(event.EventType)
296329
// When a GitOps command is sent on a pushed commit, it mistakenly treats it as a pull_request

pkg/provider/gitlab/gitlab_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,99 @@ func TestSetClient(t *testing.T) {
301301
assert.Equal(t, expected, logs[0].Message)
302302
}
303303

304+
func TestSetClientRepositoryAccessCheck(t *testing.T) {
305+
ctx, _ := rtesting.SetupFakeContext(t)
306+
observer, _ := zapobserver.New(zap.InfoLevel)
307+
fakelogger := zap.New(observer).Sugar()
308+
run := &params.Run{
309+
Clients: clients.Clients{
310+
Log: fakelogger,
311+
},
312+
}
313+
314+
tests := []struct {
315+
name string
316+
triggerTarget triggertype.Trigger
317+
sourceProjectID int
318+
setupMockResponse func(*http.ServeMux, int)
319+
expectedError string
320+
}{
321+
{
322+
name: "Non-pull request trigger should skip access check",
323+
triggerTarget: triggertype.Push,
324+
sourceProjectID: 123,
325+
setupMockResponse: func(_ *http.ServeMux, _ int) {
326+
// No mock needed - should not make the call
327+
},
328+
expectedError: "",
329+
},
330+
{
331+
name: "Pull request with successful access",
332+
triggerTarget: triggertype.PullRequest,
333+
sourceProjectID: 123,
334+
setupMockResponse: func(mux *http.ServeMux, projectID int) {
335+
path := fmt.Sprintf("/projects/%d", projectID)
336+
mux.HandleFunc(path, func(rw http.ResponseWriter, r *http.Request) {
337+
if r.Method == http.MethodGet {
338+
rw.WriteHeader(http.StatusOK)
339+
fmt.Fprint(rw, `{"id": 123, "name": "test-repo"}`)
340+
}
341+
})
342+
},
343+
expectedError: "",
344+
},
345+
{
346+
name: "Pull request with not found should return specific error",
347+
triggerTarget: triggertype.PullRequest,
348+
sourceProjectID: 456,
349+
setupMockResponse: func(mux *http.ServeMux, projectID int) {
350+
path := fmt.Sprintf("/projects/%d", projectID)
351+
mux.HandleFunc(path, func(rw http.ResponseWriter, r *http.Request) {
352+
if r.Method == http.MethodGet {
353+
// Return 404 without error to test the status code check
354+
rw.WriteHeader(http.StatusNotFound)
355+
fmt.Fprint(rw, `{"message": "404 Project Not Found"}`)
356+
}
357+
})
358+
},
359+
expectedError: "failed to access GitLab source repository ID 456: please ensure token has 'read_repository' scope on that repository",
360+
},
361+
}
362+
363+
for _, tt := range tests {
364+
t.Run(tt.name, func(t *testing.T) {
365+
mockClient, mux, tearDown := thelp.Setup(t)
366+
defer tearDown()
367+
368+
// Setup the mock for the repository access check API call
369+
if tt.setupMockResponse != nil {
370+
tt.setupMockResponse(mux, tt.sourceProjectID)
371+
}
372+
373+
v := &Provider{gitlabClient: mockClient}
374+
event := &info.Event{
375+
Provider: &info.Provider{
376+
Token: "test-token",
377+
},
378+
Organization: "test-org",
379+
Repository: "test-repo",
380+
TriggerTarget: tt.triggerTarget,
381+
SourceProjectID: tt.sourceProjectID,
382+
TargetProjectID: 123,
383+
}
384+
385+
err := v.SetClient(ctx, run, event, nil, nil)
386+
387+
if tt.expectedError != "" {
388+
assert.Assert(t, err != nil, "expected error but got none")
389+
assert.ErrorContains(t, err, tt.expectedError)
390+
} else {
391+
assert.NilError(t, err, "unexpected error: %v", err)
392+
}
393+
})
394+
}
395+
}
396+
304397
func TestSetClientDetectAPIURL(t *testing.T) {
305398
ctx, _ := rtesting.SetupFakeContext(t)
306399
observer, _ := zapobserver.New(zap.InfoLevel)

0 commit comments

Comments
 (0)