chore(iac): derive the CFN issuer host, create the GCP signing key first, read the dev Function URL from state

.gitallowed

@@ -89,6 +89,7 @@
 ^internal/database/postgres/migrations/migrate_security_test\.go:[0-9]+:[[:space:]]*rawDSN[[:space:]]+:=[[:space:]]+fmt\.Sprintf\("postgre[s]://user:pass@localhost:5432/db\?sslmode=%s",[[:space:]]+mode\)[[:space:]]*$
 ^internal/database/postgres/migrations/migrate_security_test\.go:[0-9]+:[[:space:]]*poolCfg,[[:space:]]+err[[:space:]]+:=[[:space:]]+pgxpool\.ParseConfig\("postgre[s]://user:pass@localhost:5432/db\?sslmode=require"\)[[:space:]]*$
 ^internal/database/postgres/migrations/migrate_security_test\.go:[0-9]+:[[:space:]]*cfg,[[:space:]]+err[[:space:]]+:=[[:space:]]+pgxpool\.ParseConfig\("postgre[s]://user:pass@127\.0\.0\.1:1/db\?sslmode=disable"\)[[:space:]]*$
+^terraform/environments/aws/tests/lambda_cors_origins\.tftest\.hcl:[0-9]+:[[:space:]]*defaults[[:space:]]+=[[:space:]]+\{[[:space:]]+account_id[[:space:]]+=[[:space:]]+"123[4]567[8]901[2]"[[:space:]]+\}[[:space:]]*$
 ^terraform/environments/(aws|azure|gcp)/dev\.tfvars\.example:[0-9]+:[[:space:]]*#[[:space:]]+admin_passwor[d][[:space:]]+=[[:space:]]+"YourSecurePassword123!"[[:space:]]*$
 ^terraform/modules/secrets/azure/main\.tf:[0-9]+:[[:space:]]*value[[:space:]]+=[[:space:]]+var\.smtp_passwor[d][[:space:]]+!=[[:space:]]+null[[:space:]]+\?[[:space:]]+var\.smtp_passwor[d][[:space:]]+:[[:space:]]+"PLACEHOLDER_GENERATE_IN_AZURE_PORTAL"[[:space:]]*$
 ^terraform/modules/secrets/gcp/main\.tf:[0-9]+:[[:space:]]*secret_data[[:space:]]+=[[:space:]]+var\.sendgrid_api_ke[y][[:space:]]+!=[[:space:]]+null[[:space:]]+\?[[:space:]]+var\.sendgrid_api_ke[y][[:space:]]+:[[:space:]]+"PLACEHOLDER_REPLACE_ME"[[:space:]]*$

.github/workflows/ci.yml

@@ -723,6 +723,19 @@ jobs:
             terraform -chdir="$d" test
           fi
 
+      # The Cloud Run OIDC signing key's create-time retention input must keep
+      # deployed keys unchanged and reject values Cloud KMS refuses (#324).
+      - name: Test GCP compute modules
+        if: matrix.cloud == 'gcp'
+        run: |
+          for t in terraform/modules/compute/gcp/*/tests; do
+            [ -d "$t" ] || continue
+            d=$(dirname "$t")
+            terraform -chdir="$d" init -backend=false -lockfile=readonly -input=false
+            terraform -chdir="$d" fmt -check -recursive
+            terraform -chdir="$d" test
+          done
+
       - name: Test CI/CD trust (immutable ids)
         # Skips a cloud until it has a tests dir. The lock file is gitignored,
         # so no -lockfile=readonly.
@@ -1251,6 +1264,12 @@ jobs:
       - name: Run state namespace self-tests
         run: bash scripts/test-aws-tfstate-platform-key.sh
 
+      # The Lambda deploy and rollback read the Function URL back from state
+      # for its CORS origin (#324); the reader must reject anything that is
+      # not a Function URL. terraform is stubbed, so nothing reads real state.
+      - name: Run Function URL reader self-tests
+        run: bash scripts/test-read-lambda-function-url.sh
+
   # Assert that no Terraform file declares an azurerm_key_vault_access_policy
   # resource. This project's only Key Vault sets enable_rbac_authorization =
   # true, and an RBAC-enabled vault ignores access policies entirely, so such a

.github/workflows/deploy-aws-lambda.yml

@@ -271,6 +271,17 @@ jobs:
           cd terraform/environments/aws
           terraform init -backend-config=/tmp/backend.tfbackend
 
+      # The Function URL's host is server-assigned, so its origin is read from
+      # the previous apply instead of being committed to github-*.tfvars, where
+      # it went stale whenever the URL was reissued (#324). Empty on the first
+      # deploy; "Check CORS covers the Function URL" below flags that case.
+      - name: Read deployed Function URL
+        id: deployed-url
+        run: |
+          set -euo pipefail
+          url=$(scripts/read-lambda-function-url.sh terraform/environments/aws)
+          echo "url=${url}" >> "$GITHUB_OUTPUT"
+
       - name: Terraform Plan
         id: plan
         env:
@@ -285,6 +296,7 @@ jobs:
           # accepted by the var (the Terraform local then falls back to
           # frontend_domain_names[0]).
           TF_VAR_dashboard_url: ${{ secrets.DASHBOARD_URL }}
+          TF_VAR_lambda_deployed_function_url: ${{ steps.deployed-url.outputs.url }}
           ENVIRONMENT: ${{ needs.prepare.outputs.target_environment }}
         run: |
           cd terraform/environments/aws
@@ -334,6 +346,19 @@ jobs:
             echo "log_group=$(terraform output -raw lambda_log_group_name 2>/dev/null | grep -v '::' || echo "")"
           } >> "$GITHUB_OUTPUT"
 
+      # The allowlist this apply wrote holds the origin read before the plan.
+      # If the apply created or reissued the Function URL, the live origin is
+      # not in it until the next deploy, which reads the new URL back.
+      - name: Check CORS covers the Function URL
+        env:
+          PLANNED_URL: ${{ steps.deployed-url.outputs.url }}
+        run: |
+          set -euo pipefail
+          live=$(scripts/read-lambda-function-url.sh terraform/environments/aws)
+          if [ -n "$live" ] && [ "$live" != "$PLANNED_URL" ]; then
+            echo "::warning::The Function URL is now ${live}, but this apply's CORS allowlist was built from '${PLANNED_URL}'. Re-run this workflow so the next apply adds the new origin."
+          fi
+
       - name: Save deployment info
         env:
           ENVIRONMENT: ${{ needs.prepare.outputs.target_environment }}

.github/workflows/rollback.yml

@@ -278,6 +278,11 @@ jobs:
           cd terraform/environments/aws
           terraform init -backend-config=/tmp/backend.tfbackend
 
+          # Keep the deployed Function URL's origin in the CORS allowlist, as
+          # deploy-aws-lambda.yml does; it is not in github-*.tfvars (#324).
+          TF_VAR_lambda_deployed_function_url=$(../../../scripts/read-lambda-function-url.sh .)
+          export TF_VAR_lambda_deployed_function_url
+
           terraform apply -auto-approve \
             -var-file="github-${ENVIRONMENT}.tfvars" \
             -var="image_uri=${IMAGE_URI}" \

iac/aws_target_cfn_test.go

@@ -0,0 +1,220 @@
+package iac
+
+import (
+	"fmt"
+	"regexp"
+	"sort"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
+)
+
+// Regression tests for #324 (audit finding A13b-014). The aws-target
+// CloudFormation template used to take the trust policy's condition-key host
+// as a second free-text parameter, OIDCIssuerHost, that the operator had to
+// keep equal to OIDCIssuerURL minus https:// by hand. A mismatch produced a
+// stack that deployed cleanly and then denied every AssumeRoleWithWebIdentity.
+// The host is now derived from OIDCIssuerURL, the way the Terraform sibling's
+// local.oidc_condition_host derives it.
+
+const awsTargetCFNTemplate = "federation/aws-target/cloudformation/template.yaml"
+
+// mapValue returns the value node for key in a YAML mapping node, or nil.
+func mapValue(n *yaml.Node, key string) *yaml.Node {
+	if n == nil || n.Kind != yaml.MappingNode {
+		return nil
+	}
+	for i := 0; i+1 < len(n.Content); i += 2 {
+		if n.Content[i].Kind == yaml.ScalarNode && n.Content[i].Value == key {
+			return n.Content[i+1]
+		}
+	}
+	return nil
+}
+
+// mapPath follows keys from n through nested mappings, failing the test when
+// any step is missing.
+func mapPath(t *testing.T, n *yaml.Node, keys ...string) *yaml.Node {
+	t.Helper()
+	for _, k := range keys {
+		next := mapValue(n, k)
+		require.NotNil(t, next, "template has no %q under the path %v", k, keys)
+		n = next
+	}
+	return n
+}
+
+func loadAWSTargetCFN(t *testing.T) *yaml.Node {
+	t.Helper()
+	raw, err := Modules.ReadFile(awsTargetCFNTemplate)
+	require.NoError(t, err)
+	var doc yaml.Node
+	require.NoError(t, yaml.Unmarshal(raw, &doc))
+	require.Len(t, doc.Content, 1)
+	return doc.Content[0]
+}
+
+// cfnEval evaluates the subset of CloudFormation intrinsics the trust
+// policy's condition keys use (!Ref, !Select, !Split, !Sub) against the given
+// parameter values. Anything else is an error, so a rewrite into a form this
+// cannot model fails the test instead of passing unevaluated.
+func cfnEval(n *yaml.Node, params map[string]string) (any, error) {
+	switch n.Tag {
+	case "!!str", "!!int":
+		return n.Value, nil
+	case "!Ref":
+		v, ok := params[n.Value]
+		if !ok {
+			return nil, fmt.Errorf("!Ref to %q, which is not a parameter value under test", n.Value)
+		}
+		return v, nil
+	case "!Split":
+		args, err := cfnArgs(n, 2, params)
+		if err != nil {
+			return nil, err
+		}
+		delim, s, ok := twoStrings(args)
+		if !ok {
+			return nil, fmt.Errorf("!Split arguments are not strings: %v", args)
+		}
+		return strings.Split(s, delim), nil
+	case "!Select":
+		args, err := cfnArgs(n, 2, params)
+		if err != nil {
+			return nil, err
+		}
+		idx, err := strconv.Atoi(fmt.Sprint(args[0]))
+		list, ok := args[1].([]string)
+		if err != nil || !ok || idx < 0 || idx >= len(list) {
+			return nil, fmt.Errorf("!Select %v out of range or not a list", args)
+		}
+		return list[idx], nil
+	case "!Sub":
+		return cfnSub(n, params)
+	}
+	return nil, fmt.Errorf("unsupported node tag %q (value %q)", n.Tag, n.Value)
+}
+
+// cfnArgs evaluates the elements of a sequence-form intrinsic.
+func cfnArgs(n *yaml.Node, want int, params map[string]string) ([]any, error) {
+	if n.Kind != yaml.SequenceNode || len(n.Content) != want {
+		return nil, fmt.Errorf("%s wants a %d-element list", n.Tag, want)
+	}
+	out := make([]any, 0, want)
+	for _, c := range n.Content {
+		v, err := cfnEval(c, params)
+		if err != nil {
+			return nil, err
+		}
+		out = append(out, v)
+	}
+	return out, nil
+}
+
+func twoStrings(args []any) (string, string, bool) {
+	a, ok1 := args[0].(string)
+	b, ok2 := args[1].(string)
+	return a, b, ok1 && ok2
+}
+
+var subVarRe = regexp.MustCompile(`\$\{([^}!]+)\}`)
+
+// cfnSub renders both the string and the [string, {vars}] forms of !Sub.
+func cfnSub(n *yaml.Node, params map[string]string) (any, error) {
+	format, vars := n, map[string]string{}
+	if n.Kind == yaml.SequenceNode {
+		if len(n.Content) != 2 || n.Content[1].Kind != yaml.MappingNode {
+			return nil, fmt.Errorf("!Sub list form wants [string, mapping]")
+		}
+		format = n.Content[0]
+		m := n.Content[1]
+		for i := 0; i+1 < len(m.Content); i += 2 {
+			v, err := cfnEval(m.Content[i+1], params)
+			if err != nil {
+				return nil, err
+			}
+			s, ok := v.(string)
+			if !ok {
+				return nil, fmt.Errorf("!Sub variable %q is not a string", m.Content[i].Value)
+			}
+			vars[m.Content[i].Value] = s
+		}
+	}
+	var missing error
+	out := subVarRe.ReplaceAllStringFunc(format.Value, func(ref string) string {
+		name := subVarRe.FindStringSubmatch(ref)[1]
+		if v, ok := vars[name]; ok {
+			return v
+		}
+		if v, ok := params[name]; ok {
+			return v
+		}
+		missing = fmt.Errorf("!Sub references %q, which is neither a variable nor a parameter value under test", name)
+		return ref
+	})
+	return out, missing
+}
+
+// terraformConditionHost mirrors local.oidc_condition_host in
+// federation/aws-target/terraform/main.tf.
+func terraformConditionHost(url string) string {
+	return strings.TrimSuffix(strings.TrimPrefix(url, "https://"), "/")
+}
+
+func TestAWSTargetCFN_HasNoIssuerHostParameter(t *testing.T) {
+	root := loadAWSTargetCFN(t)
+	params := mapPath(t, root, "Parameters")
+	require.Nil(t, mapValue(params, "OIDCIssuerHost"),
+		"OIDCIssuerHost must not be a parameter: the condition-key host is derived from OIDCIssuerURL (#324)")
+	raw, err := Modules.ReadFile(awsTargetCFNTemplate)
+	require.NoError(t, err)
+	require.NotContains(t, string(raw), "${OIDCIssuerHost}", "nothing may still reference the removed parameter")
+}
+
+// TestAWSTargetCFN_ConditionKeysDeriveFromIssuerURL evaluates every key of the
+// trust policy's StringEquals block for real issuer URLs and checks it is the
+// host Terraform derives, followed by :aud or :sub.
+func TestAWSTargetCFN_ConditionKeysDeriveFromIssuerURL(t *testing.T) {
+	root := loadAWSTargetCFN(t)
+	urlPattern := regexp.MustCompile(mapPath(t, root, "Parameters", "OIDCIssuerURL", "AllowedPattern").Value)
+	statements := mapPath(t, root, "Resources", "CUDlyRole", "Properties", "AssumeRolePolicyDocument", "Statement")
+	require.Equal(t, yaml.SequenceNode, statements.Kind)
+	require.Len(t, statements.Content, 1, "the trust policy has exactly one statement")
+	stringEquals := mapPath(t, statements.Content[0], "Condition", "StringEquals")
+
+	for _, url := range []string{
+		"https://accounts.google.com",
+		"https://login.microsoftonline.com/11111111-1111-1111-1111-111111111111/v2.0",
+	} {
+		t.Run(url, func(t *testing.T) {
+			require.True(t, urlPattern.MatchString(url), "fixture must satisfy OIDCIssuerURL's AllowedPattern")
+			host := terraformConditionHost(url)
+			var keys []string
+			for i := 0; i+1 < len(stringEquals.Content); i += 2 {
+				v, err := cfnEval(stringEquals.Content[i], map[string]string{"OIDCIssuerURL": url})
+				require.NoError(t, err, "condition key %d must be computed from OIDCIssuerURL alone", i/2)
+				keys = append(keys, v.(string))
+			}
+			sort.Strings(keys)
+			require.Equal(t, []string{host + ":aud", host + ":sub"}, keys)
+		})
+	}
+}
+
+// TestCFNEval_Positive is the control for the evaluator above: it must
+// resolve the derivation the template uses, and reject an unknown parameter,
+// so the condition-key test cannot pass by evaluating nothing.
+func TestCFNEval_Positive(t *testing.T) {
+	var n yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(`!Sub ["${Host}:aud", {Host: !Select [1, !Split ["https://", !Ref OIDCIssuerURL]]}]`), &n))
+	got, err := cfnEval(n.Content[0], map[string]string{"OIDCIssuerURL": "https://accounts.google.com"})
+	require.NoError(t, err)
+	require.Equal(t, "accounts.google.com:aud", got)
+
+	require.NoError(t, yaml.Unmarshal([]byte(`!Sub "${OIDCIssuerHost}:aud"`), &n))
+	_, err = cfnEval(n.Content[0], map[string]string{"OIDCIssuerURL": "https://accounts.google.com"})
+	require.Error(t, err)
+}

iac/federation/aws-target/cloudformation/template.yaml

@@ -9,25 +9,15 @@ Parameters:
     Type: String
     Description: >
       Full OIDC issuer URL (with https://) — used for the IAM OIDC provider Url property.
-      Must NOT contain a trailing slash; the value here must match OIDCIssuerHost
-      (minus the https:// prefix) exactly because AWS IAM condition keys are
-      case-sensitive and string-equal.
+      Must NOT contain a trailing slash. The trust policy's condition-key host
+      is derived from this value (the URL with https:// removed), so the two
+      cannot disagree; AWS IAM condition keys are case-sensitive and
+      string-equal.
       Azure AD: https://login.microsoftonline.com/<tenant_id>/v2.0
       GCP:      https://accounts.google.com
     AllowedPattern: "^https://[^/]+(/[^/]+)*$"
     ConstraintDescription: "OIDCIssuerURL must be https:// and must not contain a trailing slash."
 
-  OIDCIssuerHost:
-    Type: String
-    Description: >
-      OIDC issuer host and path WITHOUT the https:// prefix — used as the IAM
-      condition key. Must equal OIDCIssuerURL with the https:// stripped and no
-      trailing slash; the operator is responsible for keeping the two in sync.
-      Azure AD: login.microsoftonline.com/<tenant_id>/v2.0
-      GCP:      accounts.google.com
-    AllowedPattern: "^[^/]+(/[^/]+)*$"
-    ConstraintDescription: "OIDCIssuerHost must not start or end with a slash."
-
   OIDCAudience:
     Type: String
     Description: >
@@ -267,11 +257,22 @@ Resources:
             Principal:
               Federated: !GetAtt OIDCProvider.Arn
             Action: sts:AssumeRoleWithWebIdentity
+            # The condition-key host is OIDCIssuerURL minus its https://
+            # prefix, derived here rather than taken as a second parameter so
+            # the two cannot disagree (#324). OIDCIssuerURL's AllowedPattern
+            # guarantees exactly one leading https:// and no trailing slash, so
+            # element 1 of the split is the whole host and path. This is the
+            # same value as local.oidc_condition_host in the Terraform module.
             Condition:
               StringEquals:
-                !Sub "${OIDCIssuerHost}:aud":
-                  !If [HasAudience, !Ref OIDCAudience, sts.amazonaws.com]
-                !Sub "${OIDCIssuerHost}:sub": !Ref OIDCSubjectClaim
+                ? !Sub
+                  - "${Host}:aud"
+                  - Host: !Select [1, !Split ["https://", !Ref OIDCIssuerURL]]
+                : !If [HasAudience, !Ref OIDCAudience, sts.amazonaws.com]
+                ? !Sub
+                  - "${Host}:sub"
+                  - Host: !Select [1, !Split ["https://", !Ref OIDCIssuerURL]]
+                : !Ref OIDCSubjectClaim
       ManagedPolicyArns:
         - !Ref CUDlyPolicy