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. The value is read a page at a time into a memguard.Stream,
which keeps it encrypted, then copied once into locked memory after its size is
checked against 100 MiB and against the limit less 16 pages for the command's
keys. It comes out of the stream through Next, not Read: Read re-encrypts a
leftover piece, and memguard hangs if that runs out of locked memory.

Judgement call: with a limit of 64 KiB or less, nothing can be stored.

Model: opus-5-5

README.md

@@ -129,14 +129,14 @@ one.
 
 #### 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.
+A secret's value is held in locked memory, which is never written to swap.
+`secret` raises its locked-memory limit (`ulimit -l`) to the hard limit when it
+starts, and `secret add`, `secret import` and `secret generate secret` refuse a
+value that does not fit in that limit with 16 memory pages to spare (64 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]`
 
@@ -161,13 +161,6 @@ Retrieves and outputs a secret value to stdout.
 
 Lists all secrets in the current vault. Optional filter for substring matching.
 
-The `LAST UPDATED` column, and `updated_at` in the `--json` output, give the
-time the secret's `current` file was last written: every command that adds a
-version replaces it, and so does `secret version promote`. `created_at` gives
-the time the `value.age` of the secret's oldest version was written. Both are
-the files' modification times, so listing asks for no passphrase. Moving a
-secret to another vault keeps both times.
-
 #### `secret remove <secret-name> [--force]` / `secret rm` ⚠️ 🛑
 
 **DANGER**: Permanently removes a secret and ALL its versions. It first asks for
@@ -199,9 +192,7 @@ period.
 #### `secret version promote <secret-name> <version>`
 
 Promotes a specific version to current by rewriting the secret's `current` file
-to name it. The versions' own times are left unchanged, allowing for rollback
-scenarios; `secret list` then gives the time of the promote as the secret's last
-update.
+to name it. Does not modify any timestamps, allowing for rollback scenarios.
 
 #### `secret version remove <secret-name> <version> [--force]` / `secret version rm` ⚠️ 🛑
 

TODO.md

@@ -18,14 +18,17 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
-- 2026-10-14: A wrong passphrase, from `SB_UNLOCK_PASSPHRASE` or typed at the
-  prompt, gives the error `wrong passphrase for unlocker <id>`
-  (https://git.eeqj.de/sneak/secret/issues/157). It used to be age's "no
-  identity matched any of the recipients", followed by advice to give the vault
-  a new unlocker from its mnemonic, which would have replaced an unlocker that
-  works. The error now wraps `secret.ErrWrongPassphrase` and carries no advice
-  to use the mnemonic. A `priv.age` that is not an age file still gets that
-  advice.
+- 2026-10-14: A secret too large for the locked-memory limit (`ulimit -l`) makes
+  `secret add`, `secret import` and `secret generate secret` fail with an error
+  that names the limit, not crash with a panic
+  (https://git.eeqj.de/sneak/secret/issues/145). memguard panics when it cannot
+  lock memory, and the value was held twice, so with the usual 8 MiB limit any
+  value over about 4 MiB crashed. `secret` now raises the limit to the hard
+  limit at startup. The value is read a page at a time into a `memguard.Stream`,
+  which keeps it encrypted, and copied once into locked memory only after its
+  size is checked against 100 MiB and against the limit less 16 pages for the
+  command's keys. `generate secret` checks the length before it generates
+  anything.
 - 2026-10-14: The passphrase and mnemonic prompts ask on the controlling
   terminal (`/dev/tty`) instead of stdin, so `echo v | secret add s1 --force`,
   `cat file | secret encrypt key` and `cat file.age | secret decrypt key` ask

internal/cli/generate.go

@@ -72,12 +72,6 @@ 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()
 

internal/cli/list_times_test.go

@@ -1,161 +0,0 @@
-package cli_test
-
-import (
-	"bytes"
-	"encoding/json"
-	"testing"
-	"time"
-
-	"github.com/awnumar/memguard"
-	"github.com/spf13/afero"
-	"github.com/spf13/cobra"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/secret/internal/cli"
-	"sneak.berlin/go/secret/internal/secret"
-	"sneak.berlin/go/secret/internal/vault"
-)
-
-const (
-	// listClockStep is how long TestListShowsWhenSecretWasWritten waits
-	// between its steps, so that a time read in one step cannot be the
-	// time read in the next.
-	listClockStep = 200 * time.Millisecond
-
-	// fileTimeSlack allows for the clock the kernel stamps files with,
-	// which advances only every few milliseconds: a file's modification
-	// time can be slightly earlier than a time.Now() read before the file
-	// was written.
-	fileTimeSlack = 100 * time.Millisecond
-)
-
-// TestListShowsWhenSecretWasWritten is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/142, where `secret list` gave
-// the time of the listing as every secret's creation and last-update time.
-// Two listings with a clock step between must be the same, and give the
-// time of the add; after a forced add, the creation time is unchanged and
-// the last update is the time of that add. It runs on a real filesystem,
-// where writing a file into a directory also changes the directory's time.
-func TestListShowsWhenSecretWasWritten(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	stateDir := t.TempDir()
-
-	vlt, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t), nil)
-	require.NoError(t, err)
-
-	vlt.Mnemonic = testMnemonicBuffer(t)
-	c := cli.NewCLIInstanceWithStateDir(fs, stateDir)
-
-	beforeAdd := time.Now()
-	err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("one")), false)
-	require.NoError(t, err)
-
-	afterAdd := time.Now()
-
-	firstListing := listSecrets(t, c, true)
-
-	time.Sleep(listClockStep)
-	require.Equal(t, firstListing, listSecrets(t, c, true))
-
-	added := listedTimes(t, firstListing)
-	assert.WithinRange(t, added.CreatedAt, beforeAdd.Add(-fileTimeSlack), afterAdd)
-	assert.WithinRange(t, added.UpdatedAt, beforeAdd.Add(-fileTimeSlack), afterAdd)
-
-	time.Sleep(listClockStep)
-
-	beforeUpdate := time.Now()
-	err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("two")), true)
-	require.NoError(t, err)
-
-	afterUpdate := time.Now()
-
-	updated := listedTimes(t, listSecrets(t, c, true))
-	assert.True(t, added.CreatedAt.Equal(updated.CreatedAt),
-		"created %s, then %s", added.CreatedAt, updated.CreatedAt)
-	assert.WithinRange(t, updated.UpdatedAt,
-		beforeUpdate.Add(-fileTimeSlack), afterUpdate)
-
-	// The table's LAST UPDATED column gives the same time, to the minute
-	assert.Contains(t, listSecrets(t, c, false),
-		updated.UpdatedAt.Format("2006-01-02 15:04"))
-}
-
-// TestMovedSecretKeepsItsTimes checks that a secret moved to another vault
-// is listed there with the times it was listed with before the move, and
-// not with the time of the move.
-func TestMovedSecretKeepsItsTimes(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	stateDir := t.TempDir()
-
-	_, err := vault.CreateVault(fs, stateDir, "other", testMnemonicBuffer(t), nil)
-	require.NoError(t, err)
-
-	// "default" is created last, so it is the current vault
-	vlt, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t), nil)
-	require.NoError(t, err)
-
-	vlt.Mnemonic = testMnemonicBuffer(t)
-
-	// Two versions written apart, so that the creation and the last update
-	// are different times
-	err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("one")), false)
-	require.NoError(t, err)
-
-	time.Sleep(listClockStep)
-
-	err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("two")), true)
-	require.NoError(t, err)
-
-	c := cli.NewCLIInstanceWithStateDir(fs, stateDir)
-	c.Mnemonic = testMnemonicBuffer(t)
-	beforeMove := listedTimes(t, listSecrets(t, c, true))
-
-	time.Sleep(listClockStep)
-	require.NoError(t, c.MoveSecret(&cobra.Command{}, "default:x", "other", false))
-	require.NoError(t, vault.SelectVault(fs, stateDir, "other"))
-
-	afterMove := listedTimes(t, listSecrets(t, c, true))
-	assert.True(t, beforeMove.CreatedAt.Equal(afterMove.CreatedAt),
-		"created %s, then %s", beforeMove.CreatedAt, afterMove.CreatedAt)
-	assert.True(t, beforeMove.UpdatedAt.Equal(afterMove.UpdatedAt),
-		"updated %s, then %s", beforeMove.UpdatedAt, afterMove.UpdatedAt)
-}
-
-// listSecrets returns the output of `secret list`, or of
-// `secret list --json` when jsonOutput is set.
-func listSecrets(t *testing.T, c *cli.Instance, jsonOutput bool) string {
-	t.Helper()
-
-	var out bytes.Buffer
-
-	cmd := &cobra.Command{}
-	cmd.SetOut(&out)
-	require.NoError(t, c.ListSecrets(cmd, jsonOutput, false, ""))
-
-	return out.String()
-}
-
-// listedTimes returns the times that the output of `secret list --json`
-// gives for the one secret in the vault.
-func listedTimes(t *testing.T, listing string) secret.Metadata {
-	t.Helper()
-
-	var parsed struct {
-		Secrets []map[string]string `json:"secrets"`
-	}
-
-	require.NoError(t, json.Unmarshal([]byte(listing), &parsed))
-	require.Len(t, parsed.Secrets, 1)
-
-	createdAt, err := time.Parse(time.RFC3339Nano, parsed.Secrets[0]["created_at"])
-	require.NoError(t, err)
-
-	updatedAt, err := time.Parse(time.RFC3339Nano, parsed.Secrets[0]["updated_at"])
-	require.NoError(t, err)
-
-	return secret.Metadata{CreatedAt: createdAt, UpdatedAt: updatedAt}
-}

internal/cli/secrets.go

@@ -28,13 +28,11 @@ const (
 	// 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
+	// lockedMemoryReservePages is the locked memory, in pages, that a command
+	// needs besides the value it stores, for its keys and its read buffer,
+	// with room to spare: memguard locks whole pages, at least one for each
+	// buffer, and `secret add` needs 7.
+	lockedMemoryReservePages = 16
 	// kibibyte is the unit of the locked-memory limit in `ulimit -l`.
 	kibibyte = 1024
 )
@@ -80,12 +78,6 @@ 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()
 
@@ -185,12 +177,6 @@ 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()
 
@@ -290,9 +276,8 @@ The source secret is deleted after successful copy.`,
 // 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.
+// memory, crashes the process when it cannot lock more, so this check comes
+// before a buffer for the value is allocated.
 func checkLockedMemoryLimit(size int) error {
 	var limit unix.Rlimit
 
@@ -301,12 +286,8 @@ func checkLockedMemoryLimit(size int) error {
 		return fmt.Errorf("failed to read the locked-memory limit: %w", err)
 	}
 
-	// memguard locks whole pages, so the value is counted in whole pages.
-	pageSize := os.Getpagesize()
-	valuePages := (size + pageSize - 1) / pageSize
-
-	//nolint:gosec // G115: a page count and a page size are never negative
-	needed := uint64((valuePages + lockedMemoryReservePages) * pageSize)
+	//nolint:gosec // G115: a size and a page count are never negative
+	needed := uint64(size + lockedMemoryReservePages*os.Getpagesize())
 	if needed > limit.Cur {
 		return fmt.Errorf("%w of %d KiB (ulimit -l)",
 			errLockedMemoryLimit, limit.Cur/kibibyte)
@@ -561,12 +542,8 @@ func printSecretsJSON(
 			"name": secretName,
 		}
 
-		// A secret whose times cannot be read is listed without them
+		// Try to get metadata using GetSecretObject
 		secretObj, err := vlt.GetSecretObject(secretName)
-		if err == nil {
-			err = secretObj.LoadMetadata()
-		}
-
 		if err == nil {
 			metadata := secretObj.GetMetadata()
 			secretInfo["created_at"] = metadata.CreatedAt
@@ -642,10 +619,6 @@ func printSecretsTable(
 		lastUpdated := "unknown"
 
 		secretObj, err := vlt.GetSecretObject(secretName)
-		if err == nil {
-			err = secretObj.LoadMetadata()
-		}
-
 		if err == nil {
 			metadata := secretObj.GetMetadata()
 			lastUpdated = metadata.UpdatedAt.Format("2006-01-02 15:04")

internal/cli/secrets_size_test.go

@@ -321,157 +321,75 @@ 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()
+// TestSecretsAtLockedMemoryLimit runs the secret binary with the
+// locked-memory limit (ulimit -l) lowered. 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 generate secret
+// one over 100 MB, with exit status 1 and an error saying why, where memguard
+// would crash the process with a panic.
+func TestSecretsAtLockedMemoryLimit(t *testing.T) {
+	t.Parallel()
+
+	const (
+		limitKiB    = 1024
+		commandWait = 10 * time.Second
+	)
+
+	fits := limitKiB*kibibyte - lockedMemoryReservePages*os.Getpagesize()
+	limitError := fmt.Sprintf("locked-memory limit of %d KiB", limitKiB)
 
 	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))
+	run := func(stdin []byte, args ...string) (string, error) {
+		ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+		defer cancel()
 
-	output, err := runAtLockedMemoryLimit(t, stateDir, limitKiB,
-		bytes.NewReader(make([]byte, fits)), "add", "fits")
+		// 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 = bytes.NewReader(stdin)
+
+		output, err := cmd.CombinedOutput()
+
+		return string(output), err
+	}
+
+	output, err := run(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))
+	refused := func(wantError string, stdin []byte, args ...string) {
+		output, err := run(stdin, args...)
 
-	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))
-}
+		var exitErr *exec.ExitError
 
-// 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()
+		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)
+	}
 
-	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)
+	refused(limitError, make([]byte, fits+1), "add", "too-big")
+	refused(limitError, nil, "import", "too-big", "--source", tooBigFile)
+	refused(limitError, nil, "generate", "secret", "too-big",
+		"--length", strconv.Itoa(fits+1))
+	refused(errSecretTooLarge.Error(), nil, "generate", "secret", "too-big",
+		"--length", strconv.Itoa(maxSecretSize+1))
 }
 
 // slowReader simulates a reader that returns data in small chunks

internal/cli/unlock_failure_test.go

@@ -1,12 +1,12 @@
 // Unlock Failure Tests
 //
 // When a vault cannot be opened through its current unlocker, because a
-// file the unlocker needs is missing, the error keeps its cause and ends by
-// saying that the mnemonic still opens that vault, but only for a vault that
-// the mnemonic does open, and not when the passphrase could not be read at
-// all. A wrong passphrase gives an error that says so, without that advice.
-// When the current unlocker does not exist and the vault has another that
-// `secret unlocker select` accepts, the error says to select one instead.
+// file the unlocker needs is missing or the passphrase is wrong, the error
+// keeps its cause and ends by saying that the mnemonic still opens that
+// vault, but only for a vault that the mnemonic does open, and not when the
+// passphrase could not be read at all. When the current unlocker does not
+// exist and the vault has another that `secret unlocker select` accepts, the
+// error says to select one instead.
 // When a secret's current file is missing, the error says how to make a
 // version current again. Each test that pins such advice also follows it.
 
@@ -21,6 +21,7 @@ import (
 	"syscall"
 	"testing"
 
+	"filippo.io/age"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
@@ -164,48 +165,26 @@ func TestUnlockFailureNamesMnemonic(t *testing.T) {
 	}
 }
 
-// TestWrongPassphraseNamesPassphrase checks the error of `secret get` given
-// a passphrase that does not decrypt the passphrase unlocker: it says that
-// the passphrase is wrong, without the advice to use the mnemonic, since the
-// unlocker still opens the vault with the right passphrase. The test then
-// gives the right passphrase.
-func TestWrongPassphraseNamesPassphrase(t *testing.T) {
+// TestWrongPassphraseNamesMnemonic checks the error of `secret get` given a
+// passphrase that does not decrypt the passphrase unlocker: it keeps age's
+// error and ends with the advice that the mnemonic still opens the vault.
+func TestWrongPassphraseNamesMnemonic(t *testing.T) {
 	t.Parallel()
 
-	unlockerDir, c := newUnlockTestCLI(t, newTwoVaultFs(t))
+	_, c := newUnlockTestCLI(t, newTwoVaultFs(t))
 	c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte("wrong passphrase"))
 	t.Cleanup(c.UnlockPassphrase.Destroy)
 
 	err := c.GetSecret(discardCmd(), "x")
 
-	require.ErrorIs(t, err, secret.ErrWrongPassphrase)
-	assert.NotContains(t, err.Error(), mnemonicAdvice)
+	var noMatch *age.NoIdentityMatchError
+
+	require.ErrorAs(t, err, &noMatch)
+
 	require.EqualError(t, err, "failed to unlock vault: "+
 		"failed to get long-term key: failed to get unlocker identity: "+
-		"wrong passphrase for unlocker "+filepath.Base(unlockerDir))
-
-	c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase))
-	t.Cleanup(c.UnlockPassphrase.Destroy)
-	assert.Equal(t, "value", getSecret(t, c, "x"))
-}
-
-// TestDamagedUnlockerNamesMnemonic checks the error of `secret get` when the
-// passphrase unlocker's priv.age is not an age file: the passphrase is not
-// blamed, and the error ends with the advice that the mnemonic still opens
-// the vault.
-func TestDamagedUnlockerNamesMnemonic(t *testing.T) {
-	t.Parallel()
-
-	fs := newTwoVaultFs(t)
-	unlockerDir, c := newUnlockTestCLI(t, fs)
-
-	require.NoError(t, afero.WriteFile(fs, filepath.Join(unlockerDir, "priv.age"),
-		[]byte("damaged"), secret.FilePerms))
-
-	err := c.GetSecret(discardCmd(), "x")
-
-	require.NotErrorIs(t, err, secret.ErrWrongPassphrase)
-	require.ErrorContains(t, err, mnemonicAdvice)
+		"failed to decrypt unlocker private key: failed to create decryptor: "+
+		noMatch.Error()+mnemonicAdvice)
 }
 
 // TestMoveUnlockFailureNamesVault checks the error of `secret move` into

internal/cli/unlockers.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"log"
 	"maps"
-	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -67,13 +66,12 @@ func getDefaultGPGKey() (string, error) {
 
 	output, err := cmd.Output()
 	if err == nil {
-		defaultKey, parseErr := parseDefaultGPGKey(string(output))
-		if parseErr != nil {
-			return "", parseErr
-		}
-
-		if defaultKey != "" {
-			return defaultKey, nil
+		for line := range strings.SplitSeq(string(output), "\n") {
+			fields := strings.Split(line, ":")
+			if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" {
+				// The default key is in field 10 (index 9)
+				return fields[9], nil
+			}
 		}
 	}
 
@@ -100,27 +98,6 @@ func getDefaultGPGKey() (string, error) {
 	return "", errNoGPGSecretKeys
 }
 
-// parseDefaultGPGKey returns the default-key value in the output of
-// `gpgconf --list-options gpg`, or "" when gpg.conf sets none. gpgconf(1)
-// prints a string value with a leading `"` and percent-escaped.
-func parseDefaultGPGKey(gpgconfOutput string) (string, error) {
-	for line := range strings.SplitSeq(gpgconfOutput, "\n") {
-		fields := strings.Split(line, ":")
-		if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" {
-			// The default key is in field 10 (index 9)
-			keyID, err := url.PathUnescape(strings.TrimPrefix(fields[9], `"`))
-			if err != nil {
-				return "", fmt.Errorf("failed to decode gpgconf default-key %s: %w",
-					fields[9], err)
-			}
-
-			return keyID, nil
-		}
-	}
-
-	return "", nil
-}
-
 func newUnlockerCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "unlocker",

internal/cli/unlockers_add_test.go

@@ -2,8 +2,6 @@
 package cli
 
 import (
-	"bytes"
-	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -85,77 +83,6 @@ func TestAddPGPUnlocker(t *testing.T) {
 	}
 }
 
-// TestAddPGPUnlockerDefaultKey is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/141: with default-key set in
-// gpg.conf and no --keyid, `secret unlocker add pgp` uses that key.
-// gpg.conf names the key by its user ID, and the printed key ID must be that
-// user ID. The first secret key, which is used when gpg.conf sets no
-// default-key, prints as a hex key ID.
-func TestAddPGPUnlockerDefaultKey(t *testing.T) {
-	newTestGPGKey(t)
-	// The key comes from neither --keyid nor the environment.
-	t.Setenv(secret.EnvGPGKeyID, "")
-
-	gpgConf := filepath.Join(os.Getenv("GNUPGHOME"), "gpg.conf")
-	//nolint:gosec // G703: GNUPGHOME is the directory newTestGPGKey made
-	require.NoError(t, os.WriteFile(gpgConf,
-		[]byte("default-key "+unreadableTestGPGUserID+"\n"), 0o600))
-
-	fs := afero.NewMemMapFs()
-	_, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName,
-		testMnemonicBuffer(t), nil)
-	require.NoError(t, err)
-
-	instance, cmd := newTestInstance(fs)
-	instance.Mnemonic = testMnemonicBuffer(t)
-
-	var output bytes.Buffer
-	cmd.SetOut(&output)
-
-	require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd))
-	assert.Contains(t, output.String(),
-		"Using default GPG key: "+unreadableTestGPGUserID+"\n")
-}
-
-// TestParseDefaultGPGKey is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/141: gpgconf prints the
-// default-key value with a leading `"` and percent-escaped, and the key ID
-// is that value decoded.
-func TestParseDefaultGPGKey(t *testing.T) {
-	t.Parallel()
-
-	// gpgconfLine is the default-key line of `gpgconf --list-options gpg`
-	// up to the value, its last field.
-	const gpgconfLine = "default-key:0:0:use NAME as default secret key:" +
-		"1:1:NAME:::"
-
-	tests := []struct {
-		name  string
-		value string
-		want  string
-	}{
-		{
-			"fingerprint", `"56163D7F3FAFF709608855BEC1A360216D007F41`,
-			"56163D7F3FAFF709608855BEC1A360216D007F41",
-		},
-		{
-			"user ID with a comma", `"Doe%2c Alice <alice@example.com>`,
-			"Doe, Alice <alice@example.com>",
-		},
-		{"no default-key in gpg.conf", "", ""},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			keyID, err := parseDefaultGPGKey(gpgconfLine + test.value + "\n")
-			require.NoError(t, err)
-			assert.Equal(t, test.want, keyID)
-		})
-	}
-}
-
 // TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key
 // the keyring does not hold fails at looking up the key's fingerprint and
 // leaves no new unlocker directory. The error must come from the lookup: a

internal/cli/version.go

@@ -65,9 +65,7 @@ func VersionCommands(cli *Instance) *cobra.Command {
 		Use:   "promote <secret-name> <version>",
 		Short: "Promote a specific version to current",
 		Long: "Updates the current symlink to point to the specified " +
-			"version. The versions' own times are left unchanged; secret " +
-			"list then gives the time of the promote as the secret's last " +
-			"update.",
+			"version without modifying timestamps",
 		Args: cobra.ExactArgs(2), //nolint:mnd // secret-name and version args
 		ValidArgsFunction: func(
 			cmd *cobra.Command, args []string, toComplete string,

internal/secret/passphraseunlocker.go

@@ -16,10 +16,6 @@ import (
 // empty. A passphrase unlocker that fails with it was not tried.
 var ErrPassphraseNotRead = errors.New("failed to read passphrase")
 
-// ErrWrongPassphrase is wrapped in the error of GetIdentity when the
-// passphrase does not decrypt the unlocker's private key.
-var ErrWrongPassphrase = errors.New("wrong passphrase")
-
 // PassphraseUnlocker represents a passphrase-protected unlocker
 type PassphraseUnlocker struct {
 	Directory  string
@@ -77,12 +73,6 @@ func (p *PassphraseUnlocker) GetIdentity() (*age.X25519Identity, error) {
 		Debug("Failed to decrypt unlocker private key",
 			"error", err, "unlocker_id", p.GetID())
 
-		// age says that no identity matched when the passphrase is wrong
-		var noMatch *age.NoIdentityMatchError
-		if errors.As(err, &noMatch) {
-			return nil, fmt.Errorf("%w for unlocker %s", ErrWrongPassphrase, p.GetID())
-		}
-
 		return nil, fmt.Errorf("failed to decrypt unlocker private key: %w", err)
 	}
 	defer privKeyBuffer.Destroy()

internal/secret/secret.go

@@ -2,11 +2,10 @@ package secret
 
 import (
 	"errors"
-	"fmt"
 	"log/slog"
-	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"filippo.io/age"
 	"github.com/awnumar/memguard"
@@ -67,53 +66,31 @@ func NewSecret(vault VaultInterface, name string) *Secret {
 		Name:      name,
 		Directory: secretDir,
 		vault:     vault,
+		Metadata: Metadata{
+			CreatedAt: time.Now(),
+			UpdatedAt: time.Now(),
+		},
 	}
 }
 
-// LoadMetadata sets the secret's times from the modification times of its
-// files, which can be read without unlocking the vault. UpdatedAt is when
-// the secret's current file was last written, by an add or a promote.
-// CreatedAt is when its oldest version's value.age was written. That file
-// is never rewritten; the version's directory is no use, as its time
-// changes when the next add rewrites the version's metadata.age.
+// LoadMetadata is deprecated - metadata is now per-version and encrypted
 func (s *Secret) LoadMetadata() error {
-	fs := s.vault.GetFilesystem()
-
-	current, err := fs.Stat(filepath.Join(s.Directory, "current"))
-	if err != nil {
-		return fmt.Errorf("failed to read when secret %s was updated: %w",
-			s.Name, err)
-	}
-
-	versions, err := ListVersions(fs, s.Directory)
-	if err != nil {
-		return err
-	}
-
-	if len(versions) == 0 {
-		return fmt.Errorf("secret %s has no versions: %w", s.Name, os.ErrNotExist)
-	}
-
-	// ListVersions lists the newest version first
-	oldest := versions[len(versions)-1]
-
-	value, err := fs.Stat(
-		filepath.Join(s.Directory, "versions", oldest, "value.age"))
-	if err != nil {
-		return fmt.Errorf("failed to read when secret %s was created: %w",
-			s.Name, err)
-	}
-
+	Debug("LoadMetadata called but is deprecated in versioned model",
+		"secret_name", s.Name)
+	// For backward compatibility, we'll populate with basic info
+	now := time.Now()
 	s.Metadata = Metadata{
-		CreatedAt: value.ModTime(),
-		UpdatedAt: current.ModTime(),
+		CreatedAt: now,
+		UpdatedAt: now,
 	}
 
 	return nil
 }
 
-// GetMetadata returns the times LoadMetadata read
+// GetMetadata returns the secret metadata (deprecated)
 func (s *Secret) GetMetadata() Metadata {
+	Debug("GetMetadata called but is deprecated in versioned model", "secret_name", s.Name)
+
 	return s.Metadata
 }
 

internal/vault/secrets.go

@@ -427,8 +427,7 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
 	return longTermIdentity, nil
 }
 
-// GetSecretObject retrieves a Secret object from this vault. Its times are
-// zero until its LoadMetadata reads them.
+// GetSecretObject retrieves a Secret object with metadata loaded from this vault
 func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
 	err := ValidateSecretName(name)
 	if err != nil {
@@ -455,7 +454,16 @@ func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
 		return nil, fmt.Errorf("secret %s %w", name, ErrSecretNotFound)
 	}
 
-	return secret.NewSecret(v, name), nil
+	// Create a Secret object
+	secretObj := secret.NewSecret(v, name)
+
+	// Load the metadata from disk
+	err = secretObj.LoadMetadata()
+	if err != nil {
+		return nil, err
+	}
+
+	return secretObj, nil
 }
 
 // CopySecretVersion copies a single version from source into destSecretDir
@@ -573,8 +581,8 @@ func (v *Vault) CopySecretAllVersions(
 	}
 
 	// Copy each version and the current pointer, then move the copy into place
-	err = v.copyVersions(srcVault, srcIdentity, srcSecretName, srcSecretDir,
-		destSecretName, destSecretDir, versions, currentVersion)
+	err = v.copyVersions(srcVault, srcIdentity,
+		srcSecretName, destSecretName, destSecretDir, versions, currentVersion)
 	if err != nil {
 		return err
 	}
@@ -793,13 +801,10 @@ func (v *Vault) createAndSaveVersion(
 // pointer into a temporary directory, then moves that directory to
 // destSecretDir, replacing a secret already there. Nothing in this vault
 // changes until the copy is complete, so an interrupted copy leaves only a
-// temporary directory behind. Each copied value.age and current file gets
-// the modification time of the file it was copied from, so `secret list`,
-// which reads a secret's times from them, lists the copy with the times of
-// the source.
+// temporary directory behind.
 func (v *Vault) copyVersions(
 	srcVault *Vault, srcIdentity *age.X25519Identity,
-	srcSecretName, srcSecretDir, destSecretName, destSecretDir string,
+	srcSecretName, destSecretName, destSecretDir string,
 	versions []string, currentVersion string,
 ) error {
 	buildDir, err := secret.TempDirFor(v.fs, destSecretDir)
@@ -818,14 +823,6 @@ func (v *Vault) copyVersions(
 		if err != nil {
 			return fmt.Errorf("failed to copy version %s: %w", versionName, err)
 		}
-
-		valuePath := filepath.Join("versions", versionName, "value.age")
-
-		err = copyModTime(srcVault.fs, filepath.Join(srcSecretDir, valuePath),
-			v.fs, filepath.Join(buildDir, valuePath))
-		if err != nil {
-			return err
-		}
 	}
 
 	err = secret.SetCurrentVersion(v.fs, buildDir, currentVersion)
@@ -833,12 +830,6 @@ func (v *Vault) copyVersions(
 		return fmt.Errorf("failed to set current version: %w", err)
 	}
 
-	err = copyModTime(srcVault.fs, filepath.Join(srcSecretDir, "current"),
-		v.fs, filepath.Join(buildDir, "current"))
-	if err != nil {
-		return err
-	}
-
 	// With --force, the secret being replaced goes only now that its
 	// replacement is complete
 	exists, err := afero.DirExists(v.fs, destSecretDir)
@@ -863,26 +854,6 @@ func (v *Vault) copyVersions(
 	return nil
 }
 
-// copyModTime gives destPath in destFs the modification time of srcPath in
-// srcFs.
-func copyModTime(
-	srcFs afero.Fs, srcPath string, destFs afero.Fs, destPath string,
-) error {
-	info, err := srcFs.Stat(srcPath)
-	if err != nil {
-		return fmt.Errorf("failed to read modification time of %s: %w",
-			srcPath, err)
-	}
-
-	err = destFs.Chtimes(destPath, info.ModTime(), info.ModTime())
-	if err != nil {
-		return fmt.Errorf("failed to set modification time of %s: %w",
-			destPath, err)
-	}
-
-	return nil
-}
-
 // checkCopyDestination refuses to copy over an existing secret unless force
 // is set. A secret being replaced is removed by copyVersions, once its
 // replacement is complete.

internal/vault/vault.go

@@ -291,13 +291,10 @@ func (v *Vault) unlockLongTermKey(
 // without a mnemonic records none, and without its metadata the key cannot
 // be derived. It is not added when the passphrase could not be read: the
 // unlocker was not tried, and adding one would need a passphrase read the
-// same way. Nor is it added when the passphrase is wrong: the unlocker still
-// opens the vault with the right one. Nor is it added when the current
-// unlocker does not exist and the vault has others: the error already says
-// to select one of them.
+// same way. Nor is it added when the current unlocker does not exist and the
+// vault has others: the error already says to select one of them.
 func (v *Vault) withMnemonicAdvice(err error) error {
 	if errors.Is(err, secret.ErrPassphraseNotRead) ||
-		errors.Is(err, secret.ErrWrongPassphrase) ||
 		errors.Is(err, ErrUnlockerNotFound) {
 		return err
 	}

internal/vault/vault_error_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"filippo.io/age"
 	"github.com/awnumar/memguard"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -139,7 +140,9 @@ func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) {
 
 	err = vlt.AddSecret(testSecretName, second, true)
 
-	require.ErrorIs(t, err, secret.ErrWrongPassphrase)
+	var noMatch *age.NoIdentityMatchError
+
+	require.ErrorAs(t, err, &noMatch)
 	assert.Equal(t, before, dirContents(t, fs, secretDir))
 }