From 2fbc32fe750dac78eff6a05291ef24f910e85bfe Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 14 Oct 2026 23:07:07 +0200 Subject: [PATCH] Check the -o path before looking for the mnemonic (closes #87) age encrypt and age decrypt looked at the -o path only after the mnemonic had been obtained, so a path that could never be written still prompted for the mnemonic or ran the mnemonic command. Both now refuse, before the key is derived, an -o path that is a directory or a symlink to one, whose directory is missing or is not a directory, or that is a symlink pointing at nothing. The check creates nothing; output still looks at the path when it writes, so the file beside the target is made when it was before. The README names the -o path among what is checked first. Left out: a directory the tool may not write to is found only when it writes. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 10 +++--- internal/cli/age/age.go | 67 +++++++++++++++++++++++++++++++++++++--- internal/cli/age_test.go | 38 +++++++++++++++++++++++ 3 files changed, 106 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 589691a..6d676bd 100644 --- a/README.md +++ b/README.md @@ -126,10 +126,12 @@ the list is named by its position, never printed. Keys are derived from the mnemonic's words joined by single spaces, whatever whitespace is around or between them, so one word per line, tabs or extra spaces give the same keys. -The key index, `--words`, the `--to` recipients and the file `age encrypt` or -`age decrypt` is given to read are checked before the mnemonic is looked for, so -a run refused for one of them never prompts for the mnemonic or runs the -mnemonic command. +The key index, `--words`, the `--to` recipients, the file `age encrypt` or +`age decrypt` is given to read and its `-o` path are checked before the mnemonic +is looked for, so a run refused for one of them never prompts for the mnemonic +or runs the mnemonic command. An `-o` path is refused at that point if it is a +directory or a symlink to one, if its directory is missing or is not a +directory, or if it is a symlink that points at nothing. `KEYFUNC_MNEMONIC` and `KEYFUNC_MNEMONIC_COMMAND` are removed from the environment before the mnemonic command, the system `ssh` (`keyfunc ssh to`) and diff --git a/internal/cli/age/age.go b/internal/cli/age/age.go index 7256248..feb5137 100644 --- a/internal/cli/age/age.go +++ b/internal/cli/age/age.go @@ -11,6 +11,7 @@ import ( "os" "os/signal" "path/filepath" + "syscall" "github.com/spf13/cobra" "sneak.berlin/go/keyfunc/internal/agekey" @@ -108,9 +109,9 @@ func decrypt() *cobra.Command { } // runEncrypt encrypts to the derived recipient and any others given. -// The recipients are read and the input opened before the key is -// derived, so that a run refused for either never asks for the -// mnemonic. +// The recipients are read, the -o path looked at and the input opened +// before the key is derived, so that a run refused for any of them +// never asks for the mnemonic. func runEncrypt(cmd *cobra.Command, args []string) error { names, err := cmd.Flags().GetStringArray("to") if err != nil { @@ -127,6 +128,11 @@ func runEncrypt(cmd *cobra.Command, args []string) error { return fmt.Errorf("reading the armor flag: %w", err) } + err = checkOutput(cmd) + if err != nil { + return err + } + src, closeSrc, err := input(cmd, args) if err != nil { return err @@ -144,9 +150,14 @@ func runEncrypt(cmd *cobra.Command, args []string) error { }) } -// runDecrypt decrypts with the derived identity. The input is opened -// before the key is derived, as for runEncrypt. +// runDecrypt decrypts with the derived identity. The -o path is looked +// at and the input opened before the key is derived, as for runEncrypt. func runDecrypt(cmd *cobra.Command, args []string) error { + err := checkOutput(cmd) + if err != nil { + return err + } + src, closeSrc, err := input(cmd, args) if err != nil { return err @@ -225,6 +236,52 @@ func input(cmd *cobra.Command, args []string) (io.Reader, func(), error) { return file, func() { _ = file.Close() }, nil } +// checkOutput refuses an -o path that output could not write to, +// before anything is written: a directory or a symlink to one, one +// whose directory is missing or is not a directory, or a symlink that +// points at nothing. It creates nothing; output looks at the path again +// when it writes. +func checkOutput(cmd *cobra.Command) error { + name, err := cmd.Flags().GetString("output") + if err != nil { + return fmt.Errorf("reading the output file: %w", err) + } + + if name == "" { + return nil + } + + info, err := os.Lstat(name) + + switch { + case errors.Is(err, fs.ErrNotExist): + // replace makes its new file in this directory. When the + // directory is a file instead, os.Lstat has already failed with + // ENOTDIR, the case below. + directory := filepath.Dir(name) + + _, err = os.Stat(directory) + if err != nil { + return fmt.Errorf("looking at %s: %w", directory, err) + } + + return nil + case err != nil: + return fmt.Errorf("looking at %s: %w", name, err) + case info.Mode().Type() == fs.ModeSymlink: + info, err = os.Stat(name) + if err != nil { + return fmt.Errorf("following %s: %w", name, err) + } + } + + if info.IsDir() { + return fmt.Errorf("writing to %s: %w", name, syscall.EISDIR) + } + + return nil +} + // output has the work write to the named path, going by what is there // without following a final symlink: // diff --git a/internal/cli/age_test.go b/internal/cli/age_test.go index f62b48d..fb382d6 100644 --- a/internal/cli/age_test.go +++ b/internal/cli/age_test.go @@ -108,6 +108,44 @@ func TestAnInputFileThatCannotBeOpenedIsRefused(t *testing.T) { } } +// The mnemonic command fails in these runs, so the refusal of the -o +// path shows that it was looked at before the mnemonic was looked for. +func TestAnOutputPathThatCannotBeWrittenIsRefused(t *testing.T) { + t.Setenv(mnemonic.CommandVariable, failingMnemonicCommand) + + plain := written(t, "notes.txt", "the secret\n") + directory := t.TempDir() + missing := filepath.Join(directory, "missing") + dangling := filepath.Join(directory, "dangling") + require.NoError(t, os.Symlink(missing, dangling)) + + toDirectory := filepath.Join(directory, "to-directory") + require.NoError(t, os.Symlink(t.TempDir(), toDirectory)) + + refusals := map[string]error{ + filepath.Join(missing, "notes.age"): fs.ErrNotExist, + filepath.Join(plain, "notes.age"): syscall.ENOTDIR, + dangling: fs.ErrNotExist, + directory: syscall.EISDIR, + toDirectory: syscall.EISDIR, + } + + for name, refusal := range refusals { + out, err := execute(t, "age", "encrypt", "-o", name, plain) + require.ErrorIs(t, err, refusal, name) + require.Empty(t, out, name) + + out, err = execute(t, "age", "decrypt", "-o", name, plain) + require.ErrorIs(t, err, refusal, name) + require.Empty(t, out, name) + } + + // Nothing was made beside the two symlinks. + left, err := os.ReadDir(directory) + require.NoError(t, err) + require.Len(t, left, 2) +} + func TestAFileForAnotherKeyIsRefused(t *testing.T) { t.Setenv(mnemonic.Variable, example())