sec(iac/aws): require OIDC subject claim in the federation bundle generator

internal/api/handler_federation.go

@@ -30,6 +30,15 @@ type federationIaCData struct {
 	OIDCIssuerURL  string
 	OIDCIssuerHost string // issuer URL without https:// prefix (used as IAM condition key)
 	OIDCAudience   string
+	// OIDCSubjectClaim restricts the AWS trust policy to a single workload
+	// subject. Deliberately left empty by every generic-bundle builder below —
+	// CUDly's server has no generic way to know the calling workload's real
+	// subject claim (it is not derivable from target/source alone the way
+	// OIDCIssuerURL/OIDCAudience are). Every AWS-WIF template still emits this
+	// field as a required, uncommented value so the operator must fill it in
+	// before the bundle deploys/applies/runs, rather than the bundle silently
+	// working with no :sub condition (see #1543, #1602, #1640).
+	OIDCSubjectClaim string
 	// Azure-specific
 	SubscriptionID string
 	TenantID       string
@@ -282,6 +291,7 @@ func shellEscapeData(data federationIaCData) federationIaCData {
 	d.OIDCIssuerURL = shellEscape(data.OIDCIssuerURL)
 	d.OIDCIssuerHost = shellEscape(data.OIDCIssuerHost)
 	d.OIDCAudience = shellEscape(data.OIDCAudience)
+	d.OIDCSubjectClaim = shellEscape(data.OIDCSubjectClaim)
 	d.SubscriptionID = shellEscape(data.SubscriptionID)
 	d.TenantID = shellEscape(data.TenantID)
 	d.ProjectID = shellEscape(data.ProjectID)
@@ -769,6 +779,7 @@ func buildCFParamsJSON(data federationIaCData, source string) (string, error) {
 			{ParameterKey: "OIDCIssuerURL", ParameterValue: data.OIDCIssuerURL},
 			{ParameterKey: "OIDCIssuerHost", ParameterValue: strings.TrimPrefix(data.OIDCIssuerURL, "https://")},
 			{ParameterKey: "OIDCAudience", ParameterValue: data.OIDCAudience},
+			{ParameterKey: "OIDCSubjectClaim", ParameterValue: data.OIDCSubjectClaim},
 			{ParameterKey: "RoleName", ParameterValue: "CUDly-" + data.AccountSlug},
 		}
 	}

internal/api/handler_federation_test.go

@@ -387,6 +387,111 @@ func TestGetFederationIaC_CFNZip_ParamsValidJSON(t *testing.T) {
 	assert.GreaterOrEqual(t, len(params), 3, "should have at least 3 parameters")
 }
 
+// TestGetFederationIaC_AWSWIF_SubjectClaimThreaded is the regression test for
+// #1640: the CFN params JSON and deploy script, and the Terraform tfvars,
+// used to omit OIDCSubjectClaim / oidc_subject_claim entirely (CFN) or emit it
+// commented-out and mislabeled "Optional" (tfvars), even though the
+// CloudFormation template (#1602) and Terraform module both require it with
+// no default. Every AWS-WIF artifact must now emit the parameter/variable
+// explicitly so the deploy fails with a specific, actionable error instead of
+// either "must have values" (CFN) or a misleading "Optional" comment (tfvars).
+func TestGetFederationIaC_AWSWIF_SubjectClaimThreaded(t *testing.T) {
+	h := federationHandler()
+	ctx := context.Background()
+
+	t.Run("cfn params JSON", func(t *testing.T) {
+		res, err := h.getFederationIaC(ctx, federationReq(map[string]string{
+			"target": "aws", "source": "gcp", "format": "cfn",
+		}))
+		require.NoError(t, err)
+		zipBytes, err := base64.StdEncoding.DecodeString(res.Content)
+		require.NoError(t, err)
+		zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
+		require.NoError(t, err)
+
+		var paramsFile *zip.File
+		for _, f := range zr.File {
+			if strings.HasSuffix(f.Name, "-cf-params.json") {
+				paramsFile = f
+				break
+			}
+		}
+		require.NotNil(t, paramsFile)
+		rc, err := paramsFile.Open()
+		require.NoError(t, err)
+		defer rc.Close()
+		var buf bytes.Buffer
+		_, err = buf.ReadFrom(rc)
+		require.NoError(t, err)
+
+		var params []map[string]string
+		require.NoError(t, json.Unmarshal(buf.Bytes(), &params))
+		var found bool
+		for _, p := range params {
+			if p["ParameterKey"] == "OIDCSubjectClaim" {
+				found = true
+			}
+		}
+		assert.True(t, found, "cf-params.json must include an OIDCSubjectClaim entry")
+	})
+
+	t.Run("cfn deploy script", func(t *testing.T) {
+		res, err := h.getFederationIaC(ctx, federationReq(map[string]string{
+			"target": "aws", "source": "gcp", "format": "cfn",
+		}))
+		require.NoError(t, err)
+		zipBytes, err := base64.StdEncoding.DecodeString(res.Content)
+		require.NoError(t, err)
+		zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
+		require.NoError(t, err)
+
+		var deployScript string
+		for _, f := range zr.File {
+			if f.Name == "cloudformation/deploy-cfn.sh" {
+				rc, err := f.Open()
+				require.NoError(t, err)
+				var buf bytes.Buffer
+				_, _ = buf.ReadFrom(rc)
+				rc.Close()
+				deployScript = buf.String()
+			}
+		}
+		require.NotEmpty(t, deployScript)
+		assert.Contains(t, deployScript, `"OIDCSubjectClaim=`,
+			"deploy script must pass OIDCSubjectClaim in --parameter-overrides")
+	})
+
+	t.Run("tfvars", func(t *testing.T) {
+		res, err := h.getFederationIaC(ctx, federationReq(map[string]string{
+			"target": "aws", "source": "gcp", "format": "bundle",
+		}))
+		require.NoError(t, err)
+		zipBytes, err := base64.StdEncoding.DecodeString(res.Content)
+		require.NoError(t, err)
+		zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
+		require.NoError(t, err)
+
+		var tfvars string
+		for _, f := range zr.File {
+			if strings.HasSuffix(f.Name, ".auto.tfvars") {
+				rc, err := f.Open()
+				require.NoError(t, err)
+				var buf bytes.Buffer
+				_, _ = buf.ReadFrom(rc)
+				rc.Close()
+				tfvars = buf.String()
+			}
+		}
+		require.NotEmpty(t, tfvars)
+		assert.Contains(t, tfvars, "\noidc_subject_claim = \"",
+			"tfvars must emit oidc_subject_claim uncommented, at the start of a line")
+		assert.NotContains(t, tfvars, `# oidc_subject_claim = `,
+			"oidc_subject_claim must not be commented out as a variable assignment")
+		assert.NotContains(t, tfvars, "Optional: restrict trust to a specific workload subject claim",
+			"oidc_subject_claim is required, not optional — the module has no default and rejects empty")
+	})
+}
+
 func TestSingleFileSpec_CLI_AllScenarios(t *testing.T) {
 	cases := []struct{ target, source, wantContains string }{
 		{"aws", "aws", "aws-cross-account-cli.sh"},

internal/iacfiles/templates/README.md

@@ -49,8 +49,25 @@ from this directory via the `//go:embed` directive in `internal/iacfiles/embed.g
 
 ### Locally from the cloned repo
 
-Use `scripts/generate-federation-iac.go` — a self-contained Go script with no
-external dependencies:
+Use `scripts/generate-federation-iac.go`, a self-contained Go script with no
+external dependencies.
+
+Every AWS target with a non-AWS source requires `--oidc-subject-claim`: it is
+the workload subject the generated AWS trust policy pins to, and there is no
+working default (see #1640). Pass the calling workload's subject claim, which is
+a GCP service account's numeric unique ID or an Azure managed identity's object
+ID. On the other combinations the flag feeds nothing, so passing it is an error
+rather than a silent no-op.
+
+That is not the same as saying those bundles need no pinning. Each GCP target
+combination emits its own **required** pin for you to fill in before
+`terraform apply`, none of which `--oidc-subject-claim` populates:
+
+| `--source` | file | variable to fill in |
+|---|---|---|
+| `aws` | `<slug>-gcp-wif.tfvars` | `aws_role_name` (blank) |
+| `azure` | `<slug>-gcp-wif.tfvars` | `oidc_subject` (blank) |
+| `gcp` | `<slug>-gcp-sa-impersonation.tfvars` | `source_service_account` (placeholder) |
 
 ```bash
 # Run from the repository root
@@ -59,14 +76,16 @@ external dependencies:
 go run scripts/generate-federation-iac.go \
   --target aws --source azure \
   --account-name "prod-aws" --account-id "123456789012" \
-  --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
+  --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \
+  --oidc-subject-claim "11111111-2222-3333-4444-555555555555"
 
 # AWS target, GCP source
 go run scripts/generate-federation-iac.go \
   --target aws --source gcp \
-  --account-name "prod-aws" --account-id "123456789012"
+  --account-name "prod-aws" --account-id "123456789012" \
+  --oidc-subject-claim "123456789012345678901"
 
-# AWS target, AWS source (cross-account role, no WIF)
+# AWS target, AWS source (cross-account role, no WIF, no subject claim)
 go run scripts/generate-federation-iac.go \
   --target aws --source aws \
   --account-name "target-aws" --account-id "999888777666"
@@ -75,7 +94,8 @@ go run scripts/generate-federation-iac.go \
 go run scripts/generate-federation-iac.go \
   --target aws --source azure --format cf-params \
   --account-name "prod-aws" --account-id "123456789012" \
-  --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
+  --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \
+  --oidc-subject-claim "11111111-2222-3333-4444-555555555555"
 
 # Azure target
 go run scripts/generate-federation-iac.go \
@@ -98,6 +118,7 @@ go run scripts/generate-federation-iac.go \
   --target aws --source azure \
   --account-name "prod" --account-id "123456789012" \
   --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \
+  --oidc-subject-claim "11111111-2222-3333-4444-555555555555" \
   --output -
 ```
 

internal/iacfiles/templates/aws-cfn-deploy.sh.tmpl

@@ -35,6 +35,7 @@ aws cloudformation deploy \
     "OIDCIssuerURL={{.OIDCIssuerURL}}" \
     "OIDCIssuerHost={{.OIDCIssuerHost}}" \
     "OIDCAudience={{.OIDCAudience}}" \
+    "OIDCSubjectClaim={{.OIDCSubjectClaim}}" \
     "RoleName=CUDly-{{.AccountSlug}}" \
   --capabilities CAPABILITY_NAMED_IAM \
   --no-fail-on-empty-changeset

internal/iacfiles/templates/aws-wif-cf-params.json.tmpl

@@ -1,5 +1,6 @@
 [
-  { "ParameterKey": "OIDCIssuerURL", "ParameterValue": "{{.OIDCIssuerURL}}" },
-  { "ParameterKey": "OIDCAudience",  "ParameterValue": "{{.OIDCAudience}}" },
-  { "ParameterKey": "RoleName",      "ParameterValue": "CUDly-{{.AccountSlug}}" }
+  { "ParameterKey": "OIDCIssuerURL",    "ParameterValue": "{{.OIDCIssuerURL}}" },
+  { "ParameterKey": "OIDCAudience",     "ParameterValue": "{{.OIDCAudience}}" },
+  { "ParameterKey": "OIDCSubjectClaim", "ParameterValue": "{{.OIDCSubjectClaim}}" },
+  { "ParameterKey": "RoleName",         "ParameterValue": "CUDly-{{.AccountSlug}}" }
 ]

internal/iacfiles/templates/aws-wif-cli.sh.tmpl

@@ -12,9 +12,33 @@ set -euo pipefail
 ROLE_NAME="${ROLE_NAME:-CUDly-{{.AccountSlug}}}"
 OIDC_ISSUER_URL="${OIDC_ISSUER_URL:-{{.OIDCIssuerURL}}}"
 OIDC_AUDIENCE="${OIDC_AUDIENCE:-{{.OIDCAudience}}}"
-# Optional: restrict which OIDC subject can assume the role.
-# Useful for multi-tenant issuers. Matches TF variable oidc_subject_claim.
-OIDC_SUBJECT_CLAIM="${OIDC_SUBJECT_CLAIM:-}"
+# REQUIRED: restricts which OIDC subject can assume the role. Without a :sub
+# condition the trust policy accepts every identity the issuer can mint — the
+# same hole #1543/#1602 closed in the CloudFormation template — so this is
+# validated below rather than left to build a subject-less policy silently.
+# Matches the required TF variable oidc_subject_claim and the required
+# CloudFormation OIDCSubjectClaim parameter.
+OIDC_SUBJECT_CLAIM="${OIDC_SUBJECT_CLAIM:-{{.OIDCSubjectClaim}}}"
+if [[ -z "${OIDC_SUBJECT_CLAIM}" ]]; then
+  echo "Error: OIDC_SUBJECT_CLAIM is required. Without it the trust policy has no" >&2
+  echo "       :sub condition and every identity ${OIDC_ISSUER_URL} can mint is able" >&2
+  echo "       to assume this role. Set it to the calling workload's subject claim:" >&2
+  echo "         GCP service account : its numeric unique ID (not the email)" >&2
+  echo "         Azure managed identity: the object ID of the managed identity" >&2
+  echo "       e.g. OIDC_SUBJECT_CLAIM=123456789012345678901 bash $0" >&2
+  exit 1
+fi
+case "${OIDC_SUBJECT_CLAIM}" in
+  *[[:space:]]*|*'$'*|*'*'*)
+    echo "Error: OIDC_SUBJECT_CLAIM must not contain whitespace, '\$' or '*'." >&2
+    echo "       IAM expands \${...} policy variables inside Condition values, so a" >&2
+    echo "       value such as \${accounts.google.com:sub} would expand to the token's" >&2
+    echo "       own sub claim and match every identity the issuer can mint. '*' is" >&2
+    echo "       compared literally by StringEquals and would silently produce a role" >&2
+    echo "       nobody can assume." >&2
+    exit 1
+    ;;
+esac
 PROFILE_ARG=""
 if [[ -n "${AWS_PROFILE:-}" ]]; then PROFILE_ARG="--profile ${AWS_PROFILE}"; fi
 
@@ -37,9 +61,9 @@ PROVIDER_ARN=$(aws iam list-open-id-connect-providers $PROFILE_ARG \
   --query "OpenIDConnectProviderList[?contains(Arn, '${OIDC_HOST}')].Arn | [0]" \
   --output text)
 
-# Build trust policy — add subject claim restriction when OIDC_SUBJECT_CLAIM is set.
-if [[ -n "${OIDC_SUBJECT_CLAIM}" ]]; then
-  TRUST_POLICY=$(cat <<JSON
+# Build trust policy. OIDC_SUBJECT_CLAIM is validated non-empty above, so the
+# :sub condition below is always present — there is no branch that omits it.
+TRUST_POLICY=$(cat <<JSON
 {
   "Version": "2012-10-17",
   "Statement": [{
@@ -54,20 +78,6 @@ if [[ -n "${OIDC_SUBJECT_CLAIM}" ]]; then
 }
 JSON
 )
-else
-  TRUST_POLICY=$(cat <<JSON
-{
-  "Version": "2012-10-17",
-  "Statement": [{
-    "Effect": "Allow",
-    "Principal": {"Federated": "${PROVIDER_ARN}"},
-    "Action": "sts:AssumeRoleWithWebIdentity",
-    "Condition": {"StringEquals": {"${OIDC_HOST}:aud": "${OIDC_AUDIENCE}"}}
-  }]
-}
-JSON
-)
-fi
 
 PERMISSIONS_POLICY=$(cat <<'JSON'
 {

internal/iacfiles/templates/aws-wif.tfvars.tmpl

@@ -11,8 +11,14 @@ oidc_issuer_url = "{{.OIDCIssuerURL}}"
 oidc_audience   = "{{.OIDCAudience}}"
 role_name       = "CUDly-{{.AccountSlug}}"
 
-# Optional: restrict trust to a specific workload subject claim
-# oidc_subject_claim = ""
+# REQUIRED: restrict trust to a specific workload subject claim. The module's
+# oidc_subject_claim variable has no default and rejects an empty value —
+# terraform apply fails until this is filled in with the calling workload's
+# real subject (GCP service account numeric unique ID, Azure managed identity
+# object ID, etc.). Leaving it empty is intentionally not a working default:
+# without a :sub condition the trust policy accepts every identity the issuer
+# can mint.
+oidc_subject_claim = "{{.OIDCSubjectClaim}}"
 
 # --- CUDly auto-registration (optional) ---
 cudly_api_url = "{{.CUDlyAPIURL}}"