sec(iac): onboarding paths drift - shell/CFN/ARM templates lack the guards their Terraform siblings enforce

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code.

The structural finding

Every serious IaC security defect found in the 2026-07-28 review is the same shape: for a given onboarding job, the Terraform path enforces a guard and the shell / CloudFormation / ARM path for the identical job does not.

This class cannot be caught by reviewing one cloud's template in isolation, which is exactly why it survived. Each individual instance looks like an isolated oversight; together they are a missing invariant.

The instances (each filed separately, listed here for the pattern)

1. AWS target, OIDC subject claim. iac/federation/aws-target/cloudformation/template.yaml:52-57,171-178 defaults OIDCSubjectClaim to "" and omits the :sub condition entirely. The Terraform sibling terraform/variables.tf:28-43 makes it mandatory with a validation reading "Leaving it empty would allow any principal in the OIDC provider tenant to assume this role."

2. GCP WIF, attribute condition. arm/CUDly-CrossSubscription/setup-gcp-wif.sh:99-103,133-139 creates the AWS-mode provider with no --attribute-condition and binds impersonation pool-wide. The Terraform sibling iac/federation/gcp-target/terraform/main.tf:137-147 has a hard precondition forbidding exactly that state and quoting the same attack.

3. Azure reservation role, assignment scope. arm/CUDly-CrossSubscription/template.json:86-100 assigns the purchase role at tenant /providers/Microsoft.Capacity scope as well as subscription scope. The Terraform sibling iac/federation/azure-target/terraform/main.tf:77-83 assigns subscription-only, and the shared module terraform/modules/iam/azure/cudly-reservation-role/main.tf:58-71 documents that the tenant scope was deliberately removed.

4. AWS target, OIDC thumbprint. iac/federation/aws-target/cloudformation/template.yaml:42-50 accepts the all-zeros placeholder thumbprint for arbitrary issuers. The Terraform sibling terraform/variables.tf:79-87 rejects it unless the issuer is one AWS natively validates, reasoning that "the all-zeros value bypasses the CA-chain check entirely."

A fifth, adjacent instance in our own CI IAM: terraform/environments/azure/ci-cd-permissions/sp.tf:40-49 trusts the bare pull_request subject while the AWS and GCP siblings pin to main and named environments.

Failure scenario

A customer picks whichever onboarding path their tooling supports. Choosing CloudFormation or the ARM/shell script instead of Terraform silently opts them out of guards that the Terraform path treats as mandatory, with no warning anywhere in the docs that the paths are not equivalent. In every case above the resulting exposure is spend authority on the customer's own cloud account.

The defect is not any single template. It is that the paths are maintained independently with no mechanism keeping them in parity.

Fix direction

Beyond fixing the four instances individually:

• Parity tests in CI that assert the guard set is equivalent across the paths for each onboarding job, so a future divergence fails CI rather than shipping. At minimum: every path that creates a trust relationship must require a subject/attribute restriction, and every role assignment must be scoped no wider than the Terraform sibling's.
• Treat the Terraform module as the normative definition and generate or lint the other paths against it.
• Document explicitly, in the onboarding docs, that the paths are intended to be equivalent, so a reviewer knows divergence is a bug.

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A06-028 (medium)

A fifth instance of the same shape, this time shell-versus-shell rather than shell-versus-Terraform. internal/iacfiles/templates/azure-wif-deploy.sh.tmpl:77-81 ends the federated-credential create with || echo "(federated credential may already exist - continuing)", which swallows every non-zero exit and defeats the script's own set -euo pipefail at :14. The script then assigns the Reservation Purchaser role at subscription scope and prints '=== Done ===' (:83-97), so a Graph permission denial or a throttle produces a confident success with no credential in place at all. The same swallow is at azure-wif-cli.sh.tmpl:58-61. The GCP script does the compare-and-abort this needs: gcp-wif-cli.sh.tmpl:177-233 re-reads an existing provider and aborts unless issuer, attribute condition, mapping, state and audience all match. Audit finding A06-028.

A13b-012 (medium)

A further instance of this pattern, from finding A13b-012, and one that runs Terraform-to-Terraform rather than across formats. iac/federation/azure-target/terraform/variables.tf:19-33 declares cudly_issuer_url, cudly_federated_subject and cudly_federated_audience with no validation block on any of the three, and all three feed the federated identity credential unchecked at main.tf:56-58. The AWS sibling guards the equivalent inputs hard: the issuer at aws-target/terraform/variables.tf:11-15, the audience at :36-39, and the subject at :59-62 including the $/* rejection. Azure AD fetches JWKS from whatever host the issuer names and mints tokens for CUDly's app registration for any JWT that host signs with the default subject cudly-controller, one constant shared across every customer tenant, so a wrong or attacker-controlled issuer URL yields reservation-purchase authority in the customer's subscription. Worth adding to the parity-test set this issue proposes, since a cross-format-only test would not catch it.

A06-029 (low)

Another instance of this pattern, from finding A06-029, between sibling shell templates rather than across formats. CUDLY_FEDERATED_SUBJECT is guarded on two of the three onboarding scripts: aws-wif-cli.sh.tmpl:31-41 rejects an empty value and any $, * or whitespace because IAM expands policy variables inside Condition values, and gcp-wif-cli.sh.tmpl:50-61 pins a charset plus a 127-character cap because the value lands in a CEL string literal. Both Azure templates interpolate the override straight into the federated-credential JSON heredoc with no check at all (azure-wif-cli.sh.tmpl:26,47-56 and azure-wif-deploy.sh.tmpl:61,66-75), where a " closes the subject string and lets sibling keys such as audiences be injected into the request body. Operator-supplied rather than attacker-supplied, so this is defense in depth, but it is the same missing-invariant shape and belongs in the parity check.

[cristim]

Additional evidence from the same 2026-07-28 review pass, second axis: the same missing-parity invariant exists inside terraform/modules/** between sibling host-side modules, not only between the customer-facing onboarding paths.

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; every line reference below was re-verified against 3e9660d06.

The structural claim in this issue ("each individual instance looks like an isolated oversight; together they are a missing invariant") holds one level down as well. Where one cloud's module or one sibling module was hardened, the equivalent module for the same job was left as it was, and nothing in CI notices.

Host-side instances of the same shape

1. Project-wide vs per-secret Secret Manager binding. terraform/modules/compute/gcp/cleanup-function/main.tf:9-13 grants roles/secretmanager.secretAccessor at project scope. The sibling compute/gcp/cloud-run/main.tf:247-276 explicitly documents removing this exact anti-pattern and replacing it with per-secret bindings: "Previously this granted the Cloud Run SA project-wide roles/secretmanager.secretAccessor, which let it read every secret in the project... Now we bind only the specific secrets". The cleanup function was never migrated, and its SA can read the credential-encryption key. Filed separately at critical severity.

2. Scheduled-task auth mode. compute/gcp/cloud-run/main.tf:80-85 derives scheduled_task_auth_mode = "disabled" from whether a scheduler is enabled. The AWS and Azure siblings both hard-pin SCHEDULED_TASK_AUTH_MODE = "bearer" unconditionally (compute/aws/lambda/main.tf:77, compute/azure/container-apps/main.tf:146). GCP is the only path where an unrelated feature flag can turn off request authentication.

3. Key Vault authorization mechanism. compute/azure/cleanup-function/main.tf:31-40 and compute/azure/aks/main.tf:105-114 grant access with azurerm_key_vault_access_policy against a vault that sets enable_rbac_authorization = true (secrets/azure/main.tf:39), so the grants are inert. Every other consumer in the repo already uses azurerm_role_assignment (container-apps/main.tf:309, container-apps/scheduled-tasks.tf:439-461, secrets/azure/main.tf:295-301).

4. Lambda log group naming. compute/aws/lambda/main.tf:485-490 uses name_prefix, so the group never receives the function's logs. The sibling compute/aws/cleanup-lambda/main.tf:49-53 uses an exact name and is correct.

5. Secret-bearing variables marked sensitive. compute/azure/container-apps/variables.tf:154-161 and secrets/aws/variables.tf:79-86 are not marked sensitive, while secrets/azure/variables.tf:130-135 is, complete with the nonsensitive(keys(...)) workaround at secrets/azure/main.tf:254 that makes it usable with for_each.

6. Runtime secret fetch in a Logic App. compute/azure/cleanup-function/main.tf:132-135 uses the ARM-template function listKeys() inside a workflow definition, where it is not a valid workflow expression. The sibling container-apps/scheduled-tasks.tf:150-185 solves the identical problem correctly with a managed identity.

7. create_before_destroy on replaceable resources. compute/aws/fargate/main.tf:432-458 (aws_lb_target_group) omits it, while every other replaceable resource in the repo carries it (networking/aws/main.tf:548-550, 619-621, compute/aws/lambda/main.tf:189-191, compute/aws/lambda/signing-key.tf:25-27).

8. Edge WAF coverage. frontend/azure/main.tf:153-164 pins a Front Door SKU that cannot attach a WAF, while AWS supports an optional WAF (frontend/aws/main.tf:38) and GCP ships Cloud Armor on by default (frontend/gcp/main.tf:182-247).

9. Database ingress scoping. networking/aws/main.tf:503-524 and database/aws/main.tf:86-97 admit tcp/5432 from the whole VPC CIDR, while the Azure sibling narrows to the container-apps subnet CIDR (networking/azure/main.tf:149-161).

Why this belongs on this issue

The proposed fix here is parity tests in CI that assert the guard set is equivalent across paths for each onboarding job. The same mechanism, applied one level down, would catch all nine items above: assert that for each capability (secret access, request auth, log destination, edge filtering, database ingress) the per-cloud modules implement an equivalent guard, and fail CI on divergence. Scoping the parity work to the customer-facing templates alone leaves the larger surface uncovered.

Items 1 through 4 are filed as their own issues from this review; items 5 through 9 are carried in the grouped chore(terraform/modules) medium/low issue. This comment exists so the parity-test design accounts for both axes rather than only the onboarding one.

[cristim]

PR #459 addresses only AWS CLI WIF guard parity: exact OIDC provider selection, fail-closed list/get errors, provider audience/thumbprint checks, input validation and safe trust JSON. Independent local review approved commit 944b87e; generated Bash regression tests reproduce the original failures and pass with the fix using local CLI stubs. CI is pending; no live cloud mutations were performed.

This issue must remain open. Remaining work includes Azure CLI credential handling, GCP issuer parity, Azure Bicep/ARM role parity, host prerequisites including #131, and cross-path CI/documentation plus shared AWS issuer URL hardening. Existing Terraform issuer syntax is mirrored by #459, not fully hardened.

[cristim]

PR #459 merged as cdccc0a. The three scoped files are byte-identical to independently reviewed commit 944b87e.

Post-merge local verification extracted the actual merged Go package/templates into a separate temporary directory and ran go test -race -count=1 ./internal/iacfiles -run TestAWSWIFCLI with Go 1.26.6: passed. These tests execute generated Bash with recording AWS/curl stubs and real jq, covering exact provider selection, failure handling, audience defaults and literal trust JSON. No live IAM, deployment, registration or purchase occurred; live AWS acceptance was not tested.

Keep #91 open: Azure CLI/TF identity guards, GCP issuer parity, Azure Bicep/ARM role parity, host prerequisites including #131, and cross-path CI/documentation plus shared AWS issuer URL hardening remain unfinished.

[cristim]

PR #461 adds bounded Azure task 2: both credential shell templates fail closed, validate inputs before Azure calls and serialize literal claims with jq; Terraform applies matching validation and 24 mock-plan cases run in CI. Independent review approved fa307af after baseline/fixed generated-Bash and Terraform evidence. CI is pending. Verification is local CLI-stub/mock-provider evaluation only; no live Azure changes occurred.

Keep #91 open after this PR: GCP issuer parity, Azure Bicep/ARM role parity, host prerequisites including #131, and cross-path CI/documentation plus shared AWS issuer URL hardening remain unfinished.