fix(cli): stream decrypt and inspect instead of buffering

otdfctl/cmd/tdf/decrypt.go

@@ -2,14 +2,15 @@ package tdf
 
 import (
 	"errors"
-	"fmt"
+	"io"
+	"os"
 
 	"github.com/opentdf/platform/lib/ocrypto"
 	"github.com/opentdf/platform/otdfctl/cmd/common"
 	"github.com/opentdf/platform/otdfctl/pkg/cli"
+	"github.com/opentdf/platform/otdfctl/pkg/handlers"
 	"github.com/opentdf/platform/otdfctl/pkg/man"
 	"github.com/opentdf/platform/otdfctl/pkg/streamio"
-	"github.com/opentdf/platform/otdfctl/pkg/utils"
 	"github.com/spf13/cobra"
 )
 
@@ -45,60 +46,61 @@ func decryptRun(cmd *cobra.Command, args []string) {
 		sessionKeyAlgorithm = ocrypto.RSA2048Key
 	}
 
-	// check for piped input
-	piped := readPipedStdin()
-
-	// Prefer file argument over piped input over default filename
-	bytesToDecrypt := piped
+	// Prefer the file argument over piped input.
 	var tdfFile string
-	var err error
 	if len(args) > 0 {
 		tdfFile = args[0]
-		bytesToDecrypt, err = utils.ReadBytesFromFile(tdfFile, MaxFileSize)
-		if err != nil {
-			cli.ExitWithError("Failed to read file:", err)
+	}
+	in, closeIn, err := streamio.OpenSeekable(tdfFile)
+	switch {
+	case errors.Is(err, streamio.ErrNoInput):
+		cli.ExitWithError("Must provide ONE of the following to decrypt: [file argument, stdin input]", err)
+	case err != nil:
+		cli.ExitWithError("Failed to read file:", err)
+	}
+	defer closeIn()
+
+	// cli.ExitWithError calls os.Exit, which skips deferred functions, so both
+	// the spooled input and the partial output have to be discarded first.
+	// Declared before the destination exists so every exit below can use it.
+	var outFile *streamio.OutputFile
+	fail := func(msg string, err error) {
+		if outFile != nil {
+			outFile.Cleanup()
 		}
+		closeIn()
+		cli.ExitWithError(msg, err)
 	}
 
-	if len(bytesToDecrypt) == 0 {
-		cli.ExitWithError("Must provide ONE of the following to decrypt: [file argument, stdin input]", errors.New("no input provided"))
+	// Resolve the destination before decrypting, so the plaintext streams
+	// straight to it rather than accumulating in memory first.
+	var dest io.Writer = os.Stdout
+	if output != "" {
+		outFile, err = streamio.NewOutputFile(output, decryptedOutputFileMode)
+		if err != nil {
+			fail("Failed to write decrypted data to file", err)
+		}
+		defer outFile.Cleanup()
+		dest = outFile
 	}
 
 	ignoreAllowlist := len(kasAllowList) == 1 && kasAllowList[0] == "*"
 
-	decrypted, err := h.DecryptBytes(
-		c.Context(),
-		bytesToDecrypt,
-		assertionVerification,
-		disableAssertionVerification,
-		sessionKeyAlgorithm,
-		kasAllowList,
-		ignoreAllowlist,
-		nil,
-	)
+	err = h.Decrypt(c.Context(), dest, in, handlers.DecryptOptions{
+		AssertionVerificationKeysFile: assertionVerification,
+		DisableAssertionCheck:         disableAssertionVerification,
+		SessionKeyAlgorithm:           sessionKeyAlgorithm,
+		KASAllowList:                  kasAllowList,
+		IgnoreAllowlist:               ignoreAllowlist,
+	})
 	if err != nil {
-		cli.ExitWithError("Failed to decrypt file", err)
+		fail("Failed to decrypt file", err)
 	}
 
-	if output == "" {
-		//nolint:forbidigo // printing decrypted content to stdout
-		fmt.Print(decrypted.String())
-		return
-	}
-	// Here 'output' is the filename given with -o
-	f, err := streamio.NewOutputFile(output, decryptedOutputFileMode)
-	if err != nil {
-		cli.ExitWithError("Failed to write decrypted data to file", err)
-	}
-	defer f.Cleanup()
-	_, err = f.Write(decrypted.Bytes())
-	if err != nil {
-		f.Cleanup()
-		cli.ExitWithError("Failed to write decrypted data to file", err)
-	}
-	if err := f.Commit(); err != nil {
-		f.Cleanup()
-		cli.ExitWithError("Failed to write decrypted data to file", err)
+	if outFile != nil {
+		if err := outFile.Commit(); err != nil {
+			fail("Failed to write decrypted data to file", err)
+		}
 	}
 }
 

otdfctl/cmd/tdf/tdf.go

@@ -1,39 +1,8 @@
 package tdf
 
-import (
-	"io"
-	"os"
-
-	"github.com/opentdf/platform/otdfctl/pkg/cli"
-	"github.com/opentdf/platform/otdfctl/pkg/streamio"
-)
-
 const (
-	Size1MB     = 1024 * 1024
-	MaxFileSize = int64(10 * 1024 * 1024 * 1024) // 10 GB
-	TDF         = "TDF"
+	Size1MB = 1024 * 1024
+	TDF     = "TDF"
 	// GroupID is the group ID for TDF commands
 	GroupID = TDF
 )
-
-// readPipedStdin returns the whole of piped stdin, or nil when stdin is a
-// terminal or an empty redirect.
-//
-// Detection is delegated to streamio.PipeReader so there is a single answer to
-// "is there piped input?" across the CLI. The read itself is still unbounded;
-// callers that must not hold the payload in memory should use
-// streamio.OpenSeekable instead.
-func readPipedStdin() []byte {
-	r, ok, err := streamio.PipeReader(os.Stdin)
-	if err != nil {
-		cli.ExitWithError("failed to scan bytes from stdin", err)
-	}
-	if !ok {
-		return nil
-	}
-	buf, err := io.ReadAll(r)
-	if err != nil {
-		cli.ExitWithError("failed to scan bytes from stdin", err)
-	}
-	return buf
-}

otdfctl/e2e/action.yaml

@@ -34,7 +34,11 @@ runs:
       shell: bash
       run: |
         sudo apt-get update
-        sudo apt-get install -y gnome-keyring
+        # 'time' is GNU time, which reports 'Maximum resident set size'. The
+        # streaming suite's peak-RSS case needs it and would otherwise skip
+        # itself silently -- and that case is the only proof that encrypt and
+        # decrypt have not gone back to buffering the whole payload.
+        sudo apt-get install -y gnome-keyring time
       working-directory: otdfctl
     - name: Setup Bats and bats libs
       uses: bats-core/bats-action@2.0.0
@@ -54,12 +58,23 @@ runs:
         # suite while other files still create unnamespaced policy fixtures.
         bats --tap e2e --filter-tags namespaced_policy_migration | tee e2e/bats-results.tap
 
+        # Then every file that encrypts without attributes, also on its own.
+        # Unattributed encrypts fall back to the platform base key, and
+        # key-base.bats sets one pointing at a KAS that does not resolve and
+        # cannot unset it afterwards -- a base key can be replaced but not
+        # cleared. Anything unattributed scheduled after that file produces an
+        # undecryptable TDF, so these have to run first rather than race for a
+        # slot. Running alone also keeps streaming.bats' 1 GiB peak-RSS case
+        # from measuring itself against three neighbours competing for the same
+        # memory.
+        bats --tap e2e --filter-tags unattributed_encrypt | tee -a e2e/bats-results.tap
+
         if command -v parallel >/dev/null 2>&1; then
           echo "GNU parallel found, running remaining tests in parallel"
-          bats --tap e2e --filter-tags '!namespaced_policy_migration' --jobs 4 --no-parallelize-within-files --no-tempdir-cleanup | tee -a e2e/bats-results.tap
+          bats --tap e2e --filter-tags '!namespaced_policy_migration,!unattributed_encrypt' --jobs 4 --no-parallelize-within-files --no-tempdir-cleanup | tee -a e2e/bats-results.tap
         else
           echo "GNU parallel not found, running remaining tests sequentially"
-          bats --tap e2e --filter-tags '!namespaced_policy_migration' | tee -a e2e/bats-results.tap
+          bats --tap e2e --filter-tags '!namespaced_policy_migration,!unattributed_encrypt' | tee -a e2e/bats-results.tap
         fi
       env:
         # Define 'bats' install location in ubuntu

otdfctl/e2e/encrypt-decrypt.bats

@@ -1,6 +1,15 @@
 #!/usr/bin/env bats
 
+# bats file_tags=unattributed_encrypt
+
 # Tests for encrypt decrypt
+#
+# Tagged so action.yaml runs this file before the parallel batch. Several cases
+# here encrypt with no attributes, which falls back to the platform base key,
+# and key-base.bats sets one pointing at a KAS that does not resolve and cannot
+# unset it afterwards -- a base key can be replaced but not cleared. Scheduled
+# into the parallel batch this file would eventually land after key-base.bats
+# and fail on an undecryptable TDF. See the header of streaming.bats.
 
 setup_file() {
   export CREDSFILE=creds.json