amadan
mail: scope the orphan-claim sweep to its own app, destroy inherited identities (Task 17a review, round 2)
Round 2 of review on the domain-claim fix (a012913). Four items, three opened by this round. Item 1 (Important): the orphan-claim sweep in Teardown released ANY claim the account held with no backing record, regardless of which app attempted it. An unrelated Teardown(acct, app2) could release app3's claim while app3 was mid-retry — and the window here is not a microsecond race, it is however long the member takes to retry after a throttle: a quota bump or a support ticket, hours. Fixed by adding an App field to DomainClaim (set once, at claimDomain's first write, never rewritten by a later idempotent re-claim) and scoping the sweep to c.App == app. Teardown now releases only orphans the app being torn down actually created. Item 2 (Important): the sweep released a claim without ever calling DeleteIdentity, so a crash landing after EnsureIdentity but before PutDomain left a live SES identity the sweep would hand to whoever claimed next — same ARN, same already-minted DKIM tokens, no DNS proof required. Fixed by having the sweep call DeleteIdentity (idempotent, "already gone" is success) in the deployment's region before releasing. Item 3 (coverage): platformNameOwnedBy's `label == acct` comparison had no test pinning it as EQUALITY — both HasSuffix and HasPrefix substitutions left the suite green, and the "aa" vs "aaa" case wasn't covered. Added TestPlatformNameOwnedByRequiresExactLabelEquality, a table covering exact match, strict-prefix, strict-superset, and (closing a separate coverage gap) the bare apex with an account literally named "com" — the one case that actually distinguishes the early-return guard from the fallback logic happening to compute "com" and refuse by coincidence. Item 4 (promoted from Minor — CRITICAL in effect): mailDeploymentFor cleared Deployment.PlatformSuffix whenever DNSWriter was nil, to make a platform-kind domain degrade to custom (records emitted instead of published) without panicking on a nil writer. That also silently disabled platformNameOwnedBy for the whole deployment shape: with no suffix, DomainKindFor can only ever answer custom, so the ownership check never runs, and any account can take the global claim on any OTHER account's platform name. Fixed by keeping the suffix always, and gating record- writing (not ownership) on w != nil directly in EnsureDomain and Teardown — "is this a platform name, and whose" is a question the CONFIGURED suffix answers unconditionally; "do we have somewhere to publish" is the only question DNSWriter answers. Two cheap test gaps closed alongside: the bare-apex guard (folded into item 3's table) and the console door's ErrNotOwnPlatformName -> 400 branch (TestAPIEmailDomainEnsureRefusesAPlatformNameWithNoConfiguredWriter, which also exercises item 4 directly). Fifteen mutations run and caught: the six new to this round (label==acct -> HasSuffix, -> HasPrefix, delete the bare-apex guard, sweep releases another app's orphan, sweep skips DeleteIdentity, door drops the 400 branch) plus the nine from the previous round, re-verified against the current code shape.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 files changed,
+273
−49
internal/console/apimail.go+21 −17internal/console/apimail_test.go+37 −8internal/mail/domainclaim.go+19 −8internal/mail/walk.go+59 −16internal/mail/walk_domainclaim_test.go+137 −0
diff --git a/internal/console/apimail.go b/internal/console/apimail.go| index 7f07755a5..3eaa8152d 100644 |
| --- a/internal/console/apimail.go |
| +++ b/internal/console/apimail.go |
| @@ -288,27 +288,31 @@ func (s *Server) mailAppDomains(ctx context.Context, acct *Account, app string) |
| } |
| // mailDeploymentFor adapts s.cfg.MailDeployment and s.cfg.DNSWriter for one |
| -// EnsureDomain call. |
| +// EnsureDomain (or Teardown) call. |
| // |
| // Config.DNSWriter's own doc comment promises: "nil means platform domains |
| // behave like custom ones: records are emitted for the member instead of |
| -// published." mail.EnsureDomain itself has no such nil handling — it calls |
| -// w.EnsureRecord unconditionally for a platform-kind domain, which would |
| -// panic on a nil Writer — because DomainKindFor decides platform-vs-custom |
| -// purely from d.PlatformSuffix. Clearing PlatformSuffix here is what makes |
| -// the documented behaviour true: with no suffix, DomainKindFor can only ever |
| -// answer KindCustom, so the placeholder Writer below is provably never |
| -// called. This is this task's first caller of DNSWriter; the adaptation |
| -// belongs here, not in mail.EnsureDomain, which has no opinion on what "no |
| -// writer" should degrade to. |
| +// published." The FIRST cut of this adaptation (this task, before review) |
| +// read that promise too broadly and cleared d.PlatformSuffix whenever |
| +// DNSWriter was nil — which also, silently, turned off C1's platform-name |
| +// ownership check (mail.platformNameOwnedBy only ever runs when |
| +// DomainKindFor answers KindPlatform, and DomainKindFor can only answer |
| +// that when PlatformSuffix is set). In a deployment with no DNSWriter |
| +// configured, ANY account could then take the global claim on ANY other |
| +// account's platform name — no DKIM gets published so it is denial-of-name |
| +// rather than a send capability, but the victim still gets 409'd off their |
| +// own name (I4, review round 2). |
| +// |
| +// The fix keeps the two questions separate, because they ARE separate: |
| +// "is this domain platform-kind, and whose name is it" (decided from the |
| +// CONFIGURED suffix, unconditionally) versus "do we have somewhere to |
| +// actually publish DNS" (decided from DNSWriter alone). d.PlatformSuffix |
| +// is therefore left untouched here; mail.EnsureDomain and mail.Teardown |
| +// gate their record-writing calls on w != nil themselves, not on Kind |
| +// alone, so a nil writer degrades ONLY publishing to the documented |
| +// pending/no-op behaviour and never touches ownership. |
| func (s *Server) mailDeploymentFor() (mail.Deployment, dnsrec.Writer) { |
| - d := s.cfg.MailDeployment |
| - w := s.cfg.DNSWriter |
| - if w == nil { |
| - d.PlatformSuffix = "" |
| - w = dnsrec.NewMemWriter() |
| - } |
| - return d, w |
| + return s.cfg.MailDeployment, s.cfg.DNSWriter |
| } |
| // dnsRecordOut renders a dnsrec.Record in this API's lowercase-key style — |
diff --git a/internal/console/apimail_test.go b/internal/console/apimail_test.go| index 5be2ef963..611578d46 100644 |
| --- a/internal/console/apimail_test.go |
| +++ b/internal/console/apimail_test.go |
| @@ -437,26 +437,55 @@ func TestAPIEmailDomainEnsureReturnsPendingForACustomDomain(t *testing.T) { |
| } |
| } |
| -// TestAPIEmailDomainEnsureWithNilDNSWriterDegradesToCustom proves |
| -// mailDeploymentFor's adaptation: a deployment with no DNSWriter must not |
| -// panic on a platform-suffixed domain — it has to hand back pending records |
| -// instead, exactly Config.DNSWriter's own documented contract. |
| -func TestAPIEmailDomainEnsureWithNilDNSWriterDegradesToCustom(t *testing.T) { |
| +// TestAPIEmailDomainEnsureWithNilDNSWriterDegradesPublishingOnly proves |
| +// mailDeploymentFor's corrected adaptation (I4, review round 2): a |
| +// deployment with no DNSWriter must not panic on a platform-suffixed |
| +// domain, and it must hand back pending records for the member to publish |
| +// by hand — Config.DNSWriter's own documented contract — but Kind must stay |
| +// "platform", NOT get silently relabeled "custom". The first cut of this |
| +// fix cleared PlatformSuffix to sidestep the nil writer, which also turned |
| +// off C1's ownership check for this whole deployment shape (with no suffix, |
| +// DomainKindFor can only ever answer custom, so platformNameOwnedBy never |
| +// runs at all). Kind staying "platform" is what proves the ownership check |
| +// ran — see TestAPIEmailDomainEnsureRefusesAPlatformNameWithNoConfiguredWriter |
| +// for the direct security assertion. |
| +func TestAPIEmailDomainEnsureWithNilDNSWriterDegradesPublishingOnly(t *testing.T) { |
| fx, _, _, token := emailArmed(t, "code-email-domain-nowriter") |
| fx.srv.cfg.DNSWriter = nil |
| - app, domain := "titogo", "titogo.bab.oncarlos.com" // under the platform suffix |
| + app, domain := "titogo", "titogo.bab.oncarlos.com" // under the platform suffix, owned by fx.sqid ("bab") |
| declareEmail(t, fx, token, app, "hello@"+domain) |
| out := ensureEmailDomain(t, fx, token, app, domain) |
| dom := block(t, out, "domain") |
| - if dom["kind"] != mail.KindCustom { |
| - t.Fatalf("kind = %v, want custom (no DNSWriter degrades a platform domain to custom)", dom["kind"]) |
| + if dom["kind"] != mail.KindPlatform { |
| + t.Fatalf("kind = %v, want platform — no DNSWriter must degrade PUBLISHING only, never the domain's kind", dom["kind"]) |
| } |
| if pending := pendingList(t, out); len(pending) == 0 { |
| t.Fatal("pending is empty — a platform domain with no DNSWriter must still hand back records") |
| } |
| } |
| +// TestAPIEmailDomainEnsureRefusesAPlatformNameWithNoConfiguredWriter (I4, |
| +// review round 2): the ownership check must run even in a deployment with |
| +// no DNSWriter configured. Before this fix, mailDeploymentFor cleared |
| +// PlatformSuffix whenever DNSWriter was nil, which silently disabled |
| +// platformNameOwnedBy for the whole deployment — any account could take |
| +// the global claim on any OTHER account's platform name in that shape, |
| +// locking the real owner out of their own name (denial-of-name; no DKIM |
| +// gets published, so not a send capability, but still a 409 against the |
| +// rightful owner). |
| +func TestAPIEmailDomainEnsureRefusesAPlatformNameWithNoConfiguredWriter(t *testing.T) { |
| + fx, _, _, token := emailArmed(t, "code-email-domain-nowriter-ownership") |
| + fx.srv.cfg.DNSWriter = nil |
| + app := "titogo" |
| + declareEmail(t, fx, token, app, "hello@x.aaa.oncarlos.com") |
| + |
| + status, out := postJSON(t, emailURL(fx, app)+"/domains", map[string]any{"domain": "x.aaa.oncarlos.com"}, token) |
| + if status != http.StatusBadRequest { |
| + t.Fatalf("ensure a platform name for a DIFFERENT account (aaa), fx account is bab = %d, want 400: %v", status, out) |
| + } |
| +} |
| + |
| // TestAPIEmailDomainVerifyRequiresAnEnsuredDomain: verifying a domain that |
| // was never ensured is a speaking 404, not a crash on a missing region. |
| func TestAPIEmailDomainVerifyRequiresAnEnsuredDomain(t *testing.T) { |
diff --git a/internal/mail/domainclaim.go b/internal/mail/domainclaim.go| index 18a1ee612..53f2f90ca 100644 |
| --- a/internal/mail/domainclaim.go |
| +++ b/internal/mail/domainclaim.go |
| @@ -53,9 +53,17 @@ func domainClaimKey(domain string) string { return domainClaimsPrefix + domain + |
| // CLAUDE.md's operate-via-API rule treats as a product gap to close, not a |
| // workaround to reach for. |
| type DomainClaim struct { |
| - Schema int `json:"schema"` // 1 |
| - Domain string `json:"domain"` |
| - Account string `json:"account"` |
| + Schema int `json:"schema"` // 1 |
| + Domain string `json:"domain"` |
| + Account string `json:"account"` |
| + // App is the app that FIRST claimed the domain — attribution for the |
| + // orphan-claim sweep in Teardown (I1 round 2), and nothing else. It is |
| + // omitempty for wire-additivity: a claim written before this field |
| + // existed has none, and a legacy orphan with an empty App simply never |
| + // matches any real Teardown call (ValidApp requires a non-empty app), |
| + // which is the safe direction — unsweepable rather than swept by the |
| + // wrong caller. |
| + App string `json:"app,omitempty"` |
| CreatedAt time.Time `json:"created_at"` |
| } |
| @@ -102,11 +110,14 @@ func getDomainClaimETag(ctx context.Context, s release.Store, domain string) (*D |
| // at domainClaimKey(domain) to Put. |
| // |
| // Same account re-claiming is idempotent success (a re-run of EnsureDomain |
| -// by its own owner must never be refused by the gate it itself holds). Any |
| -// other account gets ErrDomainClaimed, with no identity ever minted and no |
| -// domain record ever written — the refusal happens before either exists. |
| -func claimDomain(ctx context.Context, s release.Store, domain, acct string, now time.Time) error { |
| - body, err := json.Marshal(DomainClaim{Schema: 1, Domain: domain, Account: acct, CreatedAt: now}) |
| +// by its own owner must never be refused by the gate it itself holds), and |
| +// deliberately does NOT rewrite App on that no-op path — App records only |
| +// who FIRST claimed the domain, for the orphan sweep's attribution (I1 |
| +// 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. |
| +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 { |
| return err |
| } |
diff --git a/internal/mail/walk.go b/internal/mail/walk.go| index 414ed3a62..b222d9e8c 100644 |
| --- a/internal/mail/walk.go |
| +++ b/internal/mail/walk.go |
| @@ -168,7 +168,7 @@ 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, now); err != nil { |
| + if err := claimDomain(ctx, s, domain, acct, app, now); 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 |
| @@ -215,7 +215,18 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec. |
| records := RecordSet(domain, d.Region, ident.DKIMTokens, d.DMARCRua) |
| var pending []dnsrec.Record |
| - if rec.Kind == KindPlatform { |
| + // GATED ON w != nil, not on rec.Kind alone (C1/item-4, review |
| + // follow-up round 2). Kind still answers truthfully from the |
| + // CONFIGURED suffix — that is what platformNameOwnedBy's ownership |
| + // check depends on above, and that check must run whether or not a |
| + // publisher is wired up, or a deployment with no DNSWriter silently |
| + // stops enforcing ownership on its own platform names (the exact bug |
| + // this round closed: ownership and publishing are two different |
| + // questions, and only publishing has any business asking about w). |
| + // nil w degrades a platform-kind domain to pending exactly like a |
| + // custom one — never a call on a nil interface, and never a "publish" |
| + // into some throwaway writer nobody durable ever reads. |
| + if rec.Kind == KindPlatform && w != nil { |
| for _, r := range records { |
| if err := w.EnsureRecord(ctx, r); err != nil { |
| // Deliberately BEFORE the record is written: a failed |
| @@ -964,7 +975,14 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, w dnsrec.Writ |
| // the records CARLOS published. A custom domain's records are the |
| // customer's to remove. |
| for region, st := range cur.Regions { |
| - if cur.Kind == KindPlatform { |
| + // Same w != nil guard as EnsureDomain's publish branch, for |
| + // the identical reason: a caller with no DNSWriter configured |
| + // must not panic on a nil interface here either, even though |
| + // nothing in this codebase wires Teardown to such a caller |
| + // yet — the guard costs nothing and keeps the two record- |
| + // writing sites in this file from drifting onto different |
| + // nil-safety assumptions. |
| + if cur.Kind == KindPlatform && w != nil { |
| for _, r := range RecordSet(cur.Domain, region, st.DKIMTokens, d.DMARCRua) { |
| if err := w.DeleteRecord(ctx, r); err != nil { |
| return err |
| @@ -1013,27 +1031,52 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, w dnsrec.Writ |
| // by ListDomains(acct) above (there is no record), so the loop just |
| // run never sees it, and without this pass it would never be |
| // reachable by any release verb at all: the member's only recourse is |
| - // Teardown, and Teardown had nothing to walk it from. Reconcile |
| - // against every claim this ACCOUNT holds — not just ones this app is |
| - // known to have touched, because nothing durable records which app |
| - // attempted a domain that was never written, so per-app scoping is |
| - // not available here — and release any with no record anywhere in the |
| - // account. A claim that still has a record is left alone regardless |
| - // of which app owns it. |
| + // Teardown, and Teardown had nothing to walk it from. |
| + // |
| + // SCOPED TO THIS APP (I1 item 1, review round 2), corrected from this |
| + // task's first cut, which swept every claim the ACCOUNT held with no |
| + // record, regardless of which app attempted it. That is wrong: an |
| + // unrelated Teardown(acct, someOtherApp) would sweep up a DIFFERENT |
| + // app's claim that is not abandoned at all — just waiting on an |
| + // ordinary retry. The window where this matters is NOT a concurrency |
| + // race (microseconds between goroutines); it is however long the |
| + // member takes to retry — a quota bump or a support ticket, hours, not |
| + // microseconds — and nothing durable told the sweep "abandoned" from |
| + // "about to be retried" apart from which app the claim belongs to. |
| + // DomainClaim.App now carries that attribution (set once, at claimDomain's |
| + // first write, never rewritten by a later idempotent re-claim), so the |
| + // sweep only ever releases a claim THIS app created. |
| claims, err := ListDomainClaims(ctx, s) |
| if err != nil { |
| return err |
| } |
| for _, c := range claims { |
| - if c.Account != acct { |
| + if c.Account != acct || c.App != app { |
| continue |
| } |
| - if rec, _, err := GetDomain(ctx, s, acct, c.Domain); err != nil { |
| + dr, _, err := GetDomain(ctx, s, acct, c.Domain) |
| + if err != nil { |
| + return err |
| + } |
| + if dr != nil { |
| + continue // a record exists; the domain loop above already handles it |
| + } |
| + // DESTROY THE IDENTITY BEFORE RELEASING (I1 item 2, review round |
| + // 2). If the crash that left this claim orphaned landed AFTER |
| + // EnsureIdentity but BEFORE PutDomain, a live SES identity exists |
| + // with no record naming it — and releasing the claim without |
| + // deleting it hands a stranger that identity's ARN and its |
| + // already-minted DKIM tokens the moment they claim the domain, |
| + // with no DNS proof of their own. EnsureDomain only ever mints an |
| + // identity in d.Region (SMTPHost's own doc comment: one region, |
| + // 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 |
| + } |
| + if err := releaseDomainClaim(ctx, s, c.Domain, acct); err != nil { |
| return err |
| - } else if rec == nil { |
| - if err := releaseDomainClaim(ctx, s, c.Domain, acct); err != nil { |
| - return err |
| - } |
| } |
| } |
diff --git a/internal/mail/walk_domainclaim_test.go b/internal/mail/walk_domainclaim_test.go| index 0f63ce830..2b7aba74c 100644 |
| --- a/internal/mail/walk_domainclaim_test.go |
| +++ b/internal/mail/walk_domainclaim_test.go |
| @@ -434,3 +434,140 @@ func TestTeardownOrphanReconciliationLeavesRecordedClaimsAlone(t *testing.T) { |
| t.Fatalf("app2's orphaned claim survived its own teardown: %+v", claim) |
| } |
| } |
| + |
| +// ---- Round 2 (review follow-up): the sweep must not over-release, and -- |
| +// ---- must not hand a stranger a live identity ------------------------- |
| + |
| +// TestTeardownOrphanSweepScopedToTheAttemptingApp (I1 item 1, round 2): an |
| +// UNRELATED Teardown for a different app in the SAME account must never |
| +// release another app's in-flight claim. The window this closes is not a |
| +// microsecond race between goroutines — it is however long the member |
| +// takes to retry after a throttle (a quota bump, a support ticket: hours), |
| +// and nothing but the App attribution on the claim tells the sweep |
| +// "abandoned" from "about to be retried". |
| +func TestTeardownOrphanSweepScopedToTheAttemptingApp(t *testing.T) { |
| + ctx := context.Background() |
| + s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter() |
| + p := NewMemProvisioner("shared") |
| + now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC) |
| + d := testDeployment() |
| + |
| + // app2 is ordinary and unrelated to the domain under test. |
| + if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app2", "app2-domain.com", now); err != nil { |
| + t.Fatalf("app2 setup: %v", err) |
| + } |
| + |
| + // app3 leaves an orphan: claimed, no record — an ordinary retryable |
| + // state, not an abandonment. |
| + p.FailNext["EnsureIdentity"] = errors.New("SES throttled") |
| + if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "app3", "inflight.com", now); err == nil { |
| + t.Fatal("expected the injected failure") |
| + } |
| + |
| + // An UNRELATED Teardown, for a DIFFERENT app in the SAME account, must |
| + // not touch app3's in-flight claim. |
| + if err := Teardown(ctx, s, p, w, d, "aaa", "app2"); err != nil { |
| + t.Fatalf("teardown app2: %v", err) |
| + } |
| + if claim, err := getDomainClaim(ctx, s, "inflight.com"); err != nil || claim == nil { |
| + t.Fatalf("app2's teardown released app3's in-flight claim: claim=%+v err=%v", claim, err) |
| + } |
| + |
| + // app3's OWN teardown (a genuine give-up) still releases it. |
| + if err := Teardown(ctx, s, p, w, d, "aaa", "app3"); err != nil { |
| + t.Fatalf("teardown app3: %v", err) |
| + } |
| + if claim, err := getDomainClaim(ctx, s, "inflight.com"); err != nil || claim != nil { |
| + t.Fatalf("app3's own teardown did not release its orphan: %+v", claim) |
| + } |
| +} |
| + |
| +// TestTeardownOrphanSweepDestroysTheIdentityBeforeReleasing (I1 item 2, |
| +// round 2): if the crash that orphaned a claim landed AFTER EnsureIdentity |
| +// minted a real SES identity but BEFORE any record was ever written, the |
| +// sweep must destroy that identity before releasing — otherwise a stranger |
| +// who claims the domain next inherits the victim's already-minted ARN and |
| +// DKIM tokens with no DNS proof of their own, exactly what the review |
| +// demonstrated (stranger inherited ARN=…identity/cust.com, |
| +// DKIM=[tokeu1 tokeu2 tokeu3] — the victim's own tokens). |
| +func TestTeardownOrphanSweepDestroysTheIdentityBeforeReleasing(t *testing.T) { |
| + ctx := context.Background() |
| + s, w := release.NewDirStore(t.TempDir()), dnsrec.NewMemWriter() |
| + p := NewMemProvisioner("shared-aws-account") |
| + now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC) |
| + d := testDeployment() // SNSTopicARN set, so EnsureConfigSet runs right after EnsureIdentity |
| + |
| + // Crash AFTER the identity is minted but BEFORE the record is ever |
| + // written: EnsureConfigSet is the walk's very next step after |
| + // EnsureIdentity, so failing it here reproduces exactly that ordering. |
| + p.FailNext["EnsureConfigSet"] = errors.New("SES config set throttle") |
| + if _, _, err := EnsureDomain(ctx, s, p, w, d, "aaa", "victimapp", "cust.com", now); err == nil { |
| + t.Fatal("expected the injected failure") |
| + } |
| + victim, ok := p.Identities[identityKey(d.Region, "cust.com")] |
| + if !ok { |
| + t.Fatal("test setup: the identity was not actually minted before the injected failure") |
| + } |
| + victimTokens := append([]string(nil), victim.DKIMTokens...) |
| + if rec, _, err := GetDomain(ctx, s, "aaa", "cust.com"); err != nil || rec != nil { |
| + t.Fatalf("a domain record exists despite the injected failure: %+v", rec) |
| + } |
| + |
| + if err := Teardown(ctx, s, p, w, d, "aaa", "victimapp"); err != nil { |
| + t.Fatalf("teardown: %v", err) |
| + } |
| + if _, ok := p.Identities[identityKey(d.Region, "cust.com")]; ok { |
| + t.Fatal("the orphan sweep released the claim without destroying the live identity") |
| + } |
| + |
| + // A stranger claiming the released domain must get a FRESH identity — |
| + // unverified, with NEW DKIM tokens — never the victim's ARN state or |
| + // already-issued tokens. |
| + rec, _, err := EnsureDomain(ctx, s, p, w, d, "bbb", "strangerapp", "cust.com", now) |
| + if err != nil { |
| + t.Fatalf("stranger should be able to claim after the sweep: %v", err) |
| + } |
| + got := rec.Regions[d.Region] |
| + for _, tok := range got.DKIMTokens { |
| + for _, old := range victimTokens { |
| + if tok == old { |
| + t.Fatalf("stranger inherited the victim's DKIM token %q", tok) |
| + } |
| + } |
| + } |
| + if got.Status != StatusPending { |
| + t.Fatalf("stranger's identity status = %q, want pending (unverified) — a fresh identity, not the victim's verified one", got.Status) |
| + } |
| +} |
| + |
| +// ---- C1 coverage gap (item 3, round 2): pin equality, not sub/superset -- |
| + |
| +// TestPlatformNameOwnedByRequiresExactLabelEquality pins platformNameOwnedBy's |
| +// comparison as EQUALITY — neither a HasSuffix nor a HasPrefix substitution |
| +// may pass this table. Also covers the bare-apex early return with an |
| +// account name ("com") chosen specifically so the FALLBACK label-extraction |
| +// logic would otherwise (wrongly) accept it — deleting the early-return |
| +// guard is invisible against every OTHER case in this file, because the |
| +// fallback happens to compute "com" from the apex and no other test |
| +// account is named that. |
| +func TestPlatformNameOwnedByRequiresExactLabelEquality(t *testing.T) { |
| + d := testDeployment() // PlatformSuffix = "oncarlos.com" |
| + cases := []struct { |
| + name string |
| + domain string |
| + acct string |
| + want bool |
| + }{ |
| + {"exact label match", "x.aaa.oncarlos.com", "aaa", true}, |
| + {"caller is a strict prefix of the real label", "x.aaa.oncarlos.com", "aa", false}, |
| + {"caller is a strict superset of the real label", "x.aa.oncarlos.com", "aaa", false}, |
| + {"bare apex refused even for the account named after the fallback's own tail label", d.PlatformSuffix, "com", false}, |
| + } |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + if got := platformNameOwnedBy(d, c.domain, c.acct); got != c.want { |
| + t.Fatalf("platformNameOwnedBy(%q, %q) = %v, want %v", c.domain, c.acct, got, c.want) |
| + } |
| + }) |
| + } |
| +} |