From 759dbb1b1d7ebcbe88b3dfbf8b1fb46af0cc5f4a Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 19:25:22 +0000 Subject: [PATCH] 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 | 6 ++ TODO.md | 9 +++ internal/cli/list_times_test.go | 118 ++++++++++++++++++++++++++++++++ internal/cli/secrets.go | 10 ++- internal/secret/secret.go | 53 ++++++++++---- internal/vault/secrets.go | 14 +--- 6 files changed, 183 insertions(+), 27 deletions(-) create mode 100644 internal/cli/list_times_test.go diff --git a/README.md b/README.md index 0bb262a..ca52508 100644 --- a/README.md +++ b/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 [--force]` / `secret rm` ⚠️ 🛑 **DANGER**: Permanently removes a secret and ALL its versions. It first asks for diff --git a/TODO.md b/TODO.md index cfc97da..a758a23 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,15 @@ 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: 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 diff --git a/internal/cli/list_times_test.go b/internal/cli/list_times_test.go new file mode 100644 index 0000000..4682b91 --- /dev/null +++ b/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} +} diff --git a/internal/cli/secrets.go b/internal/cli/secrets.go index 1728e2b..0c0ba27 100644 --- a/internal/cli/secrets.go +++ b/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") diff --git a/internal/secret/secret.go b/internal/secret/secret.go index ed9989f..4b73e36 100644 --- a/internal/secret/secret.go +++ b/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 } diff --git a/internal/vault/secrets.go b/internal/vault/secrets.go index 12f38b0..a3f2039 100644 --- a/internal/vault/secrets.go +++ b/internal/vault/secrets.go @@ -427,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 { @@ -454,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