From c51069d7a2ac4358839fc5d6c7d6ec34e8d939eb Mon Sep 17 00:00:00 2001 From: Eli Fine <5436635+ejfine@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:09:57 +0000 Subject: [PATCH 1/6] feat: accept GitHub immutable OIDC subject claims in trust policies Repos created after 2026-07-15 (and repos renamed or transferred after then) get Actions OIDC tokens with sub repo:@/@:, which the legacy-only trust condition rejects. Trust policies now list both the legacy subject and the immutable subject with the org ID pinned and the repo ID wildcarded. Adds the central_infra_github_organization_id copier question and a GITHUB_ORG_IDS mapping. --- copier.yml | 5 ++ .../iac_management/lib/constants.py.jinja | 1 + .../iac_management/lib/github_oidc_lib.py | 17 +++++-- .../lib/repo.py | 1 + .../tests/unit/iac_management/__init__.py | 0 .../test_oidc_assume_role_policy.py | 51 +++++++++++++++++++ tests/copier_data/data1.yaml | 1 + tests/copier_data/data2.yaml | 1 + tests/copier_data/data3.yaml | 1 + 9 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 template/tests/unit/iac_management/__init__.py create mode 100644 template/tests/unit/iac_management/test_oidc_assume_role_policy.py diff --git a/copier.yml b/copier.yml index 977b2b4f..52d76193 100644 --- a/copier.yml +++ b/copier.yml @@ -127,6 +127,11 @@ central_infra_github_organization_name: type: str help: What is the name of the GitHub organization that owns the central infrastructure repository? +central_infra_github_organization_id: + type: int + help: What is the numeric ID of that GitHub organization? Open https://api.github.com/orgs/ in a browser (no login needed) and read the "id" field, or run `gh api orgs/ --jq .id`. + validator: "{% if central_infra_github_organization_id <= 0 %}Must be a positive integer{% endif %}" + initial_iac_management_deploy_occurred: type: bool help: Has the initial merge to main and the deployment of the IaC Management stack occurred yet? diff --git a/template/src/aws_central_infrastructure/iac_management/lib/constants.py.jinja b/template/src/aws_central_infrastructure/iac_management/lib/constants.py.jinja index af38d288..cf260c86 100644 --- a/template/src/aws_central_infrastructure/iac_management/lib/constants.py.jinja +++ b/template/src/aws_central_infrastructure/iac_management/lib/constants.py.jinja @@ -3,4 +3,5 @@ CLOUD_COURIER_INFRA_REPO_NAME = "{% endraw %}{{ cloud_courier_infra_repo_name }} CENTRAL_INFRA_PROD_ACCOUNT_ID = "{% endraw %}{{ aws_production_account_id }}{% raw %}" IDENTITY_CENTER_ACCOUNT_ID = "{% endraw %}{{ identity_center_production_account_id }}{% raw %}" CENTRAL_INFRA_GITHUB_ORG_NAME = "{% endraw %}{{ central_infra_github_organization_name }}{% raw %}" +GITHUB_ORG_IDS = {CENTRAL_INFRA_GITHUB_ORG_NAME: {% endraw %}{{ central_infra_github_organization_id }}{% raw %}} CONFIGURE_CLOUD_COURIER = {% endraw %}{{ 'True' if configure_cloud_courier else 'False' }} diff --git a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py index ff8609ad..9ed2bca1 100644 --- a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py +++ b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py @@ -21,6 +21,8 @@ from pydantic import ConfigDict from pydantic import Field +from .constants import GITHUB_ORG_IDS + GITHUB_OIDC_URL = "https://token.actions.githubusercontent.com" CODE_ARTIFACT_SERVICE_BEARER_STATEMENT = GetPolicyDocumentStatementArgs( sid="GetCodeArtifactAuthToken", @@ -110,6 +112,17 @@ def create_role(self, *, provider_arn: str, parent: Resource | None = None) -> i def create_oidc_assume_role_policy( *, oidc_config: GithubOidcConfig, provider_arn: str ) -> AwaitableGetPolicyDocumentResult: + if oidc_config.restrictions is None: + subject_context = "*" + else: + subject_context = oidc_config.restrictions + # TODO: remove the legacy subject format once use_immutable_subject is enabled for every repo in every org in GITHUB_ORG_IDS. GitHub has announced no retirement date; existing repos keep the legacy format until opted in. Check that nothing else still trusts only the legacy format before opting in. + legacy_subject = f"repo:{oidc_config.repo_org}/{oidc_config.repo_name}:{subject_context}" + # Immutable subject format, the default for repos created after 2026-07-15: https://github.blog/changelog/2026-04-23-immutable-subject-claims-for-github-actions-oidc-tokens/ + # TODO: pin the exact repo ID instead of the `@*` wildcard, sourced from the github-repos stack outputs (requires iac-management to run after github-repos) or a GitHub API lookup. + # The wildcard is acceptable for now: the org ID is pinned, so a renamed or squatted org cannot match; the literal `@` after the repo name stops similarly prefixed repo names from matching; + # the remaining exposure is a repo in this org being deleted and recreated under the same name, which requires an org admin, who can already edit these roles; and the legacy format kept alongside is weaker anyway. + immutable_subject = f"repo:{oidc_config.repo_org}@{GITHUB_ORG_IDS[oidc_config.repo_org]}/{oidc_config.repo_name}@*:{subject_context}" return get_policy_document( statements=[ GetPolicyDocumentStatementArgs( @@ -122,9 +135,7 @@ def create_oidc_assume_role_policy( if oidc_config.restrictions is None or oidc_config.restrictions == "*" else "StringEquals", variable="token.actions.githubusercontent.com:sub", - values=[ - f"repo:{oidc_config.repo_org}/{oidc_config.repo_name}:{'*' if oidc_config.restrictions is None else oidc_config.restrictions}" - ], + values=[legacy_subject, immutable_subject], ), GetPolicyDocumentStatementConditionArgs( test="StringEquals", diff --git a/template/src/aws_central_infrastructure/{% if manage_github_repos %}github_repos{% endif %}/lib/repo.py b/template/src/aws_central_infrastructure/{% if manage_github_repos %}github_repos{% endif %}/lib/repo.py index ccf350b5..88aed9da 100644 --- a/template/src/aws_central_infrastructure/{% if manage_github_repos %}github_repos{% endif %}/lib/repo.py +++ b/template/src/aws_central_infrastructure/{% if manage_github_repos %}github_repos{% endif %}/lib/repo.py @@ -130,6 +130,7 @@ def __init__( repo_topics = ["managed-by-aws-central-infrastructure-iac-repo"] repo_topics += config.topics + # TODO: enable GitHub's immutable OIDC subject claims for every repo (ActionsRepositoryOidcSubjectClaimCustomizationTemplate with use_immutable_subject) once the provider supports it: https://github.com/integrations/terraform-provider-github/pull/3582 . That unblocks removing the legacy subject format from the trust policy in create_oidc_assume_role_policy. repo = Repository( append_resource_suffix(config.name, max_length=150), name=config.name, diff --git a/template/tests/unit/iac_management/__init__.py b/template/tests/unit/iac_management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py new file mode 100644 index 00000000..09fee9e6 --- /dev/null +++ b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py @@ -0,0 +1,51 @@ +import random +import string +from unittest.mock import ANY + +from pulumi_aws.iam import get_policy_document +from pytest_mock import MockerFixture + +import aws_central_infrastructure.iac_management.lib.github_oidc_lib as github_oidc_lib_module +from aws_central_infrastructure.iac_management.lib.constants import CENTRAL_INFRA_GITHUB_ORG_NAME +from aws_central_infrastructure.iac_management.lib.constants import GITHUB_ORG_IDS +from aws_central_infrastructure.iac_management.lib.github_oidc_lib import GithubOidcConfig +from aws_central_infrastructure.iac_management.lib.github_oidc_lib import create_oidc_assume_role_policy + + +def _random_account_id() -> str: + return "".join(random.choices(string.digits, k=12)) + + +def _random_name() -> str: + return "".join(random.choices(string.ascii_lowercase + "-", k=8)) + + +class TestCreateOidcAssumeRolePolicy: + def test_Given_no_restrictions__When_policy_created__Then_sub_matches_legacy_and_immutable_subjects( + self, mocker: MockerFixture + ) -> None: + repo_name = _random_name() + oidc_config = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name=repo_name, + ) + mock_get_policy_document = mocker.patch.object(github_oidc_lib_module, get_policy_document.__name__) + + _ = create_oidc_assume_role_policy(oidc_config=oidc_config, provider_arn=_random_name()) + + (statement,) = mock_get_policy_document.call_args.kwargs["statements"] + sub_conditions = [ + condition + for condition in statement.conditions + if condition.variable == "token.actions.githubusercontent.com:sub" + ] + + mock_get_policy_document.assert_called_once_with(statements=[ANY]) + assert len(sub_conditions) == 1 + assert sub_conditions[0].test == "StringLike" + assert sub_conditions[0].values == [ + f"repo:{CENTRAL_INFRA_GITHUB_ORG_NAME}/{repo_name}:*", + f"repo:{CENTRAL_INFRA_GITHUB_ORG_NAME}@{GITHUB_ORG_IDS[CENTRAL_INFRA_GITHUB_ORG_NAME]}/{repo_name}@*:*", + ] diff --git a/tests/copier_data/data1.yaml b/tests/copier_data/data1.yaml index e8168e97..502359fa 100644 --- a/tests/copier_data/data1.yaml +++ b/tests/copier_data/data1.yaml @@ -29,6 +29,7 @@ aws_region_for_stack: us-east-2 # Data added based on the specifics of this template central_infra_github_organization_name: my-org +central_infra_github_organization_id: 12345678 initial_iac_management_deploy_occurred: false identity_center_production_account_id: 023202320444 configure_cloud_courier: false diff --git a/tests/copier_data/data2.yaml b/tests/copier_data/data2.yaml index 0d651fb6..8df58ed1 100644 --- a/tests/copier_data/data2.yaml +++ b/tests/copier_data/data2.yaml @@ -34,6 +34,7 @@ aws_region_for_stack: us-west-1 # Data added based on the specifics of this template central_infra_github_organization_name: foobar +central_infra_github_organization_id: 12345678 initial_iac_management_deploy_occurred: true identity_center_production_account_id: 123499989012 configure_cloud_courier: true diff --git a/tests/copier_data/data3.yaml b/tests/copier_data/data3.yaml index b9f27f98..4760df0b 100644 --- a/tests/copier_data/data3.yaml +++ b/tests/copier_data/data3.yaml @@ -24,6 +24,7 @@ aws_development_account_id: 000321456870 aws_region_for_stack: us-west-1 # Data added based on the specifics of this template central_infra_github_organization_name: foobar +central_infra_github_organization_id: 12345678 initial_iac_management_deploy_occurred: true identity_center_production_account_id: 123499989012 configure_cloud_courier: true From f8d9a49ce809cd0921429db93d2cf10efc0ddc92 Mon Sep 17 00:00:00 2001 From: Eli Fine <5436635+ejfine@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:17:19 +0000 Subject: [PATCH 2/6] fix: use StringLike for every OIDC sub condition The immutable subject pattern wildcards the repo ID, which StringEquals treats literally, so roles scoped to a ref (e.g. main-only deploy, CodeArtifact publish) would still reject tokens from repos using immutable subjects. Literal restrictions match the same tokens under StringLike. --- .../iac_management/lib/github_oidc_lib.py | 4 +- .../test_oidc_assume_role_policy.py | 48 ++++++++++++++++--- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py index 9ed2bca1..3a9b3159 100644 --- a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py +++ b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py @@ -131,9 +131,7 @@ def create_oidc_assume_role_policy( actions=["sts:AssumeRoleWithWebIdentity"], conditions=[ GetPolicyDocumentStatementConditionArgs( - test="StringLike" - if oidc_config.restrictions is None or oidc_config.restrictions == "*" - else "StringEquals", + test="StringLike", variable="token.actions.githubusercontent.com:sub", values=[legacy_subject, immutable_subject], ), diff --git a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py index 09fee9e6..92b49825 100644 --- a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py +++ b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py @@ -1,7 +1,9 @@ import random import string from unittest.mock import ANY +from unittest.mock import MagicMock +from pulumi_aws.iam import GetPolicyDocumentStatementConditionArgs from pulumi_aws.iam import get_policy_document from pytest_mock import MockerFixture @@ -11,6 +13,8 @@ from aws_central_infrastructure.iac_management.lib.github_oidc_lib import GithubOidcConfig from aws_central_infrastructure.iac_management.lib.github_oidc_lib import create_oidc_assume_role_policy +CENTRAL_ORG_IMMUTABLE_PREFIX = f"{CENTRAL_INFRA_GITHUB_ORG_NAME}@{GITHUB_ORG_IDS[CENTRAL_INFRA_GITHUB_ORG_NAME]}" + def _random_account_id() -> str: return "".join(random.choices(string.digits, k=12)) @@ -20,6 +24,15 @@ def _random_name() -> str: return "".join(random.choices(string.ascii_lowercase + "-", k=8)) +def _sub_conditions_passed_to(mock_get_policy_document: MagicMock) -> list[GetPolicyDocumentStatementConditionArgs]: + (statement,) = mock_get_policy_document.call_args.kwargs["statements"] + return [ + condition + for condition in statement.conditions + if condition.variable == "token.actions.githubusercontent.com:sub" + ] + + class TestCreateOidcAssumeRolePolicy: def test_Given_no_restrictions__When_policy_created__Then_sub_matches_legacy_and_immutable_subjects( self, mocker: MockerFixture @@ -35,17 +48,38 @@ def test_Given_no_restrictions__When_policy_created__Then_sub_matches_legacy_and _ = create_oidc_assume_role_policy(oidc_config=oidc_config, provider_arn=_random_name()) - (statement,) = mock_get_policy_document.call_args.kwargs["statements"] - sub_conditions = [ - condition - for condition in statement.conditions - if condition.variable == "token.actions.githubusercontent.com:sub" - ] + sub_conditions = _sub_conditions_passed_to(mock_get_policy_document) mock_get_policy_document.assert_called_once_with(statements=[ANY]) assert len(sub_conditions) == 1 assert sub_conditions[0].test == "StringLike" assert sub_conditions[0].values == [ f"repo:{CENTRAL_INFRA_GITHUB_ORG_NAME}/{repo_name}:*", - f"repo:{CENTRAL_INFRA_GITHUB_ORG_NAME}@{GITHUB_ORG_IDS[CENTRAL_INFRA_GITHUB_ORG_NAME]}/{repo_name}@*:*", + f"repo:{CENTRAL_ORG_IMMUTABLE_PREFIX}/{repo_name}@*:*", + ] + + def test_Given_ref_restriction__When_policy_created__Then_sub_matches_both_subjects_scoped_to_that_ref( + self, mocker: MockerFixture + ) -> None: + repo_name = _random_name() + restriction = f"ref:refs/heads/{_random_name()}" + oidc_config = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name=repo_name, + restrictions=restriction, + ) + mock_get_policy_document = mocker.patch.object(github_oidc_lib_module, get_policy_document.__name__) + + _ = create_oidc_assume_role_policy(oidc_config=oidc_config, provider_arn=_random_name()) + + sub_conditions = _sub_conditions_passed_to(mock_get_policy_document) + + mock_get_policy_document.assert_called_once_with(statements=[ANY]) + assert len(sub_conditions) == 1 + assert sub_conditions[0].test == "StringLike" + assert sub_conditions[0].values == [ + f"repo:{CENTRAL_INFRA_GITHUB_ORG_NAME}/{repo_name}:{restriction}", + f"repo:{CENTRAL_ORG_IMMUTABLE_PREFIX}/{repo_name}@*:{restriction}", ] From 8f065bd83854bc659722476d2c289cf4a89fe380 Mon Sep 17 00:00:00 2001 From: Eli Fine <5436635+ejfine@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:36:30 +0000 Subject: [PATCH 3/6] feat: reject wildcard characters in OIDC restrictions Now that every sub condition uses StringLike, a * or ? inside a restriction would silently widen a scoped role. GithubOidcConfig rejects such restrictions with a ValidationError naming them, while still allowing None and a bare * (used by ECR push roles). --- .../iac_management/lib/github_oidc_lib.py | 17 +++++++- .../test_oidc_assume_role_policy.py | 43 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py index 3a9b3159..002e3c2a 100644 --- a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py +++ b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py @@ -20,10 +20,13 @@ from pydantic import BaseModel from pydantic import ConfigDict from pydantic import Field +from pydantic import field_validator from .constants import GITHUB_ORG_IDS GITHUB_OIDC_URL = "https://token.actions.githubusercontent.com" +IAM_STRING_LIKE_WILDCARD_CHARS = "*?" +ANY_SUBJECT_CONTEXT = "*" CODE_ARTIFACT_SERVICE_BEARER_STATEMENT = GetPolicyDocumentStatementArgs( sid="GetCodeArtifactAuthToken", effect="Allow", @@ -95,6 +98,18 @@ class GithubOidcConfig(BaseModel): model_config = ConfigDict(arbitrary_types_allowed=True) + @field_validator("restrictions") + @classmethod + def _reject_wildcards_in_restrictions(cls, value: str | None) -> str | None: + if value is None: + return value + if value == ANY_SUBJECT_CONTEXT: + return value + for wildcard_char in IAM_STRING_LIKE_WILDCARD_CHARS: + if wildcard_char in value: + raise ValueError(f"OIDC restriction {value!r} must not contain wildcard characters") # noqa: TRY003 # pydantic validators must raise ValueError for it to be converted into a ValidationError + return value + def create_role(self, *, provider_arn: str, parent: Resource | None = None) -> iam.Role: return iam.Role( f"{self.role_resource_name_prefix}{self.role_name}", @@ -113,7 +128,7 @@ def create_oidc_assume_role_policy( *, oidc_config: GithubOidcConfig, provider_arn: str ) -> AwaitableGetPolicyDocumentResult: if oidc_config.restrictions is None: - subject_context = "*" + subject_context = ANY_SUBJECT_CONTEXT else: subject_context = oidc_config.restrictions # TODO: remove the legacy subject format once use_immutable_subject is enabled for every repo in every org in GITHUB_ORG_IDS. GitHub has announced no retirement date; existing repos keep the legacy format until opted in. Check that nothing else still trusts only the legacy format before opting in. diff --git a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py index 92b49825..d7223d57 100644 --- a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py +++ b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py @@ -1,8 +1,11 @@ import random +import re import string from unittest.mock import ANY from unittest.mock import MagicMock +import pydantic +import pytest from pulumi_aws.iam import GetPolicyDocumentStatementConditionArgs from pulumi_aws.iam import get_policy_document from pytest_mock import MockerFixture @@ -10,6 +13,7 @@ import aws_central_infrastructure.iac_management.lib.github_oidc_lib as github_oidc_lib_module from aws_central_infrastructure.iac_management.lib.constants import CENTRAL_INFRA_GITHUB_ORG_NAME from aws_central_infrastructure.iac_management.lib.constants import GITHUB_ORG_IDS +from aws_central_infrastructure.iac_management.lib.github_oidc_lib import ANY_SUBJECT_CONTEXT from aws_central_infrastructure.iac_management.lib.github_oidc_lib import GithubOidcConfig from aws_central_infrastructure.iac_management.lib.github_oidc_lib import create_oidc_assume_role_policy @@ -83,3 +87,42 @@ def test_Given_ref_restriction__When_policy_created__Then_sub_matches_both_subje f"repo:{CENTRAL_INFRA_GITHUB_ORG_NAME}/{repo_name}:{restriction}", f"repo:{CENTRAL_ORG_IMMUTABLE_PREFIX}/{repo_name}@*:{restriction}", ] + + +class TestWhenGithubOidcConfigCreated: + @pytest.mark.parametrize("wildcard_char", ["*", "?"]) + def test_Given_restriction_containing_wildcard__Then_validation_error_names_restriction( + self, wildcard_char: str + ) -> None: + restriction = f"ref:refs/heads/{_random_name()}{wildcard_char}" + + with pytest.raises(pydantic.ValidationError, match=re.escape(restriction)): + _ = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name=_random_name(), + restrictions=restriction, + ) + + def test_Given_bare_wildcard_restriction__Then_config_keeps_it(self) -> None: + oidc_config = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name=_random_name(), + restrictions=ANY_SUBJECT_CONTEXT, + ) + + assert oidc_config.restrictions == ANY_SUBJECT_CONTEXT + + def test_Given_restrictions_explicitly_none__Then_config_has_no_restrictions(self) -> None: + oidc_config = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name=_random_name(), + restrictions=None, + ) + + assert oidc_config.restrictions is None From 5b196bd48baea06f75dfaf70d1287b6785892255 Mon Sep 17 00:00:00 2001 From: Eli Fine <5436635+ejfine@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:47:54 +0000 Subject: [PATCH 4/6] feat: require a known GitHub org ID for every OIDC role GithubOidcConfig now rejects a repo_org missing from GITHUB_ORG_IDS at construction, with a message explaining how to look the ID up, instead of a bare KeyError while building the trust policy. Also renames the restrictions validator to _allow_only_bare_wildcard to make its intent clearer. --- .../iac_management/lib/github_oidc_lib.py | 11 ++++++++++- .../iac_management/test_oidc_assume_role_policy.py | 11 +++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py index 002e3c2a..68e275f5 100644 --- a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py +++ b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py @@ -98,9 +98,18 @@ class GithubOidcConfig(BaseModel): model_config = ConfigDict(arbitrary_types_allowed=True) + @field_validator("repo_org") + @classmethod + def _require_known_org_id(cls, value: str) -> str: + if value not in GITHUB_ORG_IDS: + raise ValueError( # noqa: TRY003 # pydantic validators must raise ValueError for it to be converted into a ValidationError + f'GitHub org {value!r} has no entry in GITHUB_ORG_IDS; add it (find the ID in the "id" field at https://api.github.com/orgs/{value} or with `gh api orgs/{value} --jq .id`)' + ) + return value + @field_validator("restrictions") @classmethod - def _reject_wildcards_in_restrictions(cls, value: str | None) -> str | None: + def _allow_only_bare_wildcard(cls, value: str | None) -> str | None: if value is None: return value if value == ANY_SUBJECT_CONTEXT: diff --git a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py index d7223d57..46a8c329 100644 --- a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py +++ b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py @@ -126,3 +126,14 @@ def test_Given_restrictions_explicitly_none__Then_config_has_no_restrictions(sel ) assert oidc_config.restrictions is None + + def test_Given_repo_org_without_known_org_id__Then_validation_error_names_org(self) -> None: + unknown_org_name = _random_name() + + with pytest.raises(pydantic.ValidationError, match=re.escape(unknown_org_name)): + _ = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=unknown_org_name, + repo_name=_random_name(), + ) From 8d49ca1abac00e60f8ef7bf5bcb5dc9540adac20 Mon Sep 17 00:00:00 2001 From: Eli Fine <5436635+ejfine@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:07:25 +0000 Subject: [PATCH 5/6] test: vary org ID answers across copier test data Use a single-digit, a nine-digit, and a ten-digit (beyond 32-bit) org ID so template rendering is exercised across realistic and boundary lengths. --- tests/copier_data/data1.yaml | 2 +- tests/copier_data/data2.yaml | 2 +- tests/copier_data/data3.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/copier_data/data1.yaml b/tests/copier_data/data1.yaml index 502359fa..726f2c50 100644 --- a/tests/copier_data/data1.yaml +++ b/tests/copier_data/data1.yaml @@ -29,7 +29,7 @@ aws_region_for_stack: us-east-2 # Data added based on the specifics of this template central_infra_github_organization_name: my-org -central_infra_github_organization_id: 12345678 +central_infra_github_organization_id: 7 initial_iac_management_deploy_occurred: false identity_center_production_account_id: 023202320444 configure_cloud_courier: false diff --git a/tests/copier_data/data2.yaml b/tests/copier_data/data2.yaml index 8df58ed1..1c28e4dc 100644 --- a/tests/copier_data/data2.yaml +++ b/tests/copier_data/data2.yaml @@ -34,7 +34,7 @@ aws_region_for_stack: us-west-1 # Data added based on the specifics of this template central_infra_github_organization_name: foobar -central_infra_github_organization_id: 12345678 +central_infra_github_organization_id: 987654321 initial_iac_management_deploy_occurred: true identity_center_production_account_id: 123499989012 configure_cloud_courier: true diff --git a/tests/copier_data/data3.yaml b/tests/copier_data/data3.yaml index 4760df0b..c85b0e63 100644 --- a/tests/copier_data/data3.yaml +++ b/tests/copier_data/data3.yaml @@ -24,7 +24,7 @@ aws_development_account_id: 000321456870 aws_region_for_stack: us-west-1 # Data added based on the specifics of this template central_infra_github_organization_name: foobar -central_infra_github_organization_id: 12345678 +central_infra_github_organization_id: 4294967296 initial_iac_management_deploy_occurred: true identity_center_production_account_id: 123499989012 configure_cloud_courier: true From 628cd8908517203ca83c5c7a3c556a9b719d74cb Mon Sep 17 00:00:00 2001 From: Eli Fine <5436635+ejfine@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:34:40 +0000 Subject: [PATCH 6/6] fix: reject wildcard repo names on ref-restricted OIDC roles With every sub condition now using StringLike, a wildcard repo_name combined with a specific restriction (e.g. ref:refs/heads/main) would trust that ref in every repo in the org. GithubOidcConfig now rejects that combination, naming both values, while still allowing wildcard repo names when there is no restriction or a bare * restriction (as used by CoreInfraBaseAccess). --- .../iac_management/lib/github_oidc_lib.py | 26 +++++++++++-- .../test_oidc_assume_role_policy.py | 37 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py index 68e275f5..08ff875e 100644 --- a/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py +++ b/template/src/aws_central_infrastructure/iac_management/lib/github_oidc_lib.py @@ -1,4 +1,6 @@ +from typing import Self from typing import TypedDict +from typing import TypeGuard from ephemeral_pulumi_deploy import get_config_str from ephemeral_pulumi_deploy.utils import common_tags_native @@ -21,6 +23,7 @@ from pydantic import ConfigDict from pydantic import Field from pydantic import field_validator +from pydantic import model_validator from .constants import GITHUB_ORG_IDS @@ -86,6 +89,14 @@ def _none_if_empty[T](items: list[T]) -> list[T] | None: return items +def _is_specific_restriction(restriction: str | None) -> TypeGuard[str]: + if restriction is None: + return False + if restriction == ANY_SUBJECT_CONTEXT: + return False + return True + + class GithubOidcConfig(BaseModel): aws_account_id: str role_name: str @@ -110,15 +121,24 @@ def _require_known_org_id(cls, value: str) -> str: @field_validator("restrictions") @classmethod def _allow_only_bare_wildcard(cls, value: str | None) -> str | None: - if value is None: - return value - if value == ANY_SUBJECT_CONTEXT: + if not _is_specific_restriction(value): return value for wildcard_char in IAM_STRING_LIKE_WILDCARD_CHARS: if wildcard_char in value: raise ValueError(f"OIDC restriction {value!r} must not contain wildcard characters") # noqa: TRY003 # pydantic validators must raise ValueError for it to be converted into a ValidationError return value + @model_validator(mode="after") + def _reject_wildcard_repo_name_when_restricted(self) -> Self: + if not _is_specific_restriction(self.restrictions): + return self + for wildcard_char in IAM_STRING_LIKE_WILDCARD_CHARS: + if wildcard_char in self.repo_name: + raise ValueError( # noqa: TRY003 # pydantic validators must raise ValueError for it to be converted into a ValidationError + f"OIDC repo name {self.repo_name!r} must not contain wildcard characters when restricted to {self.restrictions!r}" + ) + return self + def create_role(self, *, provider_arn: str, parent: Resource | None = None) -> iam.Role: return iam.Role( f"{self.role_resource_name_prefix}{self.role_name}", diff --git a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py index 46a8c329..17e3448d 100644 --- a/template/tests/unit/iac_management/test_oidc_assume_role_policy.py +++ b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py @@ -127,6 +127,43 @@ def test_Given_restrictions_explicitly_none__Then_config_has_no_restrictions(sel assert oidc_config.restrictions is None + @pytest.mark.parametrize("wildcard_char", ["*", "?"]) + def test_Given_wildcard_repo_name_with_ref_restriction__Then_validation_error_names_repo_and_restriction( + self, wildcard_char: str + ) -> None: + repo_name = f"{_random_name()}{wildcard_char}" + restriction = f"ref:refs/heads/{_random_name()}" + + with pytest.raises(pydantic.ValidationError, match=f"{re.escape(repo_name)}.*{re.escape(restriction)}"): + _ = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name=repo_name, + restrictions=restriction, + ) + + def test_Given_wildcard_repo_name_without_restriction__Then_config_keeps_it(self) -> None: + oidc_config = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name="*", + ) + + assert oidc_config.repo_name == "*" + + def test_Given_wildcard_repo_name_with_bare_wildcard_restriction__Then_config_keeps_it(self) -> None: + oidc_config = GithubOidcConfig( + aws_account_id=_random_account_id(), + role_name=_random_name(), + repo_org=CENTRAL_INFRA_GITHUB_ORG_NAME, + repo_name="*", + restrictions=ANY_SUBJECT_CONTEXT, + ) + + assert oidc_config.repo_name == "*" + def test_Given_repo_org_without_known_org_id__Then_validation_error_names_org(self) -> None: unknown_org_name = _random_name()