[IAmJSD]

Root cause

terraform/modules/compute/aws/fargate/main.tf has four EventBridge invoker roles that do the same thing, each with its own hand-copied policy. eventbridge_ladder_run and eventbridge_fire_scheduled_purchases condition ecs:RunTask on ArnEquals ecs:cluster and iam:PassRole on StringEquals iam:PassedToService = ecs-tasks.amazonaws.com. eventbridge (recommendations) and eventbridge_ri_exchange had neither condition. So either role could run the task definition in any cluster in the account, and could pass the task role (which holds the SP/RI purchase grants) to any service that accepts a passed role. Reaching that needs someone who can create EventBridge targets in our account, because the roles trust only events.amazonaws.com.

Fix

The two Condition blocks from eventbridge_ladder_run are now on the recommendations and RI-exchange policies, unchanged. The Resource scoping stays as it was: RunTask on aws_ecs_task_definition.main.arn, PassRole on the task and task-execution role ARNs. After this, all four invoker policies are the same apart from their names. The comment in ci-cd-permissions/policy_boundary.tf ("The ecs:cluster condition and the task-definition Resource stay on the module policy") was only true for two of the four roles before this PR. It is now true for all four.

Runtime effect: none for the scheduled tasks. The EventBridge targets already run aws_ecs_task_definition.main in aws_ecs_cluster.main, and ECS passes the roles to ecs-tasks.amazonaws.com. That is the configuration the ladder-run and fire-scheduled-purchases roles already run with.

Tests

• terraform/modules/compute/aws/fargate/tests/eventbridge_invoker_iam.tftest.hcl (new; CI's "Test AWS compute modules" loop picks it up). It turns on all four schedules and runs command = apply against mock_provider "aws", so nothing is created. It jsondecodes each of the four rendered policies and checks two things. First, there is exactly one RunTask statement, on the module's task definition, with ArnEquals ecs:cluster = the module's cluster. Second, there is exactly one PassRole statement, on exactly the task and task-execution roles, with StringEquals iam:PassedToService = ecs-tasks.amazonaws.com. The task and task-execution roles get distinct overridden ARNs, so a PassRole statement that names only one of them can't pass because two mock ARNs collapsed into one value.

• terraform/modules/compute/aws/eventbridge_invoker_iam_test.go (new Go guard, same package and the same tree walk as the #1636 cross-account guard). The tftest names four specific policies. This guard instead finds every Allow ecs:RunTask grant (also ecs:* and ecs:Run*, case-insensitive) under terraform/ and cloudformation/stacks. It requires each grant to have:

  • a Resource on an aws_ecs_task_definition.<x>.arn reference
  • ArnEquals "ecs:cluster" = aws_ecs_cluster.<x>.arn (by reference, so a literal wildcard ARN fails)
  • at least one PassRole statement next to it, each on aws_iam_role.<x>.arn references and carrying the iam:PassedToService condition

  If someone copies the policy a fifth time, the new copy is held to the same rule. As a floor, the guard must find all four known policies, so it can't pass by finding nothing. TestRunTaskGuardRejectsUnconditionedGrants runs each matcher against the pre-fix and post-fix shapes of the recommendations policy. The permissions boundary (policy_boundary.tf) is a ceiling rather than a grant. The existing walk already skips it.

Verification

All evidence below is unit, mock or fixture evidence. Nothing was deployed, and no terraform plan/apply ran against a real backend or account.

Fail before the fix (main.tf at origin/main, new tests in place):

$ terraform -chdir=terraform/modules/compute/aws/fargate test        # Terraform 1.10.5 (CI's pin) and 1.15.8
  run "invoker_roles_are_scoped"... fail
  eventbridge: ecs:RunTask must be one statement ... conditioned on ArnEquals ecs:cluster ...
  eventbridge: iam:PassRole must be one statement ... conditioned on StringEquals iam:PassedToService ...
  eventbridge_ri_exchange: ecs:RunTask ...
  eventbridge_ri_exchange: iam:PassRole ...
Failure! 0 passed, 1 failed.

$ AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./terraform/modules/compute/aws/ -count=1 -run RunTask
--- FAIL: TestRunTaskGrantsArePinnedToClusterAndECSTasks
  fargate/main.tf: eventbridge grants ecs:RunTask without Condition ArnEquals "ecs:cluster" ...
  fargate/main.tf: eventbridge grants iam:PassRole beside ecs:RunTask without Condition StringEquals "iam:PassedToService" ...
  fargate/main.tf: eventbridge_ri_exchange grants ecs:RunTask without ...
  fargate/main.tf: eventbridge_ri_exchange grants iam:PassRole beside ecs:RunTask without ...

Only the two roles named in the issue fail. Ladder-run and fire-scheduled-purchases pass before the fix as well.

Pass after the fix:

$ terraform -chdir=terraform/modules/compute/aws/fargate init -backend=false -lockfile=readonly -input=false   # CI's exact init
$ terraform -chdir=terraform/modules/compute/aws/fargate test        # 1.10.5 and 1.15.8
Success! 1 passed, 0 failed.
$ go test ./terraform/modules/compute/aws/ -count=1                  # whole package, incl. #1636 guards
ok

Mutation probes, each applied to the fixed main.tf and then reverted:

Mutation	Go guard	tftest
drop the ecs:cluster condition (recommendations)	FAIL	FAIL
iam:PassedToService -> lambda.amazonaws.com (recommendations; RI exchange)	FAIL	FAIL
ecs:cluster -> literal arn:aws:ecs:*:*:cluster/* (fire)	FAIL	FAIL
RunTask Resource = "*" (ladder)	FAIL	FAIL
PassRole drops the task-execution role (recommendations)	pass (narrowing only)	FAIL
"ecs:RunTask" -> "ecs:*" (recommendations)	pass (still conditioned)	FAIL

Other checks run locally:

• terraform fmt -check -recursive terraform/; terraform validate on the module, and on terraform/environments/aws with init -backend=false (Terraform 1.10.5)
• .terraform.lock.hcl for the fargate module is unchanged. It already carries linux_amd64, darwin_arm64 and darwin_amd64 hashes: terraform providers lock for those three platforms left it byte-identical. CI's init -lockfile=readonly succeeds on it.
• go build ./..., go vet, golangci-lint v2.10.1 on ./terraform/modules/compute/aws/... (0 issues), gocyclo -over 10 (silent)
• go test ./internal/mocks/ -count=1 and go test ./terraform/environments/aws/ci-cd-permissions/ -count=1 pass
• git-secrets (--register-aws plus the setup-git-secrets.sh patterns, .gitallowed applied) scanned the changed files in an isolated temp repo: clean. scripts/test-git-secrets-allowlist.sh: 42 PASS, 0 FAIL. Fixtures use the 123456789012 placeholder in ARN position only.
• trivy config --severity HIGH,CRITICAL on the module: 0 findings
• Go tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null, because the default profile on this machine is invalid.
• Not run locally: tflint and gosec (not installed here). CI's pre-commit job covers them.

Real-cloud gap: these tests don't evaluate IAM. They check that the policy says what it should, not how AWS authorizes it. That these exact conditions work at runtime rests on the ladder-run and fire-scheduled-purchases roles, which already use them.

Out of scope

• The issue also suggests collapsing the four invoker roles into one for_each so the copies can't drift again. That changes resource addresses (it needs moved blocks and a state migration on every deployment), so this PR doesn't do it. The Go guard covers the drift risk in the meantime.

Closes #292

Maintainer: please mirror the issue's triage labels onto this PR: priority/p2, severity/medium, urgency/this-quarter, impact/internal, effort/xs, type/security, triaged.

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

VERDICT: APPROVE 38a786b

Reviewed head 38a786b (one commit) against origin/main b6a0649. git merge-tree --write-tree origin/main <head> is clean (tree 16f8ec75); every test below ran on that merged tree.

Findings

• nit (process) - the only commit is +492/-0, over the 400-line per-commit cap. 333 of those lines are the Go guard. Non-blocking.
• nit terraform/modules/compute/aws/eventbridge_invoker_iam_test.go:1-333 - a regex walk over HCL is a second guard on top of the tftest that already checks the rendered policies. It does catch a fifth hand-copied invoker policy, and it fails loudly when it finds nothing (the floor at :176-189). The for_each consolidation the PR defers would make most of it unnecessary. Non-blocking.

Correctness

• fargate/main.tf:945-950 and :1056-1061 add ArnEquals ecs:cluster = aws_ecs_cluster.main.arn to RunTask. :958-965 and :1069-1076 add StringEquals iam:PassedToService = ecs-tasks.amazonaws.com to PassRole. These are the same blocks the ladder-run (:1172-1192) and fire-scheduled-purchases (:1282-1302) roles already carry, and they are valid IAM: ecs:cluster is a RunTask condition key, and PassedToService for ECS tasks is ecs-tasks.amazonaws.com. Resource stays on the revisioned task-definition ARN, which is the same ARN the targets use (:884, :995).
• The targets this affects (:874-905, :985-) run aws_ecs_task_definition.main in aws_ecs_cluster.main with no role overrides, so they still match both conditions. No Go code calls RunTask (grep over all *.go finds only the two test files), so EventBridge is the only caller.
• Deploy effect: in-place updates of the two aws_iam_role_policy documents, only where enable_scheduled_tasks / enable_ri_exchange_schedule are true (github-{dev,staging,prod}.tfvars set enable_scheduled_tasks). No address changes, no replacements.
• No bootstrap re-apply needed. The deploy role already does PutRolePolicy on these roles, and the boundary (ci-cd-permissions/policy_boundary.tf:96-120) is only a ceiling. The new conditions make the policies narrower, not wider.
• The fix closes sec(iac/aws): two Fargate EventBridge roles have unconditioned RunTask and PassRole #292 as the issue describes it.

Attacked / local results (Terraform 1.10.5)

• terraform fmt -check -recursive terraform/: OK. In the fargate module, init -backend=false -lockfile=readonly succeeded, validate succeeded and test gave 1 passed. terraform/environments/aws passed init -backend=false and validate.
• go test ./terraform/modules/compute/aws/ ./terraform/environments/aws/ci-cd-permissions/ -count=1: 85 passed.
• Mutation probes, each in a scratch export and each restored afterwards. Every one was caught by both the Go guard and the tftest:
  • revert main.tf to origin/main;
  • delete the ri_exchange ecs:cluster condition;
  • change eventbridge PassedToService to lambda.amazonaws.com;
  • replace the cluster reference with the literal arn:aws:ecs:*:*:cluster/*.
• I did not run plan or apply, and nothing evaluated the policy against real AWS IAM.

[cristim]

Independent review 2 (claude-fleet, cold): APPROVE 38a786b

Findings (none blocking):

• [non-blocking] terraform/modules/compute/aws/eventbridge_invoker_iam_test.go:47 and :166 (the allowEffect filter and the quoted-action regex) - The "discovers every RunTask grant" guard misses two forms. Probes P5 and P6 both pass with Resource = "*" and no conditions:
  • (a) A data "aws_iam_policy_document" statement with no effect, which defaults to Allow, is skipped.
  • (b) An action held in a locals list (Action = local.x) is skipped.
  • All four real policies use the jsonencode form, and the tftest pins them on rendered output. So only a hypothetical fifth copy written another way is affected, and the body's "a fifth copy is held to the same rule" holds only for the jsonencode form.
  • Suggested follow-up: fail when a RunTask match sits outside a statement that has a literal Effect, instead of skipping it.
• [non-blocking, disclosed] The Go guard accepts "ecs:*" in place of "ecs:RunTask" as long as the conditions stay (probe P8). The tftest catches it.
• On origin/main, the four invoker policies (main.tf:931, 1042, 1158 and 1268) are textually identical apart from name and role, and fargate/ is unchanged since the merge-base.
• This fixes sec(iac/aws): two Fargate EventBridge roles have unconditioned RunTask and PassRole #292's root cause (copied policies drifted) for the two roles the issue names. Collapsing the roles into one for_each is deferred, with the stated reason that it changes resource addresses.

Attacked:

• Runtime safety:
  • Each EventBridge target (main.tf:879-884, 990-995, 1107-1112 and 1217-1222) runs aws_ecs_task_definition.main.arn on aws_ecs_cluster.main.arn, so the ArnEquals ecs:cluster condition matches.
  • PassedToService ecs-tasks.amazonaws.com is what ladder-run and fire already ship with.
• Blast radius:
  • terraform/, cloudformation/ and iac/ have no other identity grant of RunTask, and the CI deploy role grants none.
  • policy_boundary.tf is a ceiling and is skipped by the walk, and its comment at :96-100 now holds.
• Merge: merge-tree --write-tree origin/main <head> is clean.
  • Main changed lambda/ and ci-cd-permissions/ (fix(iac/aws): fail closed when the Lambda DB secret ARN is empty #1117, fix: six small scheduled-auth, CI, OpenAPI and IaC fixes #1127) without adding any RunTask or PassRole grant.
  • Everything was re-run on the merged-tree export.
• Mutation probes on a scratch copy of main.tf, restored and confirmed identical with cmp:
  • P1, ri_exchange cluster condition dropped: Go and tftest FAIL.
  • P2, ri_exchange PassedToService set to lambda: Go and tftest FAIL.
  • P3, recommendations ArnEquals changed to ArnLike: Go and tftest FAIL.
  • P4, a fifth unconditioned jsonencode copy in a new file: Go FAILS.
  • P5, a policy_document statement with no effect: Go passes. Gap.
  • P6, the action held in a local: Go passes. Gap.
  • P7, recommendations PassRole Resource = "*": Go and tftest FAIL.
  • P8, ecs:RunTask changed to ecs:*: Go passes, tftest FAILS.

Verified locally, with Terraform 1.10.5 and go1.26.9, no cloud credentials, under the heavy lock:

• Head:
  • terraform init -backend=false -lockfile=readonly, validate, fmt -check -recursive and test: pass.
  • go build ./..., go vet and go test -count=1 on ./terraform/modules/compute/aws/: pass.
  • gofmt -l on the new test: clean.
  • The ci-cd-permissions Go tests: pass.
• Merged tree: the same checks pass.
• Fail-before, with the new tests on base:
  • The tftest fails 4 asserts (eventbridge and ri_exchange, RunTask and PassRole).
  • TestRunTaskGrantsArePinnedToClusterAndECSTasks fails with the same 4 messages.
  • Ladder-run and fire pass, as the PR claims.
• No migration and no frontend changes.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.