feat(scorecard): enforce the AWS backplane identity conventions - #286
Closed
JohannesRudolph wants to merge 1 commit into
Closed
feat(scorecard): enforce the AWS backplane identity conventions#286JohannesRudolph wants to merge 1 commit into
JohannesRudolph wants to merge 1 commit into
Conversation
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 <noreply@anthropic.com>
Contributor
Scorecard CheckNo module changes detected relative to |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Member
Author
|
Superseded by #293, which squashes both onto current |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Azure, GCP and STACKIT backplanes have their identity conventions enforced by the scorecard — nine checks for Azure, nine for GCP (#283), four for STACKIT. AWS had none, even though
.agents/references/aws-backplane.mdalready documents them. The gap was visible: the three AWS backplanes carrying the nullable-workload_identity_federation-with-an-access-key-fallback shape that the other three providers forbid scored 100%.This adds an
AWS Backplanecategory: ten checks derived from what that reference documents.Two patterns, so the checks are pattern-scoped
The reference documents two legitimate patterns:
aws_iam_access_keyon purpose: it is the only credential the pattern has, and it authenticates a principal whose sole permission issts:AssumeRole.So a naive "no
aws_iam_access_key" check would be wrong, exactly as the task anticipated. Instead each detector declares which pattern it belongs to and reports➖for the other — the same conventionterraform_versionalready uses to report➖formanualimplementations.The deliberate decision: a backplane carrying federation machinery classifies as Pattern A even when it also mints a key. That hybrid is not a third pattern — it is precisely what the reference's first "What to Avoid" bullet forbids ("Long-lived IAM access keys for single-account building blocks"), and
aws_wif_no_access_keyis what reports it. Pattern B is recognised by minting a key without any federation machinery, and is never asked to drop it.aws_wif_oidc_provideraws_iam_openid_connect_provider(resource or data), not a hardcoded ARNaws_wif_no_access_keyaws_iam_access_key— the Azureno_app_passwordanalogueaws_wif_nonnullablenullable = false, nodefault = nullfallbackaws_wif_create_oidc_providercreate_oidc_providerpresent so backplanes can share one provideraws_wif_subject_condition:subcondition usesvar.workload_identity_federation.subjectsaws_wif_role_outputworkload_identity_federation_role, ARN constructed not read off the resourceaws_wif_integration_envAWS_ROLE_ARN+AWS_WEB_IDENTITY_TOKEN_FILEaws_cross_account_provider_aliasesconfiguration_aliasesdeclaresaws.managementandaws.backplaneaws_stackset_auto_deploymentSERVICE_MANAGED, auto-deploying,retain_stacks_on_account_removal = false,administration_role_arnignoredaws_cross_account_outputsaws_access_key_id, sensitiveaws_secret_access_key,role_nameTwo more judgement calls
role_nameis 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-regionreaches a single management account through a role it creates itself and has no such name to publish; holding it to the output would be inventing a convention rather than enforcing one.backplane/with no.tffiles of its own.aws/agentic-coding-sandboxis a composition whose backplane README says it "does not need any dedicated backplane", keeping only alandingzone/submodule there. It declares no automation principal to judge.Verification
Each check was exercised against deliberately non-compliant fixtures as well as compliant ones — one pair per pattern, plus a backplane matching neither — so none is a vacuous pass. Beyond the all-pass/all-fail pair, every branch of every detail message was driven individually: data-only OIDC provider (passes), missing WIF variable,
:subpresent but not scoped tosubjects, correctly-named output readingaws_iam_role...arn, integration missing either env var or missing entirely, StackSet present withrole_nameremoved vs. StackSet removed withrole_namestill absent, one alias declared vs. noconfiguration_aliasesat all, and a single StackSet fault in isolation. The fixtures are deleted.Full scorecard output diffed before and after: no non-AWS module's category verdict moved, the module count is unchanged (53), and the maturity distribution is identical. Repo average 74% → 75%.
Newly flagged, deliberately not fixed here
Per the repo convention that a scorecard change reports debt rather than sweeping it:
aws/s3_bucketaws_wif_no_access_key,aws_wif_nonnullable,aws_wif_create_oidc_provider,aws_wif_role_output(output is namedworkload_identity_federation_role_arn)aws/route53-dns-recordaws_wif_no_access_key,aws_wif_nonnullableaws/route53-dns-alias-recordaws_wif_no_access_key,aws_wif_nonnullableOverall scores move
s3_bucket91% → 80%, both route53 modules 91% → 87%; the three cross-account backplanes gain (alternate-contacts60% → 69%,budget-alert70% → 77%,opt-in-region60% → 67%) since they were already compliant and now have a category to say so.Follow-up: #287 stacks on this branch and converts all three to the federated path, taking the category to 100% across every AWS backplane.
Supporting changes
aws-backplane.mdjoinsREF_FILESand carries thescorecard-checksmarkers, so--fixlinks the section explaining each fix. Its two### Implementation Patternheadings became(WIF)and(Cross-Account)— they collided on one anchor. "What to Avoid" now names the fallback shape explicitly and records the three modules that still carry it.—instead ofnull%. No module hits this today.🤖 Generated with Claude Code