From 5d86f1524df643c9f04e9ce154f678ef81a8441f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 5 Aug 2026 12:29:34 +0200 Subject: [PATCH 1/2] sec(iac/aws): gate deploy-role IAM writes on a permissions boundary The cudly-terraform-deploy role held iam:CreateRole, iam:AttachRolePolicy and iam:PutRolePolicy on arn:aws:iam::*:role/cudly-* with no iam:PermissionsBoundary condition and no iam:PolicyARN condition. It could therefore create a role, attach AdministratorAccess to it, and either assume it (same-account, so a trust policy naming the deploy role suffices) or pass it to Lambda under the existing IAMPassRoleScopedByService grant: full account administrator from a GitHub Actions OIDC token, persisting past rotation of that token. Denying the three actions is not an option. The deploy path legitimately creates roles on every apply, so a blanket Deny 403s every deployment, and a name-based Deny cannot help because the escalation target is a role and any name the deploy role may legitimately use is one it may also abuse. What separates a legitimate role from an escalation vehicle is its ceiling, and a permissions boundary is the only AWS mechanism that caps a principal regardless of its identity policy. It is also the only thing that closes iam:PutRolePolicy, whose inline document supports no condition key at all. Every role in terraform/modules now carries cudly-deploy-boundary, and the three actions plus iam:PutRolePermissionsBoundary are re-granted in a new cudly-deploy-iam policy only under StringEquals on iam:PermissionsBoundary, so a role without the ceiling can be neither created nor written to. iam:AttachRolePolicy additionally carries an ArnNotEquals Deny allowlisting the four AWS managed policies the modules actually attach, which is what makes AdministratorAccess unattachable rather than merely capped. Stripping a boundary is denied outright, and the deploy role cannot boundary itself. iam:UpdateAssumeRolePolicy, iam:UpdateRole and iam:UpdateRoleDescription are deliberately left unconditioned even though they accept the key: the provider's resourceRoleUpdate issues all three BEFORE the boundary call in the same function, so conditioning them would 403 the first apply against a role that does not carry the boundary yet. The boundary is a ceiling, not a grant, so it removes no permission any role holds today: its allow list is derived from the IAM policy documents in terraform/modules plus the four attached AWS managed policies. It is per-service rather than per-action because a too-narrow boundary fails at runtime rather than at apply time. organizations and sts are the two exceptions, pinned to the exact actions and role prefix the modules use: organizations:* reaches CreateAccount and SCPs, and sts:AssumeRole on "*" escapes the boundary entirely, since the assumed session is a different principal. policy_guard_test.go turns the remaining drift into a CI failure rather than a production 403: it re-derives the service set, the attached managed-policy ARNs and the cross-account role prefix from the modules, asserts every module role is boundaried and that none is declared outside them, and asserts the structure of both new statements so the condition operators, the Deny effect and the wildcard-free ARN list cannot be weakened silently. Closes #1705 --- .../aws/ci-cd-permissions/README.md | 22 + .../aws/ci-cd-permissions/policy_boundary.tf | 197 +++ .../aws/ci-cd-permissions/policy_data.tf | 12 +- .../ci-cd-permissions/policy_guard_test.go | 1056 +++++++++++++++++ .../aws/ci-cd-permissions/policy_iam.tf | 177 +++ .../aws/ci-cd-permissions/role.tf | 5 + terraform/environments/aws/compute.tf | 14 +- terraform/environments/aws/database.tf | 3 +- terraform/environments/aws/main.tf | 9 + terraform/environments/aws/networking.tf | 5 +- terraform/environments/aws/secrets.tf | 7 +- .../compute/aws/cleanup-lambda/main.tf | 3 +- .../compute/aws/cleanup-lambda/variables.tf | 12 + terraform/modules/compute/aws/fargate/main.tf | 18 +- .../modules/compute/aws/fargate/variables.tf | 12 + terraform/modules/compute/aws/lambda/main.tf | 3 +- .../modules/compute/aws/lambda/variables.tf | 12 + terraform/modules/database/aws/main.tf | 3 +- terraform/modules/database/aws/variables.tf | 12 + terraform/modules/networking/aws/main.tf | 6 +- terraform/modules/networking/aws/variables.tf | 12 + terraform/modules/secrets/aws/main.tf | 3 +- terraform/modules/secrets/aws/variables.tf | 12 + 23 files changed, 1588 insertions(+), 27 deletions(-) create mode 100644 terraform/environments/aws/ci-cd-permissions/policy_boundary.tf create mode 100644 terraform/environments/aws/ci-cd-permissions/policy_guard_test.go create mode 100644 terraform/environments/aws/ci-cd-permissions/policy_iam.tf 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..d576e4dda --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf @@ -0,0 +1,197 @@ +# 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 that are NOT safe at + # `service:*` granularity, because at that width each of them is a + # complete escape from this boundary rather than a widening within it: + # + # - 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. + # + # Both 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..74ec782ad --- /dev/null +++ b/terraform/environments/aws/ci-cd-permissions/policy_iam.tf @@ -0,0 +1,177 @@ +# 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, neither of which supports any +# condition key at all (verified against the AWS Service Authorization +# Reference), 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 From 1c2ce2ea55948061f504b385331bd8661bc63026 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 6 Aug 2026 10:34:56 +0200 Subject: [PATCH 2/2] docs(iac/aws): correct PR #1722 comments on boundary scope and policy size Fix three inaccuracies flagged by adversarial review, all in comments only: - policy_boundary.tf: the organizations/sts comment read as a complete escape analysis. State the criterion, name organizations and sts as narrowed in this change, and note that lambda:*, ssm:* and ecs:* meet the same criterion and are tracked in #1723 instead. - policy_iam.tf: iam:CreatePolicy does support aws:RequestTag/${TagKey} and aws:TagKeys condition keys (it accepts an optional Tags parameter). iam:CreatePolicyVersion supports none. The conclusion is unchanged since neither constrains the policy document. - PR body: recompute the two new policies' compact-JSON sizes (966 and 1083, not 1127 and 1223), add the privilege-escalation residual and the unconditioned iam:AddRoleToInstanceProfile action to "Left open, deliberately". No policy logic, resources, or tests changed. --- .../aws/ci-cd-permissions/policy_boundary.tf | 30 ++++++++++++++----- .../aws/ci-cd-permissions/policy_iam.tf | 19 +++++++----- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf index d576e4dda..cdc486c6c 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_boundary.tf @@ -98,9 +98,12 @@ resource "aws_iam_policy" "workload_boundary" { Resource = "*" }, { - # organizations and sts are the two services that are NOT safe at - # `service:*` granularity, because at that width each of them is a - # complete escape from this boundary rather than a widening within it: + # 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 @@ -113,10 +116,23 @@ resource "aws_iam_policy" "workload_boundary" { # account trusts the management account root) and the ceiling is # simply gone. # - # Both 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). + # 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 = [ diff --git a/terraform/environments/aws/ci-cd-permissions/policy_iam.tf b/terraform/environments/aws/ci-cd-permissions/policy_iam.tf index 74ec782ad..081389723 100644 --- a/terraform/environments/aws/ci-cd-permissions/policy_iam.tf +++ b/terraform/environments/aws/ci-cd-permissions/policy_iam.tf @@ -12,14 +12,17 @@ # 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, neither of which supports any -# condition key at all (verified against the AWS Service Authorization -# Reference), 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. +# 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"