Skip to content

Commit 6fd0e71

Browse files
authored
Fix: prevent panics in ParseGitlab on malformed addon registry URLs (kubevela#7206)
ParseGitlab indexed the results of splitting the address without bounds checks, so a malformed (but repo-name-matching) GitLab registry URL crashed the controller/CLI instead of returning an error. Three inputs panic today: - https://gitlab.com/catalog -> slice bounds out of range [:-1] - https://gitlab.com/kubevela/catalog/tree -> index out of range [2] - https://catalog.gitlab.com/kubevela/foo -> index out of range [1] Add length/empty guards that return the existing invalid-format error (errInvalidFormatMsg, consistent with Parse) for the empty owner slice, the short host split, and the missing tree branch segment. Behaviour for all currently-valid inputs is unchanged. Also fix the caller NewAsyncReader, which dereferenced the returned content before checking the error: on any ParseGitlab error content is nil, so the assignment nil-panicked before the error could be returned. Move the error check above the dereference. Extend TestParseGitlab with the three malformed cases (wantErr). Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
1 parent 74c5724 commit 6fd0e71

3 files changed

Lines changed: 25 additions & 1 deletion

File tree

pkg/addon/source.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,10 @@ func NewAsyncReader(baseURL, bucket, repo, subPath, token string, rdType ReaderT
317317
return nil, errors.New("addon registry invalid")
318318
}
319319
_, content, err := utils.ParseGitlab(u.String(), repo)
320-
content.GitlabContent.Path = subPath
321320
if err != nil {
322321
return nil, err
323322
}
323+
content.GitlabContent.Path = subPath
324324
gitlabHelper, err := createGitlabHelper(content, token)
325325
if err != nil {
326326
return nil, errors.New("addon registry connect fail")

pkg/utils/parse.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@ func ParseGitlab(addr, repo string) (string, *Content, error) {
232232

233233
arr := strings.Split(addr, repo)
234234
owner := strings.Split(arr[0], URL.Host+"/")
235+
if len(owner) < 2 || len(owner[1]) == 0 {
236+
return "", nil, errors.New(errInvalidFormatMsg + addr)
237+
}
235238
if !strings.Contains(arr[1], "/") {
236239
// https://example.gitlab.com/<owner>/<repo>
237240
return TypeGitlab, &Content{
@@ -246,6 +249,9 @@ func ParseGitlab(addr, repo string) (string, *Content, error) {
246249

247250
// https://example.gitlab.com/<owner>/<repo>/tree/<branch>
248251
l := strings.Split(arr[1], "/")
252+
if len(l) < 3 {
253+
return "", nil, errors.New(errInvalidFormatMsg + addr)
254+
}
249255

250256
return TypeGitlab, &Content{
251257
GitlabContent: GitlabContent{

pkg/utils/parse_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,24 @@ func TestParseGitlab(t *testing.T) {
174174
repo: "repo",
175175
wantErr: true,
176176
},
177+
{
178+
name: "invalid gitlab url repo at path root without owner",
179+
addr: "https://gitlab.com/catalog",
180+
repo: "catalog",
181+
wantErr: true,
182+
},
183+
{
184+
name: "invalid gitlab url tree branch missing",
185+
addr: "https://gitlab.com/kubevela/catalog/tree",
186+
repo: "catalog",
187+
wantErr: true,
188+
},
189+
{
190+
name: "invalid gitlab url repo only in host",
191+
addr: "https://catalog.gitlab.com/kubevela/foo",
192+
repo: "catalog",
193+
wantErr: true,
194+
},
177195
}
178196

179197
for _, tc := range testCases {

0 commit comments

Comments
 (0)