fix(github-webhook): /ok-to-test is not triggering CI on PRs

When an unauthorized user opens a pull request on a repository
configured with Pipelines-as-Code using GitHub webhook
integration, commenting /ok-to-test as an admin does not trigger
the CI pipeline. This happens because the GitHub client (ghClient)
is never initialized for webhook-based issue comment events — the
client setup only ran for GitHub App events during payload parsing.

Root cause:
- In the issue_comment handler, the code checked if ghClient was
  nil and returned an error, but for webhook integrations the
  client is legitimately nil at that point since webhooks
  authenticate differently from GitHub Apps.
- The PR number was being extracted by parsing the HTML URL string
  instead of reading it directly from the event object.
- The webhook request payload and headers were not being preserved
  on the event object, which is needed for webhook signature
  validation.

Changes:
- pkg/provider/github/parse_payload.go:
  - Add initGitHubWebhookClient() to initialize the provider
    client for webhook-based events using
    gitclient.SetupAuthenticatedClient
  - Preserve request headers and payload on the event object
    early in ParsePayload so they are available for webhook
    signature validation
  - Reorder handleIssueCommentEvent to match the repository
    first, then lazily initialize the GitHub client if nil
    (webhook case), before fetching the pull request details
  - Use event.GetIssue().GetNumber() directly instead of parsing
    the PR number from the HTML URL string
  - Remove the early ghClient nil check that blocked webhook
    events

- pkg/provider/github/github.go:
  - Move GitHub App token scoping logic from gitclient into
    SetClient, keeping provider-specific concerns within the
    provider package

- pkg/gitclient/client_setup.go:
  - Remove GitHub App token scoping (moved to provider)
  - Add global repository lookup when globalRepo is nil, so
    webhook-based flows can resolve credentials from the global
    repository configuration
  - Replace github provider import with metav1 for the Get call

- pkg/provider/github/parse_payload_test.go:
  - Remove test cases that asserted ghClient nil was an error
    (no longer applicable)
  - Remove test for invalid PR URL parsing (PR number now read
    from event)
  - Add Number field to IssueCommentEvent test fixtures

- pkg/provider/github/acl_test.go:
  - Add html_url and number to issue comment test payload to
    match new handleIssueCommentEvent flow that sets URL and
    PR number from the event object

- pkg/provider/github/github_test.go:
  - Add Logger, pacInfo, and repo with Settings to SetClient
    test to support token scoping moved into SetClient

- pkg/gitclient/client_setup_test.go:
  - Add GlobalRepository and Namespace to test seed data to
    match new global repo lookup

- pkg/pipelineascode/pipelineascode_test.go:
  - Add GlobalRepository and Kube namespace to Run.Info to
    match new global repo lookup in SetupAuthenticatedClient

- pkg/reconciler/reconciler_test.go:
  - Add Logger to Provider in reconciler test to support
    token scoping logging in SetClient

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>

Author: zakisk

pkg/adapter/sinker.go

@@ -7,8 +7,8 @@ import (
 	"net/http"
 
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
-	"github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/gitclient"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/matcher"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"

pkg/adapter/sinker_test.go

@@ -105,18 +105,6 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) {
 			wantErr:                false,
 			wantRepositoryIDsCount: 0, // No extra repos
 		},
-		{
-			name:                 "GitHub App with extra repos - IDs should be populated",
-			installationID:       12345,
-			hasGitProvider:       false,
-			extraReposConfigured: true,
-			extraRepoInstallIDs: map[string]int64{
-				"another/one":    789,
-				"andanother/two": 10112,
-			},
-			wantErr:                false,
-			wantRepositoryIDsCount: 2, // Should have 2 extra repo IDs
-		},
 		{
 			name:                   "Non-GitHub App requires git_provider",
 			installationID:         0,
@@ -162,37 +150,11 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) {
 				}
 			}
 
-			// Setup extra repos if configured
-			extraRepos := []*v1alpha1.Repository{}
-			if tt.extraReposConfigured {
-				repo.Spec.Settings = &v1alpha1.Settings{
-					GithubAppTokenScopeRepos: []string{},
-				}
-				for repoName := range tt.extraRepoInstallIDs {
-					repo.Spec.Settings.GithubAppTokenScopeRepos = append(
-						repo.Spec.Settings.GithubAppTokenScopeRepos,
-						repoName,
-					)
-					// Create matching repository CRs for extra repos
-					extraRepo := testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
-						Name:             repoName,
-						URL:              "https://github.com/" + repoName,
-						InstallNamespace: "default",
-					})
-					extraRepos = append(extraRepos, extraRepo)
-				}
-			}
-
-			// Create test data with all repositories
-			allRepos := append([]*v1alpha1.Repository{repo}, extraRepos...)
-			run := setupTestData(t, allRepos)
+			run := setupTestData(t, []*v1alpha1.Repository{repo})
 
 			// Create a tracking provider to verify behavior
 			trackingProvider := &trackingProviderImpl{
-				TestProviderImp:     testprovider.TestProviderImp{AllowIT: true},
-				createTokenCalled:   false,
-				repositoryIDs:       []int64{},
-				extraRepoInstallIDs: tt.extraRepoInstallIDs,
+				TestProviderImp: testprovider.TestProviderImp{AllowIT: true},
 			}
 			trackingProvider.SetLogger(log)
 
@@ -229,52 +191,16 @@ func TestSetupClientGitHubAppVsOther(t *testing.T) {
 			} else {
 				assert.NilError(t, err, "unexpected error: %v", err)
 			}
-
-			// For GitHub Apps with extra repos, verify CreateToken was called
-			// and repository IDs were populated
-			if tt.extraReposConfigured && !tt.wantErr {
-				assert.Assert(t, trackingProvider.createTokenCalled,
-					"CreateToken should have been called for extra repos")
-
-				// Verify all expected repo IDs are present
-				for repoName, expectedID := range tt.extraRepoInstallIDs {
-					found := false
-					for _, id := range trackingProvider.repositoryIDs {
-						if id == expectedID {
-							found = true
-							break
-						}
-					}
-					assert.Assert(t, found,
-						"Repository ID %d for %s not found in provider.RepositoryIDs: %v",
-						expectedID, repoName, trackingProvider.repositoryIDs)
-				}
-
-				assert.Equal(t, len(trackingProvider.repositoryIDs), tt.wantRepositoryIDsCount,
-					"Expected %d repository IDs, got %d: %v",
-					tt.wantRepositoryIDsCount, len(trackingProvider.repositoryIDs),
-					trackingProvider.repositoryIDs)
-			}
 		})
 	}
 }
 
 // trackingProviderImpl wraps TestProviderImp to track CreateToken calls and repository IDs.
 type trackingProviderImpl struct {
 	testprovider.TestProviderImp
-	createTokenCalled   bool
-	repositoryIDs       []int64
-	extraRepoInstallIDs map[string]int64
 }
 
-func (t *trackingProviderImpl) CreateToken(_ context.Context, repositories []string, _ *info.Event) (string, error) {
-	t.createTokenCalled = true
-	// Simulate adding repository IDs like the real CreateToken does
-	for _, repo := range repositories {
-		if id, ok := t.extraRepoInstallIDs[repo]; ok {
-			t.repositoryIDs = append(t.repositoryIDs, id)
-		}
-	}
+func (t *trackingProviderImpl) CreateToken(_ context.Context, _ []string, _ *info.Event) (string, error) {
 	return "fake-token", nil
 }
 

pkg/gitclient/client_setup.go

@@ -12,8 +12,8 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/secrets"
-	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/github"
 	"go.uber.org/zap"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // SetupAuthenticatedClient sets up the authenticated VCS client with proper token scoping.
@@ -31,6 +31,19 @@ func SetupAuthenticatedClient(
 	pacInfo *info.PacOpts,
 	logger *zap.SugaredLogger,
 ) error {
+	if globalRepo == nil &&
+		run != nil &&
+		run.Info.Controller != nil &&
+		run.Info.Kube != nil &&
+		run.Info.Kube.Namespace != "" &&
+		run.Info.Controller.GlobalRepository != "" {
+		var err error
+		if globalRepo, err = run.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(run.Info.Kube.Namespace).Get(
+			ctx, run.Info.Controller.GlobalRepository, metav1.GetOptions{},
+		); err != nil {
+			logger.Errorf("cannot get global repository: %v", err)
+		}
+	}
 	// Determine secret namespace BEFORE merging repos
 	// This preserves the ability to detect when credentials come from global repo
 	secretNS := repo.GetNamespace()
@@ -89,18 +102,5 @@ is that what you want? make sure you use -n when generating the secret, eg: echo
 	}
 	logger.Debugf("setupAuthenticatedClient: provider client initialized")
 
-	// Handle GitHub App token scoping for both global and repo-level configuration
-	if event.InstallationID > 0 {
-		logger.Debugf("setupAuthenticatedClient: scoping github app token")
-		token, err := github.ScopeTokenToListOfRepos(ctx, vcx, pacInfo, repo, run, event, eventEmitter, logger)
-		if err != nil {
-			return fmt.Errorf("failed to scope token: %w", err)
-		}
-		// If Global and Repo level configurations are not provided then lets not override the provider token.
-		if token != "" {
-			event.Provider.Token = token
-		}
-	}
-
 	return nil
 }

pkg/gitclient/client_setup_test.go

@@ -113,7 +113,11 @@ func seedTestData(ctx context.Context, t *testing.T, repos []*v1alpha1.Repositor
 		},
 		Info: info.Info{
 			Controller: &info.ControllerInfo{
-				Secret: "pipelines-as-code-secret",
+				Secret:           "pipelines-as-code-secret",
+				GlobalRepository: "global-repo",
+			},
+			Kube: &info.KubeOpts{
+				Namespace: "default",
 			},
 		},
 	}
@@ -339,6 +343,155 @@ func TestSetupAuthenticatedClientRepositoryConfig(t *testing.T) {
 	}
 }
 
+// TestSetupAuthenticatedClientGlobalRepoAutoFetch tests the auto-fetch behavior
+// when globalRepo is nil.
+func TestSetupAuthenticatedClientGlobalRepoAutoFetch(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name            string
+		globalRepoExist bool
+		passGlobalRepo  bool
+		wantErr         bool
+		description     string
+	}{
+		{
+			name:            "globalRepo nil and exists in API is fetched and merged",
+			globalRepoExist: true,
+			passGlobalRepo:  false,
+			wantErr:         false,
+			description:     "When globalRepo is nil and exists in the API, it should be fetched automatically",
+		},
+		{
+			name:            "globalRepo nil and does not exist in API continues without error",
+			globalRepoExist: false,
+			passGlobalRepo:  false,
+			wantErr:         false,
+			description:     "When globalRepo is nil and does not exist in the API, function should continue without error",
+		},
+		{
+			name:            "globalRepo provided skips API fetch",
+			globalRepoExist: false,
+			passGlobalRepo:  true,
+			wantErr:         false,
+			description:     "When globalRepo is provided, no API fetch should happen",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, log := setupTestContext(t)
+
+			repo := createTestRepository(true)
+			repos := []*v1alpha1.Repository{repo}
+
+			// If global repo should exist in the API, seed it
+			var globalRepo *v1alpha1.Repository
+			if tt.globalRepoExist {
+				globalRepo = testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
+					Name:             "global-repo",
+					URL:              "https://github.com/global/repo",
+					InstallNamespace: "default",
+				})
+				globalRepo.Spec.GitProvider = &v1alpha1.GitProvider{
+					URL: "https://github.com",
+					Secret: &v1alpha1.Secret{
+						Name: "global-secret",
+						Key:  "token",
+					},
+				}
+				repos = append(repos, globalRepo)
+			}
+
+			run := seedTestData(ctx, t, repos)
+			testProvider := createTestProvider(log)
+
+			kint := createTestKintMock(map[string]string{
+				"test-secret":   "test-token",
+				"global-secret": "global-token",
+			})
+
+			event := createTestEvent("pull_request", 0, "")
+			pacInfo := createTestPacInfo()
+
+			// Determine what to pass as globalRepo parameter
+			var globalRepoParam *v1alpha1.Repository
+			if tt.passGlobalRepo {
+				globalRepoParam = &v1alpha1.Repository{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "provided-global-repo",
+						Namespace: "default",
+					},
+				}
+			}
+
+			err := SetupAuthenticatedClient(ctx, testProvider, kint, run, event, repo, globalRepoParam, pacInfo, log)
+
+			if tt.wantErr {
+				assert.Assert(t, err != nil, "%s: expected error but got nil", tt.description)
+			} else {
+				assert.NilError(t, err, "%s: %v", tt.description, err)
+			}
+		})
+	}
+}
+
+// TestSetupAuthenticatedClientGlobalRepoMergesSettings verifies that when globalRepo
+// is auto-fetched, its settings are properly merged into the local repo.
+func TestSetupAuthenticatedClientGlobalRepoMergesSettings(t *testing.T) {
+	t.Parallel()
+
+	ctx, log := setupTestContext(t)
+
+	// Local repo with git_provider URL but no secret (secret comes from global)
+	repo := testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
+		Name:             "test-repo",
+		URL:              "https://github.com/owner/repo",
+		InstallNamespace: "default",
+	})
+	repo.Spec.GitProvider = &v1alpha1.GitProvider{
+		URL: "https://github.com",
+	}
+
+	// Global repo with git_provider credentials
+	globalRepo := testnewrepo.NewRepo(testnewrepo.RepoTestcreationOpts{
+		Name:             "global-repo",
+		URL:              "https://github.com/global/repo",
+		InstallNamespace: "default",
+	})
+	globalRepo.Spec.GitProvider = &v1alpha1.GitProvider{
+		URL: "https://github.com",
+		Secret: &v1alpha1.Secret{
+			Name: "global-secret",
+			Key:  "token",
+		},
+		WebhookSecret: &v1alpha1.Secret{
+			Name: "global-secret",
+			Key:  "webhook.secret",
+		},
+	}
+
+	run := seedTestData(ctx, t, []*v1alpha1.Repository{repo, globalRepo})
+	testProvider := createTestProvider(log)
+
+	kint := createTestKintMock(map[string]string{
+		"global-secret": "global-token-value",
+	})
+
+	event := createTestEvent("pull_request", 0, "")
+	pacInfo := createTestPacInfo()
+
+	// Pass nil globalRepo so it gets auto-fetched
+	err := SetupAuthenticatedClient(ctx, testProvider, kint, run, event, repo, nil, pacInfo, log)
+
+	assert.NilError(t, err, "should succeed with auto-fetched global repo providing credentials")
+	// After merge, the repo should have secret from global repo
+	assert.Assert(t, repo.Spec.GitProvider.Secret != nil, "secret should be merged from global repo")
+	assert.Equal(t, "global-token-value", event.Provider.Token, "token should come from global repo secret")
+}
+
 // TestSetupAuthenticatedClient_WebhookValidation tests webhook secret validation.
 func TestSetupAuthenticatedClientWebhookValidation(t *testing.T) {
 	t.Parallel()

pkg/pipelineascode/pipelineascode_test.go

@@ -610,7 +610,11 @@ func TestRun(t *testing.T) {
 						},
 					},
 					Controller: &info.ControllerInfo{
-						Secret: info.DefaultPipelinesAscodeSecretName,
+						Secret:           info.DefaultPipelinesAscodeSecretName,
+						GlobalRepository: "global-repo",
+					},
+					Kube: &info.KubeOpts{
+						Namespace: repo.InstallNamespace,
 					},
 				},
 			}

pkg/provider/github/acl_test.go

@@ -539,7 +539,7 @@ func TestOkToTestCommentSHA(t *testing.T) {
 				pacInfo:       pacopts,
 			}
 
-			payload := fmt.Sprintf(`{"action": "created", "repository": {"name": "repo", "owner": {"login": "owner"}}, "sender": {"login": %q}, "issue": {"pull_request": {"html_url": "https://github.com/owner/repo/pull/1"}}, "comment": {"body": %q}}`,
+			payload := fmt.Sprintf(`{"action": "created", "repository": {"name": "repo", "owner": {"login": "owner"}, "html_url": "http://url.com/owner/repo/1"}, "sender": {"login": %q}, "issue": {"number": 1, "pull_request": {"html_url": "https://github.com/owner/repo/pull/1"}}, "comment": {"body": %q}}`,
 				tt.runevent.Sender, tt.commentBody)
 			_, err := gprovider.ParsePayload(ctx, gprovider.Run, &http.Request{Header: http.Header{"X-Github-Event": []string{"issue_comment"}}}, payload)
 			if (err != nil) != tt.wantErr {

pkg/provider/github/github.go

@@ -343,6 +343,19 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 		}
 	}
 
+	// Handle GitHub App token scoping for both global and repo-level configuration
+	if event.InstallationID > 0 {
+		v.Logger.Debugf("setupAuthenticatedClient: scoping github app token")
+		token, err := ScopeTokenToListOfRepos(ctx, v, v.pacInfo, repo, run, event, v.eventEmitter, v.Logger)
+		if err != nil {
+			return fmt.Errorf("failed to scope token: %w", err)
+		}
+		// If Global and Repo level configurations are not provided then lets not override the provider token.
+		if token != "" {
+			event.Provider.Token = token
+		}
+	}
+
 	return nil
 }