diff --git a/README.md b/README.md index 4651c36..2adaf78 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,8 @@ 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. If the value cannot be written, +as when stdout is a file on a full disk, it prints the error and exits 1. - `--version, -v`: Get a specific version (default: current) diff --git a/TODO.md b/TODO.md index 857a05f..1fd9139 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,14 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-14: `secret get` fails when it cannot write the value to stdout + (https://git.eeqj.de/sneak/secret/issues/155). The write's error was ignored, + so a script that sent the value to a file on a full disk saw `secret get` + succeed and was left with an empty or cut-short file. That error is now the + command's error, and `secret get` exits 1. A stdout the shell closed (`>&-`) + still gives exit 0, because the Go runtime opens `/dev/null` in its place + before `secret` starts and the write there succeeds. + - 2026-10-14: `secret generate mnemonic`, `secret vault list` and `secret unlocker list` print their output to stdout, as `secret list` does, so it can be piped or captured (https://git.eeqj.de/sneak/secret/issues/138). 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