Skip to content

fix(relay): raise inbound MaxLineLength from 2000 to 128KiB, add rejected_line_too_long metric - #773

Merged
jiashuoz merged 3 commits into
mainfrom
fix/relay-max-line-length
Aug 1, 2026
Merged

fix(relay): raise inbound MaxLineLength from 2000 to 128KiB, add rejected_line_too_long metric#773
jiashuoz merged 3 commits into
mainfrom
fix/relay-max-line-length

Conversation

@jiashuoz

@jiashuoz jiashuoz commented Jul 31, 2026

Copy link
Copy Markdown
Member

What

Raises the inbound relay's MaxLineLength from go-smtp's 2000-byte default to 128 KiB, records a new rejected_line_too_long SMTP intake metric when a line still exceeds it, and adds regression tests for both.

Why

Discovered while investigating a customer's 18% bounce rate on the new delivery-health dashboard: all 30 bounces were 554 5.0.0 Error: transaction failed: smtp: too long a line in input streamour own relay rejecting agent-to-agent mail at DATA. The customer's two agents (bitswarm-drone/gs@agents.e2a.dev, prod, Jul 18–20) were messaging each other with payloads containing a single >2000-char line — single-line JSON / unfolded HTML, which agent-generated mail produces constantly. They silently retried for three days before reformatting.

Review notes (why 128 KiB, not the originally proposed 1 MiB):

  • MaxLineLength bounds all lines, including pre-auth SMTP command lines, so it is memory an unauthenticated peer can force per connection (~1.37 MiB live heap/connection measured at 1 MiB; nothing bounds concurrent connections). 128 KiB cuts that exposure 8× while still being 64× the old default — comfortably covering single-line JSON and unfolded HTML.
  • It must stay at or below the 1 MiB header-scan buffer in internal/emailauth/checker.go, or the DKIM l= guard there fails open; the constant's comment now records that coupling.
  • This does not cover unwrapped-base64 attachments (an earlier draft of this PR overclaimed that): a multi-MB attachment base64'd onto one line is still a multi-MB line and bounces identically. fix(outbound): declare Content-Transfer-Encoding on composed bodies #745 (merged) already routes any e2a-composed body with a >998-octet line through quoted-printable, so the composer can no longer produce this failure — the residual value here is inbound mail from third-party MTAs.
  • The original incident surfaced as a customer complaint rather than an alert because the too-long-line rejection recorded no metric: the io.ReadAll error path in session.Data returned before any recordSMTPInbound call. It now records rejected_line_too_long (via errors.Is against go-smtp's ErrTooLongLine sentinel); other DATA read errors (client drop, MaxMessageBytes) deliberately stay unrecorded as before.

Changes

  • internal/relay/server.gosmtpSrv.MaxLineLength = 1 << 17 (128 KiB) with a comment recording the prod incident, the command-line/memory bound, the emailauth 1 MiB coupling, and the fix(outbound): declare Content-Transfer-Encoding on composed bodies #745 relationship; Data() records rejected_line_too_long on ErrTooLongLine
  • internal/telemetry/prom.go / metrics.go — new outcome in the smtpSet enum allowlist (would otherwise collapse to other); duration histogram keeps its accepted/dedup/tempfail whitelist so a mid-read abort can't distort the 2s latency SLO
  • docs/observability.mde2a_smtp_inbound_total outcome table updated
  • internal/relay/inbound_longline_test.goTestRelay_Data_AcceptsLongLines: delivers a message with a single 64KiB body line through a real socket + test DB and asserts it lands in the agent inbox
  • internal/relay/metrics_test.goTestSMTPInboundMetric_RejectedLineTooLong: a single 160KiB line gets the 554 and records exactly one rejected_line_too_long observation

Verification

  • TestRelay_Data_AcceptsLongLines fails at the old 2000-byte default and passes at 128 KiB (verified both directions locally against the test DB)
  • Full internal/relay + internal/telemetry suites green; go build ./..., go vet, gofmt clean

🤖 Generated with Kimi Code — review findings applied with Claude Code

jiashuoz and others added 3 commits July 31, 2026 00:45
go-smtp v0.24.0 defaults MaxLineLength to 2000 bytes, and the relay
never overrode it. Agent-generated mail — single-line JSON, unfolded
HTML, unwrapped base64 — routinely exceeds that, and the relay
rejected it at DATA with "554 5.0.0 ... too long a line in input
stream". Prod evidence (2026-07-18..20): a customer's two agents
(bitswarm-drone/gs@agents.e2a.dev) bounced 30 agent-to-agent messages
on exactly this before the customer reformatted their payload. Their
use case is the product's core use case.

The whole message is already capped by MaxMessageBytes (10MB), so a
1MiB per-line cap bounds per-connection memory without rejecting
realistic agent payloads.

Adds TestRelay_Data_AcceptsLongLines: delivers a message with a single
64KiB body line through a real socket + DB and asserts it lands in the
agent inbox. Verified the test FAILS at the old 2000-byte default and
passes with the new cap; full internal/relay suite green.

Co-Authored-By: Kimi <noreply@moonshot.ai>
…ne rejects

Review findings on the 1MiB cap: MaxLineLength bounds ALL lines including
pre-auth SMTP command lines, so it is per-connection memory an
unauthenticated peer can force (~1.37MiB live heap per connection at
1MiB, measured); nothing bounds concurrent connections. 128KiB still
covers single-line JSON / unfolded HTML with wide margin (64x the old
default), stays below the 1MiB header-scan buffer in
internal/emailauth/checker.go that the DKIM l= guard depends on, and
passes the existing regression test unchanged.

Also record a rejected_line_too_long SMTPInbound outcome on the DATA
read-error path (errors.Is against go-smtp's ErrTooLongLine sentinel) —
the original incident surfaced as a customer complaint because this
rejection recorded no metric. Other read errors (client drop, size cap)
stay unrecorded as before. Comment no longer claims coverage of
unwrapped base64 (a multi-MB attachment line still bounces) and notes
#745 already fixed the composer-side source, so this cap guards inbound
mail from third-party MTAs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e set

Without the prom.go allowlist entry the new outcome would collapse to
the "other" label. The duration histogram deliberately keeps its
accepted/accepted_dedup/tempfail whitelist — a mid-read abort's elapsed
time is not a processing latency and would distort the 2s SLO. Adds the
per-outcome metric regression test (554 + exactly one observation) and
updates docs/observability.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jiashuoz jiashuoz changed the title fix(relay): raise inbound MaxLineLength from 2000 to 1MiB fix(relay): raise inbound MaxLineLength from 2000 to 128KiB, add rejected_line_too_long metric Aug 1, 2026
@jiashuoz
jiashuoz merged commit f2ea24d into main Aug 1, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant