fix(ci): vend read-only plan credentials to app tf-plan

docs/apply-gate.md

@@ -49,13 +49,29 @@ from the payload:
 Tokens are minted with `scripts/github-oidc-token.sh <audience>` and verified
 in `terraform/lambda-src/shared/github_oidc.py` (RS256 against GitHub's JWKS).
 
+Plan credentials are read-only. The broker vends `javabin-ci-plan-{team}`
+(ReadOnlyAccess, read on the team's state, the state lock, and PutObject on
+`plans/*`) to tf-plan, which runs on any branch. The only way to get the
+write-capable `javabin-ci-team-{team}` is through this gate, for a reviewed
+plan.
+
+State access is limited by an explicit Deny, not by the Allow. `ReadOnlyAccess`
+(plan) and `AllowAll` (team) cover every key in the state bucket, and the
+developer boundary's `DenyXTeam` exempts that bucket, because S3 objects carry
+no team tag. Both roles therefore carry `DenyForeignTerraformState`, which
+denies the platform's state (`infra/`, `org/`, `state/`, `env:/`) and every
+other registered team's `apps/{team}/` prefix. The list is built from
+`registered_teams`, so a new top-level prefix in the bucket must be added to
+`local.foreign_state_keys` in `terraform/platform/iam/main.tf`.
+
 ## IAM Roles
 
 | Role | Purpose | Permissions |
 |------|---------|-------------|
 | `javabin-ci-apply-gate` | OIDC role for tf-apply workflow | `lambda:InvokeFunction` on gate Lambda + `s3:GetObject` on plan bucket |
 | `javabin-apply-gate` | Lambda execution role | `ssm:GetParameter` (signing key) + `s3:Get/PutObject` (plan bucket) + `sts:AssumeRole` (app CI roles) |
-| `javabin-ci-app-{repo}` | App infrastructure role | Trusted by OIDC (plan/review) AND by gate Lambda role (apply) |
+| `javabin-ci-plan-{team}` | Read-only plan role | Vended by the CI broker for tf-plan: ReadOnlyAccess + own state read (other keys denied) + lock + plan upload |
+| `javabin-ci-team-{team}` | Team infrastructure role (write) | Vended by the gate Lambda for a reviewed plan (apply); other teams' and platform state denied |
 
 ## Credential Lifetime
 

docs/reusable-workflows.md

@@ -29,7 +29,7 @@ The pipeline detects your repo contents and runs the right jobs. No repository o
 
 | Prerequisite | Why | How |
 |---|---|---|
-| Team exists in `javaBin/registry` (`teams/<team>.yaml`) | Creates the GitHub team; platform CI then creates the team's CI roles `javabin-ci-team-<team>` and `javabin-ci-deploy-<team>` | `javabin register-team`, merge the PR |
+| Team exists in `javaBin/registry` (`teams/<team>.yaml`) | Creates the GitHub team; platform CI then creates the team's CI roles `javabin-ci-plan-<team>`, `javabin-ci-team-<team>` and `javabin-ci-deploy-<team>` | `javabin register-team`, merge the PR |
 | Repo belongs to the GitHub team named in `app.yaml` `team:` | The CI broker verifies repo → team before vending team credentials | `gh api -X PUT orgs/javaBin/teams/<team>/repos/javaBin/<repo> -f permission=push` |
 | `app.yaml` at repo root | Declares the infrastructure | See [app-yaml-reference.md](app-yaml-reference.md) |
 
@@ -105,7 +105,7 @@ Docker BuildKit build + push to ECR repo `{team}-{app_name}`. Uses GitHub Action
 
 Plans the committed `terraform/` at the branch head (after expand), uploads the plan to S3 with its SHA256, has the `javabin-plan-review` Lambda review it (Bedrock Claude Haiku 5.5, platform credentials; it writes `risk.json`) and posts plan + review to the PR. A review that cannot run after retries is `FAILED`: re-run the workflow, or request an override if it persists.
 
-**AWS role:** `javabin-ci-app-broker` (OIDC) → broker Lambda → `javabin-ci-team-{team}`; the review step uses the broker OIDC role to invoke `javabin-plan-review`
+**AWS role:** `javabin-ci-app-broker` (OIDC) → broker Lambda → `javabin-ci-plan-{team}` (read-only: refresh, state read, state lock, plan upload; tf-plan runs on any branch, so it cannot change resources); the review step uses the broker OIDC role to invoke `javabin-plan-review`
 **Outputs:** `has_changes`, `plan_key`, `plan_sha256`, `risk_level`, `planned_sha`
 
 ### tf-apply.yml

terraform/lambda-src/ci_broker/handler.py

@@ -6,7 +6,10 @@
 then receive team-scoped credentials if the repo belongs to a registered team.
 
 Actions:
-  - plan:   Returns credentials for javabin-ci-team-{team} (Terraform plan/apply)
+  - plan:   Returns credentials for javabin-ci-plan-{team} (read-only Terraform
+            plan: refresh, state lock, plan upload). Write access comes only
+            from the apply gate, which vends javabin-ci-team-{team} for a
+            reviewed plan.
   - deploy: Returns credentials for javabin-ci-deploy-{team} (Docker build/ECS deploy)
 """
 
@@ -31,7 +34,9 @@
 DEPLOY_DURATION = 900  # 15 minutes for deploy
 
 ROLE_MAP = {
-    "plan": f"{PROJECT}-ci-team-",
+    # Read-only: any branch of a team repo can run tf-plan, so plan
+    # credentials must not be able to change the team's resources.
+    "plan": f"{PROJECT}-ci-plan-",
     "deploy": f"{PROJECT}-ci-deploy-",
 }
 

terraform/lambda-src/tests/test_ci_broker.py

@@ -0,0 +1,74 @@
+"""Tests for ci_broker — run: python3 -m unittest discover terraform/lambda-src/tests"""
+
+import importlib.util
+import os
+import sys
+import types
+import unittest
+from unittest import mock
+
+SRC = os.path.join(os.path.dirname(__file__), "..")
+
+
+class FakeSTS:
+    def __init__(self):
+        self.calls = []
+
+    def assume_role(self, RoleArn, RoleSessionName, DurationSeconds):
+        self.calls.append({"RoleArn": RoleArn, "DurationSeconds": DurationSeconds})
+        return {"Credentials": {"AccessKeyId": "a", "SecretAccessKey": "s", "SessionToken": "t"}}
+
+
+def load(sts):
+    """Import the handler against a stub boto3 without leaking it.
+
+    The handler reads env and builds its STS client at import time, so the
+    patches only need to hold during exec_module. patch.dict restores
+    sys.modules afterwards, so later tests that import boto3 (or shared.*)
+    get the real modules regardless of test order.
+    """
+    boto3 = types.ModuleType("boto3")
+    boto3.client = lambda svc, **kw: sts if svc == "sts" else mock.MagicMock()
+    env = {"AWS_ACCOUNT_ID": "123456789012", "PROJECT": "javabin"}
+    with mock.patch.dict(os.environ, env), \
+            mock.patch.dict(sys.modules, {"boto3": boto3}), \
+            mock.patch.object(sys, "path", [SRC] + sys.path):
+        spec = importlib.util.spec_from_file_location("ci_broker_handler", os.path.join(SRC, "ci_broker", "handler.py"))
+        mod = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(mod)
+    mod.verify = lambda token, aud: ({"repository": "javaBin/cupcake"} if token == "good"
+                                     else (_ for _ in ()).throw(mod.TokenError("bad")))
+    mod.resolve_team = lambda repo, requested: "cupcake"
+    return mod
+
+
+class CiBrokerTest(unittest.TestCase):
+    def setUp(self):
+        self.sts = FakeSTS()
+        self.broker = load(self.sts)
+
+    def call(self, action):
+        return self.broker.handler({"action": action, "id_token": "good", "team": "cupcake"}, None)
+
+    def test_plan_vends_the_read_only_plan_role_not_the_team_role(self):
+        result = self.call("plan")
+        self.assertTrue(result["approved"])
+        self.assertEqual(self.sts.calls[0]["RoleArn"], "arn:aws:iam::123456789012:role/javabin-ci-plan-cupcake")
+
+    def test_deploy_still_vends_the_deploy_role(self):
+        self.assertTrue(self.call("deploy")["approved"])
+        self.assertEqual(self.sts.calls[0]["RoleArn"], "arn:aws:iam::123456789012:role/javabin-ci-deploy-cupcake")
+
+    def test_no_action_ever_vends_the_write_capable_team_role(self):
+        for action in self.broker.ROLE_MAP:
+            self.call(action)
+        self.assertFalse(any("ci-team-" in c["RoleArn"] for c in self.sts.calls))
+
+    def test_bad_token_gets_no_credentials(self):
+        result = self.broker.handler({"action": "plan", "id_token": "forged"}, None)
+        self.assertFalse(result["approved"])
+        self.assertEqual(self.sts.calls, [])
+
+
+if __name__ == "__main__":
+    unittest.main()

terraform/platform/iam/main.tf

@@ -319,16 +319,36 @@ resource "aws_iam_role_policy" "ci_app_broker" {
 }
 
 ################################################################################
-# 2. javabin-ci-team-{team} — Per-team Terraform role (plan + apply)
+# 2. javabin-ci-team-{team} — Per-team Terraform role (apply)
 #
 # Trust: ONLY the CI broker Lambda and apply-gate Lambda can assume these roles.
+# The broker vends javabin-ci-plan-{team} for plan (2b); the broker trust here
+# stays so a broker Lambda still on the old code keeps working during rollout.
 # No direct OIDC trust — the broker validates repo→team membership via GitHub
 # API before issuing credentials. This is the hard enforcement point.
 #
 # Permissions: Allow *:* with team-based tag isolation. Resources must be
 # tagged with the correct team name.
 ################################################################################
 
+locals {
+  state_bucket_arn = "arn:aws:s3:::${var.project}-terraform-state-${var.aws_account_id}"
+
+  # State keys a team's CI roles must not touch: the platform's own state
+  # (infra/ holds the Google OAuth client secret) and every other team's
+  # apps/{team}/ prefix. A Deny with NotResource would also hit every other
+  # S3 object in the account, so the forbidden prefixes are listed instead.
+  # The boundary's DenyXTeam exempts the state bucket, and AllowAll /
+  # ReadOnlyAccess cover every key, so without this Deny a team role could
+  # read (ci_plan) or overwrite (ci_team) any state in the bucket.
+  foreign_state_keys = {
+    for team in var.registered_teams : team => concat(
+      [for prefix in ["infra", "org", "state", "env:"] : "${local.state_bucket_arn}/${prefix}/*"],
+      [for other in var.registered_teams : "${local.state_bucket_arn}/apps/${other}/*" if other != team],
+    )
+  }
+}
+
 resource "aws_iam_role" "ci_team" {
   for_each = toset(var.registered_teams)
 
@@ -407,6 +427,12 @@ resource "aws_iam_role_policy" "ci_team_allow" {
           "arn:aws:s3:::${var.project}-terraform-state-${var.aws_account_id}/apps/${each.key}/*",
         ]
       },
+      {
+        Sid      = "DenyForeignTerraformState"
+        Effect   = "Deny"
+        Action   = ["s3:GetObject", "s3:GetObjectVersion", "s3:PutObject", "s3:DeleteObject", "s3:DeleteObjectVersion"]
+        Resource = local.foreign_state_keys[each.key]
+      },
       {
         # Terraform locking: DynamoDB scoped to team's state paths.
         Sid    = "AllowTerraformLocking"
@@ -427,6 +453,142 @@ resource "aws_iam_role_policy" "ci_team_allow" {
   })
 }
 
+################################################################################
+# 2b. javabin-ci-plan-{team} — Per-team read-only Terraform plan role
+#
+# Trust: ONLY the CI broker Lambda. The broker vends this role for tf-plan,
+# which runs on any branch and on pull requests. Write access to the team's
+# resources comes only from javabin-ci-team-{team}, which the apply gate
+# vends for a reviewed plan.
+#
+# Permissions: ReadOnlyAccess (refresh + data sources), the team's state
+# (read), the state lock, and the plan upload. Named ci-plan-*, not
+# ci-team-plan-*, so it cannot collide with a team called "plan-...".
+################################################################################
+
+resource "aws_iam_role" "ci_plan" {
+  for_each = toset(var.registered_teams)
+
+  name                 = "${var.project}-ci-plan-${each.key}"
+  permissions_boundary = aws_iam_policy.developer_boundary.arn
+
+  assume_role_policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [
+      {
+        Sid    = "AllowViaCIBroker"
+        Effect = "Allow"
+        Principal = {
+          AWS = var.ci_broker_role_arn
+        }
+        Action = "sts:AssumeRole"
+      },
+    ]
+  })
+
+  tags = {
+    Name = "${var.project}-ci-plan-${each.key}"
+    team = each.key
+  }
+}
+
+resource "aws_iam_role_policy_attachment" "ci_plan_readonly" {
+  for_each = toset(var.registered_teams)
+
+  role       = aws_iam_role.ci_plan[each.key].name
+  policy_arn = "arn:aws:iam::aws:policy/ReadOnlyAccess"
+}
+
+resource "aws_iam_role_policy" "ci_plan" {
+  for_each = toset(var.registered_teams)
+
+  name = "plan-backend"
+  role = aws_iam_role.ci_plan[each.key].id
+
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [
+      {
+        # Terraform state: read-only. Plan never writes state. The Deny
+        # below is what keeps plan off other keys.
+        Sid    = "ReadTerraformState"
+        Effect = "Allow"
+        Action = ["s3:GetObject", "s3:ListBucket"]
+        Resource = [
+          "arn:aws:s3:::${var.project}-terraform-state-${var.aws_account_id}",
+          "arn:aws:s3:::${var.project}-terraform-state-${var.aws_account_id}/apps/${each.key}/*",
+        ]
+      },
+      {
+        # ReadOnlyAccess grants s3:Get* on every key, so the Allow above does
+        # not limit reads by itself. See local.foreign_state_keys.
+        Sid      = "DenyForeignTerraformState"
+        Effect   = "Deny"
+        Action   = ["s3:GetObject", "s3:GetObjectVersion"]
+        Resource = local.foreign_state_keys[each.key]
+      },
+      {
+        # Plan takes the state lock. Same scoping as ci_team.
+        Sid    = "AllowTerraformLocking"
+        Effect = "Allow"
+        Action = [
+          "dynamodb:GetItem",
+          "dynamodb:PutItem",
+          "dynamodb:DeleteItem",
+        ]
+        Resource = "arn:aws:dynamodb:${var.region}:${var.aws_account_id}:table/${var.project}-terraform-app-locks"
+        Condition = {
+          "ForAllValues:StringLike" = {
+            "dynamodb:LeadingKeys" = "${var.project}-terraform-state-${var.aws_account_id}/apps/${each.key}/*"
+          }
+        }
+      },
+      {
+        # Refreshing aws_secretsmanager_secret_version reads the value;
+        # ReadOnlyAccess does not include it. Team-prefixed secrets only.
+        Sid      = "RefreshTeamSecrets"
+        Effect   = "Allow"
+        Action   = "secretsmanager:GetSecretValue"
+        Resource = "arn:aws:secretsmanager:${var.region}:${var.aws_account_id}:secret:${each.key}-*"
+      },
+      {
+        # SecureString / secret refresh decrypts through the owning service.
+        Sid      = "DecryptViaParameterServices"
+        Effect   = "Allow"
+        Action   = "kms:Decrypt"
+        Resource = "*" # Key ARNs are not known here; scoped by kms:ViaService
+        Condition = {
+          StringEquals = {
+            "kms:ViaService" = [
+              "ssm.${var.region}.amazonaws.com",
+              "secretsmanager.${var.region}.amazonaws.com",
+            ]
+          }
+        }
+      },
+      {
+        # upload-plan.sh stores tfplan + plan-output.txt for the apply gate.
+        Sid      = "UploadPlan"
+        Effect   = "Allow"
+        Action   = "s3:PutObject"
+        Resource = "arn:aws:s3:::${var.project}-ci-plan-artifacts-${var.aws_account_id}/plans/*"
+      },
+      {
+        # Review verdicts and overrides come only from the plan-review and
+        # apply-gate Lambdas; a plan job must not mark its own plan LOW.
+        Sid    = "DenyWriteReviewVerdicts"
+        Effect = "Deny"
+        Action = ["s3:PutObject", "s3:DeleteObject"]
+        Resource = [
+          "arn:aws:s3:::${var.project}-ci-plan-artifacts-${var.aws_account_id}/*/risk.json",
+          "arn:aws:s3:::${var.project}-ci-plan-artifacts-${var.aws_account_id}/*/review.json",
+          "arn:aws:s3:::${var.project}-ci-plan-artifacts-${var.aws_account_id}/*/override.json",
+        ]
+      },
+    ]
+  })
+}
+
 ################################################################################
 # 3. javabin-ci-deploy-{team} — Per-team deploy role (build + push + deploy)
 #

terraform/platform/lambdas/main.tf

@@ -1536,6 +1536,8 @@ resource "aws_iam_role_policy" "ci_broker" {
         Effect = "Allow"
         Action = "sts:AssumeRole"
         Resource = [
+          "arn:aws:iam::${var.aws_account_id}:role/${var.project}-ci-plan-*",
+          # Kept while the broker Lambda rolls over to ci-plan-* (one apply).
           "arn:aws:iam::${var.aws_account_id}:role/${var.project}-ci-team-*",
           "arn:aws:iam::${var.aws_account_id}:role/${var.project}-ci-deploy-*",
         ]