Redesign write command: make it interactive to not leak secret into bash history

README.md

@@ -117,11 +117,24 @@ brew install treasury
 ## CLI Usage
 
 ### Write secret
+
+The secret value is never passed as a command line argument, so it does not leak into the shell history nor into the process list. Treasury asks for it and hides the input:
+```
+> treasury write development/webapp/cockpit_api_pass
+Please paste your secret:
+Success! Data written to: development/webapp/cockpit_api_pass (19 characters)
+```
+
+In scripts and CI, where there is no terminal, the secret is read from the standard input:
 ```
-> treasury write development/webapp/cockpit_api_pass superSecretPassword
-Success! Data written to: development/webapp/cockpit_api_pass
+> echo "${SECRET}" | treasury write development/webapp/cockpit_api_pass
+Success! Data written to: development/webapp/cockpit_api_pass (19 characters)
 ```
 
+The single trailing newline that `echo` adds is stripped, so there is no need for `echo -n` (which is not portable between shells anyway). Any other whitespace is treated as a part of the secret. If the secret has to end with a newline, use `printf '%s\n' "${SECRET}"` or write it from a file with `--file`.
+
+Do not put the secret itself in the pipe - `echo thisIsASecret | treasury write KEY` leaks into the shell history exactly like the old `treasury write KEY thisIsASecret` did. Pipe a variable or another command (`echo "${SECRET}" | ...`), and type secrets by hand only into the interactive prompt.
+
 Note: if secret value is equal to existing one, write is skipped. `--force` flag can be used to overwrite.
 
 ### Write file content
@@ -524,13 +537,14 @@ You can now use the treasure as a user vault with minimal policy change. Includi
 * Write user/marcin.janas/phone
 
 ```bash
-$ treasury write write user/firstname.lastname/phone +48987654321
-Success! Data written to:  user/firstname.lastname/phone
+$ treasury write user/firstname.lastname/phone
+Please paste your secret:
+Success! Data written to: user/firstname.lastname/phone (13 characters)
 ```
 
 * Read user/firstname.lastname/phone
 ```bash
-$ treasury write read user/firstname.lastname/phone
+$ treasury read user/firstname.lastname/phone
 +48987654321
 ```
 

backend/backend.go

@@ -1,11 +1,14 @@
 package backend
 
 import (
+	"context"
 	"errors"
+	"fmt"
 
 	"github.com/AirHelp/treasury/backend/s3"
 	"github.com/AirHelp/treasury/backend/ssm"
 	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/config"
 )
 
 const (
@@ -36,7 +39,25 @@ func New(options Options) (API, error) {
 	case s3Name:
 		return s3.New(options.Region, options.S3BucketName)
 	case ssmName:
-		return ssm.New(options.AWSConfig)
+		awsConfig := options.AWSConfig
+		if !isConfigured(awsConfig) {
+			var err error
+			awsConfig, err = config.LoadDefaultConfig(context.Background(), config.WithRegion(options.Region))
+			if err != nil {
+				return nil, errors.Join(
+					fmt.Errorf("unable to load SDK config with region %s", options.Region),
+					err,
+				)
+			}
+		}
+		return ssm.New(awsConfig)
 	}
 	return nil, errors.New("invalid backend")
 }
+
+// isConfigured tells whether the caller provided its own AWS configuration.
+// If it did not, the ambient one (environment, shared config, instance role)
+// is loaded, the same way the S3 backend does it.
+func isConfigured(awsConfig aws.Config) bool {
+	return awsConfig.Region != "" || awsConfig.Credentials != nil
+}

backend/backend_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/AirHelp/treasury/backend"
+	"github.com/aws/aws-sdk-go-v2/aws"
 )
 
 func TestNew(t *testing.T) {
@@ -32,6 +33,14 @@ func TestNew(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "ssm backend with AWS config provided by the caller",
+			args: backend.Options{
+				Backend:   "ssm",
+				AWSConfig: aws.Config{Region: "eu-west-1"},
+			},
+			wantErr: false,
+		},
 		{
 			name: "s3 backend without Bucket",
 			args: backend.Options{

cmd/secret_input.go

@@ -0,0 +1,62 @@
+package cmd
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/spf13/cobra"
+	"golang.org/x/term"
+)
+
+const secretPrompt = "Please paste your secret: "
+
+var errEmptySecret = errors.New("empty secret, nothing was written")
+
+// readSecret gets the secret value without exposing it in the command line.
+// On a terminal it prompts for the secret with echo disabled, otherwise it
+// reads the secret from the piped stdin, which keeps scripts and CI working.
+func readSecret(cmd *cobra.Command) (string, error) {
+	in := cmd.InOrStdin()
+
+	//nolint:gosec // G115: a file descriptor always fits in an int
+	if file, ok := in.(*os.File); ok && term.IsTerminal(int(file.Fd())) {
+		return readSecretFromTerminal(cmd, file)
+	}
+
+	data, err := io.ReadAll(in)
+	if err != nil {
+		return "", err
+	}
+
+	return validateSecret(trimEOL(string(data)))
+}
+
+func readSecretFromTerminal(cmd *cobra.Command, file *os.File) (string, error) {
+	out := cmd.OutOrStdout()
+
+	_, _ = fmt.Fprint(out, secretPrompt)
+	//nolint:gosec // G115: a file descriptor always fits in an int
+	secret, err := term.ReadPassword(int(file.Fd()))
+	_, _ = fmt.Fprintln(out)
+	if err != nil {
+		return "", err
+	}
+
+	return validateSecret(string(secret))
+}
+
+// trimEOL removes a single trailing line ending, the one shells and editors add
+// to piped input. Any other whitespace is a part of the secret.
+func trimEOL(secret string) string {
+	return strings.TrimSuffix(strings.TrimSuffix(secret, "\n"), "\r")
+}
+
+func validateSecret(secret string) (string, error) {
+	if secret == "" {
+		return "", errEmptySecret
+	}
+	return secret, nil
+}

cmd/secret_input_test.go

@@ -0,0 +1,41 @@
+package cmd
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+func TestReadSecretFromStdin(t *testing.T) {
+	testCases := []struct {
+		name     string
+		input    string
+		expected string
+		err      error
+	}{
+		{name: "plain secret", input: "superSecretPassword", expected: "superSecretPassword"},
+		{name: "trailing newline added by echo", input: "superSecretPassword\n", expected: "superSecretPassword"},
+		{name: "trailing windows newline", input: "superSecretPassword\r\n", expected: "superSecretPassword"},
+		{name: "multiline secret", input: "-----BEGIN KEY-----\nabc\n-----END KEY-----\n", expected: "-----BEGIN KEY-----\nabc\n-----END KEY-----"},
+		{name: "significant whitespace is kept", input: "secret with space \n", expected: "secret with space "},
+		{name: "empty input", input: "", err: errEmptySecret},
+		{name: "only newline", input: "\n", err: errEmptySecret},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			cmd := &cobra.Command{}
+			cmd.SetIn(strings.NewReader(testCase.input))
+
+			secret, err := readSecret(cmd)
+			if !errors.Is(err, testCase.err) {
+				t.Fatalf("expected error %v, got %v", testCase.err, err)
+			}
+			if secret != testCase.expected {
+				t.Errorf("expected secret %q, got %q", testCase.expected, secret)
+			}
+		})
+	}
+}

cmd/write.go

@@ -17,17 +17,31 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"unicode/utf8"
 
 	"github.com/AirHelp/treasury/client"
 	"github.com/spf13/cobra"
 )
 
 // writeCmd represents the write command
 var writeCmd = &cobra.Command{
-	Use:   "write ENVIRONMENT/APPLICATION/KEY SECRET or write user/USER.NAME/KEY SECRET",
+	Use:   "write ENVIRONMENT/APPLICATION/KEY or write user/USER.NAME/KEY",
 	Short: "Write secrets into Treasury",
-	Long:  `Write sends data into Treasury at the given key (path).`,
-	RunE:  write,
+	Long: `Write sends data into Treasury at the given key (path).
+
+The secret value is never given as a command line argument, so it does not end
+up in the shell history nor in the process list. When run in a terminal treasury
+asks for the secret and hides what is typed, otherwise the secret is read from
+the standard input:
+
+  treasury write development/webapp/cockpit_api_pass
+  echo "${SECRET}" | treasury write development/webapp/cockpit_api_pass
+
+The trailing newline added by echo is stripped. Pipe a variable or a command,
+never the secret itself - that would leak into the shell history again.
+
+With --file the second argument is a path to a file with the content to store.`,
+	RunE: write,
 }
 
 func init() {
@@ -38,11 +52,6 @@ func init() {
 }
 
 func write(cmd *cobra.Command, args []string) error {
-	if len(args) != 2 {
-		return errors.New("missing key and value to write")
-	}
-	key := args[0]
-	value := args[1]
 	force, err := cmd.Flags().GetBool("force")
 	if err != nil {
 		return err
@@ -53,24 +62,58 @@ func write(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	treasury, err := client.New(&client.Options{
-		Region:       s3Region,
-		S3BucketName: treasuryS3,
-	})
+	if file {
+		return writeFile(cmd, args, force)
+	}
+
+	if len(args) == 0 {
+		return errors.New("missing key to write")
+	}
+	if len(args) > 1 {
+		return errors.New("too many arguments, the secret is not passed as an argument anymore - treasury asks for it or reads it from the standard input")
+	}
+	key := args[0]
+
+	secret, err := readSecret(cmd)
 	if err != nil {
 		return err
 	}
 
-	if file {
-		err = treasury.WriteFile(key, value, force)
-	} else {
-		err = treasury.Write(key, value, force)
+	treasury, err := newClient()
+	if err != nil {
+		return err
+	}
+
+	if err := treasury.Write(key, secret, force); err != nil {
+		return err
+	}
+
+	_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Success! Data written to: %s (%d characters)\n", key, utf8.RuneCountInString(secret))
+	return nil
+}
+
+func writeFile(cmd *cobra.Command, args []string, force bool) error {
+	if len(args) != 2 {
+		return errors.New("missing key and file path to write")
 	}
+	key, filePath := args[0], args[1]
 
+	treasury, err := newClient()
 	if err != nil {
 		return err
 	}
 
-	fmt.Println("Success! Data written to: ", key)
+	if err := treasury.WriteFile(key, filePath, force); err != nil {
+		return err
+	}
+
+	_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Success! Data written to: %s\n", key)
 	return nil
 }
+
+func newClient() (*client.Client, error) {
+	return client.New(&client.Options{
+		Region:       s3Region,
+		S3BucketName: treasuryS3,
+	})
+}

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.29.0
 	github.com/onsi/gomega v1.41.0
 	github.com/spf13/cobra v1.10.2
+	golang.org/x/term v0.43.0
 )
 
 require (

go.sum

@@ -119,6 +119,8 @@ golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY=
 golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4=
+golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc=