From fa307af358515714d254bdb6d70d0403b72dabf9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 30 Sep 2026 04:24:01 +0200 Subject: [PATCH 1/3] fix(iac): enforce Azure federation identity guards Validate issuer and claim inputs before Azure CLI calls and require federated credential creation to succeed before deployment or registration. Serialize literal claim values with jq and enforce matching Terraform input validation. Verify both rendered scripts with recording CLI stubs and run mock-only Terraform guard plans in the existing Azure CI matrix. Refs #91 --- .github/workflows/ci.yml | 7 + .../tests/identity_guards.tftest.hcl | 233 ++++++++++++++++++ .../azure-target/terraform/variables.tf | 19 ++ .../iacfiles/templates/azure-wif-cli.sh.tmpl | 44 ++-- .../templates/azure-wif-deploy.sh.tmpl | 43 ++-- internal/iacfiles/templates_azure_wif_test.go | 144 +++++++++++ internal/iacfiles/templates_test.go | 8 +- 7 files changed, 459 insertions(+), 39 deletions(-) create mode 100644 iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl create mode 100644 internal/iacfiles/templates_azure_wif_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 310c6e74..edd88c6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -557,6 +557,13 @@ jobs: - name: Terraform Format Check run: terraform fmt -check -recursive terraform/ + - name: Test Azure federation identity guards + if: matrix.cloud == 'azure' + run: | + terraform -chdir=iac/federation/azure-target/terraform init -backend=false -lockfile=readonly -input=false + terraform -chdir=iac/federation/azure-target/terraform fmt -check -recursive + terraform -chdir=iac/federation/azure-target/terraform test + - name: Terraform Init run: | cd terraform/environments/${{ matrix.cloud }} diff --git a/iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl b/iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl new file mode 100644 index 00000000..170a5387 --- /dev/null +++ b/iac/federation/azure-target/terraform/tests/identity_guards.tftest.hcl @@ -0,0 +1,233 @@ +mock_provider "azuread" {} +mock_provider "http" {} +mock_provider "azurerm" { + mock_data "azurerm_subscription" { + defaults = { + id = "/subscriptions/11111111-1111-1111-1111-111111111111" + subscription_id = "11111111-1111-1111-1111-111111111111" + tenant_id = "22222222-2222-2222-2222-222222222222" + } + } +} + +variables { + subscription_id = "11111111-1111-1111-1111-111111111111" + tenant_id = "22222222-2222-2222-2222-222222222222" + cudly_issuer_url = "https://cudly.example.com/oidc" + cudly_api_url = "" +} + +run "defaults" { + command = plan + assert { + condition = ( + azuread_application_federated_identity_credential.cudly.issuer == var.cudly_issuer_url && + azuread_application_federated_identity_credential.cudly.subject == "cudly-controller" && + azuread_application_federated_identity_credential.cudly.audiences == tolist(["api://AzureADTokenExchange"]) + ) + error_message = "Default identity must be preserved exactly." + } +} + +run "literal_claims" { + command = plan + variables { + cudly_issuer_url = "https://CUDly.example.com:8443/a_b/~v1%20/oidc" + cudly_federated_subject = "quote\"backslash\\backtick`apostrophe':é" + cudly_federated_audience = "quote\"backslash\\backtick`apostrophe':é" + } + assert { + condition = ( + azuread_application_federated_identity_credential.cudly.issuer == var.cudly_issuer_url && + azuread_application_federated_identity_credential.cudly.subject == var.cudly_federated_subject && + azuread_application_federated_identity_credential.cudly.audiences == tolist([var.cudly_federated_audience]) + ) + error_message = "Literal identity bytes and singleton audience must be preserved." + } +} + +run "invalid_claim_0" { + command = plan + variables { + cudly_federated_subject = null + cudly_federated_audience = null + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_1" { + command = plan + variables { + cudly_federated_subject = "" + cudly_federated_audience = "" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_2" { + command = plan + variables { + cudly_federated_subject = "a b" + cudly_federated_audience = "a b" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_3" { + command = plan + variables { + cudly_federated_subject = "a\tb" + cudly_federated_audience = "a\tb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_4" { + command = plan + variables { + cudly_federated_subject = "a\nb" + cudly_federated_audience = "a\nb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_5" { + command = plan + variables { + cudly_federated_subject = "a\rb" + cudly_federated_audience = "a\rb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_6" { + command = plan + variables { + cudly_federated_subject = "a\u000bb" + cudly_federated_audience = "a\u000bb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_7" { + command = plan + variables { + cudly_federated_subject = "a\u000cb" + cudly_federated_audience = "a\u000cb" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_8" { + command = plan + variables { + cudly_federated_subject = "a$b" + cudly_federated_audience = "a$b" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_claim_9" { + command = plan + variables { + cudly_federated_subject = "a*b" + cudly_federated_audience = "a*b" + } + expect_failures = [var.cudly_federated_subject, var.cudly_federated_audience] +} + +run "invalid_issuer_0" { + command = plan + variables { + cudly_issuer_url = null + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_1" { + command = plan + variables { + cudly_issuer_url = "" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_2" { + command = plan + variables { + cudly_issuer_url = "http://example.com/oidc" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_3" { + command = plan + variables { + cudly_issuer_url = "https://" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_4" { + command = plan + variables { + cudly_issuer_url = "https://example.com/oidc/" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_5" { + command = plan + variables { + cudly_issuer_url = "https://user@example.com/oidc" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_6" { + command = plan + variables { + cudly_issuer_url = "https://example.com/?q=1" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_7" { + command = plan + variables { + cudly_issuer_url = "https://example.com/#f" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_8" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a b" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_9" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a\nb" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_10" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a\"b" + } + expect_failures = [var.cudly_issuer_url] +} + +run "invalid_issuer_11" { + command = plan + variables { + cudly_issuer_url = "https://example.com/a\\b" + } + expect_failures = [var.cudly_issuer_url] +} diff --git a/iac/federation/azure-target/terraform/variables.tf b/iac/federation/azure-target/terraform/variables.tf index aced3d38..50e25f0a 100644 --- a/iac/federation/azure-target/terraform/variables.tf +++ b/iac/federation/azure-target/terraform/variables.tf @@ -19,18 +19,37 @@ variable "app_display_name" { variable "cudly_issuer_url" { description = "CUDly OIDC issuer URL (e.g. https://cudly.example.com/oidc). Azure AD fetches JWKS from this issuer to verify client assertion JWTs." type = string + + # CUDly contract, mirrored by the two azure-wif shell templates. + validation { + condition = var.cudly_issuer_url == null ? false : ( + can(regex("^https://[A-Za-z0-9.-]+(:[0-9]+)?(/[A-Za-z0-9._~%/-]*)?$", var.cudly_issuer_url)) && + !endswith(var.cudly_issuer_url, "/") + ) + error_message = "Use a CUDly HTTPS issuer URL without a trailing slash, query, fragment or userinfo." + } } variable "cudly_federated_subject" { description = "Subject claim in the client assertion JWT. Must match what CUDly signs." type = string default = "cudly-controller" + + validation { + condition = var.cudly_federated_subject == null ? false : can(regex("^[^ \\t\\n\\r\\x0b\\f$*]+$", var.cudly_federated_subject)) + error_message = "Subject must be nonempty without ASCII whitespace, dollar signs or asterisks." + } } variable "cudly_federated_audience" { description = "Audience for the federated identity credential." type = string default = "api://AzureADTokenExchange" + + validation { + condition = var.cudly_federated_audience == null ? false : can(regex("^[^ \\t\\n\\r\\x0b\\f$*]+$", var.cudly_federated_audience)) + error_message = "Audience must be nonempty without ASCII whitespace, dollar signs or asterisks." + } } variable "cudly_api_url" { diff --git a/internal/iacfiles/templates/azure-wif-cli.sh.tmpl b/internal/iacfiles/templates/azure-wif-cli.sh.tmpl index fafaf495..6bfc7a97 100644 --- a/internal/iacfiles/templates/azure-wif-cli.sh.tmpl +++ b/internal/iacfiles/templates/azure-wif-cli.sh.tmpl @@ -5,7 +5,7 @@ # Usage: # bash {{.AccountSlug}}-azure-wif-cli.sh # -# Requirements: az cli logged in to the target subscription. +# Requirements: jq and az cli logged in to the target subscription. # # This script configures true Azure Workload Identity Federation — # no certificate, no private key, no client secret is created or stored. @@ -22,11 +22,24 @@ SUBSCRIPTION_ID="${SUBSCRIPTION_ID:-{{.SubscriptionID}}}" # /.well-known/openid-configuration when resolving the discovery doc, # so do NOT include the well-known suffix here. Trailing slashes and # http/https matter for Azure AD validation (AADSTS700213 if mismatched). -CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-{{.CUDlyAPIURL}}/oidc}" -CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT:-cudly-controller}" -CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE:-api://AzureADTokenExchange}" +CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL-{{.CUDlyAPIURL}}/oidc}" +CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT-cudly-controller}" +CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE-api://AzureADTokenExchange}" -: "${CUDLY_ISSUER_URL:?set CUDLY_ISSUER_URL to the OIDC issuer URL of your CUDly deployment (base URL + /oidc)}" +# Keep this CUDly input contract aligned with azure-target/terraform/variables.tf. +ISSUER_PATTERN='^https://[A-Za-z0-9.-]+(:[0-9]+)?(/[A-Za-z0-9._~%/-]*)?$' +if [[ ! "${CUDLY_ISSUER_URL}" =~ $ISSUER_PATTERN || "${CUDLY_ISSUER_URL}" == */ ]]; then + echo "ERROR: CUDLY_ISSUER_URL must be a CUDly HTTPS issuer URL without a trailing slash." >&2 + exit 1 +fi +for CLAIM_NAME in CUDLY_FEDERATED_SUBJECT CUDLY_FEDERATED_AUDIENCE; do + case "${!CLAIM_NAME}" in + ''|*[$' \t\n\r\v\f']*|*'$'*|*'*'*) + echo "ERROR: ${CLAIM_NAME} must be nonempty without ASCII whitespace, dollar signs or asterisks." >&2 + exit 1 ;; + esac +done +command -v jq >/dev/null 2>&1 || { echo "ERROR: jq is required." >&2; exit 1; } if [[ -n "${SUBSCRIPTION_ID}" && "${SUBSCRIPTION_ID}" != "" ]]; then az account set --subscription "${SUBSCRIPTION_ID}" @@ -41,23 +54,16 @@ echo "Creating service principal..." SP_OBJECT_ID=$(az ad sp create --id "${APP_ID}" --query id --output tsv) echo "Adding federated identity credential bound to CUDly OIDC issuer..." -# Idempotent: if the same name exists (re-run of the script), az throws a -# conflict error — catch and continue. A subsequent run with the same -# script is safe. -FEDCRED_PARAMS=$(cat <&1 || echo " (federated credential may already exist — continuing)" + --output none echo "" echo "=== Done ===" diff --git a/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl b/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl index 5f6bfae8..3966d606 100644 --- a/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl +++ b/internal/iacfiles/templates/azure-wif-deploy.sh.tmpl @@ -10,7 +10,7 @@ # # Set CUDLY_CONTACT_EMAIL to auto-register with CUDly after deployment. # -# Requirements: az cli v2 logged in to the target subscription. +# Requirements: jq and az cli v2 logged in to the target subscription. set -euo pipefail APP_NAME="${APP_NAME:-CUDly-target}" @@ -25,6 +25,25 @@ while [[ $# -gt 0 ]]; do esac done +CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL-{{.CUDlyAPIURL}}/oidc}" +CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT-cudly-controller}" +CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE-api://AzureADTokenExchange}" + +# Keep this CUDly input contract aligned with azure-target/terraform/variables.tf. +ISSUER_PATTERN='^https://[A-Za-z0-9.-]+(:[0-9]+)?(/[A-Za-z0-9._~%/-]*)?$' +if [[ ! "${CUDLY_ISSUER_URL}" =~ $ISSUER_PATTERN || "${CUDLY_ISSUER_URL}" == */ ]]; then + echo "ERROR: CUDLY_ISSUER_URL must be a CUDly HTTPS issuer URL without a trailing slash." >&2 + exit 1 +fi +for CLAIM_NAME in CUDLY_FEDERATED_SUBJECT CUDLY_FEDERATED_AUDIENCE; do + case "${!CLAIM_NAME}" in + ''|*[$' \t\n\r\v\f']*|*'$'*|*'*'*) + echo "ERROR: ${CLAIM_NAME} must be nonempty without ASCII whitespace, dollar signs or asterisks." >&2 + exit 1 ;; + esac +done +command -v jq >/dev/null 2>&1 || { echo "ERROR: jq is required." >&2; exit 1; } + DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" case "$TEMPLATE" in @@ -57,26 +76,18 @@ echo "SP Object ID: ${SP_OBJECT_ID}" # --------------------------------------------------------------------------- # Step 3: Add federated identity credential bound to CUDly OIDC issuer # --------------------------------------------------------------------------- -CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-{{.CUDlyAPIURL}}/oidc}" -CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT:-cudly-controller}" -CUDLY_FEDERATED_AUDIENCE="${CUDLY_FEDERATED_AUDIENCE:-api://AzureADTokenExchange}" - echo "" echo "Adding federated identity credential..." -FEDCRED_PARAMS=$(cat <&1 || echo " (federated credential may already exist — continuing)" + --output none # --------------------------------------------------------------------------- # Step 4: Deploy role assignment via Bicep/ARM diff --git a/internal/iacfiles/templates_azure_wif_test.go b/internal/iacfiles/templates_azure_wif_test.go new file mode 100644 index 00000000..0fcee1f8 --- /dev/null +++ b/internal/iacfiles/templates_azure_wif_test.go @@ -0,0 +1,144 @@ +package iacfiles + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/iac" + "github.com/stretchr/testify/require" +) + +func TestAzureWIFTerraformPlansOnly(t *testing.T) { + data, err := iac.Modules.ReadFile("federation/azure-target/terraform/tests/identity_guards.tftest.hcl") + require.NoError(t, err) + runs := regexp.MustCompile(`(?m)^run "[^"]+" \{`).FindAll(data, -1) + plans := regexp.MustCompile(`(?m)^run "[^"]+" \{\s*command\s*=\s*plan\s`).FindAll(data, -1) + require.NotEmpty(t, runs) + require.Len(t, plans, len(runs), "every mock run must explicitly start with command = plan") + commands := regexp.MustCompile(`(?m)^\s*command\s*=\s*(\w+)`).FindAllSubmatch(data, -1) + for _, command := range commands { + require.Equal(t, "plan", string(command[1]), "apply is forbidden in this mock suite") + } +} + +func runAzureWIF(t *testing.T, template string, data testTemplateData, env map[string]string) (int, string, string, []string) { + t.Helper() + return runRenderedScript(t, "azure-wif.sh", renderCLITemplate(t, template, data), func(logPath string) map[string]string { + return map[string]string{ + "az": fmt.Sprintf(`#!/usr/bin/env bash +set -euo pipefail +printf 'az %%s\n' "$*" >> %q +case "$1 $2" in + 'account set') ;; + 'account show') echo 11111111-1111-1111-1111-111111111111 ;; + 'ad app') + if [[ "$3" == 'federated-credential' ]]; then + [[ "${FAIL_CREDENTIAL:-}" == '' ]] || { echo "$FAIL_CREDENTIAL" >&2; exit 42; } + while [[ $# -gt 0 ]]; do + if [[ "$1" == '--parameters' ]]; then + jq -c . <<< "$2" >> %q + exit + fi + shift + done + exit 43 + fi + echo 22222222-2222-2222-2222-222222222222 ;; + 'ad sp') echo 33333333-3333-3333-3333-333333333333 ;; + 'deployment sub') ;; + *) exit 44 ;; +esac +`, logPath, logPath), + "curl": fmt.Sprintf("#!/usr/bin/env bash\necho curl >> %q\necho 409\n", logPath), + } + }, env) +} + +func TestAzureWIFInputGuards(t *testing.T) { + for _, template := range []string{"templates/azure-wif-cli.sh.tmpl", "templates/azure-wif-deploy.sh.tmpl"} { + t.Run(template, func(t *testing.T) { + for _, key := range []string{"CUDLY_FEDERATED_SUBJECT", "CUDLY_FEDERATED_AUDIENCE"} { + for _, value := range []string{"", "a b", "a\tb", "a\nb", "a\rb", "a\vb", "a\fb", "a$b", "a*b"} { + t.Run(key+fmt.Sprintf("%q", value), func(t *testing.T) { + code, _, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{key: value}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, key) + }) + } + } + for _, value := range []string{"", "http://example.com/oidc", "https://", "https://example.com/oidc/", "https://user@example.com/oidc", "https://example.com/?q=1", "https://example.com/#f", "https://example.com/a b", "https://example.com/a\nb", `https://example.com/a"b`, `https://example.com/a\b`} { + t.Run(fmt.Sprintf("issuer%q", value), func(t *testing.T) { + code, _, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{"CUDLY_ISSUER_URL": value}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, "CUDLY_ISSUER_URL") + }) + } + t.Run("missing rendered issuer", func(t *testing.T) { + data := baseData() + data.CUDlyAPIURL = "" + code, _, _, calls := runAzureWIF(t, template, data, nil) + require.NotZero(t, code) + require.Empty(t, calls) + }) + t.Run("missing jq", func(t *testing.T) { + code, _, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{"PATH": t.TempDir()}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, "jq is required") + }) + }) + } +} + +func TestAzureWIFFederatedCredential(t *testing.T) { + for _, template := range []string{"templates/azure-wif-cli.sh.tmpl", "templates/azure-wif-deploy.sh.tmpl"} { + t.Run(template, func(t *testing.T) { + for _, failure := range []string{"Authorization_RequestDenied", "TooManyRequests", "Conflict"} { + t.Run(failure, func(t *testing.T) { + code, stdout, stderr, calls := runAzureWIF(t, template, baseData(), map[string]string{"FAIL_CREDENTIAL": failure}) + require.Equal(t, 42, code) + require.Contains(t, stdout+stderr, failure) + require.NotContains(t, stdout, "=== Done ===") + require.NotContains(t, strings.Join(calls, "\n"), "az deployment") + require.NotContains(t, calls, "curl") + }) + } + for _, literal := range []string{"", "quote\"backslash\\backtick`apostrophe':é"} { + t.Run("literal/"+literal, func(t *testing.T) { + issuer, subject, audience := baseData().CUDlyAPIURL+"/oidc", "cudly-controller", "api://AzureADTokenExchange" + env := map[string]string{} + if literal != "" { + issuer, subject, audience = "https://CUDly.example.com:8443/a_b/~v1%20/oidc", literal, literal + env = map[string]string{"CUDLY_ISSUER_URL": issuer, "CUDLY_FEDERATED_SUBJECT": subject, "CUDLY_FEDERATED_AUDIENCE": audience} + } + code, stdout, stderr, calls := runAzureWIF(t, template, baseData(), env) + require.Zero(t, code, "%s", stderr) + require.Contains(t, stdout, "=== Done ===") + require.Contains(t, calls, "curl") + if strings.Contains(template, "deploy") { + require.Contains(t, strings.Join(calls, "\n"), "az deployment sub create") + } + var credentials []map[string]any + for _, call := range calls { + if strings.HasPrefix(call, "{") { + var credential map[string]any + require.NoError(t, json.Unmarshal([]byte(call), &credential)) + credentials = append(credentials, credential) + } + } + require.Len(t, credentials, 1) + require.Len(t, credentials[0], 5) + require.Equal(t, "cudly", credentials[0]["name"]) + require.Equal(t, issuer, credentials[0]["issuer"]) + require.Equal(t, subject, credentials[0]["subject"]) + require.Equal(t, []any{audience}, credentials[0]["audiences"]) + }) + } + }) + } +} diff --git a/internal/iacfiles/templates_test.go b/internal/iacfiles/templates_test.go index 28d2e280..9e7530bc 100644 --- a/internal/iacfiles/templates_test.go +++ b/internal/iacfiles/templates_test.go @@ -126,13 +126,13 @@ func TestCLITemplatesAutoRegister(t *testing.T) { `ACCOUNT_NAME="${CUDLY_ACCOUNT_NAME:-Azure ${SUBSCRIPTION_ID}}"`, // Secret-free redesign: must use federated identity credential, not a cert upload. "az ad app federated-credential create", - `"issuer": "${CUDLY_ISSUER_URL}"`, - `"subject": "${CUDLY_FEDERATED_SUBJECT}"`, - `"audiences": ["${CUDLY_FEDERATED_AUDIENCE}"]`, + `issuer: $issuer`, + `subject: $subject`, + `audiences: [$audience]`, // Issuer env var must default to the CUDly base URL + /oidc // so Azure AD appending /.well-known/openid-configuration // resolves to the discovery endpoint on the CUDly deployment. - `CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-https://cudly.example.com/oidc}"`, + `CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL-https://cudly.example.com/oidc}"`, }, mustNot: []string{ "/api/registrations", From bea8efed9f33685f7728715d7f5f2dc6a517ac37 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 1 Oct 2026 01:15:40 +0200 Subject: [PATCH 2/3] fix(iac): lock Azure test providers for Linux Record authenticated Linux package hashes alongside existing Darwin hashes so readonly initialization can be followed by mock-plan tests on CI. Keep provider versions and archive checksums unchanged. Verified fresh Linux readonly init and all 24 mock plans, plus Darwin checks. Removing only the new Linux hashes reproduces the CI failure. Refs #91 --- iac/federation/azure-target/terraform/.terraform.lock.hcl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/iac/federation/azure-target/terraform/.terraform.lock.hcl b/iac/federation/azure-target/terraform/.terraform.lock.hcl index fe2e1f97..68858923 100644 --- a/iac/federation/azure-target/terraform/.terraform.lock.hcl +++ b/iac/federation/azure-target/terraform/.terraform.lock.hcl @@ -3,9 +3,10 @@ provider "registry.terraform.io/hashicorp/azuread" { version = "3.8.0" - constraints = ">= 2.47.0" + constraints = "~> 3.8" hashes = [ "h1:E2YWNE3Qry4bQMlmmZ33X4hLY5hOGrEZrlRg4anI2uw=", + "h1:kIEmiknJFWKe44U9ePG5vXwoloKep0Dbi/mQ8uqCbw0=", "zh:0d26cfbf9417acd1c2295ccd5b0052abeac85ad1c3f6422ff09bf6a1ce16f00d", "zh:144d4ea92fed541a6376bc76ad65ba4738dfd7bcab4c9d6cc20d35001338d06d", "zh:1c3e89cf19118fc07d7b04257251fc9897e722c16e0a0df7b07fcd261f8c12e7", @@ -23,9 +24,10 @@ provider "registry.terraform.io/hashicorp/azuread" { provider "registry.terraform.io/hashicorp/azurerm" { version = "4.67.0" - constraints = ">= 3.95.0" + constraints = ">= 3.0.0, ~> 4.0" hashes = [ "h1:cT/1xU2vHvVboTZvjNjn7sqCH/FPP//JeMh702eGUS0=", + "h1:uBd8sVmk+t3yvxDP3tscD8VloIma0F79CRzNXQowOLY=", "zh:49a1531297510b684103c06d32e9ef1d810380ed55895f6b799913e9679141dc", "zh:4b0a8e5158296c1c02ab64e2374adf9df557fe1eb8191dffd158ed04a648bb3e", "zh:55ac4268c17bdb6f9bb9b6d761269e0e34fb88be14c7759dbcbaa9de9c94b23c", @@ -45,6 +47,7 @@ provider "registry.terraform.io/hashicorp/http" { version = "3.5.0" constraints = ">= 3.4.0" hashes = [ + "h1:8bUoPwS4hahOvzCBj6b04ObLVFXCEmEN8T/5eOHmWOM=", "h1:dl73+8wzQR++HFGoJgDqY3mj3pm14HUuH/CekVyOj5s=", "zh:047c5b4920751b13425efe0d011b3a23a3be97d02d9c0e3c60985521c9c456b7", "zh:157866f700470207561f6d032d344916b82268ecd0cf8174fb11c0674c8d0736", From 070c565cbb53eb5355f4da458161282f1e79e2ef Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 3 Oct 2026 21:56:32 +0200 Subject: [PATCH 3/3] fix(iac): require a valid Azure issuer base in the generator Reject missing or malformed Azure API base URLs before rendering so the standalone command cannot emit an unusable issuer or overwrite an output. Keep custom HTTPS paths and non-Azure optional registration unchanged. Cover the real tfvars and bundle commands, preserve existing outputs on failure, and update both Azure examples with the required URL. --- internal/iacfiles/templates/README.md | 8 +- scripts/generate-federation-iac.go | 36 ++++--- scripts/generate-federation-iac_test.go | 131 ++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 17 deletions(-) diff --git a/internal/iacfiles/templates/README.md b/internal/iacfiles/templates/README.md index 42b09adf..2042e433 100644 --- a/internal/iacfiles/templates/README.md +++ b/internal/iacfiles/templates/README.md @@ -52,6 +52,11 @@ from this directory via the `//go:embed` directive in `internal/iacfiles/embed.g Use `scripts/generate-federation-iac.go`, a self-contained Go script with no external dependencies. +Azure targets require `--cudly-api-url` with the CUDly HTTPS base URL, without a +trailing slash. The generator appends `/oidc` for the issuer and preserves custom +paths. Contact email remains optional; AWS and GCP targets may omit the API URL +to skip automatic registration. + 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 @@ -101,7 +106,8 @@ go run scripts/generate-federation-iac.go \ go run scripts/generate-federation-iac.go \ --target azure --source aws \ --account-name "prod-azure" --account-id "sub-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" \ - --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \ + --cudly-api-url "https://cudly.example.com" # GCP target, AWS source go run scripts/generate-federation-iac.go \ diff --git a/scripts/generate-federation-iac.go b/scripts/generate-federation-iac.go index 10de1b57..a671601c 100644 --- a/scripts/generate-federation-iac.go +++ b/scripts/generate-federation-iac.go @@ -62,7 +62,8 @@ // go run scripts/generate-federation-iac.go \ // --target azure --source aws \ // --account-name "prod-azure" --account-id "sub-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" \ -// --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" +// --tenant-id "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" \ +// --cudly-api-url "https://cudly.example.com" // // # GCP target, AWS source — WIF pool tfvars // # --source-account-id is CUDly's own AWS account (required when --source aws) @@ -131,13 +132,8 @@ type iacData struct { ProjectID string ServiceAccountEmail string OIDCIssuerURI string - // CUDlyAPIURL and ContactEmail feed the optional auto-registration block - // in the tfvars/deploy-script templates. The server pre-fills them from - // the dashboard URL and the authenticated session; the standalone script - // has neither, so they come from --cudly-api-url / --contact-email and - // default to "" (which the Terraform modules treat as "skip - // registration" — see the cudly_api_url/contact_email variable - // descriptions in iac/federation/*/terraform/variables.tf). + // Azure also requires CUDlyAPIURL to derive its issuer; ContactEmail is optional. + // Other targets use both fields only for optional registration. CUDlyAPIURL string ContactEmail string } @@ -504,13 +500,20 @@ func requireSourceAccountID(data *iacData, source, sourceAccountID, errMsg strin return nil } -// populateData fills target-specific fields on data from CLI flags. It reports -// an error rather than writing an invalid value into data, so a rejected flag -// stops the run before any template is rendered. This covers two independent -// gates: --oidc-subject-claim, required (and only meaningful) for an AWS -// target with a non-AWS source; and --source-account-id, required whenever -// the source is AWS itself (the aws-cross-account and gcp-wif-from-aws paths), -// since CUDly's own account has no other way to reach this standalone script. +// Mirrors iac/federation/azure-target/terraform/variables.tf's issuer grammar. +var azureAPIURLRE = regexp.MustCompile(`^https://[A-Za-z0-9.-]+(:\d+)?(/[A-Za-z0-9._~%/-]*)?$`) + +func validateAzureAPIURL(baseURL string) error { + if baseURL == "" { + return errors.New("--cudly-api-url is required when --target=azure to derive the CUDly OIDC issuer") + } + if !azureAPIURLRE.MatchString(baseURL) || strings.HasSuffix(baseURL, "/") { + return errors.New("--cudly-api-url must be a CUDly HTTPS base URL without a trailing slash, query, fragment or userinfo") + } + return nil +} + +// Validation precedes rendering so rejected input cannot overwrite an artifact. func populateData(data *iacData, target, source, tenantID, projectID, saEmail, oidcSubjectClaim, sourceAccountID string) error { // --target is checked first so that a typo there is reported as a bad // --target rather than as an inapplicable --oidc-subject-claim, which would @@ -536,6 +539,7 @@ func populateData(data *iacData, target, source, tenantID, projectID, saEmail, o case "azure": data.SubscriptionID = data.AccountExternalID data.TenantID = tenantID + return validateAzureAPIURL(data.CUDlyAPIURL) case "gcp": data.ProjectID = projectID if data.ProjectID == "" { @@ -573,7 +577,7 @@ func main() { oidcSubjectClaim := flag.String("oidc-subject-claim", "", "Subject (sub) claim restricting the AWS trust policy to one workload: a GCP service account's numeric unique ID or an Azure managed identity's object ID. Required when --target=aws and --source is not aws; there is no working default (see #1640). Letters, digits and . _ : / @ = + - only") sourceAccountID := flag.String("source-account-id", "", "AWS account ID where CUDly itself runs (required for --target aws --source aws, and --target gcp --source aws; NOT the same as --account-id, which is the target account)") contactEmail := flag.String("contact-email", "", "Contact email pre-filled for CUDly auto-registration (optional; empty skips auto-registration)") - cudlyAPIURL := flag.String("cudly-api-url", "", "CUDly API base URL pre-filled for auto-registration (optional; empty skips auto-registration)") + cudlyAPIURL := flag.String("cudly-api-url", "", "CUDly HTTPS base URL without a trailing slash (required for Azure: /oidc is appended for its issuer; otherwise optional for auto-registration)") outFile := flag.String("output", "", "Output file path; use '-' to print to stdout (default: derived filename in current directory)") templDir := flag.String("templates-dir", "internal/iacfiles/templates", "Path to templates directory (run from repo root)") modulesDir := flag.String("modules-dir", "iac/federation", "Path to Terraform modules directory (used by --format bundle)") diff --git a/scripts/generate-federation-iac_test.go b/scripts/generate-federation-iac_test.go index 9e468be7..0b3a29f4 100644 --- a/scripts/generate-federation-iac_test.go +++ b/scripts/generate-federation-iac_test.go @@ -11,9 +11,15 @@ package main // nothing exercised this path so the break went unnoticed. import ( + "archive/zip" + "io" + "os" "os/exec" + "path/filepath" "strings" "testing" + + "github.com/stretchr/testify/require" ) // runViaGoRun invokes `go run generate-federation-iac.go ` from the @@ -263,3 +269,128 @@ func TestGenerateFederationIaC_RejectsMalformedSourceAccountID(t *testing.T) { }) } } + +func TestGenerator_AzureRejectsInvalidAPIURL(t *testing.T) { + cases := []struct { + name string + url string + omit bool + }{ + {name: "omitted", omit: true}, + {name: "empty"}, + {name: "http", url: "http://cudly.example.com"}, + {name: "missing host", url: "https://"}, + {name: "space", url: "https://cudly.example.com/a b"}, + {name: "newline", url: "https://cudly.example.com/a\nb"}, + {name: "quote", url: `https://cudly.example.com/a"b`}, + {name: "backslash", url: `https://cudly.example.com/a\b`}, + {name: "userinfo", url: "https://user@cudly.example.com"}, + {name: "query", url: "https://cudly.example.com/?q=1"}, + {name: "fragment", url: "https://cudly.example.com/#f"}, + {name: "trailing slash", url: "https://cudly.example.com/"}, + {name: "nonnumeric port", url: "https://cudly.example.com:https"}, + } + for _, source := range []string{"aws", "gcp", "azure"} { + for _, format := range []string{"tfvars", "bundle"} { + for _, tc := range cases { + t.Run(source+"/"+format+"/"+tc.name, func(t *testing.T) { + output := filepath.Join(t.TempDir(), "output") + args := []string{ + "--target", "azure", "--source", source, + "--account-name", "Acme", "--account-id", "11111111-2222-3333-4444-555555555555", + "--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--output", output, + } + if format == "bundle" { + args = append(args, "--format", "bundle") + } + if !tc.omit { + args = append(args, "--cudly-api-url", tc.url) + } + res := runGenerator(t, args...) + require.NotZero(t, res.exitCode, "stdout: %s; stderr: %s", res.stdout, res.stderr) + require.Contains(t, res.stderr, "--cudly-api-url") + require.Empty(t, res.stdout) + _, err := os.Stat(output) + require.ErrorIs(t, err, os.ErrNotExist) + }) + } + } + } +} + +func TestGenerator_AzureMissingURLPreservesOutput(t *testing.T) { + for _, format := range []string{"tfvars", "bundle"} { + t.Run(format, func(t *testing.T) { + output := filepath.Join(t.TempDir(), "existing-output") + const sentinel = "existing operator data\n" + require.NoError(t, os.WriteFile(output, []byte(sentinel), 0o600)) + args := []string{"--target", "azure", "--source", "aws", "--account-name", "Acme", "--account-id", "subscription"} + if format == "bundle" { + args = append(args, "--format", "bundle") + } + res := runGenerator(t, append(args, "--output", output)...) + require.NotZero(t, res.exitCode, "stderr: %s", res.stderr) + require.Contains(t, res.stderr, "--cudly-api-url") + got, err := os.ReadFile(output) + require.NoError(t, err) + require.Equal(t, sentinel, string(got)) + if format == "tfvars" { + res = runGenerator(t, append(args, "--output", "-")...) + require.NotZero(t, res.exitCode) + require.Empty(t, res.stdout) + require.Contains(t, res.stderr, "--cudly-api-url") + } + }) + } +} + +func TestGenerator_AzurePreservesValidAPIURL(t *testing.T) { + for _, baseURL := range []string{"https://cudly.example.com", "https://CUDly.example.com:8443/a_b/~v1%20"} { + for _, source := range []string{"aws", "gcp", "azure"} { + for _, format := range []string{"tfvars", "bundle"} { + t.Run(baseURL+"/"+source+"/"+format, func(t *testing.T) { + args := []string{ + "--target", "azure", "--source", source, + "--account-name", "Acme", "--account-id", "11111111-2222-3333-4444-555555555555", + "--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--cudly-api-url", baseURL, + } + output := "-" + if format == "bundle" { + output = filepath.Join(t.TempDir(), "bundle.zip") + args = append(args, "--format", "bundle") + } + res := runGenerator(t, append(args, "--output", output)...) + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + content := res.stdout + if format == "bundle" { + archive, err := zip.OpenReader(output) + require.NoError(t, err) + defer archive.Close() + entry, err := archive.Open("terraform/acme-azure-wif.tfvars") + require.NoError(t, err) + data, err := io.ReadAll(entry) + require.NoError(t, entry.Close()) + require.NoError(t, err) + content = string(data) + } + require.Contains(t, content, `cudly_issuer_url = "`+baseURL+`/oidc"`) + require.Contains(t, content, `cudly_api_url = "`+baseURL+`"`) + require.Contains(t, content, `contact_email = ""`) + }) + } + } + } +} + +func TestGenerator_NonAzureDoesNotRequireAPIURL(t *testing.T) { + for _, target := range []string{"aws", "gcp"} { + t.Run(target, func(t *testing.T) { + res := runGenerator(t, + "--target", target, "--source", "aws", "--account-name", "Acme", "--account-id", "999888777666", + "--source-account-id", "111122223333", "--output", "-", + ) + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + require.Contains(t, res.stdout, `cudly_api_url = ""`) + }) + } +}