Fix: prevent panics in ParseGitlab on malformed addon registry URLs (#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>
This commit is contained in:
Anas Khan
2026-07-15 15:46:48 +05:30
committed by GitHub
parent 74c5724554
commit 6fd0e71e50
3 changed files with 25 additions and 1 deletions

View File

@@ -317,10 +317,10 @@ func NewAsyncReader(baseURL, bucket, repo, subPath, token string, rdType ReaderT
return nil, errors.New("addon registry invalid")
}
_, content, err := utils.ParseGitlab(u.String(), repo)
content.GitlabContent.Path = subPath
if err != nil {
return nil, err
}
content.GitlabContent.Path = subPath
gitlabHelper, err := createGitlabHelper(content, token)
if err != nil {
return nil, errors.New("addon registry connect fail")

View File

@@ -232,6 +232,9 @@ func ParseGitlab(addr, repo string) (string, *Content, error) {
arr := strings.Split(addr, repo)
owner := strings.Split(arr[0], URL.Host+"/")
if len(owner) < 2 || len(owner[1]) == 0 {
return "", nil, errors.New(errInvalidFormatMsg + addr)
}
if !strings.Contains(arr[1], "/") {
// https://example.gitlab.com/<owner>/<repo>
return TypeGitlab, &Content{
@@ -246,6 +249,9 @@ func ParseGitlab(addr, repo string) (string, *Content, error) {
// https://example.gitlab.com/<owner>/<repo>/tree/<branch>
l := strings.Split(arr[1], "/")
if len(l) < 3 {
return "", nil, errors.New(errInvalidFormatMsg + addr)
}
return TypeGitlab, &Content{
GitlabContent: GitlabContent{

View File

@@ -174,6 +174,24 @@ func TestParseGitlab(t *testing.T) {
repo: "repo",
wantErr: true,
},
{
name: "invalid gitlab url repo at path root without owner",
addr: "https://gitlab.com/catalog",
repo: "catalog",
wantErr: true,
},
{
name: "invalid gitlab url tree branch missing",
addr: "https://gitlab.com/kubevela/catalog/tree",
repo: "catalog",
wantErr: true,
},
{
name: "invalid gitlab url repo only in host",
addr: "https://catalog.gitlab.com/kubevela/foo",
repo: "catalog",
wantErr: true,
},
}
for _, tc := range testCases {