carlos / platform Public

mail: never adopt an SES identity this account did not create; unverified domain claims lapse (#216)

A member's `email domains add` reached EnsureIdentity, which on
AlreadyExists adopted the identity: read back its DKIM tokens, rewrote its
MAIL FROM, and the walk then bound it to the account's configuration set,
verify read SES's existing verdict (instant send rights), and teardown
deleted it. Any domain verified in SES outside CARLOS (tito.io, the
operator's own domains, pre-claim-gate identities) could be taken over and
broken for its owner. Separately, domain claims were first-come and
permanent, and the 409 confirmed another account held the name.

Identity ownership:
- Provisioner.EnsureIdentity takes adopt; without it an existing identity
  is refused (ErrIdentityExists) before it is read or touched. Identity
  reports Created.
- EnsureDomain adopts only an identity CARLOS already knows is this
  account's: named by the account's domain record (re-runs, sibling apps,
  legacy records), or stamped on the claim as created by this account's
  walk (claim.identity_regions, written right after create, so a crash
  before the record still re-runs). Anything else needs proof of DNS
  control: a TXT record at _carlos-mail.<domain> with value
  carlos-mail-account=<acct>, checked through an injectable resolver
  (Deployment.Resolver; net.DefaultResolver in cmd/console, nil refuses).
  Unproven: ErrIdentityNotOwned, and the claim is released if no record
  backs it. Proven: recorded as RegionState.Adopted.
- Teardown, RemoveDomain and their orphan sweeps delete an identity only
  when the claim says this account's walk created it. Adopted identities,
  identities whose claim another account now holds, and identities behind
  records written before this change (which may have adopted a foreign
  identity, the #216 attack itself) are left in SES; only this account's
  tenant association goes.

Claim lapse:
- A claim older than ClaimLapse (7 days) that was never verified (claim
  verified_at stamp, holder's record, and SES's live status all say no)
  may be taken by another account. The claim moves first under CAS, so a
  holder that verifies meanwhile wins; then the lapsed holder's own
  stamped identity is deleted if SES still has not verified it, so the new
  holder starts fresh.
- VerifyDomain stamps verified_at, and refuses (marking failed) a record
  whose claim another account now holds.

Console: the claimed-domain 409 is now the neutral "sending domain
unavailable" (verify door too); the foreign-identity 409 names the TXT
record to publish. The CLI no longer appends "choose a different domain"
to every 409. All new fields are additive (omitempty), schema unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by claude 2 weeks ago 19f61dd741a8944fb0529916b2fc600a714a0d0a parent 78fc8d4

15 files changed, +893 −47

cmd/console/main.go +3 −0
docs/site/cli.md +12 −6
internal/console/apimail.go +33 −11
internal/console/apimail_test.go +30 −0
internal/mail/aws.go +10 −1
internal/mail/aws_test.go +40 −4
internal/mail/domain.go +5 −0
internal/mail/domainclaim.go +233 −0
internal/mail/memprovisioner.go +7 −3
internal/mail/memprovisioner_test.go +27 −7
internal/mail/provisioner.go +20 −2
internal/mail/walk.go +134 −9
internal/mail/walk_identityowner_test.go +332 −0
mail_cli.go +5 −2
mail_cli_test.go +2 −2

CI passed — run details

diff --git a/cmd/console/main.go b/cmd/console/main.go

index b83594daf..94751ed66 100644

--- a/cmd/console/main.go

+++ b/cmd/console/main.go

@@ -767,6 +767,9 @@ func mailDeployment() mail.Deployment {

PlatformSuffix: strings.ToLower(strings.TrimSpace(os.Getenv("CARLOS_MAIL_PLATFORM_SUFFIX"))),

DMARCRua: strings.TrimSpace(os.Getenv("CARLOS_MAIL_DMARC_RUA")),

SNSTopicARN: strings.TrimSpace(os.Getenv("CARLOS_MAIL_SNS_TOPIC_ARN")),

+ // The proof-of-control lookup for a domain whose SES identity this

+ // account did not create (#216). Read-only DNS, no AWS.

+ Resolver: net.DefaultResolver,

}

diff --git a/docs/site/cli.md b/docs/site/cli.md

index 25ee987ec..00e2d3b0d 100644

--- a/docs/site/cli.md

+++ b/docs/site/cli.md

@@ -1181,13 +1181,19 @@ keeps polling until SES confirms them, giving up after ten minutes unless

SES never confirmed. `carlos email domains add` is that same half on its own,

for a second domain on an app already declared.

+A sending domain can be claimed by exactly one CARLOS account at a time. A

+claim that is never verified lapses after seven days, and then another

+account can add the domain. If the domain already has an SES identity that

+CARLOS did not create for your account, `domains add` refuses it and tells

+you which TXT record to publish (at `_carlos-mail.<domain>`) to prove you

+control the domain. Once that record is there, add the domain again. CARLOS

+sends with an identity adopted this way but never deletes it.

`carlos email domains rm --app hello example.com` is the way back out. A

-sending domain can be claimed by exactly one CARLOS account at a time, so a

-domain one app holds is unavailable to everybody else — including the

-account that actually controls its DNS — until something releases it. `rm`

-is that something: it stops the app sending from the domain and, when it was

-the last app using it, deletes the SES identity and releases the claim, so

-another account can add and verify the domain. It refuses while the domain is

+domain one app holds is unavailable to everybody else until something

+releases it. `rm` is that something: it stops the app sending from the domain and, when it was

+the last app using it, releases the claim so another account can add and verify

+the domain, and deletes the SES identity if CARLOS created it for you. It refuses while the domain is

where the app's own From address sits — move that first with `carlos email

enable --app hello --from <addr>`. A custom domain's DNS records are yours,

so `rm` prints them rather than deleting them: the DKIM CNAMEs are dead with

diff --git a/internal/console/apimail.go b/internal/console/apimail.go

index baf362816..08831f908 100644

--- a/internal/console/apimail.go

+++ b/internal/console/apimail.go

@@ -1015,18 +1015,29 @@ func (s *Server) handleAPIEmailDomainEnsure(w http.ResponseWriter, r *http.Reque

d, writer := s.mailDeploymentFor()

rec, pending, err := mail.EnsureDomain(ctx, s.cfg.Releases, s.cfg.MailProvisioner, writer, d, acct.Sqid, app, domain, time.Now())

if err != nil {

- // I3 (Task 17a review follow-up): ErrDomainClaimed is a PERMANENT

- // refusal, not a transient failure — re-running buys the member

- // nothing, because the outcome does not depend on anything that

- // changes between attempts. Rendering it as the generic 502 below

- // told a member to retry something that can never succeed, with a

- // server-error status for a client-side condition. 409 names the

- // real shape (a conflict with existing state) and the message stays

- // neutral — mail.ErrDomainClaimed's own text never names the

- // holding account, and this door must not add it back.

+ // I3 (Task 17a review follow-up): ErrDomainClaimed is a refusal a

+ // retry cannot change, not a transient failure. Rendering it as the

+ // generic 502 below told a member to retry, with a server-error

+ // status for a client-side condition. 409 names the real shape (a

+ // conflict with existing state).

+ //

+ // The message is the neutral one (#216), in the shape of the

+ // hostname door's hostUnavailableMsg: it used to say "already

+ // claimed by another CARLOS account", which confirmed to anybody

+ // that some tenant holds the name.

if errors.Is(err, mail.ErrDomainClaimed) {

- jsonError(w, http.StatusConflict,

- "this domain is already claimed by another CARLOS account — choose a different domain")

+ jsonError(w, http.StatusConflict, mailDomainUnavailableMsg)

+ return

+ }

+ // ErrIdentityNotOwned (#216): SES already has an identity for this

+ // domain that this account's claim did not create. The member is

+ // told exactly how to prove control, because the proof is the only

+ // way in and nobody can guess the record.

+ if errors.Is(err, mail.ErrIdentityNotOwned) {

+ jsonError(w, http.StatusConflict, fmt.Sprintf(

+ "%s already has a sending identity this account did not create. To use it, prove you control the domain: "+

+ "publish a TXT record at %s with the value %s, then add the domain again.",

+ domain, mail.ProofRecordName(domain), mail.ProofRecordValue(acct.Sqid)))

return

}

// Same reasoning for ErrNotOwnPlatformName: refused because of what

@@ -1056,6 +1067,11 @@ func (s *Server) handleAPIEmailDomainEnsure(w http.ResponseWriter, r *http.Reque

})

}

+// mailDomainUnavailableMsg is the one refusal for a sending domain another

+// account holds, in the hostname door's neutral shape (hostUnavailableMsg):

+// it says no, and not who or whether anyone does.

+const mailDomainUnavailableMsg = "sending domain unavailable"

// handleAPIEmailDomainVerify answers `carlos email domains verify`:

// re-check SES's own verdict for this deployment's region and record it.

// Account-scoped, like the domain record itself (domain.go's package

@@ -1095,6 +1111,12 @@ func (s *Server) handleAPIEmailDomainVerify(w http.ResponseWriter, r *http.Reque

// record last recorded.

rec, err := mail.VerifyDomain(ctx, s.cfg.Releases, s.cfg.MailProvisioner,

s.cfg.MailDeployment, acct.Sqid, domain, region, time.Now())

+ if errors.Is(err, mail.ErrDomainClaimed) {

+ // The claim lapsed unverified and another account took the domain

+ // (#216). The same neutral words as the add door.

+ jsonError(w, http.StatusConflict, mailDomainUnavailableMsg)

+ return

+ }

if err != nil {

log.Printf("console: api email domain verify %s/%s: %v", acct.Sqid, domain, err)

jsonError(w, http.StatusBadGateway, "could not check verification — try again")

diff --git a/internal/console/apimail_test.go b/internal/console/apimail_test.go

index 62af15b14..12b52d2e5 100644

--- a/internal/console/apimail_test.go

+++ b/internal/console/apimail_test.go

@@ -1450,6 +1450,36 @@ func TestAPIEmailDomainEnsureRefusesADomainClaimedByAnotherAccount(t *testing.T)

if strings.Contains(strings.ToLower(msg), "try again") || strings.Contains(strings.ToLower(msg), "re-run") {

t.Fatalf("refusal promised a retry would help, but this refusal is permanent: %q", msg)

}

+ // Neutral (#216): the refusal must not confirm that another account

+ // holds the domain, only that this one cannot have it.

+ if msg != mailDomainUnavailableMsg {

+ t.Fatalf("refusal = %q, want the neutral %q", msg, mailDomainUnavailableMsg)

+ }

+// TestAPIEmailDomainEnsureRefusesAForeignIdentityWithProofSteps (#216): a

+// domain already verified in SES outside CARLOS is refused with 409 and the

+// exact TXT record that would prove control, and the identity is untouched.

+func TestAPIEmailDomainEnsureRefusesAForeignIdentityWithProofSteps(t *testing.T) {

+ fx, fake, _, token := emailArmed(t, "code-email-domain-foreign")

+ app := "titogo"

+ declareEmail(t, fx, token, app, "hello@tito.io")

+ fake.Identities["eu-west-1|tito.io"] = &mail.MemIdentity{

+ ARN: "arn:aws:ses:eu-west-1:1:identity/tito.io", DKIMTokens: []string{"c1", "c2", "c3"},

+ MailFrom: "bounce.tito.io", ConfigSet: "classic",

+ Status: mail.Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"},

+ }

+ status, out := postJSON(t, emailURL(fx, app)+"/domains", map[string]any{"domain": "tito.io"}, token)

+ if status != http.StatusConflict {

+ t.Fatalf("ensure a foreign identity = %d, want 409: %v", status, out)

+ }

+ msg, _ := out["error"].(string)

+ if !strings.Contains(msg, mail.ProofRecordName("tito.io")) || !strings.Contains(msg, mail.ProofRecordValue(fx.sqid)) {

+ t.Fatalf("refusal does not say how to prove control: %q", msg)

+ }

+ if got := fake.Identities["eu-west-1|tito.io"]; got.MailFrom != "bounce.tito.io" || got.ConfigSet != "classic" {

+ t.Fatalf("the foreign identity was mutated: %+v", got)

+ }

}

// TestAPIEmailEventsReadsTheLog: the door `carlos email events` calls hands

diff --git a/internal/mail/aws.go b/internal/mail/aws.go

index 0c68bd433..682b21e1b 100644

--- a/internal/mail/aws.go

+++ b/internal/mail/aws.go

@@ -291,19 +291,27 @@ func (p *awsProvisioner) SetIdentityConfigSet(ctx context.Context, region, domai

// domain and p.account (resolved once at construction — controller ruling

// F3), in the exact shape SendPolicyJSON validates:

// arn:aws:ses:<region>:<account-id>:identity/<domain>.

-func (p *awsProvisioner) EnsureIdentity(ctx context.Context, region, domain, mailFrom string) (Identity, error) {

+//

+// With adopt false, "already exists" is ErrIdentityExists, returned before

+// the identity is read or its MAIL FROM touched: the identity may belong to

+// another product in this AWS account (#216).

+func (p *awsProvisioner) EnsureIdentity(ctx context.Context, region, domain, mailFrom string, adopt bool) (Identity, error) {

if err := p.checkRegion(region); err != nil {

return Identity{}, err

}

var tokens []string

+ created := false

out, err := p.ses.CreateEmailIdentity(ctx, &sesv2.CreateEmailIdentityInput{

EmailIdentity: aws.String(domain),

})

switch {

case err == nil:

+ created = true

if out.DkimAttributes != nil {

tokens = out.DkimAttributes.Tokens

}

+ case isAlreadyExists(err) && !adopt:

+ return Identity{}, fmt.Errorf("mail: identity %q in %s: %w", domain, region, ErrIdentityExists)

case isAlreadyExists(err):

got, gerr := p.ses.GetEmailIdentity(ctx, &sesv2.GetEmailIdentityInput{

EmailIdentity: aws.String(domain),

@@ -338,6 +346,7 @@ func (p *awsProvisioner) EnsureIdentity(ctx context.Context, region, domain, mai

return Identity{

ARN: fmt.Sprintf("arn:aws:ses:%s:%s:identity/%s", region, p.account, domain),

DKIMTokens: tokens,

+ Created: created,

}, nil

}

diff --git a/internal/mail/aws_test.go b/internal/mail/aws_test.go

index ae10580bb..b57c1f893 100644

--- a/internal/mail/aws_test.go

+++ b/internal/mail/aws_test.go

@@ -145,7 +145,7 @@ func TestEnsureIdentityAlreadyExistsReadsBackTokensAndComposesTheARN(t *testing.

}

p := &awsProvisioner{ses: stub, account: "123456789012", region: "eu-west-1"}

- got, err := p.EnsureIdentity(context.Background(), "eu-west-1", "example.com", "mail.example.com")

+ got, err := p.EnsureIdentity(context.Background(), "eu-west-1", "example.com", "mail.example.com", true)

if err != nil {

t.Fatalf("EnsureIdentity: %v", err)

}

@@ -168,6 +168,42 @@ func TestEnsureIdentityAlreadyExistsReadsBackTokensAndComposesTheARN(t *testing.

}

+// TestEnsureIdentityWithoutAdoptLeavesAnExistingIdentityAlone (#216): with

+// adopt false, an identity that already exists may belong to another product

+// in this AWS account, so it is refused before it is read or its MAIL FROM

+// rewritten.

+func TestEnsureIdentityWithoutAdoptLeavesAnExistingIdentityAlone(t *testing.T) {

+ touched := false

+ stub := &stubSES{

+ createEmailIdentity: func(*sesv2.CreateEmailIdentityInput) (*sesv2.CreateEmailIdentityOutput, error) {

+ return nil, &sesv2types.AlreadyExistsException{Message: aws.String("identity already exists")}

+ },

+ getEmailIdentity: func(*sesv2.GetEmailIdentityInput) (*sesv2.GetEmailIdentityOutput, error) {

+ touched = true

+ return &sesv2.GetEmailIdentityOutput{}, nil

+ },

+ putEmailIdentityMailFromAttributes: func(*sesv2.PutEmailIdentityMailFromAttributesInput) (*sesv2.PutEmailIdentityMailFromAttributesOutput, error) {

+ touched = true

+ return &sesv2.PutEmailIdentityMailFromAttributesOutput{}, nil

+ },

+ }

+ p := &awsProvisioner{ses: stub, account: "123456789012", region: "eu-west-1"}

+ if _, err := p.EnsureIdentity(context.Background(), "eu-west-1", "example.com", "mail.example.com", false); !errors.Is(err, ErrIdentityExists) {

+ t.Fatalf("EnsureIdentity(adopt=false) on an existing identity = %v, want ErrIdentityExists", err)

+ }

+ if touched {

+ t.Fatal("an existing identity was read or rewritten without adopt")

+ }

+ stub.createEmailIdentity = func(*sesv2.CreateEmailIdentityInput) (*sesv2.CreateEmailIdentityOutput, error) {

+ return &sesv2.CreateEmailIdentityOutput{DkimAttributes: &sesv2types.DkimAttributes{Tokens: []string{"a", "b", "c"}}}, nil

+ }

+ got, err := p.EnsureIdentity(context.Background(), "eu-west-1", "example.com", "mail.example.com", false)

+ if err != nil || !got.Created {

+ t.Fatalf("a fresh create = %+v, %v; want Created", got, err)

+ }

// TestEnsureIdentityRefusesNoTokens is the other half of the same path: an

// identity that reads back with no DKIM tokens (BYODKIM, an email-address

// identity, or a nil DkimAttributes) must not be reported as success — the

@@ -181,7 +217,7 @@ func TestEnsureIdentityRefusesNoTokens(t *testing.T) {

}

p := &awsProvisioner{ses: stub, account: "123456789012", region: "eu-west-1"}

- if _, err := p.EnsureIdentity(context.Background(), "eu-west-1", "example.com", "mail.example.com"); err == nil {

+ if _, err := p.EnsureIdentity(context.Background(), "eu-west-1", "example.com", "mail.example.com", true); err == nil {

t.Fatal("EnsureIdentity with zero DKIM tokens returned success, want an error")

}

@@ -273,7 +309,7 @@ func TestRegionGuardRefusesAMismatch(t *testing.T) {

return p.EnsureConfigSet(context.Background(), otherRegion, "cfg", "arn:aws:sns:eu-west-1:123456789012:topic")

}},

{"EnsureIdentity", func(p *awsProvisioner) error {

- _, err := p.EnsureIdentity(context.Background(), otherRegion, "example.com", "mail.example.com")

+ _, err := p.EnsureIdentity(context.Background(), otherRegion, "example.com", "mail.example.com", true)

return err

}},

{"IdentityStatus", func(p *awsProvisioner) error {

@@ -315,7 +351,7 @@ func TestRegionGuardPassesAnEmptyRegionThrough(t *testing.T) {

return p.EnsureConfigSet(context.Background(), "", "cfg", "arn:aws:sns:eu-west-1:123456789012:topic")

}},

{"EnsureIdentity", func(p *awsProvisioner) error {

- _, err := p.EnsureIdentity(context.Background(), "", "example.com", "mail.example.com")

+ _, err := p.EnsureIdentity(context.Background(), "", "example.com", "mail.example.com", true)

return err

}},

{"IdentityStatus", func(p *awsProvisioner) error {

diff --git a/internal/mail/domain.go b/internal/mail/domain.go

index b41fe8d8f..67fb72787 100644

--- a/internal/mail/domain.go

+++ b/internal/mail/domain.go

@@ -47,6 +47,11 @@ type RegionState struct {

ConfigSet string `json:"config_set"`

Status string `json:"status"`

CheckedAt time.Time `json:"checked_at"`

+ // Adopted marks an identity this account did not create but proved it

+ // controls the domain of (#216). CARLOS sends on it and never deletes

+ // it. Absent on every record written before it existed, which keeps

+ // those identities torn down as they always were.

+ Adopted bool `json:"adopted,omitempty"`

}

// DomainRecord is one sending domain.

diff --git a/internal/mail/domainclaim.go b/internal/mail/domainclaim.go

index 90cc59987..afeb47928 100644

--- a/internal/mail/domainclaim.go

+++ b/internal/mail/domainclaim.go

@@ -31,6 +31,7 @@ import (

"fmt"

"log"

"os"

+ "strings"

"time"

"github.com/carlosframework/platform/internal/release"

@@ -65,6 +66,28 @@ type DomainClaim struct {

// wrong caller.

App string `json:"app,omitempty"`

CreatedAt time.Time `json:"created_at"`

+ // IdentityRegions are the regions where this claim's own walk CREATED

+ // the SES identity, stamped right after the create (#216). It is the

+ // only record of that for a claim whose domain record was never

+ // written, so it is what lets a re-run keep the identity and lets the

+ // orphan sweeps delete it — and what stops either touching an identity

+ // somebody else made. Omitted on a claim written before it existed;

+ // such a claim's identity is then treated as not CARLOS's to delete.

+ IdentityRegions []string `json:"identity_regions,omitempty"`

+ // VerifiedAt is when VerifyDomain first saw SES verify the domain for

+ // this claim's account. A claim with it set never lapses (ClaimLapse).

+ VerifiedAt *time.Time `json:"verified_at,omitempty"`

+// createdIdentityIn reports whether this claim's walk created the domain's

+// identity in region.

+func (c *DomainClaim) createdIdentityIn(region string) bool {

+ for _, r := range c.IdentityRegions {

+ if r == region {

+ return true

+ }

+ return false

}

// ErrDomainClaimed is returned by EnsureDomain when the domain's global

@@ -74,6 +97,23 @@ type DomainClaim struct {

// holding the claim is never this error; see claimDomain.

var ErrDomainClaimed = errors.New("mail: domain is claimed")

+// ErrIdentityNotOwned is EnsureDomain's refusal of a domain whose SES

+// identity this account's claim did not create — one verified outside

+// CARLOS, or left by a claim that predates the record of who created what —

+// when the account has not proved it controls the domain (#216). Adopting

+// it would hand the account send rights it never proved, rewrite the

+// identity's MAIL FROM and configuration set under its real owner, and let

+// a later teardown delete it.

+var ErrIdentityNotOwned = errors.New("mail: domain has an SES identity this account did not create")

+// ClaimLapse is how long a claim may hold a domain without ever being

+// verified (#216). Before it, a claim was first-come and permanent, so any

+// account could hold any domain it would never prove, against every other

+// account, for the life of the deployment. SES itself gives DKIM 72 hours

+// before it marks an identity failed; a week leaves room for a slow DNS

+// change and a weekend. A claim that reaches verified never lapses.

+const ClaimLapse = 7 * 24 * time.Hour

// getDomainClaim reads domain's claim. A not-yet-claimed domain returns

// (nil, nil), matching GetDomain's convention rather than propagating

// os.ErrNotExist to every caller.

@@ -116,6 +156,10 @@ func getDomainClaimETag(ctx context.Context, s release.Store, domain string) (*D

// round 2), not who most recently retried it. Any other account gets

// ErrDomainClaimed, with no identity ever minted and no domain record ever

// written — the refusal happens before either exists.

+//

+// First-come is no longer forever: EnsureDomain hands this refusal to

+// takeLapsedClaim, which lets a claim that was never verified go after

+// ClaimLapse (#216).

func claimDomain(ctx context.Context, s release.Store, domain, acct, app string, now time.Time) error {

body, err := json.Marshal(DomainClaim{Schema: 1, Domain: domain, Account: acct, App: app, CreatedAt: now})

if err != nil {

@@ -141,6 +185,195 @@ func claimDomain(ctx context.Context, s release.Store, domain, acct, app string,

}

+// updateOwnClaim applies change to domain's claim when acct holds it, under

+// CAS. change reports whether it changed anything; a claim held by anyone

+// else, or none at all, is left alone and is not an error.

+func updateOwnClaim(ctx context.Context, s release.Store, domain, acct string, change func(*DomainClaim) bool) error {

+ for attempt := 0; ; attempt++ {

+ cur, etag, err := getDomainClaimETag(ctx, s, domain)

+ if errors.Is(err, os.ErrNotExist) {

+ return nil

+ }

+ if err != nil {

+ return err

+ }

+ if cur.Account != acct || !change(cur) {

+ return nil

+ }

+ body, err := json.Marshal(cur)

+ if err != nil {

+ return err

+ }

+ if _, err := s.Put(ctx, domainClaimKey(domain), body, etag); err == nil {

+ return nil

+ } else if !errors.Is(err, release.ErrCASConflict) || attempt >= 2 {

+ return err

+ }

+// stampClaimIdentity records on acct's claim that its walk created the

+// identity in region.

+func stampClaimIdentity(ctx context.Context, s release.Store, domain, acct, region string) error {

+ return updateOwnClaim(ctx, s, domain, acct, func(c *DomainClaim) bool {

+ if c.createdIdentityIn(region) {

+ return false

+ }

+ c.IdentityRegions = append(c.IdentityRegions, region)

+ return true

+ })

+// stampClaimVerified records the first time acct's claim was verified.

+func stampClaimVerified(ctx context.Context, s release.Store, domain, acct string, now time.Time) error {

+ return updateOwnClaim(ctx, s, domain, acct, func(c *DomainClaim) bool {

+ if c.VerifiedAt != nil {

+ return false

+ }

+ at := now.UTC()

+ c.VerifiedAt = &at

+ return true

+ })

+// claimLapsed reports whether c, held by another account, has lapsed: older

+// than ClaimLapse and never verified by any of three witnesses — the claim's

+// own stamp, the holder's domain record, and SES itself. SES is asked

+// because a holder may have published its DNS and never run verify; such a

+// domain is in use, and handing it on would also hand on a verified

+// identity.

+func claimLapsed(ctx context.Context, s release.Store, p Provisioner, c *DomainClaim, region string, now time.Time) (bool, error) {

+ if c.VerifiedAt != nil || now.Sub(c.CreatedAt) < ClaimLapse {

+ return false, nil

+ }

+ rec, _, err := GetDomain(ctx, s, c.Account, c.Domain)

+ if err != nil {

+ return false, err

+ }

+ if rec != nil {

+ for _, st := range rec.Regions {

+ if st != nil && st.Status == StatusVerified {

+ return false, nil

+ }

+ got, err := p.IdentityStatus(ctx, region, c.Domain)

+ if err != nil {

+ return false, err

+ }

+ if got.DKIM == "SUCCESS" || got.Sending == "SUCCESS" {

+ return false, nil

+ }

+ return true, nil

+// takeLapsedClaim is EnsureDomain's answer to ErrDomainClaimed: when the

+// other account's claim has lapsed, acct takes it over; otherwise the

+// refusal stands.

+//

+// The claim moves FIRST, under CAS against the etag the lapse was judged

+// on. A holder that verifies in the meantime stamps verified_at, which moves

+// the etag, so the CAS fails and the holder keeps everything. Once the claim

+// is acct's, the holder's VerifyDomain and teardown both refuse to act on

+// the domain, so its record can no longer verify into a grant.

+//

+// Only then does the lapsed holder's own half-made identity go — and only

+// when its claim says its walk created it, and SES, asked again right

+// before the delete, still has not verified it. Left in place, the new

+// holder would meet an identity it did not create and have to prove DNS

+// control for a domain whose only obstacle was an abandoned attempt. An

+// identity the lapsed claim did not create, or one SES has since verified,

+// is not touched: the new holder meets it as a foreign identity and needs

+// proof. A crash between the CAS and the delete leaves the same state.

+func takeLapsedClaim(ctx context.Context, s release.Store, p Provisioner, region, domain, acct, app string, now time.Time) error {

+ cur, etag, err := getDomainClaimETag(ctx, s, domain)

+ if errors.Is(err, os.ErrNotExist) {

+ return claimDomain(ctx, s, domain, acct, app, now) // released meanwhile

+ }

+ if err != nil {

+ return err

+ }

+ if cur.Account == acct {

+ return nil

+ }

+ lapsed, err := claimLapsed(ctx, s, p, cur, region, now)

+ if err != nil {

+ return err

+ }

+ if !lapsed {

+ return ErrDomainClaimed

+ }

+ body, err := json.Marshal(DomainClaim{Schema: 1, Domain: domain, Account: acct, App: app, CreatedAt: now})

+ if err != nil {

+ return err

+ }

+ if _, err := s.Put(ctx, domainClaimKey(domain), body, etag); err != nil {

+ if errors.Is(err, release.ErrCASConflict) {

+ return ErrDomainClaimed // the holder or another claimant moved first

+ }

+ return err

+ }

+ log.Printf("mail: claim on %s lapsed unverified (held since %s); taken by another account", domain, cur.CreatedAt.Format(time.RFC3339))

+ if !cur.createdIdentityIn(region) {

+ return nil

+ }

+ if st, err := p.IdentityStatus(ctx, region, domain); err != nil {

+ log.Printf("mail: lapsed claim %s: identity status: %v; leaving the old identity", domain, err)

+ return nil

+ } else if st.DKIM == "SUCCESS" || st.Sending == "SUCCESS" {

+ return nil

+ }

+ if old, _, err := GetDomain(ctx, s, cur.Account, domain); err == nil && old != nil &&

+ old.Regions[region] != nil && old.Regions[region].IdentityARN != "" {

+ // SES refuses to delete an associated identity.

+ if err := p.DisassociateTenantResource(ctx, TenantName(cur.Account), old.Regions[region].IdentityARN); err != nil {

+ log.Printf("mail: lapsed claim %s: disassociating: %v", domain, err)

+ }

+ // A failed delete is logged, not returned: the claim is already acct's,

+ // and the walk goes on to meet the identity as a foreign one.

+ if err := p.DeleteIdentity(ctx, region, domain); err != nil {

+ log.Printf("mail: lapsed claim %s: delete old identity: %v", domain, err)

+ }

+ return nil

+// TXTResolver is the one DNS question the walk asks: the proof record for a

+// domain whose SES identity this account did not create. net.Resolver

+// satisfies it; tests inject a fake.

+type TXTResolver interface {

+ LookupTXT(ctx context.Context, name string) ([]string, error)

+// ProofRecordName and ProofRecordValue are the TXT record a claimant

+// publishes to prove it controls a domain that already has an SES identity

+// (#216). The value names the account, so one account's proof is no use to

+// another; only somebody who can write the domain's DNS can publish it.

+func ProofRecordName(domain string) string { return "_carlos-mail." + domain }

+func ProofRecordValue(acct string) string { return "carlos-mail-account=" + acct }

+// provesControl reports whether domain publishes acct's proof record. A nil

+// resolver proves nothing, so a deployment that never wired one refuses

+// rather than adopts.

+func provesControl(ctx context.Context, r TXTResolver, domain, acct string) bool {

+ if r == nil {

+ return false

+ }

+ vals, err := r.LookupTXT(ctx, ProofRecordName(domain))

+ if err != nil {

+ return false

+ }

+ want := ProofRecordValue(acct)

+ for _, v := range vals {

+ if strings.TrimSpace(v) == want {

+ return true

+ }

+ return false

// releaseDomainClaim removes domain's claim, but only when it is held by

// acct — the same owner-gated neutrality domains.DetachClaim observes, so

// this can never be used to discover or disturb another account's claim.

diff --git a/internal/mail/memprovisioner.go b/internal/mail/memprovisioner.go

index 5d1f2480f..b144a8dd1 100644

--- a/internal/mail/memprovisioner.go

+++ b/internal/mail/memprovisioner.go

@@ -111,8 +111,9 @@ func (p *MemProvisioner) EnsureConfigSet(_ context.Context, region, name, topic

// EnsureIdentity is idempotent AND token-stable: a second call returns the

// identity already there rather than minting new tokens, because new tokens

-// would orphan every DKIM CNAME already published.

-func (p *MemProvisioner) EnsureIdentity(_ context.Context, region, domain, mailFrom string) (Identity, error) {

+// would orphan every DKIM CNAME already published. With adopt false an

+// existing identity is refused untouched, as the real backend does.

+func (p *MemProvisioner) EnsureIdentity(_ context.Context, region, domain, mailFrom string, adopt bool) (Identity, error) {

p.mu.Lock()

defer p.mu.Unlock()

if err := p.takeFailure("EnsureIdentity"); err != nil {

@@ -120,6 +121,9 @@ func (p *MemProvisioner) EnsureIdentity(_ context.Context, region, domain, mailF

}

k := identityKey(region, domain)

if got, ok := p.Identities[k]; ok {

+ if !adopt {

+ return Identity{}, fmt.Errorf("mail: identity %q in %s: %w", domain, region, ErrIdentityExists)

+ }

got.MailFrom = mailFrom

return Identity{ARN: got.ARN, DKIMTokens: got.DKIMTokens}, nil

}

@@ -135,7 +139,7 @@ func (p *MemProvisioner) EnsureIdentity(_ context.Context, region, domain, mailF

Status: p.StatusOnEnsure,

}

p.Identities[k] = id

- return Identity{ARN: id.ARN, DKIMTokens: id.DKIMTokens}, nil

+ return Identity{ARN: id.ARN, DKIMTokens: id.DKIMTokens, Created: true}, nil

}

// SetIdentityConfigSet models PutEmailIdentityConfigurationSetAttributes:

diff --git a/internal/mail/memprovisioner_test.go b/internal/mail/memprovisioner_test.go

index db81baf13..de3b47156 100644

--- a/internal/mail/memprovisioner_test.go

+++ b/internal/mail/memprovisioner_test.go

@@ -11,14 +11,14 @@ import (

// the SAME tokens — fresh tokens would orphan the DNS already published.

func TestMemEnsureIdentityIsIdempotentAndTokenStable(t *testing.T) {

ctx, p := context.Background(), NewMemProvisioner("1")

- first, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com")

+ first, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com", true)

if err != nil {

t.Fatalf("EnsureIdentity: %v", err)

}

if len(first.DKIMTokens) != 3 {

t.Fatalf("want 3 DKIM tokens, got %d", len(first.DKIMTokens))

}

- second, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com")

+ second, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com", true)

if err != nil {

t.Fatalf("re-EnsureIdentity: %v", err)

}

@@ -27,10 +27,30 @@ func TestMemEnsureIdentityIsIdempotentAndTokenStable(t *testing.T) {

}

+// Without adopt, the fake refuses an existing identity untouched, as SES

+// does through awsProvisioner (#216).

+func TestMemEnsureIdentityWithoutAdoptRefusesUntouched(t *testing.T) {

+ ctx, p := context.Background(), NewMemProvisioner("1")

+ first, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m1.x.com", false)

+ if err != nil || !first.Created {

+ t.Fatalf("create = %+v, %v", first, err)

+ }

+ if _, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m2.x.com", false); !errors.Is(err, ErrIdentityExists) {

+ t.Fatalf("re-ensure without adopt = %v, want ErrIdentityExists", err)

+ }

+ if got := p.Identities[identityKey("eu-west-1", "x.com")].MailFrom; got != "m1.x.com" {

+ t.Fatalf("MAIL FROM rewritten to %q without adopt", got)

+ }

+ again, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m1.x.com", true)

+ if err != nil || again.Created {

+ t.Fatalf("adopt = %+v, %v; want the existing identity, not Created", again, err)

+ }

func TestMemIdentitiesAreRegionScoped(t *testing.T) {

ctx, p := context.Background(), NewMemProvisioner("1")

- eu, _ := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com")

- us, _ := p.EnsureIdentity(ctx, "us-east-1", "x.com", "mail-us-east-1.x.com")

+ eu, _ := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com", true)

+ us, _ := p.EnsureIdentity(ctx, "us-east-1", "x.com", "mail-us-east-1.x.com", true)

if eu.ARN == us.ARN {

t.Fatal("one ARN served two regions")

}

@@ -124,10 +144,10 @@ func TestMemFailNextIsOneShot(t *testing.T) {

ctx, p := context.Background(), NewMemProvisioner("1")

boom := errors.New("boom")

p.FailNext["EnsureIdentity"] = boom

- if _, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m.x.com"); !errors.Is(err, boom) {

+ if _, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m.x.com", true); !errors.Is(err, boom) {

t.Fatalf("first call = %v, want boom", err)

}

- if _, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m.x.com"); err != nil {

+ if _, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "m.x.com", true); err != nil {

t.Fatalf("second call = %v, want nil", err)

}

@@ -138,7 +158,7 @@ func TestMemFailNextIsOneShot(t *testing.T) {

func TestMemProvisionerRefusesToDeleteAnAssociatedIdentity(t *testing.T) {

ctx := context.Background()

p := NewMemProvisioner("1")

- id, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com")

+ id, err := p.EnsureIdentity(ctx, "eu-west-1", "x.com", "mail-eu-west-1.x.com", true)

if err != nil {

t.Fatal(err)

}

diff --git a/internal/mail/provisioner.go b/internal/mail/provisioner.go

index 1b2d0bf00..3df1f2440 100644

--- a/internal/mail/provisioner.go

+++ b/internal/mail/provisioner.go

@@ -8,7 +8,10 @@

// by hand.

package mail

-import "context"

+import (

+ "context"

+ "errors"

// Identity is what EnsureIdentity answers: the ARN a policy names and the

// DKIM tokens whose CNAMEs must be published. The tokens are STABLE for the

@@ -18,8 +21,17 @@ import "context"

type Identity struct {

ARN string

DKIMTokens []string

+ // Created is true when this call made the identity, false when it was

+ // already there. The walk records it, because an identity CARLOS did not

+ // create is not CARLOS's to delete (discussion #216).

+ Created bool

}

+// ErrIdentityExists is EnsureIdentity's refusal, with adopt false, of a

+// domain that already has an identity. Nothing about the identity has been

+// read or changed when it is returned.

+var ErrIdentityExists = errors.New("mail: identity already exists")

// Status is SES's own verdict on an identity. All three must read

// "SUCCESS" before the identity is usable; a domain that is verified for

// sending but whose MAIL FROM has not converged still sends, from the

@@ -79,7 +91,13 @@ type Provisioner interface {

// EnsureIdentity creates the domain identity, enables Easy DKIM and

// sets the custom MAIL FROM subdomain, all idempotently.

- EnsureIdentity(ctx context.Context, region, domain, mailFrom string) (Identity, error)

+ //

+ // adopt says whether an identity that already exists may be taken over:

+ // its DKIM tokens read back and its MAIL FROM rewritten. With adopt

+ // false, an existing identity is refused with ErrIdentityExists and left

+ // exactly as it was — the walk passes false unless the identity is one

+ // this account already holds or has proved it controls (#216).

+ EnsureIdentity(ctx context.Context, region, domain, mailFrom string, adopt bool) (Identity, error)

IdentityStatus(ctx context.Context, region, domain string) (Status, error)

DeleteIdentity(ctx context.Context, region, domain string) error

diff --git a/internal/mail/walk.go b/internal/mail/walk.go

index f721f61da..ae1fd4bb7 100644

--- a/internal/mail/walk.go

+++ b/internal/mail/walk.go

@@ -63,6 +63,10 @@ type Deployment struct {

PlatformSuffix string // e.g. "oncarlos.com" — the zone CARLOS owns

DMARCRua string

SNSTopicARN string

+ // Resolver answers the proof-of-control lookup for a domain whose SES

+ // identity this account did not create (#216). nil means no proof can

+ // be checked, so such a domain is always refused.

+ Resolver TXTResolver

}

// DomainKindFor decides who publishes DNS for a domain.

@@ -181,7 +185,11 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec.

if DomainKindFor(d, domain) == KindPlatform && !platformNameOwnedBy(d, domain, acct) {

return nil, nil, fmt.Errorf("mail: %q is not %s's platform name: %w", domain, acct, ErrNotOwnPlatformName)

}

- if err := claimDomain(ctx, s, domain, acct, app, now); err != nil {

+ err := claimDomain(ctx, s, domain, acct, app, now)

+ if errors.Is(err, ErrDomainClaimed) {

+ err = takeLapsedClaim(ctx, s, p, d.Region, domain, acct, app, now)

+ }

+ if err != nil {

// Wrapping preserves errors.Is(err, ErrDomainClaimed) for callers

// (the console door translates it into its own neutral refusal) —

// %w keeps the chain — and the added context is just the domain

@@ -200,6 +208,7 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec.

if err != nil {

return nil, nil, err

}

+ hadRecord := rec != nil

if rec == nil {

rec = &DomainRecord{

Domain: domain, Kind: DomainKindFor(d, domain),

@@ -211,11 +220,59 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec.

}

rec.AddApp(app)

+ // WHOSE IDENTITY IS IT (#216). SES identities are per AWS account, and

+ // this one is shared with every other product in it, so "already

+ // exists" used to mean "adopt": read back its DKIM tokens, rewrite its

+ // MAIL FROM, bind it to this account's configuration set, and delete it

+ // at teardown. A member could do that to tito.io, or to the operator's

+ // own sign-in domain, and get send rights for it the moment verify read

+ // SES's existing verdict.

+ //

+ // So an existing identity is this account's to keep only when CARLOS

+ // already knows it is: the account's record names it (created by an

+ // earlier run, by a sibling app, or adopted with proof before), or the

+ // claim says this account's walk created it and the record was never

+ // written. Anything else needs proof of DNS control, and is recorded as

+ // Adopted so nothing CARLOS does later deletes it.

mailFrom := MailFromDomain(domain, d.Region)

- ident, err := p.EnsureIdentity(ctx, d.Region, domain, mailFrom)

+ prev := rec.Regions[d.Region]

+ owned := prev != nil && prev.IdentityARN != ""

+ adopted := owned && prev.Adopted

+ if !owned {

+ claim, err := getDomainClaim(ctx, s, domain)

+ if err != nil {

+ return nil, nil, err

+ }

+ owned = claim != nil && claim.Account == acct && claim.createdIdentityIn(d.Region)

+ }

+ ident, err := p.EnsureIdentity(ctx, d.Region, domain, mailFrom, owned)

+ if errors.Is(err, ErrIdentityExists) {

+ if !provesControl(ctx, d.Resolver, domain, acct) {

+ // Nothing of this account's stands behind a claim with no

+ // record: let it go, so a refusal never blocks the domain's

+ // real owner, and no orphan sweep is ever pointed at it.

+ if !hadRecord {

+ if rerr := releaseDomainClaim(ctx, s, domain, acct); rerr != nil {

+ log.Printf("mail: releasing claim on refused %s: %v", domain, rerr)

+ }

+ return nil, nil, fmt.Errorf("mail: %s in %s: %w", domain, d.Region, ErrIdentityNotOwned)

+ }

+ adopted = true

+ ident, err = p.EnsureIdentity(ctx, d.Region, domain, mailFrom, true)

+ }

if err != nil {

return nil, nil, fmt.Errorf("mail: ensure identity %s in %s: %w", domain, d.Region, err)

}

+ if ident.Created {

+ // Stamped before anything else can fail, so a crash from here on

+ // leaves a claim that says this identity is CARLOS's: the re-run

+ // keeps it, and the orphan sweeps may delete it.

+ adopted = false

+ if err := stampClaimIdentity(ctx, s, domain, acct, d.Region); err != nil {

+ return nil, nil, fmt.Errorf("mail: record identity for %s: %w", domain, err)

+ }

// configSet stays empty when no SNS topic is configured — recording a

// name for a set that was never created would leave later sends

@@ -288,7 +345,7 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec.

rec.Regions[d.Region] = &RegionState{

IdentityARN: ident.ARN, DKIMTokens: ident.DKIMTokens,

MailFrom: mailFrom, ConfigSet: configSet,

- Status: status, CheckedAt: now,

+ Status: status, CheckedAt: now, Adopted: adopted,

}

if _, err := PutDomain(ctx, s, acct, rec, etag); err != nil {

return nil, nil, err

@@ -439,6 +496,29 @@ func VerifyDomain(ctx context.Context, s release.Store, p Provisioner, d Deploym

return nil, fmt.Errorf("mail: domain %s has no identity in %s", domain, region)

}

+ // A record whose claim another account now holds — it lapsed unverified

+ // and was taken (#216) — must not verify into a grant on what is now

+ // somebody else's identity. It reads failed, and SES is not asked. No

+ // claim at all is a record from before the claim gate, and verifies as

+ // it always did.

+ claim, err := getDomainClaim(ctx, s, domain)

+ if err != nil {

+ return nil, err

+ }

+ if claim != nil && claim.Account != acct {

+ wasVerified := st.Status == StatusVerified

+ st.Status, st.CheckedAt = StatusFailed, now

+ if _, err := PutDomain(ctx, s, acct, rec, etag); err != nil {

+ return nil, err

+ }

+ if wasVerified {

+ if err := RefreshPolicies(ctx, s, p, acct, rec.Apps); err != nil {

+ return nil, err

+ }

+ return nil, fmt.Errorf("mail: verify %s: %w", domain, ErrDomainClaimed)

+ }

got, err := p.IdentityStatus(ctx, region, domain)

if err != nil {

return nil, fmt.Errorf("mail: identity status for %s in %s: %w", domain, region, err)

@@ -488,6 +568,14 @@ func VerifyDomain(ctx context.Context, s release.Store, p Provisioner, d Deploym

if _, err := PutDomain(ctx, s, acct, rec, etag); err != nil {

return nil, err

}

+ // Once verified, the claim never lapses. A failed stamp is only logged:

+ // the record's own verified status protects the claim just the same

+ // until the next verify retries it.

+ if st.Status == StatusVerified && claim != nil && claim.VerifiedAt == nil {

+ if err := stampClaimVerified(ctx, s, domain, acct, now); err != nil {

+ log.Printf("mail: stamping verified on claim %s: %v", domain, err)

+ }

// Grant after rebind, for #279's reason seen from the other side: a

// repair that fixes SES and the record but leaves the IAM policy naming

@@ -1247,8 +1335,34 @@ func detachDomainApp(ctx context.Context, s release.Store, p Provisioner, w dnsr

// Last referrer: remove the identity and, for a platform domain, the

// records CARLOS published. A custom domain's records are the

// customer's to remove, so they come back to the caller instead.

+ //

+ // Except where the identity is not this account's to remove (#216): one

+ // it adopted with proof, or one whose claim lapsed and now belongs to

+ // another account. Those keep their identity and their DNS; only this

+ // account's tenant association goes, so the owner can still delete the

+ // identity later.

+ //

+ // The identity itself is deleted only when the claim says this

+ // account's walk CREATED it, the same rule the orphan sweeps use. A

+ // record written before that stamp existed may have adopted somebody

+ // else's identity (the #216 attack itself, or an operator domain

+ // shared with another product), and nothing on it says which; such an

+ // identity is left in SES. Leaking it is the safe direction.

+ claim, err := getDomainClaim(ctx, s, cur.Domain)

+ if err != nil {

+ return detachNone, nil, err

+ }

+ ours := claim == nil || claim.Account == acct

var orphaned []dnsrec.Record

for region, st := range cur.Regions {

+ if !ours || st.Adopted {

+ if st.IdentityARN != "" {

+ if err := p.DisassociateTenantResource(ctx, TenantName(acct), st.IdentityARN); err != nil {

+ log.Printf("mail: disassociating tenant resource for %s/%s: %v", acct, cur.Domain, err)

+ }

+ continue

+ }

records := RecordSet(cur.Domain, region, st.DKIMTokens, d.DMARCRua)

// Same w != nil guard as EnsureDomain's publish branch, for the

// identical reason: a caller with no DNSWriter configured must not

@@ -1291,8 +1405,10 @@ func detachDomainApp(ctx context.Context, s release.Store, p Provisioner, w dnsr

log.Printf("mail: disassociating tenant resource for %s/%s: %v", acct, cur.Domain, err)

}

- if err := p.DeleteIdentity(ctx, region, cur.Domain); err != nil {

- return detachNone, nil, err

+ if claim != nil && claim.createdIdentityIn(region) {

+ if err := p.DeleteIdentity(ctx, region, cur.Domain); err != nil {

+ return detachNone, nil, err

+ }

}

// The global claim (Task 17a, spec §4a) releases HERE, in the same

@@ -1459,8 +1575,14 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, w dnsrec.Writ

// fleet-wide), so that is the only region this needs to check.

// DeleteIdentity is idempotent — "already gone" is success — so

// this is free when no identity was ever actually created.

- if err := p.DeleteIdentity(ctx, d.Region, c.Domain); err != nil {

- return err

+ //

+ // Only an identity the claim says this walk CREATED (#216). One it

+ // found already there was never adopted (EnsureDomain refuses and

+ // releases such a claim), so it is somebody else's.

+ if c.createdIdentityIn(d.Region) {

+ if err := p.DeleteIdentity(ctx, d.Region, c.Domain); err != nil {

+ return err

+ }

}

if err := releaseDomainClaim(ctx, s, c.Domain, acct); err != nil {

return err

@@ -1653,8 +1775,11 @@ func removeOrphanedClaim(ctx context.Context, s release.Store, p Provisioner,

if claim == nil || claim.Account != acct {

return nil, fmt.Errorf("mail: no sending domain %s for this account: %w", domain, ErrDomainNotReferenced)

}

- if err := p.DeleteIdentity(ctx, d.Region, domain); err != nil {

- return nil, err

+ // Only an identity this claim's walk created, as in Teardown's sweep.

+ if claim.createdIdentityIn(d.Region) {

+ if err := p.DeleteIdentity(ctx, d.Region, domain); err != nil {

+ return nil, err

+ }

}

if err := releaseDomainClaim(ctx, s, domain, acct); err != nil {

return nil, err

diff --git a/internal/mail/walk_identityowner_test.go b/internal/mail/walk_identityowner_test.go

new file mode 100644

index 000000000..353272360

--- /dev/null

+++ b/internal/mail/walk_identityowner_test.go

@@ -0,0 +1,332 @@

+// Discussion #216: a member must not be able to take over an SES identity

+// this account's claim did not create, and a sending-domain claim that was

+// never verified must not hold a domain forever.

+package mail

+import (

+ "context"

+ "errors"

+ "testing"

+ "time"

+ "github.com/carlosframework/platform/internal/dnsrec"

+ "github.com/carlosframework/platform/internal/release"

+// seedForeignIdentity stands in an identity verified in SES outside CARLOS:

+// tito.io in spec 2026-09-15 §1, an operator's own sign-in domain, or one

+// created before the claim gate existed.

+func seedForeignIdentity(p *MemProvisioner, region, domain string) *MemIdentity {

+ id := &MemIdentity{

+ ARN: "arn:aws:ses:" + region + ":shared:identity/" + domain,

+ DKIMTokens: []string{"classic1", "classic2", "classic3"},

+ MailFrom: "bounce." + domain,

+ ConfigSet: "classic-events",

+ Status: Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"},

+ }

+ p.Identities[identityKey(region, domain)] = id

+ return id

+// The HIGH finding as it stood: a member's `domains add` on a domain someone

+// else verified in SES adopted the identity, rewrote its MAIL FROM, and

+// would have bound and later deleted it.

+func TestEnsureDomainRefusesAnIdentityItDidNotCreate(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ foreign := seedForeignIdentity(p, d.Region, "tito.io")

+ _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "jelly", "tito.io", now)

+ if !errors.Is(err, ErrIdentityNotOwned) {

+ t.Fatalf("EnsureDomain on a foreign identity = %v, want ErrIdentityNotOwned", err)

+ }

+ if foreign.MailFrom != "bounce.tito.io" || foreign.ConfigSet != "classic-events" {

+ t.Fatalf("the foreign identity was mutated: MailFrom=%q ConfigSet=%q", foreign.MailFrom, foreign.ConfigSet)

+ }

+ if rec, _, err := GetDomain(ctx, s, "aaa", "tito.io"); err != nil || rec != nil {

+ t.Fatalf("a domain record was written for a refused domain: %+v %v", rec, err)

+ }

+ // The refusal leaves nothing holding the domain: no claim to block the

+ // real owner, and nothing a later Teardown could delete.

+ if c, err := getDomainClaim(ctx, s, "tito.io"); err != nil || c != nil {

+ t.Fatalf("a refused domain kept its claim: %+v %v", c, err)

+ }

+ if err := Teardown(ctx, s, p, w, d, "aaa", "jelly"); err != nil {

+ t.Fatalf("teardown: %v", err)

+ }

+ if _, ok := p.Identities[identityKey(d.Region, "tito.io")]; !ok {

+ t.Fatal("teardown deleted an identity this account never created")

+ }

+// fakeTXT is the injectable resolver: name -> TXT values.

+type fakeTXT map[string][]string

+func (f fakeTXT) LookupTXT(_ context.Context, name string) ([]string, error) {

+ if v, ok := f[name]; ok {

+ return v, nil

+ }

+ return nil, errors.New("no such host")

+// Proof of DNS control is the one way in: the claimant publishes a TXT

+// record naming its own account, and only then is the existing identity

+// adopted. Teardown still never deletes it, because CARLOS did not create

+// it.

+func TestEnsureDomainAdoptsAForeignIdentityOnlyWithDNSProof(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ seedForeignIdentity(p, d.Region, "tito.io")

+ // A proof naming a DIFFERENT account proves nothing for this one.

+ d.Resolver = fakeTXT{ProofRecordName("tito.io"): {ProofRecordValue("bbb")}}

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "jelly", "tito.io", now); !errors.Is(err, ErrIdentityNotOwned) {

+ t.Fatalf("another account's proof was accepted: %v", err)

+ }

+ d.Resolver = fakeTXT{ProofRecordName("tito.io"): {"v=spf1 -all", ProofRecordValue("aaa")}}

+ rec, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "jelly", "tito.io", now)

+ if err != nil {

+ t.Fatalf("EnsureDomain with a valid proof: %v", err)

+ }

+ st := rec.Regions[d.Region]

+ if !st.Adopted {

+ t.Fatal("an adopted identity was not recorded as adopted")

+ }

+ if st.DKIMTokens[0] != "classic1" {

+ t.Fatalf("adoption did not keep the identity's published DKIM tokens: %v", st.DKIMTokens)

+ }

+ // A re-run needs no proof any more: the record is this account's.

+ d.Resolver = nil

+ again, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "jelly", "tito.io", now)

+ if err != nil {

+ t.Fatalf("re-run after adoption: %v", err)

+ }

+ if got := again.Regions[d.Region]; !got.Adopted {

+ t.Fatal("re-run lost the adopted flag")

+ }

+ if err := Teardown(ctx, s, p, w, d, "aaa", "jelly"); err != nil {

+ t.Fatalf("teardown: %v", err)

+ }

+ if _, ok := p.Identities[identityKey(d.Region, "tito.io")]; !ok {

+ t.Fatal("teardown deleted an adopted identity")

+ }

+ if c, _ := getDomainClaim(ctx, s, "tito.io"); c != nil {

+ t.Fatalf("teardown left the claim behind: %+v", c)

+ }

+// Identities CARLOS itself created for this account keep working with no

+// proof: a re-run, a sibling app, and the crash between EnsureIdentity and

+// the record (the claim remembers the create).

+func TestEnsureDomainKeepsItsOwnIdentitiesWithoutProof(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ p.FailNext["EnsureConfigSet"] = errors.New("throttle")

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app1", "mine.com", now); err == nil {

+ t.Fatal("expected the injected failure")

+ }

+ rec, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app1", "mine.com", now)

+ if err != nil {

+ t.Fatalf("re-run after a crash past EnsureIdentity: %v", err)

+ }

+ if rec.Regions[d.Region].Adopted {

+ t.Fatal("an identity CARLOS created was recorded as adopted")

+ }

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app2", "mine.com", now); err != nil {

+ t.Fatalf("sibling app: %v", err)

+ }

+// A record written before this change (no claim stamp, no Adopted field)

+// still re-ensures and verifies. Teardown no longer deletes its identity:

+// nothing says CARLOS created it, so it is left in SES.

+func TestEnsureDomainHonoursALegacyRecord(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ id := seedForeignIdentity(p, d.Region, "old.com") // created by a pre-gate walk

+ legacy := &DomainRecord{Domain: "old.com", Kind: KindCustom, Apps: []string{"app1"}, CreatedAt: now,

+ Regions: map[string]*RegionState{d.Region: {IdentityARN: id.ARN, DKIMTokens: id.DKIMTokens, Status: StatusVerified}}}

+ if _, err := PutDomain(ctx, s, "aaa", legacy, ""); err != nil {

+ t.Fatal(err)

+ }

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app1", "old.com", now); err != nil {

+ t.Fatalf("legacy re-ensure: %v", err)

+ }

+ if _, err := VerifyDomain(ctx, s, p, d, "aaa", "old.com", d.Region, now); err != nil {

+ t.Fatalf("legacy verify: %v", err)

+ }

+ if err := Teardown(ctx, s, p, w, d, "aaa", "app1"); err != nil {

+ t.Fatalf("teardown: %v", err)

+ }

+ if _, ok := p.Identities[identityKey(d.Region, "old.com")]; !ok {

+ t.Fatal("teardown deleted a legacy record's identity that nothing says CARLOS created")

+ }

+ if c, _ := getDomainClaim(ctx, s, "old.com"); c != nil {

+ t.Fatalf("teardown left the claim behind: %+v", c)

+ }

+// The reviewer's case: on main, the #216 attack itself left a record that

+// adopted a foreign identity, with a claim and no stamp. `domains rm` after

+// this change must not delete the real owner's identity.

+func TestRemoveDomainSparesALegacyAdoptedIdentity(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ id := seedForeignIdentity(p, d.Region, "tito.io")

+ if err := claimDomain(ctx, s, "tito.io", "aaa", "jelly", now); err != nil {

+ t.Fatal(err)

+ }

+ legacy := &DomainRecord{Domain: "tito.io", Kind: KindCustom, Apps: []string{"jelly"}, CreatedAt: now,

+ Regions: map[string]*RegionState{d.Region: {IdentityARN: id.ARN, DKIMTokens: id.DKIMTokens, Status: StatusVerified}}}

+ if _, err := PutDomain(ctx, s, "aaa", legacy, ""); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := RemoveDomain(ctx, s, p, w, d, "aaa", "jelly", "tito.io"); err != nil {

+ t.Fatalf("RemoveDomain: %v", err)

+ }

+ if _, ok := p.Identities[identityKey(d.Region, "tito.io")]; !ok {

+ t.Fatal("domains rm deleted an identity a pre-fix record had adopted")

+ }

+ if c, _ := getDomainClaim(ctx, s, "tito.io"); c != nil {

+ t.Fatalf("the claim was not released: %+v", c)

+ }

+// A holder that verifies between the lapse check and the takeover keeps

+// its claim and its identity: the verified_at stamp moves the etag.

+func TestLapsedTakeoverLosesToAHolderThatVerifies(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app", "race.com", now); err != nil {

+ t.Fatal(err)

+ }

+ later := now.Add(ClaimLapse + time.Hour)

+ // Judge the lapse on the current etag, then let the holder verify.

+ cur, etag, err := getDomainClaimETag(ctx, s, "race.com")

+ if err != nil {

+ t.Fatal(err)

+ }

+ if lapsed, err := claimLapsed(ctx, s, p, cur, d.Region, later); err != nil || !lapsed {

+ t.Fatalf("setup: claim should read lapsed: %v %v", lapsed, err)

+ }

+ p.SetIdentityStatus(d.Region, "race.com", Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"})

+ if _, err := VerifyDomain(ctx, s, p, d, "aaa", "race.com", d.Region, later); err != nil {

+ t.Fatal(err)

+ }

+ body := []byte(`{"schema":1,"domain":"race.com","account":"bbb","created_at":"2026-10-11T13:00:00Z"}`)

+ if _, err := s.Put(ctx, domainClaimKey("race.com"), body, etag); !errors.Is(err, release.ErrCASConflict) {

+ t.Fatalf("a takeover on the pre-verify etag = %v, want a CAS conflict", err)

+ }

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "bbb", "app", "race.com", later); !errors.Is(err, ErrDomainClaimed) {

+ t.Fatalf("takeover after the holder verified = %v, want ErrDomainClaimed", err)

+ }

+ if _, ok := p.Identities[identityKey(d.Region, "race.com")]; !ok {

+ t.Fatal("the verified holder lost its identity")

+ }

+// Finding 2: an unverified claim lapses after ClaimLapse and another

+// account may take the domain. The squatter's half-made identity goes with

+// it, so the new claimant starts fresh, with DKIM tokens of its own.

+func TestUnverifiedClaimLapses(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ first, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "squat", "victim.com", now)

+ if err != nil {

+ t.Fatal(err)

+ }

+ squatTokens := first.Regions[d.Region].DKIMTokens

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "bbb", "real", "victim.com", now.Add(ClaimLapse-time.Hour)); !errors.Is(err, ErrDomainClaimed) {

+ t.Fatalf("a claim inside its lapse window was taken: %v", err)

+ }

+ rec, _, err := EnsureDomain(ctx, s, p, w, d, "bbb", "real", "victim.com", now.Add(ClaimLapse+time.Hour))

+ if err != nil {

+ t.Fatalf("a lapsed claim was not released to another account: %v", err)

+ }

+ if got := rec.Regions[d.Region].DKIMTokens; got[0] == squatTokens[0] {

+ t.Fatalf("the new holder inherited the lapsed holder's identity: %v", got)

+ }

+ if c, _ := getDomainClaim(ctx, s, "victim.com"); c == nil || c.Account != "bbb" {

+ t.Fatalf("claim after lapse = %+v, want bbb's", c)

+ }

+ // The lapsed holder can neither verify its stale record into a grant

+ // nor tear down the identity that is now bbb's.

+ p.SetIdentityStatus(d.Region, "victim.com", Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"})

+ if _, err := VerifyDomain(ctx, s, p, d, "aaa", "victim.com", d.Region, now.Add(ClaimLapse+2*time.Hour)); !errors.Is(err, ErrDomainClaimed) {

+ t.Fatalf("lapsed holder's verify = %v, want ErrDomainClaimed", err)

+ }

+ if old, _, _ := GetDomain(ctx, s, "aaa", "victim.com"); old == nil || old.Regions[d.Region].Status == StatusVerified {

+ t.Fatalf("lapsed holder's record reads verified: %+v", old)

+ }

+ if err := Teardown(ctx, s, p, w, d, "aaa", "squat"); err != nil {

+ t.Fatalf("lapsed holder's teardown: %v", err)

+ }

+ if _, ok := p.Identities[identityKey(d.Region, "victim.com")]; !ok {

+ t.Fatal("the lapsed holder's teardown deleted the new holder's identity")

+ }

+ if c, _ := getDomainClaim(ctx, s, "victim.com"); c == nil || c.Account != "bbb" {

+ t.Fatalf("the lapsed holder's teardown disturbed bbb's claim: %+v", c)

+ }

+// A verified claim never lapses, even if its record later reads failed —

+// and neither does one SES has verified while nobody ran verify.

+func TestVerifiedClaimDoesNotLapse(t *testing.T) {

+ ctx := context.Background()

+ s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter()

+ p := NewMemProvisioner("shared")

+ now := time.Date(2026, 10, 3, 12, 0, 0, 0, time.UTC)

+ d := testDeployment()

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app", "kept.com", now); err != nil {

+ t.Fatal(err)

+ }

+ p.SetIdentityStatus(d.Region, "kept.com", Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"})

+ if _, err := VerifyDomain(ctx, s, p, d, "aaa", "kept.com", d.Region, now); err != nil {

+ t.Fatal(err)

+ }

+ p.SetIdentityStatus(d.Region, "kept.com", Status{DKIM: "FAILED", MailFrom: "SUCCESS", Sending: "SUCCESS"})

+ if _, err := VerifyDomain(ctx, s, p, d, "aaa", "kept.com", d.Region, now); err != nil {

+ t.Fatal(err)

+ }

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "bbb", "app", "kept.com", now.Add(10*ClaimLapse)); !errors.Is(err, ErrDomainClaimed) {

+ t.Fatalf("a once-verified claim lapsed: %v", err)

+ }

+ // Verified at SES, never verified through CARLOS.

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app", "quiet.com", now); err != nil {

+ t.Fatal(err)

+ }

+ p.SetIdentityStatus(d.Region, "quiet.com", Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"})

+ if _, _, err := EnsureDomain(ctx, s, p, w, d, "bbb", "app", "quiet.com", now.Add(10*ClaimLapse)); !errors.Is(err, ErrDomainClaimed) {

+ t.Fatalf("a claim SES had verified lapsed: %v", err)

+ }

diff --git a/mail_cli.go b/mail_cli.go

index 81711c54c..3d39b9008 100644

--- a/mail_cli.go

+++ b/mail_cli.go

@@ -381,11 +381,14 @@ func isEmailNotDeclared(err error) bool {

// naming the PERMANENT ones (409 claimed, 400 not-owned) without a shred of

// retry language — the exact mistake the brief calls out. Every other

// error passes through apiErrorFromBody's own text unchanged.

+//

+// The console's own words carry the advice. This used to add "choose a

+// different domain" to every 409, which is wrong for the refusal that says

+// how to prove control of the domain (#216).

func describeDomainEnsureErr(domain string, err error) error {

var me *apiMailError

if errors.As(err, &me) && me.permanent() {

- return fmt.Errorf("%s: %s — permanent: nothing about running this command again would change the "+

- "outcome; choose a different domain", domain, me.Error())

+ return fmt.Errorf("%s: %s", domain, me.Error())

}

return fmt.Errorf("ensuring domain %s: %w", domain, err)

}

diff --git a/mail_cli_test.go b/mail_cli_test.go

index ed278cbb1..74b0f88aa 100644

--- a/mail_cli_test.go

+++ b/mail_cli_test.go

@@ -112,7 +112,7 @@ func TestAPISessionEmailDomainEnsureWireShape(t *testing.T) {

func TestAPISessionEmailDomainEnsureConflictIsPermanent(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

w.WriteHeader(http.StatusConflict)

- json.NewEncoder(w).Encode(map[string]any{"error": "this domain is already claimed by another CARLOS account — choose a different domain"})

+ json.NewEncoder(w).Encode(map[string]any{"error": "sending domain unavailable"})

}))

defer ts.Close()

@@ -704,7 +704,7 @@ func newFakeEmailConsole(t *testing.T, verifiedAfter int) *fakeEmailConsole {

case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/email/domains"):

if fc.domainConflict {

w.WriteHeader(http.StatusConflict)

- json.NewEncoder(w).Encode(map[string]any{"error": "this domain is already claimed by another CARLOS account — choose a different domain"})

+ json.NewEncoder(w).Encode(map[string]any{"error": "sending domain unavailable"})

return

}

pending := []map[string]any{}