A secret too large for the locked-memory limit fails with an error, not a panic (closes #145)

secret add, import and generate secret crashed with memguard's panic once a
value outgrew the locked-memory limit, and because the value was held twice,
half the usual 8 MiB limit was enough. secret now raises the limit to the hard
limit at startup. Each of the three commands refuses, before its first locked
allocation, a limit that leaves fewer than 8 pages for its keys and buffers.
The value is read a page at a time into a memguard.Stream, which keeps it
encrypted, and copied once into locked memory after its size is checked
against 100 MiB and the limit. It leaves the stream through Next, not Read:
Read re-encrypts a leftover piece, and memguard hangs if that runs out of
locked memory.

Model: opus-5-5

README.md

@@ -90,8 +90,7 @@ remove things pass `--force`.
 #### `secret init`
 
 Initializes the secret manager with a default vault. Prompts for a BIP39
-mnemonic phrase and creates the initial directory structure. When the vault
-`default` already exists, it fails before asking for the mnemonic or passphrase.
+mnemonic phrase and creates the initial directory structure.
 
 **Environment Variables:**
 
@@ -106,8 +105,7 @@ Lists all available vaults. The current vault is marked.
 
 #### `secret vault create <name>`
 
-Creates a new vault with the specified name. When a vault of that name already
-exists, it fails before asking for the mnemonic or passphrase.
+Creates a new vault with the specified name.
 
 **Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
 are allowed, and a name must not be empty, `.` or `..`.
@@ -155,13 +153,7 @@ contain `//`, or have `..` as a path segment.
 
 #### `secret get <secret-name> [--version <version>]`
 
-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.
+Retrieves and outputs a secret value to stdout.
 
 - `--version, -v`: Get a specific version (default: current)
 

internal/cli/create_vault_test.go

@@ -222,51 +222,12 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
-	require.Equal(t, "Error: failed to read mnemonic: there is no terminal to "+
-		"ask on. Please set the SB_SECRET_MNEMONIC environment variable or run "+
+	require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+
+		"Error: failed to read mnemonic: there is no terminal to ask on. "+
+		"Please set the SB_SECRET_MNEMONIC environment variable or run "+
 		"interactively\n", string(output))
 }
 
-// TestCreateExistingVaultAsksForNothing is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/151: `secret init` and `secret
-// vault create` asked for the mnemonic and the unlocker passphrase before
-// refusing a vault that already exists, and `secret init` printed
-// "Initialized secrets manager at" first. Each runs here as the built binary,
-// in a new session, which has no terminal, and with neither
-// SB_SECRET_MNEMONIC nor SB_UNLOCK_PASSPHRASE set, so asking for either
-// fails. Each must fail saying only that the vault exists.
-func TestCreateExistingVaultAsksForNothing(t *testing.T) {
-	t.Parallel()
-
-	stateDir := t.TempDir()
-	_, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default", nil, nil)
-	require.NoError(t, err)
-
-	// run runs the built binary with args, which must fail, and returns what
-	// it printed
-	run := func(args ...string) string {
-		//nolint:gosec // G204: test executes the freshly built secret binary
-		cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), args...)
-		cmd.Env = []string{
-			secret.EnvStateDir + "=" + stateDir,
-			"PATH=" + os.Getenv("PATH"),
-			"HOME=" + os.Getenv("HOME"),
-		}
-		cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
-
-		output, err := cmd.CombinedOutput()
-		require.Error(t, err, "secret %v", args)
-
-		return string(output)
-	}
-
-	require.Equal(t,
-		"Error: failed to create default vault: vault default already exists\n",
-		run("init"))
-	require.Equal(t, "Error: vault default already exists\n",
-		run("vault", "create", "default"))
-}
-
 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for
 // https://git.eeqj.de/sneak/secret/issues/105: `secret init` or `secret vault
 // create` killed after the passphrase prompt but before the unlocker was

internal/cli/init.go

@@ -17,9 +17,6 @@ import (
 // errPassphraseMismatch is returned when passphrase confirmation fails
 var errPassphraseMismatch = errors.New("passphrases do not match")
 
-// defaultVaultName is the name of the vault `secret init` creates
-const defaultVaultName = "default"
-
 // NewInitCmd creates the init command
 func NewInitCmd() *cobra.Command {
 	return &cobra.Command{
@@ -95,10 +92,8 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to create state directory: %w", err)
 	}
 
-	// Refuse an existing default vault before asking for anything
-	err = vault.CheckNewVaultName(cli.fs, cli.stateDir, defaultVaultName)
-	if err != nil {
-		return fmt.Errorf("failed to create default vault: %w", err)
+	if cmd != nil {
+		cmd.Printf("Initialized secrets manager at: %s\n", stateDir)
 	}
 
 	// Prompt for mnemonic
@@ -138,7 +133,7 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 	// Create the default vault with its passphrase unlocker
 	secret.Debug("Creating default vault")
 
-	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, defaultVaultName,
+	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, "default",
 		mnemonic, passphraseBuffer)
 	if err != nil {
 		secret.Debug("Failed to create default vault", "error", err)
@@ -157,7 +152,6 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 	}
 
 	if cmd != nil {
-		cmd.Printf("Initialized secrets manager at: %s\n", stateDir)
 		cmd.Printf("\nDefault vault created and configured\n")
 		cmd.Printf("Long-term public key: %s\n", ltIdentity.Recipient().String())
 		cmd.Printf("Unlocker ID: %s\n", unlocker.GetID())

internal/cli/secrets.go

@@ -463,10 +463,7 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	defer value.Destroy()
 
 	// Write the value straight from locked memory, with no trailing newline
-	_, err = cmd.OutOrStdout().Write(value.Bytes())
-	if err != nil {
-		return fmt.Errorf("failed to write secret value: %w", err)
-	}
+	_, _ = cmd.OutOrStdout().Write(value.Bytes())
 
 	return nil
 }
@@ -501,10 +498,7 @@ func (cli *Instance) GetSecretWithVersion(
 	secret.Debug("Got secret value", "valueLength", value.Size())
 
 	// Write the value straight from locked memory, with no trailing newline
-	_, err = cmd.OutOrStdout().Write(value.Bytes())
-	if err != nil {
-		return fmt.Errorf("failed to write secret value: %w", err)
-	}
+	_, _ = cmd.OutOrStdout().Write(value.Bytes())
 
 	secret.Debug("Printed value to stdout")
 

internal/cli/vault.go

@@ -267,12 +267,6 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 	}
 	defer release()
 
-	// Refuse an invalid name or an existing vault before asking for anything
-	err = vault.CheckNewVaultName(cli.fs, cli.stateDir, name)
-	if err != nil {
-		return err
-	}
-
 	mnemonic, cleanupMnemonic, err := cli.promptMnemonic()
 	if err != nil {
 		return err

internal/cli/version_test.go

@@ -8,7 +8,6 @@
 // - 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
 //
@@ -21,7 +20,6 @@ package cli
 
 import (
 	"bytes"
-	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -362,45 +360,6 @@ 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

internal/vault/management.go

@@ -212,40 +212,16 @@ func processMnemonicForVault(
 	return ltIdentity, derivationIndex, publicKeyHash, familyHash, nil
 }
 
-// CheckNewVaultName returns an error when name cannot be the name of a new
-// vault: one wrapping ErrInvalidVaultName when it is not a valid vault name,
-// and one wrapping ErrVaultExists when that vault already exists. Creating an
-// existing vault again would replace its keys, and its secrets could no
-// longer be decrypted. CreateVault checks this before writing anything; the
-// commands that create a vault also check it before they ask for the
-// mnemonic or passphrase, holding the state directory lock.
-func CheckNewVaultName(fs afero.Fs, stateDir string, name string) error {
-	err := ValidateVaultName(name)
-	if err != nil {
-		return err
-	}
-
-	exists, err := afero.DirExists(fs, filepath.Join(stateDir, "vaults.d", name))
-	if err != nil {
-		return fmt.Errorf("failed to check if vault exists: %w", err)
-	}
-
-	if exists {
-		return fmt.Errorf("vault %s %w", name, ErrVaultExists)
-	}
-
-	return nil
-}
-
 // CreateVault creates a new vault and selects it as the current vault. When
 // mnemonic is not nil, the vault's long-term key is derived from it, and the
 // returned vault has it as its Mnemonic; when it is nil, the vault has no
 // long-term key until one is imported. When passphrase is not nil, the vault
 // gets a passphrase unlocker protected by it, as its current unlocker; that
-// needs a mnemonic. It refuses an invalid name or a vault that already exists,
-// as CheckNewVaultName does, before writing anything. The commands that call
-// it hold the state directory lock, so no other command can create the vault
-// between the check and the writes.
+// needs a mnemonic. It refuses a vault that already exists before writing
+// anything: creating it again would replace its keys, and its secrets could
+// no longer be decrypted. The commands that call it hold the state directory
+// lock, so no other command can create the vault between the check and the
+// writes.
 //
 // The vault is written whole into a temporary directory, which is renamed
 // into vaults.d only once complete, and only then selected: a crash at any
@@ -257,17 +233,30 @@ func CreateVault(
 ) (*Vault, error) {
 	secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
 
-	err := CheckNewVaultName(fs, stateDir, name)
+	err := ValidateVaultName(name)
 	if err != nil {
+		secret.Debug("Invalid vault name provided", "vault_name", name)
+
 		return nil, err
 	}
 
+	secret.Debug("Vault name validation passed", "vault_name", name)
+
+	vaultDir := filepath.Join(stateDir, "vaults.d", name)
+
+	exists, err := afero.DirExists(fs, vaultDir)
+	if err != nil {
+		return nil, fmt.Errorf("failed to check if vault exists: %w", err)
+	}
+
+	if exists {
+		return nil, fmt.Errorf("vault %s %w", name, ErrVaultExists)
+	}
+
 	if passphrase != nil && mnemonic == nil {
 		return nil, fmt.Errorf("vault %s %w", name, ErrUnlockerWithoutMnemonic)
 	}
 
-	vaultDir := filepath.Join(stateDir, "vaults.d", name)
-
 	secret.Debug("Creating vault directory structure", "vault_dir", vaultDir)
 
 	err = secret.WriteDir(fs, vaultDir, func(dir string) error {