fix(cli): refuse an output destination that aliases the input

otdfctl/cmd/tdf/decrypt.go

@@ -76,7 +76,7 @@ func decryptRun(cmd *cobra.Command, args []string) {
 	// straight to it rather than accumulating in memory first.
 	var dest io.Writer = os.Stdout
 	if output != "" {
-		outFile, err = streamio.NewOutputFile(output, decryptedOutputFileMode)
+		outFile, err = streamio.NewOutputFile(output, decryptedOutputFileMode, in)
 		if err != nil {
 			fail("Failed to write decrypted data to file", err)
 		}

otdfctl/cmd/tdf/encrypt.go

@@ -120,7 +120,9 @@ func encryptRun(cmd *cobra.Command, args []string) {
 	// The SDK seeks to the end of the payload to size it, so the input has to be
 	// seekable. A file already is; a pipe is spooled to disk, which trades the
 	// temporary file for the memory a whole-payload read used to cost.
-	var in io.ReadSeeker
+	// Held as *os.File rather than io.ReadSeeker so NewOutputFile can check the
+	// destination against it.
+	var in *os.File
 	var cleanup func()
 	if filePath != "" {
 		f, err := os.Open(filePath)
@@ -148,7 +150,7 @@ func encryptRun(cmd *cobra.Command, args []string) {
 		if !strings.HasSuffix(out, ".tdf") {
 			out += ".tdf"
 		}
-		tdfFile, err = streamio.NewOutputFile(out, encryptedOutputFileMode)
+		tdfFile, err = streamio.NewOutputFile(out, encryptedOutputFileMode, in)
 		if err != nil {
 			cleanup()
 			cli.ExitWithError("Failed to write encrypted file "+out, err)

otdfctl/e2e/streaming.bats

@@ -124,7 +124,7 @@ assert_no_leftovers() {
   assert_failure
   [ ! -f "$TDF_OUT" ]
 
-  assert_no_leftovers "$BATS_TEST_TMPDIR/.payload.txt.tdf.tmp-*"
+  assert_no_leftovers "$BATS_TEST_TMPDIR/.otdfctl.tmp-*"
 }
 
 @test "decrypt leaves no output behind when it fails" {
@@ -135,7 +135,7 @@ assert_no_leftovers() {
   assert_failure
   [ ! -f "$RESULT" ]
 
-  assert_no_leftovers "$BATS_TEST_TMPDIR/.payload.out.tmp-*"
+  assert_no_leftovers "$BATS_TEST_TMPDIR/.otdfctl.tmp-*"
 }
 
 # The payoff of writing to a temp sibling: a failed decrypt over an existing
@@ -163,6 +163,33 @@ assert_no_leftovers() {
   [ -c /dev/null ]
 }
 
+# The other side of writing through: -o at a symlink resolving to the input
+# would truncate the input at open, before it has been read. Both commands open
+# their input first, so both could destroy a file they were only asked to read.
+@test "decrypt refuses an output symlink pointing at its input" {
+  ./otdfctl encrypt -o "$TDF_OUT" $COMMON "$PLAIN"
+  ln -s "$TDF_OUT" "$BATS_TEST_TMPDIR/alias.tdf"
+
+  run ./otdfctl decrypt -o "$BATS_TEST_TMPDIR/alias.tdf" $COMMON "$TDF_OUT"
+  assert_failure
+  assert_output --partial "output would overwrite the input"
+
+  # The TDF is still a TDF, not a zero-byte file.
+  ./otdfctl decrypt -o "$RESULT" $COMMON "$TDF_OUT"
+  diff "$PLAIN" "$RESULT"
+}
+
+@test "encrypt refuses an output symlink pointing at its input" {
+  ln -s "$PLAIN" "$BATS_TEST_TMPDIR/alias.tdf"
+
+  run ./otdfctl encrypt -o "$BATS_TEST_TMPDIR/alias.tdf" $COMMON "$PLAIN"
+  assert_failure
+  assert_output --partial "output would overwrite the input"
+
+  run cat "$PLAIN"
+  assert_output "$SECRET_TEXT"
+}
+
 # The point of DSPX-4499: peak RSS is bounded by segment size, not payload size.
 # Needs GNU time for 'Maximum resident set size'; BSD/shell time cannot report it.
 @test "encrypt and decrypt peak memory stay bounded on a large payload" {

otdfctl/pkg/streamio/output.go

@@ -13,10 +13,24 @@ import (
 // OutputFile that had already been committed or discarded.
 var ErrOutputFileFinished = errors.New("streamio: output file already committed or discarded")
 
+// ErrOutputAliasesInput reports that the destination resolves to the file being
+// read. Opening it would truncate the input before it has been read.
+var ErrOutputAliasesInput = errors.New("streamio: output would overwrite the input")
+
 // tempFileAttempts bounds the search for an unused temporary name, so a
 // pathological directory cannot spin here forever.
 const tempFileAttempts = 1000
 
+// tempFilePrefix names the temporary files created beside a destination.
+//
+// Deliberately fixed rather than derived from the destination: a component name
+// is capped (255 bytes on most filesystems), and a prefix carrying the
+// destination's own basename overflows that cap for a destination whose name is
+// itself valid, failing the open before a single byte has been read. The random
+// suffix is what distinguishes concurrent writers; the prefix only has to be
+// recognizable enough to sweep up after a crash.
+const tempFilePrefix = ".otdfctl.tmp-"
+
 // OutputFile writes to a temporary file alongside the destination and renames
 // it into place only once the write has succeeded, so an interrupted or failed
 // run leaves no partial output where a complete file is expected.
@@ -50,26 +64,68 @@ type OutputFile struct {
 // destination after the process umask has been applied. A destination written
 // through directly is opened with mode, which the umask likewise applies to,
 // and which has no effect at all on a destination that already exists.
-func NewOutputFile(path string, mode os.FileMode) (*OutputFile, error) {
+//
+// input is the file the caller is reading, already open, and is what the
+// destination is checked against — pass nil when there is none. Callers open
+// their input before their output precisely so a failure to read costs nothing;
+// without this the write-through open would undo that.
+func NewOutputFile(path string, mode os.FileMode, input *os.File) (*OutputFile, error) {
 	direct, err := isDirectDestination(path)
 	if err != nil {
 		return nil, err
 	}
 	if direct {
+		// Only the write-through open is destructive. An ordinary destination
+		// naming the input goes to a temp sibling and is renamed over it once
+		// the read has finished, which is in-place decryption working as asked
+		// rather than a footgun.
+		alias, err := aliasesInput(path, input)
+		if err != nil {
+			return nil, err
+		}
+		if alias {
+			return nil, fmt.Errorf("%w: %s", ErrOutputAliasesInput, path)
+		}
+
 		f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode)
 		if err != nil {
 			return nil, err
 		}
 		return &OutputFile{f: f, path: path, direct: true}, nil
 	}
 
-	f, err := createTemp(filepath.Dir(path), "."+filepath.Base(path)+".tmp-", mode)
+	f, err := createTemp(filepath.Dir(path), tempFilePrefix, mode)
 	if err != nil {
 		return nil, err
 	}
 	return &OutputFile{f: f, path: path}, nil
 }
 
+// aliasesInput reports whether a write-through open of path would land on
+// input.
+//
+// os.Stat, not os.Lstat: a symlink destination is the case that matters, since
+// the open follows it, and the link's own identity is never what gets written.
+// A dangling symlink resolves to nothing and aliases nothing — the open creates
+// the target.
+func aliasesInput(path string, input *os.File) (bool, error) {
+	if input == nil {
+		return false, nil
+	}
+	inputInfo, err := input.Stat()
+	if err != nil {
+		return false, err
+	}
+	destInfo, err := os.Stat(path)
+	if errors.Is(err, os.ErrNotExist) {
+		return false, nil
+	}
+	if err != nil {
+		return false, err
+	}
+	return os.SameFile(inputInfo, destInfo), nil
+}
+
 // createTemp is os.CreateTemp with a caller-chosen mode. os.CreateTemp hardcodes
 // 0600, and correcting that afterwards with Chmod would ignore the umask, so
 // this opens the file directly to let mode reach the kernel.