secret get fails when it cannot write the value (closes #155)
check / check (push) Waiting to run
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:
@@ -142,7 +142,8 @@ contain `//`, or have `..` as a path segment.
|
||||
|
||||
#### `secret get <secret-name> [--version <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)
|
||||
|
||||
|
||||
@@ -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: Removing the current unlocker no longer leaves the vault unable to
|
||||
open through the unlockers it still has
|
||||
(https://git.eeqj.de/sneak/secret/issues/137). `current-unlocker` went on
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user