fix(github): handle bot users in status checks

- Added logic to skip setting status for bot users in `CreateStatus` method.
- Updated `processEvent` to capture user type from GitHub events.
- Modified tests to include scenarios for bot users.
- Corrected minor formatting issues in documentation.

More details here: https://issues.redhat.com/browse/SRVKP-7242

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

Author: chmouel

docs/content/docs/guide/running.md

@@ -52,40 +52,47 @@ run a PipelineRun on CI:
   owns the repository.
 - The author of the pull request has permissions to push to branches inside the
   repository.
-
 - The author of the pull request is listed in the `OWNERS` file located in the main
   directory of the default branch on GitHub or your other service provider.
 (see below for the OWNERS file format).
 
-If the author of the pull request does not have the necessary permissions to run a
-PipelineRun, another user who does have the permissions can comment
-`/ok-to-test` on the pull request to trigger the PipelineRuns.
+If an unauthorized user attempts to trigger a PipelineRun through the creation
+of a Pull Request or by any other means, Pipelines-as-Code will block the
+execution and post a `'Pending'` status check. This check will inform the user
+that they lack the necessary permissions. Only authorized users can initiate the
+PipelineRun by commenting `/ok-to-test` on the pull request.
+
+GitHub bot users, identified through the GitHub API, are generally exempt from
+the `Pending` status check that would otherwise block a pull request. This
+means the status check is silently ignored for bots unless they have been
+explicitly authorized (using [OWNERS](#owners-file) file,
+[Policy]({{< relref "/docs/guide/policy" >}}) or other means).
 
 ## OWNERS file
 
 The `OWNERS` file follows a specific format similar to the Prow `OWNERS` file
 format (detailed at <https://www.kubernetes.dev/docs/guide/owners/>). We
 support a basic `OWNERS` configuration with `approvers` and `reviewers` lists,
-both of which have equal permissions for executing a `PipelineRun`.  
+both of which have equal permissions for executing a `PipelineRun`.
 
 If the `OWNERS` file uses `filters` instead of a simple configuration, we only
 consider the `.*` filter and extract the `approvers` and `reviewers` lists from
-it. Any other filters targeting specific files or directories are ignored.  
+it. Any other filters targeting specific files or directories are ignored.
 
 Additionally, `OWNERS_ALIASES` is supported and allows mapping alias names to
-lists of usernames.  
+lists of usernames.
 
 Including contributors in the `approvers` or `reviewers` lists within your
 `OWNERS` file grants them the ability to execute a `PipelineRun` via
-Pipelines-as-Code.  
+Pipelines-as-Code.
 
 For example, if your repository’s `main` or `master` branch contains the
-following `approvers` section:  
+following `approvers` section:
 
 ```yaml
 approvers:
   - approved
-```  
+```
 
 The user with the username `"approved"` will have the necessary
 permissions.

pkg/pipelineascode/match.go

@@ -136,10 +136,11 @@ is that what you want? make sure you use -n when generating the secret, eg: echo
 	// 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) {
 		status := provider.StatusOpts{
-			Status:     CompletedStatus,
-			Title:      "Permission denied",
-			Conclusion: failureConclusion,
-			DetailsURL: p.event.URL,
+			Status:       CompletedStatus,
+			Title:        "Permission denied",
+			Conclusion:   failureConclusion,
+			DetailsURL:   p.event.URL,
+			AccessDenied: true,
 		}
 		if allowed, err := p.checkAccessOrErrror(ctx, repo, status, "by GitOps comment on push commit"); !allowed {
 			return nil, err
@@ -151,10 +152,11 @@ is that what you want? make sure you use -n when generating the secret, eg: echo
 	// on comment we skip it for now, we are going to check later on
 	if p.event.TriggerTarget != triggertype.Push && p.event.EventType != opscomments.NoOpsCommentEventType.String() {
 		status := provider.StatusOpts{
-			Status:     queuedStatus,
-			Title:      "Pending approval, waiting for an /ok-to-test",
-			Conclusion: pendingConclusion,
-			DetailsURL: p.event.URL,
+			Status:       queuedStatus,
+			Title:        "Pending approval, waiting for an /ok-to-test",
+			Conclusion:   pendingConclusion,
+			DetailsURL:   p.event.URL,
+			AccessDenied: true,
 		}
 		if allowed, err := p.checkAccessOrErrror(ctx, repo, status, "via "+p.event.TriggerTarget.String()); !allowed {
 			return nil, err
@@ -266,10 +268,11 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	// we skipped previously so we can get the match from the event to the pipelineruns
 	if p.event.EventType == opscomments.NoOpsCommentEventType.String() || p.event.EventType == opscomments.OnCommentEventType.String() {
 		status := provider.StatusOpts{
-			Status:     queuedStatus,
-			Title:      "Pending approval, waiting for an /ok-to-test",
-			Conclusion: pendingConclusion,
-			DetailsURL: p.event.URL,
+			Status:       queuedStatus,
+			Title:        "Pending approval, waiting for an /ok-to-test",
+			Conclusion:   pendingConclusion,
+			DetailsURL:   p.event.URL,
+			AccessDenied: true,
 		}
 		if allowed, err := p.checkAccessOrErrror(ctx, repo, status, "by GitOps comment on push commit"); !allowed {
 			return nil, err
@@ -410,6 +413,7 @@ func (p *PacRun) checkAccessOrErrror(ctx context.Context, repo *v1alpha1.Reposit
 	}
 	p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryPermissionDenied", msg)
 	status.Text = msg
+
 	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)
 	}

pkg/provider/github/github.go

@@ -52,6 +52,7 @@ type Provider struct {
 	repo          *v1alpha1.Repository
 	eventEmitter  *events.EventEmitter
 	PaginedNumber int
+	userType      string // The type of user i.e bot or not
 	skippedRun
 }
 

pkg/provider/github/parse_payload.go

@@ -276,6 +276,7 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt
 		processedEvent.HeadBranch = processedEvent.BaseBranch // in push events Head Branch is the same as Basebranch
 		processedEvent.BaseURL = gitEvent.GetRepo().GetHTMLURL()
 		processedEvent.HeadURL = processedEvent.BaseURL // in push events Head URL is the same as BaseURL
+		v.userType = gitEvent.GetSender().GetType()
 	case *github.PullRequestEvent:
 		processedEvent.Repository = gitEvent.GetRepo().GetName()
 		if gitEvent.GetRepo() == nil {
@@ -291,6 +292,7 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt
 		processedEvent.HeadURL = gitEvent.GetPullRequest().Head.GetRepo().GetHTMLURL()
 		processedEvent.Sender = gitEvent.GetPullRequest().GetUser().GetLogin()
 		processedEvent.EventType = event.EventType
+		v.userType = gitEvent.GetPullRequest().GetUser().GetType()
 
 		if gitEvent.Action != nil && provider.Valid(*gitEvent.Action, pullRequestLabelEvent) {
 			processedEvent.EventType = string(triggertype.LabelUpdate)
@@ -343,6 +345,7 @@ func (v *Provider) handleReRequestEvent(ctx context.Context, event *github.Check
 		// we allow the rerequest user here, not the push user, i guess it's
 		// fine because you can't do a rereq without being a github owner?
 		runevent.Sender = event.GetSender().GetLogin()
+		v.userType = event.GetSender().GetType()
 		return runevent, nil
 	}
 	runevent.PullRequestNumber = event.GetCheckRun().GetCheckSuite().PullRequests[0].GetNumber()
@@ -373,6 +376,7 @@ func (v *Provider) handleCheckSuites(ctx context.Context, event *github.CheckSui
 		// we allow the rerequest user here, not the push user, i guess it's
 		// fine because you can't do a rereq without being a github owner?
 		runevent.Sender = event.GetSender().GetLogin()
+		v.userType = event.GetSender().GetType()
 		return runevent, nil
 		// return nil, fmt.Errorf("check suite event is not supported for push events")
 	}
@@ -401,6 +405,7 @@ func (v *Provider) handleIssueCommentEvent(ctx context.Context, event *github.Is
 	if !event.GetIssue().IsPullRequest() {
 		return info.NewEvent(), fmt.Errorf("issue comment is not coming from a pull_request")
 	}
+	v.userType = event.GetSender().GetType()
 	opscomments.SetEventTypeAndTargetPR(runevent, event.GetComment().GetBody())
 	// We are getting the full URL so we have to get the last part to get the PR number,
 	// we don't have to care about URL query string/hash and other stuff because
@@ -424,6 +429,7 @@ func (v *Provider) handleCommitCommentEvent(ctx context.Context, event *github.C
 	runevent.Organization = event.GetRepo().GetOwner().GetLogin()
 	runevent.Repository = event.GetRepo().GetName()
 	runevent.Sender = event.GetSender().GetLogin()
+	v.userType = event.GetSender().GetType()
 	runevent.URL = event.GetRepo().GetHTMLURL()
 	runevent.SHA = event.GetComment().GetCommitID()
 	runevent.HeadURL = runevent.URL

pkg/provider/github/status.go

@@ -20,6 +20,11 @@ import (
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 )
 
+const (
+	botType         = "Bot"
+	pendingApproval = "Pending approval, waiting for an /ok-to-test"
+)
+
 const taskStatusTemplate = `
 <table>
   <tr><th>Status</th><th>Duration</th><th>Name</th></tr>
@@ -35,8 +40,6 @@ const taskStatusTemplate = `
 {{- end }}
 </table>`
 
-const pendingApproval = "Pending approval, waiting for an /ok-to-test"
-
 func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status provider.StatusOpts) (*int64, error) {
 	opt := github.ListOptions{PerPage: v.PaginedNumber}
 	for {
@@ -350,6 +353,11 @@ func (v *Provider) CreateStatus(ctx context.Context, runevent *info.Event, statu
 		return fmt.Errorf("cannot set status on github no token or url set")
 	}
 
+	// If the request comes from a bot user, skip setting the status and just log the event silently
+	if statusOpts.AccessDenied && v.userType == botType {
+		return nil
+	}
+
 	switch statusOpts.Conclusion {
 	case "success":
 		statusOpts.Title = "Success"
@@ -386,7 +394,6 @@ func (v *Provider) CreateStatus(ctx context.Context, runevent *info.Event, statu
 		onPr = "/" + statusOpts.OriginalPipelineRunName
 	}
 	statusOpts.Summary = fmt.Sprintf("%s%s %s", v.pacInfo.ApplicationName, onPr, statusOpts.Summary)
-
 	// If we have an installationID which mean we have a github apps and we can use the checkRun API
 	if runevent.InstallationID > 0 {
 		return v.getOrUpdateCheckRunStatus(ctx, runevent, statusOpts)

pkg/provider/github/status_test.go

@@ -248,6 +248,8 @@ func TestGithubProviderCreateStatus(t *testing.T) {
 		titleSubstr        string
 		nilCompletedAtDate bool
 		githubApps         bool
+		accessDenied       bool
+		isBot              bool
 	}
 	tests := []struct {
 		name                 string
@@ -334,6 +336,36 @@ func TestGithubProviderCreateStatus(t *testing.T) {
 			want:    &github.CheckRun{ID: &resultid},
 			wantErr: false,
 		},
+		{
+			name: "failure from bot",
+			args: args{
+				runevent:     runEvent,
+				status:       "completed",
+				conclusion:   "failure",
+				text:         "Nay",
+				detailsURL:   "https://cireport.com",
+				titleSubstr:  "Failed",
+				githubApps:   true,
+				accessDenied: true,
+				isBot:        true,
+			},
+			wantErr: false,
+		},
+		{
+			name: "success from bot",
+			args: args{
+				runevent:    runEvent,
+				status:      "completed",
+				conclusion:  "failure",
+				text:        "Nay",
+				detailsURL:  "https://cireport.com",
+				titleSubstr: "Failed",
+				githubApps:  true,
+				isBot:       true,
+			},
+			wantErr: false,
+			want:    &github.CheckRun{ID: &resultid},
+		},
 		{
 			name: "skipped",
 			args: args{
@@ -378,14 +410,18 @@ func TestGithubProviderCreateStatus(t *testing.T) {
 			gcvs.Client = fakeclient
 			gcvs.Logger, _ = logger.GetLogger()
 			gcvs.Run = params.New()
+			if tt.args.isBot {
+				gcvs.userType = "Bot"
+			}
 
+			checkRunCreated := false
 			mux.HandleFunc("/repos/check/run/statuses/sha", func(_ http.ResponseWriter, _ *http.Request) {})
 			mux.HandleFunc(fmt.Sprintf("/repos/check/run/check-runs/%d", checkrunid), func(rw http.ResponseWriter, r *http.Request) {
 				bit, _ := io.ReadAll(r.Body)
 				checkRun := &github.CheckRun{}
 				err := json.Unmarshal(bit, checkRun)
 				assert.NilError(t, err)
-
+				checkRunCreated = true
 				if tt.args.nilCompletedAtDate {
 					// I guess that's the way you check for an undefined year,
 					// or maybe i don't understand fully how go works😅
@@ -402,6 +438,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
 				_, err = fmt.Fprintf(rw, `{"id": %d}`, resultid)
 				assert.NilError(t, err)
 			})
+
 			if tt.addExistingCheckruns {
 				tt.args.runevent.SHA = "sha"
 				mux.HandleFunc(fmt.Sprintf("/repos/%v/%v/commits/%v/check-runs", tt.args.runevent.Organization, tt.args.runevent.Repository, tt.args.runevent.SHA), func(w http.ResponseWriter, _ *http.Request) {
@@ -430,6 +467,7 @@ func TestGithubProviderCreateStatus(t *testing.T) {
 				Conclusion:      tt.args.conclusion,
 				Text:            tt.args.text,
 				DetailsURL:      tt.args.detailsURL,
+				AccessDenied:    tt.args.accessDenied,
 			}
 			if tt.pr != nil {
 				status.PipelineRun = tt.pr
@@ -469,6 +507,14 @@ func TestGithubProviderCreateStatus(t *testing.T) {
 				t.Errorf("GithubProvider.CreateStatus() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+			if tt.want == nil && checkRunCreated {
+				t.Errorf("Check run should have not be created for this test")
+				return
+			}
+			if tt.want != nil && !checkRunCreated {
+				t.Errorf("Check run should have been created for this test")
+				return
+			}
 		})
 	}
 }

pkg/provider/interface.go

@@ -24,6 +24,7 @@ type StatusOpts struct {
 	Summary                  string
 	Title                    string
 	InstanceCountForCheckRun int
+	AccessDenied             bool
 }
 
 type Interface interface {