Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions pkg/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
80 changes: 80 additions & 0 deletions pkg/webhook/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment on lines +972 to +981
})
}
}