Hold the encrypt and decrypt result in memory until it is written (closes #143)

With --output, secret encrypt and secret decrypt emptied that file before
reading any input, so a failed command left it empty, and encrypting a file
onto itself gave the encryption of nothing.

With --output they now hold the result in memory and open --output, with the
same call as before, only once encryption or decryption has succeeded. A
failure to encrypt or decrypt leaves --output untouched, and --input can be the
same file. A failure while writing --output, such as a full disk, can leave it
empty or partly written; an error from closing it is reported. Without
--output the result still goes to stdout as it is produced.

Model: opus-5-5

README.md

@@ -67,6 +67,10 @@ and unlocker set. This allows for complete isolation between different contexts
 
 ## Commands Reference
 
+Every time `secret` prints, in a table or in `--json` output, is in UTC,
+whatever `TZ` or the system's time zone is set to. The date in a version's name
+is the UTC date too.
+
 ### Confirmation Before Removal
 
 `secret rm`, `secret version rm`, `secret vault remove` and
@@ -314,6 +318,12 @@ name. The file's bytes are stored unchanged, a final newline included.
 
 ### Encryption Operations
 
+With `--output`, `secret encrypt` and `secret decrypt` hold the result in memory
+and write it to `--output` only once encryption or decryption has succeeded, so
+a failure to encrypt or decrypt leaves `--output` untouched. A failure while
+writing `--output`, such as a full disk, can leave it empty or partly written.
+The command can need up to about four times as much memory as the result.
+
 #### `secret encrypt <secret-name> [--input=file] [--output=file]`
 
 Encrypts data using an Age key stored as a secret. If the secret doesn't exist,

internal/cli/crypto.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"io"
@@ -107,8 +108,8 @@ func (cli *Instance) storeNewEncryptionKey(
 // does not exist. The caller must destroy the returned buffer. Only storing
 // a new key takes the state directory lock, so that reading an existing key
 // works on a read-only state directory and keeps no other command waiting
-// at the passphrase prompt, and Encrypt streams its input and output
-// unlocked.
+// at the passphrase prompt, and Encrypt reads its input and writes its
+// output without the lock.
 func (cli *Instance) resolveEncryptionKey(
 	vlt *vault.Vault, secretName string,
 ) (*memguard.LockedBuffer, error) {
@@ -188,17 +189,17 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	// Set up output writer
+	// Set up output writer. With --output the result is held in memory
+	// and written only once encryption has succeeded, so a failure to
+	// encrypt leaves --output untouched and --input can be the same file.
+	// A failure while writing --output, such as a full disk, can leave it
+	// empty or partly written.
 	output := cli.cmd.OutOrStdout()
 
-	if outputFile != "" {
-		file, err := cli.fs.Create(outputFile)
-		if err != nil {
-			return fmt.Errorf("failed to create output file: %w", err)
-		}
-		defer func() { _ = file.Close() }()
+	var result bytes.Buffer
 
-		output = file
+	if outputFile != "" {
+		output = &result
 	}
 
 	// Encrypt the data
@@ -217,7 +218,11 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
 		return fmt.Errorf("failed to finalize encryption: %w", err)
 	}
 
-	return nil
+	if outputFile == "" {
+		return nil
+	}
+
+	return cli.writeOutputFile(outputFile, result.Bytes())
 }
 
 // Decrypt decrypts data using an age secret key stored in a secret
@@ -278,17 +283,17 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	// Set up output writer
+	// Set up output writer. With --output the result is held in memory
+	// and written only once decryption has succeeded, so a failure to
+	// decrypt leaves --output untouched and --input can be the same file.
+	// A failure while writing --output, such as a full disk, can leave it
+	// empty or partly written.
 	output := cli.cmd.OutOrStdout()
 
-	if outputFile != "" {
-		file, err := cli.fs.Create(outputFile)
-		if err != nil {
-			return fmt.Errorf("failed to create output file: %w", err)
-		}
-		defer func() { _ = file.Close() }()
+	var result bytes.Buffer
 
-		output = file
+	if outputFile != "" {
+		output = &result
 	}
 
 	// Decrypt the data
@@ -302,6 +307,32 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
 		return fmt.Errorf("failed to decrypt data: %w", err)
 	}
 
+	if outputFile == "" {
+		return nil
+	}
+
+	return cli.writeOutputFile(outputFile, result.Bytes())
+}
+
+// writeOutputFile creates outputFile, or empties it if it exists, and
+// writes result into it.
+func (cli *Instance) writeOutputFile(outputFile string, result []byte) error {
+	file, err := cli.fs.Create(outputFile)
+	if err != nil {
+		return fmt.Errorf("failed to create output file: %w", err)
+	}
+
+	_, err = file.Write(result)
+
+	closeErr := file.Close()
+	if err == nil {
+		err = closeErr
+	}
+
+	if err != nil {
+		return fmt.Errorf("failed to write output file: %w", err)
+	}
+
 	return nil
 }
 

internal/cli/crypto_test.go

@@ -0,0 +1,185 @@
+//nolint:testpackage // sets the unexported fields of Instance
+package cli
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"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 failure to encrypt or decrypt 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))
+
+			exists, err := afero.Exists(cli.fs, filepath.Join(dir, "new.txt"))
+			require.NoError(t, err)
+			require.False(t, exists, "a failed %s created --output", tt.name)
+		})
+	}
+}
+
+// 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))
+}
+
+// errCloseFailed is returned by the files closeFailFs creates in place of
+// a successful Close, as NFS reports a full disk only when the file is
+// closed.
+var errCloseFailed = errors.New("no space left on device")
+
+// closeFailFs creates files whose Close fails with errCloseFailed.
+type closeFailFs struct {
+	afero.Fs
+}
+
+// closeFailFile is a file created through closeFailFs.
+type closeFailFile struct {
+	afero.File
+}
+
+func (f closeFailFile) Close() error {
+	_ = f.File.Close()
+
+	return errCloseFailed
+}
+
+//nolint:ireturn // afero.File is the interface required by afero.Fs
+func (f closeFailFs) Create(name string) (afero.File, error) {
+	file, err := f.Fs.Create(name)
+	if err != nil {
+		return nil, err
+	}
+
+	return closeFailFile{File: file}, nil
+}
+
+// TestDecryptFailsWhenClosingOutputFails checks that secret decrypt fails
+// when closing --output fails, since some filesystems report a failed
+// write only then.
+func TestDecryptFailsWhenClosingOutputFails(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+	cli.fs = closeFailFs{Fs: cli.fs}
+
+	err := cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"),
+		filepath.Join(dir, "out.txt"))
+	require.ErrorIs(t, err, errCloseFailed)
+}
+
+// TestDecryptToDescriptorOfDeletedFile checks that secret decrypt given
+// /dev/fd/N as --output writes through that descriptor. A program may give
+// the command a temporary file it has already deleted as stdout, pass
+// /dev/stdout as --output and read the result back from that file.
+func TestDecryptToDescriptorOfDeletedFile(t *testing.T) {
+	t.Parallel()
+
+	cli, dir := newCryptoTest(t)
+
+	file, err := os.CreateTemp(t.TempDir(), "out")
+	require.NoError(t, err)
+
+	defer func() { _ = file.Close() }()
+
+	require.NoError(t, os.Remove(file.Name()))
+
+	output := fmt.Sprintf("/dev/fd/%d", file.Fd())
+	require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), output))
+
+	// On macOS the command's open of /dev/fd/N shares this descriptor's
+	// position, which its write leaves at the end.
+	_, err = file.Seek(0, io.SeekStart)
+	require.NoError(t, err)
+
+	content, err := io.ReadAll(file)
+	require.NoError(t, err)
+	require.Equal(t, plainText, string(content))
+}

internal/cli/list_times_test.go

@@ -3,6 +3,9 @@ package cli_test
 import (
 	"bytes"
 	"encoding/json"
+	"os"
+	"os/exec"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -125,6 +128,57 @@ func TestMovedSecretKeepsItsTimes(t *testing.T) {
 		"updated %s, then %s", beforeMove.UpdatedAt, afterMove.UpdatedAt)
 }
 
+// TestListedTimesAreUTC is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/169, where times were printed
+// in the local time zone. It runs `secret list` on the built binary with
+// TZ set to New York, where the secret's last update, 03:04:05 UTC on 2
+// January, is 22:04:05 the day before.
+func TestListedTimesAreUTC(t *testing.T) {
+	t.Parallel()
+
+	stateDir := t.TempDir()
+
+	vlt, err := vault.CreateVault(
+		afero.NewOsFs(), stateDir, "default", testMnemonicBuffer(t), nil)
+	require.NoError(t, err)
+
+	vlt.Mnemonic = testMnemonicBuffer(t)
+	err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("one")), false)
+	require.NoError(t, err)
+
+	updated := time.Date(2026, time.January, 2, 3, 4, 5, 0, time.UTC)
+	current := filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "x",
+		"current")
+	require.NoError(t, os.Chtimes(current, updated, updated))
+
+	tests := []struct {
+		flags []string
+		want  string
+	}{
+		{flags: nil, want: "2026-01-02 03:04"},
+		{
+			flags: []string{"--json"},
+			want:  `"updated_at": "2026-01-02T03:04:05Z"`,
+		},
+	}
+
+	for _, tt := range tests {
+		args := append([]string{"list"}, tt.flags...)
+
+		//nolint:gosec // G204: test executes the freshly built secret binary
+		cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), args...)
+		cmd.Env = []string{
+			secret.EnvStateDir + "=" + stateDir,
+			"TZ=America/New_York",
+			"PATH=" + os.Getenv("PATH"),
+		}
+
+		output, err := cmd.CombinedOutput()
+		require.NoError(t, err, "%v: %s", args, output)
+		assert.Contains(t, string(output), tt.want, "%v", args)
+	}
+}
+
 // 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 {

internal/cli/root.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"os"
+	"time"
 
 	"github.com/awnumar/memguard"
 	"github.com/spf13/cobra"
@@ -14,6 +15,10 @@ import (
 // every memguard buffer before it returns, so the caller must do nothing
 // but exit with the code.
 func Entry() int {
+	// Every time secret prints or writes is in UTC, whatever TZ or the
+	// system's time zone is set to.
+	time.Local = time.UTC //nolint:gosmopolitan // this makes local time UTC
+
 	raiseLockedMemoryLimit()
 
 	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer