amadan

carlos / platform Public

tenant email: final review fix wave (C1, C2, I1-I6)

Seven fixes from the final whole-branch review, in one pass:

- C1: EnsureDomain now binds an identity to its configuration set
  (SetIdentityConfigSet, new on the Provisioner seam) whenever an SNS
  topic is configured — without it SES never emits a send/bounce/
  complaint event, so counts, the daily cap and the abuse pause were
  all permanently dead.
- C2: Enable's idempotence short-circuit no longer skips EnsureUser —
  the IAM policy is now rewritten from the current granted-domain set
  on every call, so attaching (or losing) a domain takes effect on a
  plain re-enable, matching spec §4.
- I1: EnvDelivered now compares _FROM's delivered value against
  Declared.From, the same way _USER is compared against KeyID, so a
  re-declared From address is re-delivered instead of drifting stale
  forever.
- I2: mail.Teardown is now wired into both places objstore.Teardown
  already was (ownership.go's permanent delete, appsettings.go's store
  destroy) — a purged app no longer strands its global domain claim.
- I3: a daily-cap pause now auto-resumes on the UTC day roll
  (Provisioned.PauseKind distinguishes it from a rate/manual pause,
  which still never auto-resumes); the CLI no longer points a paused
  member at the operator-only resume verb.
- I4: the console now computes and carries an app's default sending
  host (<app>.<sqid>.<platform-suffix>) on every email reply, so
  `carlos email enable` with no -from is a real one-command story.
- I5: sesevents_test.go's stale "certificate chain" comments now match
  the actual (chain-free) trust model.
- I6: event attribution also matches a domain's recorded MAIL FROM
  subdomain, not just its own name, so SES reporting the rewritten
  envelope no longer drops the event.

Mutation-tested C1, C2, I1, I3 and I6 by hand: each fix broken,
confirmed the relevant test fails, restored. Full gate (build, vet,
all 30 packages, GOOS=darwin build, gofmt) green throughout.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev ccc5d93234c8748478a0067d44ced6317f4cab46 parent b94d5a9
20 files changed, +1050 −76
  • internal/console/apimail.go +52 −10
  • internal/console/apimail_test.go +57 −0
  • internal/console/appsettings.go +41 −0
  • internal/console/deletion_test.go +57 −0
  • internal/console/mailcounts.go +57 −10
  • internal/console/mailcounts_test.go +79 −5
  • internal/console/ownership.go +52 −0
  • internal/console/sesevents.go +24 −1
  • internal/console/sesevents_test.go +93 −4
  • internal/mail/aws.go +19 −0
  • internal/mail/aws_test.go +53 −0
  • internal/mail/mail.go +25 −5
  • internal/mail/memprovisioner.go +22 −0
  • internal/mail/provisioner.go +10 −0
  • internal/mail/walk.go +82 −16
  • internal/mail/walk_domain_test.go +50 −0
  • internal/mail/walk_enable_test.go +134 −1
  • internal/mail/walk_lifecycle_test.go +10 −10
  • mail_cli.go +62 −13
  • mail_cli_test.go +71 −1
diff --git a/internal/console/apimail.go b/internal/console/apimail.go
index 3eaa8152d..948cc0093 100644
--- a/internal/console/apimail.go
+++ b/internal/console/apimail.go
@@ -254,6 +254,29 @@ func mailEffectivePrefix(rec *mail.Record) string {
return mail.DefaultEnvPrefix
}
+// mailDefaultHost is the app's own host — <app>.<sqid>.<platform-suffix> —
+// the default identity spec §3 promises (the same shape
+// internal/domains/reconcile.go mints a route for) and the one thing that
+// makes `carlos email enable` with no -from a one-command story (I4,
+// review follow-up). The CLI cannot compute this itself: it never learns
+// CARLOS_MAIL_PLATFORM_SUFFIX, only this console does — so it has to ride
+// along in the reply. Empty when this deployment has no platform suffix
+// configured; the CLI keeps its explicit "no From address" refusal in that
+// case, exactly as before this fix.
+//
+// DELIBERATELY INDEPENDENT OF Config.AppsDomain, the same caution
+// mailDeployment's own doc comment (cmd/console/main.go) already applies:
+// conflating the edge's routing zone with the zone SES identities are
+// verified under would repeat the class of bug #167 already cost this
+// codebase once. This uses ONLY s.cfg.MailDeployment.PlatformSuffix.
+func (s *Server) mailDefaultHost(acct *Account, app string) string {
+ suffix := s.cfg.MailDeployment.PlatformSuffix
+ if suffix == "" {
+ return ""
+ }
+ return app + "." + acct.Sqid + "." + suffix
+}
+
// mailEffectiveCap mirrors mailcounts.go's enforceMailLimits exactly — the
// cap a status reply names must be the SAME cap the limiter enforces, or a
// member reading "cap: 1000" while running on the free tier's 50 would have
@@ -362,7 +385,12 @@ func (s *Server) emailReply(ctx context.Context, acct *Account, app string, rec
missing := []string{}
if rec.Provisioned != nil {
wantKeyID := rec.Provisioned.KeyID
- got, err := mail.EnvDelivered(ctx, s.cfg.Releases, acct.Sqid, app, prefix, wantKeyID)
+ // rec.Declared is guaranteed non-nil here: Provisioned is only ever
+ // set by Enable, which itself requires Declared != nil — so
+ // rec.Declared.From is always the record's CURRENT From (I1,
+ // review follow-up: EnvDelivered compares _FROM's delivered value
+ // against it the same way _USER is compared against KeyID).
+ got, err := mail.EnvDelivered(ctx, s.cfg.Releases, acct.Sqid, app, prefix, wantKeyID, rec.Declared.From)
if err != nil {
return nil, err
}
@@ -378,13 +406,14 @@ func (s *Server) emailReply(ctx context.Context, acct *Account, app string, rec
standalone = []*mail.Standalone{}
}
return map[string]any{
- "app": app,
- "declared": rec.Declared,
- "grant": rec.Grant,
- "provisioned": rec.Provisioned,
- "standalone": standalone,
- "domains": domains,
- "env_names": names,
+ "app": app,
+ "declared": rec.Declared,
+ "grant": rec.Grant,
+ "provisioned": rec.Provisioned,
+ "standalone": standalone,
+ "default_host": s.mailDefaultHost(acct, app),
+ "domains": domains,
+ "env_names": names,
// A rotation that minted a new key but has not yet deleted the old
// one — the member-visible half of the two-key overlap.
"pending_finish": rec.Provisioned != nil && rec.Provisioned.PrevKeyID != "",
@@ -545,7 +574,15 @@ func (s *Server) handleAPIEmailStatus(w http.ResponseWriter, r *http.Request) {
return
}
if rec == nil {
- jsonError(w, http.StatusNotFound, "no email is declared for this app — declare it first")
+ // Still carries default_host (I4, review follow-up), even on this
+ // 404: `carlos email enable` with no -from makes its FIRST call
+ // here, before anything has ever been declared, and that is
+ // exactly when the app's default sending host needs to reach the
+ // CLI for the one-command story to work at all.
+ writeJSON(w, http.StatusNotFound, map[string]any{
+ "error": "no email is declared for this app — declare it first",
+ "default_host": s.mailDefaultHost(acct, app),
+ })
return
}
out, err := s.emailReply(ctx, acct, app, rec)
@@ -1012,7 +1049,12 @@ func (s *Server) handleAPIEmailPause(w http.ResponseWriter, r *http.Request) {
// writing one — it must not touch Active at all.
var rec *mail.Record
if req.Paused != nil {
- rec, err = mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct.Sqid, app, *req.Paused, time.Now())
+ // "" — this is the MANUAL (operator) pause/resume door, not an
+ // automatic one; mail.PauseKindCap/PauseKindRate are stamped only
+ // by mailcounts.go's enforceMailLimits (I3, review follow-up). An
+ // empty PauseKind is exactly what makes a manual pause never
+ // auto-resume on the UTC day roll, same as a rate pause.
+ rec, err = mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct.Sqid, app, *req.Paused, "", time.Now())
if errors.Is(err, mail.ErrNotProvisioned) {
jsonError(w, http.StatusConflict, "no provisioned email credential to pause — enable email first")
return
diff --git a/internal/console/apimail_test.go b/internal/console/apimail_test.go
index 611578d46..bc5225c6a 100644
--- a/internal/console/apimail_test.go
+++ b/internal/console/apimail_test.go
@@ -322,6 +322,63 @@ func TestAPIEmailDeclareWritesTheRecord(t *testing.T) {
auditedBy(t, fx, app, "email-declare", "alice@keymail.dev", "hello@titogo.example")
}
+// TestAPIEmailStatusCarriesTheDefaultHostEvenWhenNotDeclared (I4, review
+// follow-up): `carlos email enable` with no -from needs the app's default
+// sending host on its VERY FIRST call — before anything has ever been
+// declared, which is exactly the 404 handleAPIEmailStatus gives. The field
+// must ride along on that 404, not only on a 200.
+func TestAPIEmailStatusCarriesTheDefaultHostEvenWhenNotDeclared(t *testing.T) {
+ fx, _, _, token := emailArmed(t, "code-email-default-host-404")
+ app := "titogo"
+ if rec := emailRecord(t, fx, app); rec != nil {
+ t.Fatalf("a record exists before any declaration: %+v", rec)
+ }
+
+ status, out := getJSON(t, emailURL(fx, app), token)
+ if status != http.StatusNotFound {
+ t.Fatalf("status = %d %v, want 404 for an undeclared app", status, out)
+ }
+ want := "titogo.bab.oncarlos.com"
+ if got, _ := out["default_host"].(string); got != want {
+ t.Fatalf("default_host = %q, want %q", got, want)
+ }
+}
+
+// TestAPIEmailStatusCarriesTheDefaultHostOnceDeclared: the same field on
+// the ordinary 200 path, once something has actually been declared.
+func TestAPIEmailStatusCarriesTheDefaultHostOnceDeclared(t *testing.T) {
+ fx, _, _, token := emailArmed(t, "code-email-default-host-200")
+ app := "titogo"
+ declareEmail(t, fx, token, app, "hello@titogo.example")
+
+ status, out := getJSON(t, emailURL(fx, app), token)
+ if status != http.StatusOK {
+ t.Fatalf("status = %d %v", status, out)
+ }
+ want := "titogo.bab.oncarlos.com"
+ if got, _ := out["default_host"].(string); got != want {
+ t.Fatalf("default_host = %q, want %q", got, want)
+ }
+}
+
+// TestAPIEmailStatusDefaultHostEmptyWithNoPlatformSuffix: a deployment with
+// no CARLOS_MAIL_PLATFORM_SUFFIX configured has no zone to default into —
+// default_host must stay empty, which is what keeps the CLI's explicit
+// "no From address" refusal intact rather than defaulting to a bogus host.
+func TestAPIEmailStatusDefaultHostEmptyWithNoPlatformSuffix(t *testing.T) {
+ fx, _, _, token := emailArmed(t, "code-email-default-host-empty")
+ fx.srv.cfg.MailDeployment.PlatformSuffix = ""
+ app := "titogo"
+
+ status, out := getJSON(t, emailURL(fx, app), token)
+ if status != http.StatusNotFound {
+ t.Fatalf("status = %d %v, want 404 for an undeclared app", status, out)
+ }
+ if got, _ := out["default_host"].(string); got != "" {
+ t.Fatalf("default_host = %q, want empty with no platform suffix configured", got)
+ }
+}
+
// TestAPIEmailDeclareRefusesAnUnaddressedFrom: a From with no "@" is refused
// before anything is written.
func TestAPIEmailDeclareRefusesAnUnaddressedFrom(t *testing.T) {
diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go
index 35d55cb6b..d5ada86bb 100644
--- a/internal/console/appsettings.go
+++ b/internal/console/appsettings.go
@@ -38,6 +38,7 @@ import (
"time"
"github.com/carlosframework/platform/internal/domains"
+ "github.com/carlosframework/platform/internal/mail"
"github.com/carlosframework/platform/internal/objstore"
"github.com/carlosframework/platform/internal/release"
)
@@ -599,6 +600,34 @@ func (s *Server) handleStoreDestroy(w http.ResponseWriter, r *http.Request) {
"this deployment has to tear it down.")
return
}
+ // An email estate is torn down alongside the object store here too (I2,
+ // review follow-up): mail.Teardown is the ONLY place that releases this
+ // app's global domain claims (spec §4a), and this button — like
+ // ownership.go's handlePurgeApp, which already does the same — is one
+ // of the two backend-destroying surfaces objstore.Teardown was called
+ // from with no email counterpart. Checked BEFORE requireFreshAuth, same
+ // as the store's own backend check above: a doomed request must not
+ // spend the passkey.
+ mrec, _, mrerr := mail.Get(ctx, s.cfg.Releases, acct.Sqid, app)
+ if mrerr != nil {
+ log.Printf("console: store destroy mail read %s/%s: %v", acct.Sqid, app, mrerr)
+ fail("Could not read “" + app + "”'s email record — nothing was destroyed. Try again.")
+ return
+ }
+ mailEstate := mrec != nil && (mrec.Provisioned != nil || len(mrec.Standalone) > 0)
+ if !mailEstate {
+ // A domain claim can exist with no delivered credential at all —
+ // `carlos email domains add` alone, never followed by `enable` —
+ // and mail.Teardown releases that claim too.
+ if doms, derr := s.mailAppDomains(ctx, acct, app); derr == nil && len(doms) > 0 {
+ mailEstate = true
+ }
+ }
+ if mailEstate && s.cfg.MailProvisioner == nil {
+ fail("This console has no SES backend configured, so it cannot delete “" + app + "”'s email " +
+ "credentials — nothing was destroyed. An operator of this deployment has to tear it down.")
+ return
+ }
if !s.requireFreshAuth(w, r, addr, raw) {
return
}
@@ -612,6 +641,18 @@ func (s *Server) handleStoreDestroy(w http.ResponseWriter, r *http.Request) {
"backend is reachable.")
return
}
+ if s.cfg.MailProvisioner != nil {
+ mdep, mwriter := s.mailDeploymentFor()
+ if terr := mail.Teardown(ctx, s.cfg.Releases, s.cfg.MailProvisioner, mwriter, mdep, acct.Sqid, app); terr != nil {
+ log.Printf("console: store destroy mail teardown %s/%s: %v", acct.Sqid, app, terr)
+ fail("Destroyed the object store, but could not tear down “" + app + "”'s email estate — " +
+ "the store is already gone; try again to finish tearing down email.")
+ return
+ }
+ if mailEstate {
+ s.audit(acct, app, addr, "email-teardown", "tore down the email estate alongside the object store")
+ }
+ }
what, flash := "the store declaration", "Withdrew “"+app+"”'s store declaration."
if estate {
what = "bucket " + want
diff --git a/internal/console/deletion_test.go b/internal/console/deletion_test.go
index 2bb37f40f..b7805cdad 100644
--- a/internal/console/deletion_test.go
+++ b/internal/console/deletion_test.go
@@ -3,6 +3,7 @@ package console
import (
"context"
"encoding/json"
+ "errors"
"fmt"
"net/http"
"net/url"
@@ -11,7 +12,9 @@ import (
"time"
"github.com/carlosframework/platform/internal/appconfig"
+ "github.com/carlosframework/platform/internal/dnsrec"
"github.com/carlosframework/platform/internal/domains"
+ "github.com/carlosframework/platform/internal/mail"
"github.com/carlosframework/platform/internal/objstore"
"github.com/carlosframework/platform/internal/release"
)
@@ -737,3 +740,57 @@ func TestPurgeAdviceMapsAStoreRefusalWithoutOverclaiming(t *testing.T) {
"(the guard wraps this sentinel for an unreadable record too)", msg)
}
}
+
+// ---- purge and the email estate (I2, review follow-up) --------------------
+//
+// mail.Teardown had NO production caller anywhere before this fix. A purged
+// app left a live IAM user holding an active key, its SES identity, its
+// published DNS, and — worst — the create-only GLOBAL domain claim at
+// email/domain-claims/<domain>.json. Because release.PurgeApp removes
+// email.json while DomainRecord.Apps still names the deleted app, the
+// last-referrer branch inside mail.Teardown could never fire again for that
+// domain once the app was gone — the domain was then denied to every CARLOS
+// account, including its rightful owner, permanently.
+//
+// The proof used here is the claim's own semantics, not an internal peek at
+// bucket keys: a SECOND, unrelated account trying to ensure the SAME domain
+// is refused with ErrDomainClaimed before the purge (the claim is doing its
+// job), and succeeds after (the purge released it).
+func TestPurgeReleasesTheDomainClaim(t *testing.T) {
+ fx, c, csrf := trashPageFixture(t)
+ fake := mail.NewMemProvisioner("1")
+ writer := dnsrec.NewMemWriter()
+ fx.srv.cfg.MailProvisioner = fake
+ fx.srv.cfg.DNSWriter = writer
+ dep := mail.Deployment{Region: "eu-west-1"} // no PlatformSuffix: a custom
+ // domain keeps this test about the claim gate alone, not the SEPARATE
+ // platform-name-ownership gate (walk.go's platformNameOwnedBy), which
+ // would otherwise refuse the second account for an unrelated reason.
+
+ ctx := context.Background()
+ now := time.Now()
+ domain := "shared-claim.example.com"
+ // Deliberately no mail.Declare/Enable — kass has a domain record (and
+ // the claim it holds) but no Provisioned credential, exactly the shape
+ // `carlos email domains add` alone leaves. mail.Teardown must still
+ // find and release it: the domain-referrer set, not just a delivered
+ // credential, is what the claim traces back to.
+ if _, _, err := mail.EnsureDomain(ctx, fx.rel, fake, writer, dep, fx.sqid, "kass", domain, now); err != nil {
+ t.Fatalf("EnsureDomain for kass: %v", err)
+ }
+
+ if _, _, err := mail.EnsureDomain(ctx, fx.rel, fake, writer, dep, "zzzz", "other", domain, now); !errors.Is(err, mail.ErrDomainClaimed) {
+ t.Fatalf("a second account's EnsureDomain before the purge = %v, want ErrDomainClaimed", err)
+ }
+
+ status, _, body := postGated(t, fx, c, fx.u("/trash/delete"),
+ url.Values{"csrf": {csrf}, "app": {"kass"}, "confirm": {"kass"}})
+ if status != http.StatusSeeOther {
+ t.Fatalf("purge = %d:\n%s", status, body)
+ }
+ auditedBy(t, fx, "kass", "email-teardown", "paul@keymail.dev", "email estate")
+
+ if _, _, err := mail.EnsureDomain(ctx, fx.rel, fake, writer, dep, "zzzz", "other", domain, now); err != nil {
+ t.Fatalf("EnsureDomain after the purge = %v, want the claim released for anyone to take", err)
+ }
+}
diff --git a/internal/console/mailcounts.go b/internal/console/mailcounts.go
index a6cfd9a34..2d3ceebbb 100644
--- a/internal/console/mailcounts.go
+++ b/internal/console/mailcounts.go
@@ -141,13 +141,49 @@ func (s *Server) enforceMailLimits(ctx context.Context, acct, app string, now ti
if err != nil {
return false, "", err
}
- // Nothing provisioned, or already paused: there is nothing to stop.
- // Re-pausing would rewrite the record and re-audit on every event, and
- // — more importantly — would lift a pause that a human has not decided
- // to lift. See "NOTHING auto-resumes" below.
- if rec == nil || rec.Provisioned == nil || !rec.Provisioned.Active {
+ if rec == nil || rec.Provisioned == nil {
return false, "", nil
}
+ if !rec.Provisioned.Active {
+ // I3 (review follow-up): a CAP pause is a PER-DAY limit, because
+ // the counts it was computed from are per-day — it MUST clear on
+ // the UTC day roll, or a free-tier member's ordinary first day
+ // over 50 messages stops their email forever, with resume gated
+ // behind an operator allowlist that ships empty
+ // (CARLOS_MAIL_OPERATORS, fail-closed). A RATE pause or a MANUAL
+ // (operator) pause — PauseKind is anything other than
+ // mail.PauseKindCap, including "" — still never auto-resumes: see
+ // "RATE AND MANUAL PAUSES NEVER AUTO-RESUME" below, which still
+ // holds for those two.
+ if rec.Provisioned.PauseKind != mail.PauseKindCap {
+ return false, "", nil
+ }
+ pausedDay := rec.Provisioned.At.UTC().Format("2006-01-02")
+ today := now.UTC().Format("2006-01-02")
+ if pausedDay >= today {
+ // Paused earlier TODAY (or a clock oddity puts the pause in
+ // the future) — not a day roll yet.
+ return false, "", nil
+ }
+ if _, err := mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, false, "", now); err != nil {
+ return false, "", err
+ }
+ if a, err := s.store.AccountBySqid(acct); err != nil {
+ log.Printf("console: mail resume audit: resolving account %s: %v", acct, err)
+ } else if a != nil {
+ s.audit(a, app, "ses", "mail.resumed", fmt.Sprintf("%s: daily cap pause cleared on the UTC day roll", app))
+ }
+ // Re-read: SetPaused just wrote a fresh record (Active, PauseKind
+ // and At all moved), and everything below reads rec fresh rather
+ // than patching the in-memory copy by hand.
+ rec, _, err = mail.Get(ctx, s.cfg.Releases, acct, app)
+ if err != nil {
+ return false, "", err
+ }
+ if rec == nil || rec.Provisioned == nil || !rec.Provisioned.Active {
+ return false, "", nil
+ }
+ }
day := now.UTC().Format("2006-01-02")
c, err := s.store.MailCounts(ctx, acct, app, day)
if err != nil {
@@ -158,23 +194,26 @@ func (s *Server) enforceMailLimits(ctx context.Context, acct, app string, now ti
cap = rec.Grant.DailyCap
}
- var reason string
+ var reason, kind string
switch {
case c.Sends >= cap:
reason = fmt.Sprintf("daily cap %d reached (%d sent)", cap, c.Sends)
+ kind = mail.PauseKindCap
// The send floor is what stops one bounce on a two-message morning
// reading as a 50% bounce rate and pausing a healthy tenant.
case c.Sends >= MinSendsForRates && c.BounceRate() > BounceThreshold:
reason = fmt.Sprintf("bounce rate %.1f%% over %.0f%% (%d of %d)",
c.BounceRate()*100, BounceThreshold*100, c.Bounces, c.Sends)
+ kind = mail.PauseKindRate
case c.Sends >= MinSendsForRates && c.ComplaintRate() > ComplaintThreshold:
reason = fmt.Sprintf("complaint rate %.2f%% over %.1f%% (%d of %d)",
c.ComplaintRate()*100, ComplaintThreshold*100, c.Complaints, c.Sends)
+ kind = mail.PauseKindRate
default:
return false, "", nil
}
- if _, err := mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, true, now); err != nil {
+ if _, err := mail.SetPaused(ctx, s.cfg.Releases, s.cfg.MailProvisioner, acct, app, true, kind, now); err != nil {
return false, "", err
}
// Audit AFTER the write it announces, and never before — a pause the
@@ -188,6 +227,14 @@ func (s *Server) enforceMailLimits(ctx context.Context, acct, app string, now ti
return true, reason, nil
}
-// NOTHING auto-resumes. A pause that lifted itself at midnight would let an
-// abusive tenant send its cap again every day forever, which is not a limit
-// — it is a schedule. Resume is a decision, taken through the door.
+// RATE AND MANUAL PAUSES NEVER AUTO-RESUME. A RATE pause that lifted itself
+// at midnight would let an abusive tenant breach SES's account-review
+// thresholds again every day forever, which is not a limit — it is a
+// schedule. A MANUAL (operator) pause is a human's deliberate decision,
+// which only a human — through the door — gets to undo. Resume for either
+// is a decision, taken through the door.
+//
+// A CAP pause is different (I3, review follow-up, amending the rule this
+// comment used to state without exception): it is computed from PER-DAY
+// counts, so "never auto-resumes" turned an ordinary tier limit into a
+// permanent one — see the day-roll check above.
diff --git a/internal/console/mailcounts_test.go b/internal/console/mailcounts_test.go
index d43f8a38a..ac27f01fe 100644
--- a/internal/console/mailcounts_test.go
+++ b/internal/console/mailcounts_test.go
@@ -196,22 +196,96 @@ func TestSESThresholdConstants(t *testing.T) {
// A pause must not lift itself. Re-running enforce on a paused app with a
// clean slate must leave it paused — resume is a decision, not a timer.
-func TestEnforceNeverAutoResumes(t *testing.T) {
+// I3 (review follow-up): a CAP pause is a PER-DAY limit — the counts it was
+// computed from are per-day — so it MUST clear on the UTC day roll, or a
+// free-tier member's ordinary first day over 50 messages stops their email
+// forever, with resume gated behind an operator allowlist that ships empty
+// (CARLOS_MAIL_OPERATORS, fail-closed). This test used to assert the
+// OPPOSITE of what it asserts now — see the note SetPaused's own doc
+// comment carries about the day-roll check this behaviour depends on.
+func TestEnforceCapPauseResumesOnTheNextUTCDay(t *testing.T) {
fx, now := mailFixture(t)
seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 50})
- fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", now)
+ if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", now); err != nil {
+ t.Fatal(err)
+ }
+ rec, _, _ := mail.Get(context.Background(), fx.rel, release.OpsAccount, "seapointish")
+ if rec.Provisioned.Active {
+ t.Fatal("did not pause on the cap breach")
+ }
+ if rec.Provisioned.PauseKind != mail.PauseKindCap {
+ t.Fatalf("PauseKind = %q, want %q", rec.Provisioned.PauseKind, mail.PauseKindCap)
+ }
- next := now.AddDate(0, 0, 1) // a fresh day, no counts at all
+ next := now.AddDate(0, 0, 1) // a fresh UTC day, no counts at all
paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", next)
if err != nil {
t.Fatal(err)
}
if paused {
- t.Fatal("enforce re-paused an already-paused app — it should have returned early")
+ t.Fatal("enforce reports a NEW pause on the day-roll resume — nothing should trip on an empty day")
+ }
+ rec, _, _ = mail.Get(context.Background(), fx.rel, release.OpsAccount, "seapointish")
+ if !rec.Provisioned.Active {
+ t.Fatal("a cap pause from yesterday did not resume on the UTC day roll")
+ }
+ if rec.Provisioned.PauseKind != "" {
+ t.Fatalf("PauseKind = %q after the auto-resume, want cleared", rec.Provisioned.PauseKind)
+ }
+
+ entries, err := fx.st.Events(OpsAccountID, "seapointish", 10)
+ if err != nil {
+ t.Fatal(err)
+ }
+ found := false
+ for _, e := range entries {
+ if strings.Contains(e.Action, "mail.resumed") {
+ found = true
+ }
+ }
+ if !found {
+ t.Fatal("no audit entry for the automatic day-roll resume")
+ }
+}
+
+// The symmetric case: a RATE pause (a bounce/complaint threshold breach)
+// never auto-resumes, day roll or not — SES's account-review thresholds
+// are not a daily allowance, and lifting the brake on a schedule would let
+// an abusive tenant breach it again every day forever.
+func TestEnforceRatePauseNeverAutoResumes(t *testing.T) {
+ fx, now := mailFixture(t)
+ // Cap is 50, so raise it first: this test is about the RATE pause, and
+ // a cap breach would mask which kind actually paused it.
+ if _, err := mail.ApplyGrant(context.Background(), fx.rel, release.OpsAccount, "seapointish",
+ []string{"x.com"}, 100000, "operator", "paul@rslw.com", now); err != nil {
+ t.Fatal(err)
+ }
+ seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 1000, Bounces: 60})
+ if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", now); err != nil {
+ t.Fatal(err)
}
rec, _, _ := mail.Get(context.Background(), fx.rel, release.OpsAccount, "seapointish")
if rec.Provisioned.Active {
- t.Fatal("the pause lifted itself at midnight")
+ t.Fatal("did not pause on the bounce-rate breach")
+ }
+ if rec.Provisioned.PauseKind != mail.PauseKindRate {
+ t.Fatalf("PauseKind = %q, want %q", rec.Provisioned.PauseKind, mail.PauseKindRate)
+ }
+
+ next := now.AddDate(0, 0, 1) // a fresh UTC day
+ paused, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", next)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if paused {
+ t.Fatal("enforce re-paused an already-paused app — it should have returned early")
+ }
+ rec, _, _ = mail.Get(context.Background(), fx.rel, release.OpsAccount, "seapointish")
+ if rec.Provisioned.Active {
+ t.Fatal("a rate pause auto-resumed on the UTC day roll — it must not")
+ }
+ if rec.Provisioned.PauseKind != mail.PauseKindRate {
+ t.Fatalf("PauseKind = %q after a no-op day roll, want it to survive as %q", rec.Provisioned.PauseKind, mail.PauseKindRate)
}
}
diff --git a/internal/console/ownership.go b/internal/console/ownership.go
index 353ad31e8..4d2a05aec 100644
--- a/internal/console/ownership.go
+++ b/internal/console/ownership.go
@@ -23,6 +23,7 @@ import (
"time"
"github.com/carlosframework/platform/internal/domains"
+ "github.com/carlosframework/platform/internal/mail"
"github.com/carlosframework/platform/internal/objstore"
"github.com/carlosframework/platform/internal/release"
)
@@ -376,6 +377,57 @@ func (s *Server) handlePurgeApp(w http.ResponseWriter, r *http.Request) {
fail("Could not read “" + app + "”'s object-store record — nothing was deleted; try again.")
return
}
+ // An email estate is torn down BEFORE the prefix sweep too, for the
+ // identical reason the store is above (I2, review follow-up):
+ // mail.Teardown is the ONLY place that releases this app's domain
+ // claims — the create-only global gate at
+ // email/domain-claims/<domain>.json (spec §4a) — and release.PurgeApp
+ // below deletes email.json out from under it. Without this, a purged
+ // app leaves a live IAM user with an active key, its SES identity, its
+ // published DNS, and — worst — a domain claim that DomainRecord.Apps
+ // can never again name this (now-deleted) app to release: the domain
+ // is denied to every CARLOS account, including its rightful owner,
+ // permanently.
+ //
+ // mail.Teardown is nil-record-safe (it sweeps this account's domain
+ // records and this app's ORPHANED claims even when the app never
+ // declared email at all — its own doc comment), so it runs whenever a
+ // MailProvisioner is configured, not only when a Provisioned/Standalone
+ // estate is already known to exist; the estate check below only decides
+ // whether a MISSING provisioner has to refuse the whole delete.
+ mrec, _, mrerr := mail.Get(r.Context(), s.cfg.Releases, acct.Sqid, app)
+ if mrerr != nil {
+ log.Printf("console: purge mail read %s/%s: %v", acct.Sqid, app, mrerr)
+ fail("Could not read “" + app + "”'s email record — nothing was deleted; try again.")
+ return
+ }
+ mailEstate := mrec != nil && (mrec.Provisioned != nil || len(mrec.Standalone) > 0)
+ if !mailEstate {
+ // A domain claim can exist with no delivered credential at all —
+ // `carlos email domains add` alone, never followed by `enable` —
+ // and mail.Teardown releases that claim too. The audit trail
+ // below should say so.
+ if doms, derr := s.mailAppDomains(r.Context(), acct, app); derr == nil && len(doms) > 0 {
+ mailEstate = true
+ }
+ }
+ if mailEstate && s.cfg.MailProvisioner == nil {
+ fail("“" + app + "” has a provisioned email credential, and this console has no SES backend configured — it cannot be deleted here until email is torn down.")
+ return
+ }
+ if s.cfg.MailProvisioner != nil {
+ mctx, mcancel := context.WithTimeout(r.Context(), storeTeardownTimeout)
+ defer mcancel()
+ mdep, mwriter := s.mailDeploymentFor()
+ if terr := mail.Teardown(mctx, s.cfg.Releases, s.cfg.MailProvisioner, mwriter, mdep, acct.Sqid, app); terr != nil {
+ log.Printf("console: purge mail teardown %s/%s: %v", acct.Sqid, app, terr)
+ fail("Could not tear down “" + app + "”'s email estate — nothing was deleted; try again.")
+ return
+ }
+ if mailEstate {
+ s.audit(acct, app, addr, "email-teardown", "tore down the email estate (credentials, IAM principal, domain claims) during permanent deletion")
+ }
+ }
if err := release.PurgeApp(r.Context(), s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, time.Now()); err != nil {
log.Printf("console: purge %s/%s: %v", acct.Sqid, app, err)
fail(purgeAdvice(err, app))
diff --git a/internal/console/sesevents.go b/internal/console/sesevents.go
index 53925acfd..c9618b0a8 100644
--- a/internal/console/sesevents.go
+++ b/internal/console/sesevents.go
@@ -427,6 +427,29 @@ func (s *Server) handleSESEvents(w http.ResponseWriter, r *http.Request) {
// bounded number of accounts; this is the same shape ListDomains' own
// callers (mail.Teardown, mail.grantedDomains) already accept for a single
// account, one level up.
+// domainMatchesEvent reports whether ev's domain attributes to d — either
+// the identity's own domain (the sender-supplied From SES usually reports
+// as mail.source) or, for ANY region, d's recorded custom MAIL FROM
+// subdomain (I6, review follow-up). SES may report the REWRITTEN envelope
+// — mail-<region>.<domain>, the custom MAIL FROM MX target (spec §7) —
+// rather than the sender-supplied From, and when it does, a match against
+// d.Domain alone attributes the event to nothing: every count for that
+// send silently drops, which is a second, independent way the whole of
+// spec §9 (caps, the abuse pause) stops working. Matched against RECORDED
+// values only, never a prefix-strip heuristic — MailFromDomain's shape
+// could change, and this must not assume it.
+func domainMatchesEvent(d *mail.DomainRecord, domain string) bool {
+ if d.Domain == domain {
+ return true
+ }
+ for _, st := range d.Regions {
+ if st != nil && st.MailFrom != "" && st.MailFrom == domain {
+ return true
+ }
+ }
+ return false
+}
+
func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event) {
accts, err := s.store.Accounts()
if err != nil {
@@ -452,7 +475,7 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event) {
continue
}
for _, d := range domains {
- if d.Domain != ev.Domain {
+ if !domainMatchesEvent(d, ev.Domain) {
continue
}
for _, app := range d.Apps {
diff --git a/internal/console/sesevents_test.go b/internal/console/sesevents_test.go
index af1c90a6d..eac01babc 100644
--- a/internal/console/sesevents_test.go
+++ b/internal/console/sesevents_test.go
@@ -5,9 +5,11 @@ package console
// in member — so what stands between a forged bounce and an automatic pause
// on a stranger's say-so is exactly what these tests pin: the TopicArn
// allowlist, the SigningCertURL/SubscribeURL host check (pinned to this
-// deployment's own region when known), certificate chain and subject
-// verification, a redirect guard on both outbound fetches, and the ordering
-// that runs all of it before any network call or trusted field.
+// deployment's own region when known), HTTPS to that pinned host with
+// redirects refused on both outbound fetches, the certificate's subject
+// assertion — NOT a chain build; sesevents.go's file comment records why
+// there is no chain to build for SNS's own signing certificate — and the
+// ordering that runs all of it before any network call or trusted field.
import (
"context"
@@ -29,6 +31,9 @@ import (
"strings"
"testing"
"time"
+
+ "github.com/carlosframework/platform/internal/mail"
+ "github.com/carlosframework/platform/internal/release"
)
// errFakeSignature stands in for a real crypto failure in the seam tests —
@@ -63,7 +68,9 @@ func notReached(t *testing.T) *httptest.Server {
}
// The SigningCertURL check is the ENTIRETY of SNS signature security — half
-// of it, along with fetchVerifiedCertKey's chain/subject check. If an
+// of it, along with fetchVerifiedCertKey's subject check (no chain build —
+// sesevents.go's file comment records why there is none to build for SNS's
+// own signing certificate). If an
// attacker can point it at a host they control, they sign their own message
// with their own certificate and it verifies — which means forging the
// bounce events that trigger an automatic pause, on any tenant.
@@ -586,3 +593,85 @@ func TestVerifiedCertKeyServesFromCache(t *testing.T) {
t.Fatal("verifiedCertKey returned a different key than the one cached")
}
}
+
+// ---- event-to-domain attribution (I6, review follow-up) --------------------
+//
+// ParseEvent attributes an event by the domain of mail.source. If SES
+// reports the REWRITTEN envelope — mail-<region>.<domain>, the custom MAIL
+// FROM MX target (spec §7) — rather than the sender-supplied From, an exact
+// match against DomainRecord.Domain alone attributes the event to nothing,
+// and it is silently dropped from the counts that enforce the cap and trip
+// the abuse pause: a second, independent way spec §9 stops working.
+
+func TestDomainMatchesEventAcceptsTheIdentitysOwnDomain(t *testing.T) {
+ d := &mail.DomainRecord{Domain: "seapointish.bag.oncarlos.com"}
+ if !domainMatchesEvent(d, "seapointish.bag.oncarlos.com") {
+ t.Fatal("the identity's own domain must match")
+ }
+}
+
+func TestDomainMatchesEventAcceptsTheRecordedMailFromForAnyRegion(t *testing.T) {
+ d := &mail.DomainRecord{
+ Domain: "seapointish.bag.oncarlos.com",
+ Regions: map[string]*mail.RegionState{
+ "eu-west-1": {MailFrom: "mail-eu-west-1.seapointish.bag.oncarlos.com"},
+ "us-east-1": {MailFrom: "mail-us-east-1.seapointish.bag.oncarlos.com"},
+ },
+ }
+ if !domainMatchesEvent(d, "mail-eu-west-1.seapointish.bag.oncarlos.com") {
+ t.Fatal("the eu-west-1 MAIL FROM subdomain must match")
+ }
+ if !domainMatchesEvent(d, "mail-us-east-1.seapointish.bag.oncarlos.com") {
+ t.Fatal("a second region's MAIL FROM subdomain must match too")
+ }
+}
+
+func TestDomainMatchesEventRefusesAnUnrelatedDomain(t *testing.T) {
+ d := &mail.DomainRecord{
+ Domain: "seapointish.bag.oncarlos.com",
+ Regions: map[string]*mail.RegionState{"eu-west-1": {MailFrom: "mail-eu-west-1.seapointish.bag.oncarlos.com"}},
+ }
+ if domainMatchesEvent(d, "unrelated.example.com") {
+ t.Fatal("an unrelated domain must not match — this must stay a match against RECORDED values, never a prefix-strip heuristic")
+ }
+}
+
+// TestRecordAndEnforceMailAttributesTheRewrittenMailFromEnvelope is the
+// end-to-end proof: an SES event reporting the rewritten MAIL FROM envelope
+// (not the identity's own domain) must still land in this app's counts.
+func TestRecordAndEnforceMailAttributesTheRewrittenMailFromEnvelope(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+ now := time.Now()
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "seapointish", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ rec := &mail.DomainRecord{
+ Domain: "seapointish.bag.oncarlos.com",
+ Kind: mail.KindPlatform,
+ Apps: []string{"seapointish"},
+ Regions: map[string]*mail.RegionState{
+ "eu-west-1": {
+ IdentityARN: "arn:aws:ses:eu-west-1:1:identity/seapointish.bag.oncarlos.com",
+ MailFrom: "mail-eu-west-1.seapointish.bag.oncarlos.com",
+ Status: mail.StatusVerified,
+ },
+ },
+ CreatedAt: now,
+ }
+ if _, err := mail.PutDomain(ctx, fx.rel, release.OpsAccount, rec, ""); err != nil {
+ t.Fatal(err)
+ }
+
+ ev := mail.Event{Type: mail.EventSend, Domain: "mail-eu-west-1.seapointish.bag.oncarlos.com", At: now}
+ fx.srv.recordAndEnforceMail(ctx, ev)
+
+ day := now.UTC().Format("2006-01-02")
+ counts, err := fx.srv.store.MailCounts(ctx, release.OpsAccount, "seapointish", day)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if counts.Sends != 1 {
+ t.Fatalf("Sends = %d, want 1 — the rewritten MAIL FROM envelope must still attribute to the app", counts.Sends)
+ }
+}
diff --git a/internal/mail/aws.go b/internal/mail/aws.go
index 2e5b933de..cf03fe7f3 100644
--- a/internal/mail/aws.go
+++ b/internal/mail/aws.go
@@ -59,6 +59,7 @@ type sesAPI interface {
DeleteEmailIdentity(context.Context, *sesv2.DeleteEmailIdentityInput, ...func(*sesv2.Options)) (*sesv2.DeleteEmailIdentityOutput, error)
CreateConfigurationSet(context.Context, *sesv2.CreateConfigurationSetInput, ...func(*sesv2.Options)) (*sesv2.CreateConfigurationSetOutput, error)
CreateConfigurationSetEventDestination(context.Context, *sesv2.CreateConfigurationSetEventDestinationInput, ...func(*sesv2.Options)) (*sesv2.CreateConfigurationSetEventDestinationOutput, error)
+ PutEmailIdentityConfigurationSetAttributes(context.Context, *sesv2.PutEmailIdentityConfigurationSetAttributesInput, ...func(*sesv2.Options)) (*sesv2.PutEmailIdentityConfigurationSetAttributesOutput, error)
}
// iamAPI is the subset of *iam.Client awsProvisioner calls.
@@ -222,6 +223,24 @@ func (p *awsProvisioner) EnsureConfigSet(ctx context.Context, region, name, snsT
return nil
}
+// SetIdentityConfigSet binds domain's identity to configSet — see the
+// Provisioner interface's doc comment on why this call, separate from
+// EnsureConfigSet, is what actually makes SES emit an event for a send
+// (C1, review follow-up).
+func (p *awsProvisioner) SetIdentityConfigSet(ctx context.Context, region, domain, configSet string) error {
+ if err := p.checkRegion(region); err != nil {
+ return err
+ }
+ _, err := p.ses.PutEmailIdentityConfigurationSetAttributes(ctx, &sesv2.PutEmailIdentityConfigurationSetAttributesInput{
+ EmailIdentity: aws.String(domain),
+ ConfigurationSetName: aws.String(configSet),
+ })
+ if err != nil {
+ return fmt.Errorf("mail: set configuration set %q on identity %q: %w", configSet, domain, err)
+ }
+ return nil
+}
+
// EnsureIdentity creates the identity with Easy DKIM and sets the custom
// MAIL FROM, idempotently.
//
diff --git a/internal/mail/aws_test.go b/internal/mail/aws_test.go
index ae714ffe9..2fef04dea 100644
--- a/internal/mail/aws_test.go
+++ b/internal/mail/aws_test.go
@@ -63,6 +63,7 @@ type stubSES struct {
createEmailIdentity func(*sesv2.CreateEmailIdentityInput) (*sesv2.CreateEmailIdentityOutput, error)
getEmailIdentity func(*sesv2.GetEmailIdentityInput) (*sesv2.GetEmailIdentityOutput, error)
putEmailIdentityMailFromAttributes func(*sesv2.PutEmailIdentityMailFromAttributesInput) (*sesv2.PutEmailIdentityMailFromAttributesOutput, error)
+ putEmailIdentityConfigurationSet func(*sesv2.PutEmailIdentityConfigurationSetAttributesInput) (*sesv2.PutEmailIdentityConfigurationSetAttributesOutput, error)
}
func (s *stubSES) CreateTenant(_ context.Context, in *sesv2.CreateTenantInput, _ ...func(*sesv2.Options)) (*sesv2.CreateTenantOutput, error) {
@@ -105,6 +106,13 @@ func (s *stubSES) CreateConfigurationSetEventDestination(_ context.Context, in *
return &sesv2.CreateConfigurationSetEventDestinationOutput{}, nil
}
+func (s *stubSES) PutEmailIdentityConfigurationSetAttributes(_ context.Context, in *sesv2.PutEmailIdentityConfigurationSetAttributesInput, _ ...func(*sesv2.Options)) (*sesv2.PutEmailIdentityConfigurationSetAttributesOutput, error) {
+ if s.putEmailIdentityConfigurationSet != nil {
+ return s.putEmailIdentityConfigurationSet(in)
+ }
+ return &sesv2.PutEmailIdentityConfigurationSetAttributesOutput{}, nil
+}
+
var _ sesAPI = (*stubSES)(nil)
// TestEnsureIdentityAlreadyExistsReadsBackTokensAndComposesTheARN pins the
@@ -170,6 +178,51 @@ func TestEnsureIdentityRefusesNoTokens(t *testing.T) {
}
}
+// TestSetIdentityConfigSetCallsThroughWithTheRightNames (C1, review
+// follow-up): this is the ONLY call in the whole package that actually
+// binds an identity to the configuration set carrying SES's event
+// destination — without it SES never emits an event for that identity, no
+// matter how many messages it sends. Pinned here at the AWS-facing layer,
+// not just against the in-memory fake, so a call built with the identity
+// and set-name arguments swapped (or dropped) would fail this test.
+func TestSetIdentityConfigSetCallsThroughWithTheRightNames(t *testing.T) {
+ var gotIdentity, gotConfigSet string
+ stub := &stubSES{
+ putEmailIdentityConfigurationSet: func(in *sesv2.PutEmailIdentityConfigurationSetAttributesInput) (*sesv2.PutEmailIdentityConfigurationSetAttributesOutput, error) {
+ gotIdentity = aws.ToString(in.EmailIdentity)
+ gotConfigSet = aws.ToString(in.ConfigurationSetName)
+ return &sesv2.PutEmailIdentityConfigurationSetAttributesOutput{}, nil
+ },
+ }
+ p := &awsProvisioner{ses: stub, account: "123456789012", region: "eu-west-1"}
+
+ if err := p.SetIdentityConfigSet(context.Background(), "eu-west-1", "example.com", "carlos-bag"); err != nil {
+ t.Fatalf("SetIdentityConfigSet: %v", err)
+ }
+ if gotIdentity != "example.com" {
+ t.Fatalf("EmailIdentity = %q, want %q", gotIdentity, "example.com")
+ }
+ if gotConfigSet != "carlos-bag" {
+ t.Fatalf("ConfigurationSetName = %q, want %q", gotConfigSet, "carlos-bag")
+ }
+}
+
+// A backend failure (e.g. the identity does not exist) must propagate, not
+// be swallowed as though the binding succeeded — a silently-not-bound
+// identity is exactly the C1 defect this whole verb exists to close.
+func TestSetIdentityConfigSetPropagatesBackendError(t *testing.T) {
+ stub := &stubSES{
+ putEmailIdentityConfigurationSet: func(*sesv2.PutEmailIdentityConfigurationSetAttributesInput) (*sesv2.PutEmailIdentityConfigurationSetAttributesOutput, error) {
+ return nil, &sesv2types.NotFoundException{Message: aws.String("no such identity")}
+ },
+ }
+ p := &awsProvisioner{ses: stub, account: "123456789012", region: "eu-west-1"}
+
+ if err := p.SetIdentityConfigSet(context.Background(), "eu-west-1", "example.com", "carlos-bag"); err == nil {
+ t.Fatal("SetIdentityConfigSet swallowed a backend error")
+ }
+}
+
// wellFormedIdentityStub returns a stubSES whose CreateEmailIdentity and
// PutEmailIdentityMailFromAttributes both succeed, with three non-empty
// DKIM tokens. Used by the region-guard tests below so that success or
diff --git a/internal/mail/mail.go b/internal/mail/mail.go
index 0d4a8611d..946a254d9 100644
--- a/internal/mail/mail.go
+++ b/internal/mail/mail.go
@@ -55,14 +55,34 @@ type GrantBlock struct {
// secret; the password is derived at delivery time and lands only in the
// app's sealed config (spec §4). Active is the pause state (spec §9).
type Provisioned struct {
- IAMUser string `json:"iam_user"`
- UserARN string `json:"user_arn"`
- KeyID string `json:"key_id"`
- PrevKeyID string `json:"prev_key_id,omitempty"`
- Active bool `json:"active"`
+ IAMUser string `json:"iam_user"`
+ UserARN string `json:"user_arn"`
+ KeyID string `json:"key_id"`
+ PrevKeyID string `json:"prev_key_id,omitempty"`
+ Active bool `json:"active"`
+ // PauseKind distinguishes an automatic CAP pause from an automatic RATE
+ // pause and from a manual operator pause (I3, review follow-up) —
+ // additive field, so a record written before it existed decodes it as
+ // "", which correctly reads as "not a cap pause, do not auto-resume
+ // it". A CAP pause (PauseKindCap) is a PER-DAY limit and clears itself
+ // on the UTC day roll (internal/console/mailcounts.go's
+ // enforceMailLimits); a RATE pause (PauseKindRate) or a manual pause
+ // (PauseKind == "") never auto-resumes — SES's account-review
+ // thresholds are not a daily allowance, and a manual pause is an
+ // operator's decision to undo, not a timer's. Always cleared (set back
+ // to "") on resume, whatever resumed it.
+ PauseKind string `json:"pause_kind,omitempty"`
At time.Time `json:"at"`
}
+// The two automatic pause kinds SetPaused records (spec §9, I3 review
+// follow-up). A manual (operator-driven) pause carries neither — see
+// Provisioned.PauseKind's own doc comment.
+const (
+ PauseKindCap = "cap"
+ PauseKindRate = "rate"
+)
+
// Standalone is a displayed credential for something not running on CARLOS.
//
// It gets its OWN IAM principal, and that is forced rather than stylistic:
diff --git a/internal/mail/memprovisioner.go b/internal/mail/memprovisioner.go
index f8e693e39..feb900b24 100644
--- a/internal/mail/memprovisioner.go
+++ b/internal/mail/memprovisioner.go
@@ -30,6 +30,11 @@ type MemIdentity struct {
DKIMTokens []string
MailFrom string
Status Status
+ // ConfigSet is stamped by SetIdentityConfigSet, mirroring
+ // PutEmailIdentityConfigurationSetAttributes (C1, review follow-up).
+ // Empty until then — including right after EnsureIdentity, which never
+ // sets it itself.
+ ConfigSet string
}
type MemProvisioner struct {
@@ -127,6 +132,23 @@ func (p *MemProvisioner) EnsureIdentity(_ context.Context, region, domain, mailF
return Identity{ARN: id.ARN, DKIMTokens: id.DKIMTokens}, nil
}
+// SetIdentityConfigSet models PutEmailIdentityConfigurationSetAttributes:
+// it stamps ConfigSet on an EXISTING identity and errors if there is none —
+// the real backend would 404 too (C1, review follow-up).
+func (p *MemProvisioner) SetIdentityConfigSet(_ context.Context, region, domain, configSet string) error {
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ if err := p.takeFailure("SetIdentityConfigSet"); err != nil {
+ return err
+ }
+ got, ok := p.Identities[identityKey(region, domain)]
+ if !ok {
+ return fmt.Errorf("mail: no identity for %s in %s to set a configuration set on", domain, region)
+ }
+ got.ConfigSet = configSet
+ return nil
+}
+
func (p *MemProvisioner) IdentityStatus(_ context.Context, region, domain string) (Status, error) {
p.mu.Lock()
defer p.mu.Unlock()
diff --git a/internal/mail/provisioner.go b/internal/mail/provisioner.go
index 1889f3a27..9ab392ed4 100644
--- a/internal/mail/provisioner.go
+++ b/internal/mail/provisioner.go
@@ -43,6 +43,16 @@ type Provisioner interface {
// EnsureConfigSet creates the event destination that feeds bounce and
// complaint ingestion (spec §9).
EnsureConfigSet(ctx context.Context, region, name, snsTopicARN string) error
+ // SetIdentityConfigSet binds an identity to the configuration set that
+ // carries SES's event destination (spec §9; C1, review follow-up).
+ // EnsureConfigSet alone creates the SET — it does not touch any
+ // identity. SES only emits events (send, bounce, complaint) for a
+ // message sent WITH a configuration set, so an identity that never
+ // gets this call generates NO events, ever, no matter how many
+ // messages it sends: every count stays zero forever, the daily cap
+ // never enforces, and the automatic abuse pause never fires. Called
+ // once per (region, domain) — idempotent, like every verb here.
+ SetIdentityConfigSet(ctx context.Context, region, domain, configSet string) error
// EnsureIdentity creates the domain identity, enables Easy DKIM and
// sets the custom MAIL FROM subdomain, all idempotently.
diff --git a/internal/mail/walk.go b/internal/mail/walk.go
index b222d9e8c..6ba3cf7c6 100644
--- a/internal/mail/walk.go
+++ b/internal/mail/walk.go
@@ -211,6 +211,22 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec.
if err := p.EnsureConfigSet(ctx, d.Region, configSet, d.SNSTopicARN); err != nil {
return nil, nil, fmt.Errorf("mail: ensure config set %s: %w", configSet, err)
}
+ // C1 (review follow-up): EnsureConfigSet only creates the SET and
+ // its SNS event destination — nothing before this bound the
+ // IDENTITY just ensured above to it, and SES only emits events for
+ // a message sent WITH a configuration set. Without this call every
+ // send/bounce/complaint count for this identity stays zero
+ // forever: the daily cap never enforces and the automatic abuse
+ // pause — the one thing stopping a single tenant's stale list
+ // putting the whole shared SES account under review — never
+ // fires. Bound HERE, inside the `d.SNSTopicARN != ""` branch: the
+ // SNS-less deployment deliberately records an empty configSet
+ // (this function's own doc comment on Deployment), and calling
+ // this with an empty name would either error at a real backend or
+ // silently bind to nothing.
+ if err := p.SetIdentityConfigSet(ctx, d.Region, domain, configSet); err != nil {
+ return nil, nil, fmt.Errorf("mail: bind identity %s to config set %s: %w", domain, configSet, err)
+ }
}
records := RecordSet(domain, d.Region, ident.DKIMTokens, d.DMARCRua)
@@ -410,7 +426,18 @@ func grantedDomains(ctx context.Context, s release.Store, acct, app string) ([]*
// regardless — a disagreement that is permanent, because nothing else ever
// re-checks it. _PASS is sealed, so its value cannot be compared this way;
// Kind is the best this function can do for it.
-func EnvDelivered(ctx context.Context, s release.Store, acct, app, prefix, wantKeyID string) ([]string, error) {
+//
+// wantFrom is the From address delivery is supposed to be carrying in
+// _FROM — normally rec.Declared.From. Pass "" to skip the value check, the
+// same convention wantKeyID uses. _FROM's VALUE is compared for the
+// identical reason _USER's is (I1, review follow-up): Declare freely
+// overwrites Declared.From at any time (only the env PREFIX is locked once
+// delivered), and before this check nothing ever re-verified that a
+// changed From actually reached delivery — a re-declare followed by a
+// re-enable left the record and the delivered value disagreeing
+// permanently, the same failure class the _USER check exists to catch,
+// just for the one delivered variable it had not been applied to yet.
+func EnvDelivered(ctx context.Context, s release.Store, acct, app, prefix, wantKeyID, wantFrom string) ([]string, error) {
man, err := appconfig.Get(ctx, s, acct, app)
if err != nil {
return nil, err
@@ -425,23 +452,31 @@ func EnvDelivered(ctx context.Context, s release.Store, acct, app, prefix, wantK
}
check(host, "env")
check(port, "env")
- check(from, "env")
check(pass, "secret")
if v, ok := man.Vars[user]; !ok || v.Kind != "env" || (wantKeyID != "" && v.Value != wantKeyID) {
missing = append(missing, user)
}
+ if v, ok := man.Vars[from]; !ok || v.Kind != "env" || (wantFrom != "" && v.Value != wantFrom) {
+ missing = append(missing, from)
+ }
sort.Strings(missing)
return missing, nil
}
// Enable provisions the declared app's credential and delivers it.
//
-// Idempotent: a healthy provisioned app makes this a no-op — no fresh key,
-// no rewrite — because minting on every pass would burn IAM's two-key
-// ceiling and churn every instance's config. "Healthy" means the record
-// says provisioned AND every delivered variable is present, of the right
-// kind; anything less and the walk runs, because a missing variable means
-// the app cannot send.
+// Idempotent, but NOT all-or-nothing about it (C2, review follow-up): the
+// IAM user's policy is rewritten from the CURRENT granted domain set on
+// EVERY call, wholesale, cheaply — that is what makes attaching (or
+// losing) a domain take effect on a plain re-`enable` with no rotation.
+// Only the expensive half — mint a fresh key and re-deliver config — is
+// skipped when a healthy provisioned app makes it a no-op, because minting
+// on every pass would burn IAM's two-key ceiling and churn every
+// instance's config. "Healthy" means the record says provisioned AND every
+// delivered variable is present, of the right kind AND value where one is
+// checked; anything less and the rest of the walk runs too, because a
+// missing or stale variable means the app cannot send what the record
+// promises.
func Enable(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.PublicKey,
d Deployment, acct, app string, now time.Time) (*Record, error) {
@@ -478,9 +513,31 @@ func Enable(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.P
rec.Declared.From, strings.Join(names, ", "))
}
- // Idempotence short-circuit.
+ // C2 (review follow-up): EnsureUser runs UNCONDITIONALLY, before the
+ // idempotence short-circuit below — it is no longer gated behind it.
+ // EnsureUser is the ONLY place the delivered credential's policy is
+ // written (PutUserPolicy overwrites wholesale, spec §4), and arns/names
+ // above are recomputed from the CURRENT granted domain set on every
+ // call. Before this fix, the short-circuit returned before EnsureUser
+ // ever ran again: adding and verifying a second domain, then
+ // re-enabling, left the policy naming only the first domain forever —
+ // spec §4 promises the opposite ("attaching a custom domain adds an ARN
+ // and rewrites the policy without rotating a credential") — and the
+ // symmetric shrink (a domain leaving the granted set) left a departed
+ // ARN standing in the policy just as permanently. EnsureUser is cheap
+ // to re-run (idempotent CreateUser, one PutUserPolicy) — it is the reap
+ // and mint below that spend IAM's two-key ceiling, and THOSE are what
+ // the short-circuit still has to gate.
+ user := UserName(acct, app)
+ arn, err := p.EnsureUser(ctx, user, UserPath, arns, names)
+ if err != nil {
+ return nil, fmt.Errorf("mail: ensure user %s: %w", user, err)
+ }
+
+ // Idempotence short-circuit: governs ONLY the expensive mint-and-deliver
+ // half from here down.
if rec.Provisioned != nil {
- missing, err := EnvDelivered(ctx, s, acct, app, rec.Declared.EnvPrefix, rec.Provisioned.KeyID)
+ missing, err := EnvDelivered(ctx, s, acct, app, rec.Declared.EnvPrefix, rec.Provisioned.KeyID, rec.Declared.From)
if err != nil {
return nil, err
}
@@ -500,11 +557,6 @@ func Enable(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.P
active = rec.Provisioned.Active
}
- user := UserName(acct, app)
- arn, err := p.EnsureUser(ctx, user, UserPath, arns, names)
- if err != nil {
- return nil, fmt.Errorf("mail: ensure user %s: %w", user, err)
- }
// Reap before minting: reaching here means delivery is missing, so any
// current key is either undelivered (its secret died with a failed
// config write) or stranded by a crash between mint and record. No
@@ -753,8 +805,17 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app
// is its OWN IAM user, so there is no two-key ceiling interaction, and
// skipping them would leave a tenant paused for abuse still sending from the
// same verified domain through, say, a laptop credential.
+//
+// kind records WHICH automatic pause this is (mail.PauseKindCap or
+// mail.PauseKindRate) so mailcounts.go's enforceMailLimits can tell a
+// per-day cap pause — which it auto-resumes on the UTC day roll — from a
+// rate pause or a manual one, which it never does (I3, review follow-up).
+// Pass "" for a manual pause (the operator door) and for every resume
+// (paused == false): PauseKind is always cleared back to "" on resume,
+// whatever resumed it, so a record never carries a stale kind from a pause
+// that is already lifted.
func SetPaused(ctx context.Context, s release.Store, p Provisioner,
- acct, app string, paused bool, now time.Time) (*Record, error) {
+ acct, app string, paused bool, kind string, now time.Time) (*Record, error) {
rec, etag, err := Get(ctx, s, acct, app)
if err != nil {
@@ -778,6 +839,11 @@ func SetPaused(ctx context.Context, s release.Store, p Provisioner,
}
}
rec.Provisioned.Active = !paused
+ if paused {
+ rec.Provisioned.PauseKind = kind
+ } else {
+ rec.Provisioned.PauseKind = ""
+ }
rec.Provisioned.At = now
if _, err := Put(ctx, s, acct, app, rec, etag); err != nil {
return nil, err
diff --git a/internal/mail/walk_domain_test.go b/internal/mail/walk_domain_test.go
index 013992c94..50a9edf08 100644
--- a/internal/mail/walk_domain_test.go
+++ b/internal/mail/walk_domain_test.go
@@ -218,6 +218,56 @@ func TestEnsureDomainLeavesConfigSetEmptyWithoutSNS(t *testing.T) {
}
}
+// C1 (review follow-up): recording a config set NAME in the domain record
+// is not the same as SES actually emitting events for that identity — SES
+// only does that for a send carrying a configuration set, and nothing
+// before this task ever bound the identity to one. A domain ensured
+// against a deployment WITH an SNS topic must have its identity's default
+// configuration set stamped AT THE PROVISIONER, not just recorded in the
+// bucket.
+func TestEnsureDomainWithSNSStampsTheIdentitysConfigSet(t *testing.T) {
+ ctx := context.Background()
+ s, p, w := release.NewDirStore(t.TempDir()), NewMemProvisioner("1"), dnsrec.NewMemWriter()
+ now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC)
+ d := testDeployment() // carries an SNSTopicARN
+
+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "bag", "seapointish", "seapointish.bag.oncarlos.com", now); err != nil {
+ t.Fatalf("EnsureDomain: %v", err)
+ }
+ id, ok := p.Identities[identityKey(d.Region, "seapointish.bag.oncarlos.com")]
+ if !ok {
+ t.Fatal("no identity recorded at the provisioner")
+ }
+ if want := ConfigSetName("bag"); id.ConfigSet != want {
+ t.Fatalf("identity ConfigSet = %q, want %q — SetIdentityConfigSet was never called, so SES "+
+ "would never emit an event for this identity and the daily cap/abuse pause would never fire",
+ id.ConfigSet, want)
+ }
+}
+
+// The symmetric SNS-less case: without a configuration set to bind to,
+// SetIdentityConfigSet must not be called at all — calling it with an
+// empty name would either error at a real backend or silently bind the
+// identity to nothing.
+func TestEnsureDomainWithoutSNSStampsNoIdentityConfigSet(t *testing.T) {
+ ctx := context.Background()
+ s, p, w := release.NewDirStore(t.TempDir()), NewMemProvisioner("1"), dnsrec.NewMemWriter()
+ now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC)
+ d := testDeployment()
+ d.SNSTopicARN = ""
+
+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "bag", "seapointish", "seapointish.bag.oncarlos.com", now); err != nil {
+ t.Fatalf("EnsureDomain: %v", err)
+ }
+ id, ok := p.Identities[identityKey(d.Region, "seapointish.bag.oncarlos.com")]
+ if !ok {
+ t.Fatal("no identity recorded at the provisioner")
+ }
+ if id.ConfigSet != "" {
+ t.Fatalf("identity ConfigSet = %q, want empty — no SNS topic means no config set to bind to", id.ConfigSet)
+ }
+}
+
// acct reaches EnsureTenant — a real backend call — before anything checks
// it, unless validation gates the walk before that call. app is persisted
// into the record and later turned into an IAM principal name by other
diff --git a/internal/mail/walk_enable_test.go b/internal/mail/walk_enable_test.go
index 11bfddf9f..9e861d953 100644
--- a/internal/mail/walk_enable_test.go
+++ b/internal/mail/walk_enable_test.go
@@ -3,6 +3,7 @@ package mail
import (
"context"
"crypto/ecdh"
+ "strings"
"testing"
"time"
@@ -263,7 +264,7 @@ func TestEnvDeliveredAndEnableCatchAStaleRecordedKeyID(t *testing.T) {
t.Fatalf("Put: %v", err)
}
- missing, err := EnvDelivered(ctx, s, "bag", "seapointish", "SEAPOINTISH_SMTP", "AKIASTALE00000000")
+ missing, err := EnvDelivered(ctx, s, "bag", "seapointish", "SEAPOINTISH_SMTP", "AKIASTALE00000000", "")
if err != nil {
t.Fatalf("EnvDelivered: %v", err)
}
@@ -282,3 +283,135 @@ func TestEnvDeliveredAndEnableCatchAStaleRecordedKeyID(t *testing.T) {
t.Fatal("re-walk should mint a fresh key, not silently reuse the originally delivered one")
}
}
+
+// C2 (review follow-up): the idempotence short-circuit used to return
+// BEFORE EnsureUser — the only place the delivered credential's policy is
+// written — so a second domain added and verified after the first enable
+// never reached the policy on a plain re-`enable`. Spec §4 states the
+// opposite explicitly: "attaching a custom domain adds an ARN and rewrites
+// the policy without rotating a credential."
+func TestEnableRewritesThePolicyWhenASecondDomainIsAdded(t *testing.T) {
+ ctx, s, p, pub, now := enableFixture(t)
+ first, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now)
+ if err != nil {
+ t.Fatalf("first Enable: %v", err)
+ }
+ firstKeyID := first.Provisioned.KeyID
+
+ second := "second.bag.oncarlos.com"
+ if _, _, err := EnsureDomain(ctx, s, p, dnsrec.NewMemWriter(), testDeployment(),
+ "bag", "seapointish", second, now); err != nil {
+ t.Fatalf("EnsureDomain second: %v", err)
+ }
+ p.SetIdentityStatus("eu-west-1", second, Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"})
+ if _, err := VerifyDomain(ctx, s, p, "bag", second, "eu-west-1", now); err != nil {
+ t.Fatalf("VerifyDomain second: %v", err)
+ }
+
+ rewalked, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now)
+ if err != nil {
+ t.Fatalf("re-enable: %v", err)
+ }
+ if rewalked.Provisioned.KeyID != firstKeyID {
+ t.Fatal("re-enable minted a fresh key — the short-circuit must still govern the expensive half")
+ }
+ policy := p.Users[UserName("bag", "seapointish")].Policy
+ for _, want := range []string{
+ "seapointish.bag.oncarlos.com", second, "*@seapointish.bag.oncarlos.com", "*@" + second,
+ } {
+ if !strings.Contains(policy, want) {
+ t.Fatalf("policy after re-enable missing %q:\n%s", want, policy)
+ }
+ }
+}
+
+// The symmetric shrink case: a domain that WAS granted and verified stops
+// being verified (e.g. SES reports its DKIM/MAIL FROM failed), narrowing
+// grantedDomains on the NEXT Enable call. The surviving policy must not go
+// on naming the departed ARN and From pattern — the same defect C2 fixes,
+// approached from the other direction.
+func TestEnableRewritesThePolicyWhenADomainStopsVerifying(t *testing.T) {
+ ctx, s, p, pub, now := enableFixture(t)
+ second := "second.bag.oncarlos.com"
+ if _, _, err := EnsureDomain(ctx, s, p, dnsrec.NewMemWriter(), testDeployment(),
+ "bag", "seapointish", second, now); err != nil {
+ t.Fatalf("EnsureDomain second: %v", err)
+ }
+ p.SetIdentityStatus("eu-west-1", second, Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"})
+ if _, err := VerifyDomain(ctx, s, p, "bag", second, "eu-west-1", now); err != nil {
+ t.Fatalf("VerifyDomain second: %v", err)
+ }
+
+ first, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now)
+ if err != nil {
+ t.Fatalf("first Enable: %v", err)
+ }
+ policy := p.Users[UserName("bag", "seapointish")].Policy
+ if !strings.Contains(policy, second) {
+ t.Fatalf("policy before the regression missing %q:\n%s", second, policy)
+ }
+
+ // second regresses: SES now reports it FAILED.
+ p.SetIdentityStatus("eu-west-1", second, Status{DKIM: "FAILED", MailFrom: "FAILED", Sending: "PENDING"})
+ if _, err := VerifyDomain(ctx, s, p, "bag", second, "eu-west-1", now); err != nil {
+ t.Fatalf("VerifyDomain regression: %v", err)
+ }
+
+ rewalked, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now)
+ if err != nil {
+ t.Fatalf("re-enable after regression: %v", err)
+ }
+ if rewalked.Provisioned.KeyID != first.Provisioned.KeyID {
+ t.Fatal("re-enable minted a fresh key — the short-circuit must still govern the expensive half")
+ }
+ policy = p.Users[UserName("bag", "seapointish")].Policy
+ if strings.Contains(policy, second) {
+ t.Fatalf("policy after the regression still names the departed domain %q:\n%s", second, policy)
+ }
+ if !strings.Contains(policy, "seapointish.bag.oncarlos.com") {
+ t.Fatalf("policy after the regression lost the SURVIVING domain too:\n%s", policy)
+ }
+}
+
+// I1 (review follow-up): the exact failure class the _USER staleness check
+// was added to catch, applied to the OTHER unsealed variable it was never
+// extended to. Declare freely overwrites Declared.From at any time (only
+// the env prefix is locked once delivered); a re-declare followed by a
+// re-enable must re-deliver the new From, not leave the stale one standing
+// forever because nothing ever compared _FROM's VALUE.
+func TestEnableRedeliversAChangedFromAddress(t *testing.T) {
+ ctx, s, p, pub, now := enableFixture(t)
+ first, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now)
+ if err != nil {
+ t.Fatalf("first Enable: %v", err)
+ }
+
+ newFrom := "admin@seapointish.bag.oncarlos.com"
+ if _, err := Declare(ctx, s, "bag", "seapointish", "SEAPOINTISH_SMTP", newFrom, "paul@rslw.com", now); err != nil {
+ t.Fatalf("re-declare: %v", err)
+ }
+
+ missing, err := EnvDelivered(ctx, s, "bag", "seapointish", "SEAPOINTISH_SMTP", first.Provisioned.KeyID, newFrom)
+ if err != nil {
+ t.Fatalf("EnvDelivered: %v", err)
+ }
+ if !containsString(missing, "SEAPOINTISH_SMTP_FROM") {
+ t.Fatalf("missing = %v, want SEAPOINTISH_SMTP_FROM reported after a re-declare changed it", missing)
+ }
+
+ second, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now)
+ if err != nil {
+ t.Fatalf("re-enable: %v", err)
+ }
+ if second.Declared.From != newFrom {
+ t.Fatalf("record's Declared.From = %q, want %q", second.Declared.From, newFrom)
+ }
+ man, err := appconfig.Get(ctx, s, "bag", "seapointish")
+ if err != nil {
+ t.Fatalf("appconfig.Get: %v", err)
+ }
+ fromVar, ok := man.Vars["SEAPOINTISH_SMTP_FROM"]
+ if !ok || fromVar.Value != newFrom {
+ t.Fatalf("delivered SEAPOINTISH_SMTP_FROM = %+v, want value %q", fromVar, newFrom)
+ }
+}
diff --git a/internal/mail/walk_lifecycle_test.go b/internal/mail/walk_lifecycle_test.go
index dabdc6f48..5c6b25528 100644
--- a/internal/mail/walk_lifecycle_test.go
+++ b/internal/mail/walk_lifecycle_test.go
@@ -50,7 +50,7 @@ func TestRotateDoesNotLiftAPauseAtIAM(t *testing.T) {
if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now); err != nil {
t.Fatalf("Enable: %v", err)
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now); err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
@@ -100,7 +100,7 @@ func TestEnableRepairDoesNotLiftAPauseAtIAM(t *testing.T) {
if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now); err != nil {
t.Fatalf("first Enable: %v", err)
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now); err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
// Clear a delivered var so the idempotence short-circuit cannot fire —
@@ -136,7 +136,7 @@ func TestPauseAndResumeToggleTheKeyNotThePolicy(t *testing.T) {
user := UserName("bag", "seapointish")
policyBefore := p.Users[user].Policy
- paused, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now)
+ paused, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now)
if err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
@@ -150,7 +150,7 @@ func TestPauseAndResumeToggleTheKeyNotThePolicy(t *testing.T) {
t.Fatal("the pause rewrote the policy — it must only toggle the key")
}
- resumed, err := SetPaused(ctx, s, p, "bag", "seapointish", false, now)
+ resumed, err := SetPaused(ctx, s, p, "bag", "seapointish", false, "", now)
if err != nil {
t.Fatalf("SetPaused(false): %v", err)
}
@@ -175,7 +175,7 @@ func TestPauseTogglesBothKeysDuringARotationOverlap(t *testing.T) {
t.Fatal("fixture broken: expected a PrevKeyID after Rotate")
}
- paused, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now)
+ paused, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now)
if err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
@@ -217,7 +217,7 @@ func TestPauseReachesStandaloneCredentials(t *testing.T) {
t.Fatal("fixture broken: standalone entry missing from record")
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now); err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
su, ok := p.Users[standaloneUser]
@@ -232,7 +232,7 @@ func TestPauseReachesStandaloneCredentials(t *testing.T) {
t.Fatal("pause did not reach the standalone credential — it can still send")
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", false, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", false, "", now); err != nil {
t.Fatalf("SetPaused(false): %v", err)
}
su, ok = p.Users[standaloneUser]
@@ -259,7 +259,7 @@ func TestCreateStandaloneDoesNotBypassAPause(t *testing.T) {
if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now); err != nil {
t.Fatalf("Enable: %v", err)
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now); err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
@@ -290,7 +290,7 @@ func TestCreateStandaloneReMintWhilePausedStaysInactiveAndResumeReactivates(t *t
if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now); err != nil {
t.Fatalf("Enable: %v", err)
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", true, "", now); err != nil {
t.Fatalf("SetPaused(true): %v", err)
}
if _, err := CreateStandalone(ctx, s, p, testDeployment(), "bag", "seapointish",
@@ -316,7 +316,7 @@ func TestCreateStandaloneReMintWhilePausedStaysInactiveAndResumeReactivates(t *t
t.Fatal("a re-mint while paused produced a LIVE key")
}
- if _, err := SetPaused(ctx, s, p, "bag", "seapointish", false, now); err != nil {
+ if _, err := SetPaused(ctx, s, p, "bag", "seapointish", false, "", now); err != nil {
t.Fatalf("SetPaused(false): %v", err)
}
su, ok = p.Users[standaloneUser]
diff --git a/mail_cli.go b/mail_cli.go
index 12f72ab8a..b4b123557 100644
--- a/mail_cli.go
+++ b/mail_cli.go
@@ -179,11 +179,17 @@ type apiEmailGrant struct {
}
type apiEmailProvisioned struct {
- IAMUser string `json:"iam_user"`
- UserARN string `json:"user_arn"`
- KeyID string `json:"key_id"`
- PrevKeyID string `json:"prev_key_id,omitempty"`
- Active bool `json:"active"`
+ IAMUser string `json:"iam_user"`
+ UserARN string `json:"user_arn"`
+ KeyID string `json:"key_id"`
+ PrevKeyID string `json:"prev_key_id,omitempty"`
+ Active bool `json:"active"`
+ // PauseKind distinguishes a per-day CAP pause (clears itself on the
+ // UTC day roll) from a RATE pause or a manual operator pause (neither
+ // ever auto-resumes) — mail.Provisioned.PauseKind's wire form (I3,
+ // review follow-up). "" on an active record, on a manual pause, and on
+ // a record written before this field existed.
+ PauseKind string `json:"pause_kind,omitempty"`
At time.Time `json:"at"`
}
@@ -235,6 +241,12 @@ type apiEmailReply struct {
DailyCap int `json:"daily_cap"`
Active bool `json:"active"`
Counts apiEmailCounts `json:"counts"`
+ // DefaultHost is this app's own host — <app>.<sqid>.<platform-suffix> —
+ // spec §3's default sending identity, computed console-side (I4,
+ // review follow-up: the CLI itself never learns
+ // CARLOS_MAIL_PLATFORM_SUFFIX). Empty when this deployment has no
+ // platform suffix configured.
+ DefaultHost string `json:"default_host,omitempty"`
}
type apiDNSRecord struct {
@@ -344,7 +356,17 @@ func (s *apiSession) emailStatus(ctx context.Context, a apiAcct, app string) (ap
return apiEmailReply{}, err
}
if status != http.StatusOK {
- return apiEmailReply{}, mailAPIError(status, body)
+ // Even the "not declared" 404 carries the app's default sending
+ // host (I4, review follow-up) — `carlos email enable` with no
+ // -from needs it on the very FIRST call, before anything has ever
+ // been declared, which is exactly when this branch fires. Best
+ // effort: an unparseable body just leaves DefaultHost empty, same
+ // as a deployment with no platform suffix configured.
+ var partial struct {
+ DefaultHost string `json:"default_host"`
+ }
+ json.Unmarshal(body, &partial)
+ return apiEmailReply{DefaultHost: partial.DefaultHost}, mailAPIError(status, body)
}
var out apiEmailReply
if err := json.Unmarshal(body, &out); err != nil {
@@ -631,6 +653,17 @@ func runEmailEnableFlow(ctx context.Context, w io.Writer, sess *apiSession, acct
if from == "" && domain != "" {
from = "noreply@" + domain
}
+ // Spec §3's default identity — the app's own host — computed
+ // console-side and carried on cur (I4, review follow-up). Only when
+ // NEITHER -from NOR -domain named anything, and no prior declaration
+ // supplied one either: this is what makes `carlos email enable -app
+ // X`, with no other flags at all, a one-command story. Empty when
+ // this deployment has no platform suffix configured, in which case
+ // the explicit error below still fires, exactly as before this fix.
+ if from == "" && domain == "" && cur.DefaultHost != "" {
+ domain = cur.DefaultHost
+ from = "noreply@" + domain
+ }
if from == "" {
return errors.New("no From address to declare — pass -from, or -domain (defaults From to noreply@<domain>)")
}
@@ -802,13 +835,29 @@ func printEmailStatus(w io.Writer, res apiEmailReply) {
fmt.Fprintf(w, " sending: %s (key %s)\n", c.ok("active"), res.Provisioned.KeyID)
} else {
warnf(w, " sending: PAUSED — SMTP auth is failing for this app right now")
- // The record does not carry a causal reason (mail.Provisioned has
- // no Reason field) — the honest thing to print is the evidence a
- // member CAN see (today's counts against the cap), not a
- // diagnosis this reply cannot actually back up.
- mutedf(w, " today: %d sends, %d bounces, %d complaints, %d rejects against a cap of %d — "+
- "check the console's audit log for exactly why; carlos email resume -app %s once it's addressed",
- res.Counts.Sends, res.Counts.Bounces, res.Counts.Complaints, res.Counts.Rejects, res.DailyCap, res.App)
+ // The record does not carry a causal reason beyond PauseKind
+ // (mail.Provisioned has no free-text Reason field) — the
+ // honest thing to print is the evidence a member CAN see
+ // (today's counts against the cap), not a diagnosis this
+ // reply cannot actually back up.
+ mutedf(w, " today: %d sends, %d bounces, %d complaints, %d rejects against a cap of %d",
+ res.Counts.Sends, res.Counts.Bounces, res.Counts.Complaints, res.Counts.Rejects, res.DailyCap)
+ // I3 (review follow-up): `carlos email resume` is OPERATOR-only
+ // and 404s for an ordinary member — pointing every paused
+ // member at it, regardless of why, was actively wrong advice.
+ // A CAP pause (PauseKindCap) clears itself on the next UTC day
+ // roll (mailcounts.go's enforceMailLimits); nothing to run. A
+ // RATE pause, or a manual pause this reply carries no kind
+ // for, does not clear on its own and genuinely needs an
+ // operator.
+ switch res.Provisioned.PauseKind {
+ case "cap":
+ mutedf(w, " daily cap reached — this clears automatically at the next UTC day roll; no action needed")
+ case "rate":
+ mutedf(w, " bounce/complaint rate over threshold — this does not clear on its own; ask a deployment operator to resume it")
+ default:
+ mutedf(w, " paused by a deployment operator — ask them to resume it")
+ }
}
if res.PendingFinish {
warnf(w, " a rotation is pending finish — the previous key is still live: carlos email rotate -finish -app %s", res.App)
diff --git a/mail_cli_test.go b/mail_cli_test.go
index fdb593e55..d4dd2e2c6 100644
--- a/mail_cli_test.go
+++ b/mail_cli_test.go
@@ -390,6 +390,45 @@ func TestPrintEmailStatusPausedNamesEvidenceNotAGuess(t *testing.T) {
}
}
+// TestPrintEmailStatusCapPausePointsAtDayRollNotResumeVerb (I3, review
+// follow-up): `carlos email resume` is operator-only and 404s for an
+// ordinary member — a cap-paused member must be told the pause clears
+// itself, never sent at a verb that will refuse them.
+func TestPrintEmailStatusCapPausePointsAtDayRollNotResumeVerb(t *testing.T) {
+ var buf bytes.Buffer
+ printEmailStatus(&buf, apiEmailReply{
+ App: "seapointish", DailyCap: 50, Active: false,
+ Provisioned: &apiEmailProvisioned{KeyID: "AKIA1", PauseKind: "cap"},
+ Counts: apiEmailCounts{Sends: 51},
+ })
+ out := buf.String()
+ if !strings.Contains(out, "UTC day roll") {
+ t.Fatalf("cap-pause status does not mention the day roll:\n%s", out)
+ }
+ if strings.Contains(out, "carlos email resume") {
+ t.Fatalf("cap-pause status still points an ordinary member at the operator-only resume verb:\n%s", out)
+ }
+}
+
+// TestPrintEmailStatusRatePausePointsAtOperator: the symmetric case — a
+// rate pause genuinely needs an operator, and the status output must say
+// so rather than suggesting the pause clears on its own.
+func TestPrintEmailStatusRatePausePointsAtOperator(t *testing.T) {
+ var buf bytes.Buffer
+ printEmailStatus(&buf, apiEmailReply{
+ App: "seapointish", DailyCap: 100000, Active: false,
+ Provisioned: &apiEmailProvisioned{KeyID: "AKIA1", PauseKind: "rate"},
+ Counts: apiEmailCounts{Sends: 1000, Bounces: 60},
+ })
+ out := buf.String()
+ if !strings.Contains(out, "operator") {
+ t.Fatalf("rate-pause status does not point at an operator:\n%s", out)
+ }
+ if strings.Contains(out, "UTC day roll") {
+ t.Fatalf("rate-pause status wrongly claims it clears on its own:\n%s", out)
+ }
+}
+
func TestPrintEmailStatusActiveShowsKey(t *testing.T) {
var buf bytes.Buffer
printEmailStatus(&buf, apiEmailReply{
@@ -526,6 +565,10 @@ type fakeEmailConsole struct {
domainConflict bool
credRevoked bool
lastRevokedName string
+ // defaultHost is the not-declared GET /email 404's default_host field
+ // (I4, review follow-up) — "" reproduces a deployment with no platform
+ // suffix configured.
+ defaultHost string
}
func newFakeEmailConsole(t *testing.T, verifiedAfter int) *fakeEmailConsole {
@@ -540,7 +583,10 @@ func newFakeEmailConsole(t *testing.T, verifiedAfter int) *fakeEmailConsole {
case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/email"):
if !fc.declared {
w.WriteHeader(http.StatusNotFound)
- json.NewEncoder(w).Encode(map[string]any{"error": "no email is declared for this app — declare it first"})
+ json.NewEncoder(w).Encode(map[string]any{
+ "error": "no email is declared for this app — declare it first",
+ "default_host": fc.defaultHost,
+ })
return
}
json.NewEncoder(w).Encode(map[string]any{
@@ -689,6 +735,30 @@ func TestRunEmailEnableFlowNoFromNoDomainRefusesCleanly(t *testing.T) {
}
}
+// TestRunEmailEnableFlowNoFromNoDomainDefaultsToTheAppsPlatformHost (I4,
+// review follow-up): with neither -from nor -domain given, and nothing
+// declared before, the console's own default_host (spec §3's default
+// identity, the app's own host) must drive both — the one-command story
+// `carlos email enable -app X` promises when the deployment has a
+// platform suffix configured.
+func TestRunEmailEnableFlowNoFromNoDomainDefaultsToTheAppsPlatformHost(t *testing.T) {
+ fc := newFakeEmailConsole(t, 1) // verified on the first check
+ fc.defaultHost = "seapointish.aaa.oncarlos.com"
+ sess := newTestSession(fc.ts.URL)
+ var buf bytes.Buffer
+
+ err := runEmailEnableFlow(context.Background(), &buf, sess, apiAcct{Seg: "aaa"}, "seapointish", "", "", "", 5*time.Second)
+ if err != nil {
+ t.Fatalf("runEmailEnableFlow: %v", err)
+ }
+ if !fc.enableCalled {
+ t.Fatal("enable door was never called")
+ }
+ if fc.declaredVia.From != "noreply@seapointish.aaa.oncarlos.com" {
+ t.Fatalf("From = %q, want noreply@ the console's default_host", fc.declaredVia.From)
+ }
+}
+
// ---- runEmailTestFlow: mint / send / revoke bookkeeping --------------------
// TestRunEmailTestFlowRevokesOnSuccess: the temporary credential is revoked