amadan
mail: grant the configuration set the credential sends through (#271)
SES authorises a configuration set as a resource in its own right. The delivered policy named only the identity ARN, so the instant SetIdentityConfigSet bound a set, every credential this primitive had ever minted was refused: 554 Access denied: User '.../carlos-mail-bag-seapointish+s+cli-test-...' is not authorized to perform 'ses:SendRawEmail' on resource '...:configuration-set/carlos-bag' I found this by doing it to production. Binding the set to seapointish's identity took its sending down for about four minutes; unbinding restored it, and a real delivered message confirmed the recovery. IT IS NOT A RETROFIT HAZARD. EnsureDomain binds a config set whenever the deployment has an SNS topic, and Enable then mints the user with this same policy — so on ANY deployment with event ingest configured, a freshly enabled tenant could not send its first message. Spec §9's counts, daily cap, bounce thresholds and automatic pause have therefore never been reachable: arming the events is exactly what broke the sending they govern. The feature shipped with that behind a flag nobody had set until today. SendPolicyJSON, EnsureUser and the Provisioner seam all take the config-set ARNs now; both walks gather them from the domain records. An EMPTY set stays the ordinary shape rather than an error — a deployment with no SNS topic binds nothing, and event ingest must never become a precondition for sending. The ARNs are derived from each region's own IdentityARN, so a config-set ARN cannot name a different account or region from the identity it pairs with, and they get the identity ARNs' wildcard and shape scrutiny because they reach IAM as resource patterns just the same. MUTATION TESTING FOUND THE HOLE THAT MATTERED. Four mutations failed a test as they should — dropping the config sets from Resource, deleting the ARN validation, and ignoring region Status. The fifth did not: deleting the line in Enable that GATHERS the ARNs left the whole package green while reproducing the outage exactly. The policy renderer was tested; the wiring into it was not. TestEnableGrantsTheConfigSetToTheDeliveredUser and TestCreateStandaloneGrantsTheConfigSetToo close that at the walk level, for both IAM users, asserting against the ARNs the RECORD contributes and failing loudly if it contributes none — so they cannot pass vacuously the day EnsureDomain stops binding a set. Both mutations now fail. Gate: 32 packages, vet + gofmt clean, GOOS=darwin build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12 files changed,
+294
−26
docs/superpowers/specs/2026-08-19-tenant-email-design.md+14 −0internal/console/mailcounts_test.go+1 −1internal/mail/aws.go+2 −2internal/mail/aws_test.go+1 −1internal/mail/domain.go+25 −0internal/mail/memprovisioner.go+2 −2internal/mail/memprovisioner_test.go+4 −4internal/mail/policy.go+51 −4internal/mail/policy_test.go+126 −7internal/mail/provisioner.go+1 −1internal/mail/walk.go+8 −4internal/mail/walk_enable_test.go+59 −0
diff --git a/docs/superpowers/specs/2026-08-19-tenant-email-design.md b/docs/superpowers/specs/2026-08-19-tenant-email-design.md| index ffd302525..600dac9dc 100644 |
| --- a/docs/superpowers/specs/2026-08-19-tenant-email-design.md |
| +++ b/docs/superpowers/specs/2026-08-19-tenant-email-design.md |
| @@ -152,6 +152,20 @@ rails and needs both: |
| not part of it.) |
| - `Condition: StringLike { ses:FromAddress: ["*@<domain>", …] }` over the |
| same domain set. |
| +- **`Resource:` ALSO carries the configuration-set ARNs those identities |
| + send through** — `arn:aws:ses:<region>:<account>:configuration-set/<name>`, |
| + one per verified `(domain, region)` pair that has a set bound. Corrected |
| + 2026-08-23 (issue #271) after the omission took production sending down. |
| + SES authorises a configuration set as a resource in its OWN right, so the |
| + instant §9's event destination is bound to an identity, a policy naming |
| + only the identity refuses every send with a 554. This is not a |
| + retrofit-only hazard: the binding happens inside `EnsureDomain` whenever |
| + the deployment has an SNS topic, so without this resource **any** |
| + deployment with event ingest configured mints tenants that cannot send |
| + at all — which is why §9's counts, cap and automatic pause had never been |
| + reachable. An empty set is the ordinary shape and not an error: a |
| + deployment with no SNS topic binds nothing, and event ingest must never |
| + become a precondition for sending. |
| `Resource` alone is insufficient: it authorises sending *as that identity*, |
| and a subdomain identity covers `From:` addresses the app should not be |
diff --git a/internal/console/mailcounts_test.go b/internal/console/mailcounts_test.go| index ed54dc1dc..3f8ea00de 100644 |
| --- a/internal/console/mailcounts_test.go |
| +++ b/internal/console/mailcounts_test.go |
| @@ -72,7 +72,7 @@ func mailFixture(t *testing.T) (*consoleFixture, time.Time) { |
| t.Fatal(err) |
| } |
| p := fx.srv.cfg.MailProvisioner.(*mail.MemProvisioner) |
| - p.EnsureUser(context.Background(), "u", mail.UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, []string{"x.com"}) |
| + p.EnsureUser(context.Background(), "u", mail.UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, nil, []string{"x.com"}) |
| return fx, now |
| } |
diff --git a/internal/mail/aws.go b/internal/mail/aws.go| index cf03fe7f3..3317c917c 100644 |
| --- a/internal/mail/aws.go |
| +++ b/internal/mail/aws.go |
| @@ -344,11 +344,11 @@ func (p *awsProvisioner) DeleteIdentity(ctx context.Context, region, domain stri |
| // ---- IAM -------------------------------------------------------------- |
| -func (p *awsProvisioner) EnsureUser(ctx context.Context, name, path string, identityARNs, domains []string) (string, error) { |
| +func (p *awsProvisioner) EnsureUser(ctx context.Context, name, path string, identityARNs, configSetARNs, domains []string) (string, error) { |
| // Render the policy FIRST. SendPolicyJSON refuses an empty grant, and |
| // refusing before CreateUser is what stops a principal existing with no |
| // policy at all — an unscoped sending user, briefly, is still one. |
| - doc, err := SendPolicyJSON(identityARNs, domains) |
| + doc, err := SendPolicyJSON(identityARNs, configSetARNs, domains) |
| if err != nil { |
| return "", err |
| } |
diff --git a/internal/mail/aws_test.go b/internal/mail/aws_test.go| index 2fef04dea..0f28fa06f 100644 |
| --- a/internal/mail/aws_test.go |
| +++ b/internal/mail/aws_test.go |
| @@ -155,7 +155,7 @@ func TestEnsureIdentityAlreadyExistsReadsBackTokensAndComposesTheARN(t *testing. |
| // flows straight into EnsureUser -> SendPolicyJSON in production. A |
| // composed ARN that validator rejects would make every send fail with |
| // no way to notice until Task 12. |
| - if _, err := SendPolicyJSON([]string{got.ARN}, []string{"example.com"}); err != nil { |
| + if _, err := SendPolicyJSON([]string{got.ARN}, nil, []string{"example.com"}); err != nil { |
| t.Fatalf("composed ARN %q was rejected by SendPolicyJSON: %v", got.ARN, err) |
| } |
| } |
diff --git a/internal/mail/domain.go b/internal/mail/domain.go| index f74cd3f7b..1ce9499e1 100644 |
| --- a/internal/mail/domain.go |
| +++ b/internal/mail/domain.go |
| @@ -152,6 +152,31 @@ func (r *DomainRecord) VerifiedARNs() []string { |
| return out |
| } |
| +// VerifiedConfigSetARNs is every configuration-set ARN this domain |
| +// contributes to a credential's policy (issue #271). Only a verified region |
| +// that actually HAS a set bound contributes one — a pending region's |
| +// identity is not usable yet, and a verified region with no set is the |
| +// ordinary shape on a deployment with no SNS topic. |
| +// |
| +// The region and account are taken from the region's own IdentityARN rather |
| +// than passed in, so the config-set ARN cannot name a different account or |
| +// region from the identity it pairs with. |
| +func (r *DomainRecord) VerifiedConfigSetARNs() []string { |
| + var out []string |
| + for _, st := range r.Regions { |
| + if st == nil || st.Status != StatusVerified || st.ConfigSet == "" || st.IdentityARN == "" { |
| + continue |
| + } |
| + parts := strings.Split(st.IdentityARN, ":") |
| + if len(parts) != 6 { |
| + continue |
| + } |
| + out = append(out, strings.Join(parts[:5], ":")+":configuration-set/"+st.ConfigSet) |
| + } |
| + sort.Strings(out) |
| + return out |
| +} |
| + |
| // AddApp records an app as a referrer. Reports whether it changed anything. |
| func (r *DomainRecord) AddApp(app string) bool { |
| for _, a := range r.Apps { |
diff --git a/internal/mail/memprovisioner.go b/internal/mail/memprovisioner.go| index feb900b24..8ca357f8d 100644 |
| --- a/internal/mail/memprovisioner.go |
| +++ b/internal/mail/memprovisioner.go |
| @@ -189,7 +189,7 @@ func (p *MemProvisioner) DeleteIdentity(_ context.Context, region, domain string |
| return nil |
| } |
| -func (p *MemProvisioner) EnsureUser(_ context.Context, name, path string, identityARNs, domains []string) (string, error) { |
| +func (p *MemProvisioner) EnsureUser(_ context.Context, name, path string, identityARNs, configSetARNs, domains []string) (string, error) { |
| p.mu.Lock() |
| defer p.mu.Unlock() |
| if err := p.takeFailure("EnsureUser"); err != nil { |
| @@ -197,7 +197,7 @@ func (p *MemProvisioner) EnsureUser(_ context.Context, name, path string, identi |
| } |
| // The refusal is modelled because the real backend must refuse too: a |
| // principal with no policy is an unscoped sending user. |
| - doc, err := SendPolicyJSON(identityARNs, domains) |
| + doc, err := SendPolicyJSON(identityARNs, configSetARNs, domains) |
| if err != nil { |
| return "", err |
| } |
diff --git a/internal/mail/memprovisioner_test.go b/internal/mail/memprovisioner_test.go| index c9b4a6c58..12ccdfbb9 100644 |
| --- a/internal/mail/memprovisioner_test.go |
| +++ b/internal/mail/memprovisioner_test.go |
| @@ -53,7 +53,7 @@ func TestMemDeleteIsIdempotent(t *testing.T) { |
| // assert a paused credential really is inactive. |
| func TestMemSetKeyActiveTogglesTheKey(t *testing.T) { |
| ctx, p := context.Background(), NewMemProvisioner("1") |
| - if _, err := p.EnsureUser(ctx, "u", UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, []string{"x.com"}); err != nil { |
| + if _, err := p.EnsureUser(ctx, "u", UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, nil, []string{"x.com"}); err != nil { |
| t.Fatalf("EnsureUser: %v", err) |
| } |
| id, _, err := p.MintKey(ctx, "u") |
| @@ -75,7 +75,7 @@ func TestMemSetKeyActiveTogglesTheKey(t *testing.T) { |
| // no record names it, so only list-and-delete can. |
| func TestMemReapKeysKeepsOnlyTheNamedOnes(t *testing.T) { |
| ctx, p := context.Background(), NewMemProvisioner("1") |
| - p.EnsureUser(ctx, "u", UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, []string{"x.com"}) |
| + p.EnsureUser(ctx, "u", UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, nil, []string{"x.com"}) |
| keep, _, _ := p.MintKey(ctx, "u") |
| stranded, _, _ := p.MintKey(ctx, "u") |
| if err := p.ReapKeys(ctx, "u", []string{keep}); err != nil { |
| @@ -97,7 +97,7 @@ func TestMemReapKeysKeepsOnlyTheNamedOnes(t *testing.T) { |
| // would break ordinary rotation), and only the third must fail. |
| func TestMemMintKeyEnforcesTheTwoKeyCap(t *testing.T) { |
| ctx, p := context.Background(), NewMemProvisioner("1") |
| - if _, err := p.EnsureUser(ctx, "u", UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, []string{"x.com"}); err != nil { |
| + if _, err := p.EnsureUser(ctx, "u", UserPath, []string{"arn:aws:ses:eu-west-1:1:identity/x.com"}, nil, []string{"x.com"}); err != nil { |
| t.Fatalf("EnsureUser: %v", err) |
| } |
| if _, _, err := p.MintKey(ctx, "u"); err != nil { |
| @@ -115,7 +115,7 @@ func TestMemMintKeyEnforcesTheTwoKeyCap(t *testing.T) { |
| // no policy — an unscoped sending user is the failure this package exists to |
| // prevent. |
| func TestMemEnsureUserRefusesAnEmptyGrant(t *testing.T) { |
| - if _, err := NewMemProvisioner("1").EnsureUser(context.Background(), "u", UserPath, nil, nil); err == nil { |
| + if _, err := NewMemProvisioner("1").EnsureUser(context.Background(), "u", UserPath, nil, nil, nil); err == nil { |
| t.Fatal("EnsureUser with no ARNs returned success") |
| } |
| } |
diff --git a/internal/mail/policy.go b/internal/mail/policy.go| index 020ff03c1..82df3eacc 100644 |
| --- a/internal/mail/policy.go |
| +++ b/internal/mail/policy.go |
| @@ -47,8 +47,9 @@ func StandaloneUserName(acct, app, name string) string { |
| return "carlos-mail-" + acct + "-" + app + "+s+" + name |
| } |
| -// SendPolicyJSON renders the policy for a set of verified identity ARNs and |
| -// the domains they cover. |
| +// SendPolicyJSON renders the policy for a set of verified identity ARNs, the |
| +// configuration sets those identities send through, and the domains they |
| +// cover. |
| // |
| // Both rails are mandatory and the function refuses to render without both. |
| // An empty grant returns an error rather than a permissive document: the |
| @@ -57,7 +58,25 @@ func StandaloneUserName(acct, app, name string) string { |
| // Each ARN must have the shape arn:aws:ses:<region>:<account-id>:identity/<name>, |
| // the identity name must not contain wildcards, and it must be either exactly |
| // one of the domains or a parent domain of one of them. |
| -func SendPolicyJSON(identityARNs, domains []string) (string, error) { |
| +// |
| +// configSetARNs IS THE THIRD RESOURCE AND IT IS NOT OPTIONAL WHEN ONE IS |
| +// BOUND (issue #271). SES authorises a configuration set as a SEPARATE |
| +// resource from the identity, so the moment SetIdentityConfigSet binds one, |
| +// a policy naming only the identity refuses every send: |
| +// |
| +// 554 Access denied: User '...' is not authorized to perform |
| +// 'ses:SendRawEmail' on resource '...:configuration-set/carlos-bag' |
| +// |
| +// EnsureDomain binds a config set whenever the deployment has an SNS topic, |
| +// so this is not a retrofit-only concern: without this resource, ANY |
| +// deployment with event ingest configured mints tenants that cannot send at |
| +// all. Spec §9's counts, cap and automatic pause were unreachable for |
| +// exactly this reason — arming the events broke the sending they govern. |
| +// |
| +// An EMPTY configSetARNs is the ordinary shape, not an error: a deployment |
| +// with no SNS topic binds no set, and its policy must stay what it always |
| +// was. Event ingest must never become a precondition for sending. |
| +func SendPolicyJSON(identityARNs, configSetARNs, domains []string) (string, error) { |
| if len(identityARNs) == 0 { |
| return "", fmt.Errorf("mail: refusing to render a send policy with no identity ARNs") |
| } |
| @@ -112,6 +131,34 @@ func SendPolicyJSON(identityARNs, domains []string) (string, error) { |
| return "", fmt.Errorf("mail: identity %q in ARN %q does not match any granted domain", identityName, arn) |
| } |
| } |
| + // The config-set ARNs get the identity ARNs' scrutiny, for the identity |
| + // ARNs' reason: this string reaches IAM as a resource PATTERN, so a |
| + // wildcard would widen the grant to every configuration set in the |
| + // estate, and a wrong resource type would be a grant that silently |
| + // authorises nothing while looking complete. |
| + for _, arn := range configSetARNs { |
| + if strings.Contains(arn, "*") || strings.Contains(arn, "?") { |
| + return "", fmt.Errorf("mail: invalid configuration-set ARN %q in send policy", arn) |
| + } |
| + parts := strings.Split(arn, ":") |
| + if len(parts) != 6 || parts[0] != "arn" || parts[1] != "aws" || parts[2] != "ses" { |
| + return "", fmt.Errorf("mail: invalid configuration-set ARN %q in send policy", arn) |
| + } |
| + if parts[3] == "" || parts[4] == "" { |
| + return "", fmt.Errorf("mail: invalid configuration-set ARN %q in send policy", arn) |
| + } |
| + resourceParts := strings.SplitN(parts[5], "/", 2) |
| + if len(resourceParts) != 2 || resourceParts[0] != "configuration-set" || resourceParts[1] == "" { |
| + return "", fmt.Errorf("mail: invalid configuration-set ARN %q in send policy", arn) |
| + } |
| + } |
| + // Identities first, then config sets — one flat Resource list, because |
| + // SES wants authorisation on BOTH for a single SendRawEmail through a |
| + // bound identity, and a caller reading the rendered policy should see |
| + // the pair together. |
| + resources := make([]string, 0, len(identityARNs)+len(configSetARNs)) |
| + resources = append(resources, identityARNs...) |
| + resources = append(resources, configSetARNs...) |
| doc := map[string]any{ |
| "Version": "2012-10-17", |
| "Statement": []map[string]any{{ |
| @@ -120,7 +167,7 @@ func SendPolicyJSON(identityARNs, domains []string) (string, error) { |
| // API's ses:SendEmail is deliberately NOT granted — the |
| // delivered contract is SMTP-only, and the API is not part of it. |
| "Action": []string{"ses:SendRawEmail"}, |
| - "Resource": identityARNs, |
| + "Resource": resources, |
| "Condition": map[string]any{ |
| "StringLike": map[string]any{"ses:FromAddress": froms}, |
| }, |
diff --git a/internal/mail/policy_test.go b/internal/mail/policy_test.go| index a6ed193d0..0b4b02235 100644 |
| --- a/internal/mail/policy_test.go |
| +++ b/internal/mail/policy_test.go |
| @@ -31,6 +31,7 @@ func parsePolicy(t *testing.T, s string) policyDoc { |
| func TestPolicyCarriesBothIsolationRails(t *testing.T) { |
| got, err := SendPolicyJSON( |
| []string{"arn:aws:ses:eu-west-1:1:identity/seapointish.bag.oncarlos.com"}, |
| + nil, |
| []string{"seapointish.bag.oncarlos.com"}) |
| if err != nil { |
| t.Fatalf("SendPolicyJSON: %v", err) |
| @@ -60,7 +61,7 @@ func TestPolicyListsEveryVerifiedARN(t *testing.T) { |
| got, err := SendPolicyJSON([]string{ |
| "arn:aws:ses:eu-west-1:1:identity/x.com", |
| "arn:aws:ses:us-east-1:1:identity/x.com", |
| - }, []string{"x.com"}) |
| + }, nil, []string{"x.com"}) |
| if err != nil { |
| t.Fatalf("SendPolicyJSON: %v", err) |
| } |
| @@ -75,7 +76,7 @@ func TestPolicyWithMultipleDomainsAndARNs(t *testing.T) { |
| got, err := SendPolicyJSON([]string{ |
| "arn:aws:ses:eu-west-1:1:identity/platform.com", |
| "arn:aws:ses:us-east-1:1:identity/custom.app", |
| - }, []string{"platform.com", "custom.app"}) |
| + }, nil, []string{"platform.com", "custom.app"}) |
| if err != nil { |
| t.Fatalf("SendPolicyJSON: %v", err) |
| } |
| @@ -117,14 +118,131 @@ func TestPolicyWithMultipleDomainsAndARNs(t *testing.T) { |
| // An empty grant must not render a policy that permits everything. Refusing |
| // is the only safe direction: a wildcard here would let one app send as any |
| // identity in the estate. |
| +// TestPolicyGrantsTheConfigSetItSendsThrough is issue #271, and it is the |
| +// difference between a primitive that can emit events and one that can send |
| +// mail — before this, it could not do both. |
| +// |
| +// SES authorises a configuration set as a SEPARATE resource. The moment |
| +// SetIdentityConfigSet binds one to an identity, every send through that |
| +// identity needs ses:SendRawEmail on the config-set ARN too, and a policy |
| +// naming only the identity is refused with a 554: |
| +// |
| +// Access denied: User '...' is not authorized to perform 'ses:SendRawEmail' |
| +// on resource 'arn:aws:ses:eu-west-1:...:configuration-set/carlos-bag' |
| +// |
| +// That is not hypothetical and it is not retrofit-specific: EnsureDomain |
| +// binds a config set whenever SNSTopicARN is set, so on any deployment with |
| +// event ingest configured a freshly enabled tenant could not send at all. |
| +// Proven in production on 2026-08-23, at the cost of a short outage. |
| +func TestPolicyGrantsTheConfigSetItSendsThrough(t *testing.T) { |
| + doc, err := SendPolicyJSON( |
| + []string{"arn:aws:ses:eu-west-1:1234:identity/a.example.com"}, |
| + []string{"arn:aws:ses:eu-west-1:1234:configuration-set/carlos-bag"}, |
| + []string{"a.example.com"}, |
| + ) |
| + if err != nil { |
| + t.Fatalf("SendPolicyJSON: %v", err) |
| + } |
| + res := policyResources(t, doc) |
| + if !res["arn:aws:ses:eu-west-1:1234:identity/a.example.com"] { |
| + t.Error("the identity ARN is missing from Resource") |
| + } |
| + if !res["arn:aws:ses:eu-west-1:1234:configuration-set/carlos-bag"] { |
| + t.Error("the configuration-set ARN is missing from Resource — every send through a bound identity 554s without it") |
| + } |
| +} |
| + |
| +// TestPolicyWithNoConfigSetIsUnchanged: a deployment with no SNS topic binds |
| +// no configuration set, and its policy must stay exactly what it was. The |
| +// fix must not make event ingest a precondition for sending. |
| +func TestPolicyWithNoConfigSetIsUnchanged(t *testing.T) { |
| + doc, err := SendPolicyJSON( |
| + []string{"arn:aws:ses:eu-west-1:1234:identity/a.example.com"}, |
| + nil, |
| + []string{"a.example.com"}, |
| + ) |
| + if err != nil { |
| + t.Fatalf("SendPolicyJSON: %v", err) |
| + } |
| + res := policyResources(t, doc) |
| + if len(res) != 1 || !res["arn:aws:ses:eu-west-1:1234:identity/a.example.com"] { |
| + t.Fatalf("Resource = %v, want the identity ARN alone", res) |
| + } |
| +} |
| + |
| +// TestPolicyRefusesABadConfigSetARN: the config-set ARN reaches IAM as a |
| +// resource pattern exactly as the identity ARN does, so it gets the same |
| +// scrutiny. A wildcard would widen the grant to every configuration set in |
| +// the estate; a wrong resource type would be a silently ineffective grant. |
| +func TestPolicyRefusesABadConfigSetARN(t *testing.T) { |
| + for _, bad := range []string{ |
| + "arn:aws:ses:eu-west-1:1234:configuration-set/*", |
| + "arn:aws:ses:eu-west-1:1234:configuration-set/carlos-?ag", |
| + "arn:aws:ses:eu-west-1:1234:identity/a.example.com", |
| + "arn:aws:sns:eu-west-1:1234:configuration-set/carlos-bag", |
| + "arn:aws:ses::1234:configuration-set/carlos-bag", |
| + "arn:aws:ses:eu-west-1::configuration-set/carlos-bag", |
| + "not-an-arn", |
| + } { |
| + if _, err := SendPolicyJSON( |
| + []string{"arn:aws:ses:eu-west-1:1234:identity/a.example.com"}, |
| + []string{bad}, |
| + []string{"a.example.com"}, |
| + ); err == nil { |
| + t.Errorf("config-set ARN %q was accepted", bad) |
| + } |
| + } |
| +} |
| + |
| +// TestVerifiedConfigSetARNsSkipsWhatItShould: only a VERIFIED region with a |
| +// config set contributes one. A pending region, or a verified region that |
| +// has no set bound, must contribute nothing — the first because its identity |
| +// is not usable yet, the second because that is the ordinary no-SNS shape. |
| +func TestVerifiedConfigSetARNsSkipsWhatItShould(t *testing.T) { |
| + rec := &DomainRecord{Domain: "a.example.com", Regions: map[string]*RegionState{ |
| + "eu-west-1": {IdentityARN: "arn:aws:ses:eu-west-1:1234:identity/a.example.com", |
| + ConfigSet: "carlos-bag", Status: StatusVerified}, |
| + "us-east-1": {IdentityARN: "arn:aws:ses:us-east-1:1234:identity/a.example.com", |
| + ConfigSet: "carlos-bag", Status: StatusPending}, |
| + "eu-west-2": {IdentityARN: "arn:aws:ses:eu-west-2:1234:identity/a.example.com", |
| + ConfigSet: "", Status: StatusVerified}, |
| + }} |
| + got := rec.VerifiedConfigSetARNs() |
| + want := []string{"arn:aws:ses:eu-west-1:1234:configuration-set/carlos-bag"} |
| + if len(got) != len(want) || (len(got) == 1 && got[0] != want[0]) { |
| + t.Fatalf("VerifiedConfigSetARNs = %v, want %v", got, want) |
| + } |
| +} |
| + |
| +// policyResources pulls the Resource list out of a rendered policy. |
| +func policyResources(t *testing.T, doc string) map[string]bool { |
| + t.Helper() |
| + var parsed struct { |
| + Statement []struct { |
| + Resource []string `json:"Resource"` |
| + } `json:"Statement"` |
| + } |
| + if err := json.Unmarshal([]byte(doc), &parsed); err != nil { |
| + t.Fatalf("policy is not JSON: %v", err) |
| + } |
| + if len(parsed.Statement) != 1 { |
| + t.Fatalf("statements = %d, want 1", len(parsed.Statement)) |
| + } |
| + out := map[string]bool{} |
| + for _, r := range parsed.Statement[0].Resource { |
| + out[r] = true |
| + } |
| + return out |
| +} |
| + |
| func TestPolicyRefusesAnEmptyGrant(t *testing.T) { |
| - if _, err := SendPolicyJSON(nil, nil); err == nil { |
| + if _, err := SendPolicyJSON(nil, nil, nil); err == nil { |
| t.Fatal("SendPolicyJSON with no ARNs and no domains returned a policy, want an error") |
| } |
| - if _, err := SendPolicyJSON([]string{"arn:x"}, nil); err == nil { |
| + if _, err := SendPolicyJSON([]string{"arn:x"}, nil, nil); err == nil { |
| t.Fatal("SendPolicyJSON with no domains returned a policy, want an error") |
| } |
| - if _, err := SendPolicyJSON(nil, []string{"x.com"}); err == nil { |
| + if _, err := SendPolicyJSON(nil, nil, []string{"x.com"}); err == nil { |
| t.Fatal("SendPolicyJSON with no ARNs returned a policy, want an error") |
| } |
| } |
| @@ -132,7 +250,7 @@ func TestPolicyRefusesAnEmptyGrant(t *testing.T) { |
| // A domain that reached this far unvalidated could inject a wildcard into |
| // the condition ("*" would match every address). Refuse rather than render. |
| func TestPolicyRefusesAnInvalidDomain(t *testing.T) { |
| - if _, err := SendPolicyJSON([]string{"arn:x"}, []string{"*"}); err == nil { |
| + if _, err := SendPolicyJSON([]string{"arn:x"}, nil, []string{"*"}); err == nil { |
| t.Fatal("SendPolicyJSON accepted a wildcard domain") |
| } |
| } |
| @@ -160,7 +278,7 @@ func TestPolicyRefusesARNWildcards(t *testing.T) { |
| for _, tt := range tests { |
| t.Run(tt.name, func(t *testing.T) { |
| - _, err := SendPolicyJSON([]string{tt.arn}, []string{"example.com"}) |
| + _, err := SendPolicyJSON([]string{tt.arn}, nil, []string{"example.com"}) |
| if err == nil { |
| t.Fatalf("SendPolicyJSON accepted invalid ARN: %s", tt.arn) |
| } |
| @@ -172,6 +290,7 @@ func TestPolicyRefusesARNWildcards(t *testing.T) { |
| func TestPolicyAcceptsLegitimateARN(t *testing.T) { |
| _, err := SendPolicyJSON( |
| []string{"arn:aws:ses:eu-west-1:123456789012:identity/seapointish.bag.oncarlos.com"}, |
| + nil, |
| []string{"seapointish.bag.oncarlos.com"}) |
| if err != nil { |
| t.Fatalf("SendPolicyJSON rejected legitimate ARN: %v", err) |
diff --git a/internal/mail/provisioner.go b/internal/mail/provisioner.go| index 9ab392ed4..84e8ebce6 100644 |
| --- a/internal/mail/provisioner.go |
| +++ b/internal/mail/provisioner.go |
| @@ -63,7 +63,7 @@ type Provisioner interface { |
| // EnsureUser creates the principal if absent and writes its inline |
| // policy wholesale. It MUST refuse an empty grant rather than create a |
| // principal with no policy. |
| - EnsureUser(ctx context.Context, name, path string, identityARNs, domains []string) (arn string, err error) |
| + EnsureUser(ctx context.Context, name, path string, identityARNs, configSetARNs, domains []string) (arn string, err error) |
| MintKey(ctx context.Context, user string) (id, secret string, err error) |
| // SetKeyActive is the pause (spec §9): instant, reversible, and it |
| // needs no policy rewrite. |
diff --git a/internal/mail/walk.go b/internal/mail/walk.go| index 5df300775..b2b44723d 100644 |
| --- a/internal/mail/walk.go |
| +++ b/internal/mail/walk.go |
| @@ -498,12 +498,14 @@ func Enable(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.P |
| if len(domains) == 0 { |
| return nil, ErrNoDomains |
| } |
| - var arns, names []string |
| + var arns, configSets, names []string |
| for _, dom := range domains { |
| arns = append(arns, dom.VerifiedARNs()...) |
| + configSets = append(configSets, dom.VerifiedConfigSetARNs()...) |
| names = append(names, dom.Domain) |
| } |
| sort.Strings(arns) |
| + sort.Strings(configSets) |
| sort.Strings(names) |
| // The From address must be at a granted domain, or the policy's |
| @@ -532,7 +534,7 @@ func Enable(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.P |
| // and mint below that spend IAM's two-key ceiling, and THOSE are what |
| // the short-circuit still has to gate. |
| user := UserName(acct, app) |
| - arn, err := p.EnsureUser(ctx, user, UserPath, arns, names) |
| + arn, err := p.EnsureUser(ctx, user, UserPath, arns, configSets, names) |
| if err != nil { |
| return nil, fmt.Errorf("mail: ensure user %s: %w", user, err) |
| } |
| @@ -884,16 +886,18 @@ func CreateStandalone(ctx context.Context, s release.Store, p Provisioner, d Dep |
| if len(domains) == 0 { |
| return StandaloneCreds{}, ErrNoDomains |
| } |
| - var arns, names []string |
| + var arns, configSets, names []string |
| for _, dom := range domains { |
| arns = append(arns, dom.VerifiedARNs()...) |
| + configSets = append(configSets, dom.VerifiedConfigSetARNs()...) |
| names = append(names, dom.Domain) |
| } |
| sort.Strings(arns) |
| + sort.Strings(configSets) |
| sort.Strings(names) |
| user := StandaloneUserName(acct, app, name) |
| - if _, err := p.EnsureUser(ctx, user, UserPath, arns, names); err != nil { |
| + if _, err := p.EnsureUser(ctx, user, UserPath, arns, configSets, names); err != nil { |
| return StandaloneCreds{}, err |
| } |
| // A re-mint for the same name replaces the credential rather than |
diff --git a/internal/mail/walk_enable_test.go b/internal/mail/walk_enable_test.go| index 488d6083b..b22b2b68e 100644 |
| --- a/internal/mail/walk_enable_test.go |
| +++ b/internal/mail/walk_enable_test.go |
| @@ -142,6 +142,65 @@ func TestEnableDeliversTheDerivedPasswordNotTheSecret(t *testing.T) { |
| // A healthy provisioned app makes a re-run a no-op: no fresh key, no |
| // rewrite. Minting on every pass would burn IAM's two-key ceiling and |
| // churn every instance's config. |
| +// TestEnableGrantsTheConfigSetToTheDeliveredUser is the WALK half of issue |
| +// #271, and it exists because mutation testing showed the policy-level tests |
| +// could not see this: deleting the line in Enable that gathers config-set |
| +// ARNs left every test in this package green while reproducing the exact |
| +// outage — an identity bound to a configuration set, and a delivered |
| +// credential whose policy does not mention it, which SES refuses with a 554. |
| +// |
| +// The assertion is deliberately taken from the RECORD rather than from a |
| +// hardcoded name, and it fails when the record contributes none, so it |
| +// cannot pass vacuously the day EnsureDomain stops binding a set. |
| +func TestEnableGrantsTheConfigSetToTheDeliveredUser(t *testing.T) { |
| + ctx, s, p, pub, now := enableFixture(t) |
| + if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now); err != nil { |
| + t.Fatalf("Enable: %v", err) |
| + } |
| + dom, _, err := GetDomain(ctx, s, "bag", "seapointish.bag.oncarlos.com") |
| + if err != nil { |
| + t.Fatalf("GetDomain: %v", err) |
| + } |
| + want := dom.VerifiedConfigSetARNs() |
| + if len(want) == 0 { |
| + t.Fatal("the fixture bound no configuration set, so this test proves nothing — testDeployment() must carry an SNS topic") |
| + } |
| + policy := p.Users[UserName("bag", "seapointish")].Policy |
| + for _, arn := range want { |
| + if !strings.Contains(policy, arn) { |
| + t.Errorf("delivered user's policy does not grant %s\npolicy = %s\n"+ |
| + "SES authorises a configuration set as its own resource: without this every send is a 554", arn, policy) |
| + } |
| + } |
| +} |
| + |
| +// TestCreateStandaloneGrantsTheConfigSetToo: the standalone credential is a |
| +// SEPARATE IAM user with its own policy, minted by its own walk, so it can |
| +// regress independently of the delivered one. |
| +func TestCreateStandaloneGrantsTheConfigSetToo(t *testing.T) { |
| + ctx, s, p, pub, now := enableFixture(t) |
| + if _, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now); err != nil { |
| + t.Fatalf("Enable: %v", err) |
| + } |
| + if _, err := CreateStandalone(ctx, s, p, testDeployment(), "bag", "seapointish", "smoke", "paul@rslw.com", now); err != nil { |
| + t.Fatalf("CreateStandalone: %v", err) |
| + } |
| + dom, _, err := GetDomain(ctx, s, "bag", "seapointish.bag.oncarlos.com") |
| + if err != nil { |
| + t.Fatalf("GetDomain: %v", err) |
| + } |
| + want := dom.VerifiedConfigSetARNs() |
| + if len(want) == 0 { |
| + t.Fatal("the fixture bound no configuration set, so this test proves nothing") |
| + } |
| + policy := p.Users[StandaloneUserName("bag", "seapointish", "smoke")].Policy |
| + for _, arn := range want { |
| + if !strings.Contains(policy, arn) { |
| + t.Errorf("standalone user's policy does not grant %s\npolicy = %s", arn, policy) |
| + } |
| + } |
| +} |
| + |
| func TestEnableIsIdempotent(t *testing.T) { |
| ctx, s, p, pub, now := enableFixture(t) |
| first, err := Enable(ctx, s, p, pub, testDeployment(), "bag", "seapointish", now) |