diff --git a/internal/cmd/variable/create/create.go b/internal/cmd/variable/create/create.go index 59cdd13..fe89a17 100644 --- a/internal/cmd/variable/create/create.go +++ b/internal/cmd/variable/create/create.go @@ -3,6 +3,7 @@ package create import ( "context" "fmt" + "sort" "strings" "github.com/briandowns/spinner" @@ -135,22 +136,27 @@ func runCreateVariableNonInteractive(f *cmdutil.Factory, opts *Options) error { return fmt.Errorf("failed to create variables of service: %s", opts.name) } s.Stop() + createdKeys := make([]string, 0, len(opts.keys)) + for key := range opts.keys { + createdKeys = append(createdKeys, key) + } + sort.Strings(createdKeys) if f.JSON { - out := make([]map[string]string, 0, len(varMap)) - for k, v := range varMap { - out = append(out, map[string]string{"Key": k, "Value": v}) + out := make([]map[string]string, 0, len(createdKeys)) + for _, key := range createdKeys { + out = append(out, map[string]string{"Key": key}) } return f.Printer.JSON(out) } f.Log.Infof("Successfully created variables of service: %s\n", opts.name) - table := make([][]string, 0, len(varMap)) - for k, v := range varMap { - table = append(table, []string{k, v}) + table := make([][]string, 0, len(createdKeys)) + for _, key := range createdKeys { + table = append(table, []string{key}) } - f.Printer.Table([]string{"Key", "Value"}, table) + f.Printer.Table([]string{"Key"}, table) return nil } diff --git a/internal/cmd/variable/create/create_test.go b/internal/cmd/variable/create/create_test.go new file mode 100644 index 0000000..a8ac387 --- /dev/null +++ b/internal/cmd/variable/create/create_test.go @@ -0,0 +1,106 @@ +package create_test + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "go.uber.org/zap" + + "github.com/zeabur/cli/internal/cmd/variable/create" + "github.com/zeabur/cli/internal/cmdutil" + "github.com/zeabur/cli/pkg/api" + "github.com/zeabur/cli/pkg/model" +) + +type stubVariableClient struct { + api.Client + + variables model.Variables + updated map[string]string +} + +func (c *stubVariableClient) ListVariables(context.Context, string, string) (model.Variables, model.Variables, error) { + return c.variables, nil, nil +} + +func (c *stubVariableClient) UpdateVariables(_ context.Context, _, _ string, data map[string]string) (bool, error) { + c.updated = data + return true, nil +} + +type capturePrinter struct { + header []string + rows [][]string + jsonValue any +} + +func (p *capturePrinter) Table(header []string, rows [][]string) { + p.header = header + p.rows = rows +} + +func (p *capturePrinter) JSON(v any) error { + p.jsonValue = v + return nil +} + +func (p *capturePrinter) output() string { + encoded, _ := json.Marshal(struct { + Header []string + Rows [][]string + JSON any + }{p.header, p.rows, p.jsonValue}) + return string(encoded) +} + +func TestCreateDoesNotExposeVariableValues(t *testing.T) { + for _, jsonOutput := range []bool{false, true} { + t.Run(map[bool]string{false: "table", true: "json"}[jsonOutput], func(t *testing.T) { + client := &stubVariableClient{variables: model.Variables{ + {Key: "DATABASE_PASSWORD", Value: "existing-database-password"}, + }} + printer := &capturePrinter{} + f := &cmdutil.Factory{ + ApiClient: client, + Printer: printer, + Log: zap.NewNop().Sugar(), + } + f.Interactive = false + f.JSON = jsonOutput + + cmd := create.NewCmdCreateVariable(f) + cmd.SetArgs([]string{ + "--id", "service-id", + "--env-id", "environment-id", + "--key", "NEW_SECRET=new-secret-value", + }) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + if err := cmd.Execute(); err != nil { + t.Fatalf("create variable: %v", err) + } + + output := printer.output() + for _, sensitive := range []string{ + "existing-database-password", + "new-secret-value", + "DATABASE_PASSWORD", + } { + if strings.Contains(output, sensitive) { + t.Fatalf("create output exposed existing or sensitive data %q: %s", sensitive, output) + } + } + if !strings.Contains(output, "NEW_SECRET") { + t.Fatalf("create output did not identify the created key: %s", output) + } + if got := client.updated["DATABASE_PASSWORD"]; got != "existing-database-password" { + t.Fatalf("existing variable was not preserved in update payload: %q", got) + } + if got := client.updated["NEW_SECRET"]; got != "new-secret-value" { + t.Fatalf("new variable missing from update payload: %q", got) + } + }) + } +} diff --git a/internal/cmd/variable/delete/delete.go b/internal/cmd/variable/delete/delete.go index cad0b80..2fc9f61 100644 --- a/internal/cmd/variable/delete/delete.go +++ b/internal/cmd/variable/delete/delete.go @@ -6,6 +6,7 @@ import ( "github.com/briandowns/spinner" "github.com/spf13/cobra" + variablevalue "github.com/zeabur/cli/internal/cmd/variable/value" "github.com/zeabur/cli/internal/cmdutil" "github.com/zeabur/cli/internal/util" "github.com/zeabur/cli/pkg/fill" @@ -80,7 +81,7 @@ func runDeleteVariableInteractive(f *cmdutil.Factory, opts *Options) error { selectTable := make([]string, 0, len(varMap)) for k, v := range varMap { keyTable = append(keyTable, k) - selectTable = append(selectTable, fmt.Sprintf("%s = %s", k, v)) + selectTable = append(selectTable, fmt.Sprintf("%s = %s", k, variablevalue.Mask(v))) opts.keys[k] = v } @@ -170,11 +171,11 @@ func runDeleteVariableNonInteractive(f *cmdutil.Factory, opts *Options) error { f.Log.Infof("Successfully deleted variables of service: %s\n", opts.name) - table := make([][]string, 0, len(opts.keys)) - for k, v := range opts.keys { - table = append(table, []string{k, v}) + table := make([][]string, 0, len(opts.deleteKeys)) + for _, key := range opts.deleteKeys { + table = append(table, []string{key}) } - f.Printer.Table([]string{"Key", "Value"}, table) + f.Printer.Table([]string{"Key"}, table) return nil } diff --git a/internal/cmd/variable/delete/delete_test.go b/internal/cmd/variable/delete/delete_test.go new file mode 100644 index 0000000..4168924 --- /dev/null +++ b/internal/cmd/variable/delete/delete_test.go @@ -0,0 +1,157 @@ +package delete_test + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "go.uber.org/zap" + + variabledelete "github.com/zeabur/cli/internal/cmd/variable/delete" + "github.com/zeabur/cli/internal/cmdutil" + "github.com/zeabur/cli/pkg/api" + "github.com/zeabur/cli/pkg/fill" + "github.com/zeabur/cli/pkg/model" + "github.com/zeabur/cli/pkg/prompt" +) + +type stubVariableClient struct { + api.Client + + variables model.Variables + updated map[string]string +} + +func (c *stubVariableClient) ListVariables(context.Context, string, string) (model.Variables, model.Variables, error) { + return c.variables, nil, nil +} + +func (c *stubVariableClient) UpdateVariables(_ context.Context, _, _ string, data map[string]string) (bool, error) { + c.updated = data + return true, nil +} + +type capturePrinter struct { + header []string + rows [][]string + jsonValue any +} + +type stubParamFiller struct { + fill.ParamFiller +} + +func (*stubParamFiller) ServiceByNameWithEnvironment(fill.ServiceByNameWithEnvironmentOptions) (bool, error) { + return false, nil +} + +type capturePrompter struct { + prompt.Prompter + + options []string +} + +func (p *capturePrompter) Select(_, _ string, options []string) (int, error) { + p.options = options + return 0, nil +} + +func (*capturePrompter) Confirm(string, bool) (bool, error) { + return true, nil +} + +func (p *capturePrinter) Table(header []string, rows [][]string) { + p.header = header + p.rows = rows +} + +func (p *capturePrinter) JSON(v any) error { + p.jsonValue = v + return nil +} + +func (p *capturePrinter) output() string { + encoded, _ := json.Marshal(struct { + Header []string + Rows [][]string + JSON any + }{p.header, p.rows, p.jsonValue}) + return string(encoded) +} + +func TestDeleteDoesNotExposeRemainingVariableValues(t *testing.T) { + client := &stubVariableClient{variables: model.Variables{ + {Key: "DELETE_ME", Value: "deleted-secret-value"}, + {Key: "KEEP_ME", Value: "remaining-secret-value"}, + }} + printer := &capturePrinter{} + f := &cmdutil.Factory{ + ApiClient: client, + Printer: printer, + Log: zap.NewNop().Sugar(), + } + f.Interactive = false + + cmd := variabledelete.NewCmdDeleteVariable(f) + cmd.SetArgs([]string{ + "--id", "service-id", + "--env-id", "environment-id", + "--delete-keys", "DELETE_ME", + }) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + if err := cmd.Execute(); err != nil { + t.Fatalf("delete variable: %v", err) + } + + output := printer.output() + for _, sensitive := range []string{"deleted-secret-value", "remaining-secret-value", "KEEP_ME"} { + if strings.Contains(output, sensitive) { + t.Fatalf("delete output exposed remaining or sensitive data %q: %s", sensitive, output) + } + } + if !strings.Contains(output, "DELETE_ME") { + t.Fatalf("delete output did not identify the deleted key: %s", output) + } + if _, ok := client.updated["DELETE_ME"]; ok { + t.Fatal("deleted variable remained in update payload") + } + if got := client.updated["KEEP_ME"]; got != "remaining-secret-value" { + t.Fatalf("remaining variable was not preserved in update payload: %q", got) + } +} + +func TestDeleteInteractiveMasksValuesInSelection(t *testing.T) { + client := &stubVariableClient{variables: model.Variables{ + {Key: "DELETE_ME", Value: "deleted-secret-value"}, + }} + prompter := &capturePrompter{} + f := &cmdutil.Factory{ + ApiClient: client, + Printer: &capturePrinter{}, + Log: zap.NewNop().Sugar(), + ParamFiller: &stubParamFiller{}, + Prompter: prompter, + } + f.Interactive = true + + cmd := variabledelete.NewCmdDeleteVariable(f) + cmd.SetArgs([]string{ + "--id", "service-id", + "--env-id", "environment-id", + }) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + if err := cmd.Execute(); err != nil { + t.Fatalf("delete variable interactively: %v", err) + } + + selection := strings.Join(prompter.options, "\n") + if strings.Contains(selection, "deleted-secret-value") { + t.Fatalf("interactive delete selection exposed variable value: %s", selection) + } + if !strings.Contains(selection, "del***") { + t.Fatalf("interactive delete selection did not contain masked value: %s", selection) + } +} diff --git a/internal/cmd/variable/env/env.go b/internal/cmd/variable/env/env.go index 883615d..9364068 100644 --- a/internal/cmd/variable/env/env.go +++ b/internal/cmd/variable/env/env.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "sort" "github.com/briandowns/spinner" "github.com/hashicorp/go-envparse" @@ -114,22 +115,27 @@ func runUpdateVariableNonInteractive(f *cmdutil.Factory, opts *Options) error { return fmt.Errorf("failed to update variables of service: %s", opts.name) } s.Stop() + updatedKeys := make([]string, 0, len(envMap)) + for key := range envMap { + updatedKeys = append(updatedKeys, key) + } + sort.Strings(updatedKeys) if f.JSON { - out := make([]map[string]string, 0, len(envMap)) - for k, v := range envMap { - out = append(out, map[string]string{"Key": k, "Value": v}) + out := make([]map[string]string, 0, len(updatedKeys)) + for _, key := range updatedKeys { + out = append(out, map[string]string{"Key": key}) } return f.Printer.JSON(out) } f.Log.Infof("Successfully updated variables of service: %s\n\tRestart your service manually to apply the changes.\n", opts.name) - table := make([][]string, 0, len(envMap)) - for k, v := range envMap { - table = append(table, []string{k, v}) + table := make([][]string, 0, len(updatedKeys)) + for _, key := range updatedKeys { + table = append(table, []string{key}) } - f.Printer.Table([]string{"Key", "Value"}, table) + f.Printer.Table([]string{"Key"}, table) return nil } diff --git a/internal/cmd/variable/env/env_test.go b/internal/cmd/variable/env/env_test.go new file mode 100644 index 0000000..e9686ad --- /dev/null +++ b/internal/cmd/variable/env/env_test.go @@ -0,0 +1,96 @@ +package env_test + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "go.uber.org/zap" + + variableenv "github.com/zeabur/cli/internal/cmd/variable/env" + "github.com/zeabur/cli/internal/cmdutil" + "github.com/zeabur/cli/pkg/api" +) + +type stubVariableClient struct { + api.Client + + updated map[string]string +} + +func (c *stubVariableClient) UpdateVariables(_ context.Context, _, _ string, data map[string]string) (bool, error) { + c.updated = data + return true, nil +} + +type capturePrinter struct { + header []string + rows [][]string + jsonValue any +} + +func (p *capturePrinter) Table(header []string, rows [][]string) { + p.header = header + p.rows = rows +} + +func (p *capturePrinter) JSON(v any) error { + p.jsonValue = v + return nil +} + +func (p *capturePrinter) output() string { + encoded, _ := json.Marshal(struct { + Header []string + Rows [][]string + JSON any + }{p.header, p.rows, p.jsonValue}) + return string(encoded) +} + +func TestEnvDoesNotExposeImportedVariableValues(t *testing.T) { + for _, jsonOutput := range []bool{false, true} { + t.Run(map[bool]string{false: "table", true: "json"}[jsonOutput], func(t *testing.T) { + envFile := filepath.Join(t.TempDir(), ".env") + if err := os.WriteFile(envFile, []byte("API_TOKEN=imported-secret-value\n"), 0o600); err != nil { + t.Fatalf("write env fixture: %v", err) + } + + client := &stubVariableClient{} + printer := &capturePrinter{} + f := &cmdutil.Factory{ + ApiClient: client, + Printer: printer, + Log: zap.NewNop().Sugar(), + } + f.Interactive = false + f.JSON = jsonOutput + + cmd := variableenv.NewCmdEnvVariable(f) + cmd.SetArgs([]string{ + "--id", "service-id", + "--env-id", "environment-id", + "--file", envFile, + }) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + if err := cmd.Execute(); err != nil { + t.Fatalf("import variables: %v", err) + } + + output := printer.output() + if strings.Contains(output, "imported-secret-value") { + t.Fatalf("env output exposed imported variable value: %s", output) + } + if !strings.Contains(output, "API_TOKEN") { + t.Fatalf("env output did not identify imported key: %s", output) + } + if got := client.updated["API_TOKEN"]; got != "imported-secret-value" { + t.Fatalf("imported value missing from update payload: %q", got) + } + }) + } +} diff --git a/internal/cmd/variable/list/list.go b/internal/cmd/variable/list/list.go index 509b78d..c4c59b1 100644 --- a/internal/cmd/variable/list/list.go +++ b/internal/cmd/variable/list/list.go @@ -6,15 +6,18 @@ import ( "github.com/briandowns/spinner" "github.com/spf13/cobra" + variablevalue "github.com/zeabur/cli/internal/cmd/variable/value" "github.com/zeabur/cli/internal/cmdutil" "github.com/zeabur/cli/internal/util" "github.com/zeabur/cli/pkg/fill" + "github.com/zeabur/cli/pkg/model" ) type Options struct { id string name string environmentID string + showValues bool } func NewCmdListVariables(f *cmdutil.Factory) *cobra.Command { @@ -33,6 +36,7 @@ func NewCmdListVariables(f *cmdutil.Factory) *cobra.Command { util.AddServiceParam(cmd, &opts.id, &opts.name) util.AddEnvOfServiceParam(cmd, &opts.environmentID) + cmd.Flags().BoolVar(&opts.showValues, "show-values", false, "Show full variable values (may expose secrets)") return cmd } @@ -92,6 +96,10 @@ func runListVariablesNonInteractive(f *cmdutil.Factory, opts *Options) error { return err } s.Stop() + if !opts.showValues { + variableList = maskVariables(variableList) + readonlyVariableList = maskVariables(readonlyVariableList) + } if len(variableList) == 0 && len(readonlyVariableList) == 0 { if f.JSON { @@ -116,3 +124,13 @@ func runListVariablesNonInteractive(f *cmdutil.Factory, opts *Options) error { return nil } + +func maskVariables(variables model.Variables) model.Variables { + masked := make(model.Variables, 0, len(variables)) + for _, variable := range variables { + maskedVariable := *variable + maskedVariable.Value = variablevalue.Mask(variable.Value) + masked = append(masked, &maskedVariable) + } + return masked +} diff --git a/internal/cmd/variable/list/list_test.go b/internal/cmd/variable/list/list_test.go new file mode 100644 index 0000000..1d23662 --- /dev/null +++ b/internal/cmd/variable/list/list_test.go @@ -0,0 +1,115 @@ +package list_test + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "go.uber.org/zap" + + variablelist "github.com/zeabur/cli/internal/cmd/variable/list" + "github.com/zeabur/cli/internal/cmdutil" + "github.com/zeabur/cli/pkg/api" + "github.com/zeabur/cli/pkg/model" +) + +type stubVariableClient struct { + api.Client + + variables model.Variables + readonlyVariables model.Variables +} + +func (c *stubVariableClient) ListVariables(context.Context, string, string) (model.Variables, model.Variables, error) { + return c.variables, c.readonlyVariables, nil +} + +type capturePrinter struct { + tables [][][]string + jsonValue any +} + +func (p *capturePrinter) Table(header []string, rows [][]string) { + table := make([][]string, 0, len(rows)+1) + table = append(table, header) + table = append(table, rows...) + p.tables = append(p.tables, table) +} + +func (p *capturePrinter) JSON(v any) error { + p.jsonValue = v + return nil +} + +func (p *capturePrinter) output() string { + encoded, _ := json.Marshal(struct { + Tables [][][]string + JSON any + }{p.tables, p.jsonValue}) + return string(encoded) +} + +func TestListMasksVariableValuesByDefault(t *testing.T) { + for _, jsonOutput := range []bool{false, true} { + t.Run(map[bool]string{false: "table", true: "json"}[jsonOutput], func(t *testing.T) { + printer := runList(t, jsonOutput) + output := printer.output() + for _, secret := range []string{"service-database-password", "shared-workspace-token"} { + if strings.Contains(output, secret) { + t.Fatalf("list output exposed variable value %q: %s", secret, output) + } + } + if !strings.Contains(output, "ser***") || !strings.Contains(output, "sha***") { + t.Fatalf("list output did not contain masked values: %s", output) + } + }) + } +} + +func TestListShowValuesRequiresExplicitFlag(t *testing.T) { + for _, jsonOutput := range []bool{false, true} { + t.Run(map[bool]string{false: "table", true: "json"}[jsonOutput], func(t *testing.T) { + output := runList(t, jsonOutput, "--show-values").output() + for _, secret := range []string{"service-database-password", "shared-workspace-token"} { + if !strings.Contains(output, secret) { + t.Fatalf("--show-values did not reveal %q: %s", secret, output) + } + } + }) + } +} + +func runList(t *testing.T, jsonOutput bool, extraArgs ...string) *capturePrinter { + t.Helper() + client := &stubVariableClient{ + variables: model.Variables{ + {Key: "DATABASE_PASSWORD", Value: "service-database-password", ServiceID: "service-id"}, + }, + readonlyVariables: model.Variables{ + {Key: "SHARED_TOKEN", Value: "shared-workspace-token", ServiceID: "other-service-id"}, + }, + } + printer := &capturePrinter{} + f := &cmdutil.Factory{ + ApiClient: client, + Printer: printer, + Log: zap.NewNop().Sugar(), + } + f.Interactive = false + f.JSON = jsonOutput + + cmd := variablelist.NewCmdListVariables(f) + args := make([]string, 0, 4+len(extraArgs)) + args = append(args, + "--id", "service-id", + "--env-id", "environment-id", + ) + cmd.SetArgs(append(args, extraArgs...)) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + if err := cmd.Execute(); err != nil { + t.Fatalf("list variables: %v", err) + } + return printer +} diff --git a/internal/cmd/variable/update/update.go b/internal/cmd/variable/update/update.go index 8e8b07f..d6d9d44 100644 --- a/internal/cmd/variable/update/update.go +++ b/internal/cmd/variable/update/update.go @@ -6,6 +6,7 @@ import ( "github.com/briandowns/spinner" "github.com/spf13/cobra" + variablevalue "github.com/zeabur/cli/internal/cmd/variable/value" "github.com/zeabur/cli/internal/cmdutil" "github.com/zeabur/cli/internal/util" "github.com/zeabur/cli/pkg/fill" @@ -21,15 +22,6 @@ type Options struct { inputDone bool } -// maskValue masks a variable value for display, showing only the first 3 -// characters followed by asterisks. Short values are fully masked. -func maskValue(v string) string { - if len(v) <= 3 { - return "***" - } - return v[:3] + "***" -} - func NewCmdUpdateVariable(f *cmdutil.Factory) *cobra.Command { opts := &Options{} @@ -86,7 +78,7 @@ func runUpdateVariableInteractive(f *cmdutil.Factory, opts *Options) error { selectTable := make([]string, 0, len(varMap)) for k, v := range varMap { keyTable = append(keyTable, k) - selectTable = append(selectTable, fmt.Sprintf("%s = %s", k, maskValue(v))) + selectTable = append(selectTable, fmt.Sprintf("%s = %s", k, variablevalue.Mask(v))) opts.keys[k] = v } diff --git a/internal/cmd/variable/value/value.go b/internal/cmd/variable/value/value.go new file mode 100644 index 0000000..af9a865 --- /dev/null +++ b/internal/cmd/variable/value/value.go @@ -0,0 +1,13 @@ +package value + +const visiblePrefixLength = 3 + +// Mask hides a variable value for display while preserving the short prefix +// used by the existing interactive variable-update flow. +func Mask(raw string) string { + runes := []rune(raw) + if len(runes) <= visiblePrefixLength { + return "***" + } + return string(runes[:visiblePrefixLength]) + "***" +} diff --git a/internal/cmd/variable/value/value_test.go b/internal/cmd/variable/value/value_test.go new file mode 100644 index 0000000..adde69c --- /dev/null +++ b/internal/cmd/variable/value/value_test.go @@ -0,0 +1,28 @@ +package value_test + +import ( + "testing" + + "github.com/zeabur/cli/internal/cmd/variable/value" +) + +func TestMask(t *testing.T) { + tests := []struct { + name string + raw string + want string + }{ + {name: "empty", raw: "", want: "***"}, + {name: "short", raw: "ab", want: "***"}, + {name: "long", raw: "secret", want: "sec***"}, + {name: "unicode", raw: "密碼內容", want: "密碼內***"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := value.Mask(tt.raw); got != tt.want { + t.Fatalf("Mask(%q) = %q, want %q", tt.raw, got, tt.want) + } + }) + } +}