Skip to content

feat:wildcard repository url #2733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
johnvox wants to merge 1 commit into from
+182 −2
Closed

feat:wildcard repository url #2733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 22 additions & 2 deletions pkg/matcher/repo_runinfo_matcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@ func MatchEventURLRepo(ctx context.Context, cs *params.Run, event *info.Event, n
}
for _, repo := range repositories.Items {
repo.Spec.URL = strings.TrimSuffix(repo.Spec.URL, "/")
if repo.Spec.URL == event.URL {
match, err := matchRepo(event.URL, repo.Spec.URL)
if err != nil {
return nil, err
}
if match {
return &repo, nil
}
}

return nil, nil
}

Expand Down Expand Up @@ -89,3 +92,20 @@ func matchTarget(branch, target string) (bool, error) {

return g.Match(branch), nil
}

// matchTarget checks if a branch matches a target pattern using glob matching.
// Supports both exact string matching and glob patterns.
func matchRepo(repo, target string) (bool, error) {
if target == repo {
return true, nil
}
// Check unix glob match
globPattern, err := glob.Compile(target)
if err != nil {
return false, err
}
if globPattern.Match(repo) {
return true, nil
}
return false, nil
}
Comment on lines +96 to +111
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The implementation of matchRepo has several critical issues:

  1. Incorrect Matching Logic: regexp.MatchString performs substring matching by default. For repository matching, you should use full-string matching.
  2. Redundancy and Security: The glob library is sufficient and safer. Regular expressions are prone to ReDoS. Per project rules, if regular expressions are used, they must be sanitized at a centralized point, but removing regex entirely in favor of glob is preferred here.
  3. Performance: Compiling patterns inside a loop for every event is inefficient.

I suggest removing the regexp support and sticking to glob for wildcard matching.

// matchRepo checks if a repository URL matches a target pattern using glob matching.
// Supports both exact string matching and glob patterns.
func matchRepo(repo, target string) (bool, error) {
	if target == repo {
		return true, nil
	}
	// Check unix glob match
	globPattern, err := glob.Compile(target)
	if err != nil {
		return false, fmt.Errorf("invalid glob pattern %q: %w", target, err)
	}
	return globPattern.Match(repo), nil
}
References
  1. Sanitize user-provided input for components like regular expressions at a centralized point to avoid redundant sanitization logic.

97 changes: 97 additions & 0 deletions pkg/matcher/repo_runinfo_matcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,103 @@ func TestIncomingWebhookRule(t *testing.T) {
}
}

func TestMatchRepo(t *testing.T) {
tests := []struct {
name string
eventURL string
repoURL string
wantMatch bool
wantError bool
errorCheck string
}{
// Exact matching
{
name: "exact match",
eventURL: "https://github.com/tektoncd/pipelines-as-code",
repoURL: "https://github.com/tektoncd/pipelines-as-code",
wantMatch: true,
},
{
name: "no substring match",
eventURL: "https://github.com/forked/pipelines-as-code",
repoURL: "https://github.com/tektoncd/pipelines-as-code",
wantMatch: false,
},
// Wildcard * - matches zero or more characters
{
name: "glob * - prefix pattern",
eventURL: "https://github.com/tektoncd/pipelines-as-code",
repoURL: "https://github.com/tektoncd/*",
wantMatch: true,
},
{
name: "glob * - must match from start",
eventURL: "https://gitlab.com/tektoncd/pipelines-as-code",
repoURL: "https://github.com/tektoncd/*",
wantMatch: false,
},
{
name: "glob * - substring match with wildcards",
eventURL: "https://github.com/tektoncd/pipelines-as-code",
repoURL: "*/tektoncd/*",
wantMatch: true,
},
{
name: "glob * - catch-all",
eventURL: "https://github.com/tektoncd/pipelines-as-code",
repoURL: "*",
wantMatch: true,
},
// Wildcard ? - matches exactly one character
{
name: "glob ? - single char match",
eventURL: "https://github.com/tektoncd/pipelines-as-code-v2",
repoURL: "https://github.com/tektoncd/pipelines-as-code-v?",
wantMatch: true,
},
// Character classes [...]
{
name: "glob [range] - character class",
eventURL: "https://gitlab.com/tektoncd/pipelines-as-code",
repoURL: "https://[a-z]*.com/tektoncd/pipelines-as-code",
wantMatch: true,
},
// Error handling
{
name: "invalid glob - unclosed bracket",
eventURL: "https://github.com/tektoncd/pipelines-as-code",
repoURL: "https://[github.com/tektoncd/pipelines-as-code",
wantMatch: false,
wantError: true,
errorCheck: "unexpected end of input",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotMatch, err := matchRepo(tt.eventURL, tt.repoURL)

if tt.wantError {
if err == nil {
t.Errorf("matchRepo() expected error but got nil")
return
}
if tt.errorCheck != "" {
assert.ErrorContains(t, err, tt.errorCheck)
}
} else {
if err != nil {
t.Errorf("matchRepo() unexpected error = %v", err)
return
}
if gotMatch != tt.wantMatch {
t.Errorf("matchRepo() gotMatch = %v, want %v for eventURL=%q, repoURL=%q", gotMatch, tt.wantMatch, tt.eventURL, tt.repoURL)
}
}
})
}
}

func TestMatchTarget(t *testing.T) {
tests := []struct {
name string
Expand Down
63 changes: 63 additions & 0 deletions pkg/webhook/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,66 @@ func TestReconcilerAdmit(t *testing.T) {
})
}
}

func TestMatchRepo(t *testing.T) {
tests := []struct {
name string
url string
wantError bool
errorCheck string
}{
// Exact matching
{
name: "std https",
url: "https://github.com/tektoncd/pipelines-as-code",
wantError: false,
},
{
name: "std http",
url: "http://github.com/tektoncd/pipelines-as-code",
wantError: false,
},
// Wildcard * - matches zero or more characters
{
name: "glob * - prefix pattern",
url: "https://github.com/tektoncd/*",
wantError: false,
},
{
name: "glob * - domain",
url: "https://*/tektoncd/pipelines-as-code",
wantError: false,
},
{
name: "std not github ",
url: "https://gitlab.com/tektoncd/pipelines-as-code",
wantError: false,
},
{
name: "std not github deep path",
url: "https://gitlab.com/tektoncd/group/pipelines-as-code",
wantError: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateRepositoryURL(tt.url, "")

if tt.wantError {
if err == nil {
t.Errorf("validateRepositoryURL() expected error but got nil")
return
}
if tt.errorCheck != "" {
assert.ErrorContains(t, err, tt.errorCheck)
}
} else {
if err != nil {
t.Errorf("validateRepositoryURL() unexpected error = %v", err)
return
}
}
})
}
}
Loading