From 757af91339aed0e9c1cf4ae6828779669e5a52a0 Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Fri, 23 Oct 2026 16:09:40 +0200 Subject: [PATCH 1/6] fix(iam): per-team ECS execution roles and lock teams out of shared SGs - The shared javabin-ecs-execution role could read every /javabin/apps/* parameter and every team could pass it, so any team could mount another team's secrets in its own task definition. Each registered team now gets javabin-ecs-exec-{team}, reading only its own path; the developer boundary only lets a team pass its own. The shared role keeps image pull and logs only. - DenyShared did not cover ec2:AuthorizeSecurityGroup*/RevokeSecurityGroup*, so any team CI role could open every app to every other or remove the ALB rule on the shared ECS tasks SG (confirmed with the IAM simulator). Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/generate-modules.py | 1 + terraform/modules/platform-data/main.tf | 4 +- terraform/modules/platform-data/outputs.tf | 2 +- terraform/modules/platform-data/variables.tf | 5 ++ terraform/platform/iam/boundary.tf | 4 +- terraform/platform/iam/main.tf | 75 ++++++++++++++++---- 6 files changed, 73 insertions(+), 18 deletions(-) diff --git a/scripts/generate-modules.py b/scripts/generate-modules.py index da71a34..53f1694 100755 --- a/scripts/generate-modules.py +++ b/scripts/generate-modules.py @@ -271,6 +271,7 @@ def src(mod): blocks.append(module_block("platform", src("platform-data"), { "project": PROJECT, "domain": DOMAIN, + "team": team, })) # ECR (always) diff --git a/terraform/modules/platform-data/main.tf b/terraform/modules/platform-data/main.tf index 9228dc2..4be9be8 100644 --- a/terraform/modules/platform-data/main.tf +++ b/terraform/modules/platform-data/main.tf @@ -58,8 +58,10 @@ data "aws_ecs_cluster" "main" { cluster_name = "${var.project}-platform" } +# Per-team execution role (created by platform CI for every registered team); +# it can read only //apps//* secrets. data "aws_iam_role" "ecs_execution" { - name = "${var.project}-ecs-execution" + name = "${var.project}-ecs-exec-${var.team}" } data "aws_route53_zone" "main" { diff --git a/terraform/modules/platform-data/outputs.tf b/terraform/modules/platform-data/outputs.tf index 068bc7f..23ac52c 100644 --- a/terraform/modules/platform-data/outputs.tf +++ b/terraform/modules/platform-data/outputs.tf @@ -54,7 +54,7 @@ output "ecs_cluster_name" { } output "execution_role_arn" { - description = "ECS execution role ARN (shared across all services)" + description = "The team's ECS execution role ARN (reads only that team's secrets)" value = data.aws_iam_role.ecs_execution.arn } diff --git a/terraform/modules/platform-data/variables.tf b/terraform/modules/platform-data/variables.tf index 56e4f00..b96243d 100644 --- a/terraform/modules/platform-data/variables.tf +++ b/terraform/modules/platform-data/variables.tf @@ -15,3 +15,8 @@ variable "domain" { type = string default = "javazone.no" } + +variable "team" { + description = "Owning team — selects the team's ECS execution role" + type = string +} diff --git a/terraform/platform/iam/boundary.tf b/terraform/platform/iam/boundary.tf index 68c29c0..8ed9378 100644 --- a/terraform/platform/iam/boundary.tf +++ b/terraform/platform/iam/boundary.tf @@ -149,6 +149,8 @@ resource "aws_iam_policy" "developer_boundary" { "iam:DeleteRole", "iam:UpdateRole", "iam:DeleteRolePolicy", "iam:PutRolePolicy", "iam:AttachRolePolicy", "iam:DetachRolePolicy", "route53:DeleteHostedZone", "sns:DeleteTopic", "sns:SetTopicAttributes", + # Shared SGs (ECS tasks, ALB): a team rule here opens or cuts every app + "ec2:AuthorizeSecurityGroup*", "ec2:RevokeSecurityGroup*", ] Resource = "*" Condition = { StringEquals = { "aws:ResourceTag/team" = "shared" } } @@ -184,7 +186,7 @@ resource "aws_iam_policy" "developer_boundary" { Action = "iam:PassRole" NotResource = [ "arn:aws:iam::*:role/$${aws:PrincipalTag/team}-*", - "arn:aws:iam::*:role/${var.project}-ecs-execution", + "arn:aws:iam::*:role/${var.project}-ecs-exec-$${aws:PrincipalTag/team}", ] }, { diff --git a/terraform/platform/iam/main.tf b/terraform/platform/iam/main.tf index 2e8d90b..f98fc12 100644 --- a/terraform/platform/iam/main.tf +++ b/terraform/platform/iam/main.tf @@ -522,7 +522,7 @@ resource "aws_iam_role_policy" "ci_deploy_ecs" { Effect = "Allow" Action = ["iam:PassRole"] Resource = [ - aws_iam_role.ecs_execution.arn, + aws_iam_role.ecs_exec_team[each.key].arn, "arn:aws:iam::${var.aws_account_id}:role/${each.key}-*", ] } @@ -754,6 +754,8 @@ resource "aws_iam_role_policy" "ci_apply_gate" { # Individual task roles are created per-app via the service-role module. ################################################################################ +# Deprecated shared execution role. Kept (image pull + logs only, no secrets) +# until no task definition references it; teams can no longer pass it. resource "aws_iam_role" "ecs_execution" { name = "${var.project}-ecs-execution" @@ -781,25 +783,68 @@ resource "aws_iam_role_policy_attachment" "ecs_execution_base" { policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy" } -resource "aws_iam_role_policy" "ecs_execution_secrets" { - name = "secrets-read" - role = aws_iam_role.ecs_execution.id +################################################################################ +# Per-team ECS execution roles — javabin-ecs-exec-{team} +# +# The execution role pulls the image and resolves the task's `secrets` at +# launch. One shared role that can read every //apps/* parameter let +# any team mount another team's secrets in its own task definition. Each team +# now gets a role that reads only its own path, and the developer boundary's +# DenyPassRole only lets a team pass its own role. +# +# Tagged team=shared so DenyShared/DenyTagStrip stop teams from editing them. +################################################################################ - # //apps///... holds service-secret params and the - # Cognito client credentials written by app-auth-sync. The execution role - # fetches them at task launch via the ECS `secrets` block. - policy = jsonencode({ +resource "aws_iam_role" "ecs_exec_team" { + for_each = toset(var.registered_teams) + + name = "${var.project}-ecs-exec-${each.key}" + # The org boundary denies creating any role without one + permissions_boundary = local.org_boundary_arn + + assume_role_policy = jsonencode({ Version = "2012-10-17" Statement = [ { - Sid = "SSMAppSecrets" Effect = "Allow" - Action = [ - "ssm:GetParameters", - ] - Resource = [ - "arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter/${var.project}/apps/*", - ] + Principal = { + Service = "ecs-tasks.amazonaws.com" + } + Action = "sts:AssumeRole" + } + ] + }) + + tags = { + Name = "${var.project}-ecs-exec-${each.key}" + team = "shared" + for-team = each.key + } +} + +resource "aws_iam_role_policy_attachment" "ecs_exec_team_base" { + for_each = toset(var.registered_teams) + + role = aws_iam_role.ecs_exec_team[each.key].name + policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy" +} + +resource "aws_iam_role_policy" "ecs_exec_team_secrets" { + for_each = toset(var.registered_teams) + + name = "team-secrets-read" + role = aws_iam_role.ecs_exec_team[each.key].id + + # Service-secret params and Cognito client credentials (app-auth-sync) + # under the team's own path only. + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Sid = "SSMTeamSecrets" + Effect = "Allow" + Action = ["ssm:GetParameters"] + Resource = "arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter/${var.project}/apps/${each.key}/*" } ] }) From 84527012793ee045b78f907cd6b5e0cc65fd448f Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Fri, 23 Oct 2026 16:09:40 +0200 Subject: [PATCH 2/6] fix(ci): identify callers of ci-broker and apply-gate by GitHub OIDC Both Lambdas took the repo from the request payload, and the gate's sign action (approve a HIGH-risk plan) was open to any caller. Code running in an app's platform job (a Terraform external data source, say) could claim another repo's team credentials or sign its own override. Callers now send a GitHub OIDC token; the Lambdas verify it against GitHub's JWKS (RS256 via openssl, no new dependencies) and read the repo from its signed claims. check also requires the plan to be the repo's own, and sign requires a token from approve-override.yml in javaBin/platform in the override-approval environment, recording the token's actor. The override workflow passes inputs via env instead of interpolating them. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/approve-override.yml | 27 ++-- docs/apply-gate.md | 22 ++- scripts/github-oidc-token.sh | 16 ++ scripts/invoke-apply-gate.sh | 5 +- scripts/invoke-ci-broker.sh | 7 +- scripts/invoke-override-sign.sh | 26 +++ terraform/lambda-src/apply_gate/handler.py | 35 +++- terraform/lambda-src/ci_broker/handler.py | 14 +- terraform/lambda-src/shared/github_oidc.py | 150 ++++++++++++++++++ .../lambda-src/tests/test_github_oidc.py | 105 ++++++++++++ terraform/platform/lambdas/main.tf | 8 + 11 files changed, 392 insertions(+), 23 deletions(-) create mode 100644 scripts/github-oidc-token.sh create mode 100644 scripts/invoke-override-sign.sh create mode 100644 terraform/lambda-src/shared/github_oidc.py create mode 100644 terraform/lambda-src/tests/test_github_oidc.py diff --git a/.github/workflows/approve-override.yml b/.github/workflows/approve-override.yml index 998f3fd..bca1177 100644 --- a/.github/workflows/approve-override.yml +++ b/.github/workflows/approve-override.yml @@ -48,25 +48,26 @@ jobs: role-to-assume: arn:aws:iam::${{ env.AWS_ACCOUNT_ID }}:role/javabin-ci-override-approver aws-region: ${{ env.AWS_REGION }} + # The gate verifies this job's GitHub OIDC token (workflow, repo and + # override-approval environment) and records its actor as approver. + # Inputs go through env, never into the script text. - name: Sign override via gate Lambda - run: | - aws lambda invoke \ - --function-name javabin-apply-gate \ - --payload "$(jq -n \ - --arg action sign \ - --arg plan_key "${{ inputs.plan_key }}" \ - --arg approved_by "${{ github.actor }}" \ - --arg reason "${{ inputs.reason }}" \ - '{action: $action, plan_key: $plan_key, approved_by: $approved_by, reason: $reason}')" \ - --cli-binary-format raw-in-base64-out \ - /dev/stdout + env: + PLAN_KEY: ${{ inputs.plan_key }} + REASON: ${{ inputs.reason }} + run: sh scripts/invoke-override-sign.sh - name: Notify Slack env: SSM_WEBHOOK_PARAM: /javabin/slack/platform-override-alerts-webhook - run: sh scripts/notify-slack.sh "Risk Override Approved" "*Repo:* ${{ inputs.repo }}\n*Plan:* \`${{ inputs.plan_key }}\`\n*By:* ${{ github.actor }}\n*Reason:* ${{ inputs.reason }}" + REPO: ${{ inputs.repo }} + PLAN_KEY: ${{ inputs.plan_key }} + REASON: ${{ inputs.reason }} + run: sh scripts/notify-slack.sh "Risk Override Approved" "*Repo:* ${REPO}\n*Plan:* \`${PLAN_KEY}\`\n*By:* ${GITHUB_ACTOR}\n*Reason:* ${REASON}" - name: Retrigger failed apply env: GH_TOKEN: ${{ github.token }} - run: gh run rerun ${{ inputs.run_id }} --repo ${{ inputs.repo }} --failed + RUN_ID: ${{ inputs.run_id }} + REPO: ${{ inputs.repo }} + run: gh run rerun "$RUN_ID" --repo "$REPO" --failed diff --git a/docs/apply-gate.md b/docs/apply-gate.md index 5c0996f..e014c27 100644 --- a/docs/apply-gate.md +++ b/docs/apply-gate.md @@ -20,7 +20,10 @@ tf-plan → uploads plan + plan-output.txt to S3 plan-review → LLM reviews plan → uploads risk.json to S3 tf-apply → assumes javabin-ci-apply-gate (lightweight OIDC role) → downloads plan from S3, verifies SHA256 - → invokes gate Lambda: check(plan_key, repo_name) + → invokes gate Lambda: check(plan_key, team, id_token) + → Lambda verifies the GitHub OIDC token (aud javabin-apply-gate, + a javaBin/platform workflow on main) and takes the repo from its + signed `repository` claim; plan_key must be under plans/javaBin// → Lambda reads risk.json from S3 → LOW/MEDIUM: assumes app CI role via STS, returns temp credentials → HIGH: checks for override.json, verifies HMAC signature @@ -29,6 +32,23 @@ tf-apply → assumes javabin-ci-apply-gate (lightweight OIDC role) → tf-apply uses temp credentials for terraform init + apply ``` +## Caller identity + +The gate and broker roles can be assumed from any javaBin repo's platform +workflow job, and code inside that job (e.g. a Terraform `external` data +source) can invoke the Lambdas directly. So nothing identity-related is taken +from the payload: + +- `check` — repo comes from the job's GitHub OIDC token, the team must be the + app.yaml team the repo belongs to, and the plan must be the repo's own. +- `sign` — only a token from `approve-override.yml` in javaBin/platform, + running in the board-reviewed `override-approval` environment, is accepted. + The approver is the token's `actor`. Without this, any app's tf-apply job + could sign its own HIGH-risk override. + +Tokens are minted with `scripts/github-oidc-token.sh ` and verified +in `terraform/lambda-src/shared/github_oidc.py` (RS256 against GitHub's JWKS). + ## IAM Roles | Role | Purpose | Permissions | diff --git a/scripts/github-oidc-token.sh b/scripts/github-oidc-token.sh new file mode 100644 index 0000000..dde09b4 --- /dev/null +++ b/scripts/github-oidc-token.sh @@ -0,0 +1,16 @@ +#!/bin/sh +# Print a GitHub Actions OIDC token for the given audience. +# +# Usage: github-oidc-token.sh +# +# The job needs `permissions: id-token: write`. The platform Lambdas +# (ci-broker, apply-gate) verify the token and read the repo from its signed +# claims, so a job can only ever act as its own repo. + +set -e + +AUDIENCE="${1:?Usage: github-oidc-token.sh }" +: "${ACTIONS_ID_TOKEN_REQUEST_URL:?id-token: write permission is required}" + +curl -sSf -H "Authorization: bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" \ + "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=${AUDIENCE}" | jq -r '.value' diff --git a/scripts/invoke-apply-gate.sh b/scripts/invoke-apply-gate.sh index 528acbf..6cc9473 100644 --- a/scripts/invoke-apply-gate.sh +++ b/scripts/invoke-apply-gate.sh @@ -26,13 +26,16 @@ echo "Requesting apply credentials from gate Lambda..." # Build session name from GitHub context for alert attribution GATE_SESSION="${GITHUB_ACTOR:-unknown}-$(echo "${GITHUB_SHA:-00000000}" | cut -c1-8)-${GITHUB_RUN_ID:-0}" +ID_TOKEN=$(sh "$(dirname "$0")/github-oidc-token.sh" "${PROJECT:-javabin}-apply-gate") + PAYLOAD=$(jq -n \ --arg action "check" \ --arg plan_key "$PLAN_KEY" \ --arg repo_name "$REPO_NAME" \ --arg session_name "$GATE_SESSION" \ --arg team "$TEAM" \ - '{action: $action, plan_key: $plan_key, repo_name: $repo_name, session_name: $session_name, team: $team}') + --arg id_token "$ID_TOKEN" \ + '{action: $action, plan_key: $plan_key, repo_name: $repo_name, session_name: $session_name, team: $team, id_token: $id_token}') RESPONSE=$(aws lambda invoke \ --function-name "$LAMBDA_NAME" \ diff --git a/scripts/invoke-ci-broker.sh b/scripts/invoke-ci-broker.sh index 0829ac7..45e76e1 100755 --- a/scripts/invoke-ci-broker.sh +++ b/scripts/invoke-ci-broker.sh @@ -4,7 +4,8 @@ # Usage: invoke-ci-broker.sh # action: "plan" or "deploy" # -# Env: GITHUB_REPOSITORY (org/repo) +# Env: GITHUB_REPOSITORY (org/repo, informational — the broker takes the +# repo from the job's OIDC token; needs id-token: write) # APP_TEAM (optional) — team from app.yaml; the broker verifies the repo # belongs to it instead of picking the repo's first GitHub team # Output: Exports AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN @@ -14,13 +15,15 @@ set -e ACTION="${1:?Usage: invoke-ci-broker.sh }" REPO="${GITHUB_REPOSITORY##*/}" +ID_TOKEN=$(sh "$(dirname "$0")/github-oidc-token.sh" "${PROJECT:-javabin}-ci-broker") echo "Requesting ${ACTION} credentials for ${REPO}..." RESPONSE=$(aws lambda invoke \ --function-name "${PROJECT:-javabin}-ci-broker" \ --payload "$(jq -nc --arg repo "$REPO" --arg action "$ACTION" --arg session_name "${SESSION_NAME:-}" --arg team "${APP_TEAM:-}" \ - '{repo: $repo, action: $action, session_name: $session_name, team: $team}')" \ + --arg id_token "$ID_TOKEN" \ + '{repo: $repo, action: $action, session_name: $session_name, team: $team, id_token: $id_token}')" \ --cli-binary-format raw-in-base64-out \ /tmp/broker-response.json 2>&1) diff --git a/scripts/invoke-override-sign.sh b/scripts/invoke-override-sign.sh new file mode 100644 index 0000000..0bbd8a8 --- /dev/null +++ b/scripts/invoke-override-sign.sh @@ -0,0 +1,26 @@ +#!/bin/sh +# Ask the apply-gate Lambda to sign a risk override for a plan. +# +# Usage: invoke-override-sign.sh +# Env: PLAN_KEY, REASON; runs in approve-override.yml (the gate checks the +# OIDC token comes from that workflow and the override-approval +# environment, and records the token's actor as approver). + +set -e + +DIR=$(dirname "$0") +ID_TOKEN=$(sh "$DIR/github-oidc-token.sh" "${PROJECT:-javabin}-apply-gate") + +aws lambda invoke \ + --function-name "${PROJECT:-javabin}-apply-gate" \ + --payload "$(jq -nc --arg plan_key "$PLAN_KEY" --arg reason "$REASON" --arg id_token "$ID_TOKEN" \ + '{action: "sign", plan_key: $plan_key, reason: $reason, id_token: $id_token}')" \ + --cli-binary-format raw-in-base64-out \ + /tmp/override-response.json >/dev/null + +BODY=$(jq -r '.body' /tmp/override-response.json) +if [ "$(echo "$BODY" | jq -r '.signed // false')" != "true" ]; then + echo "ERROR: override not signed: $(echo "$BODY" | jq -r '.error // "unknown error"')" + exit 1 +fi +echo "Override signed for $PLAN_KEY" diff --git a/terraform/lambda-src/apply_gate/handler.py b/terraform/lambda-src/apply_gate/handler.py index 487e7b1..dfd5624 100644 --- a/terraform/lambda-src/apply_gate/handler.py +++ b/terraform/lambda-src/apply_gate/handler.py @@ -19,6 +19,9 @@ import boto3 from shared.github import resolve_team +from shared.github_oidc import TokenError, require_workflow +from shared.github_oidc import repo_name as oidc_repo_name +from shared.github_oidc import verify as verify_oidc logger = logging.getLogger(__name__) logger.setLevel(logging.INFO) @@ -32,6 +35,8 @@ ) PLAN_BUCKET = os.environ.get("PLAN_BUCKET", "") PROJECT = os.environ.get("PROJECT", "javabin") +GITHUB_ORG = os.environ.get("GITHUB_ORG", "javaBin") +OIDC_AUDIENCE = os.environ.get("OIDC_AUDIENCE", f"{PROJECT}-apply-gate") CREDENTIAL_DURATION = int(os.environ.get("CREDENTIAL_DURATION", "900")) # 15 min # Cache signing key across invocations @@ -87,8 +92,17 @@ def action_check(event): Output: {approved, risk_level, credentials?, reason?} """ plan_key = event["plan_key"] - repo_name = event["repo_name"] team = event.get("team") or None + # Repo from the signed GitHub OIDC token, not the payload (see ci_broker) + try: + repo_name = oidc_repo_name(verify_oidc(event.get("id_token"), OIDC_AUDIENCE)) + except TokenError as e: + logger.warning("Rejected gate call: %s", e) + return {"approved": False, "reason": f"Invalid GitHub OIDC token: {e}"} + # Plans are stored under plans///...; never apply another + # repo's (possibly LOW-risk or overridden) plan. + if not plan_key.startswith(f"plans/{GITHUB_ORG}/{repo_name}/"): + return {"approved": False, "reason": f"Plan {plan_key} does not belong to {repo_name}"} session_name = event.get("session_name") prefix = _plan_prefix(plan_key) @@ -153,11 +167,26 @@ def action_check(event): def action_sign(event): """Create a signed override token and write it to S3. - Input: {plan_key, approved_by, reason} + Input: {plan_key, reason, id_token} Output: {signed: true, plan_key} + + Only the approve-override workflow in javaBin/platform, running behind + the board-reviewed override-approval environment, may sign. Without this + check any app's tf-apply job could invoke the gate and approve its own + HIGH-risk plan. The approver is the token's actor, not a payload field. """ plan_key = event["plan_key"] - approved_by = event["approved_by"] + try: + claims = require_workflow( + verify_oidc(event.get("id_token"), OIDC_AUDIENCE), + "approve-override.yml", + repository=f"{GITHUB_ORG}/platform", + environment="override-approval", + ) + except TokenError as e: + logger.warning("Rejected override signing: %s", e) + return {"signed": False, "error": f"Invalid GitHub OIDC token: {e}"} + approved_by = claims["actor"] reason = event.get("reason", "") prefix = _plan_prefix(plan_key) diff --git a/terraform/lambda-src/ci_broker/handler.py b/terraform/lambda-src/ci_broker/handler.py index af56074..20e211b 100644 --- a/terraform/lambda-src/ci_broker/handler.py +++ b/terraform/lambda-src/ci_broker/handler.py @@ -16,6 +16,7 @@ import boto3 from shared.github import resolve_team +from shared.github_oidc import TokenError, repo_name, verify logger = logging.getLogger(__name__) logger.setLevel(logging.INFO) @@ -57,13 +58,20 @@ def _assume_role(role_arn, session_name, duration): } +OIDC_AUDIENCE = os.environ.get("OIDC_AUDIENCE", f"{PROJECT}-ci-broker") + + def handler(event, context): - repo = event.get("repo", "") action = event.get("action", "plan") requested_team = event.get("team") or None - if not repo: - return {"error": "repo is required", "approved": False} + # The repo comes from a GitHub OIDC token, never from the payload: code + # running inside an app job can invoke this Lambda and name any repo. + try: + repo = repo_name(verify(event.get("id_token"), OIDC_AUDIENCE)) + except TokenError as e: + logger.warning("Rejected broker call: %s", e) + return {"error": f"Invalid GitHub OIDC token: {e}", "approved": False} if action not in ROLE_MAP: return {"error": f"Invalid action: {action}. Must be plan or deploy", "approved": False} diff --git a/terraform/lambda-src/shared/github_oidc.py b/terraform/lambda-src/shared/github_oidc.py new file mode 100644 index 0000000..d53dda7 --- /dev/null +++ b/terraform/lambda-src/shared/github_oidc.py @@ -0,0 +1,150 @@ +"""Verify GitHub Actions OIDC tokens — shared by ci_broker and apply_gate. + +The broker and gate roles are assumable from any javaBin repo running a +platform workflow, and code running inside that job (e.g. a Terraform +`external` data source during tf-plan) can call the Lambdas too. So the repo +must not come from the request payload: the caller sends a GitHub OIDC token +minted for this audience, and the repo is read from its signed `repository` +claim, which the job cannot forge. + +RS256 is verified with the openssl CLI (in the Lambda runtime), so no +third-party packages are needed. +""" + +import base64 +import json +import os +import subprocess +import tempfile +import time +import urllib.request + +ISSUER = "https://token.actions.githubusercontent.com" +JWKS_URL = f"{ISSUER}/.well-known/jwks" +GITHUB_ORG = os.environ.get("GITHUB_ORG", "javaBin") +PLATFORM_WORKFLOW_PREFIX = f"{GITHUB_ORG}/platform/.github/workflows/" +CLOCK_SKEW = 60 + +_jwks_cache = {"keys": None, "fetched_at": 0} + + +class TokenError(ValueError): + """The token is missing, malformed, expired, or not from a platform workflow.""" + + +def _b64url_decode(s): + return base64.urlsafe_b64decode(s + "=" * (-len(s) % 4)) + + +def _der_len(n): + if n < 0x80: + return bytes([n]) + b = n.to_bytes((n.bit_length() + 7) // 8, "big") + return bytes([0x80 | len(b)]) + b + + +def _der_int(b): + if b[0] & 0x80: + b = b"\x00" + b + return b"\x02" + _der_len(len(b)) + b + + +def _der_seq(*parts): + body = b"".join(parts) + return b"\x30" + _der_len(len(body)) + body + + +def jwk_to_pem(jwk): + """Encode an RSA JWK (n, e) as a SubjectPublicKeyInfo PEM.""" + rsa_key = _der_seq(_der_int(_b64url_decode(jwk["n"])), _der_int(_b64url_decode(jwk["e"]))) + algorithm = _der_seq(b"\x06\x09\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01", b"\x05\x00") # rsaEncryption, NULL + bit_string = b"\x03" + _der_len(len(rsa_key) + 1) + b"\x00" + rsa_key + der = _der_seq(algorithm, bit_string) + b64 = base64.b64encode(der).decode() + lines = [b64[i:i + 64] for i in range(0, len(b64), 64)] + return "-----BEGIN PUBLIC KEY-----\n" + "\n".join(lines) + "\n-----END PUBLIC KEY-----\n" + + +def _jwks(force=False): + if force or not _jwks_cache["keys"] or time.time() - _jwks_cache["fetched_at"] > 3600: + with urllib.request.urlopen(JWKS_URL, timeout=10) as resp: + _jwks_cache["keys"] = {k["kid"]: k for k in json.loads(resp.read())["keys"]} + _jwks_cache["fetched_at"] = time.time() + return _jwks_cache["keys"] + + +def _verify_rs256(signing_input, signature, jwk): + with tempfile.NamedTemporaryFile("w", suffix=".pem", delete=False) as k, \ + tempfile.NamedTemporaryFile("wb", suffix=".sig", delete=False) as sig: + k.write(jwk_to_pem(jwk)) + sig.write(signature) + try: + r = subprocess.run( + ["openssl", "dgst", "-sha256", "-verify", k.name, "-signature", sig.name], + input=signing_input, capture_output=True, + ) + return r.returncode == 0 + finally: + os.unlink(k.name) + os.unlink(sig.name) + + +def verify(token, audience, now=None, keys=None): + """Return the verified claims, or raise TokenError. + + Accepts tokens from any javaBin/platform workflow on main; callers that + need a specific workflow use require_workflow() on the result. + """ + if not token or token.count(".") != 2: + raise TokenError("missing or malformed GitHub OIDC token") + header_b64, payload_b64, sig_b64 = token.split(".") + try: + header = json.loads(_b64url_decode(header_b64)) + claims = json.loads(_b64url_decode(payload_b64)) + signature = _b64url_decode(sig_b64) + except ValueError as e: + raise TokenError(f"undecodable token: {e}") + + if header.get("alg") != "RS256": + raise TokenError(f"unsupported alg {header.get('alg')!r}") + jwks = keys if keys is not None else _jwks() + jwk = jwks.get(header.get("kid")) + if jwk is None and keys is None: + jwk = _jwks(force=True).get(header.get("kid")) # key rotation + if jwk is None: + raise TokenError("unknown signing key") + if not _verify_rs256(f"{header_b64}.{payload_b64}".encode(), signature, jwk): + raise TokenError("bad signature") + + now = now if now is not None else time.time() + if claims.get("iss") != ISSUER: + raise TokenError("wrong issuer") + aud = claims.get("aud") + if audience not in (aud if isinstance(aud, list) else [aud]): + raise TokenError("wrong audience") + if claims.get("exp", 0) + CLOCK_SKEW < now or claims.get("nbf", 0) - CLOCK_SKEW > now: + raise TokenError("token expired or not yet valid") + if claims.get("repository_owner") != GITHUB_ORG: + raise TokenError("token is not from the javaBin org") + if not str(claims.get("job_workflow_ref", "")).startswith(PLATFORM_WORKFLOW_PREFIX) or \ + not str(claims.get("job_workflow_ref", "")).endswith("@refs/heads/main"): + raise TokenError("token is not from a javaBin/platform workflow on main") + return claims + + +def require_workflow(claims, workflow, repository=None, environment=None): + """Raise TokenError unless the token came from this platform workflow + file (e.g. "approve-override.yml"), optionally in a given repo and + GitHub environment.""" + if claims.get("job_workflow_ref") != f"{PLATFORM_WORKFLOW_PREFIX}{workflow}@refs/heads/main": + raise TokenError(f"token is not from {workflow}") + if repository and claims.get("repository") != repository: + raise TokenError(f"token is not from {repository}") + if environment and claims.get("environment") != environment: + raise TokenError(f"token is not from the {environment} environment") + return claims + + +def repo_name(claims): + """Repo name (without org) from verified claims.""" + return claims["repository"].split("/", 1)[1] diff --git a/terraform/lambda-src/tests/test_github_oidc.py b/terraform/lambda-src/tests/test_github_oidc.py new file mode 100644 index 0000000..7658c67 --- /dev/null +++ b/terraform/lambda-src/tests/test_github_oidc.py @@ -0,0 +1,105 @@ +"""Tests for shared.github_oidc — run: python3 -m unittest discover terraform/lambda-src/tests + +Signs tokens with a throwaway RSA key via the openssl CLI, so the real +verification path (JWK -> PEM -> openssl dgst -verify) is exercised. +""" + +import base64 +import json +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from shared import github_oidc as oidc # noqa: E402 + +NOW = 1_800_000_000 +AUD = "javabin-ci-broker" + + +def b64url(b): + return base64.urlsafe_b64encode(b).rstrip(b"=").decode() + + +@unittest.skipUnless(shutil.which("openssl"), "openssl not installed") +class GithubOidcTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.dir = tempfile.mkdtemp() + cls.key = os.path.join(cls.dir, "key.pem") + subprocess.run(["openssl", "genrsa", "-out", cls.key, "2048"], check=True, capture_output=True) + mod = subprocess.run(["openssl", "rsa", "-in", cls.key, "-noout", "-modulus"], + check=True, capture_output=True, text=True).stdout.strip().split("=")[1] + cls.jwks = {"k1": {"kid": "k1", "kty": "RSA", "e": b64url((65537).to_bytes(3, "big")), + "n": b64url(bytes.fromhex(mod))}} + + @classmethod + def tearDownClass(cls): + shutil.rmtree(cls.dir) + + def token(self, **overrides): + claims = { + "iss": oidc.ISSUER, "aud": AUD, "exp": NOW + 300, "nbf": NOW - 10, + "repository": "javaBin/cupcake", "repository_owner": "javaBin", "actor": "someone", + "job_workflow_ref": "javaBin/platform/.github/workflows/tf-plan.yml@refs/heads/main", + } + claims.update(overrides) + signing_input = f"{b64url(json.dumps({'alg': 'RS256', 'kid': 'k1'}).encode())}.{b64url(json.dumps(claims).encode())}" + sig = subprocess.run(["openssl", "dgst", "-sha256", "-sign", self.key], + input=signing_input.encode(), check=True, capture_output=True).stdout + return f"{signing_input}.{b64url(sig)}" + + def verify(self, token, audience=AUD): + return oidc.verify(token, audience, now=NOW, keys=self.jwks) + + def assertRejected(self, token, fragment, audience=AUD): + with self.assertRaises(oidc.TokenError) as cm: + self.verify(token, audience) + self.assertIn(fragment, str(cm.exception)) + + def test_valid_token_yields_repo_from_signed_claims(self): + self.assertEqual(oidc.repo_name(self.verify(self.token())), "cupcake") + + def test_tampered_payload_fails_signature(self): + head, _, sig = self.token().split(".") + forged = b64url(json.dumps({"iss": oidc.ISSUER, "aud": AUD, "exp": NOW + 300, + "repository": "javaBin/moresleep", "repository_owner": "javaBin", + "job_workflow_ref": "javaBin/platform/.github/workflows/tf-plan.yml@refs/heads/main"}).encode()) + self.assertRejected(f"{head}.{forged}.{sig}", "bad signature") + + def test_wrong_audience_expired_and_foreign_org_are_rejected(self): + self.assertRejected(self.token(), "wrong audience", audience="javabin-apply-gate") + self.assertRejected(self.token(exp=NOW - 120), "expired") + self.assertRejected(self.token(repository_owner="evil", repository="evil/cupcake"), "javaBin org") + + def test_token_from_app_repo_workflow_is_rejected(self): + # A token minted by the app's own workflow, not a platform reusable workflow + self.assertRejected(self.token(job_workflow_ref="javaBin/cupcake/.github/workflows/deploy.yaml@refs/heads/main"), + "javaBin/platform workflow") + self.assertRejected(self.token(job_workflow_ref="javaBin/platform/.github/workflows/tf-plan.yml@refs/heads/evil"), + "javaBin/platform workflow") + + def test_missing_or_garbage_token(self): + self.assertRejected(None, "missing") + self.assertRejected("a.b", "missing") + + def test_require_workflow_for_override_signing(self): + ok = self.verify(self.token(repository="javaBin/platform", environment="override-approval", + job_workflow_ref="javaBin/platform/.github/workflows/approve-override.yml@refs/heads/main")) + oidc.require_workflow(ok, "approve-override.yml", "javaBin/platform", "override-approval") + + tf_apply = self.verify(self.token(job_workflow_ref="javaBin/platform/.github/workflows/tf-apply.yml@refs/heads/main")) + with self.assertRaises(oidc.TokenError): + oidc.require_workflow(tf_apply, "approve-override.yml", "javaBin/platform", "override-approval") + + no_env = self.verify(self.token(repository="javaBin/platform", + job_workflow_ref="javaBin/platform/.github/workflows/approve-override.yml@refs/heads/main")) + with self.assertRaises(oidc.TokenError): + oidc.require_workflow(no_env, "approve-override.yml", "javaBin/platform", "override-approval") + + +if __name__ == "__main__": + unittest.main() diff --git a/terraform/platform/lambdas/main.tf b/terraform/platform/lambdas/main.tf index e6c8471..5a02b73 100644 --- a/terraform/platform/lambdas/main.tf +++ b/terraform/platform/lambdas/main.tf @@ -1383,6 +1383,10 @@ data "archive_file" "apply_gate" { content = file("${local.lambda_src_path}/shared/github.py") filename = "shared/github.py" } + source { + content = file("${local.lambda_src_path}/shared/github_oidc.py") + filename = "shared/github_oidc.py" + } } resource "aws_iam_role" "apply_gate" { @@ -1487,6 +1491,10 @@ data "archive_file" "ci_broker" { content = file("${local.lambda_src_path}/shared/github.py") filename = "shared/github.py" } + source { + content = file("${local.lambda_src_path}/shared/github_oidc.py") + filename = "shared/github_oidc.py" + } } resource "aws_iam_role" "ci_broker" { From c9f16de0593e9049c667cf92a1aa88ec4dccd67a Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Fri, 23 Oct 2026 16:09:40 +0200 Subject: [PATCH 3/6] fix(ci): create team CI roles only for registry teams sync-registered-teams listed every GitHub org team, so non-app teams like owners and developer got AWS CI and deploy roles. Use teams/*.yaml in javaBin/registry, the documented source of truth. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 8 +++--- scripts/sync-registered-teams.py | 26 ++++++++++++------- .../platform/registered-teams.auto.tfvars | 4 ++- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ae9cf2b..f9260c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -168,7 +168,7 @@ terraform/state/ | `password-set` | Self-service password-set for new hero accounts (Function URL) | | `budget-enforcer` | Scales ECS services to zero when team exceeds 200% budget | | `resource-tagger` | EventBridge-triggered, auto-tags created-by + commit on new resources | -| `ci-broker` | Validates repo ∈ app.yaml team, vends short-lived team role credentials | +| `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) | ### Scripts @@ -178,7 +178,8 @@ terraform/state/ | `scripts/generate-modules.py` | CI: validates app.yaml, generates fmt-clean module blocks into terraform/ (tests: `scripts/tests/`) | | `scripts/registry.py` | Module sources + boilerplate templates for generate-modules.py | | `scripts/resolve-tool-version.sh` | CI: resolve node/java version from the app repo's version files | -| `scripts/sync-registered-teams.py` | CI: GitHub org teams → `registered-teams.auto.tfvars` (needs App token) | +| `scripts/sync-registered-teams.py` | CI: registry `teams/*.yaml` → `registered-teams.auto.tfvars` (needs App token) | +| `scripts/github-oidc-token.sh` | CI: mint a GitHub OIDC token for the broker / apply gate | | `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 | @@ -202,6 +203,7 @@ terraform/state/ | SSM params (platform) | `/javabin/{namespace}/{name}` | | SSM params (app) | `/javabin/apps/{team}/{service}/{name}` | | ECS cluster | `javabin-platform` | +| ECS execution roles | `javabin-ecs-exec-{team}` (platform-owned, reads only `/javabin/apps/{team}/*`) | | ECR repos | `{team}-{service}` | | SNS topics | `javabin-{alerts,security,budget-enforcement}` | | Log groups | `/ecs/{team}/{service}` | @@ -287,7 +289,7 @@ The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation tar ### Known Issues - **ECS deploy stabilization**: platform-test-app task registers but service fails health check - **Cognito**: pools deployed; internal pool's `GoogleSAML` IdP was created outside Terraform -- **CI broker trusts the payload repo name**: code running inside an app job (e.g. a Terraform `external` data source in tf-plan) can mint a new OIDC token for the broker role and name another repo. Fix: verify a GitHub OIDC token in the Lambda instead of a payload field +- **App databases share one network path**: service-rds allows ingress from the shared ECS tasks SG, so every team's tasks can reach every team's DB port (password still required). Fix: per-app security groups - **Team provisioner Lambda**: All sync functions working (Google/GitHub/Budget/Cognito/Identity Center). Password-set flow deployed. - **`registered_app_repos` manually managed**: Being replaced with team-scoped IAM roles (repo→team resolved via GitHub API at runtime) - **Cost allocation tags pending activation**: `repo`, `created-by`, `commit` tags need activation in Billing console (requires billing data to appear first) diff --git a/scripts/sync-registered-teams.py b/scripts/sync-registered-teams.py index d1c608f..713dc6d 100644 --- a/scripts/sync-registered-teams.py +++ b/scripts/sync-registered-teams.py @@ -1,19 +1,22 @@ #!/usr/bin/env python3 -"""Sync registered teams from GitHub org teams. +"""Sync registered teams from the registry. -Queries the GitHub API for all teams in the javaBin org and writes -registered-teams.auto.tfvars. This drives team-scoped IAM role creation. +Lists teams/*.yaml in javaBin/registry and writes +registered-teams.auto.tfvars. This drives team-scoped IAM role creation +(CI roles, ECS execution roles). -Source of truth: GitHub teams (created by team-provisioner from registry). +Source of truth: the registry. GitHub org teams also include non-app teams +(owners, developer, ...) that must not get AWS CI roles. Usage: sync-registered-teams.py -Requires: GITHUB_TOKEN env var (with org team read access). +Requires: GITHUB_TOKEN env var (read access to javaBin/registry). """ import json import logging import os +import re import sys import urllib.request @@ -45,10 +48,15 @@ def github_get(path, token): return results +REGISTRY_REPO = os.environ.get("REGISTRY_REPO", "registry") +TEAM_NAME = re.compile(r"^[a-z]{1,12}$") + + def get_teams(token): - """Get all org teams, excluding platform-internal ones.""" - teams = github_get(f"/orgs/{GITHUB_ORG}/teams", token) - slugs = sorted(t["slug"] for t in teams if t["slug"] not in EXCLUDED_TEAMS) + """Registry team names (teams/.yaml), excluding platform-internal ones.""" + files = github_get(f"/repos/{GITHUB_ORG}/{REGISTRY_REPO}/contents/teams", token) + names = [f["name"][:-5] for f in files if f["type"] == "file" and f["name"].endswith(".yaml")] + slugs = sorted(n for n in names if TEAM_NAME.match(n) and n not in EXCLUDED_TEAMS) logger.info("Found %d teams (excluding %s)", len(slugs), ", ".join(EXCLUDED_TEAMS)) for slug in slugs: logger.info(" %s", slug) @@ -59,7 +67,7 @@ def write_tfvars(teams, output_path): """Write the registered-teams.auto.tfvars file.""" lines = [ "# GENERATED by sync-registered-teams.py — do not edit manually", - "# Source of truth: GitHub teams in javaBin org", + "# Source of truth: teams/*.yaml in javaBin/registry", "", "registered_teams = [", ] diff --git a/terraform/platform/registered-teams.auto.tfvars b/terraform/platform/registered-teams.auto.tfvars index bcc0487..9ce27b9 100644 --- a/terraform/platform/registered-teams.auto.tfvars +++ b/terraform/platform/registered-teams.auto.tfvars @@ -1,6 +1,8 @@ # GENERATED by sync-registered-teams.py — do not edit manually -# Source of truth: GitHub teams in javaBin org +# Source of truth: teams/*.yaml in javaBin/registry registered_teams = [ + "cupcake", + "jzwatch", "testteam", ] From 1fbc270dc6fb510a78aefe00b83360ecf7ec25e9 Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Fri, 23 Oct 2026 16:12:54 +0200 Subject: [PATCH 4/6] revert: keep syncing team roles from GitHub org teams in this PR Moved to its own PR: switching to registry teams destroys the roles of 21 non-app org teams (148 resources), which plans HIGH and needs a board override. The security fixes here should not wait on that. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 8 +++--- scripts/sync-registered-teams.py | 26 +++++++------------ .../platform/registered-teams.auto.tfvars | 4 +-- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f9260c6..ae9cf2b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -168,7 +168,7 @@ terraform/state/ | `password-set` | Self-service password-set for new hero accounts (Function URL) | | `budget-enforcer` | Scales ECS services to zero when team exceeds 200% budget | | `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 | +| `ci-broker` | Validates 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) | ### Scripts @@ -178,8 +178,7 @@ terraform/state/ | `scripts/generate-modules.py` | CI: validates app.yaml, generates fmt-clean module blocks into terraform/ (tests: `scripts/tests/`) | | `scripts/registry.py` | Module sources + boilerplate templates for generate-modules.py | | `scripts/resolve-tool-version.sh` | CI: resolve node/java version from the app repo's version files | -| `scripts/sync-registered-teams.py` | CI: registry `teams/*.yaml` → `registered-teams.auto.tfvars` (needs App token) | -| `scripts/github-oidc-token.sh` | CI: mint a GitHub OIDC token for the broker / apply gate | +| `scripts/sync-registered-teams.py` | CI: GitHub org teams → `registered-teams.auto.tfvars` (needs App token) | | `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 | @@ -203,7 +202,6 @@ terraform/state/ | SSM params (platform) | `/javabin/{namespace}/{name}` | | SSM params (app) | `/javabin/apps/{team}/{service}/{name}` | | ECS cluster | `javabin-platform` | -| ECS execution roles | `javabin-ecs-exec-{team}` (platform-owned, reads only `/javabin/apps/{team}/*`) | | ECR repos | `{team}-{service}` | | SNS topics | `javabin-{alerts,security,budget-enforcement}` | | Log groups | `/ecs/{team}/{service}` | @@ -289,7 +287,7 @@ The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation tar ### Known Issues - **ECS deploy stabilization**: platform-test-app task registers but service fails health check - **Cognito**: pools deployed; internal pool's `GoogleSAML` IdP was created outside Terraform -- **App databases share one network path**: service-rds allows ingress from the shared ECS tasks SG, so every team's tasks can reach every team's DB port (password still required). Fix: per-app security groups +- **CI broker trusts the payload repo name**: code running inside an app job (e.g. a Terraform `external` data source in tf-plan) can mint a new OIDC token for the broker role and name another repo. Fix: verify a GitHub OIDC token in the Lambda instead of a payload field - **Team provisioner Lambda**: All sync functions working (Google/GitHub/Budget/Cognito/Identity Center). Password-set flow deployed. - **`registered_app_repos` manually managed**: Being replaced with team-scoped IAM roles (repo→team resolved via GitHub API at runtime) - **Cost allocation tags pending activation**: `repo`, `created-by`, `commit` tags need activation in Billing console (requires billing data to appear first) diff --git a/scripts/sync-registered-teams.py b/scripts/sync-registered-teams.py index 713dc6d..d1c608f 100644 --- a/scripts/sync-registered-teams.py +++ b/scripts/sync-registered-teams.py @@ -1,22 +1,19 @@ #!/usr/bin/env python3 -"""Sync registered teams from the registry. +"""Sync registered teams from GitHub org teams. -Lists teams/*.yaml in javaBin/registry and writes -registered-teams.auto.tfvars. This drives team-scoped IAM role creation -(CI roles, ECS execution roles). +Queries the GitHub API for all teams in the javaBin org and writes +registered-teams.auto.tfvars. This drives team-scoped IAM role creation. -Source of truth: the registry. GitHub org teams also include non-app teams -(owners, developer, ...) that must not get AWS CI roles. +Source of truth: GitHub teams (created by team-provisioner from registry). Usage: sync-registered-teams.py -Requires: GITHUB_TOKEN env var (read access to javaBin/registry). +Requires: GITHUB_TOKEN env var (with org team read access). """ import json import logging import os -import re import sys import urllib.request @@ -48,15 +45,10 @@ def github_get(path, token): return results -REGISTRY_REPO = os.environ.get("REGISTRY_REPO", "registry") -TEAM_NAME = re.compile(r"^[a-z]{1,12}$") - - def get_teams(token): - """Registry team names (teams/.yaml), excluding platform-internal ones.""" - files = github_get(f"/repos/{GITHUB_ORG}/{REGISTRY_REPO}/contents/teams", token) - names = [f["name"][:-5] for f in files if f["type"] == "file" and f["name"].endswith(".yaml")] - slugs = sorted(n for n in names if TEAM_NAME.match(n) and n not in EXCLUDED_TEAMS) + """Get all org teams, excluding platform-internal ones.""" + teams = github_get(f"/orgs/{GITHUB_ORG}/teams", token) + slugs = sorted(t["slug"] for t in teams if t["slug"] not in EXCLUDED_TEAMS) logger.info("Found %d teams (excluding %s)", len(slugs), ", ".join(EXCLUDED_TEAMS)) for slug in slugs: logger.info(" %s", slug) @@ -67,7 +59,7 @@ def write_tfvars(teams, output_path): """Write the registered-teams.auto.tfvars file.""" lines = [ "# GENERATED by sync-registered-teams.py — do not edit manually", - "# Source of truth: teams/*.yaml in javaBin/registry", + "# Source of truth: GitHub teams in javaBin org", "", "registered_teams = [", ] diff --git a/terraform/platform/registered-teams.auto.tfvars b/terraform/platform/registered-teams.auto.tfvars index 9ce27b9..bcc0487 100644 --- a/terraform/platform/registered-teams.auto.tfvars +++ b/terraform/platform/registered-teams.auto.tfvars @@ -1,8 +1,6 @@ # GENERATED by sync-registered-teams.py — do not edit manually -# Source of truth: teams/*.yaml in javaBin/registry +# Source of truth: GitHub teams in javaBin org registered_teams = [ - "cupcake", - "jzwatch", "testteam", ] From 7c0cc785681cefbea27e2e83680ba31c9840b51e Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Fri, 23 Oct 2026 16:30:14 +0200 Subject: [PATCH 5/6] docs: restore CLAUDE.md notes for this PR's changes The previous revert also undid the CLAUDE.md entries for the execution roles, the OIDC token script and the broker fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ae9cf2b..3d1a8c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -168,7 +168,7 @@ terraform/state/ | `password-set` | Self-service password-set for new hero accounts (Function URL) | | `budget-enforcer` | Scales ECS services to zero when team exceeds 200% budget | | `resource-tagger` | EventBridge-triggered, auto-tags created-by + commit on new resources | -| `ci-broker` | Validates repo ∈ app.yaml team, vends short-lived team role credentials | +| `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) | ### Scripts @@ -179,6 +179,7 @@ terraform/state/ | `scripts/registry.py` | Module sources + boilerplate templates for generate-modules.py | | `scripts/resolve-tool-version.sh` | CI: resolve node/java version from the app repo's version files | | `scripts/sync-registered-teams.py` | CI: GitHub org teams → `registered-teams.auto.tfvars` (needs App token) | +| `scripts/github-oidc-token.sh` | CI: mint a GitHub OIDC token for the broker / apply gate | | `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 | @@ -202,6 +203,7 @@ terraform/state/ | SSM params (platform) | `/javabin/{namespace}/{name}` | | SSM params (app) | `/javabin/apps/{team}/{service}/{name}` | | ECS cluster | `javabin-platform` | +| ECS execution roles | `javabin-ecs-exec-{team}` (platform-owned, reads only `/javabin/apps/{team}/*`) | | ECR repos | `{team}-{service}` | | SNS topics | `javabin-{alerts,security,budget-enforcement}` | | Log groups | `/ecs/{team}/{service}` | @@ -287,7 +289,7 @@ The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation tar ### Known Issues - **ECS deploy stabilization**: platform-test-app task registers but service fails health check - **Cognito**: pools deployed; internal pool's `GoogleSAML` IdP was created outside Terraform -- **CI broker trusts the payload repo name**: code running inside an app job (e.g. a Terraform `external` data source in tf-plan) can mint a new OIDC token for the broker role and name another repo. Fix: verify a GitHub OIDC token in the Lambda instead of a payload field +- **App databases share one network path**: service-rds allows ingress from the shared ECS tasks SG, so every team's tasks can reach every team's DB port (password still required). Fix: per-app security groups - **Team provisioner Lambda**: All sync functions working (Google/GitHub/Budget/Cognito/Identity Center). Password-set flow deployed. - **`registered_app_repos` manually managed**: Being replaced with team-scoped IAM roles (repo→team resolved via GitHub API at runtime) - **Cost allocation tags pending activation**: `repo`, `created-by`, `commit` tags need activation in Billing console (requires billing data to appear first) From 33f0d8cee1dd6657102db41bdc50a7caac6d63f3 Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Fri, 23 Oct 2026 17:12:35 +0200 Subject: [PATCH 6/6] fix(iam): close exec-role takeover; verify OIDC tokens in plain Python Review feedback on #134: - Blocker: teams could rewrite the trust policy of shared roles (iam:UpdateAssumeRolePolicy was not in DenyShared), so team A could make javabin-ecs-exec-B trust itself and read team B's secrets. Denied. - RS256 verification no longer shells out to openssl: modular exponentiation plus an exact PKCS#1 v1.5 comparison, no temp files. Non-object JWT parts are a TokenError; unknown-kid JWKS refetches are limited to once a minute. - invoke-override-sign.sh still sends approved_by, which the currently deployed gate requires, so overrides keep working during rollout. - github-oidc-token.sh fails clearly when no token is returned; the gate no longer logs the id_token. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/apply-gate.md | 2 +- scripts/github-oidc-token.sh | 10 ++- scripts/invoke-override-sign.sh | 6 +- terraform/lambda-src/apply_gate/handler.py | 3 +- terraform/lambda-src/shared/github_oidc.py | 74 +++++++------------ .../lambda-src/tests/test_github_oidc.py | 10 +++ terraform/platform/iam/boundary.tf | 2 +- 7 files changed, 55 insertions(+), 52 deletions(-) diff --git a/docs/apply-gate.md b/docs/apply-gate.md index e014c27..e60200b 100644 --- a/docs/apply-gate.md +++ b/docs/apply-gate.md @@ -43,7 +43,7 @@ from the payload: app.yaml team the repo belongs to, and the plan must be the repo's own. - `sign` — only a token from `approve-override.yml` in javaBin/platform, running in the board-reviewed `override-approval` environment, is accepted. - The approver is the token's `actor`. Without this, any app's tf-apply job + The recorded approver is the token's `actor`: whoever dispatched the workflow (a board member must still approve the environment for it to run). Without this, any app's tf-apply job could sign its own HIGH-risk override. Tokens are minted with `scripts/github-oidc-token.sh ` and verified diff --git a/scripts/github-oidc-token.sh b/scripts/github-oidc-token.sh index dde09b4..299cd68 100644 --- a/scripts/github-oidc-token.sh +++ b/scripts/github-oidc-token.sh @@ -12,5 +12,11 @@ set -e AUDIENCE="${1:?Usage: github-oidc-token.sh }" : "${ACTIONS_ID_TOKEN_REQUEST_URL:?id-token: write permission is required}" -curl -sSf -H "Authorization: bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" \ - "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=${AUDIENCE}" | jq -r '.value' +RESPONSE=$(curl -sSf -H "Authorization: bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" \ + "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=${AUDIENCE}") +TOKEN=$(echo "$RESPONSE" | jq -r '.value // empty') +if [ -z "$TOKEN" ]; then + echo "ERROR: no OIDC token returned for audience ${AUDIENCE}" >&2 + exit 1 +fi +echo "$TOKEN" diff --git a/scripts/invoke-override-sign.sh b/scripts/invoke-override-sign.sh index 0bbd8a8..6851ed7 100644 --- a/scripts/invoke-override-sign.sh +++ b/scripts/invoke-override-sign.sh @@ -2,6 +2,9 @@ # Ask the apply-gate Lambda to sign a risk override for a plan. # # Usage: invoke-override-sign.sh +# approved_by is still sent for the previously deployed gate, which reads +# it; the current gate ignores it and uses the token's actor. +# # Env: PLAN_KEY, REASON; runs in approve-override.yml (the gate checks the # OIDC token comes from that workflow and the override-approval # environment, and records the token's actor as approver). @@ -14,7 +17,8 @@ ID_TOKEN=$(sh "$DIR/github-oidc-token.sh" "${PROJECT:-javabin}-apply-gate") aws lambda invoke \ --function-name "${PROJECT:-javabin}-apply-gate" \ --payload "$(jq -nc --arg plan_key "$PLAN_KEY" --arg reason "$REASON" --arg id_token "$ID_TOKEN" \ - '{action: "sign", plan_key: $plan_key, reason: $reason, id_token: $id_token}')" \ + --arg approved_by "${GITHUB_ACTOR:-}" \ + '{action: "sign", plan_key: $plan_key, reason: $reason, id_token: $id_token, approved_by: $approved_by}')" \ --cli-binary-format raw-in-base64-out \ /tmp/override-response.json >/dev/null diff --git a/terraform/lambda-src/apply_gate/handler.py b/terraform/lambda-src/apply_gate/handler.py index dfd5624..9884473 100644 --- a/terraform/lambda-src/apply_gate/handler.py +++ b/terraform/lambda-src/apply_gate/handler.py @@ -274,7 +274,8 @@ def _issue_credentials(repo_name, requested_team=None, session_name=None): def handler(event, context): - logger.info("Apply gate invoked: %s", json.dumps(event, default=str)[:500]) + logged = {k: v for k, v in event.items() if k != "id_token"} + logger.info("Apply gate invoked: %s", json.dumps(logged, default=str)[:500]) action = event.get("action", "check") diff --git a/terraform/lambda-src/shared/github_oidc.py b/terraform/lambda-src/shared/github_oidc.py index d53dda7..cc01022 100644 --- a/terraform/lambda-src/shared/github_oidc.py +++ b/terraform/lambda-src/shared/github_oidc.py @@ -7,15 +7,15 @@ minted for this audience, and the repo is read from its signed `repository` claim, which the job cannot forge. -RS256 is verified with the openssl CLI (in the Lambda runtime), so no -third-party packages are needed. +RS256 is verified in plain Python (modular exponentiation + an exact +PKCS#1 v1.5 comparison), so no third-party packages or binaries are needed. """ import base64 +import hashlib +import hmac import json import os -import subprocess -import tempfile import time import urllib.request @@ -26,6 +26,7 @@ CLOCK_SKEW = 60 _jwks_cache = {"keys": None, "fetched_at": 0} +REFETCH_MIN_SECONDS = 60 class TokenError(ValueError): @@ -36,33 +37,28 @@ def _b64url_decode(s): return base64.urlsafe_b64decode(s + "=" * (-len(s) % 4)) -def _der_len(n): - if n < 0x80: - return bytes([n]) - b = n.to_bytes((n.bit_length() + 7) // 8, "big") - return bytes([0x80 | len(b)]) + b +# DigestInfo prefix for SHA-256 in an EMSA-PKCS1-v1_5 block (RFC 8017 §9.2) +_SHA256_DIGEST_INFO = bytes.fromhex("3031300d060960864801650304020105000420") -def _der_int(b): - if b[0] & 0x80: - b = b"\x00" + b - return b"\x02" + _der_len(len(b)) + b - - -def _der_seq(*parts): - body = b"".join(parts) - return b"\x30" + _der_len(len(body)) + body - +def _verify_rs256(signing_input, signature, jwk): + """RSASSA-PKCS1-v1_5 / SHA-256 verification in plain Python. -def jwk_to_pem(jwk): - """Encode an RSA JWK (n, e) as a SubjectPublicKeyInfo PEM.""" - rsa_key = _der_seq(_der_int(_b64url_decode(jwk["n"])), _der_int(_b64url_decode(jwk["e"]))) - algorithm = _der_seq(b"\x06\x09\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01", b"\x05\x00") # rsaEncryption, NULL - bit_string = b"\x03" + _der_len(len(rsa_key) + 1) + b"\x00" + rsa_key - der = _der_seq(algorithm, bit_string) - b64 = base64.b64encode(der).decode() - lines = [b64[i:i + 64] for i in range(0, len(b64), 64)] - return "-----BEGIN PUBLIC KEY-----\n" + "\n".join(lines) + "\n-----END PUBLIC KEY-----\n" + Rebuilds the exact expected encoded message and compares it in constant + time; no parsing of the decrypted block, so no padding-oracle style + leniency. + """ + n = int.from_bytes(_b64url_decode(jwk["n"]), "big") + e = int.from_bytes(_b64url_decode(jwk["e"]), "big") + k = (n.bit_length() + 7) // 8 + if len(signature) != k: + return False + em = pow(int.from_bytes(signature, "big"), e, n).to_bytes(k, "big") + t = _SHA256_DIGEST_INFO + hashlib.sha256(signing_input).digest() + if k < len(t) + 11: + return False + expected = b"\x00\x01" + b"\xff" * (k - len(t) - 3) + b"\x00" + t + return hmac.compare_digest(em, expected) def _jwks(force=False): @@ -73,22 +69,6 @@ def _jwks(force=False): return _jwks_cache["keys"] -def _verify_rs256(signing_input, signature, jwk): - with tempfile.NamedTemporaryFile("w", suffix=".pem", delete=False) as k, \ - tempfile.NamedTemporaryFile("wb", suffix=".sig", delete=False) as sig: - k.write(jwk_to_pem(jwk)) - sig.write(signature) - try: - r = subprocess.run( - ["openssl", "dgst", "-sha256", "-verify", k.name, "-signature", sig.name], - input=signing_input, capture_output=True, - ) - return r.returncode == 0 - finally: - os.unlink(k.name) - os.unlink(sig.name) - - def verify(token, audience, now=None, keys=None): """Return the verified claims, or raise TokenError. @@ -104,13 +84,15 @@ def verify(token, audience, now=None, keys=None): signature = _b64url_decode(sig_b64) except ValueError as e: raise TokenError(f"undecodable token: {e}") + if not isinstance(header, dict) or not isinstance(claims, dict): + raise TokenError("token header and claims must be JSON objects") if header.get("alg") != "RS256": raise TokenError(f"unsupported alg {header.get('alg')!r}") jwks = keys if keys is not None else _jwks() jwk = jwks.get(header.get("kid")) - if jwk is None and keys is None: - jwk = _jwks(force=True).get(header.get("kid")) # key rotation + if jwk is None and keys is None and time.time() - _jwks_cache["fetched_at"] > REFETCH_MIN_SECONDS: + jwk = _jwks(force=True).get(header.get("kid")) # key rotation (rate-limited) if jwk is None: raise TokenError("unknown signing key") if not _verify_rs256(f"{header_b64}.{payload_b64}".encode(), signature, jwk): diff --git a/terraform/lambda-src/tests/test_github_oidc.py b/terraform/lambda-src/tests/test_github_oidc.py index 7658c67..e9eb77c 100644 --- a/terraform/lambda-src/tests/test_github_oidc.py +++ b/terraform/lambda-src/tests/test_github_oidc.py @@ -12,6 +12,7 @@ import sys import tempfile import unittest +import unittest.mock sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from shared import github_oidc as oidc # noqa: E402 @@ -82,6 +83,15 @@ def test_token_from_app_repo_workflow_is_rejected(self): self.assertRejected(self.token(job_workflow_ref="javaBin/platform/.github/workflows/tf-plan.yml@refs/heads/evil"), "javaBin/platform workflow") + def test_verification_needs_no_binaries(self): + token = self.token() + with unittest.mock.patch("subprocess.run", side_effect=AssertionError("shelled out")): + self.assertEqual(oidc.repo_name(self.verify(token)), "cupcake") + + def test_non_object_claims_are_a_token_error(self): + head = b64url(json.dumps({"alg": "RS256", "kid": "k1"}).encode()) + self.assertRejected(f"{head}.{b64url(b'[1, 2]')}.{b64url(b'x')}", "JSON objects") + def test_missing_or_garbage_token(self): self.assertRejected(None, "missing") self.assertRejected("a.b", "missing") diff --git a/terraform/platform/iam/boundary.tf b/terraform/platform/iam/boundary.tf index 8ed9378..40be12b 100644 --- a/terraform/platform/iam/boundary.tf +++ b/terraform/platform/iam/boundary.tf @@ -146,7 +146,7 @@ resource "aws_iam_policy" "developer_boundary" { "ec2:Delete*", "ec2:Modify*", "elasticloadbalancing:Delete*", "elasticloadbalancing:Modify*", "ecs:DeleteCluster", "ecs:UpdateCluster", "ecs:UpdateClusterSettings", - "iam:DeleteRole", "iam:UpdateRole", "iam:DeleteRolePolicy", + "iam:DeleteRole", "iam:UpdateRole", "iam:DeleteRolePolicy", "iam:UpdateAssumeRolePolicy", "iam:PutRolePolicy", "iam:AttachRolePolicy", "iam:DetachRolePolicy", "route53:DeleteHostedZone", "sns:DeleteTopic", "sns:SetTopicAttributes", # Shared SGs (ECS tasks, ALB): a team rule here opens or cuts every app