Write encrypt and decrypt output only once they succeed (closes #143)

`secret encrypt` and `secret decrypt` emptied the file named by
`--output` before reading any input, so a failure left it empty, and a
file given as both `--input` and `--output` was emptied before it was
read. They now write into a temporary file of mode 0600 next to it, or
next to the file it is a symbolic link to, and rename that over it only
on success, with the new `secret.WriteFileAtomicFunc`. An output file
that cannot be written still fails. Ctrl-C, SIGTERM or a hangup removes
the temporary file. A device or pipe, such as `/dev/null`, is written to
directly.

Judgement calls: the device and pipe case; a command started with
`nohup` still ignores a hangup.

Model: opus-5-5

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 <secret-name> [--force]` / `secret rm` ⚠️ 🛑
 
 **DANGER**: Permanently removes a secret and ALL its versions. It first asks for
@@ -181,7 +188,9 @@ period.
 #### `secret version promote <secret-name> <version>`
 
 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 <secret-name> <version> [--force]` / `secret version rm` ⚠️ 🛑
 
@@ -283,6 +292,19 @@ generates a new Age key.
 
 Decrypts data using an Age key stored as a secret.
 
+Both write the result to a new file next to `--output` and rename it over
+`--output` only once they have succeeded, so a failed command leaves an existing
+`--output` as it was and creates none, and `--input` and `--output` can be the
+same file. When `--output` is a symbolic link, the file it points to is
+replaced, or created if it does not exist yet, and the link kept; the new file
+is then made next to that file. The directory the new file is made in must be
+writable. The new file has mode 0600 and belongs to the user running the
+command, whatever mode and owner the file it replaces had, so a hard link to the
+old file keeps the old content. Ctrl-C, SIGTERM or a hangup removes the new
+file; a command killed any other way, such as with SIGKILL, leaves it there,
+named `.tmp-` and a number. A device or a pipe, such as `/dev/null`, is written
+to directly.
+
 ## Storage Architecture
 
 ### Directory Structure

internal/cli/crypto.go

@@ -5,9 +5,12 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
+	"sync/atomic"
 
 	"filippo.io/age"
 	"github.com/awnumar/memguard"
+	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/secret/internal/secret"
 	"sneak.berlin/go/secret/internal/vault"
@@ -188,36 +191,24 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	// Set up output writer
+	return cli.writeOutput(outputFile, func(output io.Writer) error {
-	output := cli.cmd.OutOrStdout()
+		encryptor, err := age.Encrypt(output, recipient)
-
-	if outputFile != "" {
-		file, err := cli.fs.Create(outputFile)
 		if err != nil {
-			return fmt.Errorf("failed to create output file: %w", err)
+			return fmt.Errorf("failed to create age encryptor: %w", err)
 		}
-		defer func() { _ = file.Close() }()
 
-		output = file
+		_, err = io.Copy(encryptor, input)
-	}
+		if err != nil {
+			return fmt.Errorf("failed to encrypt data: %w", err)
+		}
 
-	// Encrypt the data
+		err = encryptor.Close()
-	encryptor, err := age.Encrypt(output, recipient)
+		if err != nil {
-	if err != nil {
+			return fmt.Errorf("failed to finalize encryption: %w", err)
-		return fmt.Errorf("failed to create age encryptor: %w", err)
+		}
-	}
 
-	_, err = io.Copy(encryptor, input)
+		return nil
-	if err != nil {
+	})
-		return fmt.Errorf("failed to encrypt data: %w", err)
-	}
-
-	err = encryptor.Close()
-	if err != nil {
-		return fmt.Errorf("failed to finalize encryption: %w", err)
-	}
-
-	return nil
 }
 
 // Decrypt decrypts data using an age secret key stored in a secret
@@ -278,31 +269,123 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	// Set up output writer
+	return cli.writeOutput(outputFile, func(output io.Writer) error {
-	output := cli.cmd.OutOrStdout()
+		decryptor, err := age.Decrypt(input, identity)
+		if err != nil {
+			return fmt.Errorf("failed to create age decryptor: %w", err)
+		}
 
-	if outputFile != "" {
+		_, err = io.Copy(output, decryptor)
-		file, err := cli.fs.Create(outputFile)
+		if err != nil {
+			return fmt.Errorf("failed to decrypt data: %w", err)
+		}
+
+		return nil
+	})
+}
+
+// temporaryOutput is the name of the temporary file writeOutput is filling,
+// until it is renamed into place. The signal handler in Entry removes it,
+// since for decrypt it holds part of the secret.
+//
+//nolint:gochecknoglobals // read by the signal handler, which has no Instance
+var temporaryOutput atomic.Pointer[string]
+
+// writeOutput calls write with stdout when outputFile is empty, and with
+// outputFile itself when it is a device or a pipe, such as /dev/null.
+// Otherwise write fills a new file of mode 0600 that replaces outputFile,
+// or the file it is a symbolic link to, only once write has succeeded, so a
+// failure leaves that file as it was, and it may be the input file too.
+// An existing outputFile that cannot be written fails before write is
+// called.
+func (cli *Instance) writeOutput(
+	outputFile string, write func(output io.Writer) error,
+) error {
+	if outputFile == "" {
+		return write(cli.cmd.OutOrStdout())
+	}
+
+	info, err := cli.fs.Stat(outputFile)
+	if err == nil && !info.Mode().IsRegular() {
+		file, err := cli.fs.OpenFile(outputFile, os.O_WRONLY, 0)
 		if err != nil {
 			return fmt.Errorf("failed to create output file: %w", err)
 		}
 		defer func() { _ = file.Close() }()
 
-		output = file
+		return write(file)
 	}
 
-	// Decrypt the data
+	// An existing file that cannot be written fails here, as writing into it
-	decryptor, err := age.Decrypt(input, identity)
+	// directly would. Opened without O_TRUNC, which would empty it.
+	file, err := cli.fs.OpenFile(outputFile, os.O_WRONLY, 0)
+	if err == nil {
+		err = file.Close()
+	}
+
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("failed to create output file: %w", err)
+	}
+
+	path, err := outputPath(outputFile)
 	if err != nil {
-		return fmt.Errorf("failed to create age decryptor: %w", err)
+		return fmt.Errorf("failed to create output file: %w", err)
+	}
+
+	err = secret.WriteFileAtomicFunc(cli.fs, path, func(file afero.File) error {
+		name := file.Name()
+		temporaryOutput.Store(&name)
+
+		return write(file)
+	})
+
+	temporaryOutput.Store(nil)
+
+	return err
+}
+
+// outputPath returns the path of the file that writing to outputFile
+// replaces, with no symbolic link in it, so that filepath.Dir of it is the
+// directory that file is in. When outputFile is a symbolic link, as
+// /dev/stdout is when stdout goes to a file, that is the path the link
+// points to, whether or not a file is there yet, so the link is kept, as
+// writing through it would keep it. A link that cannot be followed, such as
+// one that points to itself, is an error.
+func outputPath(outputFile string) (string, error) {
+	path, err := filepath.EvalSymlinks(outputFile)
+	if !errors.Is(err, os.ErrNotExist) {
+		return path, err
+	}
+
+	// No file is at outputFile, or at the end of its links, yet
+	target, err := os.Readlink(outputFile)
+	if errors.Is(err, os.ErrNotExist) {
+		// Nothing is at outputFile itself. Its directory part is resolved
+		// with filepath.EvalSymlinks, since filepath.Dir would remove a link
+		// to a directory followed by ".." as text and give another directory.
+		dir, name := filepath.Split(outputFile)
+
+		dir, err = filepath.EvalSymlinks(dir)
+		if err != nil {
+			return "", err
+		}
+
+		return filepath.Join(dir, name), nil
 	}
 
-	_, err = io.Copy(output, decryptor)
 	if err != nil {
-		return fmt.Errorf("failed to decrypt data: %w", err)
+		return "", err
 	}
 
-	return nil
+	if !filepath.IsAbs(target) {
+		// Not filepath.Join, which would remove a ".." in target together
+		// with the name before it, even when that name is a link to a
+		// directory
+		dir, _ := filepath.Split(outputFile)
+		target = dir + target
+	}
+
+	return outputPath(target)
 }
 
 // isValidAgeSecretKey checks if a string is a valid age secret key by

internal/cli/crypto_test.go

@@ -0,0 +1,304 @@
+//nolint:testpackage // sets the unexported fields of Instance
+package cli
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"syscall"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/vault"
+)
+
+// plainText is the content of plain.txt in the directory newCryptoTest makes.
+const plainText = "document"
+
+// newCryptoTest returns an Instance whose current vault, in a new state
+// directory on the real filesystem, holds the age key secret "key", and a
+// new directory holding only plain.txt and plain.txt.age, its encryption
+// with that key.
+func newCryptoTest(t *testing.T) (*Instance, string) {
+	t.Helper()
+
+	fs := afero.NewOsFs()
+	stateDir := t.TempDir()
+
+	_, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t), nil)
+	require.NoError(t, err)
+
+	cli := NewCLIInstanceWithStateDir(fs, stateDir)
+	cli.Mnemonic = testMnemonicBuffer(t)
+	cli.cmd = &cobra.Command{}
+
+	dir := t.TempDir()
+	plain := filepath.Join(dir, "plain.txt")
+	require.NoError(t, afero.WriteFile(fs, plain, []byte(plainText), 0o600))
+	require.NoError(t, cli.Encrypt("key", plain, plain+".age"))
+
+	return cli, dir
+}
+
+// TestFailedCryptoLeavesOutputUnchanged is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/143, where secret encrypt and
+// secret decrypt emptied the file named by --output before they failed.
+// A failed command must leave an existing file as it was and create none,
+// and give the same error as without --output.
+func TestFailedCryptoLeavesOutputUnchanged(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		run  func(cli *Instance, dir, output string) error
+	}{
+		// Reading a directory fails once encryption has begun
+		{"encrypt", func(cli *Instance, dir, output string) error {
+			return cli.Encrypt("key", dir, output)
+		}},
+		// plain.txt is not an age file
+		{"decrypt", func(cli *Instance, dir, output string) error {
+			return cli.Decrypt("key", filepath.Join(dir, "plain.txt"), output)
+		}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			cli, dir := newCryptoTest(t)
+			existing := filepath.Join(dir, "existing.txt")
+			require.NoError(t, afero.WriteFile(cli.fs, existing, []byte("precious"), 0o600))
+
+			cli.cmd.SetOut(io.Discard)
+			toStdout := tt.run(cli, dir, "")
+			require.Error(t, toStdout)
+
+			require.EqualError(t, tt.run(cli, dir, existing), toStdout.Error())
+			require.EqualError(t, tt.run(cli, dir, filepath.Join(dir, "new.txt")),
+				toStdout.Error())
+
+			content, err := afero.ReadFile(cli.fs, existing)
+			require.NoError(t, err)
+			require.Equal(t, "precious", string(content))
+
+			// No new.txt, and no temporary file left behind
+			entries, err := afero.ReadDir(cli.fs, dir)
+			require.NoError(t, err)
+
+			names := make([]string, 0, len(entries))
+			for _, entry := range entries {
+				names = append(names, entry.Name())
+			}
+
+			require.Equal(t, []string{"existing.txt", "plain.txt", "plain.txt.age"}, names)
+		})
+	}
+}
+
+// writeFailFs fails every open of one path for writing, as a file of mode
+// 0444 does for anyone but root, who runs these tests in Docker.
+type writeFailFs struct {
+	afero.Fs
+
+	path string
+}
+
+//nolint:ireturn // afero.File is the interface required by afero.Fs
+func (f *writeFailFs) OpenFile(
+	name string, flag int, perm os.FileMode,
+) (afero.File, error) {
+	if name == f.path && flag&(os.O_WRONLY|os.O_RDWR) != 0 {
+		return nil, &os.PathError{Op: "open", Path: name, Err: syscall.EACCES}
+	}
+
+	return f.Fs.OpenFile(name, flag, perm)
+}
+
+// TestDecryptToUnwritableOutput checks that secret decrypt fails when
+// --output is a file that cannot be written, as writing into it directly
+// does, and leaves it as it was, although its directory can be written.
+func TestDecryptToUnwritableOutput(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	existing := filepath.Join(dir, "existing.txt")
+	require.NoError(t, afero.WriteFile(cli.fs, existing, []byte("precious"), 0o600))
+	cli.fs = &writeFailFs{Fs: cli.fs, path: existing}
+
+	err := cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), existing)
+	require.ErrorIs(t, err, os.ErrPermission)
+
+	content, err := afero.ReadFile(cli.fs, existing)
+	require.NoError(t, err)
+	require.Equal(t, "precious", string(content))
+}
+
+// TestCryptoInPlace checks that secret encrypt and secret decrypt given
+// one file as both --input and --output replace it with the right result.
+// Before https://git.eeqj.de/sneak/secret/issues/143 they emptied the file
+// before reading it.
+func TestCryptoInPlace(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	file := filepath.Join(dir, "plain.txt")
+
+	require.NoError(t, cli.Encrypt("key", file, file))
+	require.NoError(t, cli.Decrypt("key", file, file))
+
+	content, err := afero.ReadFile(cli.fs, file)
+	require.NoError(t, err)
+	require.Equal(t, plainText, string(content))
+}
+
+// TestDecryptToPipe checks that secret decrypt writes into a named pipe
+// given as --output, as it must into a device such as /dev/null, instead
+// of replacing it with a file.
+func TestDecryptToPipe(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	pipe := filepath.Join(dir, "pipe")
+	require.NoError(t, syscall.Mkfifo(pipe, 0o600))
+
+	received := make(chan []byte, 1)
+
+	go func() {
+		content, _ := afero.ReadFile(cli.fs, pipe)
+		received <- content
+	}()
+
+	require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), pipe))
+
+	info, err := os.Lstat(pipe)
+	require.NoError(t, err)
+	require.Equal(t, os.ModeNamedPipe, info.Mode().Type())
+	require.Equal(t, plainText, string(<-received))
+}
+
+// TestDecryptThroughLink checks that secret decrypt given a symbolic link
+// as --output replaces the file the link points to, in another directory,
+// and keeps the link, as writing through the link would.
+func TestDecryptThroughLink(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	targetDir := t.TempDir()
+	target := filepath.Join(targetDir, "target.txt")
+	require.NoError(t, afero.WriteFile(cli.fs, target, []byte("old"), 0o600))
+
+	link := filepath.Join(dir, "link")
+	require.NoError(t, os.Symlink(target, link))
+
+	require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), link))
+
+	info, err := os.Lstat(link)
+	require.NoError(t, err)
+	require.Equal(t, os.ModeSymlink, info.Mode().Type())
+
+	content, err := afero.ReadFile(cli.fs, target)
+	require.NoError(t, err)
+	require.Equal(t, plainText, string(content))
+
+	// No temporary file left beside the target
+	entries, err := afero.ReadDir(cli.fs, targetDir)
+	require.NoError(t, err)
+	require.Len(t, entries, 1)
+}
+
+// TestDecryptThroughLinkToMissingFile checks that secret decrypt given a
+// symbolic link to a file that does not exist yet as --output creates that
+// file and keeps the link, as writing through the link would. Here --output
+// is a relative link to a second link, which points into another directory.
+func TestDecryptThroughLinkToMissingFile(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	targetDir := t.TempDir()
+	target := filepath.Join(targetDir, "target.txt")
+
+	link := filepath.Join(dir, "link")
+	require.NoError(t, os.Symlink("second-link", link))
+	require.NoError(t, os.Symlink(target, filepath.Join(dir, "second-link")))
+
+	require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), link))
+
+	for _, name := range []string{"link", "second-link"} {
+		info, err := os.Lstat(filepath.Join(dir, name))
+		require.NoError(t, err)
+		require.Equal(t, os.ModeSymlink, info.Mode().Type(), name)
+	}
+
+	content, err := afero.ReadFile(cli.fs, target)
+	require.NoError(t, err)
+	require.Equal(t, plainText, string(content))
+
+	// No temporary file left beside the target
+	entries, err := afero.ReadDir(cli.fs, targetDir)
+	require.NoError(t, err)
+	require.Len(t, entries, 1)
+}
+
+// TestDecryptThroughLinkAndParent checks that secret decrypt makes the file
+// --output names in the directory its path leads to when the path holds a
+// symbolic link to a directory followed by "..": dir/link/../sub/new.txt is
+// targetDir/sub/new.txt, and dir/sub, where removing "link/.." as text
+// leads, does not exist.
+func TestDecryptThroughLinkAndParent(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	targetDir := t.TempDir()
+	require.NoError(t, os.Mkdir(filepath.Join(targetDir, "linked"), 0o700))
+	require.NoError(t, os.Mkdir(filepath.Join(targetDir, "sub"), 0o700))
+	require.NoError(t, os.Symlink(filepath.Join(targetDir, "linked"),
+		filepath.Join(dir, "link")))
+
+	// Not filepath.Join, which would remove "link/.." too
+	output := dir + "/link/../sub/new.txt"
+
+	require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), output))
+
+	content, err := afero.ReadFile(cli.fs, filepath.Join(targetDir, "sub", "new.txt"))
+	require.NoError(t, err)
+	require.Equal(t, plainText, string(content))
+}
+
+// TestDecryptThroughLoopingLink checks that secret decrypt given a symbolic
+// link that points to itself as --output fails, as writing through it
+// would, and leaves the link as it was.
+func TestDecryptThroughLoopingLink(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	link := filepath.Join(dir, "link")
+	require.NoError(t, os.Symlink("link", link))
+
+	require.Error(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), link))
+
+	target, err := os.Readlink(link)
+	require.NoError(t, err)
+	require.Equal(t, "link", target)
+}
+
+// TestDecryptToLongestName checks that secret decrypt writes an --output
+// whose name is as long as a file name can be, 255 bytes on Linux and
+// macOS: the temporary file's name must not be longer.
+func TestDecryptToLongestName(t *testing.T) {
+	t.Parallel()
+
+	const longestName = 255
+
+	cli, dir := newCryptoTest(t)
+	output := filepath.Join(dir, strings.Repeat("a", longestName))
+
+	require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), output))
+
+	content, err := afero.ReadFile(cli.fs, output)
+	require.NoError(t, err)
+	require.Equal(t, plainText, string(content))
+}

internal/cli/entry_test.go

@@ -7,7 +7,9 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"testing"
+	"time"
 
 	"github.com/awnumar/memguard"
 	"github.com/stretchr/testify/assert"
@@ -105,3 +107,115 @@ func TestInterruptExitsThroughMemguard(t *testing.T) {
 	require.ErrorAs(t, err, &exitErr)
 	assert.Equal(t, 1, exitErr.ExitCode(), "add ended with %v", err)
 }
+
+// Ctrl-C or a hangup while encrypt or decrypt waits for input must remove
+// the temporary file it is writing next to --output, which for decrypt
+// holds part of the secret.
+func TestInterruptRemovesTemporaryOutput(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	secretPath := secretBinaryPath(t)
+	env := []string{
+		secret.EnvStateDir + "=" + t.TempDir(),
+		secret.EnvMnemonic + "=" + testMnemonic,
+		secret.EnvUnlockPassphrase + "=test-passphrase",
+		"PATH=" + os.Getenv("PATH"),
+	}
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	initCmd := exec.CommandContext(ctx, secretPath, "init")
+	initCmd.Env = env
+
+	output, err := initCmd.CombinedOutput()
+	require.NoError(t, err, "init should succeed: %s", output)
+
+	// encrypt stores the new key before it starts writing, so decrypt has it
+	for _, command := range []string{"encrypt", "decrypt"} {
+		for _, sig := range []os.Signal{os.Interrupt, syscall.SIGHUP} {
+			outputDir := t.TempDir()
+
+			//nolint:gosec // G204: test executes the freshly built secret binary
+			cmd := exec.CommandContext(ctx, secretPath, command, "key",
+				"--output", filepath.Join(outputDir, "out"))
+			cmd.Env = env
+
+			// Held open and never written, so the command keeps waiting for
+			// input. Wait closes it.
+			_, err := cmd.StdinPipe()
+			require.NoError(t, err)
+			require.NoError(t, cmd.Start())
+
+			require.Eventually(t, func() bool {
+				entries, err := os.ReadDir(outputDir)
+
+				return err == nil && len(entries) > 0
+			}, commandWait, 10*time.Millisecond, "%s made no temporary file", command)
+			require.NoError(t, cmd.Process.Signal(sig))
+
+			var exitErr *exec.ExitError
+
+			require.ErrorAs(t, cmd.Wait(), &exitErr)
+
+			entries, err := os.ReadDir(outputDir)
+			require.NoError(t, err)
+			assert.Empty(t, entries, "%s left its temporary file on %v", command, sig)
+		}
+	}
+}
+
+// encrypt started with nohup must go on ignoring a hangup, which it catches
+// otherwise, and finish writing --output.
+func TestNohupIgnoresHangup(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), commandWait)
+	defer cancel()
+
+	secretPath := secretBinaryPath(t)
+	env := []string{
+		secret.EnvStateDir + "=" + t.TempDir(),
+		secret.EnvMnemonic + "=" + testMnemonic,
+		secret.EnvUnlockPassphrase + "=test-passphrase",
+		"PATH=" + os.Getenv("PATH"),
+	}
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	initCmd := exec.CommandContext(ctx, secretPath, "init")
+	initCmd.Env = env
+
+	output, err := initCmd.CombinedOutput()
+	require.NoError(t, err, "init should succeed: %s", output)
+
+	outputDir := t.TempDir()
+	outputFile := filepath.Join(outputDir, "out")
+
+	//nolint:gosec // G204: test executes the freshly built secret binary
+	cmd := exec.CommandContext(ctx, "nohup", secretPath, "encrypt", "key",
+		"--output", outputFile)
+	cmd.Env = env
+
+	stdin, err := cmd.StdinPipe()
+	require.NoError(t, err)
+	require.NoError(t, cmd.Start())
+
+	require.Eventually(t, func() bool {
+		entries, err := os.ReadDir(outputDir)
+
+		return err == nil && len(entries) > 0
+	}, commandWait, 10*time.Millisecond, "encrypt made no temporary file")
+	require.NoError(t, cmd.Process.Signal(syscall.SIGHUP))
+
+	// Time for a hangup that is caught to end encrypt before it has its input
+	time.Sleep(500 * time.Millisecond)
+
+	_, err = stdin.Write([]byte("document"))
+	require.NoError(t, err)
+	require.NoError(t, stdin.Close())
+	require.NoError(t, cmd.Wait())
+
+	_, err = os.Stat(outputFile)
+	require.NoError(t, err)
+}

internal/cli/leftovers_test.go

@@ -52,10 +52,10 @@ func TestLeftoversRemovedByNextChangingCommand(t *testing.T) {
 	}
 
 	for _, file := range []string{
-		testStateDir + "/.currentvault.tmp-4",
+		testStateDir + "/.tmp-4",
-		vaultDir + "/.current-unlocker.tmp-5",
+		vaultDir + "/.tmp-5",
-		secretDir + "/.current.tmp-6",
+		secretDir + "/.tmp-6",
-		secretDir + "/versions/" + versions[0] + "/.metadata.age.tmp-7",
+		secretDir + "/versions/" + versions[0] + "/.tmp-7",
 	} {
 		require.NoError(t, afero.WriteFile(fs, file,
 			[]byte("partial"), secret.FilePerms))

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

internal/cli/root.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"os"
+	"os/signal"
 
 	"github.com/awnumar/memguard"
 	"github.com/spf13/cobra"
@@ -14,12 +15,13 @@ import (
 // every memguard buffer before it returns, so the caller must do nothing
 // but exit with the code.
 func Entry() int {
-	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
+	// On SIGINT, SIGTERM or SIGHUP memguard runs this function, wipes every
-	// and exits with status 1. The passphrase prompt turns echo off on the
+	// buffer and exits with status 1. The function removes the temporary
-	// controlling terminal until the read finishes, so a signal there would
+	// file that encrypt or decrypt is writing. The passphrase prompt turns
-	// leave echo off. Only a process in the terminal's foreground process
+	// echo off on the controlling terminal until the read finishes, so a
-	// group may reset it: one in the background that tries is stopped
+	// signal there would leave echo off. Only a process in the terminal's
-	// instead of exiting.
+	// foreground process group may reset it: one in the background that
+	// tries is stopped instead of exiting.
 	terminal, terminalErr := os.Open(secret.TerminalPath)
 
 	var terminalState *term.State
@@ -30,7 +32,20 @@ func Entry() int {
 		terminalState, terminalErr = term.GetState(int(terminal.Fd()))
 	}
 
+	signals := []os.Signal{os.Interrupt, unix.SIGTERM}
+
+	// A command started with nohup must go on ignoring SIGHUP, which
+	// catching it would undo
+	if !signal.Ignored(unix.SIGHUP) {
+		signals = append(signals, unix.SIGHUP)
+	}
+
 	memguard.CatchSignal(func(os.Signal) {
+		name := temporaryOutput.Load()
+		if name != nil {
+			_ = os.Remove(*name)
+		}
+
 		if terminalErr != nil {
 			return
 		}
@@ -39,7 +54,7 @@ func Entry() int {
 		if err == nil && foreground == unix.Getpgrp() {
 			_ = term.Restore(int(terminal.Fd()), terminalState)
 		}
-	}, os.Interrupt, unix.SIGTERM)
+	}, signals...)
 
 	defer memguard.Purge()
 

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

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"maps"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -66,12 +67,13 @@ func getDefaultGPGKey() (string, error) {
 
 	output, err := cmd.Output()
 	if err == nil {
-		for line := range strings.SplitSeq(string(output), "\n") {
+		defaultKey, parseErr := parseDefaultGPGKey(string(output))
-			fields := strings.Split(line, ":")
+		if parseErr != nil {
-			if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" {
+			return "", parseErr
-				// The default key is in field 10 (index 9)
+		}
-				return fields[9], nil
+
-			}
+		if defaultKey != "" {
+			return defaultKey, nil
 		}
 	}
 
@@ -98,6 +100,27 @@ 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,6 +2,8 @@
 package cli
 
 import (
+	"bytes"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -83,6 +85,77 @@ 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,7 +65,9 @@ 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 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,

internal/secret/atomic.go

@@ -10,8 +10,8 @@ import (
 	"github.com/spf13/afero"
 )
 
-// tempNamePart is in the name of every temporary file WriteFileAtomic makes,
+// tempNamePart starts the name of every temporary file WriteFileAtomicFunc
-// ".NAME.tmp-123", and every temporary directory TempDirFor makes, ".tmp-123".
+// makes and every temporary directory TempDirFor makes, ".tmp-123".
 const tempNamePart = ".tmp-"
 
 // WriteFileAtomic replaces the file at path with data so that a reader, or
@@ -21,17 +21,38 @@ const tempNamePart = ".tmp-"
 // within one filesystem), is synced to disk, and is renamed over path. The
 // temporary file is removed if any step fails.
 func WriteFileAtomic(fs afero.Fs, path string, data []byte) error {
-	tmp, err := afero.TempFile(fs, filepath.Dir(path),
+	return WriteFileAtomicFunc(fs, path, func(file afero.File) error {
-		"."+filepath.Base(path)+tempNamePart+"*")
+		_, err := file.Write(data)
+		if err != nil {
+			return fmt.Errorf("failed to write %s: %w", path, err)
+		}
+
+		return nil
+	})
+}
+
+// WriteFileAtomicFunc is WriteFileAtomic for content that write streams
+// into the temporary file, which it is given, instead of holding it in
+// memory. An error from write is returned as it is. The temporary file's
+// name leaves out path's, which may already be as long as a name can be.
+func WriteFileAtomicFunc(
+	fs afero.Fs, path string, write func(file afero.File) error,
+) error {
+	tmp, err := afero.TempFile(fs, filepath.Dir(path), tempNamePart)
 	if err != nil {
 		return fmt.Errorf("failed to create temporary file for %s: %w", path, err)
 	}
 
-	_, err = tmp.Write(data)
+	err = write(tmp)
-	if err == nil {
+	if err != nil {
-		err = tmp.Sync()
+		_ = tmp.Close()
+		_ = fs.Remove(tmp.Name())
+
+		return err
 	}
 
+	err = tmp.Sync()
+
 	closeErr := tmp.Close()
 	if err == nil {
 		err = closeErr

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",
+	fs := s.vault.GetFilesystem()
-		"secret_name", s.Name)
+
-	// For backward compatibility, we'll populate with basic info
+	current, err := fs.Stat(filepath.Join(s.Directory, "current"))
-	now := time.Now()
+	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,
+		CreatedAt: value.ModTime(),
-		UpdatedAt: now,
+		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/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
+	return secret.NewSecret(v, name), nil
-	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
@@ -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,
+	err = v.copyVersions(srcVault, srcIdentity, srcSecretName, srcSecretDir,
-		srcSecretName, destSecretName, destSecretDir, versions, currentVersion)
+		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.