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 path it is a symbolic link to, and rename that over it only
on success, using the new `secret.WriteFileAtomicFunc`, which
`secret.WriteFileAtomic` now calls. Ctrl-C or SIGTERM removes the
temporary file. A device or pipe given as `--output`, such as
`/dev/null`, is written to directly.

Judgement call: the device and pipe case goes beyond the plan.

Model: opus-5-5

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
-	output := cli.cmd.OutOrStdout()
-
-	if outputFile != "" {
-		file, err := cli.fs.Create(outputFile)
+	return cli.writeOutput(outputFile, func(output io.Writer) error {
+		encryptor, err := age.Encrypt(output, recipient)
 		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
-	encryptor, err := age.Encrypt(output, recipient)
-	if err != nil {
-		return fmt.Errorf("failed to create age encryptor: %w", err)
-	}
+		err = encryptor.Close()
+		if err != nil {
+			return fmt.Errorf("failed to finalize encryption: %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)
-	}
-
-	return nil
+		return nil
+	})
 }
 
 // Decrypt decrypts data using an age secret key stored in a secret
@@ -278,31 +269,99 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
 		input = file
 	}
 
-	// Set up output writer
-	output := cli.cmd.OutOrStdout()
-
-	if outputFile != "" {
-		file, err := cli.fs.Create(outputFile)
+	return cli.writeOutput(outputFile, func(output io.Writer) error {
+		decryptor, err := age.Decrypt(input, identity)
 		if err != nil {
-			return fmt.Errorf("failed to create output file: %w", err)
+			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)
+		}
+
+		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.
+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 open output file: %w", err)
 		}
 		defer func() { _ = file.Close() }()
 
-		output = file
+		return write(file)
 	}
 
-	// Decrypt the data
-	decryptor, err := age.Decrypt(input, identity)
+	path, err := outputPath(outputFile)
 	if err != nil {
-		return fmt.Errorf("failed to create age decryptor: %w", err)
+		return fmt.Errorf("failed to open 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. 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) {
+		return outputFile, 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