secret get fails when it cannot write the value (closes #155)
check / check (push) Waiting to run

GetSecret and GetSecretWithVersion discarded the result of writing the
value to stdout, so a full disk or a failing stdout left the caller with
an empty or cut-short file and exit status 0. The write error is now
returned, so the command prints it and exits 1.

Deviation: a stdout the shell closed before `secret` started still exits
0, because the Go runtime opens /dev/null in its place and the write
there succeeds.

Model: opus-5-5
This commit is contained in:
2026-10-14 20:33:37 +00:00
parent fe94e99801
commit 81ffe12c84
4 changed files with 59 additions and 3 deletions
+8 -2
View File
@@ -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")
+41
View File
@@ -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