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

@@ -127,6 +127,17 @@ one.
 
 ### Secret Management
 
+#### Secret Size
+
+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]`
 
 Adds a secret to the current vault. Reads the secret value from stdin.
@@ -150,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
@@ -188,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` ⚠️ 🛑
 
@@ -390,6 +392,11 @@ which every command except `secret vault import` offers when the variable is not
 set, is the safer default; `secret vault import` has no prompt and needs both
 variables.
 
+The prompt asks on the terminal the command runs in, not on stdin, so it also
+asks when stdin is a pipe, as in `echo value | secret add name --force` or
+`cat file | secret encrypt key`. With no controlling terminal, as under cron or
+in a CI job, the command fails and names the variable to set.
+
 ## Security Features
 
 ### Encryption

TODO.md

@@ -18,17 +18,27 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Completed Steps
 
-- 2026-10-14: `secret list` gives each secret's own creation and last-update
-  times (https://git.eeqj.de/sneak/secret/issues/142). It gave the time of the
-  listing for both, which `Secret.LoadMetadata` set to the current time.
-  `LoadMetadata` now reads file modification times, so listing still needs no
-  passphrase: the last update is when the secret's `current` file was written,
-  and the creation is when its oldest version's `value.age` was written. That
-  version's directory would be wrong, because the next add rewrites the
-  version's `metadata.age` in it. `GetSecretObject`, which `secret get` also
-  uses, no longer loads the times; `secret list` loads them itself. A move to
-  another vault gives each copied `value.age` and `current` the modification
-  time of the file it was copied from, so the moved secret keeps its times.
+- 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
+  for the passphrase at a terminal instead of failing
+  (https://git.eeqj.de/sneak/secret/issues/144). Only a process with no
+  controlling terminal gets the error that names the variable to set. On SIGINT
+  or SIGTERM, `Entry()` restores the settings of that terminal, not of stdin,
+  since that is where the prompt turns echo off. `secret init` and
+  `secret vault create` no longer write a newline to stderr after the mnemonic
+  is typed, since the prompt ends that line on the terminal.
 - 2026-10-14: `secret generate mnemonic`, `secret vault list` and
   `secret unlocker list` print their output to stdout, as `secret list` does, so
   it can be piped or captured (https://git.eeqj.de/sneak/secret/issues/138).

internal/cli/create_vault_test.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"slices"
 	"strings"
+	"syscall"
 	"testing"
 
 	"github.com/awnumar/memguard"
@@ -197,10 +198,10 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
 
 // TestMnemonicNotReadNamesOnlyMnemonic is a regression test for
 // https://git.eeqj.de/sneak/secret/issues/115: `secret init` without
-// SB_SECRET_MNEMONIC and with a stdin that is not a terminal said "failed to
-// read mnemonic: failed to read passphrase: ...". The error must wrap
+// SB_SECRET_MNEMONIC and with no terminal said "failed to read mnemonic:
+// failed to read passphrase: ...". The error must wrap
 // secret.ErrMnemonicNotRead and name the mnemonic only. The message is
-// pinned on the built binary, whose stdin is surely not a terminal.
+// pinned on the built binary, run in a new session, which has no terminal.
 func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 	t.Parallel()
 
@@ -216,14 +217,15 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 		"PATH=" + os.Getenv("PATH"),
 		"HOME=" + os.Getenv("HOME"),
 	}
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
 
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
 	require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+
-		"Error: failed to read mnemonic: stdin is not a terminal (piped input "+
-		"or script). Please set the SB_SECRET_MNEMONIC environment variable "+
-		"or run interactively\n", string(output))
+		"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))
 }
 
 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for

internal/cli/generate.go

@@ -133,10 +133,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

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"log"
 	"log/slog"
-	"os"
 	"strings"
 
 	"github.com/awnumar/memguard"
@@ -57,13 +56,11 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) {
 	// Read mnemonic securely without echo
 	mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ")
 	if err != nil {
-		secret.Debug("Failed to read mnemonic from stdin", "error", err)
+		secret.Debug("Failed to read mnemonic", "error", err)
 
 		return nil, nil, err
 	}
 
-	fmt.Fprintln(os.Stderr) // Add newline after hidden input
-
 	return mnemonicBuffer, mnemonicBuffer.Destroy, nil
 }
 

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/passphrase_prompt_test.go

@@ -0,0 +1,204 @@
+package cli_test
+
+import (
+	"bufio"
+	"bytes"
+	"context"
+	"io"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+	"testing"
+
+	"filippo.io/age"
+	"github.com/awnumar/memguard"
+	"github.com/creack/pty"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/secret"
+	"sneak.berlin/go/secret/internal/vault"
+)
+
+// TestPassphraseAskedAtTerminalWithPipedStdin is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/144: run at a terminal with stdin
+// a pipe, as in `echo v2 | secret add s1 --force`, a command that needed
+// the passphrase failed instead of asking for it. Each command here must ask
+// on the terminal and succeed once the passphrase is typed there.
+func TestPassphraseAskedAtTerminalWithPipedStdin(t *testing.T) {
+	t.Parallel()
+
+	stateDir := t.TempDir()
+
+	passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+	defer passphrase.Destroy()
+
+	vlt, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
+		testMnemonicBuffer(t), passphrase)
+	require.NoError(t, err)
+
+	value := memguard.NewBufferFromBytes([]byte("v1"))
+	defer value.Destroy()
+
+	require.NoError(t, vlt.AddSecret("s1", value, false))
+
+	identity, err := age.GenerateX25519Identity()
+	require.NoError(t, err)
+
+	key := secret.IdentityToLockedBuffer(identity)
+	defer key.Destroy()
+
+	require.NoError(t, vlt.AddSecret("mykey", key, false))
+
+	runAtTerminal(t, stateDir, "v2\n", "add", "s1", "--force")
+
+	vlt.UnlockPassphrase = passphrase
+
+	added, err := vlt.GetSecret("s1")
+	require.NoError(t, err)
+
+	defer added.Destroy()
+
+	assert.Equal(t, "v2", added.String())
+
+	encrypted := runAtTerminal(t, stateDir, "a document\n", "encrypt", "mykey")
+	decrypted := runAtTerminal(t, stateDir, encrypted, "decrypt", "mykey")
+	assert.Equal(t, "a document\n", decrypted)
+}
+
+// TestInitWritesNoBlankLineAfterMnemonic runs `secret init 2>log` at a
+// terminal: the terminal is its controlling terminal and none of stdin,
+// stdout and stderr. The mnemonic is typed on the terminal and the prompt
+// ends its line there, so stderr must not get a newline for it as well, a
+// blank line in the log. pty.Open is described above
+// TestRemoveIgnoresTerminalOnStdout.
+func TestInitWritesNoBlankLineAfterMnemonic(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	stateDir := t.TempDir()
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, secretBinaryPath(t), "init")
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		secret.EnvUnlockPassphrase + "=" + testPassphrase,
+		"PATH=" + os.Getenv("PATH"),
+		"HOME=" + os.Getenv("HOME"),
+	}
+
+	ptmx, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	defer func() { _ = ptmx.Close() }()
+
+	deadline, _ := ctx.Deadline()
+	require.NoError(t, ptmx.SetReadDeadline(deadline))
+
+	var stderr strings.Builder
+
+	cmd.Stderr = &stderr
+	cmd.ExtraFiles = []*os.File{tty}
+	// Ctty is a descriptor in the new process: 3, the first of ExtraFiles.
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 3}
+
+	require.NoError(t, cmd.Start())
+
+	_ = tty.Close()
+
+	var (
+		shown []byte
+		char  byte
+	)
+
+	terminal := bufio.NewReader(ptmx)
+	for !bytes.HasSuffix(shown, []byte("Enter your BIP39 mnemonic phrase: ")) {
+		char, err = terminal.ReadByte()
+		require.NoError(t, err, "secret init did not ask on the terminal: %s",
+			shown)
+
+		shown = append(shown, char)
+	}
+
+	_, err = ptmx.WriteString(testMnemonic + "\n")
+	require.NoError(t, err)
+
+	// The read ends once the command has exited and so closed the terminal.
+	rest, _ := io.ReadAll(terminal)
+
+	err = cmd.Wait()
+	require.NoError(t, err, "secret init: %s%s%s", shown, rest, stderr.String())
+
+	assert.True(t, strings.HasPrefix(stderr.String(),
+		"Initialized secrets manager at: "+stateDir+"\n\n"+
+			"Default vault created and configured\n"),
+		"stderr: %q", stderr.String())
+}
+
+// runAtTerminal runs the secret binary with args and input on stdin, in a
+// new session whose controlling terminal is a new terminal, which is also
+// its stderr: a command typed at a terminal with its input piped. pty.Open
+// is described above TestRemoveIgnoresTerminalOnStdout. runAtTerminal waits
+// for the passphrase prompt on the terminal, types the passphrase there,
+// and returns what the command wrote to stdout once it has succeeded.
+func runAtTerminal(t *testing.T, stateDir, input string, args ...string) string {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, secretBinaryPath(t), args...)
+	cmd.Env = []string{
+		secret.EnvStateDir + "=" + stateDir,
+		"PATH=" + os.Getenv("PATH"),
+		"HOME=" + os.Getenv("HOME"),
+	}
+
+	ptmx, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	defer func() { _ = ptmx.Close() }()
+
+	deadline, _ := ctx.Deadline()
+	require.NoError(t, ptmx.SetReadDeadline(deadline))
+
+	var stdout strings.Builder
+
+	cmd.Stdin = strings.NewReader(input)
+	cmd.Stdout = &stdout
+	cmd.Stderr = tty
+	// Ctty is a descriptor in the new process: 2, its stderr.
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 2}
+
+	require.NoError(t, cmd.Start())
+
+	_ = tty.Close()
+
+	var (
+		shown []byte
+		char  byte
+	)
+
+	terminal := bufio.NewReader(ptmx)
+	for !bytes.HasSuffix(shown, []byte("Enter unlock passphrase: ")) {
+		char, err = terminal.ReadByte()
+		require.NoError(t, err, "secret %v did not ask on the terminal: %s",
+			args, shown)
+
+		shown = append(shown, char)
+	}
+
+	_, err = ptmx.WriteString(testPassphrase + "\n")
+	require.NoError(t, err)
+
+	// The read ends once the command has exited and so closed the terminal.
+	rest, _ := io.ReadAll(terminal)
+
+	require.NoError(t, cmd.Wait(), "secret %v: %s%s", args, shown, rest)
+
+	return stdout.String()
+}

internal/cli/root.go

@@ -14,17 +14,32 @@ 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 terminal echo
-	// off until the read finishes, so a signal there would leave echo off.
-	// Only a process in the terminal's foreground process group may reset
-	// it: one in the background that tries is stopped instead of exiting.
-	terminalState, terminalErr := term.GetState(unix.Stdin)
+	// and exits with status 1. The passphrase prompt turns echo off on the
+	// controlling terminal until the read finishes, so a signal there would
+	// leave echo off. Only a process in the terminal's foreground process
+	// group may reset it: one in the background that tries is stopped
+	// instead of exiting.
+	terminal, terminalErr := os.Open(secret.TerminalPath)
+
+	var terminalState *term.State
+
+	if terminalErr == nil {
+		defer func() { _ = terminal.Close() }()
+
+		terminalState, terminalErr = term.GetState(int(terminal.Fd()))
+	}
 
 	memguard.CatchSignal(func(os.Signal) {
-		foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP)
-		if terminalErr == nil && err == nil && foreground == unix.Getpgrp() {
-			_ = term.Restore(unix.Stdin, terminalState)
+		if terminalErr != nil {
+			return
+		}
+
+		foreground, err := unix.IoctlGetInt(int(terminal.Fd()), unix.TIOCGPGRP)
+		if err == nil && foreground == unix.Getpgrp() {
+			_ = term.Restore(int(terminal.Fd()), terminalState)
 		}
 	}, os.Interrupt, unix.SIGTERM)
 
@@ -38,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,28 @@ 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 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
 )
 
 // 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
@@ -270,88 +273,100 @@ 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 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
 
-	*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
-}
-
-// destroyBuffers destroys every buffer in the list
-func destroyBuffers(buffers []bufferInfo) {
-	for _, b := range buffers {
-		b.buffer.Destroy()
+	//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)
 	}
+
+	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 +385,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.
@@ -541,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
@@ -622,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")
@@ -661,7 +654,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 +666,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,77 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
 		"Retrieved secret should match original")
 }
 
+// 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)
+
+	secretPath, err := filepath.Abs("../../secret")
+	require.NoError(t, err)
+
+	tooBigFile := filepath.Join(t.TempDir(), "too-big")
+	require.NoError(t, os.WriteFile(tooBigFile, make([]byte, fits+1), 0o600))
+
+	run := func(stdin []byte, args ...string) (string, error) {
+		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 = 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)
+
+	refused := func(wantError string, stdin []byte, args ...string) {
+		output, err := run(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)
+	}
+
+	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
 type slowReader struct {
 	data      []byte

internal/cli/unlock_failure_test.go

@@ -18,6 +18,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"syscall"
 	"testing"
 
 	"filippo.io/age"
@@ -288,7 +289,7 @@ func TestMissingCurrentUnlockerNamesMnemonic(t *testing.T) {
 }
 
 // TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built
-// binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a
+// binary without SB_UNLOCK_PASSPHRASE and in a new session, which has no
 // terminal, so the passphrase cannot be read. The unlocker was not tried,
 // and adding one would need a passphrase read the same way, so the error
 // is the cause alone, without the advice to use the mnemonic.
@@ -319,15 +320,16 @@ func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) {
 		"PATH=" + os.Getenv("PATH"),
 		"HOME=" + os.Getenv("HOME"),
 	}
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
 
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
 	assert.Equal(t, "Error: failed to unlock vault: "+
 		"failed to get long-term key: failed to get unlocker identity: "+
-		"failed to read passphrase: stdin is not a terminal (piped input or "+
-		"script). Please set the SB_UNLOCK_PASSPHRASE environment variable or "+
-		"run interactively\n", string(output))
+		"failed to read passphrase: there is no terminal to ask on. Please "+
+		"set the SB_UNLOCK_PASSPHRASE environment variable or run "+
+		"interactively\n", string(output))
 }
 
 // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and

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/constants.go

@@ -17,6 +17,10 @@ const (
 	// EnvGPGKeyID is the environment variable for providing the GPG key ID
 	EnvGPGKeyID = "SB_GPG_KEY_ID"
 
+	// TerminalPath opens the controlling terminal of the process that opens
+	// it, where the passphrase and mnemonic prompts ask.
+	TerminalPath = "/dev/tty"
+
 	// UnlockerTimeFormat is the layout of the time, in UTC, in the name of a
 	// new unlocker's directory. It runs to the nanosecond, so that every new
 	// unlocker, even one added right after another, gets a directory of its

internal/secret/crypto.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"syscall"
 	"unsafe"
 
 	"filippo.io/age"
@@ -16,11 +15,8 @@ import (
 
 var (
 	errNilPassphraseBuffer = errors.New("passphrase buffer is nil")
-	errStdinNotTerminal    = errors.New(
-		"stdin is not a terminal (piped input or script)")
-	errStderrNotTerminal = errors.New(
-		"stderr is not a terminal (running in non-interactive mode)")
-	errNothingEntered = errors.New("nothing was entered")
+	errNoTerminal          = errors.New("there is no terminal to ask on")
+	errNothingEntered      = errors.New("nothing was entered")
 )
 
 // ErrMnemonicNotRead is wrapped in every error of ReadMnemonic: there is no
@@ -193,38 +189,33 @@ func ReadMnemonic(prompt string) (*memguard.LockedBuffer, error) {
 	return readFromTerminal(prompt, ErrMnemonicNotRead, EnvMnemonic)
 }
 
-// readFromTerminal reads input from the terminal without echoing it. Every
-// error it returns wraps notRead; without a terminal, the error says to set
-// envVar instead.
+// readFromTerminal asks on the controlling terminal and reads the answer
+// there without echoing it, whatever stdin is: in
+// `echo value | secret add name --force`, stdin is a pipe. Every error it
+// returns wraps notRead; without a terminal, the error says to set envVar
+// instead.
 func readFromTerminal(
 	prompt string, notRead error, envVar string,
 ) (*memguard.LockedBuffer, error) {
-	// Check if stdin is a terminal
-	if !term.IsTerminal(syscall.Stdin) {
-		// Not a terminal - never read secrets from piped input
-		// for security reasons
+	terminal, err := os.OpenFile(TerminalPath, os.O_RDWR, 0)
+	if err != nil {
+		Debug("Failed to open the controlling terminal", "error", err)
+
 		return nil, fmt.Errorf(
 			"%w: %w. Please set the %s environment variable or run interactively",
-			notRead, errStdinNotTerminal, envVar)
+			notRead, errNoTerminal, envVar)
 	}
+	defer func() { _ = terminal.Close() }()
 
-	// stdin is a terminal, check if stderr is also a terminal for
-	// interactive prompting
-	if !term.IsTerminal(syscall.Stderr) {
-		return nil, fmt.Errorf("%w: %w. Please set the %s environment variable",
-			notRead, errStderrNotTerminal, envVar)
-	}
+	_, _ = fmt.Fprint(terminal, prompt)
 
-	// Both stdin and stderr are terminals - use secure password reading
-	fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout
-
-	input, err := term.ReadPassword(syscall.Stdin)
+	input, err := term.ReadPassword(int(terminal.Fd()))
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", notRead, err)
 	}
 
-	// Print newline to stderr since ReadPassword doesn't echo
-	fmt.Fprintln(os.Stderr)
+	// The newline typed after the input, which ReadPassword did not echo
+	_, _ = fmt.Fprintln(terminal)
 
 	if len(input) == 0 {
 		return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered)

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.