amadan
Design: round two review fixes
Reap keys rather than deactivate them on entering per-instance mode, because enable delivers before its CAS write; a per-instance pause kind so the day roll cannot re-arm the app credential; a caller reverse index that doubles as the slug collision check; the uid predicate replaces "unit-backed"; an address branch through enable and the policy renderer; and the config-set ARN deduped, which the 2048-char policy limit needs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 file changed,
+89
−36
docs/superpowers/specs/2026-09-15-tenant-email-senders-design.md+89 −36
diff --git a/docs/superpowers/specs/2026-09-15-tenant-email-senders-design.md b/docs/superpowers/specs/2026-09-15-tenant-email-senders-design.md| index 5fd37a42d..2bec1dd5a 100644 |
| --- a/docs/superpowers/specs/2026-09-15-tenant-email-senders-design.md |
| +++ b/docs/superpowers/specs/2026-09-15-tenant-email-senders-design.md |
| @@ -166,9 +166,17 @@ creation and stored on the record: |
| uniqueness. The exact rule lives in one function, `SenderNames`, with |
| a test that enumerates the bounds. |
| -Because the slug is a hash, nothing parses it back. `SenderOfCaller` |
| -(§8) resolves a caller identity by looking up the recorded `IAMUser`, |
| -not by decomposing the name. |
| +`SenderNames` also covers standalone credentials, whose current name |
| +(`carlos-mail-<acct>-<app>+s+<name>`, `policy.go:92`) already reaches 95 |
| +characters against IAM's 64 with nothing checking it. The bounds test |
| +enumerates every form. |
| + |
| +Because the slug is a hash, nothing parses it back, and a 40-bit slug |
| +needs collision detection. Both are served by a create-only reverse |
| +index at `email/callers/<iamuser>.json` → (account, app, sender), |
| +written **before** `MintKey`: its create-only Put is the collision check, |
| +and it is what `SenderOfCaller` (§8) reads, so attributing an event costs |
| +one Get rather than a listing. |
| **Configuration sets stay per account** (`carlos-<acct>`), because an |
| identity has only one default set and §3 rules out per-sender sets on a |
| @@ -226,8 +234,8 @@ apps/<acct>/<app>/instances-config/<host>.json instance ← new |
| `SetExecDeliveryAt` does, so the fast loop converges. |
| - Guards that assume a two-layer merge and must be revisited: |
| `lostKeys`/`defaultMissing` (`sync.go:260`, `:388`), the mass-removal |
| - guard (`syncexec.go:351`), `allShadowed`/`AwaitingRestart` |
| - (`syncexec.go:712`, `:796`). `ConfigRouteFingerprints` |
| + guard (`syncexec.go:351`), `AwaitingRestart` (`syncexec.go:712`) and |
| + `allShadowed` (`syncexec.go:813`). `ConfigRouteFingerprints` |
| (`routefingerprint.go:58`) digests registry columns only and cannot |
| see a bucket-side change; the generation bump is that layer's signal. |
| - Only the platform writes this layer: no CLI verb, no console door. It |
| @@ -237,19 +245,32 @@ apps/<acct>/<app>/instances-config/<host>.json instance ← new |
| A prefix listing cannot decide this without racing an app-sender enable, |
| so the app sender carries `PerInstance bool`, and the transition is one |
| -CAS write on `senders/app.json` that, in order: |
| - |
| -1. Sets `PerInstance`. |
| -2. `SetKeyActive(false)` on the app sender's key, its previous key and |
| - every standalone credential — otherwise the credential already |
| - delivered to every instance keeps authorising sends for every domain, |
| - and addr-shaped routes are never bounced (`syncexec.go:440`), so the |
| - old environment survives in running processes. |
| -3. Unsets the app sender's `CARLOS_SMTP_*` vars from the default bundle. |
| +CAS write on `senders/app.json` that sets `PerInstance` together with |
| +`Active = false` and `PauseKind = PauseKindPerInstance`, then: |
| + |
| +1. **`ReapKeys(user, nil)` on the app sender's IAM user and on every |
| + standalone user** — list-and-delete, not `SetKeyActive(false)` on the |
| + recorded ids. Deactivating recorded ids is not enough, because |
| + `Enable` mints and delivers a key *before* its own CAS `Put` |
| + (`walk.go:717-738`): an enable racing the transition leaves an |
| + active, unrecorded key in every instance's bundle that nothing would |
| + ever reap. Deleting whatever keys exist makes the racing delivery |
| + inert. Instance `Enable` re-runs the same reap. |
| +2. Unsets the app sender's `CARLOS_SMTP_*` vars from the default bundle. |
| + |
| +`PauseKindPerInstance` matters because `resumeStaleCapPause` |
| +(`mailcounts.go:160`) runs on the status read and reactivates *every* |
| +key including standalones (`walk.go:973`); a cap-paused app sender would |
| +otherwise re-arm the credential every instance still holds on the next |
| +UTC day roll. `SetPaused(false)` and `resumeStaleCapPause` both refuse |
| +while `PerInstance` is set. |
| App-sender `Enable` refuses when `PerInstance` is set, under that same |
| etag. Instance `Enable` requires it. Leaving per-instance mode, once the |
| -last instance sender is gone, is a re-enable of the app sender. |
| +last instance sender is gone, is one CAS that clears `PerInstance`, sets |
| +`Active = true` and clears `PauseKind`, followed by a normal enable — |
| +otherwise `Enable` carries the false `Active` forward (`walk.go:698`) |
| +and mints a dead key. |
| `grantedDomains` filters on the domain record's new `Senders` list, not |
| `Apps`, and an instance sender's claim never adds the app to `Apps`. So |
| @@ -275,16 +296,18 @@ gains an `instance` field. |
| Two checks at `enable`, both refusals with an explaining message: |
| -- **Backing.** The instance's route must isolate its config file from |
| - siblings: unit-backed (systemd reads the 0600 file as root and drops |
| - to a per-unit `DynamicUser`), or exec-backed under an isolated-exec |
| - layout with a distinct uid. Plain exec-backed routes share the agent's |
| - uid, so `<dataDir>/<host>.config.env` is readable by every sibling and |
| - a per-instance credential buys nothing (`admintoken.go:6-12`). This is |
| - the dependency on #126's per-instance uids. |
| -- **Delivery.** The instance's box must ack the new config-ack schema |
| - (§6.5), otherwise its agent cannot render the instance layer and the |
| - credential would never arrive. |
| +- **A uid of its own.** The predicate is not "unit-backed" — a host_role |
| + app runs its units as `User=carlos-<app>` with `DynamicUser=no` |
| + (`caps/render.go:101-112`), one uid per *app*, and `ProtectProc` |
| + hides other users' processes, not a sibling's at the same uid. The |
| + check is that this instance's process gets a uid no sibling shares: |
| + `DynamicUser=yes` with no host_role grant, or an exec route under an |
| + isolated-exec layout with a per-instance uid. Plain exec routes share |
| + the agent's uid and `<dataDir>/<host>.config.env` is readable by every |
| + sibling (`internal/activator/admintoken.go:6-12`), so a per-instance |
| + credential buys nothing there. This is the dependency on #126. |
| +- **Delivery.** The instance's box must be running an agent that renders |
| + the instance layer (§6.5). |
| **Domain attach.** `claimDomain` gains a `Sender` field and refuses a |
| re-claim whose (app, sender) differs from the holder's, *including |
| @@ -301,9 +324,16 @@ refused `EnsureIdentity` can never wedge an app's teardown. |
| of the instance overlay the agent rendered. The console compares it with |
| the etag its own `Put` returned, which is decidable — unlike a key-id |
| map, which cannot distinguish an old agent from a new agent rendering a |
| -route with no mail vars. `Schema` is what the enable-time delivery check |
| -reads. `carlos email status --instance <host>` reports delivered, stale |
| -or never delivered. |
| +route with no mail vars. |
| + |
| +An ack is written only when a pass changes a file (`configack.go:66`), |
| +so a fresh instance, or one whose config has not moved since its agent |
| +was rolled, has no ack at all — absence cannot mean "old agent". Two |
| +changes follow: the agent rewrites the ack with its `Schema` on every |
| +pass, changed or not, and the enable-time check reads the box record's |
| +agent version, so its refusal names the box and the version rather than |
| +guessing. `carlos email status --instance <host>` reports delivered, |
| +stale or never delivered. |
| ## 7. #147: a sender under an identity CARLOS does not own |
| @@ -319,6 +349,17 @@ provisioner wrapper refuses those verbs for any domain with such a |
| record, and a test pins the refusal. `EnsureDomain` refuses the domain |
| before claiming it (§6.4), so the refusal can never strand a claim. |
| +**Enable takes an address branch.** Today `Enable` resolves granted |
| +domains and refuses with `ErrNoDomains` when there are none |
| +(`walk.go:627`), then insists the From address sits at a granted domain |
| +name (`:647`), and `SendPolicyJSON` requires each identity name to be a |
| +granted domain or a parent of one (`policy.go:167`). An external |
| +identity has no `DomainRecord` in the account by design, so an address |
| +sender needs its own branch in both: the identity ARN comes from the |
| +external-identity record, the From address from `Sender.Address`, and |
| +the policy renderer gets an address form whose identity check is that |
| +the address's domain is the identity or a child of it. |
| + |
| **Verification.** `Status.Verified()` requires `MailFrom == "SUCCESS"` |
| (`provisioner.go:45`), which an identity with no custom MAIL FROM never |
| reaches. External identities are checked with a relaxed predicate: DKIM |
| @@ -341,6 +382,9 @@ configuration set. So the sender's env bundle gains |
| `<P>_CONFIG_SET`, documented as a header the app must set, and |
| `carlos email status` says plainly when a sender has sent nothing — |
| the honour-system limitation you accepted, and one the relay removes. |
| +If the spike leaves a `ses:TenantName` condition in the policy, the |
| +message must carry `X-SES-TENANT` as well, so the bundle also gains |
| +`<P>_TENANT`; if the condition comes out, it does not. |
| Enable also reads the external identity's default configuration set: if |
| it has one that is not associated with the sender's tenant, a |
| tenant-tagged send is refused outright, so enable refuses first, with |
| @@ -361,10 +405,12 @@ built now. |
| **Attribution by caller.** Every sender has its own IAM user, and SES |
| stamps `mail.tags["ses:caller-identity"]` with the user name on Send, |
| -Bounce, Complaint, Delivery and Reject. `SenderOfCaller(caller)` |
| -resolves it to (account, app, sender) through the recorded `IAMUser`, |
| -covering the address-sender case where the domain names Classic, not |
| -CARLOS. `recordAndEnforceMail` counts there once, and falls back to |
| +Bounce, Complaint, Delivery and Reject. `SenderOfCaller(caller)` reads |
| +the reverse index `email/callers/<iamuser>.json` (§5) — one Get per |
| +event, against today's walk of every account's domain records — and |
| +resolves it to (account, app, sender), covering the address-sender case |
| +where the domain names Classic, not CARLOS. |
| +`recordAndEnforceMail` counts there once, and falls back to |
| today's domain-wide loop only when the caller is unknown, skipping any |
| app in per-instance mode so its events are never charged to the app |
| sender. |
| @@ -416,8 +462,11 @@ fields keep describing the app sender, so an older CLI still works. |
| forbidden verbs; instance enable writes only the instance layer; |
| entering per-instance mode deactivates the app key and every |
| standalone, and unsets the vars in the same pass. |
| -- Race test: instance enable interleaved with app enable leaves no app |
| - credential in the bundle. |
| +- Race test: an app enable that mints and delivers before its CAS `Put`, |
| + interleaved with the per-instance transition, leaves no usable app |
| + credential — the reap deleted the key even though the vars landed. |
| +- A cap-paused app sender that enters per-instance mode is not |
| + reactivated by the next UTC day roll. |
| - Name tests enumerating the IAM and SES bounds, including the longest |
| legal account and app. |
| - Policy tests: address condition (case-insensitive), tenant condition |
| @@ -463,8 +512,12 @@ sender, and is where a non-SES SMTP provider attaches. |
| sender plus one per standalone credential. At titogo's scale this is |
| the ceiling that eventually forces the relay. |
| - IAM inline policy: 2,048 characters per user, all policies aggregated. |
| - An app sender with many domains can reach it; `SendPolicyJSON` should |
| - refuse with a clear error rather than let IAM reject it. |
| + This bites sooner than it looks: every granted domain contributes the |
| + *same* account configuration-set ARN and nothing dedupes it |
| + (`walk.go:634`, `:343`, `policy.go:205`), so a twelve-domain app is |
| + already over. `slices.Compact` after the sort in the three callers |
| + takes that to about twenty, and `SendPolicyJSON` refuses with the |
| + count rather than letting IAM reject the policy. |
| - Configuration sets: 10,000 per region, not adjustable. Per account |
| here, so not a constraint. |
| - SES tenants: 10,000 per region, adjustable. Associations per tenant |