Stop vault creation when a vault's metadata cannot be read (closes #140)

GetNextDerivationIndex passed over a vault whose vault-metadata.json could
not be read or parsed, so the next vault made from the same mnemonic could
get that vault's derivation index and so the same long-term key. It now
returns an error naming the file, and vault create, vault import and init
stop before writing anything. A vault directory with no metadata file is
still passed over.

The regression test corrupts one vault's metadata and requires CreateVault
with the same mnemonic to fail, naming the file, and leave no new vault.

Unverified by test: an unreadable metadata file; checked by hand only.

Model: opus-5-5

README.md

@@ -150,13 +150,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 +181,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` ⚠️ 🛑
 
@@ -533,9 +524,7 @@ secret decrypt encryption/mykey --input document.txt.age --output document.txt
 - **Derivation Index**: Each vault uses a unique derivation index from the
   mnemonic, and thus a unique key pair; creating a vault or importing a mnemonic
   fails, with an error naming the file, when any vault's `vault-metadata.json`
-  exists but cannot be read or parsed; a vault directory with no
-  `vault-metadata.json` is skipped, so the index it uses can be given to a new
-  vault
+  cannot be read or parsed, as that vault's index is then unknown
 - **Public Key Hash**: Double SHA-256 hash of the vault's public key; the same
   hash of the index-0 public key identifies vaults from the same mnemonic
 - **Automatic Key Derivation**: When creating vaults with a mnemonic, keys are

TODO.md

@@ -18,14 +18,13 @@ 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: Creating a vault, or importing a mnemonic into one, fails with an
+  error naming the file when any vault's `vault-metadata.json` exists but cannot
+  be read or parsed, and writes nothing
+  (https://git.eeqj.de/sneak/secret/issues/140). `vault.GetNextDerivationIndex`
+  passed over such a vault, so the next vault made from the same mnemonic could
+  be given its derivation index, and so its long-term key. A vault directory
+  with no `vault-metadata.json` is still passed over.
 - 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/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

@@ -541,12 +541,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 +618,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/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/metadata_test.go

@@ -1,7 +1,6 @@
 package vault_test
 
 import (
-	"errors"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -167,86 +166,31 @@ func testGetNextDerivationIndex(t *testing.T, fs afero.Fs) {
 	}
 }
 
-// errMetadataUnreadable is returned by the test filesystem in place of a
-// successful open of one vault's metadata file.
-var errMetadataUnreadable = errors.New("input/output error")
-
-// metadataReadFailFs fails every open of the file at unreadablePath. The
-// file still exists, so checking for it succeeds and only reading it fails.
-type metadataReadFailFs struct {
-	afero.Fs
-
-	unreadablePath string
-}
-
-//nolint:ireturn // afero.File is the interface required by afero.Fs
-func (f *metadataReadFailFs) Open(name string) (afero.File, error) {
-	if name == f.unreadablePath {
-		return nil, errMetadataUnreadable
-	}
-
-	return f.Fs.Open(name)
-}
-
 // TestCreateVaultWithCorruptMetadataFails checks that creating a vault from
 // a mnemonic fails, naming the file and writing nothing, when another
-// vault's metadata file exists but cannot be parsed or read. That vault's
-// derivation index is then unknown, so the new vault could get the same one
-// and the same key.
+// vault's metadata cannot be parsed. That vault's derivation index is then
+// unknown, so the new vault could get the same one and the same key.
 func TestCreateVaultWithCorruptMetadataFails(t *testing.T) {
 	t.Parallel()
 
+	fs := afero.NewMemMapFs()
+
+	_, err := vault.CreateVault(fs, testStateDir, "work", testMnemonicBuffer(t), nil)
+	require.NoError(t, err)
+
 	vaultsDir := filepath.Join(testStateDir, "vaults.d")
 	metadataPath := filepath.Join(vaultsDir, "work", "vault-metadata.json")
 
-	tests := []struct {
-		name string
-		// breakMetadata makes the work vault's metadata file unusable on
-		// base and returns the filesystem to create the second vault on.
-		breakMetadata func(t *testing.T, base afero.Fs) afero.Fs
-	}{
-		{
-			name: "parsing the file fails",
-			breakMetadata: func(t *testing.T, base afero.Fs) afero.Fs {
-				t.Helper()
+	err = afero.WriteFile(fs, metadataPath, []byte("{"), secret.FilePerms)
+	require.NoError(t, err)
 
-				err := afero.WriteFile(base, metadataPath, []byte("{"),
-					secret.FilePerms)
-				require.NoError(t, err)
+	_, err = vault.CreateVault(fs, testStateDir, "other", testMnemonicBuffer(t), nil)
+	require.ErrorContains(t, err, metadataPath)
 
-				return base
-			},
-		},
-		{
-			name: "reading the file fails",
-			breakMetadata: func(_ *testing.T, base afero.Fs) afero.Fs {
-				return &metadataReadFailFs{Fs: base, unreadablePath: metadataPath}
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			base := afero.NewMemMapFs()
-
-			_, err := vault.CreateVault(base, testStateDir, "work",
-				testMnemonicBuffer(t), nil)
-			require.NoError(t, err)
-
-			fs := tt.breakMetadata(t, base)
-
-			_, err = vault.CreateVault(fs, testStateDir, "other",
-				testMnemonicBuffer(t), nil)
-			require.ErrorContains(t, err, metadataPath)
-
-			entries, err := afero.ReadDir(base, vaultsDir)
-			require.NoError(t, err)
-			require.Len(t, entries, 1)
-			assert.Equal(t, "work", entries[0].Name())
-		})
-	}
+	entries, err := afero.ReadDir(fs, vaultsDir)
+	require.NoError(t, err)
+	require.Len(t, entries, 1)
+	assert.Equal(t, "work", entries[0].Name())
 }
 
 func testMetadataPersistence(t *testing.T, fs afero.Fs) {

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))
 }