From 1135b9c8764341ed507e95fc1ec3520484469cb0 Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Mon, 24 Aug 2026 21:06:47 +0200 Subject: [PATCH] feat(scorecard): enforce the AWS backplane identity conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Azure, GCP and STACKIT backplanes have their identity conventions enforced by the scorecard. AWS had none, even though `.agents/references/aws-backplane.md` documents them — so the three AWS backplanes carrying the nullable-`workload_identity_federation`-with-an-access-key-fallback shape that the other three providers forbid scored 100%. Adds an `AWS Backplane` category: ten checks over what that reference already documents. **AWS documents two legitimate patterns, so the checks are pattern-scoped.** Pattern A (OIDC + IAM role) is for a building block acting in a single account; Pattern B (IAM user + CloudFormation StackSet) is for org-wide building blocks that must reach every account in an OU, and it mints an `aws_iam_access_key` on purpose. A blanket "no `aws_iam_access_key`" check would therefore be wrong. Each check declares which pattern it belongs to and reports `➖` for the other, the way the `terraform_version` check already reports `➖` for `manual` implementations. A backplane carrying federation machinery classifies as Pattern A *even when it also mints a key*: that hybrid is exactly the fallback shape the reference's first "What to Avoid" bullet forbids, and `aws_wif_no_access_key` is what reports it. | Check | Pattern | Enforces | |---|---|---| | `aws_wif_oidc_provider` | A | `aws_iam_openid_connect_provider` (resource or data), not a hardcoded ARN | | `aws_wif_no_access_key` | A | no `aws_iam_access_key` — the Azure `no_app_password` analogue | | `aws_wif_nonnullable` | A | `nullable = false`, no `default = null` fallback | | `aws_wif_create_oidc_provider` | A | `create_oidc_provider` present so backplanes can share one provider | | `aws_wif_subject_condition` | A | the `:sub` condition uses `var.workload_identity_federation.subjects` | | `aws_wif_role_output` | A | `workload_identity_federation_role`, ARN constructed not read off the resource | | `aws_wif_integration_env` | A | integration wires `AWS_ROLE_ARN` + `AWS_WEB_IDENTITY_TOKEN_FILE` | | `aws_cross_account_provider_aliases` | B | `configuration_aliases` declares `aws.management` and `aws.backplane` | | `aws_stackset_auto_deployment` | B | `SERVICE_MANAGED`, auto-deploying, `retain_stacks_on_account_removal = false`, `administration_role_arn` ignored | | `aws_cross_account_outputs` | B | `aws_access_key_id`, sensitive `aws_secret_access_key`, `role_name` | Each was checked against deliberately non-compliant fixtures as well as compliant ones — one per pattern, plus a backplane matching neither — so no check is a vacuous pass, and every branch of every detail message was exercised. The fixtures are deleted. Two judgement calls worth recording: - `role_name` is required only of StackSet-based Pattern B backplanes. The reference ties that output to "the IAM role deployed by StackSet to each target account"; `aws/opt-in-region` reaches a single management account through a role it creates itself and has no such name to publish. Requiring it there would be inventing a convention rather than enforcing one. - The category does not apply to a `backplane/` holding no `.tf` files of its own. `aws/agentic-coding-sandbox` is a composition whose backplane README says it "does not need any dedicated backplane", keeping only a `landingzone/` submodule there — it declares no automation principal to judge. `aws-backplane.md` joins `REF_FILES` and carries the `scorecard-checks` markers, so `--fix` links the section explaining each fix. Its two `### Implementation Pattern` headings are now `(WIF)` and `(Cross-Account)` — they collided on one anchor. Its "What to Avoid" section now names the fallback shape explicitly and says why Pattern B's key is a different thing. One supporting change: a category whose checks are all pattern-scoped can mark every one of them not applicable, leaving no score to render. The per-category table now prints `—` for that instead of `null%`. No module hits it today. **Newly flagged, deliberately not fixed here:** `aws/s3_bucket` scores 43% and `aws/route53-dns-record` / `aws/route53-dns-alias-record` 71%, all three failing `aws_wif_no_access_key` and `aws_wif_nonnullable`; s3_bucket additionally lacks `create_oidc_provider` and names its output `workload_identity_federation_role_arn`. Per the repo's convention that a scorecard change reports debt rather than sweeping it, they stay for whoever is next in those modules. No non-AWS module's verdict moved; repo average 74% → 75%. Co-Authored-By: Claude Opus 5 --- .agents/references/aws-backplane.md | 23 ++- tools/scorecard/scorecard.mjs | 239 +++++++++++++++++++++++++++- 2 files changed, 258 insertions(+), 4 deletions(-) diff --git a/.agents/references/aws-backplane.md b/.agents/references/aws-backplane.md index bba6eedb..a4a70008 100644 --- a/.agents/references/aws-backplane.md +++ b/.agents/references/aws-backplane.md @@ -17,7 +17,8 @@ Use WIF when the building block acts within a single AWS account (the backplane - **OIDC-native**: AWS supports federated OIDC identities via `aws_iam_openid_connect_provider` out of the box. - **Shared OIDC provider**: Multiple backplanes can share a single OIDC provider in the same AWS account using `create_oidc_provider = false`. -### Implementation Pattern + +### Implementation Pattern (WIF) ```hcl # backplane/main.tf — WIF-based automation principal @@ -82,6 +83,7 @@ resource "aws_iam_role" "backplane" { # Attach a service-specific policy to aws_iam_role.backplane ``` + ### Backplane Variables (WIF) ```hcl @@ -102,6 +104,7 @@ variable "create_oidc_provider" { } ``` + ### Backplane Outputs (WIF) ```hcl @@ -125,7 +128,8 @@ Use this pattern when the building block must act in **many target accounts** ac - **OU-scoped access**: Access is limited to the specified OUs; accounts outside those OUs cannot be reached. - **Minimal IAM user**: The IAM user in the backplane account only holds `sts:AssumeRole` on the specific role name — no direct service permissions. -### Implementation Pattern + +### Implementation Pattern (Cross-Account) ```hcl # backplane/main.tf — IAM user + CloudFormation StackSet pattern @@ -252,6 +256,7 @@ variable "stackset_region" { } ``` + ### Backplane Outputs (Cross-Account) ```hcl @@ -274,6 +279,7 @@ output "role_name" { --- + ## What to Avoid - ❌ Long-lived IAM access keys for single-account building blocks — use WIF (Pattern A) instead @@ -281,10 +287,23 @@ output "role_name" { - ❌ Overly broad IAM policies (`"*"` actions on `"*"` resources) — scope to minimum required actions and resources - ❌ `retain_stacks_on_account_removal = true` in StackSets — orphaned roles in removed accounts are a security risk +The first of these has a specific shape worth naming: a `workload_identity_federation` variable that +defaults to `null`, with `count = var.workload_identity_federation == null ? 1 : 0` selecting an +`aws_iam_user` and an `aws_iam_access_key` on the null branch. That is a single-account backplane +keeping a long-lived key as a fallback, and it is what the bullet forbids — the choice is between the +two patterns, not between federation and a key inside Pattern A. Pattern B's access key is a +different thing: it is the only credential that pattern has, and it authenticates a principal whose +sole permission is `sts:AssumeRole`. + +`modules/aws/s3_bucket`, `modules/aws/route53-dns-record` and `modules/aws/route53-dns-alias-record` +still carry the fallback shape. They are the remaining exceptions, not a pattern to copy — fix one +the next time you are in it. + --- ## `meshstack_integration.tf` Wiring (AWS) + ### WIF pattern ```hcl diff --git a/tools/scorecard/scorecard.mjs b/tools/scorecard/scorecard.mjs index 0cb69116..6c04cc03 100755 --- a/tools/scorecard/scorecard.mjs +++ b/tools/scorecard/scorecard.mjs @@ -49,6 +49,15 @@ const CATEGORIES = { description: "meshstack_integration.tf conventions", appliesTo: (mod) => existsSync(join(mod.path, "meshstack_integration.tf")), }, + aws_backplane: { + id: "aws_backplane", + name: "AWS Backplane", + description: "AWS automation principal conventions (WIF or cross-account StackSet)", + // A backplane/ holding no .tf files of its own declares no automation principal — the + // agentic-coding-sandbox composition keeps only a landingzone/ submodule there — so the + // category does not apply to it. + appliesTo: (mod) => mod.provider === "aws" && readAllBackplaneTf(mod) !== null, + }, azure_backplane: { id: "azure_backplane", name: "Azure Backplane", @@ -78,6 +87,26 @@ const CATEGORIES = { }, }; +// AWS backplanes come in two documented identity patterns, and most checks belong to exactly one: +// "wif" — OIDC provider + IAM role, for a building block acting in a single account +// "cross_account" — IAM user + assumable role (usually distributed by a StackSet), for org-wide +// building blocks that must reach every account in an OU +// A backplane carrying federation machinery is classified "wif" even when it also mints an access +// key: that hybrid is the optional-WIF-with-key-fallback shape the reference forbids, and +// aws_wif_no_access_key is what reports it. Everything else that mints a key is cross-account. +function awsBackplanePattern(mod) { + const allTf = readAllBackplaneTf(mod); + if (!allTf) return "none"; + const federated = + /(resource|data)\s+"aws_iam_openid_connect_provider"/.test(allTf) || + /^variable\s+"workload_identity_federation"/m.test(allTf); + if (federated) return "wif"; + return /resource\s+"aws_iam_access_key"/.test(allTf) ? "cross_account" : "none"; +} + +const NOT_WIF = { pass: null, detail: "not a workload identity federation backplane" }; +const NOT_CROSS_ACCOUNT = { pass: null, detail: "not a cross-account backplane" }; + // ─── Detector functions ───────────────────────────────────────────────────── // Each detector returns { pass: boolean, detail?: string } @@ -434,6 +463,207 @@ const detectors = [ }, }, + // ─── AWS Backplane ────────────────────────────────────────────────────── + // AWS documents two legitimate identity patterns, so a check has to know which one a backplane + // implements before it can judge it. Pattern B mints an `aws_iam_access_key` on purpose, so a + // blanket "no access key" check would be wrong there — it only applies on the federation path. + { + id: "aws_wif_oidc_provider", + category: "aws_backplane", + name: "Federates via aws_iam_openid_connect_provider", + emoji: "🔐", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const allTf = readAllBackplaneTf(mod); + return { + pass: /(resource|data)\s+"aws_iam_openid_connect_provider"/.test(allTf), + detail: "no aws_iam_openid_connect_provider — the trust policy must reference a managed OIDC provider, not a hardcoded ARN", + }; + }, + }, + { + id: "aws_wif_no_access_key", + category: "aws_backplane", + name: "No aws_iam_access_key on the federation path", + emoji: "🚫", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const allTf = readAllBackplaneTf(mod); + return { + pass: !/resource\s+"aws_iam_access_key"/.test(allTf), + detail: "aws_iam_access_key alongside workload identity federation — a long-lived key fallback is not a supported path for a single-account building block", + }; + }, + }, + { + id: "aws_wif_nonnullable", + category: "aws_backplane", + name: "workload_identity_federation is non-nullable", + emoji: "⚡", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const varsTf = readBackplaneFile(mod, "variables.tf"); + if (!varsTf) return { pass: false, detail: "no variables.tf" }; + const wifVar = extractVariableBlocks(varsTf).get("workload_identity_federation"); + if (!wifVar) return { pass: false, detail: 'variable "workload_identity_federation" not found' }; + const hasDefaultNull = /default\s*=\s*null/.test(wifVar); + return { + pass: /nullable\s*=\s*false/.test(wifVar) || !hasDefaultNull, + detail: hasDefaultNull + ? "default = null makes federation optional — the null branch is the access key path" + : undefined, + }; + }, + }, + { + id: "aws_wif_create_oidc_provider", + category: "aws_backplane", + name: "create_oidc_provider variable allows sharing the provider", + emoji: "♻️", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const varsTf = readBackplaneFile(mod, "variables.tf"); + if (!varsTf) return { pass: false, detail: "no variables.tf" }; + return { + pass: extractVariableBlocks(varsTf).has("create_oidc_provider"), + detail: "missing create_oidc_provider — a second backplane in the same AWS account cannot reuse the meshStack OIDC provider and its apply fails on EntityAlreadyExists", + }; + }, + }, + { + id: "aws_wif_subject_condition", + category: "aws_backplane", + name: "Trust policy scopes :sub to the BBD's WIF subjects", + emoji: "🛂", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const allTf = readAllBackplaneTf(mod); + // The condition variable is a template carrying nested quotes — + // `"${trimprefix(var....issuer, "https://")}:sub"` — so match to end of line, not to the + // next quote. + const hasSubCondition = /^\s*variable\s*=.*:sub"/m.test(allTf); + const hasSubjects = /var\.workload_identity_federation\.subjects/.test(allTf); + return { + pass: hasSubCondition && hasSubjects, + detail: hasSubCondition + ? "the :sub condition does not use var.workload_identity_federation.subjects, so it is not scoped to this building block definition" + : "no :sub condition — the role is assumable by every subject the meshStack issuer signs", + }; + }, + }, + { + id: "aws_wif_role_output", + category: "aws_backplane", + name: "Outputs workload_identity_federation_role as a constructed ARN", + emoji: "📤", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const outputsTf = readBackplaneFile(mod, "outputs.tf"); + if (!outputsTf) return { pass: false, detail: "no outputs.tf" }; + const blocks = extractOutputBlocks(outputsTf); + const roleOutput = blocks.get("workload_identity_federation_role"); + if (!roleOutput) { + const nearMiss = [...blocks.keys()].find((n) => n.startsWith("workload_identity_federation_role")); + return { + pass: false, + detail: nearMiss + ? `output is named "${nearMiss}" — the convention is "workload_identity_federation_role"` + : 'missing output "workload_identity_federation_role"', + }; + } + return { + pass: /arn:aws:iam::/.test(roleOutput) && !/aws_iam_role\.[\w-]+(\[\d+\])?\.arn/.test(roleOutput), + detail: "ARN is read off aws_iam_role instead of being constructed — that closes a dependency cycle through the BBD UUID in the WIF subjects", + }; + }, + }, + { + id: "aws_wif_integration_env", + category: "aws_backplane", + name: "Integration wires AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE", + emoji: "🌐", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "wif") return NOT_WIF; + const content = readIntegrationTf(mod); + if (!content) return { pass: false, detail: "no integration file" }; + const hasRoleArn = /\bAWS_ROLE_ARN\b/.test(content); + const hasTokenFile = /\bAWS_WEB_IDENTITY_TOKEN_FILE\b/.test(content); + return { + pass: hasRoleArn && hasTokenFile, + detail: hasRoleArn + ? "AWS_WEB_IDENTITY_TOKEN_FILE is not wired — the AWS SDK has no token to exchange" + : "AWS_ROLE_ARN is not wired as an environment input", + }; + }, + }, + { + id: "aws_cross_account_provider_aliases", + category: "aws_backplane", + name: "Declares aws.management and aws.backplane aliases", + emoji: "🧭", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "cross_account") return NOT_CROSS_ACCOUNT; + const allTf = readAllBackplaneTf(mod); + // Read the configuration_aliases list itself: a bare `provider = aws.backplane` elsewhere in + // the module is a use, not a declaration. + const aliases = allTf.match(/configuration_aliases\s*=\s*\[[^\]]*\]/)?.[0]; + if (!aliases) return { pass: false, detail: "no configuration_aliases — the caller cannot point the backplane at two accounts" }; + const hasManagement = /aws\.management\b/.test(aliases); + const hasBackplane = /aws\.backplane\b/.test(aliases); + return { + pass: hasManagement && hasBackplane, + detail: hasManagement + ? "no aws.backplane alias — the IAM user must live in a dedicated automation account" + : "no aws.management alias — the org-wide resources must be applied against the management account", + }; + }, + }, + { + id: "aws_stackset_auto_deployment", + category: "aws_backplane", + name: "StackSet is SERVICE_MANAGED, auto-deploying, retaining nothing", + emoji: "📚", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "cross_account") return NOT_CROSS_ACCOUNT; + const allTf = readAllBackplaneTf(mod); + const blocks = extractResourceBlocks(allTf, "aws_cloudformation_stack_set"); + if (blocks.size === 0) return { pass: null, detail: "no aws_cloudformation_stack_set resources" }; + const faults = []; + for (const [name, body] of blocks) { + if (!/permission_model\s*=\s*"SERVICE_MANAGED"/.test(body)) faults.push(`${name}: not SERVICE_MANAGED`); + if (!/auto_deployment\s*\{[^}]*enabled\s*=\s*true/.test(body)) faults.push(`${name}: auto_deployment not enabled`); + if (!/retain_stacks_on_account_removal\s*=\s*false/.test(body)) faults.push(`${name}: retain_stacks_on_account_removal is not false`); + if (!/ignore_changes\s*=\s*\[[^\]]*administration_role_arn/.test(body)) faults.push(`${name}: administration_role_arn not in ignore_changes`); + } + return { pass: faults.length === 0, detail: faults.join("; ") }; + }, + }, + { + id: "aws_cross_account_outputs", + category: "aws_backplane", + name: "Outputs the access key, a sensitive secret, and the target role name", + emoji: "🔑", + fn: (mod) => { + if (awsBackplanePattern(mod) !== "cross_account") return NOT_CROSS_ACCOUNT; + const outputsTf = readBackplaneFile(mod, "outputs.tf"); + if (!outputsTf) return { pass: false, detail: "no outputs.tf" }; + const blocks = extractOutputBlocks(outputsTf); + if (!blocks.has("aws_access_key_id")) return { pass: false, detail: 'missing output "aws_access_key_id"' }; + const secret = blocks.get("aws_secret_access_key"); + if (!secret) return { pass: false, detail: 'missing output "aws_secret_access_key"' }; + if (!/sensitive\s*=\s*true/.test(secret)) + return { pass: false, detail: "aws_secret_access_key is not marked sensitive = true" }; + // role_name names the role a StackSet deploys into the target accounts. A backplane that + // reaches a single account through a role it creates itself has no such name to publish, so + // only StackSet-based backplanes are held to it. + const hasStackSet = extractResourceBlocks(readAllBackplaneTf(mod), "aws_cloudformation_stack_set").size > 0; + return { + pass: !hasStackSet || blocks.has("role_name"), + detail: 'missing output "role_name" — the building block cannot name the role it assumes in the target account', + }; + }, + }, + // ─── Azure Backplane ──────────────────────────────────────────────────── { id: "azure_uses_uami", @@ -1183,6 +1413,7 @@ function discoverModules() { const REF_FILES = [ "AGENTS.md", + ".agents/references/aws-backplane.md", ".agents/references/azure-backplane.md", ".agents/references/gcp-backplane.md", ".agents/references/stackit-backplane.md", @@ -1451,9 +1682,13 @@ function main() { const checkMarks = cr.checks .map((c) => (c.result.pass === null ? "➖" : c.result.pass ? "✅" : "❌")) .join(" | "); - const scoreEmoji = cr.score >= 80 ? "🟢" : cr.score >= 50 ? "🟡" : "🔴"; + // A pattern-scoped category can mark every one of its checks not applicable, which leaves + // no score to render. + const scoreCell = cr.score === null + ? "—" + : `${cr.score >= 80 ? "🟢" : cr.score >= 50 ? "🟡" : "🔴"} ${cr.score}%`; lines.push( - `| \`${r.mod.id}\` | ${scoreEmoji} ${cr.score}% | ${checkMarks} |` + `| \`${r.mod.id}\` | ${scoreCell} | ${checkMarks} |` ); } lines.push("");