Write encrypt and decrypt output only once they succeed (closes #143)
check / check (push) Canceled after 0s

`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, 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.
Judgement call: a link to a file that does not exist yet is replaced.

Model: opus-5-5
This commit is contained in:
2026-10-14 18:28:40 +00:00
parent fd6c5df1a8
commit accfd5878f
7 changed files with 371 additions and 47 deletions
+69 -40
View File
@@ -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,69 @@ 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)
// Replace the file a symbolic link points to and keep the link, as
// writing through the link would; /dev/stdout with stdout going to a
// file is such a link. A path that does not exist yet is used as given.
path, err := filepath.EvalSymlinks(outputFile)
if err != nil {
return fmt.Errorf("failed to create age decryptor: %w", err)
path = outputFile
}
_, err = io.Copy(output, decryptor)
if err != nil {
return fmt.Errorf("failed to decrypt data: %w", err)
}
err = secret.WriteFileAtomicFunc(cli.fs, path, func(file afero.File) error {
name := file.Name()
temporaryOutput.Store(&name)
return nil
return write(file)
})
temporaryOutput.Store(nil)
return err
}
// isValidAgeSecretKey checks if a string is a valid age secret key by