Skip to content

Commit bb1f6d9

Browse files
chuongld20claude
andcommitted
fix: harden registry — empty query guard, URL path traversal prevention
Review findings: - MatchesQuery("") and Search("") no longer match everything - Pull rejects URLs with "..", "//", or ":" to prevent SSRF/path traversal - Added tests for empty query and unsafe URL edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4a3735e commit bb1f6d9

4 files changed

Lines changed: 37 additions & 4 deletions

File tree

internal/registry/registry.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ func (r *RemoteRegistry) Search(query string) ([]IndexEntry, error) {
8383
return nil, err
8484
}
8585

86+
if query == "" {
87+
return nil, nil
88+
}
89+
8690
q := strings.ToLower(query)
8791
var matches []IndexEntry
8892
for _, e := range entries {
@@ -113,10 +117,12 @@ func (r *RemoteRegistry) Pull(name string, localReg *template.LocalRegistry) (*t
113117
return nil, fmt.Errorf("template %q not found in registry", name)
114118
}
115119

116-
// Only allow relative URLs to prevent SSRF via malicious index entries.
120+
// Only allow safe relative URLs to prevent SSRF via malicious index entries.
117121
templateURL := entry.URL
118-
if strings.HasPrefix(templateURL, "http://") || strings.HasPrefix(templateURL, "https://") {
119-
return nil, fmt.Errorf("template %q has absolute URL which is not allowed", name)
122+
if strings.HasPrefix(templateURL, "http://") || strings.HasPrefix(templateURL, "https://") ||
123+
strings.HasPrefix(templateURL, "//") || strings.Contains(templateURL, "..") ||
124+
strings.Contains(templateURL, ":") {
125+
return nil, fmt.Errorf("template %q has unsafe URL %q", name, templateURL)
120126
}
121127
templateURL = r.baseURL + "/" + strings.TrimLeft(templateURL, "/")
122128

internal/registry/registry_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ func TestSearch(t *testing.T) {
9696
{"application", 3},
9797
{"react", 1},
9898
{"nonexistent", 0},
99+
{"", 0}, // empty query returns nothing
99100
}
100101

101102
for _, tt := range tests {
@@ -181,6 +182,28 @@ func TestPush(t *testing.T) {
181182
}
182183
}
183184

185+
func TestPullUnsafeURL(t *testing.T) {
186+
unsafeIndex := `templates:
187+
- name: evil
188+
version: "1.0.0"
189+
description: Malicious template
190+
url: "../../etc/passwd"
191+
`
192+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
193+
w.Write([]byte(unsafeIndex))
194+
}))
195+
defer srv.Close()
196+
197+
dir := t.TempDir()
198+
localReg := template.NewLocalRegistry(dir, nil)
199+
reg := NewRemoteRegistry(srv.URL)
200+
201+
_, err := reg.Pull("evil", localReg)
202+
if err == nil {
203+
t.Fatal("expected error for unsafe URL with path traversal")
204+
}
205+
}
206+
184207
func TestPushNotFound(t *testing.T) {
185208
dir := t.TempDir()
186209
localReg := template.NewLocalRegistry(dir, nil)

internal/template/template.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@ type Template struct {
2727
}
2828

2929
// MatchesQuery returns true if the template's name or description
30-
// contains the query string (case-insensitive).
30+
// contains the query string (case-insensitive). Returns false for empty queries.
3131
func (t *Template) MatchesQuery(query string) bool {
32+
if query == "" {
33+
return false
34+
}
3235
q := strings.ToLower(query)
3336
return strings.Contains(strings.ToLower(t.Name), q) ||
3437
strings.Contains(strings.ToLower(t.Description), q)

internal/template/template_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ func TestMatchesQuery(t *testing.T) {
9292
{"mysql", true},
9393
{"nonexistent", false},
9494
{"lara", true},
95+
{"", false},
9596
}
9697

9798
for _, tt := range tests {

0 commit comments

Comments
 (0)