From 2668911d556d4f1ea2a0ea5f3a666a640e22275e Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Mon, 10 Feb 2025 11:57:41 +0530 Subject: [PATCH] Fix GitOps Command Issue on Pushed Commit by Unautorized User Issue: when an unautorized user sends GitOps comment on a pushed commit, PAC is triggering CI since access check is done only for pull_request event in verifyRepoAndUser func of controller. Solution: added a check for push event and Ops comment event type in verifyRepoAndUser func. https://issues.redhat.com/browse/SRVKP-7110 Signed-off-by: Zaki Shaikh --- pkg/pipelineascode/match.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go index 14d397969..e69825444 100644 --- a/pkg/pipelineascode/match.go +++ b/pkg/pipelineascode/match.go @@ -132,6 +132,14 @@ is that what you want? make sure you use -n when generating the secret, eg: echo return repo, err } + // Verify whether the sender of the GitOps command (e.g., /test) has the appropriate permissions to + // trigger CI on the repository, as any user is able to comment on a pushed commit in open-source repositories. + if p.event.TriggerTarget == triggertype.Push && opscomments.IsAnyOpsEventType(p.event.EventType) { + if allowed, err := p.checkAccessOrFail(ctx, repo); !allowed { + return nil, err + } + } + // Check if the submitter is allowed to run this. // on push we don't need to check the policy since the user has pushed to the repo so it has access to it. // on comment we skip it for now, we are going to check later on @@ -410,3 +418,29 @@ func (p *PacRun) createNeutralStatus(ctx context.Context) error { return nil } + +func (p *PacRun) checkAccessOrFail(ctx context.Context, repo *v1alpha1.Repository) (bool, error) { + allowed, err := p.vcx.IsAllowed(ctx, p.event) + if err != nil { + return false, err + } + if allowed { + return true, nil + } + msg := fmt.Sprintf("User %s is not allowed to trigger CI by GitOps command on this repo.", p.event.Sender) + if p.event.AccountID != "" { + msg = fmt.Sprintf("User: %s AccountID: %s is not allowed to trigger CI by GitOps command on this repo.", p.event.Sender, p.event.AccountID) + } + p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryPermissionDenied", msg) + status := provider.StatusOpts{ + Status: CompletedStatus, + Title: fmt.Sprintf("User %s is not allowed", p.event.Sender), + Conclusion: failureConclusion, + Text: msg, + DetailsURL: p.event.URL, + } + if err := p.vcx.CreateStatus(ctx, p.event, status); err != nil { + return false, fmt.Errorf("failed to run create status, user is not allowed to run the CI:: %w", err) + } + return false, nil +}