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..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 @@ -20,8 +22,14 @@ from pydantic import BaseModel from pydantic import ConfigDict from pydantic import Field +from pydantic import field_validator +from pydantic import model_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", @@ -81,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 @@ -93,6 +109,36 @@ 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 _allow_only_bare_wildcard(cls, value: str | None) -> str | None: + 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}", @@ -110,6 +156,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 = 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. + 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( @@ -118,13 +175,9 @@ 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=[ - 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..17e3448d --- /dev/null +++ b/template/tests/unit/iac_management/test_oidc_assume_role_policy.py @@ -0,0 +1,176 @@ +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 + +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 + +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)) + + +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 + ) -> 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()) + + 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_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}", + ] + + +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 + + @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() + + 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(), + ) diff --git a/tests/copier_data/data1.yaml b/tests/copier_data/data1.yaml index e8168e97..726f2c50 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: 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 0d651fb6..1c28e4dc 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: 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 b9f27f98..c85b0e63 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: 4294967296 initial_iac_management_deploy_occurred: true identity_center_production_account_id: 123499989012 configure_cloud_courier: true