diff --git a/terraform/environments/aws/ci-cd-permissions/README.md b/terraform/environments/aws/ci-cd-permissions/README.md index 6cb9c528c..10de3defd 100644 --- a/terraform/environments/aws/ci-cd-permissions/README.md +++ b/terraform/environments/aws/ci-cd-permissions/README.md @@ -11,9 +11,31 @@ CI/CD pipeline uses to deploy infrastructure on AWS. It optionally sets up keyle | `aws_iam_role.cudly_deploy` | The deploy role; assumed by GitHub Actions or a human operator | | `aws_iam_policy.networking` | VPC, subnets, security groups, ALB, ECS cluster | | `aws_iam_policy.compute` | ECS services/tasks, ECR, CloudWatch Logs, SSM | +| `aws_iam_policy.compute_b` | Overflow for `aws_iam_policy.compute`, which is close to the 6144-character managed-policy limit | | `aws_iam_policy.data` | RDS, ElastiCache, S3 (state bucket), Secrets Manager | +| `aws_iam_policy.iam` | IAM role creation and policy attachment, gated on the permissions boundary below (#1705) | +| `aws_iam_policy.workload_boundary` | `cudly-deploy-boundary`: the permissions ceiling every role the deploy role creates must carry | | `aws_iam_openid_connect_provider.github` | GitHub Actions OIDC provider (conditional on `github_repo`) | +## Apply this root BEFORE merging changes that touch the boundary + +This root is applied by hand, by a privileged human, and never by a deploy workflow. That +makes the ordering between it and `terraform/environments/aws` load-bearing whenever the +permissions boundary is involved: + +- **Apply here first, then merge.** `deploy-aws-lambda.yml` triggers on pushes to `main` + under `terraform/environments/aws/**` and the AWS compute/database/secrets/networking + modules. A merge that adds or changes `permissions_boundary` on a module role makes the + next deploy call `iam:PutRolePermissionsBoundary`, and that grant lives in + `aws_iam_policy.iam` here. Merging before applying this root turns every AWS deploy red + with `AccessDenied` until the apply happens. It is recoverable, not destructive, but it + is avoidable. +- **Rolling back past the boundary change needs care.** `rollback.yml` applies the + environment root from the dispatched ref's checkout. A ref that predates the boundary has + no `permissions_boundary` in config while the live roles have one, so the provider issues + `iam:DeleteRolePermissionsBoundary`, which `IAMDenyStripRoleBoundary` in `policy_iam.tf` + denies. Roll back to a ref at or after the boundary change. + ## Prerequisites - Terraform >= 1.6 diff --git a/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf new file mode 100644 index 000000000..cdc486c6c --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf @@ -0,0 +1,213 @@ +# Permissions boundary for every IAM role cudly-terraform-deploy creates or +# manages. This is the ceiling half of the fix for #1705; policy_iam.tf holds +# the delegation half (the iam:PermissionsBoundary conditions that force the +# deploy role to apply this boundary, and the iam:PolicyARN allowlist). +# +# WHY A BOUNDARY AND NOT A DENY. The deploy role legitimately creates roles on +# every apply: twelve of them across terraform/modules (Lambda execution, +# Fargate task and task-execution, four EventBridge invoker roles, fck-nat, VPC +# flow logs, RDS proxy, secret rotation, and the cleanup Lambda's role, which is +# declared but not instantiated from any environment root today). Which of the +# twelve exist in a given environment depends on compute_platform and the +# enable_* flags; the count is not the point, the fact that the deploy path +# creates roles at all is. Denying iam:CreateRole, +# iam:PutRolePolicy or iam:AttachRolePolicy outright takes the pipeline down. +# Denying by name does not help either: the escalation target is a role, so any +# name the deploy role may legitimately use is also a name it may abuse. What +# distinguishes a legitimate role from an escalation vehicle is not its name but +# its ceiling, and a permissions boundary is the only AWS mechanism that caps +# what a principal can do regardless of what its identity policy says. It is +# also the only thing that closes iam:PutRolePolicy, whose inline document has +# no condition key at all and so cannot be constrained any other way. +# +# THE NAME IS LOAD-BEARING. `cudly-deploy-boundary` matches `cudly-deploy-*`, so +# IAMDenyModifyDeployRoleAndPolicies in policy_data.tf already denies the deploy +# role iam:CreatePolicyVersion, iam:SetDefaultPolicyVersion, iam:DeletePolicy +# and iam:DeletePolicyVersion against it. Those four are the complete mutation +# set for a managed policy, and none of them supports a condition key (verified +# against the AWS Service Authorization Reference), so a resource-scoped Deny is +# the only way to protect this document. Renaming it out of the cudly-deploy-* +# namespace silently removes that protection and lets a compromised deploy role +# rewrite its own ceiling. +# +# THE ALLOW LIST IS A CEILING, NOT A GRANT. A boundary grants nothing on its +# own; effective permissions are the intersection of it and the role's identity +# policy. Every service listed below is already reachable by at least one +# workload role today, so this document removes no permission any role currently +# has. Granularity is deliberately per-service rather than per-action: a +# too-narrow boundary fails at RUNTIME (a Lambda 403s in production) rather than +# at apply time, which is a far worse failure mode than the apply-time 403s this +# module has produced before (#1496, #1514, #1671, #1698). Per-service keeps the +# blast radius of a miss to "a whole new AWS service was added", which is a +# conscious change, rather than "someone added one more action". +# +# DRIFT IS GUARDED IN CI. Granting a workload role an action in a service that +# is not listed here would deploy cleanly and then 403 at runtime. That is +# exactly the invisible failure the per-service granularity is meant to make +# rare, and TestBoundaryCoversWorkloadServices in +# terraform/environments/aws/ci-cd-permissions/policy_guard_test.go turns what +# is left of it into a CI failure: it re-derives the service set from the IAM +# policy documents in terraform/modules and fails if any of them is missing +# here. Add the service to the list below in the same change. +resource "aws_iam_policy" "workload_boundary" { + name = "cudly-deploy-boundary" + description = "CUDly Terraform deploy: permissions ceiling for every role the deploy role creates or manages" + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + # The services CUDly workload roles actually use. Derived from the IAM + # policy documents in terraform/modules plus the four AWS managed + # policies those modules attach: AWSLambdaBasicExecutionRole (logs), + # AWSLambdaVPCAccessExecutionRole (ec2, logs), + # AmazonECSTaskExecutionRolePolicy (ecr, logs) and + # AmazonSSMManagedInstanceCore (ssm, ssmmessages, ec2messages, s3, ec2). + # + # `iam:` is absent on purpose and is the whole point of the statement: + # because a boundary caps by intersection, omitting a service denies it + # outright. No workload role can create a role, attach a policy, write + # an inline policy, mint an access key or touch a permissions boundary, + # no matter what its identity policy says. That holds for IAM actions + # that do not exist yet, which an explicit Deny list could not promise. + # iam:PassRole is the one exception and gets its own scoped statement + # below. + Sid = "WorkloadServiceCeiling" + Effect = "Allow" + Action = [ + "ce:*", + "ec2:*", + "ec2messages:*", + "ecr:*", + "ecs:*", + "elasticache:*", + "es:*", + "kms:*", + "lambda:*", + "logs:*", + "memorydb:*", + "rds:*", + "redshift:*", + "s3:*", + "savingsplans:*", + "secretsmanager:*", + "ses:*", + "ssm:*", + "ssmmessages:*", + ] + Resource = "*" + }, + { + # organizations and sts are the two services narrowed in THIS change, + # because at `service:*` granularity each of them is a complete escape + # from this boundary rather than a widening within it: the criterion is + # "does this let a boundaried role keep running as a DIFFERENT + # principal, with no iam:PassRole involved" (PassRoleCeiling below is + # what scopes PassRole itself, so it does not help here). + # + # - organizations:* includes CreateAccount (mints a fresh account that + # trusts this one), AttachPolicy/DetachPolicy (rewrites SCPs) and + # RemoveAccountFromOrganization. + # - sts:AssumeRole on "*" is the shorter twin of the unscoped + # iam:PassRole that PassRoleCeiling below deliberately refuses to + # grant. The assumed session is a DIFFERENT principal, so this + # boundary does not follow it: one AssumeRole into a role that + # trusts this account (OrganizationAccountAccessRole in every member + # account trusts the management account root) and the ceiling is + # simply gone. + # + # THIS IS NOT THE COMPLETE SET, and the statement below should not be + # read as a finished escape analysis. At least three more services meet + # the same criterion and are left at full service width in + # WorkloadServiceCeiling above: lambda:* (UpdateFunctionCode on any + # function, then invoke, runs as that function's execution role), + # ssm:* (SendCommand / StartSession to any SSM-managed instance, runs + # as its instance profile) and ecs:* (UpdateService onto an existing + # task definition revision, or ExecuteCommand into a running task). + # None of those three needs iam:PassRole, so PassRoleCeiling's + # cudly-*-only scoping does not constrain them either. Narrowing them + # is out of scope for this change and tracked in #1723. + # + # organizations and sts are therefore pinned to exactly what the + # modules grant. organizations is action-scoped rather than + # resource-scoped because the Organizations API supports no + # resource-level restrictions (see the org_discovery policy in + # modules/compute/aws/{lambda,fargate}/main.tf). + Sid = "OrganizationsDiscoveryCeiling" + Effect = "Allow" + Action = [ + "organizations:DescribeAccount", + "organizations:DescribeOrganization", + "organizations:ListAccounts", + ] + Resource = "*" + }, + { + # sts:GetCallerIdentity is separated from AssumeRole because it takes no + # resource. It is listed even though no module grants it: AWS documents + # it as requiring no permissions, so code may call it without an + # explicit grant, and a boundary that omitted it could turn an + # unremarkable call into a runtime failure. + Sid = "StsIdentityCeiling" + Effect = "Allow" + Action = ["sts:GetCallerIdentity"] + Resource = "*" + }, + { + # COUPLED TO A MODULE DEFAULT. This mirrors the Resource on the + # cross_account_sts inline policy in + # modules/compute/aws/{lambda,fargate}/main.tf, which is + # arn:aws:iam::*:role/${var.cross_account_role_name_prefix}* with + # cross_account_role_name_prefix defaulting to "CUDly". Overriding that + # variable without widening this statement caps the override and the + # cross-account read 403s at RUNTIME, not at apply time. + # TestBoundaryMatchesCrossAccountRolePrefix in policy_guard_test.go + # fails in CI if the module default and this pattern drift apart. + # + # Scoped rather than "*" for the reason given in the previous statement: + # an unscoped AssumeRole escapes the boundary entirely. + Sid = "CrossAccountAssumeRoleCeiling" + Effect = "Allow" + Action = ["sts:AssumeRole"] + Resource = ["arn:aws:iam::*:role/CUDly*"] + }, + { + # The four EventBridge invoker roles in modules/compute/aws/fargate pass + # the task and task-execution roles to ecs:RunTask, so iam:PassRole + # cannot be omitted from the ceiling. It is scoped to cudly-* rather + # than "*" because an unscoped PassRole re-opens the escalation this + # boundary closes: a boundaried role with lambda:* and PassRole on "*" + # could hand an unrelated administrator role to a new Lambda and run as + # it. Scoping to cudly-* confines it to roles this deploy role created, + # which carry this boundary because iam:CreateRole is now conditioned on + # it (policy_iam.tf). It is NOT a guarantee that every cudly-* role in + # the account is boundaried: one created by hand, from the console, or + # before this change carries no boundary and remains a legal PassRole + # target. Closing that would need a tag or a naming split, and is not + # worth the coupling; the roles Terraform manages are all boundaried + # after the first apply. + Sid = "PassRoleCeiling" + Effect = "Allow" + Action = ["iam:PassRole"] + Resource = ["arn:aws:iam::*:role/cudly-*"] + }, + { + # cudly-terraform-deploy is itself a cudly-* role, so PassRoleCeiling + # above would otherwise let a boundaried workload role pass the deploy + # role to a Lambda and inherit the deploy role's permissions. That is a + # workload -> deploy escalation rather than deploy -> admin, but it is + # the same shape as the #542 self-pass loop and is closed the same way. + # An explicit Deny always beats the Allow. + Sid = "DenyPassDeployRole" + Effect = "Deny" + Action = ["iam:PassRole"] + Resource = ["arn:aws:iam::*:role/cudly-terraform-deploy"] + }, + ] + }) + + tags = { + Project = "CUDly" + ManagedBy = "terraform" + } +} diff --git a/terraform/environments/aws/ci-cd-permissions/policy_data.tf b/terraform/environments/aws/ci-cd-permissions/policy_data.tf index 22996efb3..cc5ff66e2 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_data.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_data.tf @@ -8,13 +8,20 @@ resource "aws_iam_policy" "data" { { Sid = "IAMRolesAndPolicies" Effect = "Allow" + # iam:AttachRolePolicy, iam:CreateRole and iam:PutRolePolicy USED TO BE + # in this list. They moved to IAMRoleMutationRequiresBoundary in + # policy_iam.tf, which re-grants them only when the target role carries + # the cudly-deploy-boundary permissions boundary (#1705). They must not + # come back here: an unconditioned Allow authorizes the request on its + # own, so restoring any of the three silently reopens the escalation and + # makes the conditioned statement decorative. Anything added below is + # granted with no ceiling at all, so check first whether it belongs in + # policy_iam.tf instead. Action = [ "iam:AddRoleToInstanceProfile", - "iam:AttachRolePolicy", "iam:CreateInstanceProfile", "iam:CreatePolicy", "iam:CreatePolicyVersion", - "iam:CreateRole", "iam:DeleteInstanceProfile", "iam:DeletePolicy", "iam:DeletePolicyVersion", @@ -31,7 +38,6 @@ resource "aws_iam_policy" "data" { "iam:ListPolicyVersions", "iam:ListRolePolicies", "iam:ListRoleTags", - "iam:PutRolePolicy", "iam:RemoveRoleFromInstanceProfile", "iam:TagInstanceProfile", "iam:TagPolicy", diff --git a/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go new file mode 100644 index 000000000..4a21b3c43 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/policy_guard_test.go @@ -0,0 +1,1056 @@ +// Package cicdpermissions_test guards the two halves of the #1705 IAM +// escalation fix against drift between the Terraform modules that actually +// run workload roles and the two policy documents in this directory that are +// supposed to bound them. +// +// policy_boundary.tf defines a permissions boundary that caps every role the +// deploy role creates: a service missing from its WorkloadServiceCeiling +// allow list is invisible at `terraform apply` time (the apply succeeds) +// and only shows up as a runtime AccessDenied the first time a Lambda or +// ECS task calls that service, per the "THE ALLOW LIST IS A CEILING, NOT A +// GRANT" note in that file. policy_iam.tf is the delegation half: it forces +// every role the deploy role creates to carry that boundary and allowlists +// the exact managed-policy ARNs the deploy role may attach, closing the +// escalation named in #1705 where any managed policy in the account +// (AdministratorAccess included) could be attached to any cudly-* role. +// +// These tests re-derive both sides from source so a module change that +// silently widens what a workload role can do, or a boundary/allowlist edit +// that silently narrows what modules need, fails CI instead of surfacing as +// a production 403 or an apply-time AccessDenied. +// +// Two of them (TestDeployPolicyGatesRoleMutationOnBoundary and +// TestDeployPolicyDeniesUnapprovedManagedPolicyAttachment) do not check for +// drift at all: they pin the internal shape of the two statements that do the +// actual work, because every way of reopening #1705 by hand is a single-token +// edit inside one of them (StringEquals -> StringEqualsIfExists, +// ArnNotEquals -> ArnEquals, a "*" added to the ARN allowlist, Deny -> Allow) +// that leaves every drift-based assertion in this file green. +package cicdpermissions_test + +import ( + "fmt" + "io/fs" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" +) + +// modulesDir is the Terraform module tree these tests re-derive the +// workload-role IAM surface from, relative to this test's own directory +// (terraform/environments/aws/ci-cd-permissions). +const modulesDir = "../../../modules" + +// envRootDir is the environment root that instantiates those modules +// (terraform/environments/aws), one level up from this directory. Its own +// *.tf files are expected to contain no aws_iam_role of their own: every role +// on the deploy path must come from a module under modulesDir, because that is +// the only tree TestEveryModuleRoleHasPermissionsBoundary can see. +const envRootDir = ".." + +// boundaryFile and iamFile are the two policy documents under test, in the +// same directory as this test file. dataFile holds the Deny that protects +// them both. +const ( + boundaryFile = "policy_boundary.tf" + iamFile = "policy_iam.tf" + dataFile = "policy_data.tf" +) + +// envMainFile is the environment root file that hardcodes the boundary ARN +// passed down to every module as var.permissions_boundary_arn. +var envMainFile = filepath.Join(envRootDir, "main.tf") + +// crossAccountModuleVariableFiles are the two modules that declare +// cross_account_role_name_prefix. Their defaults must agree with each other +// and with CrossAccountAssumeRoleCeiling in policy_boundary.tf; see +// TestBoundaryMatchesCrossAccountRolePrefix. +var crossAccountModuleVariableFiles = []string{ + filepath.Join(modulesDir, "compute", "aws", "lambda", "variables.tf"), + filepath.Join(modulesDir, "compute", "aws", "fargate", "variables.tf"), +} + +// crossAccountPrefixVariable is the module variable whose default the +// CrossAccountAssumeRoleCeiling statement in policy_boundary.tf mirrors. +const crossAccountPrefixVariable = "cross_account_role_name_prefix" + +// deployPolicyNamePrefix is the managed-policy name prefix that +// IAMDenyModifyDeployRoleAndPolicies in policy_data.tf protects (it denies the +// deploy role the complete mutation set for any policy matching +// arn:aws:iam::*:policy/cudly-deploy-*). Every policy document in this +// directory, the boundary above all, must be named inside it; see +// TestBoundaryPolicyNameStaysInProtectedNamespace and the "THE NAME IS +// LOAD-BEARING" note in policy_boundary.tf. +const deployPolicyNamePrefix = "cudly-deploy-" + +// minGuardedPolicyFiles is the number of policy_*.tf documents, excluding +// policy_iam.tf, present in this directory when this guard was written +// (policy_boundary, policy_compute, policy_compute_b, policy_data, +// policy_networking). guardedPolicyFiles globs rather than hardcodes the list +// so a policy document added later is scanned automatically, and this floor is +// the vacuity guard for that glob: a rename or a move that makes the glob +// return less than it did turns into a failure instead of a silently smaller +// scan. +const minGuardedPolicyFiles = 5 + +// gatedIAMActions are the three actions that can raise a role's privileges +// and so must only ever be granted through IAMRoleMutationRequiresBoundary +// (conditioned on the target role carrying cudly-deploy-boundary), never +// through an unconditioned Allow elsewhere. +var gatedIAMActions = []string{"iam:CreateRole", "iam:PutRolePolicy", "iam:AttachRolePolicy"} + +// boundaryGatedRoleMutationActions is the exact Action set of the +// IAMRoleMutationRequiresBoundary statement in policy_iam.tf: the three +// gatedIAMActions plus iam:PutRolePermissionsBoundary, which is what attaches +// the boundary to the roles that predate #1705. The set, not the Sid, is how +// TestDeployPolicyGatesRoleMutationOnBoundary locates that statement, so +// renaming the Sid does not silently skip the check. +var boundaryGatedRoleMutationActions = []string{ + "iam:AttachRolePolicy", + "iam:CreateRole", + "iam:PutRolePermissionsBoundary", + "iam:PutRolePolicy", +} + +// boundaryGatedRoleMutationResource is the Resource that statement applies to. +const boundaryGatedRoleMutationResource = "arn:aws:iam::*:role/cudly-*" + +// boundaryConditionKey and boundaryConditionOperator are the condition the +// same statement must carry, verbatim. The operator matters as much as the +// key: StringEqualsIfExists, ForAllValues:StringEquals and +// ForAnyValue:StringEquals all evaluate to true when the key is absent, and an +// absent iam:PermissionsBoundary is exactly the boundary-less iam:CreateRole +// this statement exists to refuse. +const ( + boundaryConditionKey = "iam:PermissionsBoundary" + boundaryConditionOperator = "StringEquals" +) + +// boundaryPolicyReference is the Terraform expression the condition value must +// resolve to, i.e. the boundary declared in policy_boundary.tf rather than a +// literal ARN that could drift from it. +const boundaryPolicyReference = "aws_iam_policy.workload_boundary.arn" + +// attachDenyConditionKey and attachDenyConditionOperator are the condition on +// the IAMDenyAttachUnapprovedManagedPolicy statement in policy_iam.tf. +// ArnNotEquals is what makes it an allowlist ("deny everything except these"); +// ArnEquals inverts it into a four-entry blocklist that leaves +// AdministratorAccess attachable, which is the #1705 escalation verbatim. +const ( + attachDenyConditionKey = "iam:PolicyARN" + attachDenyConditionOperator = "ArnNotEquals" +) + +// attachDenyAction is the single action that Deny covers. +const attachDenyAction = "iam:AttachRolePolicy" + +// iamServiceExemption is the one AWS service prefix deliberately absent from +// WorkloadServiceCeiling in policy_boundary.tf. iam:PassRole is the only IAM +// action any workload role in terraform/modules uses (the fargate +// EventBridge invoker roles passing the task/task-execution roles to +// ecs:RunTask), and it is covered by the boundary's dedicated, narrowly +// scoped PassRoleCeiling statement instead of the per-service ceiling. See +// the "iam: is absent on purpose" comment in policy_boundary.tf. +const iamServiceExemption = "iam" + +// actionAssignmentPattern matches an `Action = [...]`, `Action = "..."`, +// `actions = [...]` or `actions = "..."` HCL assignment and captures the +// value (list or single string) that follows. Anchoring on this assignment, +// rather than grepping the whole file for `"svc:Thing"` strings, is what +// keeps IAM condition keys like ses:FromAddress, sts:ExternalId, +// iam:PassedToService and ecs:cluster out of the derived action set: they +// appear inside Condition blocks, never inside an Action/actions list, so +// this pattern never sees them. +var actionAssignmentPattern = regexp.MustCompile(`(?m)^[ \t]*(?:Action|actions)\s*=\s*(\[[^\]]*\]|"[^"]*")`) + +// actionStringPattern extracts individual "service:Action" literals from the +// value captured by actionAssignmentPattern. +var actionStringPattern = regexp.MustCompile(`"([A-Za-z0-9]+:[A-Za-z0-9_*]+)"`) + +// resourceAssignmentPattern matches a `Resource = [...]` or `Resource = "..."` +// assignment inside a single statement block and captures the value. +var resourceAssignmentPattern = regexp.MustCompile(`(?m)^[ \t]*Resource\s*=\s*(\[[^\]]*\]|"[^"]*")`) + +// quotedStringPattern extracts every double-quoted literal from a captured +// list or scalar value. +var quotedStringPattern = regexp.MustCompile(`"([^"]*)"`) + +// effectPattern captures the Effect of a single statement block. +var effectPattern = regexp.MustCompile(`(?m)^[ \t]*Effect\s*=\s*"([^"]*)"`) + +// sidPattern captures the Sid of a single statement block. +var sidPattern = regexp.MustCompile(`(?m)^[ \t]*Sid\s*=\s*"([^"]*)"`) + +// conditionAssignmentPattern matches the `Condition = {` that opens a +// statement's condition block. +var conditionAssignmentPattern = regexp.MustCompile(`Condition\s*=\s*\{`) + +// conditionOperatorPattern matches one `Operator = {` line inside a Condition +// block. The optional quotes are what let it see the qualified forms +// ("ForAllValues:StringEquals"), which HCL requires quoting because of the +// colon and which must therefore not slip past a bare-identifier pattern. +var conditionOperatorPattern = regexp.MustCompile(`(?m)^[ \t]*"?([A-Za-z0-9]+(?::[A-Za-z0-9]+)?)"?[ \t]*=[ \t]*\{`) + +// extractActionListActions returns every "service:Action" literal found +// strictly inside Action/actions assignments in content, per +// actionAssignmentPattern above. +func extractActionListActions(content string) []string { + var actions []string + for _, m := range actionAssignmentPattern.FindAllStringSubmatch(content, -1) { + for _, am := range actionStringPattern.FindAllStringSubmatch(m[1], -1) { + actions = append(actions, am[1]) + } + } + return actions +} + +// findAwsModuleFiles returns every *.tf file under modulesDir whose +// directory path contains an "aws" path segment, i.e. the AWS-specific +// Terraform modules and not their Azure/GCP siblings. +func findAwsModuleFiles(t *testing.T) []string { + t.Helper() + + var files []string + err := filepath.WalkDir(modulesDir, func(path string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if d.IsDir() || filepath.Ext(path) != ".tf" { + return nil + } + for _, part := range strings.Split(filepath.ToSlash(filepath.Dir(path)), "/") { + if part == "aws" { + files = append(files, path) + break + } + } + return nil + }) + if err != nil { + t.Fatalf("walking %s for aws module .tf files: %v", modulesDir, err) + } + return files +} + +// readPolicySource reads path and strips its whole-line comments, so every +// assertion in this file is made against what Terraform will actually render +// and never against prose that merely mentions an action, an ARN or a service. +// That distinction is live, not theoretical: policy_iam.tf's own comments name +// `arn:aws:iam::aws:policy/service-role/*` (explaining why a pattern is NOT +// used) and policy_data.tf's name iam:AttachRolePolicy (explaining where it +// moved to), so a substring check against the raw file text would happily +// confirm the presence of a grant that had been deleted. +func readPolicySource(t *testing.T, path string) string { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading %s: %v", path, err) + } + return stripCommentLines(string(data)) +} + +// guardedPolicyFiles returns the cudly-deploy-* policy documents in this +// directory that must never unconditionally grant gatedIAMActions. +// policy_iam.tf is excluded because it is the one place those actions ARE +// meant to be Allow-granted (conditioned on the permissions boundary). +// +// This globs rather than listing names so a policy document added later is +// guarded from the moment it lands: a hardcoded slice would have let a new +// policy_*.tf granting an unconditioned iam:CreateRole reopen #1705 without a +// single test noticing. minGuardedPolicyFiles keeps that glob from silently +// degenerating into a smaller (or empty) scan. +func guardedPolicyFiles(t *testing.T) []string { + t.Helper() + + matches, err := filepath.Glob("policy_*.tf") + if err != nil { + t.Fatalf("globbing policy_*.tf: %v", err) + } + + var files []string + for _, m := range matches { + if filepath.Base(m) == iamFile { + continue + } + files = append(files, m) + } + sort.Strings(files) + + if len(files) < minGuardedPolicyFiles { + t.Fatalf("globbing policy_*.tf found only %d guarded policy documents (%v), fewer than the %d that existed when this guard was written; a policy document was renamed or moved out of this directory and is no longer being scanned for unconditioned grants of %v", len(files), files, minGuardedPolicyFiles, gatedIAMActions) + } + return files +} + +// balancedBraceEnd returns the index just past the "}" in content that +// matches the "{" at content[openIdx], by counting brace depth. It does not +// need to be string-literal-aware: every "{" these files contain, including +// Terraform interpolation braces like "${var.x}", has a matching "}", so +// plain counting resolves the same block boundaries a real HCL parser would. +func balancedBraceEnd(content string, openIdx int) int { + depth := 0 + for i := openIdx; i < len(content); i++ { + switch content[i] { + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + return i + 1 + } + } + } + return len(content) +} + +// stripCommentLines drops every line whose trimmed content starts with "#". +// Terraform comments in this repo are always whole-line, never trailing, so +// this is enough to keep prose that merely mentions an action name (e.g. the +// "iam:AttachRolePolicy ... USED TO BE in this list" comment in +// policy_data.tf) from being mistaken for a live grant. +func stripCommentLines(content string) string { + lines := strings.Split(content, "\n") + kept := make([]string, 0, len(lines)) + for _, line := range lines { + if strings.HasPrefix(strings.TrimSpace(line), "#") { + continue + } + kept = append(kept, line) + } + return strings.Join(kept, "\n") +} + +// statementArrayPattern matches the `Statement = [` that opens an IAM policy +// document's statement array. +var statementArrayPattern = regexp.MustCompile(`Statement\s*=\s*\[`) + +// extractStatementBlocks returns the text of each top-level `{...}` object +// inside every `Statement = [ ... ]` array in content, using balancedBraceEnd +// so each returned block is exactly one IAM policy statement (Sid/Effect/ +// Action/Resource/Condition), regardless of how much nested punctuation it +// contains. +// +// Every array, not just the first: a file may declare more than one +// aws_iam_policy, and stopping at the first array would make a second policy +// resource appended to a guarded file completely invisible to +// TestBoundaryGatedActionsAreNotUnconditionallyGranted. +func extractStatementBlocks(content string) []string { + var stmts []string + for _, loc := range statementArrayPattern.FindAllStringIndex(content, -1) { + i := loc[1] + for i < len(content) { + switch content[i] { + case ']': + i = len(content) // end of this array; the outer loop supplies the next one + case '{': + end := balancedBraceEnd(content, i) + stmts = append(stmts, content[i:end]) + i = end + default: + i++ + } + } + } + return stmts +} + +// effectIsAllowPattern matches an Effect = "Allow" assignment within a +// single statement block. +var effectIsAllowPattern = regexp.MustCompile(`Effect\s*=\s*"Allow"`) + +// statementSid returns a statement block's Sid, or "" if it has none. Used +// only in failure messages: no assertion keys off a Sid, because a Sid is free +// text that can be renamed without changing what the statement does. +func statementSid(stmt string) string { + if m := sidPattern.FindStringSubmatch(stmt); m != nil { + return m[1] + } + return "" +} + +// statementEffect returns a statement block's Effect ("Allow" / "Deny"), or "" +// if it has none. +func statementEffect(stmt string) string { + if m := effectPattern.FindStringSubmatch(stmt); m != nil { + return m[1] + } + return "" +} + +// statementActions returns a statement block's Action list, sorted. +func statementActions(stmt string) []string { + actions := extractActionListActions(stmt) + sort.Strings(actions) + return actions +} + +// statementResources returns a statement block's Resource list, sorted. A +// scalar `Resource = "*"` comes back as a one-element slice. +func statementResources(stmt string) []string { + m := resourceAssignmentPattern.FindStringSubmatch(stmt) + if m == nil { + return nil + } + var resources []string + for _, q := range quotedStringPattern.FindAllStringSubmatch(m[1], -1) { + resources = append(resources, q[1]) + } + sort.Strings(resources) + return resources +} + +// conditionOperator is one `Operator = { ... }` entry inside a statement's +// Condition block: its name verbatim (including any ForAllValues:/ForAnyValue: +// qualifier) and the body it introduces. +type conditionOperator struct { + name string + body string +} + +// statementConditionOperators returns every condition operator in a statement +// block. Returning all of them, rather than looking one up by name, is what +// lets the callers assert the condition is EXACTLY the operator they expect: +// a check that merely found "StringEquals" somewhere would stay green if a +// second, permissive operator were added beside it. +func statementConditionOperators(stmt string) []conditionOperator { + loc := conditionAssignmentPattern.FindStringIndex(stmt) + if loc == nil { + return nil + } + open := loc[1] - 1 // the "{" the match ends on + body := stmt[open:balancedBraceEnd(stmt, open)] + + var ops []conditionOperator + for _, m := range conditionOperatorPattern.FindAllStringSubmatchIndex(body, -1) { + opOpen := m[1] - 1 // the "{" this operator's match ends on + ops = append(ops, conditionOperator{ + name: body[m[2]:m[3]], + body: body[opOpen:balancedBraceEnd(body, opOpen)], + }) + } + return ops +} + +// singleConditionOperator returns the statement's one and only condition +// operator, failing the test if it has none, more than one, or one whose name +// is not want. want is compared verbatim, so StringEqualsIfExists, +// ForAllValues:StringEquals and ArnEquals are all rejected against a want of +// StringEquals / ArnNotEquals respectively. +func singleConditionOperator(t *testing.T, stmt, want, why string) conditionOperator { + t.Helper() + + ops := statementConditionOperators(stmt) + if len(ops) != 1 { + names := make([]string, 0, len(ops)) + for _, op := range ops { + names = append(names, op.name) + } + t.Fatalf("%s: statement %q has %d condition operators (%v), want exactly one (%s). %s", iamFile, statementSid(stmt), len(ops), names, want, why) + } + if ops[0].name != want { + t.Fatalf("%s: statement %q is conditioned on %q, want exactly %q. %s", iamFile, statementSid(stmt), ops[0].name, want, why) + } + return ops[0] +} + +// findStatements returns every statement block in file (comments stripped) +// that match keeps. +func findStatements(t *testing.T, file string, keep func(stmt string) bool) []string { + t.Helper() + + all := extractStatementBlocks(readPolicySource(t, file)) + if len(all) == 0 { + t.Fatalf("%s: found zero Statement objects; the statement splitter in this test is broken and would otherwise pass vacuously", file) + } + + var matched []string + for _, stmt := range all { + if keep(stmt) { + matched = append(matched, stmt) + } + } + return matched +} + +// equalStringSets reports whether got and want hold the same elements, +// ignoring order and duplicates. +func equalStringSets(got, want []string) bool { + set := func(in []string) map[string]bool { + out := map[string]bool{} + for _, v := range in { + out[v] = true + } + return out + } + g, w := set(got), set(want) + if len(g) != len(w) { + return false + } + for k := range g { + if !w[k] { + return false + } + } + return true +} + +// sortedKeys returns the keys of set, sorted, for stable failure messages. +func sortedKeys(set map[string]bool) []string { + out := make([]string, 0, len(set)) + for k := range set { + out = append(out, k) + } + sort.Strings(out) + return out +} + +// TestBoundaryCoversWorkloadServices re-derives the set of AWS service +// prefixes granted to workload roles by the Terraform modules under +// terraform/modules/**/aws and asserts policy_boundary.tf's +// WorkloadServiceCeiling covers every one of them (via a ":*" wildcard +// or an explicit ":" grant for that exact action), except iam +// (see iamServiceExemption). A service missing here deploys cleanly and then +// 403s the first time the workload role calls it, because a permissions +// boundary caps effective permissions to the intersection of the role's +// identity policy and this ceiling (#1705). +func TestBoundaryCoversWorkloadServices(t *testing.T) { + files := findAwsModuleFiles(t) + if len(files) == 0 { + t.Fatalf("found zero *.tf files under an \"aws\" path segment in %s; the module tree may have moved and this test can no longer see what it is supposed to guard", modulesDir) + } + + serviceActions := map[string]map[string]bool{} + for _, f := range files { + for _, action := range extractActionListActions(readPolicySource(t, f)) { + svc, _, ok := strings.Cut(action, ":") + if !ok { + continue + } + if serviceActions[svc] == nil { + serviceActions[svc] = map[string]bool{} + } + serviceActions[svc][action] = true + } + } + if len(serviceActions) == 0 { + t.Fatalf("parsed zero IAM actions out of %d aws module files; the Action/actions extraction in this test is broken and would otherwise pass vacuously", len(files)) + } + + boundaryContent := readPolicySource(t, boundaryFile) + + // iam is exempt from the loop below (see iamServiceExemption), but + // granting "iam:*" in the ceiling would silently defeat the entire + // boundary: a boundaried role's effective IAM permissions would then be + // whatever its identity policy allows, with no cap at all. Assert the + // exemption stays narrow (PassRoleCeiling only), not wide open. + if strings.Contains(boundaryContent, `"iam:*"`) { + t.Errorf("%s grants \"iam:*\" in the WorkloadServiceCeiling statement; that defeats the boundary entirely, because iam is otherwise deliberately omitted from the ceiling so no workload role can create a role, attach a policy, or touch a permissions boundary, no matter what its identity policy says. iam:PassRole is meant to be covered only by the scoped PassRoleCeiling statement", boundaryFile) + } + + services := make([]string, 0, len(serviceActions)) + for svc := range serviceActions { + services = append(services, svc) + } + sort.Strings(services) + + for _, svc := range services { + if svc == iamServiceExemption { + continue + } + + wildcard := `"` + svc + `:*"` + if strings.Contains(boundaryContent, wildcard) { + continue + } + + var missing []string + for action := range serviceActions[svc] { + literal := `"` + action + `"` + if !strings.Contains(boundaryContent, literal) { + missing = append(missing, action) + } + } + if len(missing) > 0 { + sort.Strings(missing) + t.Errorf("policy_boundary.tf's WorkloadServiceCeiling does not cover service %q: found neither %s nor an explicit grant for %v (used by terraform/modules). Add the service to WorkloadServiceCeiling in %s in the same change, or the workload role that calls one of these actions will deploy cleanly and then 403 at runtime, because a permissions boundary caps effective permissions to the intersection of the role's identity policy and this ceiling", svc, wildcard, missing, boundaryFile) + } + } +} + +// policyARNAssignmentPattern matches a whole `policy_arn = "..."` line, i.e. +// an aws_iam_role_policy_attachment's policy_arn argument. Anchoring on the +// full line (not just the substring "policy_arn") is what keeps this from +// also matching `output "secret_read_policy_arn" { ... }` blocks, whose +// output name merely ends in "policy_arn". +var policyARNAssignmentPattern = regexp.MustCompile(`(?m)^[ \t]*policy_arn\s*=\s*"([^"]*)"[ \t]*$`) + +// moduleAttachedManagedPolicyARNs collects every literal managed-policy ARN +// assigned to policy_arn in an aws_iam_role_policy_attachment under +// terraform/modules/**/aws. It is the single source of truth for both halves +// of the managed-policy allowlist guard: the drift check +// (TestDeployPolicyAllowsAttachedManagedPolicies, "everything the modules +// attach is allowed") and the tamper check +// (TestDeployPolicyDeniesUnapprovedManagedPolicyAttachment, "nothing beyond +// what the modules attach is allowed"). Deriving both from the same function +// is what makes the allowlist an equality rather than two independent +// inequalities that could be satisfied by a widened list. +func moduleAttachedManagedPolicyARNs(t *testing.T) map[string]bool { + t.Helper() + + files := findAwsModuleFiles(t) + if len(files) == 0 { + t.Fatalf("found zero *.tf files under an \"aws\" path segment in %s", modulesDir) + } + + arns := map[string]bool{} + for _, f := range files { + for _, m := range policyARNAssignmentPattern.FindAllStringSubmatch(readPolicySource(t, f), -1) { + v := m[1] + // Skip anything that is a Terraform expression rather than a + // literal ARN: policy_arn = aws_iam_policy.x.arn (no quotes) + // never matches this pattern at all, but a quoted interpolation + // like policy_arn = "${aws_iam_policy.x.arn}" would, and is not + // a fixed ARN this test can check against a static allowlist. + if strings.Contains(v, "${") || strings.Contains(v, "aws_iam_policy.") { + continue + } + arns[v] = true + } + } + if len(arns) == 0 { + t.Fatalf("found zero literal policy_arn assignments under aws module files; the attachment scan in this test is broken and would otherwise pass vacuously") + } + return arns +} + +// TestDeployPolicyAllowsAttachedManagedPolicies collects every literal +// managed-policy ARN assigned to policy_arn in an aws_iam_role_policy_ +// attachment under terraform/modules/**/aws and asserts each one appears in +// policy_iam.tf's IAMDenyAttachUnapprovedManagedPolicy ArnNotEquals +// allowlist. A module that attaches a policy missing from that allowlist +// fails `terraform apply` with AccessDenied, because that statement denies +// iam:AttachRolePolicy for any policy ARN not on the list. +func TestDeployPolicyAllowsAttachedManagedPolicies(t *testing.T) { + arns := moduleAttachedManagedPolicyARNs(t) + iamContent := readPolicySource(t, iamFile) + + for _, arn := range sortedKeys(arns) { + if !strings.Contains(iamContent, `"`+arn+`"`) { + t.Errorf("terraform/modules attaches managed policy %q, but it is not in IAMDenyAttachUnapprovedManagedPolicy's ArnNotEquals allowlist in %s; add it there in the same change, or terraform apply fails with AccessDenied the first time this attachment runs", arn, iamFile) + } + } +} + +// TestDeployPolicyGatesRoleMutationOnBoundary pins the internal shape of the +// one statement in policy_iam.tf that re-grants the privilege-raising IAM +// actions, because every other test in this file stays green while that +// statement is quietly defanged. +// +// It asserts, structurally rather than by substring: +// - exactly one statement grants boundaryGatedRoleMutationActions, and it is +// an Allow scoped to arn:aws:iam::*:role/cudly-*; +// - its condition is EXACTLY one operator, spelled StringEquals. Not +// StringEqualsIfExists, not ForAllValues:StringEquals, not +// ForAnyValue:StringEquals: all three evaluate to true when the key is +// absent from the request, and a CreateRole that simply omits +// PermissionsBoundary leaves the key absent. Swapping the one word +// re-admits the boundary-less role creation that is the whole of #1705, +// and nothing else in this file would notice; +// - the condition key is iam:PermissionsBoundary and its value references +// aws_iam_policy.workload_boundary.arn, so the boundary being demanded is +// the document in policy_boundary.tf and not a literal ARN that can drift +// from it or a different policy entirely. +func TestDeployPolicyGatesRoleMutationOnBoundary(t *testing.T) { + want := append([]string(nil), boundaryGatedRoleMutationActions...) + sort.Strings(want) + + matches := findStatements(t, iamFile, func(stmt string) bool { + return equalStringSets(statementActions(stmt), want) + }) + if len(matches) != 1 { + t.Fatalf("%s: found %d statements whose Action set is exactly %v, want exactly 1. That statement is the only thing that forces every role the deploy role creates to carry cudly-deploy-boundary; if it was split, merged or had an action added or removed, re-derive this test's expectation deliberately rather than widening it, because losing it reopens the #1705 escalation", iamFile, len(matches), want) + } + stmt := matches[0] + + if effect := statementEffect(stmt); effect != "Allow" { + t.Fatalf("%s: statement %q has Effect %q, want \"Allow\"; it is the conditioned re-grant of %v and a Deny here takes the whole deploy pipeline down instead of bounding it", iamFile, statementSid(stmt), effect, want) + } + + if got := statementResources(stmt); !equalStringSets(got, []string{boundaryGatedRoleMutationResource}) { + t.Errorf("%s: statement %q applies to Resource %v, want exactly [%q]; widening it grants boundary-gated role mutation outside the cudly-* namespace, narrowing it silently drops roles the deploy path creates", iamFile, statementSid(stmt), got, boundaryGatedRoleMutationResource) + } + + op := singleConditionOperator(t, stmt, boundaryConditionOperator, + "StringEqualsIfExists, ForAllValues:StringEquals and ForAnyValue:StringEquals all evaluate to TRUE when the key is absent from the request, and an iam:CreateRole that simply omits PermissionsBoundary leaves it absent: the Allow would then authorize creating a role with no ceiling at all, which is exactly the #1705 escalation this statement exists to close") + + keys := quotedStringPattern.FindAllStringSubmatch(op.body, -1) + if len(keys) != 1 || keys[0][1] != boundaryConditionKey { + got := make([]string, 0, len(keys)) + for _, k := range keys { + got = append(got, k[1]) + } + t.Fatalf("%s: statement %q conditions on keys %v, want exactly [%q]; any other key leaves the boundary unenforced", iamFile, statementSid(stmt), got, boundaryConditionKey) + } + + // The value is a bare Terraform reference, not a quoted literal, so it is + // whatever follows the "=" on the condition key's line. + valuePattern := regexp.MustCompile(`"` + regexp.QuoteMeta(boundaryConditionKey) + `"\s*=\s*([^\n]+)`) + m := valuePattern.FindStringSubmatch(op.body) + if m == nil { + t.Fatalf("%s: statement %q has no value assigned to condition key %q", iamFile, statementSid(stmt), boundaryConditionKey) + } + if value := strings.TrimSpace(m[1]); value != boundaryPolicyReference { + t.Errorf("%s: statement %q requires iam:PermissionsBoundary == %s, want exactly %s (the boundary declared in %s). A literal ARN or a reference to a different policy can drift from the document that actually caps workload roles, leaving roles created with a boundary that grants more than the ceiling does", iamFile, statementSid(stmt), value, boundaryPolicyReference, boundaryFile) + } +} + +// TestDeployPolicyDeniesUnapprovedManagedPolicyAttachment pins the internal +// shape of the Deny that closes the escalation named in #1705: attaching an +// arbitrary managed policy (AdministratorAccess above all) to a cudly-* role. +// +// Three single-token edits reopen it while every drift-based assertion in this +// file stays green, so each gets an explicit assertion here: +// - ArnNotEquals -> ArnEquals inverts an allowlist into a four-entry +// blocklist, and everything else in the account becomes attachable; +// - adding a pattern such as "arn:aws:iam::aws:policy/*" to the list exempts +// every AWS managed policy, because the Arn* operators wildcard-match; +// - Effect Deny -> Allow removes the statement's teeth entirely (the +// statement then simply grants what IAMRoleMutationRequiresBoundary +// already grants). +// +// It also pins the list to equal the module-derived attachment set, so the +// allowlist can neither shrink below what the modules need (an apply-time +// AccessDenied) nor grow past it (a silently wider escalation surface). +func TestDeployPolicyDeniesUnapprovedManagedPolicyAttachment(t *testing.T) { + matches := findStatements(t, iamFile, func(stmt string) bool { + return statementEffect(stmt) == "Deny" && equalStringSets(statementActions(stmt), []string{attachDenyAction}) + }) + if len(matches) != 1 { + t.Fatalf("%s: found %d Deny statements on exactly [%q], want exactly 1. That Deny is the entire fix for the escalation named in #1705 (any managed policy in the account, AdministratorAccess included, attachable to any cudly-* role); if its Effect was flipped to Allow, or the statement split, this is what that looks like", iamFile, len(matches), attachDenyAction) + } + stmt := matches[0] + + op := singleConditionOperator(t, stmt, attachDenyConditionOperator, + "ArnNotEquals is what makes the list an ALLOWLIST (deny every policy ARN except these). ArnEquals inverts it into a four-entry blocklist, leaving arn:aws:iam::aws:policy/AdministratorAccess attachable to any cudly-* role, which is the #1705 escalation verbatim") + + valuePattern := regexp.MustCompile(`"` + regexp.QuoteMeta(attachDenyConditionKey) + `"\s*=\s*(\[[^\]]*\]|"[^"]*")`) + m := valuePattern.FindStringSubmatch(op.body) + if m == nil { + t.Fatalf("%s: statement %q has no %q list under %s; without it the Deny applies to every iam:AttachRolePolicy call and takes the deploy pipeline down", iamFile, statementSid(stmt), attachDenyConditionKey, attachDenyConditionOperator) + } + + allowedMatches := quotedStringPattern.FindAllStringSubmatch(m[1], -1) + allowed := make([]string, 0, len(allowedMatches)) + for _, q := range allowedMatches { + allowed = append(allowed, q[1]) + } + if len(allowed) == 0 { + t.Fatalf("%s: statement %q has an empty %q list", iamFile, statementSid(stmt), attachDenyConditionKey) + } + + for _, arn := range allowed { + if strings.Contains(arn, "*") { + t.Errorf("%s: statement %q exempts %q from the Deny, and it contains a wildcard. The Arn* condition operators match wildcards in the policy value, so a pattern here is not an exact ARN: \"arn:aws:iam::aws:policy/*\" exempts AdministratorAccess, and a pattern scoped to this account's own namespace exempts the `*` on `*` policy the deploy role can still mint with iam:CreatePolicy. Every entry must be a literal ARN", iamFile, statementSid(stmt), arn) + } + } + + want := sortedKeys(moduleAttachedManagedPolicyARNs(t)) + if !equalStringSets(allowed, want) { + sort.Strings(allowed) + t.Errorf("%s: statement %q exempts %v from the Deny, but terraform/modules attaches exactly %v. These two must be equal: an entry the modules do not attach widens what a compromised deploy role may attach to a cudly-* role for no operational reason, and a missing entry fails terraform apply with AccessDenied", iamFile, statementSid(stmt), allowed, want) + } +} + +// awsIAMPolicyResourcePattern matches an `resource "aws_iam_policy" "" {` +// block header and captures the resource's local name. +var awsIAMPolicyResourcePattern = regexp.MustCompile(`resource\s+"aws_iam_policy"\s+"([A-Za-z0-9_]+)"\s*\{`) + +// nameAssignmentPattern matches a top-level `name = "..."` argument. +var nameAssignmentPattern = regexp.MustCompile(`(?m)^[ \t]*name\s*=\s*"([^"]*)"`) + +// permissionsBoundaryLocalPattern matches the environment root's +// `permissions_boundary_arn = "..."` local, whose value is the hardcoded ARN +// every module receives as var.permissions_boundary_arn. +var permissionsBoundaryLocalPattern = regexp.MustCompile(`(?m)^[ \t]*permissions_boundary_arn\s*=\s*"([^"]*)"`) + +// TestBoundaryPolicyNameStaysInProtectedNamespace asserts the three-way name +// coupling that policy_boundary.tf's header calls load-bearing, and that no +// other test in this file would notice breaking. +// +// The boundary document is protected from its own deploy role only by +// IAMDenyModifyDeployRoleAndPolicies in policy_data.tf, which denies +// iam:CreatePolicyVersion, iam:SetDefaultPolicyVersion, iam:DeletePolicy and +// iam:DeletePolicyVersion (the complete mutation set for a managed policy, +// none of which supports any condition key) against +// arn:aws:iam::*:policy/cudly-deploy-*. Renaming the boundary out of that +// prefix removes the protection silently and lets a compromised deploy role +// rewrite its own ceiling. Meanwhile the ARN the modules are handed is +// hardcoded in the environment root, so a rename that did not update it would +// point every role at a boundary that does not exist. +func TestBoundaryPolicyNameStaysInProtectedNamespace(t *testing.T) { + boundaryContent := readPolicySource(t, boundaryFile) + + loc := awsIAMPolicyResourcePattern.FindStringSubmatchIndex(boundaryContent) + if loc == nil { + t.Fatalf("%s: found no resource \"aws_iam_policy\" block; this test can no longer see what it is supposed to guard", boundaryFile) + } + if resourceName := boundaryContent[loc[2]:loc[3]]; resourceName != "workload_boundary" { + t.Fatalf("%s: first aws_iam_policy resource is %q, want \"workload_boundary\"; %s references aws_iam_policy.workload_boundary.arn in its permissions-boundary condition and would no longer resolve", boundaryFile, resourceName, iamFile) + } + + block := boundaryContent[loc[1]-1 : balancedBraceEnd(boundaryContent, loc[1]-1)] + nameMatch := nameAssignmentPattern.FindStringSubmatch(block) + if nameMatch == nil { + t.Fatalf("%s: aws_iam_policy.workload_boundary has no name argument", boundaryFile) + } + boundaryName := nameMatch[1] + + if !strings.HasPrefix(boundaryName, deployPolicyNamePrefix) { + t.Errorf("%s: aws_iam_policy.workload_boundary is named %q, which is outside the %q namespace. IAMDenyModifyDeployRoleAndPolicies in %s protects this document by resource ARN prefix only, because none of the four managed-policy mutation actions supports a condition key; a name outside the prefix silently lets a compromised cudly-terraform-deploy rewrite its own permissions boundary", boundaryFile, boundaryName, deployPolicyNamePrefix, dataFile) + } + + // The Deny that provides that protection must actually cover the prefix. + wantDenyResource := "arn:aws:iam::*:policy/" + deployPolicyNamePrefix + "*" + denies := findStatements(t, dataFile, func(stmt string) bool { + return statementSid(stmt) == "IAMDenyModifyDeployRoleAndPolicies" + }) + if len(denies) != 1 { + t.Fatalf("%s: found %d statements with Sid IAMDenyModifyDeployRoleAndPolicies, want exactly 1; it is the only thing standing between the deploy role and the policy documents that bound it", dataFile, len(denies)) + } + if effect := statementEffect(denies[0]); effect != "Deny" { + t.Errorf("%s: IAMDenyModifyDeployRoleAndPolicies has Effect %q, want \"Deny\"", dataFile, effect) + } + resources := statementResources(denies[0]) + found := false + for _, r := range resources { + if r == wantDenyResource { + found = true + break + } + } + if !found { + t.Errorf("%s: IAMDenyModifyDeployRoleAndPolicies applies to %v, which does not include %q. Without that entry the deploy role may call iam:CreatePolicyVersion + iam:SetDefaultPolicyVersion on %q and rewrite the ceiling that bounds every role it creates", dataFile, resources, wantDenyResource, boundaryName) + } + + // The ARN handed to every module is hardcoded in the environment root, so + // it has to name the same policy. + envMainContent := readPolicySource(t, envMainFile) + arnMatch := permissionsBoundaryLocalPattern.FindStringSubmatch(envMainContent) + if arnMatch == nil { + t.Fatalf("%s: found no permissions_boundary_arn local; every module in this environment is passed var.permissions_boundary_arn from it, and this test can no longer verify it names %q", envMainFile, boundaryName) + } + if wantSuffix := ":policy/" + boundaryName; !strings.HasSuffix(arnMatch[1], wantSuffix) { + t.Errorf("%s: local.permissions_boundary_arn is %q, which does not end in %q. Every module role is created with that ARN as its boundary, and IAMRoleMutationRequiresBoundary in %s only permits the boundary declared in %s (%q); a mismatch fails every apply with AccessDenied, or, if the named policy happens to exist and is weaker, boundaries every workload role with the wrong ceiling", envMainFile, arnMatch[1], wantSuffix, iamFile, boundaryFile, boundaryName) + } +} + +// iamRoleResourcePattern matches an `resource "aws_iam_role" "" {` +// block header and captures the role's local name. +var iamRoleResourcePattern = regexp.MustCompile(`resource\s+"aws_iam_role"\s+"([A-Za-z0-9_]+)"\s*\{`) + +// permissionsBoundaryArgumentPattern matches the exact +// `permissions_boundary = var.permissions_boundary_arn` argument a module role +// must carry. Matching the whole assignment rather than the bare attribute +// name is the point: `permissions_boundary = null` and +// `permissions_boundary = var.something_else` both contain the attribute and +// both leave the role unbounded. +var permissionsBoundaryArgumentPattern = regexp.MustCompile(`(?m)^[ \t]*permissions_boundary[ \t]*=[ \t]*var\.permissions_boundary_arn[ \t]*$`) + +// TestEveryModuleRoleHasPermissionsBoundary asserts every aws_iam_role +// resource under terraform/modules/**/aws sets permissions_boundary to +// var.permissions_boundary_arn exactly. A role created without one is either +// rejected at apply time by IAMRoleMutationRequiresBoundary in policy_iam.tf +// (which only allows iam:CreateRole when the request sets this exact +// boundary), or, if that statement is ever weakened, runs with no ceiling at +// all. `permissions_boundary = null` is the same thing with an attribute +// present, which is why the assertion is on the value and not on the name. +func TestEveryModuleRoleHasPermissionsBoundary(t *testing.T) { + files := findAwsModuleFiles(t) + if len(files) == 0 { + t.Fatalf("found zero *.tf files under an \"aws\" path segment in %s", modulesDir) + } + + roleCount := 0 + for _, f := range files { + data, err := os.ReadFile(f) + if err != nil { + t.Fatalf("reading %s: %v", f, err) + } + content := string(data) + + for _, loc := range iamRoleResourcePattern.FindAllStringSubmatchIndex(content, -1) { + roleCount++ + name := content[loc[2]:loc[3]] + openBrace := loc[1] - 1 // the "{" the match ends on + end := balancedBraceEnd(content, openBrace) + block := stripCommentLines(content[openBrace:end]) + line := 1 + strings.Count(content[:loc[0]], "\n") + + if permissionsBoundaryArgumentPattern.MatchString(block) { + continue + } + detail := "has no permissions_boundary attribute" + if strings.Contains(block, "permissions_boundary") { + detail = "sets permissions_boundary to something other than var.permissions_boundary_arn (null, a different variable and a literal all leave the role uncapped)" + } + t.Errorf("%s:%d: resource \"aws_iam_role\" %q %s; every role the deploy role creates must carry cudly-deploy-boundary (policy_boundary.tf), which this environment supplies as var.permissions_boundary_arn, or IAMRoleMutationRequiresBoundary in policy_iam.tf denies its creation at apply time", f, line, name, detail) + } + } + if roleCount == 0 { + t.Fatalf("found zero aws_iam_role resources under aws module files; the resource scan in this test is broken and would otherwise pass vacuously") + } +} + +// TestNoIAMRolesOutsideGuardedModules asserts the environment root declares no +// aws_iam_role of its own. TestEveryModuleRoleHasPermissionsBoundary only +// walks terraform/modules/**/aws, so a role declared directly in +// terraform/environments/aws/*.tf would be created by the deploy role while +// being invisible to the boundary guard. It is a guard on the guard's reach, +// not on the roles themselves. +// +// terraform/environments/aws/ci-cd-permissions (this directory) is a separate, +// manually applied bootstrap root and is deliberately out of scope: the roles +// it declares, cudly-terraform-deploy among them, are the ones the boundary +// exists to bound and cannot be bounded by it. +func TestNoIAMRolesOutsideGuardedModules(t *testing.T) { + files, err := filepath.Glob(filepath.Join(envRootDir, "*.tf")) + if err != nil { + t.Fatalf("globbing %s: %v", filepath.Join(envRootDir, "*.tf"), err) + } + if len(files) == 0 { + t.Fatalf("found zero *.tf files in %s; the environment root moved and this test would otherwise pass vacuously", envRootDir) + } + + for _, f := range files { + content := readPolicySource(t, f) + for _, loc := range iamRoleResourcePattern.FindAllStringSubmatchIndex(content, -1) { + name := content[loc[2]:loc[3]] + t.Errorf("%s declares resource \"aws_iam_role\" %q directly in the environment root. Every role on the deploy path must live in a module under %s, because that is the only tree TestEveryModuleRoleHasPermissionsBoundary walks: a role declared here is created by cudly-terraform-deploy with no test asserting it carries cudly-deploy-boundary. Move it into a module and pass local.permissions_boundary_arn down as var.permissions_boundary_arn", f, name, modulesDir) + } + } +} + +// TestBoundaryGatedActionsAreNotUnconditionallyGranted asserts that none of +// gatedIAMActions is granted by an Allow statement in guardedPolicyFiles. +// Those three actions can raise a role's privileges and must only be granted +// through IAMRoleMutationRequiresBoundary in policy_iam.tf, conditioned on +// the target role carrying cudly-deploy-boundary; an unconditioned Allow +// anywhere else authorizes the request on its own and makes that +// conditioned statement decorative (the exact #1705 regression). +// +// This deliberately only inspects Allow statements' Action lists, not the +// file as a whole: policy_data.tf legitimately names iam:AttachRolePolicy +// and iam:PutRolePolicy inside IAMDenyModifyDeployRoleAndPolicies (a Deny +// statement) and inside prose comments explaining why those two moved out of +// IAMRolesAndPolicies. A whole-file substring check would either false-flag +// that Deny statement and the comments, or (if narrowed to dodge those) fail +// to catch a real regression. Splitting into statement objects via +// extractStatementBlocks and checking each one's own Effect and Action list +// avoids both failure modes. +func TestBoundaryGatedActionsAreNotUnconditionallyGranted(t *testing.T) { + for _, f := range guardedPolicyFiles(t) { + t.Run(f, func(t *testing.T) { + stmts := extractStatementBlocks(readPolicySource(t, f)) + if len(stmts) == 0 { + t.Fatalf("%s: found zero Statement objects; the statement splitter in this test is broken and would otherwise pass vacuously", f) + } + + for _, stmt := range stmts { + if !effectIsAllowPattern.MatchString(stmt) { + continue + } + granted := map[string]bool{} + for _, action := range extractActionListActions(stmt) { + granted[action] = true + } + for _, gated := range gatedIAMActions { + if granted[gated] { + t.Errorf("%s: an Allow statement grants %s unconditionally. This action must only be granted by IAMRoleMutationRequiresBoundary in policy_iam.tf, gated on the target role carrying cudly-deploy-boundary; an unconditioned Allow here reopens the #1705 escalation and makes that conditioned statement decorative", f, gated) + } + } + } + }) + } +} + +// variableDefaultPattern matches the `default = "..."` argument of a +// Terraform variable block. +var variableDefaultPattern = regexp.MustCompile(`(?m)^[ \t]*default\s*=\s*"([^"]*)"`) + +// variableStringDefault returns the string default of the named variable +// declared in file. +func variableStringDefault(t *testing.T, file, variable string) string { + t.Helper() + + content := readPolicySource(t, file) + header := regexp.MustCompile(`variable\s+"` + regexp.QuoteMeta(variable) + `"\s*\{`) + loc := header.FindStringIndex(content) + if loc == nil { + t.Fatalf("%s: declares no variable %q; this test can no longer verify the boundary matches it", file, variable) + } + openBrace := loc[1] - 1 + block := content[openBrace:balancedBraceEnd(content, openBrace)] + + m := variableDefaultPattern.FindStringSubmatch(block) + if m == nil { + t.Fatalf("%s: variable %q has no string default; policy_boundary.tf's CrossAccountAssumeRoleCeiling hardcodes the default's value, so a variable without one leaves the boundary pinned to a prefix nothing produces", file, variable) + } + return m[1] +} + +// TestBoundaryMatchesCrossAccountRolePrefix asserts the coupling that +// policy_boundary.tf's CrossAccountAssumeRoleCeiling comment declares: its +// Resource, arn:aws:iam::*:role/CUDly*, mirrors the Resource of the +// cross_account_sts inline policy in modules/compute/aws/lambda/main.tf and +// modules/compute/aws/fargate/main.tf, which is +// arn:aws:iam::*:role/${var.cross_account_role_name_prefix}*. +// +// WHY THIS MATTERS MORE THAN A NORMAL DRIFT CHECK. A permissions boundary +// grants nothing; it caps. If the module default moves to some other prefix +// (or the two modules drift apart from each other) while this statement stays +// on CUDly*, the task/Lambda role's identity policy allows sts:AssumeRole on +// the new prefix but the boundary does not, so the intersection is empty. +// Nothing fails at `terraform apply`: the policies are written exactly as +// requested and the apply is green. The failure is a RUNTIME AccessDenied the +// first time CUDly tries to read a linked account, i.e. cross-account cost +// collection silently stops working in a deployed environment. That is the +// failure mode policy_boundary.tf's "THE ALLOW LIST IS A CEILING, NOT A GRANT" +// note calls out as far worse than an apply-time 403, and the only place it +// can be caught cheaply is here, in CI. +func TestBoundaryMatchesCrossAccountRolePrefix(t *testing.T) { + defaults := map[string]string{} + for _, f := range crossAccountModuleVariableFiles { + defaults[f] = variableStringDefault(t, f, crossAccountPrefixVariable) + } + + var prefix string + for _, f := range crossAccountModuleVariableFiles { + if prefix == "" { + prefix = defaults[f] + continue + } + if defaults[f] != prefix { + t.Fatalf("the two modules that grant cross-account sts:AssumeRole disagree on the default of %q: %v. policy_boundary.tf's CrossAccountAssumeRoleCeiling can only mirror one prefix, so whichever module lost the coin toss deploys cleanly and then 403s at RUNTIME on its first cross-account read, not at apply time. Align both defaults, then widen the boundary in the same change", crossAccountPrefixVariable, defaults) + } + } + if prefix == "" { + t.Fatalf("resolved an empty default for %q; an empty prefix widens the module grant to every role in every account and this test would compare against a boundary pattern of arn:aws:iam::*:role/*", crossAccountPrefixVariable) + } + + want := fmt.Sprintf("arn:aws:iam::*:role/%s*", prefix) + + stmts := findStatements(t, boundaryFile, func(stmt string) bool { + return statementSid(stmt) == "CrossAccountAssumeRoleCeiling" + }) + if len(stmts) != 1 { + t.Fatalf("%s: found %d statements with Sid CrossAccountAssumeRoleCeiling, want exactly 1; it is the boundary's only grant of sts:AssumeRole, and without it every cross-account read 403s at RUNTIME in a deployed environment while `terraform apply` stays green", boundaryFile, len(stmts)) + } + + if got := statementResources(stmts[0]); !equalStringSets(got, []string{want}) { + t.Errorf("%s: CrossAccountAssumeRoleCeiling caps sts:AssumeRole at %v, but %s defaults to %q in %v, so the modules grant %q. A permissions boundary caps by intersection: this mismatch is INVISIBLE at `terraform apply` (both documents are written exactly as configured, the apply is green) and surfaces as a RUNTIME AccessDenied the first time CUDly assumes a role in a linked account, i.e. cross-account cost collection silently stops working in the deployed environment. Change both in the same commit", boundaryFile, got, crossAccountPrefixVariable, prefix, crossAccountModuleVariableFiles, want) + } +} diff --git a/terraform/environments/aws/ci-cd-permissions/policy_iam.tf b/terraform/environments/aws/ci-cd-permissions/policy_iam.tf new file mode 100644 index 000000000..081389723 --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/policy_iam.tf @@ -0,0 +1,180 @@ +# The delegation half of the fix for #1705. policy_boundary.tf defines the +# ceiling; this policy is what forces the deploy role to apply that ceiling to +# every role it creates, and what stops it attaching AdministratorAccess. +# +# WHY A SEPARATE MANAGED POLICY. The four statements below cannot live in +# cudly-deploy-data: a managed policy is capped at 6144 characters excluding +# whitespace, cudly-deploy-data already renders to roughly 5 KB and +# cudly-deploy-compute sits close enough to the ceiling that an earlier fix had +# to be split into policy_compute_b.tf. Splitting is free here because a role's +# effective permissions are the union of its attached policies, and a Deny in +# any one of them beats an Allow in any other. Attaching a fifth policy leaves +# the role at five of the ten managed policies AWS permits. +# +# WHAT REMAINS OPEN, DELIBERATELY. The deploy role keeps iam:CreatePolicy and +# iam:CreatePolicyVersion on cudly-* policies. iam:CreatePolicy supports +# aws:RequestTag/${TagKey} and aws:TagKeys (it accepts an optional Tags +# parameter), and iam:CreatePolicyVersion supports no condition key at all +# (verified against the AWS Service Authorization Reference). Neither +# constrains the policy DOCUMENT, which is the point here, so a compromised +# deploy role can still mint a managed policy whose document is `*` on `*`. +# That policy is inert: attaching it to any role is denied by +# IAMDenyAttachUnapprovedManagedPolicy below, and its own document cannot be +# reached from a workload role because those are capped by the boundary. +# Denying policy creation instead would break every apply that manages the +# module-level managed policy in modules/secrets/aws. +resource "aws_iam_policy" "iam" { + name = "cudly-deploy-iam" + description = "CUDly Terraform deploy: IAM role mutation gated on the cudly-deploy-boundary permissions boundary" + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + # The four actions that can raise a role's privileges, re-granted only + # when the target role carries cudly-deploy-boundary. They were removed + # from the unconditioned IAMRolesAndPolicies statement in policy_data.tf + # in the same change; leaving them there would make this statement + # decorative, since the unconditioned Allow would still authorize the + # request on its own. + # + # HOW THE CONDITION KEY BEHAVES, because the two halves differ and the + # difference is what makes the transition safe: + # - iam:CreateRole and iam:PutRolePermissionsBoundary carry a + # PermissionsBoundary parameter in the request, so the key is the + # boundary being SET. A CreateRole that omits it leaves the key + # absent, an absent key is a mismatch, and the Allow does not apply: + # a role without this boundary cannot be created at all. + # - iam:PutRolePolicy and iam:AttachRolePolicy target an existing + # role, so the key is the boundary ALREADY attached to that role. + # Against a boundary-less role the key is absent and the call is + # denied. + # + # That second half is why iam:PutRolePermissionsBoundary is granted + # here: the roles that already exist in deployed environments predate + # this change and carry no boundary, and the first apply after this + # lands is what attaches it to them. That apply is safe because + # aws_iam_role_policy and aws_iam_role_policy_attachment both reference + # their role, so Terraform's dependency graph puts the + # PutRolePermissionsBoundary call ahead of any PutRolePolicy or + # AttachRolePolicy against the same role. + # + # iam:UpdateAssumeRolePolicy, iam:UpdateRole and + # iam:UpdateRoleDescription are deliberately NOT conditioned even though + # they accept the key, and this is the one place where conditioning + # "everything that supports the key" would have broken the pipeline. + # All three are issued from inside the provider's resourceRoleUpdate + # (internal/service/iam/role.go), whose d.HasChange blocks run in source + # order: assume_role_policy -> description -> max_session_duration -> + # permissions_boundary. They therefore fire BEFORE the boundary is + # attached, so on the first apply against a boundary-less role a + # conditioned UpdateAssumeRolePolicy would 403 before the + # PutRolePermissionsBoundary that would have satisfied it. + # Nothing is lost by leaving them unconditioned: rewriting the trust + # policy of an existing cudly-* role only yields a role that is itself + # capped by the boundary, and a role that does not exist yet cannot be + # created without one. + # + # StringEquals, not StringEqualsIfExists and not a ForAllValues + # qualifier: both of those return true when the key is absent, which + # would silently re-admit the boundary-less CreateRole this statement + # exists to stop. + Sid = "IAMRoleMutationRequiresBoundary" + Effect = "Allow" + Action = [ + "iam:AttachRolePolicy", + "iam:CreateRole", + "iam:PutRolePermissionsBoundary", + "iam:PutRolePolicy", + ] + Resource = ["arn:aws:iam::*:role/cudly-*"] + Condition = { + StringEquals = { + "iam:PermissionsBoundary" = aws_iam_policy.workload_boundary.arn + } + } + }, + { + # The named escalation in #1705: iam:AttachRolePolicy took the ROLE as + # its resource and carried no iam:PolicyARN condition, so any managed + # policy in the account or in the aws namespace could be attached to any + # cudly-* role, AdministratorAccess included. + # + # This is an allowlist of the managed policies terraform/modules + # actually attaches, and it is the complete set: all four are AWS + # managed, and the one customer-managed policy the modules declare + # (aws_iam_policy.secret_read in modules/secrets/aws) is created but + # never attached to anything. Adding a fifth attachment to a module + # without adding it here fails at apply time with AccessDenied, which is + # the visible failure mode; TestDeployPolicyAllowsAttachedManagedPolicies + # in policy_guard_test.go catches it earlier, in CI. + # + # ArnNotEquals, not ArnNotLike: exact ARNs, no patterns. A pattern such + # as arn:aws:iam::aws:policy/service-role/* would readmit every AWS + # service-role policy, and one scoped to the account's own namespace + # would readmit the `*` on `*` policy the deploy role can still create + # (see the note above this resource). Resource is "*" rather than + # role/cudly-* so the Deny cannot be sidestepped by a role name outside + # that prefix. + Sid = "IAMDenyAttachUnapprovedManagedPolicy" + Effect = "Deny" + Action = ["iam:AttachRolePolicy"] + Resource = ["*"] + Condition = { + ArnNotEquals = { + "iam:PolicyARN" = [ + "arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore", + "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole", + "arn:aws:iam::aws:policy/service-role/AWSLambdaVPCAccessExecutionRole", + "arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy", + ] + } + } + }, + { + # Stripping the boundary would undo everything above: a role whose + # boundary has been removed is an unbounded role, and IAMRoleMutation + # RequiresBoundary would then refuse to write to it, so the strip is + # useful to an attacker only in combination with a role it can already + # reach some other way. Denying it outright costs the deploy path + # nothing, because no module ever removes a boundary; if one ever needs + # to, the removal belongs in the manually applied bootstrap root next to + # this Deny, not in a CI-driven apply. + # + # ONE OPERATIONAL CONSEQUENCE, and it is not obvious. rollback.yml + # applies the environment root from the DISPATCHED ref's checkout. A + # rollback to a ref that predates this change has no permissions_boundary + # in config while the live roles have one, so the provider issues + # DeleteRolePermissionsBoundary and this Deny fails the rollback. Roll + # back to a ref at or after this change, or re-apply the boundary by + # hand from the bootstrap root first. See ci-cd-permissions/README.md. + Sid = "IAMDenyStripRoleBoundary" + Effect = "Deny" + Action = ["iam:DeleteRolePermissionsBoundary"] + Resource = ["arn:aws:iam::*:role/cudly-*"] + }, + { + # cudly-terraform-deploy is a cudly-* role, so IAMRoleMutationRequires + # Boundary would otherwise let it put cudly-deploy-boundary on itself. + # That is a self-inflicted denial of service rather than an escalation + # (the boundary allows no IAM mutation, so the next apply could create + # nothing and IAMDenyStripRoleBoundary above would refuse to take it + # back off), which makes it a one-way door worth closing. Kept separate + # from IAMDenyStripRoleBoundary rather than folded into it: Action and + # Resource are matched as a cross product, so a single combined + # statement would deny iam:PutRolePermissionsBoundary against every + # cudly-* role and block the transition apply this whole change depends + # on. + Sid = "IAMDenyBoundaryOnDeployRole" + Effect = "Deny" + Action = ["iam:PutRolePermissionsBoundary"] + Resource = ["arn:aws:iam::*:role/cudly-terraform-deploy"] + }, + ] + }) + + tags = { + Project = "CUDly" + ManagedBy = "terraform" + } +} diff --git a/terraform/environments/aws/ci-cd-permissions/role.tf b/terraform/environments/aws/ci-cd-permissions/role.tf index 4a2133551..6cdbbd7aa 100644 --- a/terraform/environments/aws/ci-cd-permissions/role.tf +++ b/terraform/environments/aws/ci-cd-permissions/role.tf @@ -174,3 +174,8 @@ resource "aws_iam_role_policy_attachment" "data" { role = aws_iam_role.cudly_deploy.name policy_arn = aws_iam_policy.data.arn } + +resource "aws_iam_role_policy_attachment" "iam" { + role = aws_iam_role.cudly_deploy.name + policy_arn = aws_iam_policy.iam.arn +} diff --git a/terraform/environments/aws/compute.tf b/terraform/environments/aws/compute.tf index 123b19567..a32bf9593 100644 --- a/terraform/environments/aws/compute.tf +++ b/terraform/environments/aws/compute.tf @@ -29,9 +29,10 @@ module "compute_lambda" { source = "../../modules/compute/aws/lambda" count = var.compute_platform == "lambda" ? 1 : 0 - stack_name = local.stack_name - environment = var.environment - region = var.region + stack_name = local.stack_name + environment = var.environment + region = var.region + permissions_boundary_arn = local.permissions_boundary_arn # Container image (from build module or var.image_uri) image_uri = var.enable_docker_build ? module.build[0].image_uri : var.image_uri @@ -134,9 +135,10 @@ module "compute_fargate" { source = "../../modules/compute/aws/fargate" count = var.compute_platform == "fargate" ? 1 : 0 - stack_name = local.stack_name - environment = var.environment - region = var.region + stack_name = local.stack_name + environment = var.environment + region = var.region + permissions_boundary_arn = local.permissions_boundary_arn # Container image (from build module or var.image_uri) image_uri = var.enable_docker_build ? module.build[0].image_uri : var.image_uri diff --git a/terraform/environments/aws/database.tf b/terraform/environments/aws/database.tf index 0cb32b580..d57c935ce 100644 --- a/terraform/environments/aws/database.tf +++ b/terraform/environments/aws/database.tf @@ -5,7 +5,8 @@ module "database" { source = "../../modules/database/aws" - stack_name = local.stack_name + stack_name = local.stack_name + permissions_boundary_arn = local.permissions_boundary_arn # Database configuration engine_version = var.database_engine_version diff --git a/terraform/environments/aws/main.tf b/terraform/environments/aws/main.tf index a9c250c7c..15789e160 100644 --- a/terraform/environments/aws/main.tf +++ b/terraform/environments/aws/main.tf @@ -53,6 +53,15 @@ resource "random_id" "suffix" { locals { stack_name = "${var.project_name}-${var.environment}-${random_id.suffix.hex}" + + # ARN of the permissions boundary that every IAM role created by this + # environment's modules must carry. cudly-terraform-deploy's IAM grants + # are conditioned on iam:PermissionsBoundary, so a role created without + # this boundary cannot have its inline policies or managed-policy + # attachments written and the apply fails with AccessDenied. See + # terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + permissions_boundary_arn = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:policy/cudly-deploy-boundary" + # Dashboard URL for CORS and email links. # # Priority (matches outputs.tf's frontend_url chain, modulo the Lambda diff --git a/terraform/environments/aws/networking.tf b/terraform/environments/aws/networking.tf index 4390eead2..1aba520e1 100644 --- a/terraform/environments/aws/networking.tf +++ b/terraform/environments/aws/networking.tf @@ -5,8 +5,9 @@ module "networking" { source = "../../modules/networking/aws" - stack_name = local.stack_name - region = var.region + stack_name = local.stack_name + region = var.region + permissions_boundary_arn = local.permissions_boundary_arn vpc_cidr = var.vpc_cidr az_count = var.az_count diff --git a/terraform/environments/aws/secrets.tf b/terraform/environments/aws/secrets.tf index da7f4decb..2a87ecd8b 100644 --- a/terraform/environments/aws/secrets.tf +++ b/terraform/environments/aws/secrets.tf @@ -5,9 +5,10 @@ module "secrets" { source = "../../modules/secrets/aws" - stack_name = local.stack_name - environment = var.environment - region = var.region + stack_name = local.stack_name + environment = var.environment + region = var.region + permissions_boundary_arn = local.permissions_boundary_arn # Generate random password for dev (in prod, you'd provide this via tfvars) database_password = null # Will be auto-generated diff --git a/terraform/modules/compute/aws/cleanup-lambda/main.tf b/terraform/modules/compute/aws/cleanup-lambda/main.tf index f5cada040..a7781067a 100644 --- a/terraform/modules/compute/aws/cleanup-lambda/main.tf +++ b/terraform/modules/compute/aws/cleanup-lambda/main.tf @@ -1,6 +1,7 @@ # IAM role for cleanup Lambda resource "aws_iam_role" "cleanup" { - name = "${var.stack_name}-cleanup-lambda" + name = "${var.stack_name}-cleanup-lambda" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" diff --git a/terraform/modules/compute/aws/cleanup-lambda/variables.tf b/terraform/modules/compute/aws/cleanup-lambda/variables.tf index 046f58a95..cbe622e4e 100644 --- a/terraform/modules/compute/aws/cleanup-lambda/variables.tf +++ b/terraform/modules/compute/aws/cleanup-lambda/variables.tf @@ -3,6 +3,18 @@ variable "stack_name" { type = string } +variable "permissions_boundary_arn" { + description = <<-EOT + ARN of the permissions boundary that every IAM role in this module must + carry. Required, not optional: cudly-terraform-deploy's IAM grants are + conditioned on iam:PermissionsBoundary, so a role created without this + boundary cannot have its inline policies or managed-policy attachments + written and the apply fails with AccessDenied. See + terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + EOT + type = string +} + variable "image_uri" { description = "Docker image URI containing the cleanup Lambda handler" type = string diff --git a/terraform/modules/compute/aws/fargate/main.tf b/terraform/modules/compute/aws/fargate/main.tf index eb97312fd..14d2c1fc4 100644 --- a/terraform/modules/compute/aws/fargate/main.tf +++ b/terraform/modules/compute/aws/fargate/main.tf @@ -67,7 +67,8 @@ resource "aws_ecs_cluster_capacity_providers" "main" { # Task Execution Role (for ECS to pull images, write logs) resource "aws_iam_role" "task_execution" { - name = "${local.name_prefix}-task-execution" + name = "${local.name_prefix}-task-execution" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" @@ -116,7 +117,8 @@ resource "aws_iam_role_policy" "task_execution_secrets" { # Task Role (for application to access AWS services) resource "aws_iam_role" "task" { - name = "${local.name_prefix}-task" + name = "${local.name_prefix}-task" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" @@ -859,7 +861,8 @@ resource "aws_cloudwatch_event_target" "recommendations" { resource "aws_iam_role" "eventbridge" { count = var.enable_scheduled_tasks ? 1 : 0 - name = "${local.name_prefix}-eventbridge" + name = "${local.name_prefix}-eventbridge" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" @@ -959,7 +962,8 @@ resource "aws_cloudwatch_event_target" "ri_exchange" { resource "aws_iam_role" "eventbridge_ri_exchange" { count = var.enable_ri_exchange_schedule ? 1 : 0 - name = "${local.name_prefix}-eb-ri-exchange" + name = "${local.name_prefix}-eb-ri-exchange" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" @@ -1064,7 +1068,8 @@ resource "aws_cloudwatch_event_target" "ladder_run" { resource "aws_iam_role" "eventbridge_ladder_run" { count = var.enable_ladder_run_schedule ? 1 : 0 - name = "${local.name_prefix}-eb-ladder-run" + name = "${local.name_prefix}-eb-ladder-run" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" @@ -1173,7 +1178,8 @@ resource "aws_cloudwatch_event_target" "fire_scheduled_purchases" { resource "aws_iam_role" "eventbridge_fire_scheduled_purchases" { count = var.enable_fire_scheduled_purchases_schedule ? 1 : 0 - name = "${local.name_prefix}-eb-fire-sched-purchases" + name = "${local.name_prefix}-eb-fire-sched-purchases" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" diff --git a/terraform/modules/compute/aws/fargate/variables.tf b/terraform/modules/compute/aws/fargate/variables.tf index bc269d539..f5d1ff897 100644 --- a/terraform/modules/compute/aws/fargate/variables.tf +++ b/terraform/modules/compute/aws/fargate/variables.tf @@ -5,6 +5,18 @@ variable "stack_name" { type = string } +variable "permissions_boundary_arn" { + description = <<-EOT + ARN of the permissions boundary that every IAM role in this module must + carry. Required, not optional: cudly-terraform-deploy's IAM grants are + conditioned on iam:PermissionsBoundary, so a role created without this + boundary cannot have its inline policies or managed-policy attachments + written and the apply fails with AccessDenied. See + terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + EOT + type = string +} + variable "environment" { description = "Environment name (dev/staging/prod)" type = string diff --git a/terraform/modules/compute/aws/lambda/main.tf b/terraform/modules/compute/aws/lambda/main.tf index 53c940b0d..b2ae5e504 100644 --- a/terraform/modules/compute/aws/lambda/main.tf +++ b/terraform/modules/compute/aws/lambda/main.tf @@ -196,7 +196,8 @@ resource "aws_security_group" "lambda" { # ============================================== resource "aws_iam_role" "lambda" { - name_prefix = "${var.stack_name}-lambda-" + name_prefix = "${var.stack_name}-lambda-" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" diff --git a/terraform/modules/compute/aws/lambda/variables.tf b/terraform/modules/compute/aws/lambda/variables.tf index 587be104b..b6527e750 100644 --- a/terraform/modules/compute/aws/lambda/variables.tf +++ b/terraform/modules/compute/aws/lambda/variables.tf @@ -3,6 +3,18 @@ variable "stack_name" { type = string } +variable "permissions_boundary_arn" { + description = <<-EOT + ARN of the permissions boundary that every IAM role in this module must + carry. Required, not optional: cudly-terraform-deploy's IAM grants are + conditioned on iam:PermissionsBoundary, so a role created without this + boundary cannot have its inline policies or managed-policy attachments + written and the apply fails with AccessDenied. See + terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + EOT + type = string +} + variable "enable_migration_alarm" { description = "Create the migration-failure CloudWatch metric filter + alarm. Defaults to false because the metric filter requires logs:PutMetricFilter on the deploy SA, which is granted via the ci-cd-permissions bootstrap (root CLAUDE.md CI/CD IAM split). Leaving it false keeps deploys unblocked when that permission is absent; set true only after re-applying the bootstrap so the deploy role can manage the filter." type = bool diff --git a/terraform/modules/database/aws/main.tf b/terraform/modules/database/aws/main.tf index c4a269e5b..97ae7b31d 100644 --- a/terraform/modules/database/aws/main.tf +++ b/terraform/modules/database/aws/main.tf @@ -260,7 +260,8 @@ resource "aws_security_group" "rds_proxy" { resource "aws_iam_role" "rds_proxy" { count = var.enable_rds_proxy ? 1 : 0 - name_prefix = "${var.stack_name}-rds-proxy-" + name_prefix = "${var.stack_name}-rds-proxy-" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" diff --git a/terraform/modules/database/aws/variables.tf b/terraform/modules/database/aws/variables.tf index 14935efe5..9f5f6ad1d 100644 --- a/terraform/modules/database/aws/variables.tf +++ b/terraform/modules/database/aws/variables.tf @@ -3,6 +3,18 @@ variable "stack_name" { type = string } +variable "permissions_boundary_arn" { + description = <<-EOT + ARN of the permissions boundary that every IAM role in this module must + carry. Required, not optional: cudly-terraform-deploy's IAM grants are + conditioned on iam:PermissionsBoundary, so a role created without this + boundary cannot have its inline policies or managed-policy attachments + written and the apply fails with AccessDenied. See + terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + EOT + type = string +} + variable "vpc_id" { description = "VPC ID where database will be deployed" type = string diff --git a/terraform/modules/networking/aws/main.tf b/terraform/modules/networking/aws/main.tf index 0ddc78e3d..84a3dc797 100644 --- a/terraform/modules/networking/aws/main.tf +++ b/terraform/modules/networking/aws/main.tf @@ -170,7 +170,8 @@ resource "aws_security_group" "fck_nat" { resource "aws_iam_role" "fck_nat" { count = var.enable_nat_gateway ? 1 : 0 - name_prefix = "${var.stack_name}-fck-nat-" + name_prefix = "${var.stack_name}-fck-nat-" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" @@ -650,7 +651,8 @@ resource "aws_cloudwatch_log_group" "flow_logs" { resource "aws_iam_role" "flow_logs" { count = var.enable_flow_logs ? 1 : 0 - name_prefix = "${var.stack_name}-flow-logs-" + name_prefix = "${var.stack_name}-flow-logs-" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" diff --git a/terraform/modules/networking/aws/variables.tf b/terraform/modules/networking/aws/variables.tf index 8713344ed..032499b8f 100644 --- a/terraform/modules/networking/aws/variables.tf +++ b/terraform/modules/networking/aws/variables.tf @@ -3,6 +3,18 @@ variable "stack_name" { type = string } +variable "permissions_boundary_arn" { + description = <<-EOT + ARN of the permissions boundary that every IAM role in this module must + carry. Required, not optional: cudly-terraform-deploy's IAM grants are + conditioned on iam:PermissionsBoundary, so a role created without this + boundary cannot have its inline policies or managed-policy attachments + written and the apply fails with AccessDenied. See + terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + EOT + type = string +} + variable "region" { description = "AWS region" type = string diff --git a/terraform/modules/secrets/aws/main.tf b/terraform/modules/secrets/aws/main.tf index 949e36c48..9362e1a6f 100644 --- a/terraform/modules/secrets/aws/main.tf +++ b/terraform/modules/secrets/aws/main.tf @@ -303,7 +303,8 @@ resource "aws_lambda_function" "rotation" { resource "aws_iam_role" "rotation" { count = var.enable_secret_rotation ? 1 : 0 - name_prefix = "${var.stack_name}-rotation-" + name_prefix = "${var.stack_name}-rotation-" + permissions_boundary = var.permissions_boundary_arn assume_role_policy = jsonencode({ Version = "2012-10-17" diff --git a/terraform/modules/secrets/aws/variables.tf b/terraform/modules/secrets/aws/variables.tf index 942b89a50..29ee2d574 100644 --- a/terraform/modules/secrets/aws/variables.tf +++ b/terraform/modules/secrets/aws/variables.tf @@ -3,6 +3,18 @@ variable "stack_name" { type = string } +variable "permissions_boundary_arn" { + description = <<-EOT + ARN of the permissions boundary that every IAM role in this module must + carry. Required, not optional: cudly-terraform-deploy's IAM grants are + conditioned on iam:PermissionsBoundary, so a role created without this + boundary cannot have its inline policies or managed-policy attachments + written and the apply fails with AccessDenied. See + terraform/environments/aws/ci-cd-permissions/policy_boundary.tf. + EOT + type = string +} + variable "environment" { description = "Environment name (dev/staging/prod)" type = string