Write encrypt and decrypt output only once they succeed (closes #143)
check / check (push) Waiting to run

`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
This commit is contained in:
2026-10-14 21:59:00 +00:00
parent 72cd6c30a1
commit e4a866949b
8 changed files with 616 additions and 54 deletions
+119 -36
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,123 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
input = file
}
// Set up output writer
output := cli.cmd.OutOrStdout()
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)
}
if outputFile != "" {
file, err := cli.fs.Create(outputFile)
_, 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.
// 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
decryptor, err := age.Decrypt(input, identity)
// 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 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