[IAmJSD]

Root cause

iac/federation/aws-target/cloudformation/template.yaml built the role's trust policy as a YAML mapping whose two StringEquals keys were !Sub expressions (${Host}:aud and ${Host}:sub, with the host taken from OIDCIssuerURL since #1312). CloudFormation does not accept an intrinsic function as a map key, so it cannot load the template at all. cfn-lint 1.58.0 on main reports:

E0000 Unhashable type "{'Fn::Sub': ['${Host}:aud', {'Host': {'Fn::Select': [1, {'Fn::Split': ['https://', {'Ref': 'OIDCIssuerURL'}]}]}}]}" (line 268)

So a customer who downloads the AWS-target bundle (format=cfn, or the cloudformation/ part of format=bundle) and runs deploy-cfn.sh gets a template-format error and no stack. The repo's own Go evaluator in iac/aws_target_cfn_test.go accepted the template, so CI never noticed.

Fix

• Template. AssumeRolePolicyDocument is now a single Fn::Sub JSON string. It produces the same policy as before:

  • one Allow statement, with Principal.Federated set to the OIDC provider ARN
  • Action: sts:AssumeRoleWithWebIdentity
  • StringEquals on exactly <host>:aud and <host>:sub. The host is still !Select [1, !Split ["https://", OIDCIssuerURL]], the audience still falls back to sts.amazonaws.com when it is empty, and the subject is still the required OIDCSubjectClaim.

  There is no wildcard, no new operator and no new key.

• Stricter AllowedPatterns. Fn::Sub inserts text as-is, with no JSON escaping. So OIDCIssuerURL (both its host and path segments), OIDCAudience and OIDCSubjectClaim now also reject ", \ and control characters (\x00-\x1F). Every rule they already enforced is unchanged. With those characters excluded, no value can end its JSON string early to add, replace or widen a condition. The ConstraintDescriptions explain why. No real issuer URL, audience or subject contains these characters, and the API and generator already restrict subjects more tightly than this.

• CI. A new local pre-commit hook pins cfn-lint==1.58.0 with --non-zero-exit-code error and lints all four CloudFormation templates: cloudformation/stacks/* and iac/federation/*/cloudformation. It follows the same pattern as the zizmor hook. check-yaml skips these directories, so until now nothing checked that they load.

• Comments only. In iac/federation/aws-target/terraform/variables.tf and scripts/generate-federation-iac.go, comments claimed the Terraform validation and the CloudFormation pattern were byte-for-byte equivalent. They are updated to say the template now also rejects ", \ and control characters. Terraform does not need those rejections because jsonencode escapes them. No Terraform behavior changes.

Closes #1489

Tests

All new and changed tests are unit or fixture tests. Nothing calls AWS.

• iac/aws_target_cfn_trust_policy_test.go (new):
  • TestAWSTargetCFN_MappingKeysArePlainStrings walks every mapping in the template and fails on any key that is not a plain string. This is the load-failure regression itself.
  • TestAWSTargetCFN_ConditionKeysDeriveFromIssuerURL renders the document for the Google and Azure issuers, with and without an audience. It decodes the result with a strict decoder (duplicate keys and trailing data are rejected) and checks it equals the exact intended policy.
  • TestAWSTargetCFN_TrustPolicyValuesCannotEscapeJSON tests the injection property. Each of the three parameters gets values carrying every ASCII character plus U+00A0, U+00E9, U+2028 and U+2029 (in both the host and path positions for the URL). Each value the parameter's AllowedPattern/MinLength admits must render to valid JSON that equals the intended policy with that value in place.
  • TestAWSTargetCFN_RejectsJSONBreakout checks that concrete breakout payloads are rejected, for example 1"}},{"Effect":"Allow","Principal":{"AWS":....
  • TestDecodeStrictJSON_Control is a control test for the strict decoder.
• iac/aws_target_cfn_test.go: the evaluator now also handles !If, !Equals and !Not, and evaluates the template's real Conditions section. The old structural test is replaced by the rendering tests above. TestCFNEval_Positive adds controls for !If (both branches, plus an undefined condition), !Not/!Equals, and single-pass !Sub.
• internal/api/handler_federation_cfn_params_test.go: TestCFNBundle_TemplatePassesCFNLint builds the real format=cfn download for gcp and azure sources. It asserts that cloudformation/template.yaml is byte-identical to the committed file, so the pre-commit lint covers what customers download. When cfn-lint is on PATH it also runs it on the extracted template and requires exit 0 (no findings, warnings included). Without cfn-lint it skips, the same way the Terraform bundle test skips without terraform.

Verification

All commands were run locally on Linux, Go 1.26.9, with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true. cfn-lint 1.58.0 was installed in a throwaway venv.

cfn-lint (static analysis):

$ cfn-lint <main's template.yaml>
E0000 Unhashable type "{'Fn::Sub': ['${Host}:aud', ...]}" (line 268)      # exit 2
$ cfn-lint iac/federation/aws-target/cloudformation/template.yaml
                                                                          # exit 0, no findings

Pre-commit hook:

$ pre-commit run cfn-lint --all-files     # this branch
Lint CloudFormation templates....Passed
$ pre-commit run cfn-lint --all-files     # same tree, aws-target template swapped for main's
Lint CloudFormation templates....Failed   # E0000 on the aws-target template; the two W3005 warnings are printed but do not fail it

Fail-before / pass-after, unit/fixture evidence. With main's template in this tree:

$ go test ./iac/ -count=1
--- FAIL: TestAWSTargetCFN_MappingKeysArePlainStrings   (key at line 268 is a sequence node tagged "!Sub")
--- FAIL: TestAWSTargetCFN_ConditionKeysDeriveFromIssuerURL
--- FAIL: TestAWSTargetCFN_TrustPolicyValuesCannotEscapeJSON
--- FAIL: TestAWSTargetCFN_RejectsJSONBreakout          (OIDCIssuerURL must reject "https://accounts.google.com\"")
$ PATH=<venv>/bin:$PATH go test ./internal/api/ -count=1 -run TestCFNBundle_TemplatePassesCFNLint
--- FAIL: .../gcp, .../azure   (E0000 Unhashable type ... line 268)

With the fix, all of these pass, and the bundle test passes both with and without cfn-lint on PATH.

Mutation probes (template edited, then go test ./iac/). Each one fails at least one test:

• " allowed in OIDCSubjectClaim
• \ allowed in OIDCAudience
• control characters allowed in the URL path segment
• " allowed in the URL host
• StringEquals changed to StringLike
• audience fallback changed to "*"
• the :sub condition dropped
• an extra sts:TagSession action added
• the host taken from !Select [0, ...]

Other checks:

• go build ./..., go vet ./... and go vet -tags integration ./...: clean.
• go test -count=1 on ./iac/, ./scripts/ (generator bundle tests), ./internal/api/, ./internal/iacfiles/ and ./internal/mocks/: all ok.
• golangci-lint v2.10.1 run --allow-parallel-runners ./iac/... ./internal/api/... ./scripts/...: 0 issues.
• gocyclo -over 10 -ignore "_test\.go" scripts/generate-federation-iac.go: clean. Only a comment changed in that file.
• bash scripts/check-aws-iam-parity.sh and bash scripts/test-aws-iam-parity.sh: pass (the permissions policy is unchanged).
• terraform fmt -check and terraform validate (init -backend=false) in iac/federation/aws-target/terraform: clean.
• pre-commit trailing-whitespace, end-of-file-fixer, check-yaml, go-fmt and cfn-lint on the changed files: pass.

Not run here: trivy config (not installed on this machine), the full pre-commit job, the integration suites and the race tests. A real aws cloudformation validate-template/deploy was deliberately not run (no AWS calls). One limitation: cfn-lint does not parse the JSON inside a Fn::Sub string (I checked by adding a stray comma: cfn-lint still exits 0). That is why the Go tests decode and compare the rendered JSON themselves.

Combined-tree verification at local queue commit 89fe5df6 = origin/main 3bfc2fe5 + fix/issue-1484, fix/issue-1498, fix/issue-1489 and fix/issue-1494 merged in that order. Passed: go build ./...; go vet with and without -tags integration; go test -race -short -count=1 ./...; golangci-lint v2.10.1 with and without the integration tag; gocyclo -over 10; go test -race -tags integration on internal/api, internal/config and internal/database/postgres/migrations; the git-secrets scan and scripts/test-git-secrets-allowlist.sh.

Out of scope / notes

• cfn-lint reports one W3005 warning (redundant DependsOn) on each of cloudformation/stacks/CUDly/template.yaml and iac/federation/aws-cross-account/cloudformation/template.yaml. These are not load failures, so the hook gates on errors only, and I left those two templates untouched. Fixing the warnings would mean deleting two redundant DependsOn lines; after that the hook could drop --non-zero-exit-code error.
• The Terraform module's oidc_audience and oidc_subject_claim validations still accept " and \. That is safe there because jsonencode escapes them, but values containing them are now accepted by Terraform and rejected by CloudFormation. Tightening Terraform to match is a possible follow-up for parity. It is not needed for correctness.
• Existing stacks: the version before chore(iac): derive the CFN issuer host, create the GCP signing key first, read the dev Function URL from state #1312 also used !Sub keys (${OIDCIssuerHost}:aud), and chore(iac): derive the CFN issuer host, create the GCP signing key first, read the dev Function URL from state #1312 recorded the same E0000 for it. So, as far as this history shows, no customer stack can have been created from this template, and there is no in-place upgrade to worry about. I did not check every older revision.

Maintainers: please mirror the issue's triage labels onto this PR: priority/p2, type/bug. The issue has no triaged label.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE a36582f5c200b15eda34fb25aa5e3e1cc86f44dc

Blocking:

• none

Non-blocking:

• Acceptance criterion "cfn-lint passes on every shipped template": with default settings cfn-lint 1.58.0 still exits 4 (warnings) on cloudformation/stacks/CUDly/template.yaml (W3005 at 827) and iac/federation/aws-cross-account/cloudformation/template.yaml (W3005 at 243). The hook gates on errors only, and the PR body says so openly. Every template now loads, which is what the issue was about, so I don't think this blocks Closes #1489. Dropping the two redundant DependsOn lines in a follow-up would let the hook use the default exit code.
• In CI the Go bundle test always runs its byte-for-byte check, but it skips the cfn-lint run because cfn-lint is not on the Go job's PATH. Real cfn-lint coverage in CI therefore comes from the pre-commit hook plus that byte-identity check. That is a sound pairing; I note it only so the "a test validates the rendered bundle with cfn-lint" criterion isn't over-read.
• CFN/TF parity: the Terraform oidc_audience / oidc_subject_claim validations still accept " and \, which the template now rejects. This is safe because of jsonencode, as the PR says. Optional follow-up.

Attacked:

• Security (sensitive path). I compared the old mapping-form trust policy with the new Fn::Sub JSON statement by statement: same single Allow, Principal.Federated is the provider ARN, the action is only sts:AssumeRoleWithWebIdentity, StringEquals has exactly <host>:aud / <host>:sub, the audience falls back to sts.amazonaws.com, and there is no wildcard or new operator.
• JSON-injection surface:
  • Every value that reaches the string (issuer URL, audience, subject, and the provider ARN, which is derived from the URL) now excludes ", \ and \x00-\x1F. The $/* guards are unchanged.
  • YAML single quoting leaves the Java-regex escapes literal, and \s/\x00 mean the same in RE2 and Java for these ranges.
  • Mutation: dropping " from the OIDCSubjectClaim pattern makes TrustPolicyValuesCannotEscapeJSON/OIDCSubjectClaim and RejectsJSONBreakout fail.
• Installed cfn-lint 1.58.0 in a throwaway venv:
  • main's aws-target template gives E0000 (exit 2); this head's gives exit 0.
  • All four templates matched by the hook's files: regex pass with --non-zero-exit-code error. Those are the only AWSTemplateFormatVersion files in the repo.
  • CI's pre-commit job has no egress block, so the python hook can install.
• Ran: go build ./...; go vet with and without -tags integration on iac, internal/api and scripts; go test -race -short -count=1 ./iac/... ./scripts/ (ok); TestCFNBundle_* with cfn-lint on PATH (pass, lint actually executed).
• Fail-before: with main's template restored, all four TestAWSTargetCFN_* tests and TestCFNBundle_TemplatePassesCFNLint/{gcp,azure} fail. Restored afterwards.

Body-stale: none found. The cfn-lint output, the W3005 notes and the four-template coverage all match what I reproduced.

_{Reviewer: rF1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE a36582f5c200b15eda34fb25aa5e3e1cc86f44dc

Blocking:

• none

Non-blocking:

• .pre-commit-config.yaml:167: the hook gates on errors only (--non-zero-exit-code error). The issue's first acceptance line is "cfn-lint passes on every shipped template", and plain cfn-lint still exits 4 on cloudformation/stacks/CUDly/template.yaml:827 and iac/federation/aws-cross-account/cloudformation/template.yaml:243 (W3005, redundant DependsOn). Neither is a load failure, and the PR body says so. Deleting the two DependsOn lines in a follow-up would let the hook drop the flag and meet the criterion literally.
• internal/api/handler_federation_cfn_params_test.go:150: TestCFNBundle_TemplatePassesCFNLint skips its cfn-lint step in CI because the Go test jobs don't install cfn-lint. Only the byte-equality half runs there. Together with the pre-commit hook, which runs --all-files in CI, that still covers the downloaded bundle, so the second acceptance line holds indirectly rather than through the Go test alone.
• Parity follow-up, as the body notes: Terraform's oidc_audience/oidc_subject_claim still accept " and \. That's safe under jsonencode, but the two front ends now accept different input sets.

Attacked:

• JSON breakout through Fn::Sub. A JSON string can only be ended by " or changed by \ or a raw control character. All three parameters that reach the string (OIDCIssuerURL in both host and path segments, OIDCAudience, OIDCSubjectClaim) now exclude ", \\ and \x00-\x1F. The provider ARN's caller-controlled part is the same URL. Java (CloudFormation) and RE2 (the test) read \x00-\x1F and \s the same way, and the single-quoted YAML leaves \\ as a literal-backslash class member. Substitution is single-pass, so a ${ inside a value isn't re-expanded. The trust policy is otherwise unchanged: one Allow statement, StringEquals on exactly <host>:aud/<host>:sub, and the audience falls back to sts.amazonaws.com.
• cfn-lint 1.58.0 (venv) on all 4 templates: aws-target and CUDly-CrossAccount are clean, the other two are W3005 only, and main's aws-target template gets E0000. pre-commit run cfn-lint --all-files on this head: Passed.
• Fail-before: with main's template swapped in, go test ./iac/ fails all 4 new trust-policy tests, and TestCFNBundle_TemplatePassesCFNLint (with cfn-lint on PATH) fails gcp and azure with E0000. Restored afterwards. Mutation: allowing " in OIDCSubjectClaim fails TrustPolicyValuesCannotEscapeJSON and RejectsJSONBreakout.
• go build ./...; go vet (with and without -tags integration) on iac, internal/api and scripts; go test -race -short -count=1 ./iac/ ./scripts/ and -run TestCFNBundle ./internal/api/ with cfn-lint on PATH: all pass. golangci-lint v2.10.1 on iac and internal/api: 0 issues.

Body-stale:

• none

_{Reviewer: rF3. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at a36582f5c200b15eda34fb25aa5e3e1cc86f44dc merged onto main (b9271e88), together with #1507 in that order (combined tree 158319e3, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on a36582f5 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly a36582f5c200b15eda34fb25aa5e3e1cc86f44dc.