fix(ci): review app plans in a platform Lambda instead of with team roles

.github/workflows/tf-plan.yml

@@ -126,29 +126,29 @@ jobs:
         run: .platform/scripts/upload-plan.sh "${{ inputs.tf_root }}"
 
       # ----------------------------------------------------------------
-      # LLM Plan Review (runs before PR comment so both are combined)
+      # LLM plan review — done by the javabin-plan-review Lambda with
+      # platform credentials (team roles have no Bedrock access, and only
+      # the Lambda may write risk.json). Re-assume the broker OIDC role,
+      # which may invoke it.
       # ----------------------------------------------------------------
+      - name: Assume broker role for plan review
+        if: steps.plan.outputs.has_changes == 'true'
+        uses: aws-actions/configure-aws-credentials@v6
+        with:
+          role-to-assume: arn:aws:iam::${{ inputs.aws_account_id }}:role/javabin-ci-app-broker
+          aws-region: ${{ inputs.aws_region }}
+          role-session-name: ${{ env.SESSION_NAME }}
+
       - name: Run LLM review
         id: review
         if: steps.plan.outputs.has_changes == 'true'
         env:
-          REVIEW_RESULT_PATH: review-result.json
-        run: sh .platform/scripts/extract-review-risk.sh .platform/scripts/review-plan.py "${{ inputs.tf_root }}/plan-output.txt"
-
-      - name: Upload risk assessment to S3
-        if: steps.plan.outputs.has_changes == 'true' && steps.review.outputs.risk_level != ''
-        run: |
-          PLAN_DIR=$(dirname "${{ steps.upload.outputs.plan_key }}")
-          echo '{"level":"${{ steps.review.outputs.risk_level }}","reviewed_at":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'"}' | \
-            aws s3 cp - "s3://${PLAN_BUCKET}/${PLAN_DIR}/risk.json" --content-type application/json
+          NOTIFY: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
+        run: sh .platform/scripts/invoke-plan-review.sh "${{ steps.upload.outputs.plan_key }}"
 
       - name: Post plan + review to PR
         if: github.event_name == 'pull_request'
         env:
           GH_TOKEN: ${{ github.token }}
           PR_NUMBER: ${{ github.event.pull_request.number }}
         run: sh .platform/scripts/post-plan-comment.sh "${{ inputs.tf_root }}/plan-output.txt" "${{ steps.plan.outputs.has_changes }}"
-
-      - name: Alert Slack on blocked risk
-        if: github.event_name == 'push' && github.ref == 'refs/heads/main' && (steps.review.outputs.risk_level == 'HIGH' || steps.review.outputs.risk_level == 'FAILED')
-        run: sh .platform/scripts/notify-high-risk.sh /javabin/slack/platform-override-alerts-webhook "https://github.com/javaBin/platform/actions/workflows/approve-override.yml"

CLAUDE.md

@@ -96,7 +96,7 @@ terraform/platform/
   iam/             GitHub OIDC, CI roles, permission boundary
   compute/         ECS cluster, ECR base config
   monitoring/      SNS, EventBridge, Config, GuardDuty, Security Hub
-  lambdas/         slack-alert, cost-report, daily-cost-check, compliance-reporter, resource-tagger, budget-enforcer, override-cleanup, team-provisioner, apply-gate, securityhub-summary, password-set, ci-broker, app-auth-sync
+  lambdas/         slack-alert, cost-report, daily-cost-check, compliance-reporter, resource-tagger, budget-enforcer, override-cleanup, team-provisioner, apply-gate, securityhub-summary, password-set, ci-broker, app-auth-sync, plan-review
   identity/        Cognito user pools (internal + external). Identity Center is in terraform/org/
 ```
 
@@ -170,6 +170,7 @@ terraform/state/
 | `resource-tagger` | EventBridge-triggered, auto-tags created-by + commit on new resources |
 | `ci-broker` | Verifies the job's GitHub OIDC token (repo from signed claims), checks repo ∈ app.yaml team, vends short-lived team role credentials |
 | `app-auth-sync` | Creates/deletes Cognito app clients requested by app.yaml `auth:` (SSM request → client + credentials) |
+| `plan-review` | LLM review of app Terraform plans with platform credentials; writes `risk.json` (invoked by app tf-plan) |
 
 ### Scripts
 | Script | What |
@@ -183,7 +184,8 @@ terraform/state/
 | `scripts/provision-teams.py` | CI: fetch team YAMLs from registry, invoke team-provisioner Lambda |
 | `scripts/provision-groups.py` | CI: resolve members.yaml + access.yaml, invoke team-provisioner Lambda |
 | `scripts/sync-members.py` | CI: sync approved heroes from Google Sheets into members.yaml |
-| `scripts/review-plan.py` | CI: LLM plan review via Bedrock |
+| `scripts/review-plan.py` | CI: LLM plan review via Bedrock (platform CI; also bundled into the plan-review Lambda) |
+| `scripts/invoke-plan-review.sh` | CI: app tf-plan → plan-review Lambda |
 | `scripts/notify-slack.py` | CI: generic Slack webhook notification |
 | `scripts/invoke-apply-gate.sh` | CI: invoke gate Lambda for apply credentials |
 | `scripts/run-plan.sh` | CI: terraform plan with exit code handling |

docs/lambda-functions.md

@@ -116,6 +116,19 @@ Idempotent: skips resources that already have a `created-by` tag (preserves orig
 
 **Manual run:** `aws lambda invoke --function-name javabin-app-auth-sync --payload '{"action":"reconcile"}' --cli-binary-format raw-in-base64-out /dev/stdout`
 
+## plan-review
+
+**Trigger:** Invoked by app `tf-plan` (via the `javabin-ci-app-broker` OIDC role) after the plan is uploaded
+**Purpose:** LLM risk review of app Terraform plans with platform credentials. Team CI roles have no Bedrock access (developer boundary), and any code in a plan job runs with them.
+
+**Flow:** Verify the job's GitHub OIDC token (aud `javabin-plan-review`) → plan must be under `plans/javaBin/<repo>/` → read `plan-output.txt` → review with Bedrock using `scripts/review-plan.py` (bundled as `review_plan.py`), up to 3 attempts with backoff → write `risk.json` (read by apply-gate) and `review.json` → on HIGH/FAILED for pushes to main, post to the override Slack channel → return risk + markdown for the PR comment.
+
+Team roles are denied writing `risk.json`, `review.json` and `override.json` in the plan bucket. `risk.json` is written as FAILED before the review starts, so a crash or timeout still leaves a verdict the gate (and a board override) can act on. The plan text is uploaded by the plan job itself, so the review catches mistakes; it is not a control against code in the job forging its own plan text.
+
+| SSM Parameter | Use |
+|---------------|-----|
+| `/javabin/slack/platform-override-alerts-webhook` | HIGH/FAILED alerts |
+
 ## Shared Module: pricing
 
 `terraform/lambda-src/shared/pricing.py` — provides `estimate_cost()` for enriching alerts with cost impact. Uses AWS Pricing API (us-east-1 endpoint). Caches pricing data across Lambda invocations.

docs/reusable-workflows.md

@@ -103,9 +103,9 @@ Docker BuildKit build + push to ECR repo `{team}-{app_name}`. Uses GitHub Action
 
 ### tf-plan.yml
 
-Plans the committed `terraform/` at the branch head (after expand), uploads the plan to S3 with its SHA256, runs the LLM review (`scripts/review-plan.py`, Bedrock Claude Haiku 5.5) and posts plan + review to the PR.
+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}`
+**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`
 **Outputs:** `has_changes`, `plan_key`, `plan_sha256`, `risk_level`, `planned_sha`
 
 ### tf-apply.yml

scripts/invoke-plan-review.sh

@@ -0,0 +1,42 @@
+#!/bin/sh
+# Ask the plan-review Lambda to review an uploaded app plan.
+#
+# Usage: invoke-plan-review.sh <plan_key>
+#
+# Runs with the javabin-ci-app-broker OIDC role (team roles have no Bedrock
+# access). The Lambda verifies this job's GitHub OIDC token, reviews the
+# plan text next to <plan_key>, and writes risk.json itself.
+#
+# Env: NOTIFY=true to post HIGH/FAILED to the override Slack channel.
+# Output: review-output.txt (markdown for the PR comment),
+#         risk_level=<LOW|MEDIUM|HIGH|FAILED> in GITHUB_OUTPUT.
+
+set -e
+
+PLAN_KEY="${1:?Usage: invoke-plan-review.sh <plan_key>}"
+ID_TOKEN=$(sh "$(dirname "$0")/github-oidc-token.sh" "${PROJECT:-javabin}-plan-review")
+
+# A failed invoke is a FAILED review, not a failed job: the PR still gets a
+# comment and the gate still blocks.
+echo '{}' > /tmp/plan-review.json
+aws lambda invoke \
+  --function-name "${PROJECT:-javabin}-plan-review" \
+  --cli-read-timeout 310 \
+  --payload "$(jq -nc --arg plan_key "$PLAN_KEY" --arg id_token "$ID_TOKEN" --argjson notify "${NOTIFY:-false}" \
+    '{plan_key: $plan_key, id_token: $id_token, notify: $notify}')" \
+  --cli-binary-format raw-in-base64-out \
+  /tmp/plan-review.json >/dev/null || echo '{"error": "could not invoke the plan-review Lambda"}' > /tmp/plan-review.json
+
+ERROR=$(jq -r '.error // .errorMessage // (if .risk then empty else "empty response" end)' /tmp/plan-review.json)
+if [ -n "$ERROR" ]; then
+  echo "::error::Plan review failed: $ERROR"
+  RISK=FAILED
+  printf '**Risk: ⚪ FAILED**\n\nPlan review could not run: %s\n' "$ERROR" > review-output.txt
+else
+  RISK=$(jq -r '.risk' /tmp/plan-review.json)
+  jq -r '.markdown' /tmp/plan-review.json > review-output.txt
+fi
+
+cat review-output.txt
+echo "risk_level=${RISK}" >> "${GITHUB_OUTPUT:-/dev/null}"
+echo "Plan review risk: ${RISK}"

scripts/review-plan.py

@@ -6,6 +6,9 @@
 
 Uses AWS CLI for Bedrock Converse (no boto3 version dependency).
 Writes structured JSON to REVIEW_RESULT_PATH (default: review-result.json).
+
+Also bundled into the javabin-plan-review Lambda (app plans), which passes
+its own boto3-based `converse` to review_plan().
 """
 
 import json
@@ -67,8 +70,28 @@
 CATEGORY_EMOJI = {"security": "\U0001f512", "cost": "\U0001f4b0", "destruction": "\U0001f4a5", "routine": "\u2705"}
 
 
-def review_plan(plan_text):
-    """Send plan to Bedrock via AWS CLI for LLM review, return structured dict."""
+def cli_converse(messages, tool_config):
+    """Bedrock Converse via the AWS CLI. Returns the response dict or None."""
+    result = subprocess.run(
+        [
+            "aws", "bedrock-runtime", "converse",
+            "--model-id", MODEL_ID,
+            "--messages", json.dumps(messages),
+            "--tool-config", json.dumps(tool_config),
+            "--region", REGION,
+        ],
+        capture_output=True,
+        text=True,
+        timeout=120,
+    )
+    if result.returncode != 0:
+        print(f"AWS CLI error: {result.stderr}", file=sys.stderr)
+        return None
+    return json.loads(result.stdout)
+
+
+def review_plan(plan_text, converse=cli_converse):
+    """Send plan to Bedrock for LLM review, return structured dict (or None)."""
     if len(plan_text) > 80000:
         plan_text = plan_text[:80000] + "\n\n... (truncated)"
 
@@ -92,25 +115,10 @@ def review_plan(plan_text):
 
     messages = [{"role": "user", "content": [{"text": prompt}]}]
 
-    result = subprocess.run(
-        [
-            "aws", "bedrock-runtime", "converse",
-            "--model-id", MODEL_ID,
-            "--messages", json.dumps(messages),
-            "--tool-config", json.dumps(REVIEW_TOOL_CONFIG),
-            "--region", REGION,
-        ],
-        capture_output=True,
-        text=True,
-        timeout=120,
-    )
-
-    if result.returncode != 0:
-        print(f"AWS CLI error: {result.stderr}", file=sys.stderr)
+    response = converse(messages, REVIEW_TOOL_CONFIG)
+    if not response:
         return None
 
-    response = json.loads(result.stdout)
-
     for content in response.get("output", {}).get("message", {}).get("content", []):
         if "toolUse" in content and content["toolUse"]["name"] == "submit_review":
             return content["toolUse"]["input"]

terraform/lambda-src/plan_review/handler.py

@@ -0,0 +1,124 @@
+"""Plan review — LLM risk review of an app's Terraform plan, on platform credentials.
+
+tf-plan runs with the team's CI role, which the developer boundary keeps away
+from Bedrock (and must: any code in a plan job runs with those credentials).
+So app plans are reviewed here instead:
+
+  1. Verify the caller's GitHub OIDC token; the repo comes from its signed
+     claims, and the plan must be that repo's own (plans/javaBin/<repo>/...).
+  2. Read plan-output.txt next to the plan in the plan bucket.
+  3. Review it with Bedrock (review_plan.py, shared with platform CI),
+     retrying transient failures.
+  4. Write risk.json (read by the apply gate) and review.json. Team roles
+     cannot write these objects. (The plan text itself is uploaded by the
+     plan job, so the review guards against mistakes, not against code in
+     the job forging its own plan text.)
+  5. For HIGH or FAILED on main, post the override request to Slack.
+
+Input:  {plan_key, id_token, notify?}
+Output: {risk, markdown} or {error}
+"""
+
+import json
+import logging
+import os
+import time
+
+import boto3
+from botocore.config import Config
+
+import review_plan
+from shared.github_oidc import TokenError, repo_name, verify
+from shared.slack import get_webhook_url, post_to_slack
+
+logger = logging.getLogger(__name__)
+logger.setLevel(logging.INFO)
+
+PROJECT = os.environ.get("PROJECT", "javabin")
+GITHUB_ORG = os.environ.get("GITHUB_ORG", "javaBin")
+PLAN_BUCKET = os.environ["PLAN_BUCKET"]
+OIDC_AUDIENCE = os.environ.get("OIDC_AUDIENCE", f"{PROJECT}-plan-review")
+SLACK_PARAM = os.environ.get("SLACK_WEBHOOK_PARAM", f"/{PROJECT}/slack/platform-override-alerts-webhook")
+OVERRIDE_URL = f"https://github.com/{GITHUB_ORG}/platform/actions/workflows/approve-override.yml"
+ATTEMPTS = int(os.environ.get("REVIEW_ATTEMPTS", "3"))
+
+s3 = boto3.client("s3")
+ssm = boto3.client("ssm")
+# Bounded per call, so retries fit inside the Lambda timeout; the handler
+# does its own retrying.
+bedrock = boto3.client("bedrock-runtime", config=Config(read_timeout=90, connect_timeout=10,
+                                                         retries={"max_attempts": 1, "mode": "standard"}))
+CALL_BUDGET_MS = 100_000
+
+
+def converse(messages, tool_config):
+    return bedrock.converse(modelId=review_plan.MODEL_ID, messages=messages, toolConfig=tool_config)
+
+
+def review_with_retries(plan_text, context=None):
+    for attempt in range(1, ATTEMPTS + 1):
+        if context and context.get_remaining_time_in_millis() < CALL_BUDGET_MS:
+            logger.warning("Stopping after %d attempts: not enough time left for another", attempt - 1)
+            return None
+        try:
+            review = review_plan.review_plan(plan_text, converse=converse)
+            if review:
+                return review
+            logger.warning("Review attempt %d returned no structured output", attempt)
+        except Exception as e:  # throttling, timeouts, model errors
+            logger.warning("Review attempt %d failed: %s", attempt, e)
+        if attempt < ATTEMPTS:
+            time.sleep(2 ** attempt)
+    return None
+
+
+def put_json(key, body):
+    s3.put_object(Bucket=PLAN_BUCKET, Key=key, Body=json.dumps(body).encode(), ContentType="application/json")
+
+
+def notify(repo, plan_key, risk, review):
+    summary = review.get("summary", "") if review else "The LLM review could not run; re-run the workflow or request an override."
+    text = (f"*Apply blocked — {risk} risk*\n*Repo:* {GITHUB_ORG}/{repo}\n*Plan:* `{plan_key}`\n{summary}\n"
+            f"<{OVERRIDE_URL}|Approve override>")
+    try:
+        post_to_slack(get_webhook_url(ssm, SLACK_PARAM), {"text": text})
+    except Exception as e:
+        logger.error("Slack notification failed: %s", e)
+
+
+def handler(event, context):
+    try:
+        repo = repo_name(verify(event.get("id_token"), OIDC_AUDIENCE))
+    except TokenError as e:
+        logger.warning("Rejected review call: %s", e)
+        return {"error": f"Invalid GitHub OIDC token: {e}"}
+
+    plan_key = event.get("plan_key", "")
+    prefix = f"plans/{GITHUB_ORG}/{repo}/"
+    if not plan_key.startswith(prefix) or not plan_key.endswith("/tfplan") or ".." in plan_key:
+        return {"error": f"Plan {plan_key!r} does not belong to {repo}"}
+    plan_dir = plan_key.rsplit("/", 1)[0]
+
+    # Record FAILED first: if anything below crashes or times out, the gate
+    # still finds a verdict (and a board override can unblock it).
+    put_json(f"{plan_dir}/risk.json", {"level": "FAILED", "reviewed_at": None, "reviewer": f"{PROJECT}-plan-review"})
+
+    plan_text = s3.get_object(Bucket=PLAN_BUCKET, Key=f"{plan_dir}/plan-output.txt")["Body"].read().decode()
+    if not plan_text.strip():
+        review = {"risk": "LOW", "summary": "Empty plan", "findings": []}
+    else:
+        review = review_with_retries(plan_text, context)
+
+    risk = review["risk"] if review else "FAILED"
+    reviewed_at = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
+    put_json(f"{plan_dir}/risk.json", {"level": risk, "reviewed_at": reviewed_at, "reviewer": f"{PROJECT}-plan-review"})
+    put_json(f"{plan_dir}/review.json", review or {"risk": "FAILED"})
+    logger.info("Reviewed %s: %s", plan_key, risk)
+
+    if risk in ("HIGH", "FAILED") and event.get("notify"):
+        notify(repo, plan_key, risk, review)
+
+    markdown = review_plan.format_review(review) if review else (
+        "**Risk: ⚪ FAILED**\n\nThe LLM review could not run after "
+        f"{ATTEMPTS} attempts. Re-run the workflow; if it keeps failing, request an override.")
+    return {"risk": risk, "markdown": markdown}