diff --git a/README.md b/README.md index 1e2b834..9e9fd61 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,13 @@ contain `//`, or have `..` as a path segment. #### `secret get [--version ]` -Retrieves and outputs a secret value to stdout. +Retrieves and outputs a secret value to stdout. A failed write of the value, +such as to a file on a full disk, prints the error and exits 1, with two +exceptions. If the program reading from a pipe has already exited, as in +`secret get s | true`, the broken-pipe signal (SIGPIPE) ends `secret` with no +message and the shell reports exit status 141. If the shell closed stdout before +`secret` started (`secret get s >&-`), the value is written to `/dev/null` and +`secret get` exits 0. - `--version, -v`: Get a specific version (default: current) diff --git a/internal/cli/secrets.go b/internal/cli/secrets.go index 1728e2b..6598ff9 100644 --- a/internal/cli/secrets.go +++ b/internal/cli/secrets.go @@ -443,7 +443,10 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error { defer value.Destroy() // Write the value straight from locked memory, with no trailing newline - _, _ = cmd.OutOrStdout().Write(value.Bytes()) + _, err = cmd.OutOrStdout().Write(value.Bytes()) + if err != nil { + return fmt.Errorf("failed to write secret value: %w", err) + } return nil } @@ -478,7 +481,10 @@ func (cli *Instance) GetSecretWithVersion( secret.Debug("Got secret value", "valueLength", value.Size()) // Write the value straight from locked memory, with no trailing newline - _, _ = cmd.OutOrStdout().Write(value.Bytes()) + _, err = cmd.OutOrStdout().Write(value.Bytes()) + if err != nil { + return fmt.Errorf("failed to write secret value: %w", err) + } secret.Debug("Printed value to stdout") diff --git a/internal/cli/version_test.go b/internal/cli/version_test.go index 018b5d3..8904da6 100644 --- a/internal/cli/version_test.go +++ b/internal/cli/version_test.go @@ -8,6 +8,7 @@ // - TestPromoteNonExistentVersion: Tests error handling for invalid promotion // - TestGetSecretWithVersion: Tests `secret get --version` flag functionality // - TestGetSecretWritesBinaryValue: Tests `secret get` output of binary values +// - TestGetSecretReturnsWriteError: Tests `secret get` when stdout fails // - TestVersionCommandStructure: Tests command structure and help text // - TestListVersionsEmptyOutput: Tests edge case with no versions // @@ -20,6 +21,7 @@ package cli import ( "bytes" + "os" "path/filepath" "strings" "testing" @@ -360,6 +362,45 @@ func TestGetSecretWritesBinaryValue(t *testing.T) { assert.Equal(t, value, buf.Bytes()) } +// TestGetSecretReturnsWriteError checks that `secret get`, with and without +// a version, returns the error of a failed write of the value. +func TestGetSecretReturnsWriteError(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + cli := NewCLIInstanceWithStateDir(fs, testStateDir) + cli.Mnemonic = testMnemonicBuffer(t) + + setupTestVault(t, fs) + + vlt, err := vault.GetCurrentVault(fs, testStateDir) + require.NoError(t, err) + + addTestSecret(t, vlt, []byte("value"), false) + + vaultDir, err := vlt.GetDirectory() + require.NoError(t, err) + + versions, err := secret.ListVersions(fs, + filepath.Join(vaultDir, "secrets.d", "test%secret")) + require.NoError(t, err) + require.Len(t, versions, 1) + + // Stdout is a file closed before the value is written to it + out, err := os.Create(filepath.Join(t.TempDir(), "stdout")) + require.NoError(t, err) + require.NoError(t, out.Close()) + + cmd := newRootCmd() + cmd.SetOut(out) + + err = cli.GetSecret(cmd, "test/secret") + require.ErrorIs(t, err, os.ErrClosed) + + err = cli.GetSecretWithVersion(cmd, "test/secret", versions[0]) + require.ErrorIs(t, err, os.ErrClosed) +} + //nolint:paralleltest // reads process environment to determine the state dir func TestVersionCommandStructure(t *testing.T) { // Test that version commands are properly structured