diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index c131330316..add0aaef21 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -105,10 +105,8 @@ func (w *Webhook) ServeHTTP(rw http.ResponseWriter, r *http.Request) { w.logAndReturn(rw, err) return } - - path := strings.Replace(u.EscapedPath()[1:], "/_git/", "(/_git)?/", 1) - - regexpStr := `(?i)(http://|https://|\w+@|ssh://(\w+@)?|git@(ssh\.)?)` + u.Hostname() + + path := strings.Replace(regexp.QuoteMeta(u.EscapedPath()[1:]), `/_git/`, `(/_git)?/`, 1) + regexpStr := `(?i)(http://|https://|\w+@|ssh://(\w+@)?|git@(ssh\.)?)` + regexp.QuoteMeta(u.Hostname()) + "(:[0-9]+|)[:/](v\\d/)?" + path + "(\\.git)?$" repoRegexp, err := regexp.Compile(regexpStr) if err != nil { diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index c63a02ea66..fef0cda6da 100644 --- a/pkg/webhook/webhook_test.go +++ b/pkg/webhook/webhook_test.go @@ -902,3 +902,83 @@ func TestErrorReadingRequest(t *testing.T) { t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusInternalServerError) } } + +// TestGitHubWildcardURLDoesNotMatchUnintendedRepo verifies that regex metacharacters +// in a webhook payload's repo URL (e.g. a hostname or path of ".*") are treated as +// literals and do not accidentally match GitRepos whose URLs are unrelated to the +// webhook source. +func TestGitHubWildcardURLDoesNotMatchUnintendedRepo(t *testing.T) { + tests := []struct { + name string + gitRepoURL string // the GitRepo's configured repo + webhookRepoURL string // the repo URL embedded in the webhook payload + }{ + { + name: "wildcard hostname does not match unrelated gitrepo", + gitRepoURL: "https://github.com/example/repo", + webhookRepoURL: "https://.*/example/repo", + }, + { + name: "wildcard path does not match unrelated gitrepo", + gitRepoURL: "https://github.com/example/repo", + webhookRepoURL: "https://github.com/.*/.*", + }, + { + name: "wildcard hostname and path do not match unrelated gitrepo", + gitRepoURL: "https://github.com/example/repo", + webhookRepoURL: "https://.*/.*", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sch := runtime.NewScheme() + if err := v1alpha1.AddToScheme(sch); err != nil { + t.Fatalf("unable to add to scheme: %v", err) + } + utilruntime.Must(corev1.AddToScheme(sch)) + + gitRepo := &v1alpha1.GitRepo{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Spec: v1alpha1.GitRepoSpec{ + Repo: tt.gitRepoURL, + Branch: "main", + }, + } + fakeClient := cfake.NewClientBuilder(). + WithScheme(sch). + WithRuntimeObjects(gitRepo). + WithStatusSubresource(gitRepo). + Build() + + w := &Webhook{client: fakeClient, namespace: "default"} + + commit := "af69d162de5a276abc86e0686b2b44033cd3f442" + jsonBody := fmt.Appendf(nil, `{ + "ref": "refs/heads/main", + "after": "%s", + "repository": {"html_url": "%s"} + }`, commit, tt.webhookRepoURL) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/", bytes.NewReader(jsonBody)) + if err != nil { + t.Fatalf("failed to create request: %v", err) + } + req.Header.Set("X-GitHub-Event", "push") + + rr := httptest.NewRecorder() + w.ServeHTTP(rr, req) + + updated := &v1alpha1.GitRepo{} + if err := fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}, updated); err != nil { + t.Fatalf("failed to get gitrepo: %v", err) + } + if updated.Status.WebhookCommit != "" { + t.Errorf("expected WebhookCommit to remain empty (no match), but got %q", updated.Status.WebhookCommit) + } + }) + } +}