fix(variable): mask values by default (PLA-2368)

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
 }

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)
+			}
+		})
+	}
+}

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
 }

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)
+	}
+}

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
 }