secret list gives each secret's own creation and last-update times (closes #142)

secret list and secret list --json gave the time of the listing as every
secret's creation and last-update time, because Secret.LoadMetadata set
both to the current time. It 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.

GetSecretObject no longer loads the times, so secret get reads no more
files than before and keeps its error for a missing current file;
secret list calls LoadMetadata itself.

Judgement call: the creation time comes from value.age, not from the
oldest version's directory, whose time changes when the next add
rewrites that version's metadata.age.

Model: opus-5-5

README.md

@@ -150,6 +150,12 @@ 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.
+
 #### `secret remove <secret-name> [--force]` / `secret rm` ⚠️ 🛑
 
 **DANGER**: Permanently removes a secret and ALL its versions. It first asks for
@@ -241,6 +247,14 @@ that the vault then opens only with its mnemonic (see
 [Confirmation Before Removal](#confirmation-before-removal)). An unlocker
 directory that `secret unlocker list` skips with a warning, because its metadata
 cannot be read or parsed, is removed by the directory name the warning gives.
+Removing the current unlocker makes the first of the vault's other unlockers
+that `secret unlocker select` accepts, in the order `secret unlocker list` shows
+them, the current one, and says which; when there is none, the vault is left
+without a current unlocker. `secret unlocker select` refuses an unlocker whose
+type is not `passphrase` or `pgp`. When `current-unlocker` names an unlocker
+that does not exist and the vault has another that `secret unlocker select`
+accepts, opening the vault fails with an error that says to select one with
+`secret unlocker select`.
 
 - `--force, -f`: Remove without asking, even the last unlocker
 - **CRITICAL WARNING**: Without unlockers and without your mnemonic phrase,

TODO.md

@@ -18,6 +18,36 @@ 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.
+- 2026-10-14: Removing the current unlocker no longer leaves the vault unable to
+  open through the unlockers it still has
+  (https://git.eeqj.de/sneak/secret/issues/137). `current-unlocker` went on
+  naming the removed unlocker, so every command that opened the vault failed,
+  with advice to use the mnemonic. `secret unlocker remove` of the current
+  unlocker now makes the first of the vault's other unlockers that
+  `secret unlocker select` accepts, in the order `secret unlocker list` shows,
+  current before removing it, and says so; when there is none, it also removes
+  `current-unlocker`. `vault.RemoveUnlocker` returns the ID of the unlocker it
+  made current. When `current-unlocker` names an unlocker that does not exist
+  and the vault has another that `secret unlocker select` accepts, the error
+  wraps `vault.ErrUnlockerNotFound` and says to run `secret unlocker select`,
+  without the advice to use the mnemonic.
+- 2026-10-14: A `secret add --force` that cannot unlock the vault, given a wrong
+  passphrase or mnemonic, leaves the secret as it was
+  (https://git.eeqj.de/sneak/secret/issues/136). The new version used to be
+  written before the vault was unlocked to set `notAfter` on the version it
+  replaces, so each failed add left one more version that `secret version list`
+  showed. The vault is now unlocked, and that version's metadata read, before
+  anything is written. `generate secret --force` and `import --force` take the
+  same path.
 - 2026-10-12: A mnemonic typed with extra spaces, tabs or line breaks gives the
   same long-term key as its words separated by single spaces
   (https://git.eeqj.de/sneak/secret/issues/135). The key was derived from the

internal/cli/list_times_test.go

@@ -0,0 +1,118 @@
+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"))
+}
+
+// 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

@@ -541,8 +541,12 @@ func printSecretsJSON(
 			"name": secretName,
 		}
 
-		// Try to get metadata using GetSecretObject
+		// A secret whose times cannot be read is listed without them
 		secretObj, err := vlt.GetSecretObject(secretName)
+		if err == nil {
+			err = secretObj.LoadMetadata()
+		}
+
 		if err == nil {
 			metadata := secretObj.GetMetadata()
 			secretInfo["created_at"] = metadata.CreatedAt
@@ -618,6 +622,10 @@ 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/unlock_failure_test.go

@@ -4,9 +4,11 @@
 // 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 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.
+// 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.
 
 package cli_test
 
@@ -222,6 +224,69 @@ func TestMoveUnlockFailureNamesVault(t *testing.T) {
 	assert.Equal(t, "value", getSecret(t, c, "y"))
 }
 
+// TestMissingCurrentUnlockerNamesSelect checks the error of `secret get`
+// when current-unlocker names an unlocker that does not exist and the vault
+// has another: it names the missing unlocker and says to select another
+// one, without the advice to use the mnemonic. The test then follows that
+// advice.
+func TestMissingCurrentUnlockerNamesSelect(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+	unlockerDir, c := newUnlockTestCLI(t, fs)
+
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(unlockTestVaultDir, "current-unlocker"),
+		[]byte("missing"), secret.FilePerms))
+
+	err := c.GetSecret(discardCmd(), "x")
+
+	require.ErrorIs(t, err, vault.ErrUnlockerNotFound)
+	require.EqualError(t, err, "failed to unlock vault: "+
+		"failed to get long-term key: failed to get current unlocker: "+
+		"unlocker with ID missing not found; the vault 'default' has other "+
+		"unlockers: run 'secret unlocker select' with the ID of one of them "+
+		"to make it the current unlocker")
+
+	require.NoError(t, c.UnlockerSelect(filepath.Base(unlockerDir)))
+	assert.Equal(t, "value", getSecret(t, c, "x"))
+}
+
+// TestMissingCurrentUnlockerNamesMnemonic checks the error of `secret get`
+// when the unlocker that current-unlocker names does not exist and the
+// vault's only other unlocker is the directory "odd", whose metadata names a
+// type that `secret unlocker select` refuses: the error ends with the advice
+// that the mnemonic still opens the vault, not with advice to select another
+// unlocker. The test then follows that advice.
+func TestMissingCurrentUnlockerNamesMnemonic(t *testing.T) {
+	t.Parallel()
+
+	fs := newTwoVaultFs(t)
+	unlockerDir, c := newUnlockTestCLI(t, fs)
+
+	require.NoError(t, fs.RemoveAll(unlockerDir))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(unlockTestVaultDir, "unlockers.d", "odd",
+			"unlocker-metadata.json"),
+		[]byte(`{"type": "odd"}`), secret.FilePerms))
+
+	err := c.GetSecret(discardCmd(), "x")
+
+	var cause *os.PathError
+
+	require.ErrorAs(t, err, &cause)
+	require.NotErrorIs(t, err, vault.ErrUnlockerNotFound)
+	require.EqualError(t, err, "failed to unlock vault: "+
+		"failed to get long-term key: failed to get current unlocker: "+
+		"failed to read unlocker metadata: "+cause.Error()+mnemonicAdvice)
+
+	c.Mnemonic = testMnemonicBuffer(t)
+	require.NoError(t, c.UnlockersAdd("passphrase", discardCmd()))
+
+	c.Mnemonic = nil
+	assert.Equal(t, "value", getSecret(t, c, "x"))
+}
+
 // TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built
 // binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a
 // terminal, so the passphrase cannot be read. The unlocker was not tried,

internal/cli/unlockers.go

@@ -222,8 +222,11 @@ func newUnlockerRemoveCmd() *cobra.Command {
 		Long: `Remove an unlocker from the current vault. Asks for ` +
 			`confirmation first, saying whether it is the vault's last ` +
 			`unlocker; when stdin is not a terminal, fails unless --force ` +
-			`is given. Warning: Without unlockers and without your ` +
-			`mnemonic, vault data will be permanently inaccessible.`,
+			`is given. Removing the current unlocker makes another of the ` +
+			`vault's unlockers current, if it has one that ` +
+			`'secret unlocker select' accepts. Warning: Without ` +
+			`unlockers and without your mnemonic, vault data will be ` +
+			`permanently inaccessible.`,
 		Args:              cobra.ExactArgs(1),
 		ValidArgsFunction: getUnlockerIDsCompletionFunc(cli.fs, cli.stateDir),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -612,13 +615,17 @@ func (cli *Instance) removeUnlocker(
 			"have your mnemonic phrase to access this vault again!")
 	}
 
-	err := found.vlt.RemoveUnlocker(unlockerID)
+	newCurrentID, err := found.vlt.RemoveUnlocker(unlockerID)
 	if err != nil {
 		return err
 	}
 
 	cmd.Printf("Removed unlocker '%s'\n", unlockerID)
 
+	if newCurrentID != "" {
+		cmd.Printf("Switched current unlocker to '%s'\n", newCurrentID)
+	}
+
 	return nil
 }
 

internal/cli/unlockers_remove_test.go

@@ -0,0 +1,94 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"bytes"
+	"path/filepath"
+	"testing"
+
+	"github.com/awnumar/memguard"
+	"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"
+)
+
+// TestUnlockerRemoveCurrent is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/137. A vault has a passphrase
+// unlocker and a PGP unlocker, the current one. Removing the PGP unlocker
+// makes the passphrase unlocker current and says so, and the vault then
+// opens through it. Removing the passphrase unlocker too removes
+// current-unlocker.
+//
+// The second case also gives the vault the unlocker directory "odd", whose
+// metadata names a type that `secret unlocker select` refuses, and which
+// sorts before the passphrase unlocker: neither removal makes it current.
+//
+//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
+func TestUnlockerRemoveCurrent(t *testing.T) {
+	tests := []struct {
+		name    string
+		withOdd bool
+	}{
+		{name: "two unlockers", withOdd: false},
+		{name: "and the directory odd", withOdd: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			newTestGPGKey(t)
+
+			passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+			t.Cleanup(passphrase.Destroy)
+
+			fs := afero.NewMemMapFs()
+			vlt, err := vault.CreateVault(fs, listTestStateDir,
+				listTestVaultName, testMnemonicBuffer(t), nil)
+			require.NoError(t, err)
+
+			passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphrase)
+			require.NoError(t, err)
+
+			if tt.withOdd {
+				require.NoError(t, afero.WriteFile(fs,
+					filepath.Join(testVaultDir(listTestVaultName), "unlockers.d",
+						"odd", "unlocker-metadata.json"),
+					[]byte(`{"type": "odd"}`), secret.FilePerms))
+			}
+
+			instance, cmd := newTestInstance(fs)
+			instance.UnlockPassphrase = passphrase
+
+			cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
+			require.NoError(t, instance.UnlockersAdd(unlockerTypePGP, cmd))
+
+			pgpUnlocker, err := vlt.GetCurrentUnlocker()
+			require.NoError(t, err)
+			require.Equal(t, unlockerTypePGP, pgpUnlocker.GetType())
+
+			var output bytes.Buffer
+
+			cmd.SetOut(&output)
+			require.NoError(t,
+				instance.UnlockersRemove(pgpUnlocker.GetID(), true, cmd))
+			assert.Contains(t, output.String(),
+				"Switched current unlocker to '"+passphraseUnlocker.GetID()+"'")
+
+			reopened := vault.NewVault(fs, listTestStateDir, listTestVaultName)
+			reopened.UnlockPassphrase = passphrase
+
+			_, err = reopened.GetOrDeriveLongTermKey()
+			require.NoError(t, err)
+
+			require.NoError(t,
+				instance.UnlockersRemove(passphraseUnlocker.GetID(), true, cmd))
+
+			exists, err := afero.Exists(fs, filepath.Join(
+				testVaultDir(listTestVaultName), "current-unlocker"))
+			require.NoError(t, err)
+			assert.False(t, exists,
+				"current-unlocker is left after the passphrase unlocker")
+		})
+	}
+}

internal/secret/secret.go

@@ -2,10 +2,11 @@ package secret
 
 import (
 	"errors"
+	"fmt"
 	"log/slog"
+	"os"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"filippo.io/age"
 	"github.com/awnumar/memguard"
@@ -66,31 +67,53 @@ func NewSecret(vault VaultInterface, name string) *Secret {
 		Name:      name,
 		Directory: secretDir,
 		vault:     vault,
-		Metadata: Metadata{
-			CreatedAt: time.Now(),
-			UpdatedAt: time.Now(),
-		},
 	}
 }
 
-// LoadMetadata is deprecated - metadata is now per-version and encrypted
+// 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.
 func (s *Secret) LoadMetadata() error {
-	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()
+	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)
+	}
+
 	s.Metadata = Metadata{
-		CreatedAt: now,
-		UpdatedAt: now,
+		CreatedAt: value.ModTime(),
+		UpdatedAt: current.ModTime(),
 	}
 
 	return nil
 }
 
-// GetMetadata returns the secret metadata (deprecated)
+// GetMetadata returns the times LoadMetadata read
 func (s *Secret) GetMetadata() Metadata {
-	Debug("GetMetadata called but is deprecated in versioned model", "secret_name", s.Name)
-
 	return s.Metadata
 }
 

internal/vault/errors_test.go

@@ -81,7 +81,9 @@ func TestVaultErrors(t *testing.T) {
 			return vlt.CopySecretAllVersions(vlt, versionless, "copy", false)
 		}, vault.ErrNoVersions},
 		{"remove a missing unlocker", func(vlt *vault.Vault) error {
-			return vlt.RemoveUnlocker(missingName)
+			_, err := vlt.RemoveUnlocker(missingName)
+
+			return err
 		}, vault.ErrUnlockerNotFound},
 		{"select a missing unlocker", func(vlt *vault.Vault) error {
 			return vlt.SelectUnlocker(missingName)

internal/vault/secrets.go

@@ -219,6 +219,15 @@ func (v *Vault) addVersion(
 	name, secretDir string, value *memguard.LockedBuffer,
 	previousVersion *secret.Version,
 ) error {
+	// Unlock the vault before writing anything, so that a wrong passphrase
+	// or mnemonic leaves the secret as it was
+	err := v.loadPreviousVersionMetadata(previousVersion)
+	if err != nil {
+		return err
+	}
+
+	// Taken after the unlock, which can wait for the user to type the
+	// passphrase, so that the new version starts when it is created
 	now := time.Now()
 
 	// Create the new version and save the encrypted value
@@ -418,7 +427,8 @@ func (v *Vault) UnlockVault() (*age.X25519Identity, error) {
 	return longTermIdentity, nil
 }
 
-// GetSecretObject retrieves a Secret object with metadata loaded from this vault
+// GetSecretObject retrieves a Secret object from this vault. Its times are
+// zero until its LoadMetadata reads them.
 func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
 	err := ValidateSecretName(name)
 	if err != nil {
@@ -445,16 +455,7 @@ func (v *Vault) GetSecretObject(name string) (*secret.Secret, error) {
 		return nil, fmt.Errorf("secret %s %w", name, ErrSecretNotFound)
 	}
 
-	// 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
+	return secret.NewSecret(v, name), nil
 }
 
 // CopySecretVersion copies a single version from source into destSecretDir
@@ -632,8 +633,35 @@ func (v *Vault) checkExistingSecret(
 	return true, previousVersion, nil
 }
 
+// loadPreviousVersionMetadata unlocks the vault and loads the metadata of
+// the version being superseded. It is a no-op when previousVersion is nil.
+func (v *Vault) loadPreviousVersionMetadata(
+	previousVersion *secret.Version,
+) error {
+	if previousVersion == nil {
+		return nil
+	}
+
+	ltIdentity, err := v.GetOrDeriveLongTermKey()
+	if err != nil {
+		secret.Debug("Failed to get long-term key for previous version", "error", err)
+
+		return fmt.Errorf("failed to get long-term key: %w", err)
+	}
+
+	err = previousVersion.LoadMetadata(ltIdentity)
+	if err != nil {
+		secret.Debug("Failed to load previous version metadata", "error", err)
+
+		return fmt.Errorf("failed to load previous version metadata: %w", err)
+	}
+
+	return nil
+}
+
 // updatePreviousVersion sets the notAfter timestamp on the version being
-// superseded. It is a no-op when previousVersion is nil.
+// superseded, whose metadata loadPreviousVersionMetadata has loaded. It is
+// a no-op when previousVersion is nil.
 func (v *Vault) updatePreviousVersion(
 	previousVersion *secret.Version, now *time.Time,
 ) error {
@@ -641,22 +669,13 @@ func (v *Vault) updatePreviousVersion(
 		return nil
 	}
 
-	// Get long-term key to decrypt/encrypt metadata
+	// loadPreviousVersionMetadata unlocked the vault, so this returns the
+	// key it keeps in memory
 	ltIdentity, err := v.GetOrDeriveLongTermKey()
 	if err != nil {
-		secret.Debug("Failed to get long-term key for metadata update", "error", err)
-
 		return fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
-	// Load previous version metadata
-	err = previousVersion.LoadMetadata(ltIdentity)
-	if err != nil {
-		secret.Debug("Failed to load previous version metadata", "error", err)
-
-		return fmt.Errorf("failed to load previous version metadata: %w", err)
-	}
-
 	// Update notAfter timestamp
 	previousVersion.Metadata.NotAfter = now
 

internal/vault/secrets_version_test.go

@@ -10,6 +10,8 @@
 //   version
 // - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter)
 //   across versions
+// - TestVaultVersionTimestampsAfterUnlock: Tests that those timestamps are
+//   taken after AddSecret unlocks the vault
 // - TestVaultGetNonExistentVersion: Tests error handling for invalid versions
 // - TestUpdateVersionMetadata: Tests metadata update functionality
 //
@@ -295,6 +297,70 @@ func TestVaultVersionTimestamps(t *testing.T) {
 	assert.Nil(t, secondVersion.Metadata.NotAfter) // Current version
 }
 
+// longTermKeyReadFs records when longterm.age, the last file an unlock
+// reads, was last opened.
+type longTermKeyReadFs struct {
+	afero.Fs
+
+	readAt time.Time
+}
+
+//nolint:ireturn // afero.File is the interface required by afero.Fs
+func (f *longTermKeyReadFs) Open(name string) (afero.File, error) {
+	if filepath.Base(name) == "longterm.age" {
+		f.readAt = time.Now()
+	}
+
+	return f.Fs.Open(name)
+}
+
+// TestVaultVersionTimestampsAfterUnlock checks that when AddSecret has to
+// unlock the vault to replace a version, the new version's notBefore and the
+// replaced version's notAfter are taken after the unlock, which can wait for
+// the user to type the passphrase.
+func TestVaultVersionTimestampsAfterUnlock(t *testing.T) {
+	t.Parallel()
+
+	fs := &longTermKeyReadFs{Fs: afero.NewMemMapFs()}
+
+	mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
+	defer mnemonic.Destroy()
+
+	passphrase := memguard.NewBufferFromBytes([]byte("test-passphrase"))
+	defer passphrase.Destroy()
+
+	created, err := CreateVault(fs, testStateDir, "test", mnemonic, passphrase)
+	require.NoError(t, err)
+
+	addTestSecretToVault(t, created, testSecretPath, []byte("version-1"), false)
+
+	// A new instance is locked; AddSecret unlocks it with the passphrase
+	vault := NewVault(fs, testStateDir, "test")
+	vault.SetUnlockPassphrase(passphrase)
+
+	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)
+	require.False(t, fs.readAt.IsZero(), "AddSecret did not unlock the vault")
+
+	ltIdentity, err := vault.GetOrDeriveLongTermKey()
+	require.NoError(t, err)
+
+	vaultDir, _ := vault.GetDirectory()
+	secretDir := filepath.Join(vaultDir, "secrets.d", "test%secret")
+
+	versions, err := secret.ListVersions(fs, secretDir)
+	require.NoError(t, err)
+	require.Len(t, versions, 2)
+
+	newVersion := secret.NewVersion(vault, testSecretPath, versions[0])
+	require.NoError(t, newVersion.LoadMetadata(ltIdentity))
+
+	replacedVersion := secret.NewVersion(vault, testSecretPath, versions[1])
+	require.NoError(t, replacedVersion.LoadMetadata(ltIdentity))
+
+	assert.False(t, newVersion.Metadata.NotBefore.Before(fs.readAt))
+	assert.False(t, replacedVersion.Metadata.NotAfter.Before(fs.readAt))
+}
+
 func TestVaultGetNonExistentVersion(t *testing.T) {
 	t.Parallel()
 

internal/vault/unlockers.go

@@ -5,8 +5,10 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"time"
 
@@ -19,6 +21,7 @@ import (
 // Unlocker metadata type strings.
 const (
 	unlockerTypePassphrase = "passphrase"
+	unlockerTypePGP        = "pgp"
 )
 
 // GetCurrentUnlocker returns the current unlocker for this vault
@@ -52,6 +55,11 @@ func (v *Vault) GetCurrentUnlocker() (secret.Unlocker, error) {
 		return nil, err
 	}
 
+	err = v.checkCurrentUnlockerExists(unlockerDir)
+	if err != nil {
+		return nil, err
+	}
+
 	secret.DebugWith("Resolved unlocker directory",
 		slog.String("unlocker_dir", unlockerDir),
 		slog.String("vault_name", v.Name),
@@ -74,7 +82,7 @@ func (v *Vault) GetCurrentUnlocker() (secret.Unlocker, error) {
 		passphraseUnlocker := secret.NewPassphraseUnlocker(v.fs, unlockerDir, metadata)
 		passphraseUnlocker.Passphrase = v.UnlockPassphrase
 		unlocker = passphraseUnlocker
-	case "pgp":
+	case unlockerTypePGP:
 		secret.Debug("Creating PGP unlocker instance", "unlocker_type", metadata.Type)
 
 		unlocker = secret.NewPGPUnlocker(v.fs, unlockerDir, metadata)
@@ -119,6 +127,37 @@ func (v *Vault) resolveUnlockerDirectory(currentUnlockerPath string) (string, er
 	return absolutePath, nil
 }
 
+// checkCurrentUnlockerExists fails with ErrUnlockerNotFound, saying to
+// select another unlocker, when unlockerDir, the unlocker directory that
+// current-unlocker names, does not exist and the vault has another unlocker
+// that SelectUnlocker accepts. When it has none, it returns nil: reading the
+// missing unlocker then fails, with the advice that the mnemonic still opens
+// the vault.
+func (v *Vault) checkCurrentUnlockerExists(unlockerDir string) error {
+	exists, err := afero.DirExists(v.fs, unlockerDir)
+	if err != nil {
+		return fmt.Errorf("failed to check for the current unlocker: %w", err)
+	}
+
+	if exists {
+		return nil
+	}
+
+	otherID, err := v.firstOtherUnlockerID(filepath.Base(unlockerDir))
+	if err != nil {
+		return err
+	}
+
+	if otherID == "" {
+		return nil
+	}
+
+	return fmt.Errorf("unlocker with ID %s %w; the vault '%s' has other "+
+		"unlockers: run %s with the ID of one of them to make it the current "+
+		"unlocker", filepath.Base(unlockerDir), ErrUnlockerNotFound, v.Name,
+		v.withVaultSelect("'secret unlocker select'"))
+}
+
 // findUnlockerByID finds an unlocker by its ID and returns the unlocker
 // instance and its directory path. A directory that ListUnlockers skips is
 // skipped here too, with the same warning. Such a directory has no ID: if
@@ -159,7 +198,7 @@ func (v *Vault) findUnlockerByID(
 		switch metadata.Type {
 		case unlockerTypePassphrase:
 			tempUnlocker = secret.NewPassphraseUnlocker(v.fs, unlockerDirPath, metadata)
-		case "pgp":
+		case unlockerTypePGP:
 			tempUnlocker = secret.NewPGPUnlocker(v.fs, unlockerDirPath, metadata)
 		default:
 			continue
@@ -282,10 +321,15 @@ func (v *Vault) HasUnlocker(unlockerID string) (bool, error) {
 // RemoveUnlocker removes an unlocker from this vault. An unlocker
 // directory that ListUnlockers skips is removed by its directory name; its
 // type is unknown, so only the directory is removed.
-func (v *Vault) RemoveUnlocker(unlockerID string) error {
+//
+// It returns the ID of the unlocker it made current, or "" when it made none
+// current. If the removed unlocker is the current one, the unlocker
+// firstOtherUnlockerID returns is made current before the removal; with
+// none, current-unlocker is removed after it.
+func (v *Vault) RemoveUnlocker(unlockerID string) (string, error) {
 	vaultDir, err := v.GetDirectory()
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	// Find the unlocker directory and create the unlocker instance
@@ -294,19 +338,92 @@ func (v *Vault) RemoveUnlocker(unlockerID string) error {
 	// Find the unlocker by ID
 	unlocker, unlockerDir, err := v.findUnlockerByID(unlockersDir, unlockerID)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	if unlockerDir == "" {
-		return fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
+		return "", fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
+	}
+
+	currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker")
+
+	currentDir, err := v.resolveUnlockerDirectory(currentUnlockerPath)
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return "", err
+	}
+
+	isCurrent := currentDir == unlockerDir
+	newCurrentID := ""
+
+	if isCurrent {
+		newCurrentID, err = v.selectOtherUnlocker(unlockerID)
+		if err != nil {
+			return "", err
+		}
 	}
 
 	if unlocker == nil {
-		return secret.RemoveDirAtomic(v.fs, unlockerDir)
+		err = secret.RemoveDirAtomic(v.fs, unlockerDir)
+	} else {
+		err = unlocker.Remove()
 	}
 
-	// Use the unlocker's Remove method
-	return unlocker.Remove()
+	if err != nil {
+		return "", err
+	}
+
+	if isCurrent && newCurrentID == "" {
+		err = v.fs.Remove(currentUnlockerPath)
+		if err != nil {
+			return "", fmt.Errorf("failed to remove current-unlocker file: %w", err)
+		}
+	}
+
+	return newCurrentID, nil
+}
+
+// selectOtherUnlocker makes the unlocker firstOtherUnlockerID returns for
+// unlockerID the current unlocker and returns its ID. It returns "" when
+// there is no such unlocker.
+func (v *Vault) selectOtherUnlocker(unlockerID string) (string, error) {
+	otherID, err := v.firstOtherUnlockerID(unlockerID)
+	if err != nil {
+		return "", err
+	}
+
+	if otherID == "" {
+		return "", nil
+	}
+
+	err = v.SelectUnlocker(otherID)
+	if err != nil {
+		return "", err
+	}
+
+	return otherID, nil
+}
+
+// firstOtherUnlockerID returns the ID of the vault's first unlocker other
+// than unlockerID, in ID order, the order `secret unlocker list` shows, that
+// SelectUnlocker accepts, or "" when there is none. SelectUnlocker refuses
+// an unlocker whose metadata names a type other than passphrase or pgp,
+// which ListUnlockers still returns.
+func (v *Vault) firstOtherUnlockerID(unlockerID string) (string, error) {
+	unlockers, err := v.ListUnlockers()
+	if err != nil {
+		return "", err
+	}
+
+	for _, id := range slices.Sorted(maps.Keys(unlockers)) {
+		unlockerType := unlockers[id].Type
+
+		if id != unlockerID && (unlockerType == unlockerTypePassphrase ||
+			unlockerType == unlockerTypePGP) {
+			return id, nil
+		}
+	}
+
+	return "", nil
 }
 
 // SelectUnlocker selects an unlocker as current for this vault

internal/vault/vault.go

@@ -291,9 +291,11 @@ 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.
+// 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) {
+	if errors.Is(err, secret.ErrPassphraseNotRead) ||
+		errors.Is(err, ErrUnlockerNotFound) {
 		return err
 	}
 
@@ -304,15 +306,21 @@ func (v *Vault) withMnemonicAdvice(err error) error {
 		return err
 	}
 
-	// 'secret unlocker add' acts on the current vault only.
-	steps := "'secret unlocker add passphrase'"
-
-	current, currentErr := GetCurrentVault(v.fs, v.stateDir)
-	if currentErr != nil || current.Name != v.Name {
-		steps = fmt.Sprintf("'secret vault select %s', then %s", v.Name, steps)
-	}
-
 	return fmt.Errorf("%w; the vault '%s' still opens with its mnemonic: run "+
 		"%s with %s set to the mnemonic to give it a new unlocker",
-		err, v.Name, steps, secret.EnvMnemonic)
+		err, v.Name, v.withVaultSelect("'secret unlocker add passphrase'"),
+		secret.EnvMnemonic)
+}
+
+// withVaultSelect returns command, a quoted 'secret unlocker' command, for
+// advice about this vault. Such a command acts on the current vault only,
+// so when this vault is not the current one, the command that selects it
+// comes first.
+func (v *Vault) withVaultSelect(command string) string {
+	current, err := GetCurrentVault(v.fs, v.stateDir)
+	if err != nil || current.Name != v.Name {
+		return fmt.Sprintf("'secret vault select %s', then %s", v.Name, command)
+	}
+
+	return command
 }

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"
@@ -103,3 +104,78 @@ func TestAddSecretCleansUpOnFailure(t *testing.T) {
 	require.Len(t, entries, 1)
 	assert.Equal(t, "pub.age", entries[0].Name())
 }
+
+// TestAddSecretForceWithWrongPassphraseChangesNothing checks that adding a
+// version to an existing secret, with a passphrase that does not open the
+// vault, fails and leaves the secret's directory as it was.
+func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+
+	passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+	t.Cleanup(passphrase.Destroy)
+
+	created, err := vault.CreateVault(fs, testStateDir, testVaultName,
+		testMnemonicBuffer(t), passphrase)
+	require.NoError(t, err)
+
+	first := memguard.NewBufferFromBytes([]byte("first"))
+	t.Cleanup(first.Destroy)
+
+	require.NoError(t, created.AddSecret(testSecretName, first, false))
+
+	wrong := memguard.NewBufferFromBytes([]byte("wrong passphrase"))
+	t.Cleanup(wrong.Destroy)
+
+	vlt := vault.NewVault(fs, testStateDir, testVaultName)
+	vlt.SetUnlockPassphrase(wrong)
+
+	secretDir := filepath.Join(testStateDir, "vaults.d", testVaultName,
+		"secrets.d", testSecretName)
+	before := dirContents(t, fs, secretDir)
+
+	second := memguard.NewBufferFromBytes([]byte("second"))
+	t.Cleanup(second.Destroy)
+
+	err = vlt.AddSecret(testSecretName, second, true)
+
+	var noMatch *age.NoIdentityMatchError
+
+	require.ErrorAs(t, err, &noMatch)
+	assert.Equal(t, before, dirContents(t, fs, secretDir))
+}
+
+// dirContents maps every file under dir to its contents, and every
+// directory, written with a trailing "/", to "".
+func dirContents(t *testing.T, fs afero.Fs, dir string) map[string]string {
+	t.Helper()
+
+	contents := map[string]string{}
+
+	err := afero.Walk(fs, dir, func(
+		path string, info os.FileInfo, err error,
+	) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			contents[path+"/"] = ""
+
+			return nil
+		}
+
+		content, err := afero.ReadFile(fs, path)
+		if err != nil {
+			return err
+		}
+
+		contents[path] = string(content)
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	return contents
+}