diff --git a/cmd/auth/seturl/cmd.go b/cmd/auth/seturl/cmd.go index b43022661..b83a9a112 100644 --- a/cmd/auth/seturl/cmd.go +++ b/cmd/auth/seturl/cmd.go @@ -16,15 +16,19 @@ package seturl import ( "github.com/datarobot/cli/internal/auth" + "github.com/datarobot/cli/internal/cli" "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/log" "github.com/datarobot/cli/internal/telemetry" "github.com/spf13/cobra" ) func Cmd() *cobra.Command { cmd := &cobra.Command{ - Use: "set-url [url]", - Short: "🌐 Configure your DataRobot environment URL.", + Use: "set-url [url]", + SilenceErrors: true, + SilenceUsage: true, + Short: "🌐 Configure your DataRobot environment URL.", Long: `Configure your DataRobot environment URL with an interactive selection. This command helps you choose the correct DataRobot environment: @@ -34,28 +38,33 @@ This command helps you choose the correct DataRobot environment: • Custom/On-Premise: Your organization's DataRobot URL 💡 If you're unsure, check the URL you use to log in to DataRobot in your browser.`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { var url string if len(args) > 0 { url = args[0] } + // An explicit arg that won't validate is the user's to fix; report it + // rather than silently dropping into the interactive picker. if url != "" { - err := config.SetURLToConfig(url) - if err == nil { - _ = auth.WriteConfigFileSilent() - _ = auth.EnsureAuthenticatedE(cmd, args) + if err := config.SetURLToConfig(url); err != nil { + log.Error(err.Error()) - return + return cli.ErrSilent } - } - urlChanged := auth.SetURLAction() + _ = auth.WriteConfigFileSilent() + _ = auth.EnsureAuthenticatedE(cmd, args) + + return nil + } - if urlChanged { + if auth.SetURLAction() { _ = auth.WriteConfigFileSilent() _ = auth.EnsureAuthenticatedE(cmd, args) } + + return nil }, } diff --git a/cmd/auth/seturl/cmd_test.go b/cmd/auth/seturl/cmd_test.go new file mode 100644 index 000000000..e1cb1a560 --- /dev/null +++ b/cmd/auth/seturl/cmd_test.go @@ -0,0 +1,32 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package seturl + +import ( + "testing" + + "github.com/datarobot/cli/internal/cli" + "github.com/stretchr/testify/require" +) + +// An explicit arg with an unsupported scheme errors out instead of falling +// through to the interactive picker. +func TestSetURLRejectsNonHTTPSchemeArg(t *testing.T) { + cmd := Cmd() + + err := cmd.RunE(cmd, []string{"ftp://app.datarobot.com"}) + + require.ErrorIs(t, err, cli.ErrSilent) +} diff --git a/docs/commands/auth.md b/docs/commands/auth.md index f131eec92..189266a50 100644 --- a/docs/commands/auth.md +++ b/docs/commands/auth.md @@ -295,6 +295,10 @@ dr auth set-url [url] - `url` (optional) - DataRobot instance URL. For example: `https://app.datarobot.com` +A bare host like `app.datarobot.com` is accepted and defaults to `https`. A URL whose +scheme is not `http` or `https` is rejected: `dr auth set-url ftp://host` prints +`unsupported URL scheme "ftp", use https://` and exits non-zero. + **Interactive mode:** If you run `dr auth set-url` without providing a URL, the CLI shows a picker. Move with diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 0ed525fd2..0f1512cc5 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -125,12 +125,7 @@ func ValidateEndpoint(endpoint string) error { return err } - // Checked here, not in SchemeHostOnly, which set-url and export share. - if scheme, _, _ := strings.Cut(baseURL, "://"); scheme != "http" && scheme != "https" { - return fmt.Errorf("unsupported URL scheme %q, use https://", scheme) - } - - return nil + return config.RequireHTTPScheme(baseURL) } // ReportEnvCredentialsError writes a classified explanation of why an diff --git a/internal/config/api.go b/internal/config/api.go index 3b24cda42..354e19e8c 100644 --- a/internal/config/api.go +++ b/internal/config/api.go @@ -16,6 +16,7 @@ package config import ( "errors" + "fmt" "net/http" "net/http/httputil" "net/url" @@ -59,6 +60,26 @@ func SchemeHostOnly(longURL string) (string, error) { return parsedURL.String(), nil } +// unsupportedSchemeError is an ErrInvalidURL, so the interactive picker re-asks +// on a bad scheme instead of aborting, while keeping its specific message. +type unsupportedSchemeError struct{ scheme string } + +func (e *unsupportedSchemeError) Error() string { + return fmt.Sprintf("unsupported URL scheme %q, use https://", e.scheme) +} + +func (e *unsupportedSchemeError) Unwrap() error { return ErrInvalidURL } + +// RequireHTTPScheme rejects a normalized base URL whose scheme is not http or +// https. SchemeHostOnly stays scheme-agnostic (export and GetBaseURL share it). +func RequireHTTPScheme(baseURL string) error { + if scheme, _, _ := strings.Cut(baseURL, "://"); scheme != "http" && scheme != "https" { + return &unsupportedSchemeError{scheme} + } + + return nil +} + func GetBaseURL() string { if endpoint := viper.GetString(DataRobotURL); endpoint != "" { if newURL, err := SchemeHostOnly(endpoint); err == nil { @@ -160,6 +181,13 @@ func SaveURLToConfig(newURL string) error { return err } + // Empty is the reset case below; a non-empty host has to be http/https. + if newURL != "" { + if err = RequireHTTPScheme(newURL); err != nil { + return err + } + } + if err = CreateConfigFileDirIfNotExists(); err != nil { return err } @@ -187,6 +215,10 @@ func SetURLToConfig(newURL string) error { return err } + if err := RequireHTTPScheme(newURL); err != nil { + return err + } + viper.Set(DataRobotURL, newURL+DRAPIURLSuffix) return nil diff --git a/internal/config/api_test.go b/internal/config/api_test.go index f418268bd..70431ebfe 100644 --- a/internal/config/api_test.go +++ b/internal/config/api_test.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -95,6 +96,21 @@ func (suite *APITestSuite) TestSetURLToConfig() { input: "not a url", expectError: true, }, + { + name: "http scheme is accepted", + input: "http://localhost:8080", + expectedURL: "http://localhost:8080/api/v2", + }, + { + name: "ftp scheme is rejected", + input: "ftp://app.datarobot.com", + expectError: true, + }, + { + name: "file scheme is rejected", + input: "file://host/etc/passwd", + expectError: true, + }, } for _, tc := range tests { @@ -121,6 +137,15 @@ func (suite *APITestSuite) TestSetURLToConfigDoesNotWriteFile() { suite.NoFileExists(configFile, "SetURLToConfig must not write the config file to disk") } +// SaveURLToConfig is the template-setup write path; it must reject a bad scheme +// too, or the custom-host picker persists an endpoint the CLI cannot use. +func (suite *APITestSuite) TestSaveURLToConfigRejectsNonHTTPScheme() { + err := SaveURLToConfig("ftp://app.datarobot.com") + + suite.Require().Error(err) + suite.Empty(viper.GetString(DataRobotURL), "a rejected scheme must not be persisted") +} + func (suite *APITestSuite) TestCommandPathToTrace() { tests := []struct { name string @@ -251,3 +276,32 @@ func TestRedactSecretFields_CoversTheOtherNames(t *testing.T) { assert.NotContains(t, out, "hunter2", "field %q", field) } } + +// RequireHTTPScheme expects an already-normalized base URL (SchemeHostOnly runs +// first), so a scheme-less string is rejected, not defaulted. +func TestRequireHTTPScheme(t *testing.T) { + tests := []struct { + name string + baseURL string + wantErr string + }{ + {"https accepted", "https://app.datarobot.com", ""}, + {"http accepted", "http://localhost:8080", ""}, + {"ftp rejected", "ftp://app.datarobot.com", `unsupported URL scheme "ftp"`}, + {"file rejected", "file://host", `unsupported URL scheme "file"`}, + {"scheme-less rejected", "app.datarobot.com", "unsupported URL scheme"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := RequireHTTPScheme(tc.baseURL) + + if tc.wantErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tc.wantErr) + require.ErrorIs(t, err, ErrInvalidURL, "picker re-asks on ErrInvalidURL") + } + }) + } +}