diff --git a/.changes/unreleased/BUG FIXES-20260616-150518.yaml b/.changes/unreleased/BUG FIXES-20260616-150518.yaml new file mode 100644 index 0000000..eb94879 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20260616-150518.yaml @@ -0,0 +1,3 @@ +kind: BUG FIXES +body: Disallow api commands containing non-profile hostname URL argument and non-https schemes. +time: 2026-06-16T15:05:18.528781-06:00 diff --git a/.changes/unreleased/BUG FIXES-20260616-150538.yaml b/.changes/unreleased/BUG FIXES-20260616-150538.yaml new file mode 100644 index 0000000..f4a3ea3 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20260616-150538.yaml @@ -0,0 +1,3 @@ +kind: BUG FIXES +body: Profile configuration files are now created with owner-rw permissions only. +time: 2026-06-16T15:05:38.766655-06:00 diff --git a/.changes/unreleased/BUG FIXES-20260616-150604.yaml b/.changes/unreleased/BUG FIXES-20260616-150604.yaml new file mode 100644 index 0000000..4862f6d --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20260616-150604.yaml @@ -0,0 +1,3 @@ +kind: BUG FIXES +body: hostname telemetry is anonymized when not HCP Terraform. +time: 2026-06-16T15:06:04.201058-06:00 diff --git a/README.md b/README.md index 5ea0770..d23713b 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Comprehensive, official CLI access to the HCP Terraform / Terraform Enterprise p The `tfctl` CLI provides high-level commands for common workflows, such as managing runs, variables, and workspaces, and direct API access for advanced automation. It supports multiple configuration profiles, allowing you to switch between different HCP Terraform organizations and Terraform Enterprise instances. It also integrates with AI coding agents to facilitate agent-assisted management of Terraform workflows. -![tfctl](assets/hero.png "tfctl") +![tfctl](assets/demo.gif "tfctl demo") ## Installation You can install the CLI, command completion utility, and agent skill separately. @@ -89,9 +89,9 @@ Verify that the login is successful before leaving the token page in your browse If the CLI does not find a token configured for the active profile, it checks your Terraform configuration for a matching token. Refer to [Terraform tokens](#terraform-tokens) for more information. -### Set organization +### Set default organization -Run the `tfctl profile set default_organization` command to set the organization. Replace `` with your HCP Terraform or Terraform Enterprise organization name. +Run the `tfctl profile set default_organization` command to set the default organization. Replace `` with your HCP Terraform or Terraform Enterprise organization name. ```bash $ tfctl profile set default_organization @@ -172,7 +172,7 @@ If you have not configured a particular option for the active profile, `tfctl` c `TFCTL_HOSTNAME`: The Terraform Enterprise or HCP Terraform hostname to use. Defaults to `app.terraform.io`. -`TFCTL_TOKEN`: An HCP Terraform API token to use in conjunction with the default profile. +`TFCTL_TOKEN`: An HCP Terraform API token to use in conjunction with the default profile. This variable is not used in conjunction with any other profile. `TFCTL_TOKEN_`: An HCP Terraform API token to use in conjunction with the named profile. diff --git a/assets/demo.gif b/assets/demo.gif new file mode 100644 index 0000000..08a67fb Binary files /dev/null and b/assets/demo.gif differ diff --git a/assets/hero.png b/assets/hero.png deleted file mode 100644 index 7c1e911..0000000 Binary files a/assets/hero.png and /dev/null differ diff --git a/cmd/tfctl/main.go b/cmd/tfctl/main.go index 1e8969e..90795b3 100644 --- a/cmd/tfctl/main.go +++ b/cmd/tfctl/main.go @@ -55,8 +55,16 @@ func realMain() int { } }() + initialLogLevel := logging.LevelDefault + for _, a := range args { + if a == "--debug" { + initialLogLevel = logging.LevelDebug + break + } + } + // The logger level will need to be set by the command after parsing flags. - logger := logging.NewLogger(io) + logger := logging.NewLogger(io, initialLogLevel) // Add the logger to the shutdown context because this is the context used throughout // the command execution lifecycle. @@ -71,7 +79,7 @@ func realMain() int { return 1 } - activeProfile, err := loadActiveProfile(loader) + activeProfile, err := loadActiveProfile(shutdownCtx, loader) if err != nil { fmt.Fprintln(io.Err(), err) return 1 @@ -145,7 +153,7 @@ func realMain() int { } // loadActiveProfile loads the active profile. -func loadActiveProfile(loader *profile.Loader) (*profile.Profile, error) { +func loadActiveProfile(ctx context.Context, loader *profile.Loader) (*profile.Profile, error) { // Load the active profile activeProfile, err := loader.GetActiveProfile() if err != nil { @@ -157,7 +165,7 @@ func loadActiveProfile(loader *profile.Loader) (*profile.Profile, error) { return nil, fmt.Errorf("failed to save default active profile config: %w", err) } - if err := loader.DefaultProfile().Write(); err != nil { + if err := loader.DefaultProfile(ctx).Write(); err != nil { return nil, fmt.Errorf("failed to save default profile config: %w", err) } @@ -167,7 +175,7 @@ func loadActiveProfile(loader *profile.Loader) (*profile.Profile, error) { } } - return loader.LoadProfile(activeProfile.Name) + return loader.LoadProfile(ctx, activeProfile.Name) } // IsAutocomplete returns true if the CLI is being run in an autocomplete diff --git a/go.mod b/go.mod index 957100e..045d1e2 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/hashicorp/cli v1.1.7 github.com/hashicorp/go-hclog v1.6.3 github.com/hashicorp/go-multierror v1.1.1 - github.com/hashicorp/go-tfe/v2 v2.0.0-20260611161741-624e4864f63b + github.com/hashicorp/go-tfe/v2 v2.0.0-beta1 github.com/hashicorp/go-version v1.9.0 github.com/hashicorp/hcl/v2 v2.24.0 github.com/itchyny/gojq v0.12.19 @@ -86,7 +86,7 @@ require ( github.com/mattn/go-runewidth v0.0.19 // indirect github.com/microsoft/kiota-http-go v1.5.6 // indirect github.com/microsoft/kiota-serialization-form-go v1.1.3 // indirect - github.com/microsoft/kiota-serialization-json-go v1.1.2 // indirect + github.com/microsoft/kiota-serialization-json-go v1.1.3 // indirect github.com/microsoft/kiota-serialization-multipart-go v1.1.2 // indirect github.com/microsoft/kiota-serialization-text-go v1.1.3 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect diff --git a/go.sum b/go.sum index 0d90c4d..ba3e08c 100644 --- a/go.sum +++ b/go.sum @@ -119,8 +119,8 @@ github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVH github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= -github.com/hashicorp/go-tfe/v2 v2.0.0-20260611161741-624e4864f63b h1:l5n1LEe/DByj/2+4TwEbfvbwFf0hu6gZ+HyJM8gykds= -github.com/hashicorp/go-tfe/v2 v2.0.0-20260611161741-624e4864f63b/go.mod h1:gosuJ9PH3NLxkCoCW3EIeHHli+5QqLUkboBiUZ1ljCM= +github.com/hashicorp/go-tfe/v2 v2.0.0-beta1 h1:+PKJssuEaY27h+YV75vubEJSRJc4Qic+in58301ILng= +github.com/hashicorp/go-tfe/v2 v2.0.0-beta1/go.mod h1:gosuJ9PH3NLxkCoCW3EIeHHli+5QqLUkboBiUZ1ljCM= github.com/hashicorp/go-version v1.9.0 h1:CeOIz6k+LoN3qX9Z0tyQrPtiB1DFYRPfCIBtaXPSCnA= github.com/hashicorp/go-version v1.9.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= @@ -169,8 +169,8 @@ github.com/microsoft/kiota-http-go v1.5.6 h1:KBdk7sxWYXZnRRExLjIcNt4I7LoOfh/XQJW github.com/microsoft/kiota-http-go v1.5.6/go.mod h1:bpJkXfBAcnmiXRg03GXdnb/vF3Sqk3+EgLvXXjmzzQM= github.com/microsoft/kiota-serialization-form-go v1.1.3 h1:eUY8eHXPFe4ma8cAdx0ya3g4NPlZgbPT+GlFC3xcgGY= github.com/microsoft/kiota-serialization-form-go v1.1.3/go.mod h1:RMO99zyik+NvZjdVcIeyu6ikyfuKhQtzq2RK0fWJJio= -github.com/microsoft/kiota-serialization-json-go v1.1.2 h1:eJrPWeQ665nbjO0gsHWJ0Bw6V/ZHHU1OfFPaYfRG39k= -github.com/microsoft/kiota-serialization-json-go v1.1.2/go.mod h1:deaGt7fjZarywyp7TOTiRsjfYiyWxwJJPQZytXwYQn8= +github.com/microsoft/kiota-serialization-json-go v1.1.3 h1:e9Bx6jXlmDLc/j+9IcMzt2tDrp1EkxNFjEhYteMjKJQ= +github.com/microsoft/kiota-serialization-json-go v1.1.3/go.mod h1:HUTiYs9llTGLjh9+O+yOkBbNEaZ1kxh3sBPU5tPhmeI= github.com/microsoft/kiota-serialization-multipart-go v1.1.2 h1:1pUyA1QgIeKslQwbk7/ox1TehjlCUUT3r1f8cNlkvn4= github.com/microsoft/kiota-serialization-multipart-go v1.1.2/go.mod h1:j2K7ZyYErloDu7Kuuk993DsvfoP7LPWvAo7rfDpdPio= github.com/microsoft/kiota-serialization-text-go v1.1.3 h1:8z7Cebn0YAAr++xswVgfdxZjnAZ4GOB9O7XP4+r5r/M= diff --git a/internal/commands/api/api.go b/internal/commands/api/api.go index 3af0ed9..87e811c 100644 --- a/internal/commands/api/api.go +++ b/internal/commands/api/api.go @@ -252,11 +252,16 @@ func NewCmdAPI(inv *cmd.Invocation) *cmd.Command { path = resolvedPath } + // URL safety validation resolvedURL, err := client.ResolveURL(*apiClient.BaseURL, path) if err != nil { return fmt.Errorf("invalid input path/URL %q", path) } + if resolvedURL.Scheme != "https" { + return fmt.Errorf("invalid input path/URL %q: must use https scheme", path) + } + opts.URL = resolvedURL opts.Client = apiClient opts.Quiet = inv.GetGlobalFlags().Quiet diff --git a/internal/commands/api/api_test.go b/internal/commands/api/api_test.go index 3ec1f02..32a41bb 100644 --- a/internal/commands/api/api_test.go +++ b/internal/commands/api/api_test.go @@ -20,8 +20,10 @@ import ( "github.com/stretchr/testify/require" "github.com/hashicorp/tfctl-cli/internal/pkg/client" + "github.com/hashicorp/tfctl-cli/internal/pkg/cmd" "github.com/hashicorp/tfctl-cli/internal/pkg/format" "github.com/hashicorp/tfctl-cli/internal/pkg/iostreams" + "github.com/hashicorp/tfctl-cli/internal/pkg/profile" ) func TestRunAPI_DefaultGet(t *testing.T) { @@ -307,6 +309,25 @@ func TestRunAPI_InlineQueryParamsSparseFieldsets(t *testing.T) { require.Equal(t, "name", req.Query.Get("fields[workspaces]")) } +func TestNewCmdAPI_NonHTTPSReturnsError(t *testing.T) { + t.Parallel() + + io := iostreams.Test() + inv := &cmd.Invocation{ + IO: io, + Output: format.New(io), + ShutdownCtx: context.Background(), + Profile: &profile.Profile{ + Name: "test", + Hostname: "example.com", + Token: "test-token", + }, + } + cmd := NewCmdAPI(inv) + err := cmd.RunF(cmd, []string{"http://example.com/api/v2/things"}) + require.ErrorContains(t, err, "must use https scheme") +} + func TestRunAPI_InlineQueryParamsMergedWithFlags(t *testing.T) { t.Parallel() diff --git a/internal/commands/auth/login_test.go b/internal/commands/auth/login_test.go index e2cf0bc..9d159d7 100644 --- a/internal/commands/auth/login_test.go +++ b/internal/commands/auth/login_test.go @@ -59,7 +59,7 @@ func TestLoginFromStdin(t *testing.T) { srv := newFakeTFE(t, "testuser") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -76,7 +76,7 @@ func TestLoginFromStdin(t *testing.T) { r.Contains(io.Error.String(), "Successfully logged in") r.Contains(io.Error.String(), "testuser") - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal("my-test-token", loaded.Token) } @@ -87,7 +87,7 @@ func TestLoginFromStdin_CustomHostname(t *testing.T) { srv := newFakeTFE(t, "admin") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -110,7 +110,7 @@ func TestLoginFromStdin_EmptyToken(t *testing.T) { r := require.New(t) l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) r.NoError(p.Write()) io := iostreams.Test() @@ -132,7 +132,7 @@ func TestLoginFromStdin_NoInput(t *testing.T) { r := require.New(t) l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) r.NoError(p.Write()) io := iostreams.Test() @@ -153,7 +153,7 @@ func TestLoginFromStdin_WhitespaceToken(t *testing.T) { r := require.New(t) l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) r.NoError(p.Write()) io := iostreams.Test() @@ -176,7 +176,7 @@ func TestLoginFromStdin_TokenWithWhitespace(t *testing.T) { srv := newFakeTFE(t, "testuser") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -191,7 +191,7 @@ func TestLoginFromStdin_TokenWithWhitespace(t *testing.T) { r.NoError(runLogin(t, opts)) - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal("my-token-with-spaces", loaded.Token) } @@ -201,7 +201,7 @@ func TestLoginInteractive_NoTTY(t *testing.T) { r := require.New(t) l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) r.NoError(p.Write()) io := iostreams.Test() @@ -223,7 +223,7 @@ func TestLoginInteractive_Success(t *testing.T) { srv := newFakeTFE(t, "interactive-user") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -246,7 +246,7 @@ func TestLoginInteractive_Success(t *testing.T) { r.Contains(io.Error.String(), "Successfully logged in") r.Contains(io.Error.String(), "interactive-user") - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal("interactive-token", loaded.Token) } @@ -280,11 +280,11 @@ func TestLoginFromStdin_DifferentProfile(t *testing.T) { r.NoError(runLogin(t, &LoginOpts{IO: io, Profile: p2, Token: true})) // Verify tokens were saved to the correct profiles - loadedProd, err := l.LoadProfile("production") + loadedProd, err := l.LoadProfile(context.Background(), "production") r.NoError(err) r.Equal("prod-token", loadedProd.Token) - loadedStaging, err := l.LoadProfile("staging") + loadedStaging, err := l.LoadProfile(context.Background(), "staging") r.NoError(err) r.Equal("staging-token", loadedStaging.Token) } @@ -295,11 +295,11 @@ func TestLoginFromStdin_DryRun(t *testing.T) { srv := newFakeTFE(t, "testuser") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) - initial, err := l.LoadProfile(p.Name) + initial, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) initialToken := initial.Token @@ -317,7 +317,7 @@ func TestLoginFromStdin_DryRun(t *testing.T) { r.Contains(io.Error.String(), "would save token") r.Contains(io.Error.String(), p.Name) - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal(initialToken, loaded.Token) r.NotEqual("my-new-token", loaded.Token) @@ -329,11 +329,11 @@ func TestLoginInteractive_DryRun(t *testing.T) { srv := newFakeTFE(t, "testuser") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) - initial, err := l.LoadProfile(p.Name) + initial, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) initialToken := initial.Token @@ -353,7 +353,7 @@ func TestLoginInteractive_DryRun(t *testing.T) { r.NoError(runLogin(t, opts)) r.Contains(io.Error.String(), "would save token") - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal(initialToken, loaded.Token) r.NotEqual("interactive-token", loaded.Token) @@ -365,7 +365,7 @@ func TestLoginFromStdin_QuietMode(t *testing.T) { srv := newFakeTFE(t, "testuser") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -382,7 +382,7 @@ func TestLoginFromStdin_QuietMode(t *testing.T) { r.NoError(runLogin(t, opts)) r.Empty(io.Error.String()) - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal("my-token", loaded.Token) } @@ -393,7 +393,7 @@ func TestLoginFromStdin_VerifyFails(t *testing.T) { srv := newFakeTFE(t, "") // empty username → 401 l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -410,7 +410,7 @@ func TestLoginFromStdin_VerifyFails(t *testing.T) { r.Error(err) r.Contains(err.Error(), "failed to verify token") - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.NotEqual("bad-token", loaded.Token) } @@ -421,7 +421,7 @@ func TestLoginInteractive_ConfirmOpensBrowserWithSource(t *testing.T) { srv := newFakeTFE(t, "interactive-user") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) @@ -453,7 +453,7 @@ func TestLoginInteractive_ConfirmOpensBrowserWithSource(t *testing.T) { r.Contains(openedURL, "?source=tfctl-login") r.Contains(io.Error.String(), "Do you want to proceed") - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal("interactive-token", loaded.Token) } @@ -463,10 +463,10 @@ func TestLoginInteractive_DeclineDoesNotOpenBrowser(t *testing.T) { r := require.New(t) l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) r.NoError(p.Write()) - initial, err := l.LoadProfile(p.Name) + initial, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) initialToken := initial.Token @@ -493,7 +493,7 @@ func TestLoginInteractive_DeclineDoesNotOpenBrowser(t *testing.T) { r.False(opened, "browser must not open when the user declines") r.Contains(io.Error.String(), "Login canceled.") - loaded, err := l.LoadProfile(p.Name) + loaded, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal(initialToken, loaded.Token, "token is unchanged when login is declined") } @@ -504,7 +504,7 @@ func TestLoginFromStdin_DoesNotPromptOrOpenBrowser(t *testing.T) { srv := newFakeTFE(t, "testuser") l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) p.Hostname = srv.URL r.NoError(p.Write()) diff --git a/internal/commands/profile/profiles/create.go b/internal/commands/profile/profiles/create.go index f2ede44..f98978e 100644 --- a/internal/commands/profile/profiles/create.go +++ b/internal/commands/profile/profiles/create.go @@ -92,7 +92,6 @@ type CreateOpts struct { func createRun(ctx context.Context, opts *CreateOpts) error { logger := logging.FromContext(ctx) - logger.Debug("creating profile", "name", opts.Name, "hostname", opts.Hostname) // Get the existing profiles profiles, err := opts.Profiles.ListProfiles() @@ -115,9 +114,14 @@ func createRun(ctx context.Context, opts *CreateOpts) error { // Set the hostname if provided if opts.Hostname != "" { - p.Hostname = opts.Hostname + err := p.SetHostname(opts.Hostname) + if err != nil { + return err + } } + logger.Debug("creating profile", "name", opts.Name, "hostname", opts.Hostname) + if opts.DryRun { cs := opts.IO.ColorScheme() fmt.Fprintf(opts.IO.Err(), "%s would create profile %q for %s\n", cs.DryRunLabel(), opts.Name, p.GetHostname()) diff --git a/internal/commands/profile/profiles/create_test.go b/internal/commands/profile/profiles/create_test.go index 6bbaec1..d64793f 100644 --- a/internal/commands/profile/profiles/create_test.go +++ b/internal/commands/profile/profiles/create_test.go @@ -68,3 +68,42 @@ func TestCreateDryRun(t *testing.T) { r.NoError(err) r.NotContains(profiles, "dry_run_profile") } + +func TestCreateInvalidHostname(t *testing.T) { + t.Parallel() + r := require.New(t) + l := profile.TestLoader(t) + io := iostreams.Test() + + opts := &CreateOpts{ + IO: io, + Profiles: l, + Name: "invalid_hostname_profile", + Hostname: "invalidhostname/with/slash", + } + + r.EqualError(createRun(context.Background(), opts), `invalid hostname "invalidhostname/with/slash": must be a valid hostname (with optional port)`) +} + +func TestCreateHostnameWithScheme(t *testing.T) { + t.Parallel() + r := require.New(t) + l := profile.TestLoader(t) + io := iostreams.Test() + + opts := &CreateOpts{ + IO: io, + Profiles: l, + Name: "hostname_with_scheme", + Hostname: "https://example.com:8080", + } + + r.NoError(createRun(context.Background(), opts)) + profiles, err := l.ListProfiles() + r.NoError(err) + r.Contains(profiles, "hostname_with_scheme") + + p, err := l.LoadProfile(context.Background(), "hostname_with_scheme") + r.NoError(err) + r.Equal("example.com:8080", p.GetHostname()) +} diff --git a/internal/commands/profile/profiles/list.go b/internal/commands/profile/profiles/list.go index 89f21af..2e90bf8 100644 --- a/internal/commands/profile/profiles/list.go +++ b/internal/commands/profile/profiles/list.go @@ -56,7 +56,7 @@ type ListOpts struct { Profiles *profile.Loader } -func listRun(_ context.Context, opts *ListOpts) error { +func listRun(ctx context.Context, opts *ListOpts) error { profileNames, err := opts.Profiles.ListProfiles() if err != nil { return fmt.Errorf("failed to list profiles: %w", err) @@ -64,7 +64,7 @@ func listRun(_ context.Context, opts *ListOpts) error { profiles := make([]*profile.Profile, len(profileNames)) for i, n := range profileNames { - p, err := opts.Profiles.LoadProfile(n) + p, err := opts.Profiles.LoadProfile(ctx, n) if err != nil { return fmt.Errorf("failed to load profile %q: %w", n, err) } diff --git a/internal/commands/profile/profiles/rename.go b/internal/commands/profile/profiles/rename.go index aaab06d..d16ab7a 100644 --- a/internal/commands/profile/profiles/rename.go +++ b/internal/commands/profile/profiles/rename.go @@ -97,7 +97,7 @@ func renameRun(ctx context.Context, opts *RenameOpts) error { } // Load the existing profile - existing, err := opts.Profiles.LoadProfile(opts.ExistingName) + existing, err := opts.Profiles.LoadProfile(ctx, opts.ExistingName) if err != nil { if errors.Is(err, profile.ErrNoProfileFilePresent) { return fmt.Errorf("profile %q does not exist", opts.ExistingName) diff --git a/internal/commands/profile/set.go b/internal/commands/profile/set.go index 05beafd..a98692b 100644 --- a/internal/commands/profile/set.go +++ b/internal/commands/profile/set.go @@ -150,7 +150,7 @@ func setRun(ctx context.Context, opts *SetOpts) error { write := true switch opts.Property { case "hostname": - write, err = opts.validateHostname() + write, err = opts.setValidHostname() case "organization": write, err = opts.validateOrg() } @@ -188,7 +188,7 @@ func setRun(ctx context.Context, opts *SetOpts) error { // Notify user about hostname changes if hostnameChanged { fmt.Fprintf(opts.IO.Err(), "\n%s Hostname changed to %q. Default organization and token settings have been cleared.\n", - opts.IO.ColorScheme().WarningLabel(), opts.Value) + opts.IO.ColorScheme().WarningLabel(), opts.Profile.Hostname) fmt.Fprintf(opts.IO.Err(), "Please run %s to reconfigure your token for this hostname.\n\n", opts.IO.ColorScheme().String(fmt.Sprintf("%s auth login", version.Name)).Bold()) fmt.Fprintf(opts.IO.Err(), "It's also recommended to run %s to set a default organization.\n\n", @@ -198,7 +198,13 @@ func setRun(ctx context.Context, opts *SetOpts) error { return nil } -func (o *SetOpts) validateHostname() (bool, error) { +func (o *SetOpts) setValidHostname() (bool, error) { + hostname, err := profile.NormalizeHostname(o.Profile.Hostname) + if err != nil { + return false, err + } + o.Profile.Hostname = hostname + o.Value = hostname return true, nil } diff --git a/internal/commands/profile/set_test.go b/internal/commands/profile/set_test.go index 3c489d5..acd4644 100644 --- a/internal/commands/profile/set_test.go +++ b/internal/commands/profile/set_test.go @@ -114,7 +114,7 @@ func TestSet_Organization(t *testing.T) { r := require.New(t) io := iostreams.Test() l := profile.TestLoader(t) - p := l.DefaultProfile() + p := l.DefaultProfile(context.Background()) r.NoError(p.Write()) o := &SetOpts{ IO: io, @@ -133,7 +133,7 @@ func TestSet_Organization(t *testing.T) { } checkOrg := func(expected string) { - loadedProfile, err := l.LoadProfile(p.Name) + loadedProfile, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.Equal(expected, loadedProfile.DefaultOrganization) } @@ -169,7 +169,27 @@ func TestSetDryRun(t *testing.T) { r.Equal("dry-run-org", o.Profile.DefaultOrganization) r.Contains(io.Error.String(), `would set profile property "default_organization" to "dry-run-org"`) - reloaded, err := l.LoadProfile("test") + reloaded, err := l.LoadProfile(context.Background(), "test") r.NoError(err) r.Equal("original-org", reloaded.DefaultOrganization) } + +func TestSetInvalidHostname(t *testing.T) { + t.Parallel() + r := require.New(t) + + l := profile.TestLoader(t) + io := iostreams.Test() + + p, err := l.NewProfile("test") + r.NoError(err) + o := &SetOpts{ + IO: io, + Profile: p, + Property: "hostname", + Value: "my/deployment:8080", + } + + err = setRun(context.Background(), o) + r.ErrorContains(err, "invalid hostname \"my/deployment:8080\": must be a valid hostname (with optional port)") +} diff --git a/internal/commands/profile/unset_test.go b/internal/commands/profile/unset_test.go index 67814de..beab60a 100644 --- a/internal/commands/profile/unset_test.go +++ b/internal/commands/profile/unset_test.go @@ -83,7 +83,7 @@ func TestUnset(t *testing.T) { r.NoError(err) // Load the profile from disk - reread, err := l.LoadProfile("test") + reread, err := l.LoadProfile(context.Background(), "test") r.NoError(err) c.CheckProfile(reread, r) }) @@ -112,7 +112,7 @@ func TestUnsetDryRun(t *testing.T) { r.NoError(unsetRun(context.Background(), o)) r.Contains(io.Error.String(), `would unset profile property "default_organization"`) - reloaded, err := l.LoadProfile("test") + reloaded, err := l.LoadProfile(context.Background(), "test") r.NoError(err) r.Equal("keep-me", reloaded.DefaultOrganization) } diff --git a/internal/pkg/client/client.go b/internal/pkg/client/client.go index e19b19d..64caab6 100644 --- a/internal/pkg/client/client.go +++ b/internal/pkg/client/client.go @@ -6,7 +6,6 @@ package client import ( "context" - "encoding/json" "errors" "fmt" "net/http" @@ -133,6 +132,7 @@ func (c *Client) Do(ctx context.Context, req *Request) (*http.Response, error) { httpResp, err := c.Adapter.Client.Do(httpReq) if err != nil { + // Unwrap url.Error to get the underlying error type for better error handling by callers. var urlErr *url.Error if errors.As(err, &urlErr) { return nil, urlErr.Err @@ -140,10 +140,6 @@ func (c *Client) Do(ctx context.Context, req *Request) (*http.Response, error) { return nil, err } - if httpResp.StatusCode >= 400 { - return nil, tfe.APIErrorFactory(httpResp, nil) - } - return httpResp, nil } @@ -194,39 +190,6 @@ func httpMethod(method string) abs.HttpMethod { } } -// SummarizeAPIErrors attempts to extract meaningful error messages from typical API error responses. -func SummarizeAPIErrors(body []byte) string { - var payload struct { - Errors []struct { - Status string `json:"status"` - Title string `json:"title"` - Detail string `json:"detail"` - } `json:"errors"` - Error string `json:"error"` - Message string `json:"message"` - } - if err := json.Unmarshal(body, &payload); err != nil { - return "" - } - if len(payload.Errors) > 0 { - parts := make([]string, 0, len(payload.Errors)) - for _, item := range payload.Errors { - if item.Detail != "" { - parts = append(parts, strings.TrimSpace(item.Title+": "+item.Detail)) - continue - } - if item.Title != "" { - parts = append(parts, item.Title) - } - } - return strings.Join(parts, ", ") - } - if payload.Message != "" { - return payload.Message - } - return payload.Error -} - // SetTelemetry wraps the HTTP transport to emit network telemetry about outgoing requests. func (c *Client) SetTelemetry(tel *telemetry.Telemetry) { // Wrap the HTTP transport to inject telemetry context and attributes. diff --git a/internal/pkg/client/client_test.go b/internal/pkg/client/client_test.go index d2b4777..e8f7a2a 100644 --- a/internal/pkg/client/client_test.go +++ b/internal/pkg/client/client_test.go @@ -189,44 +189,6 @@ func TestClientDo_UnwrapsTransportErrors(t *testing.T) { require.ErrorIs(t, err, wantErr) } -func TestSummarizeAPIErrors(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - body string - want string - }{ - { - name: "json api errors", - body: `{"errors":[{"title":"invalid request","detail":"workspace not found"}]}`, - want: "invalid request: workspace not found", - }, - { - name: "message fallback", - body: `{"message":"rate limit exceeded"}`, - want: "rate limit exceeded", - }, - { - name: "error fallback", - body: `{"error":"unauthorized"}`, - want: "unauthorized", - }, - { - name: "invalid json", - body: `not-json`, - want: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - require.Equal(t, tt.want, SummarizeAPIErrors([]byte(tt.body))) - }) - } -} - func TestClientSetLogger_Response(t *testing.T) { t.Parallel() diff --git a/internal/pkg/cmd/command_internal.go b/internal/pkg/cmd/command_internal.go index 053832d..9ca1264 100644 --- a/internal/pkg/cmd/command_internal.go +++ b/internal/pkg/cmd/command_internal.go @@ -58,13 +58,18 @@ func (c *Command) errorToExitCode(_ []string, inv *Invocation, err error) int { fmt.Fprintf(io.Err(), "%s Server error: %s\n", cs.ErrorLabel(), apiErr) return 5 } - fmt.Fprint(io.Err(), heredoc.New(io, heredoc.WithPreserveNewlines(), heredoc.WithWidth(0)).Mustf(` -%s Request error: %s. + if len(apiErr.Details) > 0 { + fmt.Fprint(io.Err(), heredoc.New(io, heredoc.WithPreserveNewlines(), heredoc.WithWidth(0)).Mustf(` + %s Request error: %s. -{{ Bold "Error Details" }} - - %s + {{ Bold "Error Details" }} + - %s + + `, cs.ErrorLabel(), apiErr.Message, strings.Join(apiErr.Details, "\n - "))) + } else { + fmt.Fprintf(io.Err(), "%s Request error: %s\n", cs.ErrorLabel(), apiErr) + } -`, cs.ErrorLabel(), apiErr.Message, strings.Join(apiErr.Details, "\n - "))) return 1 } else if errors.As(err, &exitCodeErr) { exitCode = exitCodeErr.Code diff --git a/internal/pkg/cmd/invocation.go b/internal/pkg/cmd/invocation.go index 25855fc..ff914b3 100644 --- a/internal/pkg/cmd/invocation.go +++ b/internal/pkg/cmd/invocation.go @@ -249,7 +249,7 @@ func (i *Invocation) applyGlobalFlags(_ *Command) error { return err } - p, err := l.LoadProfile(i.flags.profile) + p, err := l.LoadProfile(i.ShutdownCtx, i.flags.profile) if err != nil { return err } diff --git a/internal/pkg/logging/logging.go b/internal/pkg/logging/logging.go index 6b0c3e4..b3a63d6 100644 --- a/internal/pkg/logging/logging.go +++ b/internal/pkg/logging/logging.go @@ -16,7 +16,17 @@ import ( type ctxKey struct{} -var loggingKey = ctxKey{} +const ( + // LevelDefault is the default logging level for the application, which is error. + LevelDefault = hclog.Error + + // LevelDebug is the logging level that includes debug messages, which is more verbose than the default. + LevelDebug = hclog.Debug +) + +var ( + loggingKey = ctxKey{} +) // WithLogger returns a new context with the provided logger. func WithLogger(ctx context.Context, logger hclog.Logger) context.Context { @@ -32,11 +42,11 @@ func FromContext(ctx context.Context) hclog.Logger { } // NewLogger constructs a new logger configured based on the provided IOStreams. -func NewLogger(io iostreams.IOStreams) hclog.Logger { +func NewLogger(io iostreams.IOStreams, initialLevel hclog.Level) hclog.Logger { // Create the Logger logOpt := &hclog.LoggerOptions{ Name: version.Name, - Level: hclog.Error, + Level: initialLevel, Output: io.Err(), TimeFn: time.Now, TimeFormat: "15:04:05.000", diff --git a/internal/pkg/profile/hostcache.go b/internal/pkg/profile/hostcache.go index f8466b5..a6e4552 100644 --- a/internal/pkg/profile/hostcache.go +++ b/internal/pkg/profile/hostcache.go @@ -45,7 +45,12 @@ type CheckRefreshFunc func(mTime *time.Time) RefreshResult // NewHostCacheLoader creates a new HostCacheLoader for the given hostname, using the provided logger for logging. func NewHostCacheLoader(ctx context.Context, baseDir, hostname string) (*HostCacheLoader, error) { logger := logging.FromContext(ctx) - hostDir := filepath.Join(baseDir, normalizeHostname(hostname)) + hostnameNormal, err := NormalizeHostname(hostname) + if err != nil { + return nil, err + } + + hostDir := filepath.Join(baseDir, hostnameNormal) if err := os.MkdirAll(hostDir, 0o766); err != nil { return nil, err } diff --git a/internal/pkg/profile/loader.go b/internal/pkg/profile/loader.go index bf9b466..2d4c177 100644 --- a/internal/pkg/profile/loader.go +++ b/internal/pkg/profile/loader.go @@ -17,7 +17,6 @@ import ( "github.com/google/uuid" "github.com/hashicorp/hcl/v2/hclsimple" "github.com/mitchellh/go-homedir" - "golang.org/x/net/idna" "github.com/hashicorp/tfctl-cli/internal/pkg/logging" "github.com/hashicorp/tfctl-cli/version" @@ -80,7 +79,7 @@ func newLoader(dir string) (*Loader, error) { if err != nil { // If the directory doesn't exist, create it. if errors.Is(err, fs.ErrNotExist) { - if err := os.MkdirAll(path, 0766); err != nil { + if err := os.MkdirAll(path, 0700); err != nil { return nil, fmt.Errorf("failed to created %s config directory %q: %w", version.Name, path, err) } } else { @@ -94,7 +93,7 @@ func newLoader(dir string) (*Loader, error) { if err != nil { // If the directory doesn't exist, create it. if errors.Is(err, fs.ErrNotExist) { - if err := os.MkdirAll(profilesDir, 0766); err != nil { + if err := os.MkdirAll(profilesDir, 0700); err != nil { return nil, fmt.Errorf("failed to created %s profiles directory %q: %w", version.Name, profilesDir, err) } } else { @@ -196,7 +195,8 @@ func (l *Loader) ListProfiles() ([]string, error) { // LoadProfile loads a profile given its name. If the profile can not be found, // ErrNoProfileFilePresent will be returned. Otherwise, an error will be // returned if the profile is invalid. -func (l *Loader) LoadProfile(name string) (*Profile, error) { +func (l *Loader) LoadProfile(ctx context.Context, name string) (*Profile, error) { + logger := logging.FromContext(ctx) // Expand the directory. path := filepath.Join(l.profilesDir, fmt.Sprintf("%s.hcl", name)) @@ -224,27 +224,43 @@ func (l *Loader) LoadProfile(name string) (*Profile, error) { // If there's no default organization set, use the environment variable if it's set. if c.DefaultOrganization == "" { if orgID, ok := os.LookupEnv(envVarOrganization); ok && orgID != "" { + logger.Debug("Setting default_organization from "+envVarOrganization, "organization", orgID) c.DefaultOrganization = orgID } } - // If there's no token set, check the credentials file and environment variables. - if c.Token == "" { - credsToken, err := tokenFromCredentials(c.Hostname) - if err != nil { - return nil, err - } - c.tokenFromEnv = credsToken + // If there's no token set, check the credentials file and environment variables. These are + // checked in a careful order of precedence. + + if c.Token != "" { + logger.Debug("Using token from profile", "name", c.Name) } - if c.Token == "" { + // 1. Check for a token specific to tfctl (TFCTL_TOKEN_{profileName} or TFCTL_TOKEN for the default profile) + if c.GetToken() == "" { if envToken := os.Getenv(profileTokenEnvVar(c.Name)); envToken != "" { + logger.Debug("Setting token from environment", "var", profileTokenEnvVar(c.Name)) c.tokenFromEnv = envToken } } - if c.Token == "" { - if envToken := os.Getenv(legacyTokenEnvVar(c.Hostname)); envToken != "" { + // 2. Check for a token in the terraform credentials file that matches the hostname of the profile + if c.GetToken() == "" { + credsToken, err := tokenFromCredentials(c.GetHostname()) + if err != nil { + return nil, err + } + if credsToken != "" { + logger.Debug("Setting token from terraform credentials file", "hostname", c.GetHostname()) + c.tokenFromEnv = credsToken + } + } + + // 3. Check for a token in the terraform environment variable that matches the hostname of the + // profile (support for TF_TOKEN_{normalizedHostname} + if c.GetToken() == "" { + if envToken := os.Getenv(terraformTokenEnvVar(c.GetHostname())); envToken != "" { + logger.Debug("Setting token from terraform environment", "var", terraformTokenEnvVar(c.GetHostname())) c.tokenFromEnv = envToken } } @@ -256,25 +272,6 @@ func (l *Loader) LoadProfile(name string) (*Profile, error) { return &c, nil } -// LoadProfiles loads all the available profiles. -func (l *Loader) LoadProfiles() ([]*Profile, error) { - profileNames, err := l.ListProfiles() - if err != nil { - return nil, err - } - - var profiles []*Profile - for _, n := range profileNames { - p, err := l.LoadProfile(n) - if err != nil { - return nil, fmt.Errorf("failed to load profile %q: %w", n, err) - } - profiles = append(profiles, p) - } - - return profiles, nil -} - // DeleteProfile deletes the profile with the given name. If the profile can not be found, // ErrNoProfileFilePresent will be returned. Otherwise, an error will be // returned if the profile can not be deleted for any other reason.. @@ -304,7 +301,8 @@ const ( // DefaultProfile returns the minimal default profile. If environment // variables related to organization and project are set, they are honored here. -func (l *Loader) DefaultProfile() *Profile { +func (l *Loader) DefaultProfile(ctx context.Context) *Profile { + logger := logging.FromContext(ctx) profile, err := l.NewProfile(ProfileNameDefault) if err != nil { panic("The default profile should always be valid. This is always a developer error: " + err.Error()) @@ -317,7 +315,14 @@ func (l *Loader) DefaultProfile() *Profile { hostname := DefaultHostname if envHostname, ok := os.LookupEnv(envVarHostname); ok && envHostname != "" { - hostname = envHostname + hostnameNormal, err := NormalizeHostname(envHostname) + if err != nil { + logger.Debug("Invalid hostname set by environment (using default)", "error", err) + hostnameNormal = DefaultHostname + } else { + logger.Debug("Using hostname from "+envVarHostname, "hostname", hostnameNormal) + } + hostname = hostnameNormal } profile.Hostname = hostname @@ -335,17 +340,6 @@ func (l *Loader) NewProfile(name string) (*Profile, error) { return p, p.Validate() } -func normalizeHostname(hostname string) string { - hostname = strings.TrimSpace(hostname) - hostname = strings.TrimPrefix(hostname, "https://") - hostname = strings.TrimPrefix(hostname, "http://") - hostname = strings.TrimRight(hostname, "/") - if asciiHost, err := idna.Lookup.ToASCII(hostname); err == nil { - return asciiHost - } - return hostname -} - func profileTokenEnvVar(profileName string) string { if profileName == "" || profileName == "default" { return envVarToken @@ -353,8 +347,11 @@ func profileTokenEnvVar(profileName string) string { return fmt.Sprintf(envVarTokenProfileFormat, profileName) } -func legacyTokenEnvVar(hostname string) string { - hostname = normalizeHostname(hostname) +func terraformTokenEnvVar(hostname string) string { + hostname, err := NormalizeHostname(hostname) + if err != nil { + return "" + } var b strings.Builder b.WriteString("TF_TOKEN_") @@ -393,7 +390,11 @@ func tokenFromCredentials(hostname string) (string, error) { return "", fmt.Errorf("parse %s: %w", path, err) } - hostname = normalizeHostname(hostname) + hostname, err = NormalizeHostname(hostname) + if err != nil { + return "", err + } + entry, ok := creds.Credentials[hostname] if !ok { return "", nil diff --git a/internal/pkg/profile/loader_test.go b/internal/pkg/profile/loader_test.go index 63aca19..f901572 100644 --- a/internal/pkg/profile/loader_test.go +++ b/internal/pkg/profile/loader_test.go @@ -146,7 +146,7 @@ func TestLoader_LoadProfile(t *testing.T) { r := require.New(t) l := TestLoader(t) - p, err := l.LoadProfile("test") + p, err := l.LoadProfile(context.Background(), "test") r.Nil(p) r.ErrorIs(err, ErrNoProfileFilePresent) }) @@ -161,7 +161,7 @@ func TestLoader_LoadProfile(t *testing.T) { path := filepath.Join(l.configDir, ProfileDir, fmt.Sprintf("%s.hcl", name)) r.NoError(os.WriteFile(path, []byte("invalid!"), 0x777)) - p, err := l.LoadProfile(name) + p, err := l.LoadProfile(context.Background(), name) r.Nil(p) r.ErrorContains(err, "failed to decode profile") }) @@ -178,7 +178,7 @@ func TestLoader_LoadProfile(t *testing.T) { default_organization = "123"`, ), 0x777)) - p, err := l.LoadProfile(name) + p, err := l.LoadProfile(context.Background(), name) r.Nil(p) r.ErrorContains(err, "profile path name does not match name in file") }) @@ -193,7 +193,7 @@ default_organization = "123"`, p.DefaultOrganization = "123" r.NoError(p.Write()) - out, err := l.LoadProfile(p.Name) + out, err := l.LoadProfile(context.Background(), p.Name) r.NotNil(out) r.Equal(p.Name, out.Name) r.Equal(p.DefaultOrganization, out.DefaultOrganization) @@ -239,82 +239,59 @@ func TestLoader_LoadProfileEnv(t *testing.T) { r := require.New(t) l, err := newLoader(t.TempDir()) r.NoError(err) - prof := l.DefaultProfile() + prof := l.DefaultProfile(context.Background()) r.Equal("xyz", prof.DefaultOrganization) }) - //nolint:paralleltest - t.Run("valid active profile, env set", func(t *testing.T) { - r := require.New(t) - l := TestLoader(t) + t.Run("default profile, hostname env set", func(t *testing.T) { + t.Setenv(envVarHostname, "https://example.com/with/path") - p, err := l.NewProfile("test") + r := require.New(t) + l, err := newLoader(t.TempDir()) r.NoError(err) - r.NoError(p.Write()) - - t.Setenv(envVarOrganization, "xyz") + prof := l.DefaultProfile(context.Background()) - out, err := l.LoadProfile(p.Name) - r.NoError(err) - r.NotNil(out) - r.Equal("xyz", out.DefaultOrganization) + r.Equal("example.com", prof.Hostname) }) -} -func TestLoader_LoadProfiles(t *testing.T) { - t.Parallel() + t.Run("default profile, hostname with port env set", func(t *testing.T) { + t.Setenv(envVarHostname, "example.com:8080") - t.Run("no profile", func(t *testing.T) { - t.Parallel() r := require.New(t) - l := TestLoader(t) - - profiles, err := l.LoadProfiles() - r.Nil(profiles) + l, err := newLoader(t.TempDir()) r.NoError(err) + prof := l.DefaultProfile(context.Background()) + + r.Equal("example.com:8080", prof.Hostname) }) - t.Run("valid profile", func(t *testing.T) { - t.Parallel() - r := require.New(t) - l := TestLoader(t) + t.Run("default profile, invalid hostname env set", func(t *testing.T) { + t.Setenv(envVarHostname, "example/youtube") - p, err := l.NewProfile("test") + r := require.New(t) + l, err := newLoader(t.TempDir()) r.NoError(err) - p.DefaultOrganization = "123" - r.NoError(p.Write()) + prof := l.DefaultProfile(context.Background()) - out, err := l.LoadProfiles() - r.NoError(err) - r.Len(out, 1) - r.Equal(p.Name, out[0].Name) - r.Equal(p.DefaultOrganization, out[0].DefaultOrganization) - r.NoError(err) + r.Equal(DefaultHostname, prof.Hostname) }) - t.Run("valid profiles", func(t *testing.T) { - t.Parallel() + //nolint:paralleltest + t.Run("valid active profile, env set", func(t *testing.T) { r := require.New(t) l := TestLoader(t) p, err := l.NewProfile("test") r.NoError(err) - p.DefaultOrganization = "123" r.NoError(p.Write()) - p2, err := l.NewProfile("test2") - r.NoError(err) - p2.DefaultOrganization = "456" - r.NoError(p2.Write()) + t.Setenv(envVarOrganization, "xyz") - out, err := l.LoadProfiles() + out, err := l.LoadProfile(context.Background(), p.Name) r.NoError(err) r.NotNil(out) - r.Equal(p.Name, out[0].Name) - r.Equal(p.DefaultOrganization, out[0].DefaultOrganization) - r.Equal(p2.Name, out[1].Name) - r.Equal(p2.DefaultOrganization, out[1].DefaultOrganization) + r.Equal("xyz", out.DefaultOrganization) }) } @@ -344,7 +321,7 @@ func TestLoader_DeleteProfile(t *testing.T) { path := filepath.Join(l.configDir, ProfileDir, fmt.Sprintf("%s.hcl", name)) r.NoError(os.WriteFile(path, []byte("invalid!"), 0x777)) - p, err := l.LoadProfile(name) + p, err := l.LoadProfile(context.Background(), name) r.Nil(p) r.ErrorContains(err, "failed to decode profile") }) diff --git a/internal/pkg/profile/profile.go b/internal/pkg/profile/profile.go index 3237cb3..53d0c60 100644 --- a/internal/pkg/profile/profile.go +++ b/internal/pkg/profile/profile.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "net/url" "os" "path/filepath" "reflect" @@ -18,6 +19,7 @@ import ( "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclwrite" "github.com/posener/complete" + "golang.org/x/net/idna" ) const ( @@ -38,6 +40,8 @@ var ( // ErrInvalidProfileName is returned if a profile is created with an invalid // profile name. ErrInvalidProfileName = errors.New("profile name may only include a-z, A-Z, 0-9, or '_', must start with a letter, and can be no longer than 64 characters") + + validHostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9.-]+(:\d+)?$`) ) // ActiveProfile stores the active profile. @@ -179,7 +183,8 @@ func (p *Profile) Write() error { path := fmt.Sprintf("%s/%s.hcl", p.dir, p.Name) f := hclwrite.NewEmptyFile() gohcl.EncodeIntoBody(p, f.Body()) - return os.WriteFile(path, f.Bytes(), 0o666) + + return os.WriteFile(path, f.Bytes(), os.FileMode(0o600)) } // String returns an HCL formatted string representation of the profile. @@ -256,6 +261,51 @@ func (p *Profile) GetHostname() string { return p.Hostname } +// SetHostname sets the profile's hostname after validating it. The hostname should be a hostname +// with an optional port, and should not include a scheme. If the hostname includes a scheme, the +// scheme will be stripped. +func (p *Profile) SetHostname(hostname string) error { + if p == nil { + return nil + } + + hostname, err := NormalizeHostname(hostname) + if err != nil { + return err + } + p.Hostname = hostname + return nil +} + +// NormalizeHostname validates and normalizes the given hostname by stripping any extra URL data, +// like paths. It also converts domain names to their idna ASCII form. +func NormalizeHostname(hostname string) (string, error) { + u, err := url.Parse(hostname) + if err != nil { + return "", fmt.Errorf("invalid hostname %q: must be a valid hostname (with optional port)", hostname) + } + + if err == nil && u.Host != "" { + hostname = u.Host + } + + if asciiHost, err := idna.Lookup.ToASCII(hostname); err == nil { + return asciiHost, nil + } + + if !validHostnamePattern.MatchString(hostname) { + return "", fmt.Errorf("invalid hostname %q: must be a valid hostname (with optional port)", hostname) + } + + return hostname, nil +} + +// IsHCPTerraform returns true if the profile's hostname is a known HCP Terraform hostname +// in any geo. +func (p *Profile) IsHCPTerraform() bool { + return p.GetHostname() == "app.terraform.io" || p.GetHostname() == "app.eu.terraform.io" +} + // SetDefaultOrganization sets the default organization. func (p *Profile) SetDefaultOrganization(name string) *Profile { if p == nil { diff --git a/internal/pkg/profile/profile_test.go b/internal/pkg/profile/profile_test.go index 599d8aa..faa6471 100644 --- a/internal/pkg/profile/profile_test.go +++ b/internal/pkg/profile/profile_test.go @@ -5,7 +5,9 @@ package profile import ( "context" + "os" "path" + "path/filepath" "strings" "testing" "time" @@ -121,6 +123,123 @@ func TestCore_Getters(t *testing.T) { r.Equal("token-from-env", p.GetToken()) } +func TestNormalizeHostname(t *testing.T) { + t.Parallel() + + cases := []struct { + Name string + Input string + Expected string + Error string + }{ + { + Name: "valid hostname", + Input: "example.com", + Expected: "example.com", + }, + { + Name: "hostname with scheme", + Input: "https://example.com", + Expected: "example.com", + }, + { + Name: "hostname with path", + Input: "example.com/some/path", + Error: `invalid hostname "example.com/some/path": must be a valid hostname (with optional port)`, + }, + { + Name: "invalid hostname", + Input: "invalid/hostname", + Error: `invalid hostname "invalid/hostname": must be a valid hostname (with optional port)`, + }, + { + Name: "hostname with unicode characters", + Input: "täst.com", + Expected: "xn--tst-qla.com", + }, + } + + for _, c := range cases { + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + r := require.New(t) + + output, err := NormalizeHostname(c.Input) + if c.Error == "" { + r.NoError(err) + r.Equal(c.Expected, output) + } else { + r.ErrorContains(err, c.Error) + } + }) + } +} + +func TestProfile_SetHostname(t *testing.T) { + t.Parallel() + + cases := []struct { + Name string + Hostname string + Error string + Expected string + }{ + { + Name: "valid hostname", + Hostname: "example.com", + Error: "", + Expected: "example.com", + }, + { + Name: "valid hostname with port", + Hostname: "example.com:8080", + Error: "", + Expected: "example.com:8080", + }, + { + Name: "hostname with scheme", + Hostname: "https://example.com", + Error: "", + Expected: "example.com", + }, + { + Name: "invalid hostname with slash", + Hostname: "invalid/hostname", + Error: `invalid hostname "invalid/hostname": must be a valid hostname (with optional port)`, + }, + { + Name: "cannot be parsed", + Hostname: "http://[invalid:hostname]", + Error: `invalid hostname "http://[invalid:hostname]": must be a valid hostname (with optional port)`, + }, + } + + for _, c := range cases { + t.Run(c.Name, func(t *testing.T) { + p := &Profile{} + r := require.New(t) + err := p.SetHostname(c.Hostname) + if c.Error == "" { + r.NoError(err) + } else { + r.ErrorContains(err, c.Error) + } + + if c.Expected != "" { + r.Equal(c.Expected, p.GetHostname()) + } + }) + } + + t.Run("nil profile", func(t *testing.T) { + p := (*Profile)(nil) + r := require.New(t) + err := p.SetHostname("example.com") + r.NoError(err) + r.Equal("", p.GetHostname()) + }) +} + func TestProfile_HostCache(t *testing.T) { t.Parallel() r := require.New(t) @@ -139,3 +258,21 @@ func TestProfile_HostCache(t *testing.T) { r.FileExists(path.Join(h.dir, "test.json")) } + +func TestProfile_WritePermissions(t *testing.T) { + t.Parallel() + r := require.New(t) + + p := &Profile{ + Name: "test", + dir: t.TempDir(), + } + err := p.Write() + r.NoError(err) + + path := filepath.Join(p.dir, "test.hcl") + + info, err := os.Stat(path) + r.NoError(err) + r.Equal(os.FileMode(0o600), info.Mode().Perm()) +} diff --git a/internal/pkg/profile/testing.go b/internal/pkg/profile/testing.go index 0fee16f..2d8bab6 100644 --- a/internal/pkg/profile/testing.go +++ b/internal/pkg/profile/testing.go @@ -4,13 +4,14 @@ package profile import ( + "context" "testing" ) // TestProfile returns a profile appropriate for use during testing. If // interacting with more than one profile, prefer using TestLoader. func TestProfile(t *testing.T) *Profile { //nolint:paralleltest - defaultProfile := TestLoader(t).DefaultProfile() + defaultProfile := TestLoader(t).DefaultProfile(context.Background()) defaultProfile.Hostname = "app.test.terraform.io" defaultProfile.hostCacheDir = t.TempDir() defaultProfile.DefaultOrganization = "test-organization" diff --git a/internal/pkg/telemetry/telemetry.go b/internal/pkg/telemetry/telemetry.go index b855f78..d24f87d 100644 --- a/internal/pkg/telemetry/telemetry.go +++ b/internal/pkg/telemetry/telemetry.go @@ -258,9 +258,14 @@ func (t *Telemetry) StartCommand(ctx context.Context, info CommandInfo) context. if info.Profile != nil { attrs = append(attrs, - attribute.String("hostname", info.Profile.GetHostname()), attribute.Bool("is_named_profile", info.Profile.Name != profile.ProfileNameDefault), ) + hostname := info.Profile.GetHostname() + if !info.Profile.IsHCPTerraform() { + hostname = generateStableID(info.Profile.GetHostname(), 0) + } + + attrs = append(attrs, attribute.String("hostname", hostname)) } if agent := detectAgent(); agent != "" { diff --git a/internal/pkg/telemetry/telemetry_test.go b/internal/pkg/telemetry/telemetry_test.go index c2bb78f..87d6623 100644 --- a/internal/pkg/telemetry/telemetry_test.go +++ b/internal/pkg/telemetry/telemetry_test.go @@ -257,9 +257,11 @@ func TestStartCommand_CreatesSpanWithAttributes(t *testing.T) { tel, exporter := newTestTelemetry(t) + p := profile.TestProfile(t) + ctx := tel.StartCommand(context.Background(), CommandInfo{ Command: "run start", - Profile: profile.TestProfile(t), + Profile: p, DryRun: true, }) require.NotNil(t, ctx) @@ -285,6 +287,8 @@ func TestStartCommand_CreatesSpanWithAttributes(t *testing.T) { assert.Equal(t, false, attrs["is_tty"]) assert.NotEmpty(t, attrs["os"]) assert.NotEmpty(t, attrs["arch"]) + assert.NotEqual(t, attrs["hostname"], p.GetHostname(), "Value must be a hashed value of the test profile hostname") + assert.NotEmpty(t, attrs["hostname"]) } func TestStartCommand_IncludesCIAttribute(t *testing.T) {