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
failed command leaves --output untouched, and --input can be the same file.
Without --output the result still goes to stdout as it is produced.

Model: opus-5-5

README.md

@@ -90,7 +90,8 @@ remove things pass `--force`.
 #### `secret init`
 
 Initializes the secret manager with a default vault. Prompts for a BIP39
-mnemonic phrase and creates the initial directory structure.
+mnemonic phrase and creates the initial directory structure. When the vault
+`default` already exists, it fails before asking for the mnemonic or passphrase.
 
 **Environment Variables:**
 
@@ -105,7 +106,8 @@ Lists all available vaults. The current vault is marked.
 
 #### `secret vault create <name>`
 
-Creates a new vault with the specified name.
+Creates a new vault with the specified name. When a vault of that name already
+exists, it fails before asking for the mnemonic or passphrase.
 
 **Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
 are allowed, and a name must not be empty, `.` or `..`.
@@ -142,7 +144,13 @@ contain `//`, or have `..` as a path segment.
 
 #### `secret get <secret-name> [--version <version>]`
 
-Retrieves and outputs a secret value to stdout.
+Retrieves and outputs a secret value to stdout. A failed write of the value,
+such as to a file on a full disk, prints the error and exits 1, with two
+exceptions. If the program reading from a pipe has already exited, as in
+`secret get s | true`, the broken-pipe signal (SIGPIPE) ends `secret` with no
+message and the shell reports exit status 141. If the shell closed stdout before
+`secret` started (`secret get s >&-`), the value is written to `/dev/null` and
+`secret get` exits 0.
 
 - `--version, -v`: Get a specific version (default: current)
 
@@ -292,18 +300,8 @@ 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.
+With `--output`, both hold the result in memory and write it to `--output` only
+once they have succeeded, so a failed command leaves `--output` untouched.
 
 ## Storage Architecture
 

internal/cli/create_vault_test.go

@@ -222,12 +222,51 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
 	output, err := cmd.CombinedOutput()
 	require.Error(t, err)
 
-	require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+
-		"Error: failed to read mnemonic: there is no terminal to ask on. "+
-		"Please set the SB_SECRET_MNEMONIC environment variable or run "+
+	require.Equal(t, "Error: failed to read mnemonic: there is no terminal to "+
+		"ask on. Please set the SB_SECRET_MNEMONIC environment variable or run "+
 		"interactively\n", string(output))
 }
 
+// TestCreateExistingVaultAsksForNothing is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/151: `secret init` and `secret
+// vault create` asked for the mnemonic and the unlocker passphrase before
+// refusing a vault that already exists, and `secret init` printed
+// "Initialized secrets manager at" first. Each runs here as the built binary,
+// in a new session, which has no terminal, and with neither
+// SB_SECRET_MNEMONIC nor SB_UNLOCK_PASSPHRASE set, so asking for either
+// fails. Each must fail saying only that the vault exists.
+func TestCreateExistingVaultAsksForNothing(t *testing.T) {
+	t.Parallel()
+
+	stateDir := t.TempDir()
+	_, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default", nil, nil)
+	require.NoError(t, err)
+
+	// run runs the built binary with args, which must fail, and returns what
+	// it printed
+	run := func(args ...string) string {
+		//nolint:gosec // G204: test executes the freshly built secret binary
+		cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), args...)
+		cmd.Env = []string{
+			secret.EnvStateDir + "=" + stateDir,
+			"PATH=" + os.Getenv("PATH"),
+			"HOME=" + os.Getenv("HOME"),
+		}
+		cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
+
+		output, err := cmd.CombinedOutput()
+		require.Error(t, err, "secret %v", args)
+
+		return string(output)
+	}
+
+	require.Equal(t,
+		"Error: failed to create default vault: vault default already exists\n",
+		run("init"))
+	require.Equal(t, "Error: vault default already exists\n",
+		run("vault", "create", "default"))
+}
+
 // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for
 // https://git.eeqj.de/sneak/secret/issues/105: `secret init` or `secret vault
 // create` killed after the passphrase prompt but before the unlocker was

internal/cli/crypto.go

@@ -1,16 +1,14 @@
 package cli
 
 import (
+	"bytes"
 	"errors"
 	"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"
@@ -191,24 +189,38 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	return cli.writeOutput(outputFile, func(output io.Writer) error {
-		encryptor, err := age.Encrypt(output, recipient)
-		if err != nil {
-			return fmt.Errorf("failed to create age encryptor: %w", err)
-		}
+	// Set up output writer. With --output the result is held in memory
+	// and written only once encryption has succeeded, so a failure leaves
+	// --output untouched and --input can be the same file.
+	output := cli.cmd.OutOrStdout()
 
-		_, err = io.Copy(encryptor, input)
-		if err != nil {
-			return fmt.Errorf("failed to encrypt data: %w", err)
-		}
+	var result bytes.Buffer
 
-		err = encryptor.Close()
-		if err != nil {
-			return fmt.Errorf("failed to finalize encryption: %w", err)
-		}
+	if outputFile != "" {
+		output = &result
+	}
 
+	// Encrypt the data
+	encryptor, err := age.Encrypt(output, recipient)
+	if err != nil {
+		return fmt.Errorf("failed to create age encryptor: %w", err)
+	}
+
+	_, err = io.Copy(encryptor, input)
+	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)
+	}
+
+	if outputFile == "" {
 		return nil
-	})
+	}
+
+	return cli.writeOutputFile(outputFile, &result)
 }
 
 // Decrypt decrypts data using an age secret key stored in a secret
@@ -269,123 +281,52 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	return cli.writeOutput(outputFile, func(output io.Writer) error {
-		decryptor, err := age.Decrypt(input, identity)
-		if err != nil {
-			return fmt.Errorf("failed to create age decryptor: %w", err)
-		}
+	// Set up output writer. With --output the result is held in memory
+	// and written only once decryption has succeeded, so a failure leaves
+	// --output untouched and --input can be the same file.
+	output := cli.cmd.OutOrStdout()
 
-		_, err = io.Copy(output, decryptor)
-		if err != nil {
-			return fmt.Errorf("failed to decrypt data: %w", err)
-		}
+	var result bytes.Buffer
 
-		return nil
-	})
-}
+	if outputFile != "" {
+		output = &result
+	}
 
-// 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]
+	// Decrypt the data
+	decryptor, err := age.Decrypt(input, identity)
+	if err != nil {
+		return fmt.Errorf("failed to create age decryptor: %w", err)
+	}
+
+	_, err = io.Copy(output, decryptor)
+	if err != nil {
+		return fmt.Errorf("failed to decrypt data: %w", err)
+	}
 
-// 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())
+		return nil
 	}
 
-	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() }()
-
-		return write(file)
-	}
-
-	// An existing file that cannot be written fails here, as writing into it
-	// 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 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
+	return cli.writeOutputFile(outputFile, &result)
 }
 
-// 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
-	}
-
+// writeOutputFile creates outputFile, or empties it if it exists, and
+// writes result into it.
+func (cli *Instance) writeOutputFile(
+	outputFile string, result *bytes.Buffer,
+) error {
+	file, err := cli.fs.Create(outputFile)
 	if err != nil {
-		return "", err
+		return fmt.Errorf("failed to create output file: %w", err)
+	}
+	defer func() { _ = file.Close() }()
+
+	_, err = result.WriteTo(file)
+	if err != nil {
+		return fmt.Errorf("failed to write output file: %w", err)
 	}
 
-	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)
+	return nil
 }
 
 // isValidAgeSecretKey checks if a string is a valid age secret key by

internal/cli/crypto_test.go

@@ -3,10 +3,7 @@ package cli
 
 import (
 	"io"
-	"os"
 	"path/filepath"
-	"strings"
-	"syscall"
 	"testing"
 
 	"github.com/spf13/afero"
@@ -85,58 +82,13 @@ func TestFailedCryptoLeavesOutputUnchanged(t *testing.T) {
 			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)
+			exists, err := afero.Exists(cli.fs, filepath.Join(dir, "new.txt"))
 			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)
+			require.False(t, exists, "a failed %s created --output", tt.name)
 		})
 	}
 }
 
-// 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
@@ -154,151 +106,3 @@ func TestCryptoInPlace(t *testing.T) {
 	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,9 +7,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
-	"syscall"
 	"testing"
-	"time"
 
 	"github.com/awnumar/memguard"
 	"github.com/stretchr/testify/assert"
@@ -107,115 +105,3 @@ 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/init.go

@@ -17,6 +17,9 @@ import (
 // errPassphraseMismatch is returned when passphrase confirmation fails
 var errPassphraseMismatch = errors.New("passphrases do not match")
 
+// defaultVaultName is the name of the vault `secret init` creates
+const defaultVaultName = "default"
+
 // NewInitCmd creates the init command
 func NewInitCmd() *cobra.Command {
 	return &cobra.Command{
@@ -92,8 +95,10 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to create state directory: %w", err)
 	}
 
-	if cmd != nil {
-		cmd.Printf("Initialized secrets manager at: %s\n", stateDir)
+	// Refuse an existing default vault before asking for anything
+	err = vault.CheckNewVaultName(cli.fs, cli.stateDir, defaultVaultName)
+	if err != nil {
+		return fmt.Errorf("failed to create default vault: %w", err)
 	}
 
 	// Prompt for mnemonic
@@ -133,7 +138,7 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 	// Create the default vault with its passphrase unlocker
 	secret.Debug("Creating default vault")
 
-	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, "default",
+	vlt, err := vault.CreateVault(cli.fs, cli.stateDir, defaultVaultName,
 		mnemonic, passphraseBuffer)
 	if err != nil {
 		secret.Debug("Failed to create default vault", "error", err)
@@ -152,6 +157,7 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
 	}
 
 	if cmd != nil {
+		cmd.Printf("Initialized secrets manager at: %s\n", stateDir)
 		cmd.Printf("\nDefault vault created and configured\n")
 		cmd.Printf("Long-term public key: %s\n", ltIdentity.Recipient().String())
 		cmd.Printf("Unlocker ID: %s\n", unlocker.GetID())

internal/cli/leftovers_test.go

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

internal/cli/root.go

@@ -2,7 +2,6 @@ package cli
 
 import (
 	"os"
-	"os/signal"
 
 	"github.com/awnumar/memguard"
 	"github.com/spf13/cobra"
@@ -15,13 +14,12 @@ import (
 // every memguard buffer before it returns, so the caller must do nothing
 // but exit with the code.
 func Entry() int {
-	// On SIGINT, SIGTERM or SIGHUP memguard runs this function, wipes every
-	// buffer and exits with status 1. The function removes the temporary
-	// file that encrypt or decrypt is writing. The passphrase prompt turns
-	// echo off on the controlling terminal until the read finishes, so a
-	// signal there would leave echo off. Only a process in the terminal's
-	// foreground process group may reset it: one in the background that
-	// tries is stopped instead of exiting.
+	// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
+	// and exits with status 1. The passphrase prompt turns echo off on the
+	// controlling terminal until the read finishes, so a signal there would
+	// leave echo off. Only a process in the terminal's 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
@@ -32,20 +30,7 @@ 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
 		}
@@ -54,7 +39,7 @@ func Entry() int {
 		if err == nil && foreground == unix.Getpgrp() {
 			_ = term.Restore(int(terminal.Fd()), terminalState)
 		}
-	}, signals...)
+	}, os.Interrupt, unix.SIGTERM)
 
 	defer memguard.Purge()
 

internal/cli/secrets.go

@@ -443,7 +443,10 @@ func (cli *Instance) GetSecret(cmd *cobra.Command, secretName string) error {
 	defer value.Destroy()
 
 	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cmd.OutOrStdout().Write(value.Bytes())
+	_, err = cmd.OutOrStdout().Write(value.Bytes())
+	if err != nil {
+		return fmt.Errorf("failed to write secret value: %w", err)
+	}
 
 	return nil
 }
@@ -478,7 +481,10 @@ func (cli *Instance) GetSecretWithVersion(
 	secret.Debug("Got secret value", "valueLength", value.Size())
 
 	// Write the value straight from locked memory, with no trailing newline
-	_, _ = cmd.OutOrStdout().Write(value.Bytes())
+	_, err = cmd.OutOrStdout().Write(value.Bytes())
+	if err != nil {
+		return fmt.Errorf("failed to write secret value: %w", err)
+	}
 
 	secret.Debug("Printed value to stdout")
 

internal/cli/vault.go

@@ -267,6 +267,12 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 	}
 	defer release()
 
+	// Refuse an invalid name or an existing vault before asking for anything
+	err = vault.CheckNewVaultName(cli.fs, cli.stateDir, name)
+	if err != nil {
+		return err
+	}
+
 	mnemonic, cleanupMnemonic, err := cli.promptMnemonic()
 	if err != nil {
 		return err

internal/cli/version_test.go

@@ -8,6 +8,7 @@
 // - TestPromoteNonExistentVersion: Tests error handling for invalid promotion
 // - TestGetSecretWithVersion: Tests `secret get --version` flag functionality
 // - TestGetSecretWritesBinaryValue: Tests `secret get` output of binary values
+// - TestGetSecretReturnsWriteError: Tests `secret get` when stdout fails
 // - TestVersionCommandStructure: Tests command structure and help text
 // - TestListVersionsEmptyOutput: Tests edge case with no versions
 //
@@ -20,6 +21,7 @@ package cli
 
 import (
 	"bytes"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -360,6 +362,45 @@ func TestGetSecretWritesBinaryValue(t *testing.T) {
 	assert.Equal(t, value, buf.Bytes())
 }
 
+// TestGetSecretReturnsWriteError checks that `secret get`, with and without
+// a version, returns the error of a failed write of the value.
+func TestGetSecretReturnsWriteError(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
+	cli.Mnemonic = testMnemonicBuffer(t)
+
+	setupTestVault(t, fs)
+
+	vlt, err := vault.GetCurrentVault(fs, testStateDir)
+	require.NoError(t, err)
+
+	addTestSecret(t, vlt, []byte("value"), false)
+
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+
+	versions, err := secret.ListVersions(fs,
+		filepath.Join(vaultDir, "secrets.d", "test%secret"))
+	require.NoError(t, err)
+	require.Len(t, versions, 1)
+
+	// Stdout is a file closed before the value is written to it
+	out, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
+	require.NoError(t, err)
+	require.NoError(t, out.Close())
+
+	cmd := newRootCmd()
+	cmd.SetOut(out)
+
+	err = cli.GetSecret(cmd, "test/secret")
+	require.ErrorIs(t, err, os.ErrClosed)
+
+	err = cli.GetSecretWithVersion(cmd, "test/secret", versions[0])
+	require.ErrorIs(t, err, os.ErrClosed)
+}
+
 //nolint:paralleltest // reads process environment to determine the state dir
 func TestVersionCommandStructure(t *testing.T) {
 	// Test that version commands are properly structured

internal/secret/atomic.go

@@ -10,8 +10,8 @@ import (
 	"github.com/spf13/afero"
 )
 
-// tempNamePart starts the name of every temporary file WriteFileAtomicFunc
-// makes and every temporary directory TempDirFor makes, ".tmp-123".
+// tempNamePart is in the name of every temporary file WriteFileAtomic makes,
+// ".NAME.tmp-123", 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,38 +21,17 @@ 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 {
-	return WriteFileAtomicFunc(fs, path, func(file afero.File) error {
-		_, 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)
+	tmp, err := afero.TempFile(fs, filepath.Dir(path),
+		"."+filepath.Base(path)+tempNamePart+"*")
 	if err != nil {
 		return fmt.Errorf("failed to create temporary file for %s: %w", path, err)
 	}
 
-	err = write(tmp)
-	if err != nil {
-		_ = tmp.Close()
-		_ = fs.Remove(tmp.Name())
-
-		return err
+	_, err = tmp.Write(data)
+	if err == nil {
+		err = tmp.Sync()
 	}
 
-	err = tmp.Sync()
-
 	closeErr := tmp.Close()
 	if err == nil {
 		err = closeErr

internal/vault/management.go

@@ -212,16 +212,40 @@ func processMnemonicForVault(
 	return ltIdentity, derivationIndex, publicKeyHash, familyHash, nil
 }
 
+// CheckNewVaultName returns an error when name cannot be the name of a new
+// vault: one wrapping ErrInvalidVaultName when it is not a valid vault name,
+// and one wrapping ErrVaultExists when that vault already exists. Creating an
+// existing vault again would replace its keys, and its secrets could no
+// longer be decrypted. CreateVault checks this before writing anything; the
+// commands that create a vault also check it before they ask for the
+// mnemonic or passphrase, holding the state directory lock.
+func CheckNewVaultName(fs afero.Fs, stateDir string, name string) error {
+	err := ValidateVaultName(name)
+	if err != nil {
+		return err
+	}
+
+	exists, err := afero.DirExists(fs, filepath.Join(stateDir, "vaults.d", name))
+	if err != nil {
+		return fmt.Errorf("failed to check if vault exists: %w", err)
+	}
+
+	if exists {
+		return fmt.Errorf("vault %s %w", name, ErrVaultExists)
+	}
+
+	return nil
+}
+
 // CreateVault creates a new vault and selects it as the current vault. When
 // mnemonic is not nil, the vault's long-term key is derived from it, and the
 // returned vault has it as its Mnemonic; when it is nil, the vault has no
 // long-term key until one is imported. When passphrase is not nil, the vault
 // gets a passphrase unlocker protected by it, as its current unlocker; that
-// needs a mnemonic. It refuses a vault that already exists before writing
-// anything: creating it again would replace its keys, and its secrets could
-// no longer be decrypted. The commands that call it hold the state directory
-// lock, so no other command can create the vault between the check and the
-// writes.
+// needs a mnemonic. It refuses an invalid name or a vault that already exists,
+// as CheckNewVaultName does, before writing anything. The commands that call
+// it hold the state directory lock, so no other command can create the vault
+// between the check and the writes.
 //
 // The vault is written whole into a temporary directory, which is renamed
 // into vaults.d only once complete, and only then selected: a crash at any
@@ -233,30 +257,17 @@ func CreateVault(
 ) (*Vault, error) {
 	secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
 
-	err := ValidateVaultName(name)
+	err := CheckNewVaultName(fs, stateDir, name)
 	if err != nil {
-		secret.Debug("Invalid vault name provided", "vault_name", name)
-
 		return nil, err
 	}
 
-	secret.Debug("Vault name validation passed", "vault_name", name)
-
-	vaultDir := filepath.Join(stateDir, "vaults.d", name)
-
-	exists, err := afero.DirExists(fs, vaultDir)
-	if err != nil {
-		return nil, fmt.Errorf("failed to check if vault exists: %w", err)
-	}
-
-	if exists {
-		return nil, fmt.Errorf("vault %s %w", name, ErrVaultExists)
-	}
-
 	if passphrase != nil && mnemonic == nil {
 		return nil, fmt.Errorf("vault %s %w", name, ErrUnlockerWithoutMnemonic)
 	}
 
+	vaultDir := filepath.Join(stateDir, "vaults.d", name)
+
 	secret.Debug("Creating vault directory structure", "vault_dir", vaultDir)
 
 	err = secret.WriteDir(fs, vaultDir, func(dir string) error {