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,7 +90,8 @@ 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.
+mnemonic phrase and creates the initial directory structure. When the vault
+`default` already exists, it fails before asking for the mnemonic or passphrase.
 
 **Environment Variables:**
 
@@ -105,7 +106,8 @@ Lists all available vaults. The current vault is marked.
 
 #### `secret vault create <name>`
 
-Creates a new vault with the specified 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.
 
 **Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
 are allowed, and a name must not be empty, `.` or `..`.
@@ -127,6 +129,17 @@ one.
 
 ### Secret Management
 
+#### Secret Size
+
+While `secret` works on a secret's value it keeps the value in locked memory, so
+the value counts against the locked-memory limit (`ulimit -l`). `secret` raises
+that limit to the hard limit when it starts. `secret add`, `secret import` and
+`secret generate secret` refuse a value that does not fit in the limit with 8
+memory pages to spare (32 KiB with 4 KiB pages), with an error that names the
+limit. Linux's usual limit of 8 MiB allows a value of nearly 8 MiB; a larger one
+needs a higher hard limit, such as `memlock` in `/etc/security/limits.conf`. No
+value may be over 100 MiB, whatever the limit.
+
 #### `secret add <secret-name> [--force]`
 
 Adds a secret to the current vault. Reads the secret value from stdin.
@@ -142,7 +155,13 @@ 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. 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)
 

internal/cli/create_vault_test.go

@@ -222,12 +222,51 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
-	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 "+
+	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 "+
 		"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/generate.go

@@ -72,6 +72,12 @@ func newGenerateSecretCmd() *cobra.Command {
 				return fmt.Errorf("failed to initialize CLI: %w", err)
 			}
 
+			// Before readSecretEnv, which makes the first locked allocation
+			err = checkLockedMemoryLimit(0)
+			if err != nil {
+				return err
+			}
+
 			destroySecrets := cli.readSecretEnv()
 			defer destroySecrets()
 
@@ -133,10 +139,16 @@ func (cli *Instance) GenerateSecret(
 		return errLengthTooSmall
 	}
 
-	var (
-		secretValue string
-		err         error
-	)
+	if length > maxSecretSize {
+		return errSecretTooLarge
+	}
+
+	err := checkLockedMemoryLimit(length)
+	if err != nil {
+		return err
+	}
+
+	var secretValue string
 
 	switch secretType {
 	case "base58":

internal/cli/init.go

@@ -17,6 +17,9 @@ 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{
@@ -92,8 +95,10 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to create state directory: %w", err)
 	}
 
-	if cmd != nil {
-		cmd.Printf("Initialized secrets manager at: %s\n", stateDir)
+	// 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)
 	}
 
 	// Prompt for mnemonic
@@ -133,7 +138,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, "default",
+	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, defaultVaultName,
 		mnemonic, passphraseBuffer)
 	if err != nil {
 		secret.Debug("Failed to create default vault", "error", err)
@@ -152,6 +157,7 @@ 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/root.go

@@ -14,6 +14,8 @@ import (
 // every memguard buffer before it returns, so the caller must do nothing
 // but exit with the code.
 func Entry() int {
+	raiseLockedMemoryLimit()
+
 	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
 	// and exits with status 1. The passphrase prompt turns echo off on the
 	// controlling terminal until the read finishes, so a signal there would
@@ -51,6 +53,19 @@ func Entry() int {
 	return 0
 }
 
+// raiseLockedMemoryLimit raises this process's limit on locked memory, which
+// holds every secret value, to the hard limit, the most a process may set
+// without privileges. When that fails the limit stays as it was, and
+// checkLockedMemoryLimit names it in its error.
+func raiseLockedMemoryLimit() {
+	var limit unix.Rlimit
+
+	if unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit) == nil {
+		limit.Cur = limit.Max
+		_ = unix.Setrlimit(unix.RLIMIT_MEMLOCK, &limit)
+	}
+}
+
 func newRootCmd() *cobra.Command {
 	secret.Debug("newRootCmd starting")
 

internal/cli/secrets.go

@@ -14,6 +14,7 @@ import (
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
+	"golang.org/x/sys/unix"
 	"sneak.berlin/go/secret/internal/secret"
 	"sneak.berlin/go/secret/internal/vault"
 )
@@ -24,26 +25,30 @@ const (
 	// vaultSecretParts is the number of parts when splitting vault:secret
 	vaultSecretParts = 2
 
-	// initialBufferSize is the starting size for secret read buffers (4KB)
-	initialBufferSize = 4 * 1024
 	// maxSecretSize is the maximum allowed size of a secret (100MB)
 	maxSecretSize = 100 * 1024 * 1024
+
+	// lockedMemoryReservePages is the locked memory, in pages, that `secret
+	// add`, `secret import` and `secret generate secret` may need besides
+	// the value they store, for keys and the read buffer: memguard locks
+	// whole pages, at least one for each buffer. `secret add` was measured
+	// to need 7 with both SB_SECRET_MNEMONIC and SB_UNLOCK_PASSPHRASE set,
+	// and 6 with one; the eighth is to spare.
+	lockedMemoryReservePages = 8
+	// kibibyte is the unit of the locked-memory limit in `ulimit -l`.
+	kibibyte = 1024
 )
 
 // Sentinel errors for secret operations
 var (
-	errSecretTooLarge              = errors.New("secret too large: exceeds 100MB limit")
+	errSecretTooLarge    = errors.New("secret too large: exceeds 100MB limit")
+	errLockedMemoryLimit = errors.New(
+		"secret too large for the locked-memory limit")
 	errCrossVaultSourceUnqualified = errors.New(
 		"source must specify vault (e.g., vault:secret) for cross-vault move")
 	errMoveOntoItself = errors.New("cannot be moved onto itself")
 )
 
-// bufferInfo tracks a protected buffer and the number of bytes used in it
-type bufferInfo struct {
-	buffer *memguard.LockedBuffer
-	used   int
-}
-
 // ParseVaultSecretRef parses a "vault:secret" or just "secret" reference
 // Returns (vaultName, secretName, isQualified)
 // If no vault is specified, returns empty vaultName and isQualified=false
@@ -75,6 +80,12 @@ func newAddCmd() *cobra.Command {
 
 			cli.cmd = cmd // Set the command for stdin access
 
+			// Before readSecretEnv, which makes the first locked allocation
+			err = checkLockedMemoryLimit(0)
+			if err != nil {
+				return err
+			}
+
 			destroySecrets := cli.readSecretEnv()
 			defer destroySecrets()
 
@@ -174,6 +185,12 @@ func newImportCmd() *cobra.Command {
 				return fmt.Errorf("failed to initialize CLI: %w", err)
 			}
 
+			// Before readSecretEnv, which makes the first locked allocation
+			err = checkLockedMemoryLimit(0)
+			if err != nil {
+				return err
+			}
+
 			destroySecrets := cli.readSecretEnv()
 			defer destroySecrets()
 
@@ -270,88 +287,105 @@ The source secret is deleted after successful copy.`,
 	return cmd
 }
 
-// updateBufferSize updates the buffer size based on usage pattern
-func updateBufferSize(currentSize int, sameSize *int) int {
-	const (
-		doubleAfterBuffers = 2
-		growthFactor       = 2
-	)
+// checkLockedMemoryLimit returns an error naming the locked-memory limit
+// (`ulimit -l`) when a value of size bytes does not fit in it with
+// lockedMemoryReservePages. memguard, which holds secret values in locked
+// memory, crashes or hangs the process when it cannot lock more, so a
+// command calls this with size 0 before its first locked allocation, and
+// again with the value's size before a buffer for the value is allocated.
+func checkLockedMemoryLimit(size int) error {
+	var limit unix.Rlimit
 
-	*sameSize++
-	if *sameSize >= doubleAfterBuffers {
-		*sameSize = 0
-
-		return currentSize * growthFactor
+	err := unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit)
+	if err != nil {
+		return fmt.Errorf("failed to read the locked-memory limit: %w", err)
 	}
 
-	return currentSize
-}
+	// memguard locks whole pages, so the value is counted in whole pages.
+	pageSize := os.Getpagesize()
+	valuePages := (size + pageSize - 1) / pageSize
 
-// destroyBuffers destroys every buffer in the list
-func destroyBuffers(buffers []bufferInfo) {
-	for _, b := range buffers {
-		b.buffer.Destroy()
+	//nolint:gosec // G115: a page count and a page size are never negative
+	needed := uint64((valuePages + lockedMemoryReservePages) * pageSize)
+	if needed > limit.Cur {
+		return fmt.Errorf("%w of %d KiB (ulimit -l)",
+			errLockedMemoryLimit, limit.Cur/kibibyte)
 	}
+
+	return nil
 }
 
-// readSecretFromReader reads all data from reader into protected buffers,
-// enforcing the maximum secret size. On failure the accumulated buffers
-// are destroyed; on success the caller must destroy them.
-func readSecretFromReader(reader io.Reader) ([]bufferInfo, int, error) {
-	var buffers []bufferInfo
+// readSecretFromReader reads all of reader into a new locked buffer, which
+// the caller must destroy, leaving out one trailing newline when trimNewline
+// is set. It reads a page at a time into a memguard.Stream, which keeps what
+// it is given encrypted, so that the value is in locked memory only once, in
+// the buffer returned, and both size checks come before that buffer is
+// allocated. It takes the value back out with Stream.Next, not Stream.Read:
+// Read puts back the part of a piece that does not fit, and memguard hangs
+// when it runs out of locked memory doing that.
+func readSecretFromReader(
+	reader io.Reader, trimNewline bool,
+) (*memguard.LockedBuffer, error) {
+	stream := memguard.NewStream()
 
-	totalSize := 0
-	currentBufferSize := initialBufferSize
-	sameSize := 0
+	chunk := memguard.NewBuffer(os.Getpagesize())
+	defer chunk.Destroy()
+
+	size := 0
+	endsWithNewline := false
 
 	for {
-		// Create a new buffer
-		buffer := memguard.NewBuffer(currentBufferSize)
-		n, err := io.ReadFull(reader, buffer.Bytes())
-
-		if n == 0 {
-			// No data read, destroy the unused buffer
-			buffer.Destroy()
-		} else {
-			buffers = append(buffers, bufferInfo{buffer: buffer, used: n})
-			totalSize += n
-
-			if totalSize > maxSecretSize {
-				destroyBuffers(buffers)
-
-				return nil, 0, errSecretTooLarge
+		n, err := io.ReadFull(reader, chunk.Bytes())
+		if n > 0 {
+			size += n
+			if size > maxSecretSize {
+				return nil, errSecretTooLarge
 			}
 
-			// If we filled the buffer, consider growing for next iteration
-			if n == currentBufferSize {
-				currentBufferSize = updateBufferSize(currentBufferSize, &sameSize)
-			}
+			endsWithNewline = chunk.Bytes()[n-1] == '\n'
+
+			// Write encrypts the bytes and wipes them from chunk.
+			_, _ = stream.Write(chunk.Bytes()[:n])
 		}
 
-		if err == io.EOF || errors.Is(err, io.ErrUnexpectedEOF) {
+		if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
 			break
-		} else if err != nil {
-			destroyBuffers(buffers)
+		}
 
-			return nil, 0, err
+		if err != nil {
+			return nil, err
 		}
 	}
 
-	return buffers, totalSize, nil
-}
+	// Freed before the value's buffer is allocated, so that the two are
+	// never locked at once.
+	chunk.Destroy()
 
-// combineBuffers copies the used portions of buffers into a single
-// protected buffer of totalSize bytes
-func combineBuffers(buffers []bufferInfo, totalSize int) *memguard.LockedBuffer {
-	valueBuffer := memguard.NewBuffer(totalSize)
-
-	offset := 0
-	for _, b := range buffers {
-		copy(valueBuffer.Bytes()[offset:], b.buffer.Bytes()[:b.used])
-		offset += b.used
+	if trimNewline && endsWithNewline {
+		size--
 	}
 
-	return valueBuffer
+	err := checkLockedMemoryLimit(size)
+	if err != nil {
+		return nil, err
+	}
+
+	value := memguard.NewBuffer(size)
+
+	// copy stops at the value's size, so a trimmed newline is not copied.
+	for offset := 0; offset < size; {
+		decrypted, err := stream.Next()
+		if err != nil {
+			value.Destroy()
+
+			return nil, fmt.Errorf("failed to decrypt the value read: %w", err)
+		}
+
+		offset += copy(value.Bytes()[offset:], decrypted.Bytes())
+		decrypted.Destroy()
+	}
+
+	return value, nil
 }
 
 // AddSecret adds a secret to the current vault
@@ -370,35 +404,21 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 
 	secret.Debug("Got current vault", "vault_name", vlt.GetName())
 
-	// Read secret value directly into protected buffers
-	secret.Debug("Reading secret value from stdin into protected buffers")
+	// Read the secret value, less a trailing newline, into locked memory
+	secret.Debug("Reading secret value from stdin into a protected buffer")
 
-	buffers, totalSize, err := readSecretFromReader(cli.cmd.InOrStdin())
+	valueBuffer, err := readSecretFromReader(cli.cmd.InOrStdin(), true)
 	if err != nil {
-		if errors.Is(err, errSecretTooLarge) {
+		if errors.Is(err, errSecretTooLarge) || errors.Is(err, errLockedMemoryLimit) {
 			return err
 		}
 
 		return fmt.Errorf("failed to read secret value: %w", err)
 	}
-	defer destroyBuffers(buffers)
-
-	// Check for trailing newline in the last buffer
-	if len(buffers) > 0 && totalSize > 0 {
-		lastBuffer := &buffers[len(buffers)-1]
-		if lastBuffer.buffer.Bytes()[lastBuffer.used-1] == '\n' {
-			lastBuffer.used--
-			totalSize--
-		}
-	}
-
-	secret.Debug("Read secret value from stdin",
-		"value_length", totalSize, "buffers", len(buffers))
-
-	// Combine all buffers into a single protected buffer
-	valueBuffer := combineBuffers(buffers, totalSize)
 	defer valueBuffer.Destroy()
 
+	secret.Debug("Read secret value from stdin", "value_length", valueBuffer.Size())
+
 	// Locked only now that stdin has been read: in `secret encrypt key |
 	// secret add name`, holding the lock while reading would leave each
 	// command waiting for the other.
@@ -443,7 +463,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 +501,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")
 
@@ -661,7 +687,7 @@ func (cli *Instance) ImportSecret(
 
 	vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
 
-	// Read secret value from the source file into protected buffers
+	// Read secret value from the source file into a protected buffer
 	file, err := cli.fs.Open(sourceFile)
 	if err != nil {
 		return fmt.Errorf("failed to open file %s: %w", sourceFile, err)
@@ -673,14 +699,10 @@ func (cli *Instance) ImportSecret(
 		}
 	}()
 
-	buffers, totalSize, err := readSecretFromReader(file)
+	valueBuffer, err := readSecretFromReader(file, false)
 	if err != nil {
 		return fmt.Errorf("failed to read secret from file %s: %w", sourceFile, err)
 	}
-	defer destroyBuffers(buffers)
-
-	// Combine all buffers into a single protected buffer
-	valueBuffer := combineBuffers(buffers, totalSize)
 	defer valueBuffer.Destroy()
 
 	// Locked only now that the file has been read, as in AddSecret: the

internal/cli/secrets_size_test.go

@@ -3,17 +3,23 @@ package cli
 
 import (
 	"bytes"
+	"context"
 	"crypto/rand"
 	"fmt"
 	"io"
+	"os"
+	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/secret"
 	"sneak.berlin/go/secret/internal/vault"
 	"sneak.berlin/go/secret/pkg/agehd"
 )
@@ -315,6 +321,159 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
 		"Retrieved secret should match original")
 }
 
+// newBinaryTestStateDir returns a new state directory holding a vault made
+// from the test mnemonic, for the secret binary to use.
+func newBinaryTestStateDir(t *testing.T) string {
+	t.Helper()
+
+	stateDir := t.TempDir()
+	_, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
+		testMnemonicBuffer(t), nil)
+	require.NoError(t, err)
+
+	return stateDir
+}
+
+// runAtLockedMemoryLimit runs the secret binary with args, stdin, the state
+// directory stateDir and the test mnemonic, its locked-memory limit (ulimit
+// -l) lowered to limitKiB, and returns what it printed.
+func runAtLockedMemoryLimit(
+	t *testing.T, stateDir string, limitKiB int, stdin io.Reader,
+	args ...string,
+) (string, error) {
+	t.Helper()
+
+	const commandWait = 10 * time.Second
+
+	secretPath, err := filepath.Abs("../../secret")
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	// The shell lowers the limit for itself and the binary only.
+	script := fmt.Sprintf(`ulimit -l %d && exec "$0" "$@"`, limitKiB)
+
+	//nolint:gosec // G204: runs the freshly built secret binary
+	cmd := exec.CommandContext(ctx, "sh",
+		append([]string{"-c", script, secretPath}, args...)...)
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		secret.EnvMnemonic + "=" + testMnemonic,
+	}
+	cmd.Stdin = stdin
+
+	output, err := cmd.CombinedOutput()
+
+	return string(output), err
+}
+
+// requireRefusedAtLockedMemoryLimit runs the secret binary as
+// runAtLockedMemoryLimit does, and requires it to exit with status 1 and
+// print wantError, with no panic.
+func requireRefusedAtLockedMemoryLimit(
+	t *testing.T, stateDir string, limitKiB int, wantError string,
+	stdin io.Reader, args ...string,
+) {
+	t.Helper()
+
+	output, err := runAtLockedMemoryLimit(t, stateDir, limitKiB, stdin, args...)
+
+	var exitErr *exec.ExitError
+
+	require.ErrorAs(t, err, &exitErr, "%v: %s", args, output)
+	assert.Equal(t, 1, exitErr.ExitCode(), "%v: %s", args, output)
+	assert.Contains(t, output, wantError, args)
+	assert.NotContains(t, output, "panic", args)
+}
+
+// TestSecretsAtLockedMemoryLimit runs the secret binary with the
+// locked-memory limit (ulimit -l) lowered to 1 MiB, where memguard would crash
+// the process with a panic instead of returning an error. add must store the
+// largest value that fits, which takes holding it in locked memory only once.
+// add, import and generate secret must refuse a value one byte larger, and one
+// over 100 MB, with exit status 1 and an error saying why.
+func TestSecretsAtLockedMemoryLimit(t *testing.T) {
+	t.Parallel()
+
+	const limitKiB = 1024
+
+	fits := limitKiB*kibibyte - lockedMemoryReservePages*os.Getpagesize()
+	limitError := fmt.Sprintf("locked-memory limit of %d KiB", limitKiB)
+	stateDir := newBinaryTestStateDir(t)
+
+	tooBigFile := filepath.Join(t.TempDir(), "too-big")
+	require.NoError(t, os.WriteFile(tooBigFile, make([]byte, fits+1), 0o600))
+
+	// Sparse, so that it takes no room on disk
+	overMaxSize, err := os.Create(filepath.Join(t.TempDir(), "over-max-size"))
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = overMaxSize.Close() })
+	require.NoError(t, overMaxSize.Truncate(maxSecretSize+1))
+
+	output, err := runAtLockedMemoryLimit(t, stateDir, limitKiB,
+		bytes.NewReader(make([]byte, fits)), "add", "fits")
+	require.NoError(t, err, "the largest value that fits: %s", output)
+
+	requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB, limitError,
+		bytes.NewReader(make([]byte, fits+1)), "add", "too-big")
+	requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB, limitError,
+		nil, "import", "too-big", "--source", tooBigFile)
+	requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB, limitError,
+		nil, "generate", "secret", "too-big", "--length", strconv.Itoa(fits+1))
+
+	requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB,
+		errSecretTooLarge.Error(), overMaxSize, "add", "too-big")
+	requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB,
+		errSecretTooLarge.Error(), nil,
+		"import", "too-big", "--source", overMaxSize.Name())
+	requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB,
+		errSecretTooLarge.Error(), nil,
+		"generate", "secret", "too-big", "--length", strconv.Itoa(maxSecretSize+1))
+}
+
+// TestSecretsUnderSmallLockedMemoryLimits runs add, import and generate
+// secret with a short value under two small locked-memory limits (ulimit
+// -l). Under 20 KiB, too little for the buffers they need besides the value,
+// each must refuse it with exit status 1 and an error naming the limit, where
+// memguard would panic or hang. Under 64 KiB, the default of older Linux
+// kernels, each must store it.
+func TestSecretsUnderSmallLockedMemoryLimits(t *testing.T) {
+	t.Parallel()
+
+	const (
+		tooSmallLimitKiB   = 20
+		oldDefaultLimitKiB = 64
+		shortValue         = "hunter2"
+	)
+
+	tooSmallLimitError := fmt.Sprintf("locked-memory limit of %d KiB",
+		tooSmallLimitKiB)
+	stateDir := newBinaryTestStateDir(t)
+
+	shortFile := filepath.Join(t.TempDir(), "short")
+	require.NoError(t, os.WriteFile(shortFile, []byte(shortValue), 0o600))
+
+	requireRefusedAtLockedMemoryLimit(t, stateDir, tooSmallLimitKiB,
+		tooSmallLimitError, strings.NewReader(shortValue), "add", "added")
+	requireRefusedAtLockedMemoryLimit(t, stateDir, tooSmallLimitKiB,
+		tooSmallLimitError, nil, "import", "imported", "--source", shortFile)
+	requireRefusedAtLockedMemoryLimit(t, stateDir, tooSmallLimitKiB,
+		tooSmallLimitError, nil, "generate", "secret", "generated")
+
+	output, err := runAtLockedMemoryLimit(t, stateDir, oldDefaultLimitKiB,
+		strings.NewReader(shortValue), "add", "added")
+	require.NoError(t, err, "add: %s", output)
+
+	output, err = runAtLockedMemoryLimit(t, stateDir, oldDefaultLimitKiB,
+		nil, "import", "imported", "--source", shortFile)
+	require.NoError(t, err, "import: %s", output)
+
+	output, err = runAtLockedMemoryLimit(t, stateDir, oldDefaultLimitKiB,
+		nil, "generate", "secret", "generated")
+	require.NoError(t, err, "generate secret: %s", output)
+}
+
 // slowReader simulates a reader that returns data in small chunks
 type slowReader struct {
 	data      []byte

internal/cli/vault.go

@@ -267,6 +267,12 @@ 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,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

internal/vault/management.go

@@ -212,16 +212,40 @@ 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 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.
+// 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.
 //
 // 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
@@ -233,30 +257,17 @@ func CreateVault(
 ) (*Vault, error) {
 	secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
 
-	err := ValidateVaultName(name)
+	err := CheckNewVaultName(fs, stateDir, 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 {