sneak/secret
1
0
Fork 0
Code Issues 3 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

secret decrypt and secret encrypt truncate the --output file before they can fail, and --input equal to --output destroys the input #143

New Issue
Closed
opened 2026-10-13 00:30:25 +02:00 by clawbot · 3 comments
clawbot commented 2026-10-13 00:30:25 +02:00
Collaborator
Copy Link
Copy Source

Severity: major. A failing command empties a file the user named, and giving the same file as input and output silently replaces a document with the encryption of nothing. Nothing stored in the vault is affected.

What happens. Both commands create (truncate) the output file before reading any input or checking the key.

printf precious | tee out.txt
secret decrypt mykey -i wrongkey.age -o out.txt     # Error: ... no identity matched any of the recipients
wc -c out.txt                                       # 0
cp doc.txt same.txt
secret encrypt mykey -i same.txt -o same.txt        # exit 0; same.txt is an age file with an empty payload
secret decrypt mykey -i same.age -o same.age        # Error: ... failed to read intro: EOF; same.age is 0 bytes

Where.

secret/internal/cli/crypto.go Line 195 in ef0ae90768
file, err := cli.fs.Create(outputFile)
(encrypt),
secret/internal/cli/crypto.go Line 285 in ef0ae90768
file, err := cli.fs.Create(outputFile)
and
secret/internal/cli/crypto.go Line 295 in ef0ae90768
decryptor, err := age.Decrypt(input, identity)
(decrypt creates the output, then age.Decrypt fails on the header).

Correct behaviour. The README documents neither case. Recommended: write to a temporary file next to the output and rename it into place only on success, as secret.WriteFileAtomic does inside the state directory; that also makes input and output the same file safe. Refusing -i f -o f outright (os.SameFile) is the simpler alternative.

Definition of done

  • A failed encrypt or decrypt leaves an existing output file unchanged and creates no empty one.
  • -i f -o f either fails before touching f or produces the right result.
  • Tests for both cases fail before the fix.

Model: fable-5-1

Severity: major. A failing command empties a file the user named, and giving the same file as input and output silently replaces a document with the encryption of nothing. Nothing stored in the vault is affected. **What happens.** Both commands create (truncate) the output file before reading any input or checking the key. ``` printf precious | tee out.txt secret decrypt mykey -i wrongkey.age -o out.txt # Error: ... no identity matched any of the recipients wc -c out.txt # 0 cp doc.txt same.txt secret encrypt mykey -i same.txt -o same.txt # exit 0; same.txt is an age file with an empty payload secret decrypt mykey -i same.age -o same.age # Error: ... failed to read intro: EOF; same.age is 0 bytes ``` **Where.** https://git.eeqj.de/sneak/secret/src/commit/ef0ae90768c8b18a829033b3ebd4cf98fcaeb3d1/internal/cli/crypto.go#L195 (encrypt), https://git.eeqj.de/sneak/secret/src/commit/ef0ae90768c8b18a829033b3ebd4cf98fcaeb3d1/internal/cli/crypto.go#L285 and https://git.eeqj.de/sneak/secret/src/commit/ef0ae90768c8b18a829033b3ebd4cf98fcaeb3d1/internal/cli/crypto.go#L295 (decrypt creates the output, then `age.Decrypt` fails on the header). **Correct behaviour.** The README documents neither case. Recommended: write to a temporary file next to the output and rename it into place only on success, as `secret.WriteFileAtomic` does inside the state directory; that also makes input and output the same file safe. Refusing `-i f -o f` outright (`os.SameFile`) is the simpler alternative. **Definition of done** - A failed encrypt or decrypt leaves an existing output file unchanged and creates no empty one. - `-i f -o f` either fails before touching `f` or produces the right result. - Tests for both cases fail before the fix. Model: fable-5-1
sneak was assigned by clawbot 2026-10-13 00:30:25 +02:00
clawbot commented 2026-10-14 19:42:53 +02:00
Author
Collaborator
Copy Link
Copy Source

Reassigned to clawbot: sneak ruled that nothing is a release candidate while a known bug is open (sneak/project-management#32).

Plan: the issue's recommendation: write to a temporary file next to the output and rename it into place only on success, which also makes the same file as input and output give the right result. The temporary file is created with mode 0600, as os.CreateTemp does, which suits a file holding a decrypted secret.

Model: opus-5-5

Reassigned to clawbot: sneak ruled that nothing is a release candidate while a known bug is open (https://git.eeqj.de/sneak/project-management/issues/32). Plan: the issue's recommendation: write to a temporary file next to the output and rename it into place only on success, which also makes the same file as input and output give the right result. The temporary file is created with mode 0600, as `os.CreateTemp` does, which suits a file holding a decrypted secret. Model: opus-5-5
sneak was unassigned by clawbot 2026-10-14 19:42:54 +02:00
clawbot self-assigned this 2026-10-14 19:42:54 +02:00
clawbot added this to the 1.0.0 milestone 2026-10-14 19:42:54 +02:00
clawbot commented 2026-10-14 19:59:38 +02:00
Author
Collaborator
Copy Link
Copy Source

Built as planned in #162.

Beyond the plan: a device or pipe given as --output, such as /dev/null, is written to directly, since renaming over it fails in /dev for a normal user and replaces the device as root.

Judgement call: a symbolic link given as --output is replaced by the new file, not followed.

Model: opus-5-5

Built as planned in https://git.eeqj.de/sneak/secret/pulls/162. Beyond the plan: a device or pipe given as `--output`, such as `/dev/null`, is written to directly, since renaming over it fails in `/dev` for a normal user and replaces the device as root. Judgement call: a symbolic link given as `--output` is replaced by the new file, not followed. Model: opus-5-5
clawbot commented 2026-10-15 01:33:00 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan changed after four reviews of #162. Writing to a temporary file and renaming it over --output behaves differently from next for links, devices, /dev/fd, single-file mounts and interrupted runs, and each fix added more code.

New plan: secret encrypt and secret decrypt produce the whole result in memory and open --output only once that has succeeded, the same way next opens it. A failed command then never touches --output, the same file as input and output gives the right result because the input is read in full first, and every kind of --output behaves as on next. The one difference, stated in the README: the result is held in memory before it is written, so the command needs about as much memory as the result. The temporary-file code, the interrupt handling and the test changes that came with it are removed.

Model: opus-5-5

Plan changed after four reviews of https://git.eeqj.de/sneak/secret/pulls/162. Writing to a temporary file and renaming it over `--output` behaves differently from `next` for links, devices, `/dev/fd`, single-file mounts and interrupted runs, and each fix added more code. New plan: `secret encrypt` and `secret decrypt` produce the whole result in memory and open `--output` only once that has succeeded, the same way `next` opens it. A failed command then never touches `--output`, the same file as input and output gives the right result because the input is read in full first, and every kind of `--output` behaves as on `next`. The one difference, stated in the README: the result is held in memory before it is written, so the command needs about as much memory as the result. The temporary-file code, the interrupt handling and the test changes that came with it are removed. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/secret#143