From 1f31e442a8fffff6e83f52a880a3c3de75a00e92 Mon Sep 17 00:00:00 2001 From: Dipak Dhangar Date: Wed, 30 Sep 2026 21:28:12 +0530 Subject: [PATCH 1/3] fix(import-url): validate artifact URL scheme Signed-off-by: Dipak Dhangar --- cmd/importURL.go | 18 +++- cmd/importURL_test.go | 193 +++++++++++++++++++++++++----------------- 2 files changed, 131 insertions(+), 80 deletions(-) diff --git a/cmd/importURL.go b/cmd/importURL.go index a62eb03c..db54c590 100644 --- a/cmd/importURL.go +++ b/cmd/importURL.go @@ -48,7 +48,11 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm mainArtifact := true secret := "" - f, mainArtifact, secret = parseImportURLArg(f) + var err error + f, mainArtifact, secret, err = parseImportURLArg(f) + if err != nil { + return err + } // Try downloading the artifcat msg, err := mc.DownloadArtifact(f, mainArtifact, secret) @@ -64,7 +68,15 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm return importURLCmd } -func parseImportURLArg(f string) (string, bool, string) { +func parseImportURLArg(f string) (string, bool, string, error) { + if !strings.HasPrefix(f, "https://") && !strings.HasPrefix(f, "http://") { + return "", false, "", errors.Wrapf( + errors.KindUsage, + "invalid artifact URL '%s': must start with http:// or https://", + f, + ) + } + mainArtifact := true secret := "" @@ -84,5 +96,5 @@ func parseImportURLArg(f string) (string, bool, string) { } } } - return f, mainArtifact, secret + return f, mainArtifact, secret, nil } diff --git a/cmd/importURL_test.go b/cmd/importURL_test.go index d733c9e8..5fc8f444 100644 --- a/cmd/importURL_test.go +++ b/cmd/importURL_test.go @@ -19,148 +19,187 @@ package cmd import ( "testing" + "github.com/microcks/microcks-cli/pkg/errors" "github.com/stretchr/testify/assert" ) func TestParseImportURLArg(t *testing.T) { tests := []struct { - name string - input string - expectedURL string + name string + input string + expectedURL string expectedMainArtifact bool - expectedSecret string + expectedSecret string + wantErr bool + expectedErrMsg string }{ { - name: "standard URL without suffixes", - input: "https://example.com/openapi.yaml", - expectedURL: "https://example.com/openapi.yaml", + name: "standard URL without suffixes", + input: "https://example.com/openapi.yaml", + expectedURL: "https://example.com/openapi.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "standard URL with mainArtifact true", - input: "https://example.com/spec1.yaml:true", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact true", + input: "https://example.com/spec1.yaml:true", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "standard URL with mainArtifact false", - input: "https://example.com/spec1.yaml:false", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact false", + input: "https://example.com/spec1.yaml:false", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: false, - expectedSecret: "", + expectedSecret: "", }, { - name: "standard URL with mainArtifact and secret", - input: "https://example.com/spec1.yaml:true:my-secret", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact and secret", + input: "https://example.com/spec1.yaml:true:my-secret", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: true, - expectedSecret: "my-secret", + expectedSecret: "my-secret", }, { - name: "standard URL with mainArtifact false and secret", - input: "https://example.com/spec1.yaml:false:my-secret-token", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact false and secret", + input: "https://example.com/spec1.yaml:false:my-secret-token", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: false, - expectedSecret: "my-secret-token", + expectedSecret: "my-secret-token", }, { - name: "URL with port and no suffixes", - input: "http://localhost:8585/spec.yaml", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port and no suffixes", + input: "http://localhost:8585/spec.yaml", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with port and mainArtifact true", - input: "http://localhost:8585/spec.yaml:true", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port and mainArtifact true", + input: "http://localhost:8585/spec.yaml:true", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with port and mainArtifact false", - input: "http://localhost:8585/spec.yaml:false", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port and mainArtifact false", + input: "http://localhost:8585/spec.yaml:false", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: false, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with port, mainArtifact and secret", - input: "http://localhost:8585/spec.yaml:true:my-secret-token", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port, mainArtifact and secret", + input: "http://localhost:8585/spec.yaml:true:my-secret-token", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: true, - expectedSecret: "my-secret-token", + expectedSecret: "my-secret-token", }, { - name: "URL with port, mainArtifact false and secret", - input: "http://localhost:8585/spec.yaml:false:my-secret-token", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port, mainArtifact false and secret", + input: "http://localhost:8585/spec.yaml:false:my-secret-token", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: false, - expectedSecret: "my-secret-token", + expectedSecret: "my-secret-token", }, { - name: "malformed bool suffix preserves URL", - input: "http://localhost:8585/spec.yaml:tru", - expectedURL: "http://localhost:8585/spec.yaml:tru", + name: "malformed bool suffix preserves URL", + input: "http://localhost:8585/spec.yaml:tru", + expectedURL: "http://localhost:8585/spec.yaml:tru", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "malformed bool with secret preserves URL", - input: "http://localhost:8585/spec.yaml:tru:mysecret", - expectedURL: "http://localhost:8585/spec.yaml:tru:mysecret", + name: "malformed bool with secret preserves URL", + input: "http://localhost:8585/spec.yaml:tru:mysecret", + expectedURL: "http://localhost:8585/spec.yaml:tru:mysecret", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with path and no port", - input: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", - expectedURL: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", + name: "URL with path and no port", + input: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", + expectedURL: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "short URL without path", - input: "http://localhost:true", - expectedURL: "http://localhost", + name: "short URL without path", + input: "http://localhost:true", + expectedURL: "http://localhost", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "boolean-like secret is not consumed as primary flag", - input: "http://example.com/api:true:false", - expectedURL: "http://example.com/api", + name: "boolean-like secret is not consumed as primary flag", + input: "http://example.com/api:true:false", + expectedURL: "http://example.com/api", expectedMainArtifact: true, - expectedSecret: "false", + expectedSecret: "false", }, { - name: "secret containing colons", - input: "http://example.com/api:true:my:secret:token", - expectedURL: "http://example.com/api", + name: "secret containing colons", + input: "http://example.com/api:true:my:secret:token", + expectedURL: "http://example.com/api", expectedMainArtifact: true, - expectedSecret: "my:secret:token", + expectedSecret: "my:secret:token", }, { - name: "URL with port and secret containing colons", - input: "http://localhost:8080/api:false:auth:basic:user:pass", - expectedURL: "http://localhost:8080/api", + name: "URL with port and secret containing colons", + input: "http://localhost:8080/api:false:auth:basic:user:pass", + expectedURL: "http://localhost:8080/api", expectedMainArtifact: false, - expectedSecret: "auth:basic:user:pass", + expectedSecret: "auth:basic:user:pass", }, { - name: "boolean host with port does not get misparsed as primary flag", - input: "http://true:8080/api:false:secret", - expectedURL: "http://true:8080/api", + name: "boolean host with port does not get misparsed as primary flag", + input: "http://true:8080/api:false:secret", + expectedURL: "http://true:8080/api", expectedMainArtifact: false, - expectedSecret: "secret", + expectedSecret: "secret", + }, + { + name: "relative path without scheme", + input: "spec.yaml", + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec.yaml': must start with http:// or https://", + }, + { + name: "relative path with suffixes without scheme", + input: "spec.yaml:true:mysecret", + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec.yaml:true:mysecret': must start with http:// or https://", + }, + { + name: "unsupported ftp scheme", + input: "ftp://example.com/spec.yaml", + wantErr: true, + expectedErrMsg: "invalid artifact URL 'ftp://example.com/spec.yaml': must start with http:// or https://", + }, + { + name: "file scheme", + input: "file:///tmp/spec.yaml", + wantErr: true, + expectedErrMsg: "invalid artifact URL 'file:///tmp/spec.yaml': must start with http:// or https://", + }, + { + name: "empty string", + input: "", + wantErr: true, + expectedErrMsg: "invalid artifact URL '': must start with http:// or https://", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - url, mainArtifact, secret := parseImportURLArg(tt.input) + url, mainArtifact, secret, err := parseImportURLArg(tt.input) + if tt.wantErr { + assert.EqualError(t, err, tt.expectedErrMsg) + assert.Equal(t, errors.KindUsage, errors.KindOf(err)) + return + } + assert.NoError(t, err) assert.Equal(t, tt.expectedURL, url) assert.Equal(t, tt.expectedMainArtifact, mainArtifact) assert.Equal(t, tt.expectedSecret, secret) From 2806c15326fe91311a7d73c294208c6f35594446 Mon Sep 17 00:00:00 2001 From: Dipak Dhangar Date: Sun, 11 Oct 2026 18:42:32 +0530 Subject: [PATCH 2/3] fix(import-url): validate artifact URL scheme Signed-off-by: Dipak Dhangar --- cmd/importURL.go | 41 +++++++++-------- cmd/importURL_test.go | 102 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 113 insertions(+), 30 deletions(-) diff --git a/cmd/importURL.go b/cmd/importURL.go index db54c590..eaa9cfa1 100644 --- a/cmd/importURL.go +++ b/cmd/importURL.go @@ -31,12 +31,8 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm Use: "import-url", Short: "import API artifacts from URL on Microcks server", Long: `import API artifacts from URL on Microcks server`, + Args: validateImportURLArgs, RunE: func(cmd *cobra.Command, args []string) error { - // Parse subcommand args first. - if len(args) == 0 { - return errors.Wrapf(errors.KindUsage, "import-url requires a argument") - } - specificationFiles := args[0] mc, _, err := newCommandClient(globalClientOpts) @@ -48,11 +44,7 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm mainArtifact := true secret := "" - var err error - f, mainArtifact, secret, err = parseImportURLArg(f) - if err != nil { - return err - } + f, mainArtifact, secret = parseImportURLArg(f) // Try downloading the artifcat msg, err := mc.DownloadArtifact(f, mainArtifact, secret) @@ -68,20 +60,31 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm return importURLCmd } -func parseImportURLArg(f string) (string, bool, string, error) { - if !strings.HasPrefix(f, "https://") && !strings.HasPrefix(f, "http://") { - return "", false, "", errors.Wrapf( - errors.KindUsage, - "invalid artifact URL '%s': must start with http:// or https://", - f, - ) +func validateImportURLArgs(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return errors.Wrapf(errors.KindUsage, "import-url requires a argument") } + for _, f := range strings.Split(args[0], ",") { + lower := strings.ToLower(f) + if !strings.HasPrefix(lower, "https://") && !strings.HasPrefix(lower, "http://") { + return errors.Wrapf( + errors.KindUsage, + "invalid artifact URL '%s': must start with http:// or https://", + f, + ) + } + } + return nil +} + +func parseImportURLArg(f string) (string, bool, string) { mainArtifact := true secret := "" // Check if URL starts with https or http - if strings.HasPrefix(f, "https://") || strings.HasPrefix(f, "http://") { + lower := strings.ToLower(f) + if strings.HasPrefix(lower, "https://") || strings.HasPrefix(lower, "http://") { parts := strings.Split(f, ":") n := len(parts) @@ -96,5 +99,5 @@ func parseImportURLArg(f string) (string, bool, string, error) { } } } - return f, mainArtifact, secret, nil + return f, mainArtifact, secret } diff --git a/cmd/importURL_test.go b/cmd/importURL_test.go index 5fc8f444..604896d3 100644 --- a/cmd/importURL_test.go +++ b/cmd/importURL_test.go @@ -19,6 +19,7 @@ package cmd import ( "testing" + "github.com/microcks/microcks-cli/pkg/connectors" "github.com/microcks/microcks-cli/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -30,8 +31,6 @@ func TestParseImportURLArg(t *testing.T) { expectedURL string expectedMainArtifact bool expectedSecret string - wantErr bool - expectedErrMsg string }{ { name: "standard URL without suffixes", @@ -159,50 +158,131 @@ func TestParseImportURLArg(t *testing.T) { expectedMainArtifact: false, expectedSecret: "secret", }, + { + name: "uppercase HTTP scheme with suffix", + input: "HTTP://example.com/spec.yaml:false:mysecret", + expectedURL: "HTTP://example.com/spec.yaml", + expectedMainArtifact: false, + expectedSecret: "mysecret", + }, + { + name: "mixed-case Https scheme with suffix", + input: "Https://example.com/spec.yaml:true:mysecret", + expectedURL: "Https://example.com/spec.yaml", + expectedMainArtifact: true, + expectedSecret: "mysecret", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + url, mainArtifact, secret := parseImportURLArg(tt.input) + assert.Equal(t, tt.expectedURL, url) + assert.Equal(t, tt.expectedMainArtifact, mainArtifact) + assert.Equal(t, tt.expectedSecret, secret) + }) + } +} + +func TestValidateImportURLArgs(t *testing.T) { + tests := []struct { + name string + args []string + wantErr bool + expectedErrMsg string + }{ + { + name: "missing args", + args: []string{}, + wantErr: true, + expectedErrMsg: "import-url requires a argument", + }, + { + name: "valid standard URL", + args: []string{"https://example.com/spec.yaml"}, + wantErr: false, + }, + { + name: "valid uppercase HTTP scheme", + args: []string{"HTTP://example.com/spec.yaml"}, + wantErr: false, + }, + { + name: "valid mixed-case Https scheme", + args: []string{"Https://example.com/spec.yaml:false:secret"}, + wantErr: false, + }, { name: "relative path without scheme", - input: "spec.yaml", + args: []string{"spec.yaml"}, wantErr: true, expectedErrMsg: "invalid artifact URL 'spec.yaml': must start with http:// or https://", }, { name: "relative path with suffixes without scheme", - input: "spec.yaml:true:mysecret", + args: []string{"spec.yaml:true:mysecret"}, wantErr: true, expectedErrMsg: "invalid artifact URL 'spec.yaml:true:mysecret': must start with http:// or https://", }, { name: "unsupported ftp scheme", - input: "ftp://example.com/spec.yaml", + args: []string{"ftp://example.com/spec.yaml"}, wantErr: true, expectedErrMsg: "invalid artifact URL 'ftp://example.com/spec.yaml': must start with http:// or https://", }, { name: "file scheme", - input: "file:///tmp/spec.yaml", + args: []string{"file:///tmp/spec.yaml"}, wantErr: true, expectedErrMsg: "invalid artifact URL 'file:///tmp/spec.yaml': must start with http:// or https://", }, { name: "empty string", - input: "", + args: []string{""}, wantErr: true, expectedErrMsg: "invalid artifact URL '': must start with http:// or https://", }, + { + name: "comma-separated valid URLs", + args: []string{"https://example.com/spec1.yaml,http://example.com/spec2.yaml"}, + wantErr: false, + }, + { + name: "comma-separated with second URL invalid fails upfront", + args: []string{"https://example.com/spec1.yaml,spec2.yaml:true"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec2.yaml:true': must start with http:// or https://", + }, + { + name: "comma-separated with first URL invalid fails upfront", + args: []string{"spec1.yaml,https://example.com/spec2.yaml"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec1.yaml': must start with http:// or https://", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - url, mainArtifact, secret, err := parseImportURLArg(tt.input) + err := validateImportURLArgs(nil, tt.args) if tt.wantErr { assert.EqualError(t, err, tt.expectedErrMsg) assert.Equal(t, errors.KindUsage, errors.KindOf(err)) return } assert.NoError(t, err) - assert.Equal(t, tt.expectedURL, url) - assert.Equal(t, tt.expectedMainArtifact, mainArtifact) - assert.Equal(t, tt.expectedSecret, secret) }) } } + +func TestImportURLCommandExecutionRejectsInvalidArgsBeforeRunE(t *testing.T) { + // Create the command with dummy client options. + // If Args validator did not reject the arguments upfront, RunE would proceed + // to newCommandClient, which would fail with a missing microcksURL connection/usage error. + cmd := NewImportURLCommand(&connectors.ClientOptions{}) + cmd.SetArgs([]string{"https://example.com/spec1.yaml,spec2.yaml:true"}) + + err := cmd.Execute() + assert.Error(t, err) + assert.Equal(t, errors.KindUsage, errors.KindOf(err)) + assert.EqualError(t, err, "invalid artifact URL 'spec2.yaml:true': must start with http:// or https://") +} From a64d332c8a4f98fb41cd9e8c5346dd582e653d9e Mon Sep 17 00:00:00 2001 From: Dipak Dhangar Date: Tue, 13 Oct 2026 16:16:14 +0530 Subject: [PATCH 3/3] fix(import-url): remove redundant scheme check Signed-off-by: Dipak Dhangar --- cmd/importURL.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/cmd/importURL.go b/cmd/importURL.go index eaa9cfa1..7d83ea57 100644 --- a/cmd/importURL.go +++ b/cmd/importURL.go @@ -82,21 +82,17 @@ func parseImportURLArg(f string) (string, bool, string) { mainArtifact := true secret := "" - // Check if URL starts with https or http - lower := strings.ToLower(f) - if strings.HasPrefix(lower, "https://") || strings.HasPrefix(lower, "http://") { - parts := strings.Split(f, ":") - n := len(parts) + parts := strings.Split(f, ":") + n := len(parts) - for i := 2; i < n; i++ { - if val, parseErr := strconv.ParseBool(parts[i]); parseErr == nil { - mainArtifact = val - if i+1 < n { - secret = strings.Join(parts[i+1:], ":") - } - f = strings.Join(parts[:i], ":") - break + for i := 2; i < n; i++ { + if val, parseErr := strconv.ParseBool(parts[i]); parseErr == nil { + mainArtifact = val + if i+1 < n { + secret = strings.Join(parts[i+1:], ":") } + f = strings.Join(parts[:i], ":") + break } } return f, mainArtifact, secret