fix(iac): enforce AWS CLI federation guard parity

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

@@ -6,18 +6,13 @@
 # Usage:
 #   bash {{.AccountSlug}}-aws-wif-cli.sh
 #
-# Requirements: aws-cli v2, credentials for target account {{.AccountExternalID}}.
+# Requirements: aws-cli v2, jq, credentials for target account {{.AccountExternalID}}.
 set -euo pipefail
 
 ROLE_NAME="${ROLE_NAME:-CUDly-{{.AccountSlug}}}"
 OIDC_ISSUER_URL="${OIDC_ISSUER_URL:-{{.OIDCIssuerURL}}}"
-OIDC_AUDIENCE="${OIDC_AUDIENCE:-{{.OIDCAudience}}}"
-# 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_AUDIENCE="${OIDC_AUDIENCE-{{.OIDCAudience}}}"
+OIDC_AUDIENCE="${OIDC_AUDIENCE:-sts.amazonaws.com}"
 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
@@ -29,7 +24,7 @@ if [[ -z "${OIDC_SUBJECT_CLAIM}" ]]; then
   exit 1
 fi
 case "${OIDC_SUBJECT_CLAIM}" in
-  *[[:space:]]*|*'$'*|*'*'*)
+  *[$' \t\n\r\v\f']*|*'$'*|*'*'*)
     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
@@ -39,45 +34,68 @@ case "${OIDC_SUBJECT_CLAIM}" in
     exit 1
     ;;
 esac
+case "${OIDC_AUDIENCE}" in
+  *[$' \t\n\r\v\f']*|*'$'*|*'*'*)
+    echo "Error: OIDC_AUDIENCE must not contain whitespace, '\$' or '*'." >&2
+    exit 1 ;;
+esac
+# Keep this issuer check equivalent to aws-target/terraform/variables.tf.
+if ! [[ "$OIDC_ISSUER_URL" =~ ^https://[^/]+(/[^/].*[^/]|/[^/]+)?$ ]] || [[ "$OIDC_ISSUER_URL" == */ ]]; then
+  echo "Error: OIDC_ISSUER_URL must start with https:// and must not end with a trailing slash." >&2
+  exit 1
+fi
+command -v jq >/dev/null || { echo "Error: jq is required." >&2; exit 1; }
 PROFILE_ARG=""
 if [[ -n "${AWS_PROFILE:-}" ]]; then PROFILE_ARG="--profile ${AWS_PROFILE}"; fi
 
-OIDC_HOST=$(echo "${OIDC_ISSUER_URL}" | sed -E 's#^https://##; s#/$##')
+OIDC_HOST="${OIDC_ISSUER_URL#https://}"
 
 echo "Creating/ensuring OIDC provider for ${OIDC_ISSUER_URL}..."
 # shellcheck disable=SC2086
-EXISTING_PROVIDER=$(aws iam list-open-id-connect-providers $PROFILE_ARG \
-  --query "OpenIDConnectProviderList[?contains(Arn, '${OIDC_HOST}')].Arn | [0]" \
-  --output text 2>/dev/null || echo "None")
-if [[ "${EXISTING_PROVIDER}" == "None" || -z "${EXISTING_PROVIDER}" ]]; then
+PROVIDERS=$(aws iam list-open-id-connect-providers $PROFILE_ARG --output json)
+PROVIDER_ARN=$(jq -er --arg host "$OIDC_HOST" '
+  if (.OpenIDConnectProviderList | type) != "array" then error("Invalid provider list")
+  else [.OpenIDConnectProviderList[].Arn | select(split(":oidc-provider/")[1] == $host)]
+    | if length > 1 then error("Multiple providers match the issuer") else .[0] // "" end
+  end' <<< "$PROVIDERS")
+if [[ -z "$PROVIDER_ARN" ]]; then
   # shellcheck disable=SC2086
-  aws iam create-open-id-connect-provider $PROFILE_ARG \
+  CREATED_PROVIDER=$(aws iam create-open-id-connect-provider $PROFILE_ARG \
     --url "${OIDC_ISSUER_URL}" \
     --client-id-list "${OIDC_AUDIENCE}" \
-    --thumbprint-list "0000000000000000000000000000000000000000" >/dev/null
+    --output json)
+  PROVIDER_ARN=$(jq -er '.OpenIDConnectProviderArn | select(type == "string" and length > 0)' <<< "$CREATED_PROVIDER")
+fi
+if ! [[ "$PROVIDER_ARN" =~ ^arn:[a-z0-9-]+:iam::[0-9]{12}:oidc-provider/ ]] || [[ "${PROVIDER_ARN#*:oidc-provider/}" != "$OIDC_HOST" ]]; then
+  echo "Error: returned provider ARN does not match OIDC_ISSUER_URL." >&2
+  exit 1
 fi
 # shellcheck disable=SC2086
-PROVIDER_ARN=$(aws iam list-open-id-connect-providers $PROFILE_ARG \
-  --query "OpenIDConnectProviderList[?contains(Arn, '${OIDC_HOST}')].Arn | [0]" \
-  --output text)
+PROVIDER=$(aws iam get-open-id-connect-provider $PROFILE_ARG --open-id-connect-provider-arn "$PROVIDER_ARN" --output json)
+if ! jq -e --arg host "$OIDC_HOST" --arg audience "$OIDC_AUDIENCE" '
+  .Url == $host and
+  (.ClientIDList | type == "array" and all(.[]; type == "string") and index($audience) != null) and
+  (.ThumbprintList | type == "array" and all(.[]; type == "string" and
+    test("^[0-9a-fA-F]{40}$") and . != "0000000000000000000000000000000000000000"))
+  ' <<< "$PROVIDER" >/dev/null; then
+  echo "Error: provider URL, audience or thumbprints do not match. Inspect $PROVIDER_ARN before rerunning; shared providers are not modified." >&2
+  exit 1
+fi
 
-# 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
+TRUST_POLICY=$(jq -n --arg host "$OIDC_HOST" --arg audience "$OIDC_AUDIENCE" \
+  --arg subject "$OIDC_SUBJECT_CLAIM" --arg provider "$PROVIDER_ARN" '
 {
   "Version": "2012-10-17",
   "Statement": [{
     "Effect": "Allow",
-    "Principal": {"Federated": "${PROVIDER_ARN}"},
+    "Principal": {"Federated": $provider},
     "Action": "sts:AssumeRoleWithWebIdentity",
     "Condition": {"StringEquals": {
-      "${OIDC_HOST}:aud": "${OIDC_AUDIENCE}",
-      "${OIDC_HOST}:sub": "${OIDC_SUBJECT_CLAIM}"
+      ($host + ":aud"): $audience,
+      ($host + ":sub"): $subject
     }}
   }]
-}
-JSON
-)
+}')
 
 PERMISSIONS_POLICY=$(cat <<'JSON'
 {

internal/iacfiles/templates_aws_wif_test.go

@@ -0,0 +1,210 @@
+package iacfiles
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+const awsWIFProvider = "arn:aws:iam::123456789012:oidc-provider/accounts.google.com"
+
+func awsWIFStub(logPath string) string {
+	return fmt.Sprintf(`#!/usr/bin/env bash
+set -euo pipefail
+jq -cn --args '$ARGS.positional' -- "$@" >> %q
+[[ "$1 $2" != "${FAIL_OPERATION:-}" ]] || exit 42
+case "$1 $2" in
+  'iam list-open-id-connect-providers')
+    if [[ "$*" == *'--output text'* ]]; then
+      jq -r --arg host "$TEST_HOST" '[.OpenIDConnectProviderList[].Arn | select(contains($host))][0] // "None"' <<< "$LIST_RESPONSE"
+    else
+      printf '%%s\n' "$LIST_RESPONSE"
+    fi ;;
+  'iam create-open-id-connect-provider') printf '%%s\n' "$CREATE_RESPONSE" ;;
+  'iam get-open-id-connect-provider') printf '%%s\n' "$PROVIDER_RESPONSE" ;;
+  'iam get-role') echo 'arn:aws:iam::123456789012:role/CUDly' ;;
+  'sts get-caller-identity') echo 123456789012 ;;
+  'iam create-role'|'iam put-role-policy') echo '{}' ;;
+  *) exit 43 ;;
+esac
+`, logPath)
+}
+
+func runAWSWIF(t *testing.T, data testTemplateData, overrides map[string]string) (int, string, string, [][]string) {
+	t.Helper()
+	host := strings.TrimPrefix(data.OIDCIssuerURL, "https://")
+	arn := "arn:aws:iam::123456789012:oidc-provider/" + host
+	provider, err := json.Marshal(map[string]any{
+		"Url": host, "ClientIDList": []string{data.OIDCAudience, "sts.amazonaws.com"},
+		"ThumbprintList": []string{strings.Repeat("a", 40)},
+	})
+	require.NoError(t, err)
+	env := map[string]string{
+		"TEST_HOST": host, "LIST_RESPONSE": `{"OpenIDConnectProviderList":[]}`,
+		"CREATE_RESPONSE":   fmt.Sprintf(`{"OpenIDConnectProviderArn":%q}`, arn),
+		"PROVIDER_RESPONSE": string(provider),
+	}
+	for key, value := range overrides {
+		env[key] = value
+	}
+	code, stdout, stderr, lines := runRenderedScript(t, "aws-wif-cli.sh",
+		renderCLITemplate(t, "templates/aws-wif-cli.sh.tmpl", data),
+		func(logPath string) map[string]string {
+			return map[string]string{"aws": awsWIFStub(logPath), "curl": fmt.Sprintf("#!/usr/bin/env bash\njq -cn --args '[\"curl\"] + $ARGS.positional' -- \"$@\" >> %q\necho 409\n", logPath)}
+		}, env)
+	calls := make([][]string, 0, len(lines))
+	for _, line := range lines {
+		var args []string
+		require.NoError(t, json.Unmarshal([]byte(line), &args))
+		calls = append(calls, args)
+	}
+	return code, stdout, stderr, calls
+}
+
+func awsWIFData() testTemplateData {
+	data := baseData()
+	data.OIDCIssuerURL = "https://accounts.google.com"
+	data.OIDCAudience = "sts.amazonaws.com"
+	data.OIDCSubjectClaim = "123456789012345678901"
+	return data
+}
+
+func awsWIFOperation(calls [][]string, operation string) []string {
+	for _, call := range calls {
+		if len(call) > 1 && call[1] == operation {
+			return call
+		}
+	}
+	return nil
+}
+
+func awsWIFArgument(t *testing.T, call []string, flag string) string {
+	t.Helper()
+	for i, arg := range call {
+		if arg == flag && i+1 < len(call) {
+			return call[i+1]
+		}
+	}
+	t.Fatalf("missing %s in %v", flag, call)
+	return ""
+}
+
+func TestAWSWIFCLIProviderGuards(t *testing.T) {
+	listed := `{"OpenIDConnectProviderList":[{"Arn":"` + awsWIFProvider + `.evil.example"},{"Arn":"` + awsWIFProvider + `/other"},{"Arn":"` + awsWIFProvider + `"}]}`
+	cases := []struct {
+		name           string
+		env            map[string]string
+		create, reject bool
+	}{
+		{"absent", nil, true, false},
+		{"exact with lookalikes", map[string]string{"LIST_RESPONSE": listed}, false, false},
+		{"only lookalike", map[string]string{"LIST_RESPONSE": `{"OpenIDConnectProviderList":[{"Arn":"` + awsWIFProvider + `.evil.example"}]}`}, true, false},
+		{"list denied", map[string]string{"FAIL_OPERATION": "iam list-open-id-connect-providers"}, false, true},
+		{"create denied", map[string]string{"FAIL_OPERATION": "iam create-open-id-connect-provider"}, true, true},
+		{"get denied", map[string]string{"FAIL_OPERATION": "iam get-open-id-connect-provider"}, true, true},
+		{"missing audience", map[string]string{"LIST_RESPONSE": listed, "PROVIDER_RESPONSE": `{"Url":"accounts.google.com","ClientIDList":["other"],"ThumbprintList":[]}`}, false, true},
+		{"wrong URL", map[string]string{"PROVIDER_RESPONSE": `{"Url":"accounts.google.com.evil","ClientIDList":["sts.amazonaws.com"],"ThumbprintList":[]}`}, true, true},
+		{"zero thumbprint", map[string]string{"PROVIDER_RESPONSE": `{"Url":"accounts.google.com","ClientIDList":["sts.amazonaws.com"],"ThumbprintList":["0000000000000000000000000000000000000000"]}`}, true, true},
+		{"malformed list", map[string]string{"LIST_RESPONSE": `not-json`}, false, true},
+		{"malformed provider", map[string]string{"PROVIDER_RESPONSE": `{}`}, true, true},
+		{"wrong created ARN", map[string]string{"CREATE_RESPONSE": `{"OpenIDConnectProviderArn":"` + awsWIFProvider + `.evil"}`}, true, true},
+		{"duplicate exact", map[string]string{"LIST_RESPONSE": `{"OpenIDConnectProviderList":[{"Arn":"` + awsWIFProvider + `"},{"Arn":"` + awsWIFProvider + `"}]}`}, false, true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			code, stdout, stderr, calls := runAWSWIF(t, awsWIFData(), tc.env)
+			require.Equal(t, tc.create, awsWIFOperation(calls, "create-open-id-connect-provider") != nil, "%v", calls)
+			if tc.reject {
+				require.NotZero(t, code, "stdout=%s stderr=%s", stdout, stderr)
+				require.Nil(t, awsWIFOperation(calls, "create-role"))
+				require.Nil(t, awsWIFOperation(calls, "put-role-policy"))
+				require.NotContains(t, stdout, "=== Done ===")
+				for _, call := range calls {
+					require.NotEqual(t, "curl", call[0], "registration after failed provider verification")
+				}
+				return
+			}
+			require.Zero(t, code, "%s", stderr)
+			if tc.create {
+				require.NotContains(t, awsWIFOperation(calls, "create-open-id-connect-provider"), "--thumbprint-list")
+			}
+			require.Equal(t, awsWIFProvider, awsWIFArgument(t, awsWIFOperation(calls, "get-open-id-connect-provider"), "--open-id-connect-provider-arn"))
+			var policy map[string]any
+			require.NoError(t, json.Unmarshal([]byte(awsWIFArgument(t, awsWIFOperation(calls, "create-role"), "--assume-role-policy-document")), &policy))
+			want := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"` + awsWIFProvider + `"},"Action":"sts:AssumeRoleWithWebIdentity","Condition":{"StringEquals":{"accounts.google.com:aud":"sts.amazonaws.com","accounts.google.com:sub":"123456789012345678901"}}}]}`
+			actual, err := json.Marshal(policy)
+			require.NoError(t, err)
+			require.JSONEq(t, want, string(actual))
+		})
+	}
+}
+
+func TestAWSWIFCLIInputGuards(t *testing.T) {
+	for _, key := range []string{"OIDC_AUDIENCE", "OIDC_SUBJECT_CLAIM"} {
+		for _, invalid := range []string{"a b", "a\tb", "a\nb", "a\rb", "a\vb", "a\fb", "a$b", "a*b"} {
+			t.Run(key+"/"+fmt.Sprintf("%q", invalid), func(t *testing.T) {
+				code, _, stderr, calls := runAWSWIF(t, awsWIFData(), map[string]string{key: invalid})
+				require.NotZero(t, code)
+				require.Empty(t, calls)
+				require.Contains(t, stderr, key)
+			})
+		}
+	}
+	for _, issuer := range []string{"http://accounts.google.com", "https://accounts.google.com/", "https://"} {
+		t.Run(issuer, func(t *testing.T) {
+			code, _, stderr, calls := runAWSWIF(t, awsWIFData(), map[string]string{"OIDC_ISSUER_URL": issuer})
+			require.NotZero(t, code)
+			require.Empty(t, calls)
+			require.Contains(t, stderr, "OIDC_ISSUER_URL")
+		})
+	}
+	t.Run("missing jq", func(t *testing.T) {
+		code, _, stderr, calls := runAWSWIF(t, awsWIFData(), map[string]string{"PATH": t.TempDir()})
+		require.NotZero(t, code)
+		require.Empty(t, calls)
+		require.Contains(t, stderr, "jq is required")
+	})
+}
+
+func TestAWSWIFCLIAudienceAndLiteralClaims(t *testing.T) {
+	for _, tc := range []struct {
+		name, rendered, want string
+		override             map[string]string
+	}{
+		{"Azure default", "api://AzureADTokenExchange", "api://AzureADTokenExchange", nil},
+		{"rendered empty", "", "sts.amazonaws.com", nil},
+		{"explicit empty", "api://AzureADTokenExchange", "sts.amazonaws.com", map[string]string{"OIDC_AUDIENCE": ""}},
+		{"literal override", "default", `a"b\c`, map[string]string{"OIDC_AUDIENCE": `a"b\c`}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			data := awsWIFData()
+			data.OIDCAudience = tc.rendered
+			if tc.name == "Azure default" {
+				data.Source = "azure"
+				data.OIDCIssuerURL = "https://login.microsoftonline.com/11111111-1111-1111-1111-111111111111/v2.0"
+			}
+			host := strings.TrimPrefix(data.OIDCIssuerURL, "https://")
+			env := map[string]string{"OIDC_SUBJECT_CLAIM": `a"b\c`}
+			for k, v := range tc.override {
+				env[k] = v
+			}
+			body, err := json.Marshal(map[string]any{"Url": host, "ClientIDList": []string{tc.want}, "ThumbprintList": []string{strings.Repeat("a", 40)}})
+			require.NoError(t, err)
+			env["PROVIDER_RESPONSE"] = string(body)
+			code, _, stderr, calls := runAWSWIF(t, data, env)
+			require.Zero(t, code, "%s", stderr)
+			require.Equal(t, tc.want, awsWIFArgument(t, awsWIFOperation(calls, "create-open-id-connect-provider"), "--client-id-list"))
+			var policy struct {
+				Statement []struct {
+					Condition struct{ StringEquals map[string]string }
+				}
+			}
+			require.NoError(t, json.Unmarshal([]byte(awsWIFArgument(t, awsWIFOperation(calls, "create-role"), "--assume-role-policy-document")), &policy))
+			require.Len(t, policy.Statement, 1)
+			require.Equal(t, map[string]string{host + ":aud": tc.want, host + ":sub": `a"b\c`}, policy.Statement[0].Condition.StringEquals)
+		})
+	}
+}

internal/iacfiles/templates_test.go

@@ -102,7 +102,7 @@ func TestCLITemplatesAutoRegister(t *testing.T) {
 				`TARGET_ACCOUNT_ID=$(aws sts get-caller-identity`,
 				// #1640: the trust policy's :sub condition must be present
 				// unconditionally — there is no longer a code path that omits it.
-				`"${OIDC_HOST}:sub": "${OIDC_SUBJECT_CLAIM}"`,
+				`($host + ":sub"): $subject`,
 			},
 			mustNot: []string{
 				"/api/registrations",
@@ -237,7 +237,7 @@ func TestAWSWIFCLI_SubjectClaimRequired(t *testing.T) {
 
 	// Exactly one Condition block, and it always carries :sub — no branch
 	// builds a StringEquals map with only :aud.
-	subCondition := `"${OIDC_HOST}:sub": "${OIDC_SUBJECT_CLAIM}"`
+	subCondition := `($host + ":sub"): $subject`
 	if n := strings.Count(rendered, subCondition); n != 1 {
 		t.Errorf("expected exactly one :sub condition in the rendered trust policy, found %d", n)
 	}
@@ -276,9 +276,12 @@ func awsStubScript(logPath string) string {
 		// JSON, so newlines inside the arguments are folded to spaces first.
 		"args=\"$*\"\n" +
 		"printf '%s\\n' \"${args//$'\\n'/ }\" >> '" + logPath + "'\n" +
-		// "None" is what the script's provider-lookup branches expect when no
-		// OIDC provider exists yet, so the rest of the script proceeds.
-		"echo None\n"
+		"case \"$1 $2\" in\n" +
+		"  'iam list-open-id-connect-providers') echo '{\"OpenIDConnectProviderList\":[]}' ;;\n" +
+		"  'iam create-open-id-connect-provider') echo '{\"OpenIDConnectProviderArn\":\"" + awsWIFProvider + "\"}' ;;\n" +
+		"  'iam get-open-id-connect-provider') echo '{\"Url\":\"accounts.google.com\",\"ClientIDList\":[\"sts.amazonaws.com\"],\"ThumbprintList\":[]}' ;;\n" +
+		"  *) echo None ;;\n" +
+		"esac\n"
 }
 
 // runRenderedWIFScript writes the rendered aws-wif-cli.sh to a temp file, puts a