fix(cli): drop the encrypt-side stdin spool

otdfctl/cmd/tdf/encrypt.go

@@ -1,6 +1,7 @@
 package tdf
 
 import (
+	"bytes"
 	"errors"
 	"io"
 	"log/slog"
@@ -27,26 +28,37 @@ var (
 	EncryptCmd = &encryptDoc.Command
 )
 
-// detectMimeType sniffs the payload's type from its head and rewinds, so the
-// whole payload still reaches the encoder.
+// detectMimeType sniffs the payload's type from its head and returns a reader
+// that still yields the whole payload.
 //
 // Detection needs only the first megabyte, which is what mimetype is limited to
-// anyway, so this reads a bounded prefix rather than the whole payload.
-func detectMimeType(in io.ReadSeeker, fileExt string) (string, error) {
+// anyway, so this reads a bounded prefix rather than the whole payload. A
+// seekable input is rewound and handed back unchanged, which matters: the SDK
+// measures a seekable payload and keeps the archive in the compact ZIP32
+// layout. A pipe cannot be rewound, so the sniffed prefix is pushed back in
+// front of it instead — a megabyte held in memory at most.
+func detectMimeType(in io.Reader, fileExt string) (string, io.Reader, error) {
 	mimetype.SetLimit(Size1MB) // limit to 1MB
 
 	head := make([]byte, Size1MB)
 	// A payload shorter than the sniff window is the common case, not an error.
 	n, err := io.ReadFull(in, head)
 	if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) {
-		return "", err
+		return "", nil, err
 	}
-	if _, err := in.Seek(0, io.SeekStart); err != nil {
-		return "", err
+	head = head[:n]
+
+	rest := in
+	if seeker, ok := in.(io.Seeker); ok {
+		if _, err := seeker.Seek(0, io.SeekStart); err != nil {
+			return "", nil, err
+		}
+	} else {
+		rest = io.MultiReader(bytes.NewReader(head), in)
 	}
 
 	// defaults to application/octet-stream if nothing is recognized
-	detected := mimetype.Detect(head[:n]).String()
+	detected := mimetype.Detect(head).String()
 	if detected == "application/octet-stream" && fileExt != "" {
 		// mime.TypeByExtension is the extension lookup. mimetype.Lookup takes a
 		// MIME type string, so passing it a bare extension always returned nil
@@ -57,7 +69,7 @@ func detectMimeType(in io.ReadSeeker, fileExt string) (string, error) {
 			detected = byExt
 		}
 	}
-	return detected, nil
+	return detected, rest, nil
 }
 
 func encryptRun(cmd *cobra.Command, args []string) {
@@ -115,26 +127,21 @@ func encryptRun(cmd *cobra.Command, args []string) {
 		cliExit("ONLY ONE")
 	}
 
-	// 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
-	var cleanup func()
+	// The SDK encrypts straight from a reader, so piped input goes to it as-is
+	// rather than through a temporary file. A file is still opened seekably: the
+	// SDK measures a seekable payload and keeps the archive in the compact ZIP32
+	// layout, which a pipe has to give up.
+	var in io.Reader = piped
+	cleanup := func() {}
 	if filePath != "" {
 		f, err := os.Open(filePath)
 		if err != nil {
 			cli.ExitWithError("Failed to read file:", err)
 		}
 		in, cleanup = f, func() { f.Close() }
-	} else {
-		f, spoolCleanup, err := streamio.Spool(piped)
-		if err != nil {
-			cli.ExitWithError("Failed to read stdin:", err)
-		}
-		in, cleanup = f, spoolCleanup
 	}
 	// cli.ExitWithError calls os.Exit, which skips deferred functions, so every
-	// exit below goes through fail() to discard the spool and any partial output.
+	// exit below goes through fail() to discard any partial output.
 	defer cleanup()
 
 	// Resolve the destination before encrypting, so the payload streams straight
@@ -168,7 +175,7 @@ func encryptRun(cmd *cobra.Command, args []string) {
 	// auto-detect mime type if not provided
 	if fileMimeType == "" {
 		slog.Debug("detecting mime type of file")
-		fileMimeType, err = detectMimeType(in, fileExt)
+		fileMimeType, in, err = detectMimeType(in, fileExt)
 		if err != nil {
 			fail("Failed to read file:", err)
 		}

otdfctl/cmd/tdf/encrypt_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestDetectMimeTypeRewindsForTheEncoder(t *testing.T) {
+func TestDetectMimeTypePreservesThePayload(t *testing.T) {
 	for _, tc := range []struct {
 		name    string
 		content string
@@ -19,30 +19,60 @@ func TestDetectMimeTypeRewindsForTheEncoder(t *testing.T) {
 		{name: "text", content: "hello, world\n", want: "text/plain; charset=utf-8"},
 		{name: "json", content: `{"a":1}`, want: "application/json"},
 		// Larger than the sniff window, so a detector that consumed the reader
-		// instead of rewinding would truncate the payload.
+		// without handing the prefix back would truncate the payload.
 		{name: "larger than sniff window", content: strings.Repeat("a", Size1MB+7), want: "text/plain; charset=utf-8"},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			in := strings.NewReader(tc.content)
+			for _, seekable := range []bool{true, false} {
+				name := "seekable"
+				if !seekable {
+					name = "pipe"
+				}
+				t.Run(name, func(t *testing.T) {
+					var in io.Reader = strings.NewReader(tc.content)
+					if !seekable {
+						in = pipeReader{in}
+					}
 
-			got, err := detectMimeType(in, "")
-			require.NoError(t, err)
-			assert.Equal(t, tc.want, got)
+					got, rest, err := detectMimeType(in, "")
+					require.NoError(t, err)
+					assert.Equal(t, tc.want, got)
 
-			// The whole payload must still reach the encoder.
-			all, err := io.ReadAll(in)
-			require.NoError(t, err)
-			assert.Len(t, all, len(tc.content))
+					// The whole payload must still reach the encoder.
+					all, err := io.ReadAll(rest)
+					require.NoError(t, err)
+					assert.Equal(t, tc.content, string(all))
+				})
+			}
 		})
 	}
 }
 
+// pipeReader hides the Seek method of the reader it wraps, standing in for
+// stdin on the end of a pipe.
+type pipeReader struct{ inner io.Reader }
+
+func (r pipeReader) Read(p []byte) (int, error) { return r.inner.Read(p) }
+
+// TestDetectMimeTypeKeepsSeekability guards the ZIP32 layout: the SDK only
+// measures a payload it can seek, and a sniffed input that came back wrapped in
+// io.MultiReader would silently force every file encrypt to ZIP64.
+func TestDetectMimeTypeKeepsSeekability(t *testing.T) {
+	in := strings.NewReader("hello, world\n")
+
+	_, rest, err := detectMimeType(in, "")
+	require.NoError(t, err)
+
+	_, ok := rest.(io.Seeker)
+	assert.True(t, ok, "a seekable input must stay seekable")
+}
+
 func TestDetectMimeTypeFallsBackToExtension(t *testing.T) {
 	// Bytes mimetype cannot classify, so the extension decides. ".pdf" is in
 	// Go's builtin table, so this does not depend on the host's mime.types.
 	unrecognized := bytes.Repeat([]byte{0x01, 0x02, 0x03, 0x04}, 8)
 
-	got, err := detectMimeType(bytes.NewReader(unrecognized), "pdf")
+	got, _, err := detectMimeType(bytes.NewReader(unrecognized), "pdf")
 	require.NoError(t, err)
 	assert.Equal(t, "application/pdf", got)
 }
@@ -53,15 +83,15 @@ func TestDetectMimeTypeUnknownExtensionStaysOctetStream(t *testing.T) {
 	// The previous implementation called mimetype.Lookup(fileExt).String().
 	// Lookup takes a MIME type string, not an extension, so it returned nil for
 	// every extension and this path panicked.
-	got, err := detectMimeType(bytes.NewReader(unrecognized), "zzzznotathing")
+	got, _, err := detectMimeType(bytes.NewReader(unrecognized), "zzzznotathing")
 	require.NoError(t, err)
 	assert.Equal(t, "application/octet-stream", got)
 }
 
 func TestDetectMimeTypeEmptyPayload(t *testing.T) {
 	in := strings.NewReader("")
 
-	got, err := detectMimeType(in, "")
+	got, _, err := detectMimeType(in, "")
 	require.NoError(t, err)
 	assert.Equal(t, "text/plain", got)
 }

otdfctl/e2e/streaming.bats

@@ -126,6 +126,35 @@ setup() {
   assert_output "0"
 }
 
+# extra_field_len reads the extra field length from the payload's local file
+# header, at offset 28. A ZIP64 archive carries the extended information extra
+# field there; a ZIP32 one has none. od -tu1 rather than -tu2 because only GNU
+# od can be told the endianness.
+extra_field_len() {
+  local lo hi
+  read -r lo hi <<<"$(od -An -tu1 -j28 -N2 "$1")"
+  echo $((lo + hi * 256))
+}
+
+# Encrypting from a pipe no longer spools to disk, so the payload can no longer
+# be measured, so the archive has to be ZIP64 -- the choice is fixed before the
+# first segment goes out. A file is still measured and stays ZIP32. The layout
+# is what to assert on: both forms round-trip, so a quiet return to spooling
+# would show up nowhere else.
+@test "encrypt measures a file and streams a pipe" {
+  ./otdfctl encrypt -o "$TDF_OUT" $COMMON "$PLAIN"
+  [ "$(extra_field_len "$TDF_OUT")" -eq 0 ]
+
+  local piped_tdf="$BATS_TEST_TMPDIR/piped.tdf"
+  run bash -c "echo '$SECRET_TEXT' | ./otdfctl encrypt $COMMON >'$piped_tdf'"
+  assert_success
+  [ "$(extra_field_len "$piped_tdf")" -gt 0 ]
+
+  ./otdfctl decrypt -o "$RESULT" $COMMON "$piped_tdf"
+  run cat "$RESULT"
+  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/handlers/tdf.go

@@ -52,10 +52,11 @@ type EncryptOptions struct {
 // bounded by the SDK's segment size rather than by the payload length, so the
 // payload may be larger than RAM.
 //
-// in must be seekable: the SDK measures the payload by seeking to its end
-// before encrypting, and knowing the length up front is what lets it avoid
-// defaulting to ZIP64. A caller holding a pipe should spool it first.
-func (h Handler) Encrypt(ctx context.Context, out io.Writer, in io.ReadSeeker, o EncryptOptions) error {
+// in need not be seekable, but a seekable input produces a smaller TDF: the SDK
+// measures it by seeking to the end and can then keep the archive in the
+// compact ZIP32 layout. An unmeasurable payload is written as ZIP64, since the
+// choice is fixed before the first segment goes out.
+func (h Handler) Encrypt(ctx context.Context, out io.Writer, in io.Reader, o EncryptOptions) error {
 	switch o.TDFType {
 	// Encrypt the data as a ZTDF
 	case "", tdf.TypeTDF3, tdf.TypeZTDF:

spec/DSPX-4499.md

@@ -182,6 +182,11 @@ embedded markdown (`docs/man/*/_index.md`, read via `doc.GetDocFlag`), not in Go
   `GetTdfType` must seek too. Piped input is therefore spooled to a temp file on both
   sides, trading the whole-payload allocation for a disk write and a writable `TMPDIR`.
 
+  Superseded on the encrypt side once DSPX-2604 landed: `CreateTDF` takes an `io.Reader`,
+  so piped input goes to it directly. Seekability is now an optimization rather than a
+  requirement — a measurable payload stays ZIP32, a piped one is written as ZIP64. Decrypt
+  and inspect still spool, and always will until there is a streaming reader.
+
 - **Mime detection must not consume the input, and must not hide the `Seeker`.** The
   earlier revision wrapped input in `bufio.NewReaderSize(in, Size1MB+1)` and used `Peek`.
   That is wrong here: a `*bufio.Reader` is not an `io.Seeker`, so wrapping a file would
@@ -232,8 +237,9 @@ embedded markdown (`docs/man/*/_index.md`, read via `doc.GetDocFlag`), not in Go
 
 - Building a streaming/chunked **reader** in the SDK. That would let decrypt take a pipe
   without spooling, but it is a large SDK change well beyond this ticket's CLI-layer scope.
-- Relaxing `CreateTDF` to `io.Reader`, which would remove the encrypt-side spool. That is
-  DSPX-2604 and lands separately; the spool is ~10 lines to delete afterwards.
+- Relaxing `CreateTDF` to `io.Reader`, which removes the encrypt-side spool. That is
+  DSPX-2604 and lands separately; the spool is ~10 lines to delete afterwards. (Done, in
+  the commit that removed it — see the seekability note above.)
 - Removing the dead `--tdf-type`/`-t` flag on `decrypt`, documented as deprecated and never
   read by `decryptRun`. Noted, left alone.
 - Any change to `nanotdf` — no nanotdf implementation exists in this repo.
@@ -277,10 +283,11 @@ forced with an unresolvable attribute FQN and a KAS allowlist that excludes the
 As of this change, `streaming.bats` is the **only** e2e coverage of `encrypt`, `decrypt`
 and `inspect` that actually executes in CI.
 
-Ten cases: file→file, file→stdout, and fully-piped round-trips; decrypt-from-stdin
+Eleven cases: file→file, file→stdout, and fully-piped round-trips; decrypt-from-stdin
 (asserting the spool is removed); inspect from both a file and stdin; empty-stdin rejection
-for both commands; no-partial-output-on-failure for both commands; and a 1 GiB peak-RSS
-bound for both commands, which skips where GNU `time` is unavailable.
+for both commands; no-partial-output-on-failure for both commands; a 1 GiB peak-RSS bound
+for both commands, which skips where GNU `time` is unavailable; and, once the encrypt-side
+spool was removed, a ZIP32-vs-ZIP64 assertion that a file is measured and a pipe is not.
 
 ## Side effect worth knowing about