Skip to content

feat(outbound): require single-use provider authorization (B5) - #998

Merged
jiashuoz merged 5 commits into
mainfrom
feat/sending-provider-submit
Sep 5, 2026
Merged

feat(outbound): require single-use provider authorization (B5)#998
jiashuoz merged 5 commits into
mainfrom
feat/sending-provider-submit

Conversation

@jiashuoz

@jiashuoz jiashuoz commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What

Slice B5 of the sending abuse prevention plan (Task 5: token-requiring SMTP adapter).

  • outbound.ProviderSubmitter — the provider seam. SubmitOnce(ctx, auth, Envelope) requires a sendingpolicy.ProviderAuthorization, validates the envelope against it, derives X-E2A-Provider-Attempt, X-SES-TENANT, and the stable X-E2A-Message-ID only from the token (plus X-SES-CONFIGURATION-SET from config), strips every occurrence of those headers from the composed MIME (mixed case, duplicates, folded continuations), redeems the token, then dials. RCPT TO is issued from the token's canonical envelope. Every early exit is I/O-free and leaves the token spendable; an unconfigured relay is refused before redemption so retries do not burn ordinals.
  • Settlement — a definite 5xx settles as permanently rejected; an acceptance settles with the SES message id. ProviderSettlement gains ProviderMessageID, which SettleProvider binds to the attempt's sending_feedback_correlations row exactly once, normalized to SES's bare form (NormalizeProviderMessageID) so the worker and the future feedback finalizer agree on one spelling. A different id returns ErrProviderMessageIDConflict. Settlement now also requires the attempt to have actually dialed (call_state = 'started'). A settlement that fails after acceptance is reported on ProviderResult.SettlementErr, never as a send error, so callers retry settlement and never resubmit.
  • Gate hardening — RedeemProviderCall re-proves the abuse pause immediately before the socket opens; a tenant name that cannot be a header value holds at mint with a visible reason instead of wedging in the adapter.
  • Relay — a failure after the terminating dot is marked ErrProviderAcceptanceUnknown; no classifier calls it permanent and the seam leaves it unsettled.
  • internal/testutil/testdb — the DB helper extracted into a leaf package. testutil bundles the contract server, which imports outbound; once outbound imports sendingpolicy, any internal test file using testutil.TestDB in either package is an import cycle. testutil.TestDB and friends remain as forwarding wrappers; no existing caller changes.

What this does NOT do

  • Wire any worker to the new submitter (B6). Sender.SubmitOnce and the raw relay methods remain, unchanged, for the current path and the notification callers.
  • The AST closure guard that rejects tokenless socket calls (Task 7).
  • Bind MAIL FROM to the token. The authorization carries recipients and tenant, not the sender; binding it needs the gate to learn the composed sender at acceptance (a gate change, tracked as follow-up). SES enforces sender-identity ownership on its side.
  • Make sending_feedback_correlations.provider_message_id UNIQUE (one-attempt-per-id). Follow-up migration.

Prod behaviour is unchanged: nothing calls ProviderSubmitter yet.

Review round 1

Two parallel reviews (correctness + adversarial). Findings addressed in fix(outbound): harden the provider seam after review — each has a named test and was mutation-checked: pause between consume and submit; settle without redeem; provider-id shape mismatch (qualified vs bare); caller-supplied message id; RCPT from the caller's spelling; bare-CR / \r\r\n pseudo-separator and leading-continuation header smuggling; racy socket counter with no positive control; ambiguous outcome and accepted-but-unsettled shapes untested; lost-250 unclassifiable; CRLF tenant name mintable via DB edit; header order vs the legacy path.

Deferred with rationale above: MAIL FROM binding, UNIQUE provider id index.

Review round 2

Mutation-tested re-review of the fix commit: no blockers, three should-fixes, all addressed in fix(outbound): keep the acceptance-unknown marker honest with named tests that fail under mutation — the marker now survives the relay's context remap (deadline/cancel after the dot), is attached only when no reply code came back (a post-DATA 554 stays permanent), and the customer-only guard on the redeem-time pause re-check is pinned by the reviewer's own test (a pause notice to a paused owner must still redeem). The compare-side id normalization also gained a direct test.

Notes for B6: a pause caught at redemption surfaces as ErrAuthorizationInvalid (the next ConsumeAttempt returns account_paused), so the worker must not emit email.failed on it; and ErrProviderAcceptanceUnknown is the signal to branch on for "maybe sent".

Gate for the next slice

Per the plan's B5 row: main worker has not switched; old caller path remains available only inside the package.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX

jiashuoz and others added 3 commits September 4, 2026 20:17
@jiashuoz @claude
test(testutil): extract the database helper into a leaf package
903e0e3 
testutil bundles the contract server, which imports outbound. Once
outbound imports sendingpolicy (the provider seam requires the gate's
token), any INTERNAL test file in outbound or sendingpolicy that calls
testutil.TestDB closes an import cycle.

Move db.go into internal/testutil/testdb, a leaf that depends only on
identity and migrations, and keep testutil.TestDB / TestDBURL /
OpenPreparedTestDB / TruncateAll as forwarding wrappers so every
existing caller is unchanged. The contract server and its River test
use the two newly exported helpers (Truncate, BaseTestDBURL) instead of
package-private ones. The one internal sendingpolicy test imports
testdb directly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
@jiashuoz @claude
feat(outbound): require single-use provider authorization
ec366b3 
Add ProviderSubmitter, the one path from a composed message to an SMTP
transaction with SES. It requires a sendingpolicy.ProviderAuthorization
for every call, proves the envelope is the authorized one, derives
X-E2A-Provider-Attempt and X-SES-TENANT only from the token (and
X-SES-CONFIGURATION-SET from configuration), strips every occurrence of
those headers — mixed case and folded forms — from the composed MIME,
redeems the token, and only then dials. Every early exit is I/O-free
and leaves the token intact; a misconfigured relay is refused before
redemption so a retry does not burn an ordinal.

A definite permanent rejection is settled as such; an acceptance is
settled with the provider's message id, which SettleProvider now binds
to the attempt's feedback correlation exactly once (same id replays,
a different id is refused). A settlement that fails after acceptance
is reported on the result, never as a send error, so the caller retries
settlement rather than resubmitting.

The legacy tokenless Sender.SubmitOnce path stays for now; Task 7
migrates its callers and makes the relay's socket-opening methods
package-private.

Tests: zero-network on missing, mismatched, stale, and reused tokens
(asserted against a socket counter); exact and single attempt/tenant
headers with smuggled spellings removed and the body untouched; a
physical retry redeems a distinct ordinal; provider id binding and its
conflict rule. Removing the header strip, reordering redemption before
envelope validation, or dropping the id binding each fail a test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
@jiashuoz @claude
fix(outbound): harden the provider seam after review
1e9e8a3 
Two review passes (correctness + adversarial) over the first cut. Every
finding below has a named test, and removing each fix fails it.

Gate:
- RedeemProviderCall re-proves the abuse pause. ConsumeAttempt linearized
  it, but that transaction has committed; a pause landing before the dial
  now invalidates the token. Unlocked read, because the account-control
  key precedes the operation key this transaction already holds.
- SettleProvider requires call_state = 'started', not just a confirmed
  reservation: a provider id bound to an attempt that never dialed is a
  ledger claim about a send that did not happen.
- The provider message id is normalized to SES's bare form on bind and on
  compare (NormalizeProviderMessageID). The relay returns it qualified
  (<id@region.amazonses.com>); SNS feedback carries it bare; the worker
  and the feedback finalizer must not refuse each other's spelling.
- A tenant name that cannot be a header value holds at mint with
  ses_tenant_unnamed instead of wedging silently in the adapter.

Adapter:
- X-E2A-Message-ID is derived from the token (a customer operation IS its
  message id), not trusted from the caller; Envelope.MessageID is gone.
- RCPT TO is issued from AuthorizedRecipients(), the canonical envelope
  the budget priced, never the caller's spelling of it.
- A bare CR anywhere in the header section, or a leading continuation
  line, is refused before redemption (ErrMalformedHeaderSection): a
  receiver that treats a lone CR as a line break would see a header this
  walker did not.
- Provider headers are emitted in the legacy path's order (configuration
  set first) so the worker swap is byte-identical for shared headers.
- The relay marks a failure after the terminating dot with
  ErrProviderAcceptanceUnknown; the adapter leaves it unsettled and no
  classifier calls it permanent.

Tests: pause between consume and submit; settle without redeem; bare CR
and leading continuation refused with zero sockets; canonical RCPT on the
wire; lost 250 unsettled and marked; 4xx unsettled; accepted-but-unsettled
reported on the result with a nil error; a positive control proving the
socket counter observes a dial; provider id normalization across the three
spellings. The socket counter now waits for the accept goroutine.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
jiashuoz added a commit that referenced this pull request Sep 5, 2026
@jiashuoz @claude
test(sendingpolicy): redeem before settling in the ramp tests
78ab6db 
Settlement reports what the provider did, so it is only meaningful for an
attempt that opened the socket. The adapter (B5, #998) redeems the token
immediately before it dials, and the gate there refuses to settle an
attempt whose call_state is not 'started'. These tests settled straight
after ConsumeAttempt; route them through a consumeAndRedeem helper so the
two slices can land in either order.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
@jiashuoz @claude
fix(outbound): keep the acceptance-unknown marker honest
615aae6 
Re-review of the previous fix commit found the marker leaking in both
directions. The relay's deferred context remap replaced the error
wholesale, so a deadline or cancellation after the terminating dot — the
likeliest way to lose a 250 — dropped ErrProviderAcceptanceUnknown and
classified as a connection outage the worker would re-drive. And the
marker was joined to every post-DATA failure, including a coded 554
content rejection, which is the provider's definite answer and must
classify permanent. Now the marker survives the remap and is attached
only when no reply code came back.

Also pins the customer-only guard on the redeem-time pause re-check: the
notice telling an account it was paused is sourced from that paused
account and must still go out. Reviewer-authored test adopted. And the
compare-side normalization of the provider id now has a test that writes
a qualified spelling directly and replays bare.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
jiashuoz added a commit that referenced this pull request Sep 5, 2026
@jiashuoz @claude
feat(sending): compose ramp with protection gate (#992)
30089b9 
* feat(sending): compose ramp with protection gate

B4 of the sending-protection plan. The custom-domain ramp and the sending
budget answer different questions — "has this domain earned this volume
yet?" versus "has this account or the platform exposed SES enough today?"
— and this slice makes one transaction answer both, most restrictive
winning.

One lock order for the ramp. The store previously used three: Reserve
took domain → scope → reservation → counter, Confirm took reservation →
counter → scope, Release took reservation → counter. Three orders over
four keys is a deadlock waiting for traffic, and it becomes unavoidable
once the gate composes both ledgers into one transaction, because that
transaction already holds highly contended global budget counters when it
reaches the ramp. internal/sendramp/tx.go now holds the single order
(domain identity → registrable-domain scope → message reservation → UTC
day counter) and the pool-owning methods are thin wrappers, so there is
no second implementation to drift.

Composition, in internal/sendingpolicy/ramp.go:

- Probation is now the ramp's answer rather than a stand-in. Shared-relay
  traffic stays probationary at every plan level and never graduates; a
  custom domain is probationary until its scope has one qualified day.
  That classification decides which budget counters the transaction must
  lock, and the budget counters come BEFORE the ramp keys in the
  normative order — so it is read unlocked. That is sound only because
  ramp progress is monotonic: a stale answer can be stale in the strict
  direction and no other.
- The ramp is authorized last, after the budget has been reacquired. A
  ramp hold therefore arrives with budget units already taken, and those
  are released before returning the hold — keeping them would charge an
  account for a send its own domain was not allowed to make.
- SettleProvider now moves the ramp: acceptance advances a qualified day,
  a definite permanent rejection releases the units, and retryable or
  ambiguous results leave the reservation standing, because a message
  that might have been delivered must not release capacity.
- CancelAttempt releases both ledgers; DeferAttempt still releases only
  the budget. A rate deferral was not rejected by anyone, and giving back
  its ramp claim would let the same message re-qualify a stage it already
  qualified.

Disabled mode is genuinely pass-through: no scope row, no counter, and
above all no `exempt` write. Writing `exempt` while the ramp is off would
permanently grandfather every domain that happened to send during the
disabled window, and the phase-3 activation would then find nothing left
to ramp. Production ships in exactly this state.

A fixture note worth keeping: the ramp ledger is keyed by REGISTRABLE
domain, so `ramp-1.example.test` and `ramp-2.example.test` are one scope.
The first version of these tests read the hostname key, found empty rows,
and would have passed for the wrong reason. Each fixture domain is now
its own eTLD+1, and the one test that is about sharing builds two
hostnames under a single registrable domain deliberately.

Tests: 264 in the package plus the existing ramp suite, green under
-race. Covers the 150/213/277 stage caps and their 75/107/139
qualification bars, every probation class, budget-allow/ramp-hold and its
mirror, settlement idempotency, permanent-rejection release,
defer-versus-cancel, subdomain scope sharing, and a Free account that can
qualify stage one but not stage two without losing its progress.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX

* fix(sending): keep the ramp binding under retry, refusal, and rebind

Review of the ramp composition found seven ways the stage cap stopped being
a cap. All of them come from one shape: the sending budget is keyed by
submission ATTEMPT while the ramp reservation is keyed by MESSAGE, and code
that treats the two as interchangeable gives away capacity that was already
spent.

The refund. CancelAttempt released the ramp for any cancelled attempt.
Attempt one authorizes and hands 100 recipients to SES, the result is
ambiguous so nothing settles and the reservation correctly stands, River
allocates attempt two, a suppression cancels it — and the ramp counter goes
to zero for mail that is already in flight. It repeats, so the cap became
advisory. cancelRamp now asks about the OPERATION rather than the ordinal: if
any attempt was ever authorized, only SettleProvider may give those units
back. A reservation no attempt has authorized — the shape today's outbound
worker produces before this module is involved — is still refundable, which
is also where the mirror bug lived: the `released` early return meant a
cancel following a deferral never reached the ramp at all.

The stranding. A permanent sendramp error (`reservation already released` is
reachable by Reserve-after-Cancel on one operation) came back as a hard
error, rolling the transaction back with the attempt still `reserved`. Every
later execution failed identically, so nothing could ever release its units:
50 recipients pinned on global_all, global_probation, and account_daily
until midnight, three of them enough to close probationary sending for the
platform. Permanent refusals are now a terminal hold that releases, exactly
as the envelope path already answers the same class of loss.

The rebind. A domain with an unverified SENDING identity was ramp
pass-through AND, because InspectScopeTx never read the column, reported
established once its scope had a qualified day — no cap and no probation
charge. The wire identity is frozen at acceptance but the agent's registered
domain is not: verifying a child subdomain rebinds the account's agents onto
it, the child's SES identity stays pending while its DKIM is never
published, and the ramp resolves the domain live. An accepted backlog went
out uncapped under the parent's frozen From. It now holds, and classifies as
probationary. The two legacy states that mean "this domain already earned
its volume" — `exempt` and `complete` — are checked first and are untouched.

Three smaller ones. Reserve still classified probation as `op.Shared` with a
comment deferring to a task this commit is; the early hold therefore never
bounded the probation pool, the stored column disagreed with the class every
release targets, and each authorization paid a needless release-and-reacquire
on the platform's hottest counter rows. The ramp's own source read answered a
vanished message with a RETRYABLE hold, and because it runs before the
envelope resolution that answers it terminally, the wrong answer won whenever
the ramp was armed — a worker snoozing forever instead of failing once. And
in the ledger itself: a released-to-confirmed restoration errored instead of
no-opping when maintenance had reaped the day's counter (the reservation
outlives it by design), while ReleaseTx recorded `released` even when its
guarded decrement matched nothing, so a later restoration added back units
the counter never returned.

Tests: every fix above has a regression test that fails before it. The file
also gained the coverage Task 4 Step 2 asks for and did not have — racing
workers against one stage cap with the budget counters contended (exactly one
cap admitted, no deadlock), cross-midnight re-age of BOTH ledgers in one
authorization, settlement arriving days after its attempt, and a
FOR UPDATE NOWAIT probe that proves the named domain → scope → reservation →
day-counter suborder is actually taken rather than merely documented. Weak
assertions were tightened: the probation tests now assert the hold REASON,
the ramp-hold test checks all three pools it charged, and the Free-plan test
proves progression RESUMES after upgrade rather than merely surviving it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX

* test(sendramp): pin InspectScopeTx classification in its own package

InspectScopeTx decides which budget pool a send charges before the ramp
lock is taken, but every test of it lived in sendingpolicy, so the
per-package coverage gate saw it at 0% and failed the package at 76.6%.

Pin each branch where the code lives: missing domain, legacy exempt,
stamped complete, unverified identity with an old scope (must stay in
probation and ignore the scope), verified with no scope (day zero), a
completed scope behind a still-ramping domain row, the day-zero/day-one
boundary, and registrable-domain scope lookup with a hostname-keyed
decoy. Removing the unverified guard makes the fourth test fail.

Package coverage: 76.6% -> 84.9%.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX

* test(sendingpolicy): redeem before settling in the ramp tests

Settlement reports what the provider did, so it is only meaningful for an
attempt that opened the socket. The adapter (B5, #998) redeems the token
immediately before it dials, and the gate there refuses to settle an
attempt whose call_state is not 'started'. These tests settled straight
after ConsumeAttempt; route them through a consumeAndRedeem helper so the
two slices can land in either order.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jiashuoz
Merge remote-tracking branch 'origin/main' into feat/sending-provider…
e9633ae 
…-submit

# Conflicts:
#	internal/sendingpolicy/gate.go
@jiashuoz
jiashuoz merged commit 9c186cc into main Sep 5, 2026
29 checks passed
@jiashuoz
jiashuoz deleted the feat/sending-provider-submit branch September 5, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jiashuoz