diff --git a/README.md b/README.md index 4651c36..2cb9e2f 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,13 @@ 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 [--force]` / `secret rm` ⚠️ 🛑 **DANGER**: Permanently removes a secret and ALL its versions. It first asks for @@ -181,7 +188,9 @@ period. #### `secret version promote ` Promotes a specific version to current by rewriting the secret's `current` file -to name it. Does not modify any timestamps, allowing for rollback scenarios. +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. #### `secret version remove [--force]` / `secret version rm` ⚠️ 🛑 diff --git a/TODO.md b/TODO.md index 857a05f..dec185f 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,17 @@ 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: `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). diff --git a/internal/cli/list_times_test.go b/internal/cli/list_times_test.go new file mode 100644 index 0000000..7bd87c5 --- /dev/null +++ b/internal/cli/list_times_test.go @@ -0,0 +1,161 @@ +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} +} 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/cli/version.go b/internal/cli/version.go index 1304143..ca862a4 100644 --- a/internal/cli/version.go +++ b/internal/cli/version.go @@ -65,7 +65,9 @@ func VersionCommands(cli *Instance) *cobra.Command { Use: "promote ", Short: "Promote a specific version to current", Long: "Updates the current symlink to point to the specified " + - "version without modifying timestamps", + "version. The versions' own times are left unchanged; secret " + + "list then gives the time of the promote as the secret's last " + + "update.", Args: cobra.ExactArgs(2), //nolint:mnd // secret-name and version args ValidArgsFunction: func( cmd *cobra.Command, args []string, toComplete string, 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..532f058 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 @@ -581,8 +573,8 @@ func (v *Vault) CopySecretAllVersions( } // Copy each version and the current pointer, then move the copy into place - err = v.copyVersions(srcVault, srcIdentity, - srcSecretName, destSecretName, destSecretDir, versions, currentVersion) + err = v.copyVersions(srcVault, srcIdentity, srcSecretName, srcSecretDir, + destSecretName, destSecretDir, versions, currentVersion) if err != nil { return err } @@ -801,10 +793,13 @@ 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. +// 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. func (v *Vault) copyVersions( srcVault *Vault, srcIdentity *age.X25519Identity, - srcSecretName, destSecretName, destSecretDir string, + srcSecretName, srcSecretDir, destSecretName, destSecretDir string, versions []string, currentVersion string, ) error { buildDir, err := secret.TempDirFor(v.fs, destSecretDir) @@ -823,6 +818,14 @@ 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) @@ -830,6 +833,12 @@ 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) @@ -854,6 +863,26 @@ 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.