From da4a1858658f5fc00696ed21ec83f4b62bc19138 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 23:44:14 +0100 Subject: [PATCH 1/7] test(iam): add a grantNeed helper to check deploy grants against real ARNs assertGranted checks an action and a sample real ARN against the deploy-role Allow statements only, never the workload boundary. Its unit test includes a dead ARN scope that must be reported ungranted. Test-only; no policy change (compute stays 5933 of 6144). Refs #157 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/grant_needs_test.go | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 terraform/environments/aws/ci-cd-permissions/grant_needs_test.go diff --git a/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go b/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go new file mode 100644 index 000000000..c430418a3 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go @@ -0,0 +1,88 @@ +package cicdpermissions_test + +import "testing" + +// grantNeed is one action a Terraform apply performs, with a sample real ARN +// of the resource it runs against. An empty arn means the action has no +// resource type (action-only). +type grantNeed struct { + action string + arn string +} + +// grantedBy reports whether any statement in allow authorizes need. +func grantedBy(allow []string, need grantNeed) bool { + for _, stmt := range allow { + if !actionPermitted(statementActions(stmt), need.action) { + continue + } + if need.arn == "" { + return true + } + for _, r := range statementResources(stmt) { + if iamARNMatches(r, need.arn) { + return true + } + } + } + return false +} + +// deployAllowStatements returns every Allow statement of the deploy-role +// documents. The boundary caps the workload roles and its broad grants +// (logs:*, rds:*) would mask a missing deploy grant, so it is excluded. +func deployAllowStatements(t *testing.T) []string { + t.Helper() + var allow []string + for _, f := range deployPolicyFiles(t) { + for _, stmt := range extractStatementBlocks(readPolicySource(t, f)) { + if statementEffect(stmt) == "Allow" { + allow = append(allow, stmt) + } + } + } + if len(allow) == 0 { + t.Fatal("found no Allow statements; the statement splitter is broken and this test would pass vacuously") + } + return allow +} + +// assertGranted fails for every need that no deploy Allow statement authorizes. +// Deny statements are ignored: none touches rds, route53, acm, logs or sns +// today, so a future Deny on those services must be a conscious change here. +func assertGranted(t *testing.T, label string, needs []grantNeed) { + t.Helper() + allow := deployAllowStatements(t) + for _, n := range needs { + if !grantedBy(allow, n) { + t.Errorf("%s: no deploy policy statement grants %s on %q", label, n.action, n.arn) + } + } +} + +func TestGrantedBy(t *testing.T) { + stmt := func(resource string) string { + return `{ + Effect = "Allow" + Action = ["rds:ModifyDBProxy"] + Resource = ` + resource + ` + }` + } + const proxy = "arn:aws:rds:us-east-1:123456789012:db-proxy:prx-0123456789abcdef" + cases := []struct { + name string + stmt string + need grantNeed + want bool + }{ + {"action only", stmt(`"*"`), grantNeed{"rds:ModifyDBProxy", ""}, true}, + {"wrong action", stmt(`"*"`), grantNeed{"rds:DeleteDBProxy", ""}, false}, + {"real arn", stmt(`["arn:aws:rds:*:*:db-proxy:*"]`), grantNeed{"rds:ModifyDBProxy", proxy}, true}, + {"dead arn scope", stmt(`["arn:aws:rds:*:*:proxy:cudly-*"]`), grantNeed{"rds:ModifyDBProxy", proxy}, false}, + } + for _, c := range cases { + if got := grantedBy([]string{c.stmt}, c.need); got != c.want { + t.Errorf("%s: grantedBy = %v, want %v", c.name, got, c.want) + } + } +} From cef804bf6bd9e8ebe727247f6943537a93c6bfbe Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:00:16 +0100 Subject: [PATCH 2/7] fix(iam): drop unmatchable RDS proxy resource scopes RDSResourceScoped named proxy:cudly-* and target-group:cudly-*, which match no real ARN (the type segment is db-proxy and ids are opaque prx-...), so DeleteDBProxy, ModifyDBProxy and Register/Deregister DBProxyTargets only looked granted. Remove the four actions and the two dead scopes; behavior is unchanged because they authorized nothing. Tests: every rds: Resource must match a real ARN; each kept action must have its required resource types; the four proxy actions need db-proxy and target-group entries. A tripwire pins enable_rds_proxy to one literal false (the module default is true) and lists the grants to add when it is enabled. Also fixes the stale database.tf line reference. Policy size (compact JSON): data 4528 -> 4357 of 6144. Refs #157 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/policy_data.tf | 29 +--- .../rds_proxy_tripwire_test.go | 122 ++++++++++++++++ .../aws/ci-cd-permissions/rds_scopes_test.go | 136 ++++++++++++++++++ 3 files changed, 265 insertions(+), 22 deletions(-) create mode 100644 terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go create mode 100644 terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go diff --git a/terraform/environments/aws/ci-cd-permissions/policy_data.tf b/terraform/environments/aws/ci-cd-permissions/policy_data.tf index cc5ff66e2..7a4406b5f 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_data.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_data.tf @@ -214,21 +214,12 @@ resource "aws_iam_policy" "data" { # NO rds:Describe* action lives here; they are all in # RDSDescribeAccountWide below. See that statement for why. # - # KNOWN DEAD SCOPES: the proxy actions below (DeleteDBProxy, - # ModifyDBProxy, RegisterDBProxyTargets, DeregisterDBProxyTargets) - # can never be authorized, because their only resource types are - # `proxy` and `target-group`, whose real ARNs are - # arn:aws:rds:::db-proxy:prx- and - # arn:aws:rds:::target-group:prx-tg-: a different ARN - # segment (`db-proxy`, not `proxy`) and a server-assigned opaque id - # that never contains the resource name. `proxy:cudly-*` and - # `target-group:cudly-*` therefore match nothing. Nothing exercises - # this today (modules/database/aws is instantiated with - # enable_rds_proxy = false, terraform/environments/aws/database.tf), - # so it is left as-is rather than widened to db-proxy:* here; fixing - # it needs a scoping mechanism that actually constrains an opaque id - # (tag condition), tracked separately. Do NOT add new proxy actions - # to this statement; they would be dead on arrival. + # RDS DB proxy mutations (DeleteDBProxy, ModifyDBProxy, + # Register/DeregisterDBProxyTargets) are deliberately absent: their + # resource types are db-proxy and target-group with server-assigned + # opaque ids, which no cudly-* name scope can match. enable_rds_proxy + # is a literal false, pinned by rds_proxy_tripwire_test.go, which lists + # the grants to add when it is turned on (#157). Sid = "RDSResourceScoped" Effect = "Allow" Action = [ @@ -239,19 +230,13 @@ resource "aws_iam_policy" "data" { "rds:DeleteDBInstance", "rds:DeleteDBSubnetGroup", "rds:ListTagsForResource", - "rds:DeleteDBProxy", - "rds:DeregisterDBProxyTargets", "rds:ModifyDBInstance", - "rds:ModifyDBProxy", "rds:ModifyDBSubnetGroup", - "rds:RegisterDBProxyTargets", "rds:RemoveTagsFromResource", ] Resource = [ "arn:aws:rds:*:*:db:cudly-*", "arn:aws:rds:*:*:subgrp:cudly-*", - "arn:aws:rds:*:*:proxy:cudly-*", - "arn:aws:rds:*:*:target-group:cudly-*", "arn:aws:rds:*:*:snapshot:cudly-*", ] }, @@ -301,7 +286,7 @@ resource "aws_iam_policy" "data" { # opaque ids (see the RDSResourceScoped note above), so no name-based # ARN scope can ever match them. Those are belt-and-braces rather than # load-bearing: enable_rds_proxy is a literal `false` at - # terraform/environments/aws/database.tf:28, not a variable, so no + # terraform/environments/aws/database.tf:41, not a variable, so no # tfvars can turn the proxy resources on. # # All six are read-only and expose only resource metadata; every diff --git a/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go b/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go new file mode 100644 index 000000000..cc8f77b21 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go @@ -0,0 +1,122 @@ +package cicdpermissions_test + +import ( + "io/fs" + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +const rdsProxyGrantsNeeded = `enable_rds_proxy is no longer a literal false, so the RDS proxy can be created, and the deploy role cannot manage it (#157). +Before enabling it, add to the deploy policy (policy_data.tf): + - db-proxy:* with StringEqualsIgnoreCase aws:ResourceTag/Project = CUDly: rds:ModifyDBProxy, rds:DeleteDBProxy, + rds:DeregisterDBProxyTargets, rds:AddTagsToResource, rds:RemoveTagsFromResource, and rds:ListTagsForResource + (the provider reads tags on every refresh, proxy.go @Tags(identifierAttribute="arn")) + - target-group:* (no tag condition; the default target group has no tags argument): rds:ModifyDBProxyTargetGroup, + rds:RegisterDBProxyTargets (also needs db:cudly-*), rds:DeregisterDBProxyTargets + - rds:CreateDBProxy stays on "*" (tags travel inside the create call) +Then exercise it once in staging (create, no-op re-plan, destroy) and update this tripwire.` + +// envTerraformFiles returns the comment-stripped text of every *.tf directly in +// terraform/environments/aws. +func envTerraformFiles(t *testing.T, envDir string) map[string]string { + t.Helper() + files, err := filepath.Glob(filepath.Join(envDir, "*.tf")) + if err != nil || len(files) == 0 { + t.Fatalf("globbing %s/*.tf: %v (%d files); the tripwire would pass vacuously", envDir, err, len(files)) + } + out := map[string]string{} + for _, f := range files { + data, err := os.ReadFile(f) + if err != nil { + t.Fatalf("reading %s: %v", f, err) + } + var kept []string + for _, line := range strings.Split(string(data), "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "//") { + continue + } + kept = append(kept, line) + } + out[f] = strings.Join(kept, "\n") + } + return out +} + +// trimTrailingComment drops a trailing # or // comment and surrounding space. +func trimTrailingComment(v string) string { + if i := strings.IndexAny(v, "#"); i >= 0 { + v = v[:i] + } + if i := strings.Index(v, "//"); i >= 0 { + v = v[:i] + } + return strings.TrimSpace(v) +} + +func TestTrimTrailingComment(t *testing.T) { + for in, want := range map[string]string{ + " false": "false", + " false # off": "false", + " false // off": "false", + " true # false": "true", + } { + if got := trimTrailingComment(in); got != want { + t.Errorf("trimTrailingComment(%q) = %q, want %q", in, got, want) + } + } +} + +// TestRDSProxyStaysDisabledUntilTheDeployRoleCanManageIt fails when anything +// but one explicit literal false reaches modules/database/aws's +// enable_rds_proxy. The module default is true, so deleting the environment's +// line, or adding a second caller, would silently enable the proxy. +func TestRDSProxyStaysDisabledUntilTheDeployRoleCanManageIt(t *testing.T) { + const envDir = "../" + const moduleDir = "../../../modules/database/aws" + + assignment := regexp.MustCompile(`(?m)^\s*enable_rds_proxy\s*=(.*)$`) + var values []string + for _, text := range envTerraformFiles(t, envDir) { + for _, m := range assignment.FindAllStringSubmatch(text, -1) { + values = append(values, trimTrailingComment(m[1])) + } + } + if len(values) != 1 || values[0] != "false" { + t.Fatalf("want exactly one uncommented `enable_rds_proxy = false` in %s*.tf, got %q.\n%s", envDir, values, rdsProxyGrantsNeeded) + } + + // Single caller of the module: the env's explicit false is the only thing + // overriding the module default of true. + source := regexp.MustCompile(`(?m)^\s*source\s*=\s*"[^"]*modules/database/aws"`) + moduleAbs, _ := filepath.Abs(moduleDir) + callers := 0 + root := "../../../" + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + abs, _ := filepath.Abs(p) + if d.IsDir() && (abs == moduleAbs || d.Name() == ".terraform") { + return filepath.SkipDir + } + if d.IsDir() || !strings.HasSuffix(p, ".tf") { + return nil + } + data, err := os.ReadFile(p) + if err != nil { + return err + } + callers += len(source.FindAllString(stripCommentLines(string(data)), -1)) + return nil + }) + if err != nil { + t.Fatalf("walking %s: %v", root, err) + } + if callers != 1 { + t.Fatalf("modules/database/aws has %d callers, want exactly 1 (the env passing enable_rds_proxy = false; the module default is true).\n%s", callers, rdsProxyGrantsNeeded) + } +} diff --git a/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go b/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go new file mode 100644 index 000000000..171526135 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go @@ -0,0 +1,136 @@ +package cicdpermissions_test + +import ( + "strings" + "testing" +) + +// realRDSARNs are real ARN shapes per RDS resource type, as the Service +// Authorization Reference defines them (Amazon RDS, resource types table, +// https://docs.aws.amazon.com/service-authorization/latest/reference/list_amazonrds.html, +// machine-readable form https://servicereference.us-east-1.amazonaws.com/v1/rds/rds.json, +// rds.json read 2026-11-04). Proxy and target-group ids are server-assigned and +// opaque (prx-..., prx-tg-...), never the resource name, and the proxy type's +// ARN segment is db-proxy, not proxy (#157). +var realRDSARNs = map[string]string{ + "db": "arn:aws:rds:us-east-1:123456789012:db:cudly-prod-abc-postgres", + "subgrp": "arn:aws:rds:us-east-1:123456789012:subgrp:cudly-prod-abc-db-subnet", + "snapshot": "arn:aws:rds:us-east-1:123456789012:snapshot:cudly-prod-abc-snap", + "db-proxy": "arn:aws:rds:us-east-1:123456789012:db-proxy:prx-0123456789abcdef", + "target-group": "arn:aws:rds:us-east-1:123456789012:target-group:prx-tg-0123456789abcdef", +} + +// requiredRDSTypes lists, for each RDS action the deploy policy keeps, the +// resource types the SAR marks required (*) for it: IAM authorizes the call +// against each of them, so a grant that leaves one unmatched is dead. Hand +// copied from the SAR RDS actions table (URL above) because rds.json does not +// mark required types and optional types (pg, og, secgrp on CreateDBInstance) +// work without a grant. The required markers come from rds.json plus knowledge +// of the HTML page, which could not be rendered to verify them; recheck against +// the page when adding an action. The tagging actions list many optional types and mark +// none required, so they only need some matching entry. +var requiredRDSTypes = map[string][]string{ + "rds:CreateDBInstance": {"db"}, + "rds:CreateDBSnapshot": {"db", "snapshot"}, + "rds:CreateDBSubnetGroup": {"subgrp"}, + "rds:DeleteDBInstance": {"db"}, + "rds:DeleteDBSubnetGroup": {"subgrp"}, + "rds:ModifyDBInstance": {"db"}, + "rds:ModifyDBSubnetGroup": {"subgrp"}, + "rds:AddTagsToResource": nil, + "rds:ListTagsForResource": nil, + "rds:RemoveTagsFromResource": nil, +} + +func matchesAnyRealRDSARN(pattern string) bool { + for _, arn := range realRDSARNs { + if iamARNMatches(pattern, arn) { + return true + } + } + return false +} + +// rdsStatements returns the Allow statements of policy_data.tf that grant any +// rds: action. +func rdsStatements(t *testing.T) []string { + t.Helper() + return findStatements(t, "policy_data.tf", func(stmt string) bool { + if statementEffect(stmt) != "Allow" { + return false + } + for _, a := range statementActions(stmt) { + if strings.HasPrefix(a, "rds:") { + return true + } + } + return false + }) +} + +func TestRDSResourceScopedPatternsMatchRealARNs(t *testing.T) { + stmts := rdsStatements(t) + if len(stmts) == 0 { + t.Fatal("found no rds: Allow statements in policy_data.tf; the test would pass vacuously") + } + for _, stmt := range stmts { + for _, r := range statementResources(stmt) { + if r != "*" && !matchesAnyRealRDSARN(r) { + t.Errorf("statement %q: Resource %q matches no real RDS ARN, so every action scoped only by it is a dead grant (#157)", statementSid(stmt), r) + } + } + } +} + +func TestRDSKeptActionsHaveEveryRequiredResourceType(t *testing.T) { + allow := rdsStatements(t) + // A newly granted action must get a table entry, or it goes unchecked. + for _, stmt := range allow { + for _, a := range statementActions(stmt) { + if _, ok := requiredRDSTypes[a]; !ok && !strings.HasPrefix(a, "rds:Describe") && a != "rds:CreateDBProxy" { + t.Errorf("statement %q grants %s, which has no entry in requiredRDSTypes; add its required resource types", statementSid(stmt), a) + } + } + } + for action, types := range requiredRDSTypes { + if len(types) == 0 { + if !grantedBy(allow, grantNeed{action: action, arn: realRDSARNs["db"]}) { + t.Errorf("%s: no statement grants it on a real db ARN", action) + } + continue + } + for _, typ := range types { + if !grantedBy(allow, grantNeed{action: action, arn: realRDSARNs[typ]}) { + t.Errorf("%s: required resource type %s (%s) is not granted", action, typ, realRDSARNs[typ]) + } + } + } +} + +// TestRDSProxyMutatingActionsNeedProxyARNs: these four need db-proxy and +// target-group ARNs (opaque ids), which a cudly-* name scope cannot express. +// They stay out of the deploy policy until enable_rds_proxy is turned on and +// the working grants are added (see rds_proxy_tripwire_test.go). +func TestRDSProxyMutatingActionsNeedProxyARNs(t *testing.T) { + allow := deployAllowStatements(t) + for _, action := range []string{ + "rds:DeleteDBProxy", + "rds:ModifyDBProxy", + "rds:RegisterDBProxyTargets", + "rds:DeregisterDBProxyTargets", + } { + for _, stmt := range allow { + if !actionPermitted(statementActions(stmt), action) { + continue + } + var proxy, targetGroup bool + for _, r := range statementResources(stmt) { + proxy = proxy || iamARNMatches(r, realRDSARNs["db-proxy"]) + targetGroup = targetGroup || iamARNMatches(r, realRDSARNs["target-group"]) + } + if !proxy || !targetGroup { + t.Errorf("statement %q grants %s without both db-proxy and target-group Resource entries that match real ARNs; it is a dead grant (#157)", statementSid(stmt), action) + } + } + } +} From 04e2ffab138f7e4607136f526e1b6ec9985c743d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:01:53 +0100 Subject: [PATCH 3/7] test(iam): pin monitoring and migration-alarm deploy grants The grants for enable_monitoring and enable_migration_alarm already exist (policy_monitoring.tf, policy_compute.tf) but nothing pinned them, and both flags default to false so no deploy exercises them. Pin each action against the real ARN shapes from the Service Authorization Reference (SNS topic ARN has no type segment, dashboard ARN has an empty region and a slash, alarm uses alarm:) and fail on a topic/ segment. Pin test over existing grants, so there is no fails-before step; proven by mutation probes instead. No policy change: monitoring stays 983 of 6144. Refs #157 Co-Authored-By: Claude Opus 5.5 --- .../monitoring_apply_test.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go diff --git a/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go b/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go new file mode 100644 index 000000000..277cfe942 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go @@ -0,0 +1,49 @@ +package cicdpermissions_test + +import ( + "strings" + "testing" +) + +// TestDeployPolicyGrantsEveryActionTheMonitoringModuleNeeds pins grants that +// policy_monitoring.tf and policy_compute.tf already hold for +// enable_monitoring and enable_migration_alarm, both default-false and so +// unexercised by any deploy. The ARNs are the real shapes from the Service +// Authorization Reference: an SNS topic ARN has NO type segment, a CloudWatch +// dashboard ARN has an empty region and a `/` before the name, and an alarm ARN +// uses `alarm:` (#157). +func TestDeployPolicyGrantsEveryActionTheMonitoringModuleNeeds(t *testing.T) { + const ( + topic = "arn:aws:sns:us-east-1:123456789012:cudly-prod-abc-alerts" + dashboard = "arn:aws:cloudwatch::123456789012:dashboard/cudly-prod-abc-prod" + alarm = "arn:aws:cloudwatch:us-east-1:123456789012:alarm:cudly-prod-abc-lambda-errors" + lambdaLG = "arn:aws:logs:us-east-1:123456789012:log-group:/aws/lambda/cudly-prod-abc-api-20260101" + ecsLG = "arn:aws:logs:us-east-1:123456789012:log-group:/ecs/cudly-prod-abc" + ) + var needs []grantNeed + add := func(arn string, actions ...string) { + for _, a := range actions { + needs = append(needs, grantNeed{action: a, arn: arn}) + } + } + add(topic, "sns:CreateTopic", "sns:DeleteTopic", "sns:GetTopicAttributes", "sns:SetTopicAttributes", + "sns:Subscribe", "sns:Unsubscribe", "sns:GetSubscriptionAttributes", "sns:SetSubscriptionAttributes", + "sns:ListSubscriptionsByTopic", "sns:TagResource", "sns:UntagResource", "sns:ListTagsForResource") + add(dashboard, "cloudwatch:PutDashboard", "cloudwatch:GetDashboard", "cloudwatch:DeleteDashboards") + add(alarm, "cloudwatch:PutMetricAlarm", "cloudwatch:DeleteAlarms", "cloudwatch:TagResource") + // The migration alarm's filter is on the Lambda log group; the monitoring + // module's filters are on the Lambda and ECS groups. + add(lambdaLG, "logs:PutMetricFilter", "logs:DeleteMetricFilter") + add(ecsLG, "logs:PutMetricFilter", "logs:DeleteMetricFilter") + // No resource-level permissions for these. + add("", "logs:DescribeMetricFilters", "logs:PutQueryDefinition", "logs:DescribeQueryDefinitions", "logs:DeleteQueryDefinition") + assertGranted(t, "monitoring", needs) + + for _, stmt := range findStatements(t, "policy_monitoring.tf", func(string) bool { return true }) { + for _, r := range statementResources(stmt) { + if strings.Contains(r, ":topic/") || strings.Contains(r, ":sns:*:*:topic") { + t.Errorf("statement %q: Resource %q has a topic type segment, which an SNS topic ARN does not have", statementSid(stmt), r) + } + } + } +} From 4f48ac0fb594a52102f105bd3d6fa68cff32f894 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:02:21 +0100 Subject: [PATCH 4/7] docs(lambda): point the migration alarm gate at policy_monitoring The comment said the deploy role lacks logs:PutMetricFilter until the bootstrap grants it. policy_monitoring.tf (LogMetricFilters) now grants it and monitoring_apply_test.go pins it; the gate stays because the bootstrap must be re-applied first. Comment only. Refs #157 Co-Authored-By: Claude Opus 5.5 --- terraform/modules/compute/aws/lambda/migration-alarm.tf | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/terraform/modules/compute/aws/lambda/migration-alarm.tf b/terraform/modules/compute/aws/lambda/migration-alarm.tf index 7c62424d4..db478f7a9 100644 --- a/terraform/modules/compute/aws/lambda/migration-alarm.tf +++ b/terraform/modules/compute/aws/lambda/migration-alarm.tf @@ -23,9 +23,11 @@ resource "aws_cloudwatch_log_metric_filter" "migration_failed" { # Gated off by default. Creating a metric filter needs logs:PutMetricFilter, - # which the CI deploy SA does not hold until the ci-cd-permissions bootstrap - # grants it (root CLAUDE.md "CI/CD IAM" bootstrap-vs-runtime split). Leaving - # this ungated 403s every terraform apply and blocks all deploys. Set + # which policy_monitoring.tf (LogMetricFilters) grants on the Lambda log + # groups and monitoring_apply_test.go pins. The deploy role only + # holds it once the ci-cd-permissions bootstrap has been re-applied (root + # CLAUDE.md "CI/CD IAM" bootstrap-vs-runtime split). Leaving this ungated + # 403s every terraform apply and blocks all deploys. Set # enable_migration_alarm=true only after re-applying the bootstrap. count = var.enable_migration_alarm ? 1 : 0 From 94a0cee844890ee8533118c434cbc27026472504 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:03:16 +0100 Subject: [PATCH 5/7] fix(iam): grant acm tag removal and log retention removal acm:RemoveTagsFromCertificate is the call the provider makes to drop a tag key from aws_acm_certificate.frontend, and logs:DeleteRetentionPolicy the one for a log group retention going to 0. Neither was granted, so removing a tag or setting lambda_log_retention_days = 0 would fail the apply. ACM stays on "*" like its siblings (opaque certificate ids); the logs action reuses the three existing cudly-* log-group scopes. Tests check both on real ARNs (failing before the grant) and pin that any statement granting PutRetentionPolicy also grants Delete. Policy size (compact JSON): compute 5933 -> 5962, networking 2755 -> 2787, both of 6144. Refs #157 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/optin_paths_test.go | 36 +++++++++++++++++++ .../aws/ci-cd-permissions/policy_compute.tf | 1 + .../ci-cd-permissions/policy_networking.tf | 1 + 3 files changed, 38 insertions(+) create mode 100644 terraform/environments/aws/ci-cd-permissions/optin_paths_test.go diff --git a/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go b/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go new file mode 100644 index 000000000..9c3a547c0 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go @@ -0,0 +1,36 @@ +package cicdpermissions_test + +import "testing" + +// TestDeployPolicyGrantsTagAndRetentionRemoval covers calls the AWS provider +// makes when an attribute is REMOVED rather than set, which a first apply never +// exercises (#157). Real ARN shapes per the Service Authorization Reference. +func TestDeployPolicyGrantsTagAndRetentionRemoval(t *testing.T) { + const logs = "arn:aws:logs:us-east-1:123456789012:log-group:" + assertGranted(t, "acm/logs removal", []grantNeed{ + // aws_acm_certificate.frontend drops a tag key (environments/aws/acm.tf). + {"acm:RemoveTagsFromCertificate", "arn:aws:acm:us-east-1:123456789012:certificate/11111111-2222-3333-4444-555555555555"}, + // A log group retention going from N days to 0 (never expire). + {"logs:DeleteRetentionPolicy", logs + "/aws/lambda/cudly-prod-abc-api-20260101"}, + {"logs:DeleteRetentionPolicy", logs + "/ecs/cudly-prod-abc"}, + {"logs:DeleteRetentionPolicy", logs + "/aws/vpc/cudly-prod-abc"}, + }) +} + +// TestPutRetentionPolicyIsPairedWithDelete: a statement that can set a log +// retention must also be able to clear it, or lowering it to 0 fails the apply. +func TestPutRetentionPolicyIsPairedWithDelete(t *testing.T) { + seen := false + for _, stmt := range deployAllowStatements(t) { + if !actionPermitted(statementActions(stmt), "logs:PutRetentionPolicy") { + continue + } + seen = true + if !actionPermitted(statementActions(stmt), "logs:DeleteRetentionPolicy") { + t.Errorf("statement %q grants logs:PutRetentionPolicy without logs:DeleteRetentionPolicy", statementSid(stmt)) + } + } + if !seen { + t.Fatal("no statement grants logs:PutRetentionPolicy; the pairing check would pass vacuously") + } +} diff --git a/terraform/environments/aws/ci-cd-permissions/policy_compute.tf b/terraform/environments/aws/ci-cd-permissions/policy_compute.tf index 55d1554c0..a424295e7 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_compute.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_compute.tf @@ -35,6 +35,7 @@ resource "aws_iam_policy" "compute" { Action = [ "logs:CreateLogGroup", "logs:DeleteLogGroup", + "logs:DeleteRetentionPolicy", "logs:ListTagsForResource", "logs:PutRetentionPolicy", "logs:TagResource", diff --git a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf index c1521f081..c1d8829a1 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf @@ -154,6 +154,7 @@ resource "aws_iam_policy" "networking" { "acm:DescribeCertificate", "acm:GetCertificate", "acm:ListTagsForCertificate", + "acm:RemoveTagsFromCertificate", "acm:RequestCertificate", ] Resource = "*" From 1fb4332435f16b78d9da4bded22433b7fe452015 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:04:17 +0100 Subject: [PATCH 6/7] fix(iam): grant hosted zone lifecycle for the subdomain zone create_subdomain_zone = true makes aws_route53_zone.subdomain call CreateHostedZone, ChangeTagsForResource and (on destroy) DeleteHostedZone, none of which were granted. CreateHostedZone has no resource type and joins the "*" Route53 actions; the other two go in a new statement on arn:aws:route53:::hostedzone/*. The SAR defines no condition keys for the opaque hostedzone id, so neither a name scope nor a tag gate is possible, and the role already changes records on "*", a stronger capability. Policy size (compact JSON): networking 2787 -> 2982 of 6144. Refs #157 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/optin_paths_test.go | 13 +++++++++++++ .../aws/ci-cd-permissions/policy_networking.tf | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go b/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go index 9c3a547c0..cf264f167 100644 --- a/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go +++ b/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go @@ -34,3 +34,16 @@ func TestPutRetentionPolicyIsPairedWithDelete(t *testing.T) { t.Fatal("no statement grants logs:PutRetentionPolicy; the pairing check would pass vacuously") } } + +// TestDeployPolicyGrantsHostedZoneLifecycle covers create_subdomain_zone = true +// (environments/aws/route53.tf). CreateHostedZone has no resource type; the +// hostedzone ARN carries an opaque id and the SAR defines no condition keys for +// it, so neither a name scope nor a tag gate is possible (#157). +func TestDeployPolicyGrantsHostedZoneLifecycle(t *testing.T) { + const zone = "arn:aws:route53:::hostedzone/Z0123456789ABCDEFGHIJ" + assertGranted(t, "route53 zone", []grantNeed{ + {"route53:CreateHostedZone", ""}, + {"route53:ChangeTagsForResource", zone}, + {"route53:DeleteHostedZone", zone}, + }) +} diff --git a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf index c1d8829a1..692f081a3 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_networking.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_networking.tf @@ -164,6 +164,7 @@ resource "aws_iam_policy" "networking" { Effect = "Allow" Action = [ "route53:ChangeResourceRecordSets", + "route53:CreateHostedZone", "route53:GetChange", "route53:GetHostedZone", "route53:ListHostedZones", @@ -172,6 +173,19 @@ resource "aws_iam_policy" "networking" { ] Resource = "*" }, + { + # create_subdomain_zone (environments/aws/route53.tf). The hostedzone + # id is opaque and the SAR defines no condition keys for it, so it can + # be scoped by neither name nor tag. The role already changes records + # on "*" above, a stronger capability than deleting an (empty) zone. + Sid = "Route53HostedZoneLifecycle" + Effect = "Allow" + Action = [ + "route53:ChangeTagsForResource", + "route53:DeleteHostedZone", + ] + Resource = "arn:aws:route53:::hostedzone/*" + }, ] }) From 5f738f629b9823acf35a8ec778a1e03ef72eabfe Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 16:22:35 +0100 Subject: [PATCH 7/7] test(iam): follow decision A in the RDS proxy tripwire and bound the new grants The tripwire checklist now tags-gates both db-proxy:* and target-group:* after a live check that the default target group inherits the Project tag, and goes back to the owner if it does not. TestRDSProxyMutatingActionsNeedProxyARNs checks the SAR resource types per action instead of demanding db-proxy and target-group for all of them, so decision A's own statements pass. New upper bounds keep the #157 actions off "*" (acm:RemoveTagsFromCertificate exempt like its siblings, route53:CreateHostedZone has no resource type), keep DeleteRetentionPolicy on the cudly log-group prefixes and the hosted zone actions on hostedzone/*. A need with an empty arn now fails unless marked noResource. Refs #157 Co-Authored-By: Claude Opus 5.5 --- .../aws/ci-cd-permissions/grant_needs_test.go | 24 ++++-- .../monitoring_apply_test.go | 4 +- .../aws/ci-cd-permissions/optin_paths_test.go | 84 +++++++++++++++++-- .../rds_proxy_tripwire_test.go | 13 +-- .../aws/ci-cd-permissions/rds_scopes_test.go | 64 +++++++++----- 5 files changed, 148 insertions(+), 41 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go b/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go index c430418a3..6e1c70248 100644 --- a/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go +++ b/terraform/environments/aws/ci-cd-permissions/grant_needs_test.go @@ -3,11 +3,13 @@ package cicdpermissions_test import "testing" // grantNeed is one action a Terraform apply performs, with a sample real ARN -// of the resource it runs against. An empty arn means the action has no -// resource type (action-only). +// of the resource it runs against. noResource marks an action the SAR gives no +// resource type (action-only); without it an empty arn is a bug in the need and +// fails instead of matching any statement. type grantNeed struct { - action string - arn string + action string + arn string + noResource bool } // grantedBy reports whether any statement in allow authorizes need. @@ -16,9 +18,12 @@ func grantedBy(allow []string, need grantNeed) bool { if !actionPermitted(statementActions(stmt), need.action) { continue } - if need.arn == "" { + if need.noResource { return true } + if need.arn == "" { + return false + } for _, r := range statementResources(stmt) { if iamARNMatches(r, need.arn) { return true @@ -75,10 +80,11 @@ func TestGrantedBy(t *testing.T) { need grantNeed want bool }{ - {"action only", stmt(`"*"`), grantNeed{"rds:ModifyDBProxy", ""}, true}, - {"wrong action", stmt(`"*"`), grantNeed{"rds:DeleteDBProxy", ""}, false}, - {"real arn", stmt(`["arn:aws:rds:*:*:db-proxy:*"]`), grantNeed{"rds:ModifyDBProxy", proxy}, true}, - {"dead arn scope", stmt(`["arn:aws:rds:*:*:proxy:cudly-*"]`), grantNeed{"rds:ModifyDBProxy", proxy}, false}, + {"action only", stmt(`"*"`), grantNeed{action: "rds:ModifyDBProxy", noResource: true}, true}, + {"wrong action", stmt(`"*"`), grantNeed{action: "rds:DeleteDBProxy", noResource: true}, false}, + {"real arn", stmt(`["arn:aws:rds:*:*:db-proxy:*"]`), grantNeed{action: "rds:ModifyDBProxy", arn: proxy}, true}, + {"dead arn scope", stmt(`["arn:aws:rds:*:*:proxy:cudly-*"]`), grantNeed{action: "rds:ModifyDBProxy", arn: proxy}, false}, + {"empty arn is not action-only", stmt(`"*"`), grantNeed{action: "rds:ModifyDBProxy"}, false}, } for _, c := range cases { if got := grantedBy([]string{c.stmt}, c.need); got != c.want { diff --git a/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go b/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go index 277cfe942..430716a2b 100644 --- a/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go +++ b/terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go @@ -36,7 +36,9 @@ func TestDeployPolicyGrantsEveryActionTheMonitoringModuleNeeds(t *testing.T) { add(lambdaLG, "logs:PutMetricFilter", "logs:DeleteMetricFilter") add(ecsLG, "logs:PutMetricFilter", "logs:DeleteMetricFilter") // No resource-level permissions for these. - add("", "logs:DescribeMetricFilters", "logs:PutQueryDefinition", "logs:DescribeQueryDefinitions", "logs:DeleteQueryDefinition") + for _, a := range []string{"logs:DescribeMetricFilters", "logs:PutQueryDefinition", "logs:DescribeQueryDefinitions", "logs:DeleteQueryDefinition"} { + needs = append(needs, grantNeed{action: a, noResource: true}) + } assertGranted(t, "monitoring", needs) for _, stmt := range findStatements(t, "policy_monitoring.tf", func(string) bool { return true }) { diff --git a/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go b/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go index cf264f167..c8d5c994e 100644 --- a/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go +++ b/terraform/environments/aws/ci-cd-permissions/optin_paths_test.go @@ -9,11 +9,11 @@ func TestDeployPolicyGrantsTagAndRetentionRemoval(t *testing.T) { const logs = "arn:aws:logs:us-east-1:123456789012:log-group:" assertGranted(t, "acm/logs removal", []grantNeed{ // aws_acm_certificate.frontend drops a tag key (environments/aws/acm.tf). - {"acm:RemoveTagsFromCertificate", "arn:aws:acm:us-east-1:123456789012:certificate/11111111-2222-3333-4444-555555555555"}, + {action: "acm:RemoveTagsFromCertificate", arn: "arn:aws:acm:us-east-1:123456789012:certificate/11111111-2222-3333-4444-555555555555"}, // A log group retention going from N days to 0 (never expire). - {"logs:DeleteRetentionPolicy", logs + "/aws/lambda/cudly-prod-abc-api-20260101"}, - {"logs:DeleteRetentionPolicy", logs + "/ecs/cudly-prod-abc"}, - {"logs:DeleteRetentionPolicy", logs + "/aws/vpc/cudly-prod-abc"}, + {action: "logs:DeleteRetentionPolicy", arn: logs + "/aws/lambda/cudly-prod-abc-api-20260101"}, + {action: "logs:DeleteRetentionPolicy", arn: logs + "/ecs/cudly-prod-abc"}, + {action: "logs:DeleteRetentionPolicy", arn: logs + "/aws/vpc/cudly-prod-abc"}, }) } @@ -42,8 +42,78 @@ func TestPutRetentionPolicyIsPairedWithDelete(t *testing.T) { func TestDeployPolicyGrantsHostedZoneLifecycle(t *testing.T) { const zone = "arn:aws:route53:::hostedzone/Z0123456789ABCDEFGHIJ" assertGranted(t, "route53 zone", []grantNeed{ - {"route53:CreateHostedZone", ""}, - {"route53:ChangeTagsForResource", zone}, - {"route53:DeleteHostedZone", zone}, + {action: "route53:CreateHostedZone", noResource: true}, + {action: "route53:ChangeTagsForResource", arn: zone}, + {action: "route53:DeleteHostedZone", arn: zone}, }) } + +// resourcesOf returns the Resource entries of every deploy Allow statement that +// grants action, failing when none does (a vacuous bound). +func resourcesOf(t *testing.T, action string) map[string][]string { + t.Helper() + out := map[string][]string{} + for _, stmt := range deployAllowStatements(t) { + if actionPermitted(statementActions(stmt), action) { + out[statementSid(stmt)] = statementResources(stmt) + } + } + if len(out) == 0 { + t.Fatalf("no deploy statement grants %s; its upper-bound check would pass vacuously", action) + } + return out +} + +// TestNewActionsAreNotGrantedOnWildcard is the upper bound for the #157 +// additions: they stay off Resource "*" where the SAR gives them a resource +// type. route53:CreateHostedZone has none, so it is exempt. The ACM actions +// stay on "*" like their siblings DeleteCertificate and AddTagsToCertificate, +// which this PR must not reshape: acm:RemoveTagsFromCertificate is exempt here +// and a tag gate for the whole ACM group is a follow-up. +func TestNewActionsAreNotGrantedOnWildcard(t *testing.T) { + for _, action := range []string{ + "logs:DeleteRetentionPolicy", + "route53:DeleteHostedZone", + "route53:ChangeTagsForResource", + } { + for sid, resources := range resourcesOf(t, action) { + for _, r := range resources { + if r == "*" { + t.Errorf("statement %q grants %s on Resource \"*\"; the SAR gives it a resource type", sid, action) + } + } + } + } +} + +func TestRetentionRemovalStaysOnCudlyLogGroups(t *testing.T) { + prefixes := []string{ + "arn:aws:logs:*:*:log-group:/aws/lambda/cudly-", + "arn:aws:logs:*:*:log-group:/aws/vpc/cudly-", + "arn:aws:logs:*:*:log-group:/ecs/cudly-", + } + for sid, resources := range resourcesOf(t, "logs:DeleteRetentionPolicy") { + for _, r := range resources { + ok := false + for _, p := range prefixes { + ok = ok || (len(r) > len(p) && r[:len(p)] == p) + } + if !ok { + t.Errorf("statement %q grants logs:DeleteRetentionPolicy on %q, outside the cudly log-group prefixes", sid, r) + } + } + } +} + +func TestHostedZoneActionsStayOnHostedZoneARNs(t *testing.T) { + const want = "arn:aws:route53:::hostedzone/*" + for _, action := range []string{"route53:DeleteHostedZone", "route53:ChangeTagsForResource"} { + for sid, resources := range resourcesOf(t, action) { + for _, r := range resources { + if r != want { + t.Errorf("statement %q grants %s on %q, want only %q", sid, action, r, want) + } + } + } + } +} diff --git a/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go b/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go index cc8f77b21..1eb6b7734 100644 --- a/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go +++ b/terraform/environments/aws/ci-cd-permissions/rds_proxy_tripwire_test.go @@ -10,12 +10,15 @@ import ( ) const rdsProxyGrantsNeeded = `enable_rds_proxy is no longer a literal false, so the RDS proxy can be created, and the deploy role cannot manage it (#157). -Before enabling it, add to the deploy policy (policy_data.tf): - - db-proxy:* with StringEqualsIgnoreCase aws:ResourceTag/Project = CUDly: rds:ModifyDBProxy, rds:DeleteDBProxy, - rds:DeregisterDBProxyTargets, rds:AddTagsToResource, rds:RemoveTagsFromResource, and rds:ListTagsForResource +Decision A (#157): first run the real-AWS check that the default target group inherits the proxy's Project tag +(create a proxy with tags in staging, then rds list-tags-for-resource on its target-group ARN). If it is +untagged, go back to the owner; do not grant target-group:* without a gate. If tagged, add to the deploy policy +(policy_data.tf), both gated by StringEqualsIgnoreCase aws:ResourceTag/Project = CUDly: + - db-proxy:*: rds:ModifyDBProxy, rds:DeleteDBProxy, rds:DeregisterDBProxyTargets, rds:AddTagsToResource, + rds:RemoveTagsFromResource, and rds:ListTagsForResource (the provider reads tags on every refresh, proxy.go @Tags(identifierAttribute="arn")) - - target-group:* (no tag condition; the default target group has no tags argument): rds:ModifyDBProxyTargetGroup, - rds:RegisterDBProxyTargets (also needs db:cudly-*), rds:DeregisterDBProxyTargets + - target-group:*: rds:ModifyDBProxyTargetGroup, rds:RegisterDBProxyTargets (also needs db:cudly-*), + rds:DeregisterDBProxyTargets - rds:CreateDBProxy stays on "*" (tags travel inside the create call) Then exercise it once in staging (create, no-op re-plan, destroy) and update this tripwire.` diff --git a/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go b/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go index 171526135..2fb153fd1 100644 --- a/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go +++ b/terraform/environments/aws/ci-cd-permissions/rds_scopes_test.go @@ -87,7 +87,9 @@ func TestRDSKeptActionsHaveEveryRequiredResourceType(t *testing.T) { // A newly granted action must get a table entry, or it goes unchecked. for _, stmt := range allow { for _, a := range statementActions(stmt) { - if _, ok := requiredRDSTypes[a]; !ok && !strings.HasPrefix(a, "rds:Describe") && a != "rds:CreateDBProxy" { + _, kept := requiredRDSTypes[a] + _, proxy := requiredRDSProxyTypes[a] + if !kept && !proxy && !strings.HasPrefix(a, "rds:Describe") && a != "rds:CreateDBProxy" { t.Errorf("statement %q grants %s, which has no entry in requiredRDSTypes; add its required resource types", statementSid(stmt), a) } } @@ -107,29 +109,53 @@ func TestRDSKeptActionsHaveEveryRequiredResourceType(t *testing.T) { } } -// TestRDSProxyMutatingActionsNeedProxyARNs: these four need db-proxy and -// target-group ARNs (opaque ids), which a cudly-* name scope cannot express. -// They stay out of the deploy policy until enable_rds_proxy is turned on and -// the working grants are added (see rds_proxy_tripwire_test.go). +// requiredRDSProxyTypes lists the resource types the SAR (Amazon RDS actions +// table, https://docs.aws.amazon.com/service-authorization/latest/reference/list_amazonrds.html, +// machine-readable https://servicereference.us-east-1.amazonaws.com/v1/rds/rds.json, +// read 2026-11-05) requires for each proxy action. rds.json names the proxy +// type "proxy" (ARN segment db-proxy) and does not mark required types; +// DeregisterDBProxyTargets lists cluster, db, proxy and target-group, of which +// the proxy and target group are the ones a proxy-scoped grant must cover. +// These ids are opaque (prx-..., prx-tg-...), so a cudly-* name scope cannot +// express them. Not granted until enable_rds_proxy is turned on (see +// rds_proxy_tripwire_test.go). +var requiredRDSProxyTypes = map[string][]string{ + "rds:DeleteDBProxy": {"db-proxy"}, + "rds:ModifyDBProxy": {"db-proxy"}, + "rds:RegisterDBProxyTargets": {"db", "target-group"}, + "rds:DeregisterDBProxyTargets": {"db-proxy", "target-group"}, + "rds:ModifyDBProxyTargetGroup": {"target-group"}, +} + +// TestRDSProxyMutatingActionsNeedProxyARNs: a proxy action, once granted, must +// be authorizable against every resource type it requires, and no statement +// granting it may match none of them (a dead grant). func TestRDSProxyMutatingActionsNeedProxyARNs(t *testing.T) { allow := deployAllowStatements(t) - for _, action := range []string{ - "rds:DeleteDBProxy", - "rds:ModifyDBProxy", - "rds:RegisterDBProxyTargets", - "rds:DeregisterDBProxyTargets", - } { + for action, types := range requiredRDSProxyTypes { + var holders []string for _, stmt := range allow { - if !actionPermitted(statementActions(stmt), action) { - continue + if actionPermitted(statementActions(stmt), action) { + holders = append(holders, stmt) + } + } + if len(holders) == 0 { + continue // not granted yet; the tripwire covers that + } + for _, stmt := range holders { + matched := false + for _, typ := range types { + for _, r := range statementResources(stmt) { + matched = matched || iamARNMatches(r, realRDSARNs[typ]) + } } - var proxy, targetGroup bool - for _, r := range statementResources(stmt) { - proxy = proxy || iamARNMatches(r, realRDSARNs["db-proxy"]) - targetGroup = targetGroup || iamARNMatches(r, realRDSARNs["target-group"]) + if !matched { + t.Errorf("statement %q grants %s but its Resource matches none of %v; it is a dead grant (#157)", statementSid(stmt), action, types) } - if !proxy || !targetGroup { - t.Errorf("statement %q grants %s without both db-proxy and target-group Resource entries that match real ARNs; it is a dead grant (#157)", statementSid(stmt), action) + } + for _, typ := range types { + if !grantedBy(holders, grantNeed{action: action, arn: realRDSARNs[typ]}) { + t.Errorf("%s: required resource type %s (%s) is not granted", action, typ, realRDSARNs[typ]) } } }