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

[jeremy]

What

Caps readStdinContent at maxStdinContent = 1 << 20, using the
io.LimitReader(r, cap+1)-then-compare idiom already in this package
(agent_hook.go:238, upgrade_selfupdate.go:339/:359). All 36 --accepting
call sites funnel through it, so they all inherit the bound.

notes set --file <path> was the file twin of the same read — a bare
os.ReadFile — so it gets the same bound and the same error shape via a small
notesFileContent helper. Which side of the - the bytes arrive on no longer
changes whether they're accepted.

Why

readStdinContent was a bare io.ReadAll with no cap.
yes | basecamp api post /valid --data - is a perfectly valid invocation
that read until the process died. That's why the recent pre-read ordering work
(#641, #645) didn't touch it: nothing here is a doomed invocation to reject
early — the read itself was unbounded.

The sharpest illustration: boost create - read the entire stream, copied it to
a string, and then rejected it for exceeding 16 characters
(boost.go:307). It now stops at 1 MiB before the 16-rune check.

Refuse, don't truncate. Silently posting the first megabyte would write
partial content to Basecamp and report success — unrecoverable once saved.

maxStdinContent is declared alongside the stdin machinery rather than reusing
maxAgentHookInput. Same value today, different purpose — one bounds a JSON
envelope an agent harness writes, the other bounds prose a person pipes — and
they should stay free to move apart.

Left alone deliberately: internal/editor/editor.go:49 and the TUI composer at
widget/composer.go:719. Both read back what the user's own $EDITOR just
wrote, which is not a streaming source.

The trailing-newline edge

The cap counts bytes read, so it lands before the trailing-newline trim: a
body of exactly 1 MiB plus a trailing \n is a refusal. That's the only
implementable answer — telling "the overflow is just a newline" apart from any
other overflow requires reading past the cap, which is the thing being
prevented. Documented in the doc comment and pinned by a subtest so a future
reader doesn't "fix" it.

Testing

• make check passes (via bin/ci), including the AST ordering backstop
  TestSyntacticArgChecksPrecedeStdinReads

Boundary pairs, not just rejection cases — stdin_test.go asserts exactly
maxStdinContent is accepted and one byte over is a usage error;
notes_test.go does the same for --file (the file helper's +1/compare is
written separately and could be off by one in the accept direction). The
boundary was verified by mutation: bumping the const to 1<<20 + 1 fails
all three assertions, so an off-by-one in either direction is caught.

Smoke (against a dead endpoint, so a proceeding read fails at the network and a
capped read fails before it):

1048576 (at cap)   → proceeds to the request  exit=6
1048577 (over cap) → "stdin for <content> exceeds 1048576 bytes"  exit=1
printf 'hello'     → unaffected, proceeds     exit=6
notes set --file <2 MiB>  → "--file /…/big.md exceeds 1048576 bytes"  exit=1
boost create 999 - (over) → "stdin for <content> exceeds 1048576 bytes"  exit=1
yes | api post --data -   → "stdin for --data exceeds 1048576 bytes", terminates <1s

The --file message names the flag and path rather than mirroring
stdin for %s verbatim — parallel shape, but it says what the caller can act
on.

---

Summary by cubic

Bound stdin and notes --file reads at 1 MiB to prevent unbounded reads and make file vs stdin behave the same. Previously we read all input; now we refuse inputs over the cap rather than truncating.

• All - placeholders now go through capped readStdinContent; on overflow we return a usage error: "stdin for exceeds 1048576 bytes".
• notes set --file <path> now reads via notesFileContent with the same cap; overflow error: "--file exceeds 1048576 bytes".
• The cap applies before trimming the trailing newline. Exactly 1 MiB is accepted; 1 MiB plus newline is refused.
• No changes to internal/editor/editor.go or the TUI composer; they read back from $EDITOR, not a stream.
• Tests cover the acceptance/refusal boundary in internal/commands/stdin_test.go and internal/commands/notes_test.go.

Migration: If you pipe more than 1 MiB, the command now fails with a usage error. Reduce the input size or split content before invoking the command.

^{Written for commit 35ee365. Summary will update on new commits.}

[Copilot]

Pull request overview

Bounds stdin and notes set --file input at 1 MiB to prevent unbounded memory consumption.

Changes:

• Rejects stdin exceeding 1 MiB without truncation.
• Applies the same limit to note files.
• Adds boundary and overflow tests.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
internal/commands/stdin.go	Adds the shared stdin limit and overflow error.
internal/commands/stdin_test.go	Tests exact-limit and overflow behavior.
internal/commands/notes.go	Adds bounded file reading for notes.
internal/commands/notes_test.go	Tests note-file size boundaries.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.