diff --git a/.github/workflows/tf-plan.yml b/.github/workflows/tf-plan.yml index fad29b5..326fa81 100644 --- a/.github/workflows/tf-plan.yml +++ b/.github/workflows/tf-plan.yml @@ -126,21 +126,25 @@ 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' @@ -148,7 +152,3 @@ jobs: 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" diff --git a/CLAUDE.md b/CLAUDE.md index 3d1a8c8..93e74d1 100644 --- a/CLAUDE.md +++ b/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 | diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index 4a38c85..17be8af 100644 --- a/docs/lambda-functions.md +++ b/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//` → 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. diff --git a/docs/reusable-workflows.md b/docs/reusable-workflows.md index 434ad93..4a3f435 100644 --- a/docs/reusable-workflows.md +++ b/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 diff --git a/scripts/invoke-plan-review.sh b/scripts/invoke-plan-review.sh new file mode 100644 index 0000000..36efdc2 --- /dev/null +++ b/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 +# +# 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 , 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= in GITHUB_OUTPUT. + +set -e + +PLAN_KEY="${1:?Usage: invoke-plan-review.sh }" +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}" diff --git a/scripts/review-plan.py b/scripts/review-plan.py index cacf13e..94f73fd 100755 --- a/scripts/review-plan.py +++ b/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"] diff --git a/terraform/lambda-src/plan_review/handler.py b/terraform/lambda-src/plan_review/handler.py new file mode 100644 index 0000000..4844e16 --- /dev/null +++ b/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//...). + 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} diff --git a/terraform/lambda-src/tests/test_plan_review.py b/terraform/lambda-src/tests/test_plan_review.py new file mode 100644 index 0000000..7757c92 --- /dev/null +++ b/terraform/lambda-src/tests/test_plan_review.py @@ -0,0 +1,119 @@ +"""Tests for plan_review — run: python3 -m unittest discover terraform/lambda-src/tests""" + +import importlib.util +import io +import json +import os +import sys +import types +import unittest +from unittest import mock + +HERE = os.path.dirname(__file__) +SRC = os.path.join(HERE, "..") + + +class FakeS3: + def __init__(self, objects): + self.objects = dict(objects) + + def get_object(self, Bucket, Key): + return {"Body": io.BytesIO(self.objects[Key].encode())} + + def put_object(self, Bucket, Key, Body, ContentType): + self.objects[Key] = Body.decode() + + +class FakeBedrock: + def __init__(self, responses): + self.responses = list(responses) + self.calls = 0 + + def converse(self, **kw): + self.calls += 1 + r = self.responses.pop(0) + if isinstance(r, Exception): + raise r + return r + + +def tool_response(risk): + return {"output": {"message": {"content": [{"toolUse": {"name": "submit_review", "input": { + "risk": risk, "summary": "Adds an ECS service", "findings": [{"category": "routine", "detail": "ok"}]}}}]}}} + + +PLAN = "plans/javaBin/frosting/123/tfplan" + + +def load(s3, bedrock, repo="frosting"): + os.environ["PLAN_BUCKET"] = "bucket" + os.environ["REVIEW_ATTEMPTS"] = "3" + boto3 = types.ModuleType("boto3") + boto3.client = lambda svc, **kw: {"s3": s3, "bedrock-runtime": bedrock, "ssm": mock.MagicMock()}[svc] + sys.modules["boto3"] = boto3 + botocore = types.ModuleType("botocore"); config = types.ModuleType("botocore.config") + config.Config = lambda **kw: None + sys.modules["botocore"] = botocore; sys.modules["botocore.config"] = config + rp = importlib.util.spec_from_file_location("review_plan", os.path.join(SRC, "..", "..", "scripts", "review-plan.py")) + review_plan = importlib.util.module_from_spec(rp) + rp.loader.exec_module(review_plan) + sys.modules["review_plan"] = review_plan + sys.path.insert(0, SRC) + spec = importlib.util.spec_from_file_location("plan_review_handler", os.path.join(SRC, "plan_review", "handler.py")) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + mod.verify = lambda token, aud: {"repository": f"javaBin/{repo}"} if token == "good" else (_ for _ in ()).throw(mod.TokenError("bad")) + mod.time.sleep = lambda s: None + return mod + + +class PlanReviewTest(unittest.TestCase): + def setUp(self): + self.s3 = FakeS3({"plans/javaBin/frosting/123/plan-output.txt": "Plan: 15 to add, 0 to change, 0 to destroy."}) + + def test_review_writes_risk_and_returns_markdown(self): + h = load(self.s3, FakeBedrock([tool_response("LOW")])) + result = h.handler({"plan_key": PLAN, "id_token": "good"}, None) + self.assertEqual(result["risk"], "LOW") + self.assertIn("Adds an ECS service", result["markdown"]) + self.assertEqual(json.loads(self.s3.objects["plans/javaBin/frosting/123/risk.json"])["level"], "LOW") + + def test_transient_failures_are_retried(self): + bedrock = FakeBedrock([RuntimeError("ThrottlingException"), tool_response("MEDIUM")]) + h = load(self.s3, bedrock) + self.assertEqual(h.handler({"plan_key": PLAN, "id_token": "good"}, None)["risk"], "MEDIUM") + self.assertEqual(bedrock.calls, 2) + + def test_persistent_failure_is_failed_not_high(self): + h = load(self.s3, FakeBedrock([RuntimeError("boom")] * 3)) + result = h.handler({"plan_key": PLAN, "id_token": "good"}, None) + self.assertEqual(result["risk"], "FAILED") + self.assertIn("could not run", result["markdown"]) + self.assertEqual(json.loads(self.s3.objects["plans/javaBin/frosting/123/risk.json"])["level"], "FAILED") + + def test_failed_is_recorded_before_review_so_a_crash_leaves_a_verdict(self): + s3 = FakeS3({}) # plan text missing: get_object raises + h = load(s3, FakeBedrock([])) + with self.assertRaises(KeyError): + h.handler({"plan_key": PLAN, "id_token": "good"}, None) + self.assertEqual(json.loads(s3.objects["plans/javaBin/frosting/123/risk.json"])["level"], "FAILED") + + def test_stops_retrying_when_time_runs_out(self): + bedrock = FakeBedrock([RuntimeError("slow")] * 3) + h = load(self.s3, bedrock) + ctx = types.SimpleNamespace(get_remaining_time_in_millis=lambda: 50_000) + self.assertEqual(h.handler({"plan_key": PLAN, "id_token": "good"}, ctx)["risk"], "FAILED") + self.assertEqual(bedrock.calls, 0) + + def test_another_repos_plan_is_refused(self): + h = load(self.s3, FakeBedrock([]), repo="cupcake") + self.assertIn("does not belong", h.handler({"plan_key": PLAN, "id_token": "good"}, None)["error"]) + self.assertNotIn("plans/javaBin/frosting/123/risk.json", self.s3.objects) + + def test_invalid_token_is_refused(self): + h = load(self.s3, FakeBedrock([])) + self.assertIn("Invalid GitHub OIDC token", h.handler({"plan_key": PLAN, "id_token": "forged"}, None)["error"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/terraform/platform/iam/main.tf b/terraform/platform/iam/main.tf index f98fc12..e35ae8f 100644 --- a/terraform/platform/iam/main.tf +++ b/terraform/platform/iam/main.tf @@ -306,12 +306,14 @@ resource "aws_iam_role_policy" "ci_app_broker" { name = "invoke-ci-broker-only" role = aws_iam_role.ci_app_broker.id + # The broker vends team credentials; plan-review reviews app plans with + # platform credentials (team roles have no Bedrock access). policy = jsonencode({ Version = "2012-10-17" Statement = [{ Effect = "Allow" Action = "lambda:InvokeFunction" - Resource = var.ci_broker_function_arn + Resource = [var.ci_broker_function_arn, var.plan_review_function_arn] }] }) } @@ -378,6 +380,18 @@ resource "aws_iam_role_policy" "ci_team_allow" { Action = "*" Resource = "*" }, + { + # 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", + ] + }, { # Terraform state: S3 scoped to team's key prefix. Sid = "AllowTerraformBackend" diff --git a/terraform/platform/iam/variables.tf b/terraform/platform/iam/variables.tf index 95c0ca0..3e07fa3 100644 --- a/terraform/platform/iam/variables.tf +++ b/terraform/platform/iam/variables.tf @@ -39,3 +39,8 @@ variable "ci_broker_function_arn" { description = "ARN of the CI broker Lambda function (for OIDC broker role invoke permission)" type = string } + +variable "plan_review_function_arn" { + description = "ARN of the plan review Lambda (broker role may invoke it)" + type = string +} diff --git a/terraform/platform/lambdas/main.tf b/terraform/platform/lambdas/main.tf index 5a02b73..216c6c7 100644 --- a/terraform/platform/lambdas/main.tf +++ b/terraform/platform/lambdas/main.tf @@ -1727,3 +1727,133 @@ resource "aws_lambda_permission" "app_auth_sync_reconcile" { principal = "events.amazonaws.com" source_arn = aws_cloudwatch_event_rule.app_auth_sync_reconcile.arn } + +################################################################################ +# Plan Review — LLM risk review of app Terraform plans (javabin-plan-review) +# +# App tf-plan jobs run with team CI roles, which the developer boundary keeps +# away from Bedrock. tf-plan invokes this Lambda (via the broker OIDC role) +# with its GitHub OIDC token; the Lambda reviews the plan text in the plan +# bucket and writes risk.json, which team roles cannot write themselves. +################################################################################ + +locals { + plan_bucket = "${var.project}-ci-plan-artifacts-${var.aws_account_id}" +} + +data "archive_file" "plan_review" { + type = "zip" + output_path = "${path.module}/builds/plan_review.zip" + output_file_mode = "0644" + + source { + content = file("${local.lambda_src_path}/plan_review/handler.py") + filename = "handler.py" + } + # Same prompt and schema as platform CI's own review + source { + content = file("${path.module}/../../../scripts/review-plan.py") + filename = "review_plan.py" + } + source { + content = file("${local.lambda_src_path}/shared/__init__.py") + filename = "shared/__init__.py" + } + source { + content = file("${local.lambda_src_path}/shared/github_oidc.py") + filename = "shared/github_oidc.py" + } + source { + content = file("${local.lambda_src_path}/shared/slack.py") + filename = "shared/slack.py" + } +} + +resource "aws_iam_role" "plan_review" { + name = "${var.project}-plan-review" + # The org boundary denies creating any role without one + permissions_boundary = var.org_boundary_arn + + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = [{ + Effect = "Allow" + Principal = { Service = "lambda.amazonaws.com" } + Action = "sts:AssumeRole" + }] + }) +} + +resource "aws_iam_role_policy" "plan_review" { + name = "${var.project}-plan-review" + role = aws_iam_role.plan_review.id + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Sid = "ReadPlanText" + Effect = "Allow" + Action = "s3:GetObject" + Resource = "arn:aws:s3:::${local.plan_bucket}/plans/*/plan-output.txt" + }, + { + Sid = "WriteReview" + Effect = "Allow" + Action = "s3:PutObject" + Resource = [ + "arn:aws:s3:::${local.plan_bucket}/plans/*/risk.json", + "arn:aws:s3:::${local.plan_bucket}/plans/*/review.json", + ] + }, + { + Sid = "BedrockInferenceProfile" + Effect = "Allow" + Action = ["bedrock:InvokeModel", "bedrock:Converse"] + Resource = "arn:aws:bedrock:${var.region}:${var.aws_account_id}:inference-profile/eu.anthropic.*" + }, + { + # Cross-region inference profiles route to EU foundation models; + # the condition only allows them via our inference profiles. + Sid = "BedrockFoundationModels" + Effect = "Allow" + Action = ["bedrock:InvokeModel", "bedrock:Converse"] + Resource = "arn:aws:bedrock:eu-*::foundation-model/anthropic.*" + Condition = { + StringLike = { + "bedrock:InferenceProfileArn" = "arn:aws:bedrock:${var.region}:${var.aws_account_id}:inference-profile/eu.anthropic.*" + } + } + }, + { + Sid = "SlackWebhook" + Effect = "Allow" + Action = "ssm:GetParameter" + Resource = "arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter/${var.project}/slack/platform-override-alerts-webhook" + }, + ] + }) +} + +resource "aws_iam_role_policy_attachment" "plan_review_logs" { + role = aws_iam_role.plan_review.name + policy_arn = "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" +} + +resource "aws_lambda_function" "plan_review" { + function_name = "${var.project}-plan-review" + role = aws_iam_role.plan_review.arn + handler = "handler.handler" + runtime = "python3.12" + timeout = 300 + memory_size = 256 + filename = data.archive_file.plan_review.output_path + source_code_hash = data.archive_file.plan_review.output_base64sha256 + + environment { + variables = { + PROJECT = var.project + PLAN_BUCKET = local.plan_bucket + } + } +} diff --git a/terraform/platform/lambdas/outputs.tf b/terraform/platform/lambdas/outputs.tf index 58273aa..18b7c2f 100644 --- a/terraform/platform/lambdas/outputs.tf +++ b/terraform/platform/lambdas/outputs.tf @@ -67,3 +67,8 @@ output "budget_enforcement_topic_arn" { description = "ARN of the budget-enforcement SNS topic" value = aws_sns_topic.budget_enforcement.arn } + +output "plan_review_function_arn" { + description = "ARN of the plan review Lambda (invoked by app tf-plan via the broker OIDC role)" + value = aws_lambda_function.plan_review.arn +} diff --git a/terraform/platform/main.tf b/terraform/platform/main.tf index e0112f2..5e72cf1 100644 --- a/terraform/platform/main.tf +++ b/terraform/platform/main.tf @@ -31,15 +31,16 @@ module "ingress" { } module "iam" { - source = "./iam" - project = var.project - region = var.region - aws_account_id = var.aws_account_id - registered_teams = var.registered_teams - github_org = var.github_org - apply_gate_role_arn = module.lambdas.apply_gate_role_arn - ci_broker_role_arn = module.lambdas.ci_broker_role_arn - ci_broker_function_arn = module.lambdas.ci_broker_function_arn + source = "./iam" + project = var.project + region = var.region + aws_account_id = var.aws_account_id + registered_teams = var.registered_teams + github_org = var.github_org + apply_gate_role_arn = module.lambdas.apply_gate_role_arn + ci_broker_role_arn = module.lambdas.ci_broker_role_arn + ci_broker_function_arn = module.lambdas.ci_broker_function_arn + plan_review_function_arn = module.lambdas.plan_review_function_arn } module "compute" {