fix(iam): drain remaining deploy-role grant gaps and pin them with tests

terraform/environments/aws/ci-cd-permissions/grant_needs_test.go

@@ -0,0 +1,94 @@
+package cicdpermissions_test
+
+import "testing"
+
+// grantNeed is one action a Terraform apply performs, with a sample real ARN
+// 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
+	noResource bool
+}
+
+// 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.noResource {
+			return true
+		}
+		if need.arn == "" {
+			return false
+		}
+		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{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 {
+			t.Errorf("%s: grantedBy = %v, want %v", c.name, got, c.want)
+		}
+	}
+}

terraform/environments/aws/ci-cd-permissions/monitoring_apply_test.go

@@ -0,0 +1,51 @@
+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.
+	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 }) {
+		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)
+			}
+		}
+	}
+}

terraform/environments/aws/ci-cd-permissions/optin_paths_test.go

@@ -0,0 +1,119 @@
+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).
+		{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).
+		{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"},
+	})
+}
+
+// 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")
+	}
+}
+
+// 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{
+		{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)
+				}
+			}
+		}
+	}
+}

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",

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:<r>:<a>:db-proxy:prx-<opaque> and
-        # arn:aws:rds:<r>:<a>:target-group:prx-tg-<opaque>: 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

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 = "*"
@@ -163,6 +164,7 @@ resource "aws_iam_policy" "networking" {
         Effect = "Allow"
         Action = [
           "route53:ChangeResourceRecordSets",
+          "route53:CreateHostedZone",
           "route53:GetChange",
           "route53:GetHostedZone",
           "route53:ListHostedZones",
@@ -171,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/*"
+      },
     ]
   })