From 6fd0e71e503a7450c1581366caf2c57df2cac4a7 Mon Sep 17 00:00:00 2001 From: Anas Khan Date: Wed, 15 Jul 2026 15:46:48 +0530 Subject: [PATCH] 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> --- pkg/addon/source.go | 2 +- pkg/utils/parse.go | 6 ++++++ pkg/utils/parse_test.go | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/addon/source.go b/pkg/addon/source.go index 1a5e0a3e5..d2cb68167 100644 --- a/pkg/addon/source.go +++ b/pkg/addon/source.go @@ -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") diff --git a/pkg/utils/parse.go b/pkg/utils/parse.go index 7cfe91855..fbaf38008 100644 --- a/pkg/utils/parse.go +++ b/pkg/utils/parse.go @@ -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// return TypeGitlab, &Content{ @@ -246,6 +249,9 @@ func ParseGitlab(addr, repo string) (string, *Content, error) { // https://example.gitlab.com///tree/ l := strings.Split(arr[1], "/") + if len(l) < 3 { + return "", nil, errors.New(errInvalidFormatMsg + addr) + } return TypeGitlab, &Content{ GitlabContent: GitlabContent{ diff --git a/pkg/utils/parse_test.go b/pkg/utils/parse_test.go index e9193a50c..4204dcd8d 100644 --- a/pkg/utils/parse_test.go +++ b/pkg/utils/parse_test.go @@ -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 {