Bound stdin (and the --file twin) at 1 MiB

internal/commands/notes.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"strings"
 
@@ -196,11 +197,11 @@ func notesContent(cmd *cobra.Command, args []string, file string) (string, error
 		}
 		return notesRequireContent(content)
 	case file != "":
-		data, err := os.ReadFile(file)
+		content, err := notesFileContent(file)
 		if err != nil {
-			return "", output.ErrUsage(fmt.Sprintf("failed to read %s: %v", file, err))
+			return "", err
 		}
-		return notesRequireContent(string(data))
+		return notesRequireContent(content)
 	case positional != "":
 		content, err := resolveContentValue(cmd, positional, 0, "[content]")
 		if err != nil {
@@ -215,6 +216,25 @@ func notesContent(cmd *cobra.Command, args []string, file string) (string, error
 	)
 }
 
+// notesFileContent reads --file under the same cap as --file -, so which side
+// of the "-" the bytes arrive on never changes whether they are accepted.
+func notesFileContent(file string) (string, error) {
+	f, err := os.Open(file)
+	if err != nil {
+		return "", output.ErrUsage(fmt.Sprintf("failed to read %s: %v", file, err))
+	}
+	defer f.Close()
+
+	data, err := io.ReadAll(io.LimitReader(f, maxStdinContent+1))
+	if err != nil {
+		return "", output.ErrUsage(fmt.Sprintf("failed to read %s: %v", file, err))
+	}
+	if len(data) > maxStdinContent {
+		return "", output.ErrUsage(fmt.Sprintf("--file %s exceeds %d bytes", file, maxStdinContent))
+	}
+	return string(data), nil
+}
+
 // notesRequireContent refuses to blank the note by accident.
 //
 // set replaces everything, so an empty file or an empty pipe would silently

internal/commands/notes_test.go

@@ -124,6 +124,26 @@ func TestNotesSetReadsFromAFile(t *testing.T) {
 	assert.NotContains(t, body.Note.Content, "# Heading", "raw Markdown must not reach the wire")
 }
 
+// --file and --file - are the same read, so they agree on the boundary: a file
+// of exactly maxStdinContent is content, one byte past it is a usage error.
+func TestNotesSetBoundsFileAtTheStdinCap(t *testing.T) {
+	dir := t.TempDir()
+	atCap := filepath.Join(dir, "at-cap.md")
+	require.NoError(t, os.WriteFile(atCap, []byte(strings.Repeat("a", maxStdinContent)), 0o600))
+	overCap := filepath.Join(dir, "over-cap.md")
+	require.NoError(t, os.WriteFile(overCap, []byte(strings.Repeat("a", maxStdinContent+1)), 0o600))
+
+	app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute())
+	require.NoError(t, executeRecordingCommand(NewNotesCmd(), app, "set", "--file", atCap))
+	assert.NotEmpty(t, transport.recorded())
+
+	app, transport, _ = setupPersonalFeedApp(t, notesUpdateRoute())
+	err := executeRecordingCommand(NewNotesCmd(), app, "set", "--file", overCap)
+	outErr := requireBookmarksUsageError(t, err)
+	assert.Contains(t, outErr.Message, "exceeds 1048576 bytes")
+	assert.Empty(t, transport.recorded(), "a rejected write must not reach the server")
+}
+
 func TestNotesSetReadsDashFromStdin(t *testing.T) {
 	app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute())
 

internal/commands/stdin.go

@@ -37,6 +37,13 @@ func allowDash(cmd *cobra.Command, tokens ...string) {
 	cmd.Annotations[stdinarg.AnnotationAllowDash] = merged
 }
 
+// maxStdinContent caps how much a "-" placeholder — and the --file twin that
+// shares its refusals — will read. It matches maxAgentHookInput by coincidence
+// of scale, not by kinship: that one bounds a JSON envelope an agent harness
+// writes, this one bounds prose a person pipes, and the two should stay free
+// to move apart.
+const maxStdinContent = 1 << 20
+
 // readStdinContent reads content for a "-" placeholder from piped stdin.
 //
 // Nothing piped (a TTY) is a usage error rather than a silent read: waiting on
@@ -45,6 +52,15 @@ func allowDash(cmd *cobra.Command, tokens ...string) {
 // never an intentional write, and for update-style commands it would be an
 // implicit clear.
 //
+// A stream over maxStdinContent is refused as well, and refused rather than
+// truncated: `yes | basecamp api post /valid --data -` is a perfectly valid
+// invocation that would otherwise read until the process dies, and silently
+// posting the first megabyte would write partial content to Basecamp and
+// report success — unrecoverable once saved. The cap counts bytes read, so it
+// lands before the trim below rather than on the trimmed result: telling an
+// overflow that is only a trailing newline from any other would mean reading
+// past the cap, which is the thing being prevented.
+//
 // Trailing newlines — LF and CRLF alike — are trimmed: Markdown bodies don't
 // care, but titles and boosts (16-rune limit) do, and virtually every pipe
 // ends with one. Interior line breaks are untouched.
@@ -55,10 +71,13 @@ func readStdinContent(cmd *cobra.Command, what string) (string, error) {
 			stdinEscapeHint(cmd, what),
 		)
 	}
-	data, err := io.ReadAll(cmd.InOrStdin())
+	data, err := io.ReadAll(io.LimitReader(cmd.InOrStdin(), maxStdinContent+1))
 	if err != nil {
 		return "", output.ErrUsage(fmt.Sprintf("failed to read %s from stdin: %v", what, err))
 	}
+	if len(data) > maxStdinContent {
+		return "", output.ErrUsage(fmt.Sprintf("stdin for %s exceeds %d bytes", what, maxStdinContent))
+	}
 	content := strings.TrimRight(string(data), "\r\n")
 	if strings.TrimSpace(content) == "" {
 		return "", output.ErrUsage(fmt.Sprintf("stdin for %s is empty", what))

internal/commands/stdin_test.go

@@ -95,6 +95,39 @@ func TestReadStdinContentBlankPipeIsUsageError(t *testing.T) {
 	assert.Contains(t, outErr.Message, "empty")
 }
 
+// The cap is a read bound, so it is checked against the bytes read rather than
+// the trimmed content: exactly maxStdinContent is content, one byte past it is
+// a refusal — including when that byte is the trailing newline every pipe ends
+// with, which is the only way to stop reading at the cap at all.
+func TestReadStdinContentAtTheCapIsAccepted(t *testing.T) {
+	body := strings.Repeat("a", maxStdinContent)
+	cmd := &cobra.Command{Use: "x"}
+	cmd.SetIn(strings.NewReader(body))
+
+	content, err := readStdinContent(cmd, "<content>")
+	require.NoError(t, err)
+	assert.Equal(t, body, content)
+}
+
+func TestReadStdinContentOverTheCapIsUsageError(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		body string
+	}{
+		{"one byte over", strings.Repeat("a", maxStdinContent+1)},
+		{"the cap plus a trailing newline", strings.Repeat("a", maxStdinContent) + "\n"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cmd := &cobra.Command{Use: "x"}
+			cmd.SetIn(strings.NewReader(tc.body))
+
+			_, err := readStdinContent(cmd, "<content>")
+			outErr := requireUsageErr(t, err)
+			assert.Contains(t, outErr.Message, "exceeds 1048576 bytes")
+		})
+	}
+}
+
 func TestResolveContentArgJoinsLiteralArgs(t *testing.T) {
 	cmd := &cobra.Command{Use: "x"}
 	content, err := resolveContentArg(cmd, []string{"hello", "world"}, 1)