From 6669afe04271b31049509dbdf6200bf322e94076 Mon Sep 17 00:00:00 2001 From: jadrol Date: Tue, 11 Aug 2026 11:37:14 +0200 Subject: [PATCH] redesign write command: make it interactive to not leak secret into bash history --- README.md | 24 ++++++++++--- backend/backend.go | 23 +++++++++++- backend/backend_test.go | 9 +++++ cmd/secret_input.go | 62 ++++++++++++++++++++++++++++++++ cmd/secret_input_test.go | 41 +++++++++++++++++++++ cmd/write.go | 77 +++++++++++++++++++++++++++++++--------- go.mod | 1 + go.sum | 2 ++ test/bats/tests.bats | 31 ++++++++++++---- 9 files changed, 241 insertions(+), 29 deletions(-) create mode 100644 cmd/secret_input.go create mode 100644 cmd/secret_input_test.go diff --git a/README.md b/README.md index 013db28..fb52486 100644 --- a/README.md +++ b/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 ``` diff --git a/backend/backend.go b/backend/backend.go index 90f6d65..947414e 100644 --- a/backend/backend.go +++ b/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 +} diff --git a/backend/backend_test.go b/backend/backend_test.go index 027e9c6..2828dc8 100644 --- a/backend/backend_test.go +++ b/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{ diff --git a/cmd/secret_input.go b/cmd/secret_input.go new file mode 100644 index 0000000..89c0a4c --- /dev/null +++ b/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 +} diff --git a/cmd/secret_input_test.go b/cmd/secret_input_test.go new file mode 100644 index 0000000..3f12596 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/cmd/write.go b/cmd/write.go index 2d6e1a5..27c733b 100644 --- a/cmd/write.go +++ b/cmd/write.go @@ -17,6 +17,7 @@ package cmd import ( "errors" "fmt" + "unicode/utf8" "github.com/AirHelp/treasury/client" "github.com/spf13/cobra" @@ -24,10 +25,23 @@ import ( // 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, + }) +} diff --git a/go.mod b/go.mod index 3716a48..3f4513e 100644 --- a/go.mod +++ b/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 ( diff --git a/go.sum b/go.sum index 7fa4707..fb75b3c 100644 --- a/go.sum +++ b/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= diff --git a/test/bats/tests.bats b/test/bats/tests.bats index 902a2b8..c593137 100644 --- a/test/bats/tests.bats +++ b/test/bats/tests.bats @@ -26,37 +26,56 @@ invalid_aws_region=us-west-1 # } @test "write" { - run $treasury write development/treasury/key1 secret1 + run bash -c "echo secret1 | $treasury write development/treasury/key1" [ $status -eq 0 ] [[ ${lines[0]} =~ "Success!" ]] + [[ ${lines[0]} =~ "7 characters" ]] } @test "write second" { - run $treasury write development/treasury/key2 secret2 + run bash -c "echo secret2 | $treasury write development/treasury/key2" [ $status -eq 0 ] [[ ${lines[0]} =~ "Success!" ]] } @test "write second not forced" { - run $treasury write development/treasury/key2 secret2 + run bash -c "echo secret2 | $treasury write development/treasury/key2" [ $status -eq 0 ] [[ ${lines[0]} =~ "Success!" ]] } @test "write second forced" { - run $treasury write development/treasury/key2 secret2 --force + run bash -c "echo secret2 | $treasury write development/treasury/key2 --force" [ $status -eq 0 ] [[ ${lines[0]} =~ "Success!" ]] } +@test "write secret without trailing newline on stdin" { + run bash -c "printf %s secret2 | $treasury write development/treasury/key2 --force" + [ $status -eq 0 ] + [[ ${lines[0]} =~ "7 characters" ]] +} + @test "write-wrong-data" { - run $treasury write test secret1 + run bash -c "echo secret1 | $treasury write test" + [ $status -eq 255 ] + [[ ${lines[0]} =~ "Error" ]] +} + +@test "write-secret-as-argument-is-rejected" { + run $treasury write development/treasury/key1 secret1 + [ $status -eq 255 ] + [[ ${lines[0]} =~ "Error" ]] +} + +@test "write-empty-secret-is-rejected" { + run bash -c "echo | $treasury write development/treasury/key1" [ $status -eq 255 ] [[ ${lines[0]} =~ "Error" ]] } @test "write random key" { - run $treasury write development/application/"${randomKey}" secret + run bash -c "echo secret | $treasury write development/application/${randomKey}" [ $status -eq 0 ] [[ ${lines[0]} =~ "Success!" ]] }