Merge pull request #274 from carlosframework/fix-configset-policy
Grants the configuration set the credential sends through, closing #271. Merged with the required check unable to run (Actions billing-dead); gate reproduced locally: 32 packages, vet and 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
CI passed — run details
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) |