From 41b88f2c858bc6cd65a7b019bc929dd806abd496 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 7 Aug 2026 12:10:27 -0400 Subject: [PATCH] fix(test): restore exact SHOC principal pin in terraform Pin shoc_consumer_role_arn's Terraform default and example to the trusted ARN, and require grant sites to consume local.shoc_consumer_role_arn only. --- terraform/terraform.tfvars.example | 3 +- terraform/variables.tf | 3 +- tests/test_cross_account_principal_pin.py | 101 +++++++++++++++++----- 3 files changed, 84 insertions(+), 23 deletions(-) diff --git a/terraform/terraform.tfvars.example b/terraform/terraform.tfvars.example index ddd2d5b..b8c8002 100644 --- a/terraform/terraform.tfvars.example +++ b/terraform/terraform.tfvars.example @@ -4,5 +4,6 @@ web_ui_auth_token_secret_arn = "arn:aws:secretsmanager:us-east-1:011934824531:se shoc_hmac_secret_arn = "arn:aws:secretsmanager:us-east-1:011934824531:secret:workorder-ingest/shoc-webhook-hmac-puYTcB" # Required: no Terraform default. Live emitter target today (contract Rev 2026-07-23). shoc_webhook_url = "https://api.dev.seahaven.com/api/webhooks/work-orders" -# Required exact-ARN pin for cross-account HMAC read (docs/shoc-webhook-contract.md). +# Exact-ARN pin for cross-account HMAC/API trust (matches variable default; +# docs/shoc-webhook-contract.md). Changing this is a deliberate IAM review. shoc_consumer_role_arn = "arn:aws:iam::396287094661:role/shoc-backend-dev" diff --git a/terraform/variables.tf b/terraform/variables.tf index 1df0930..7eee3f6 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -21,5 +21,6 @@ variable "shoc_webhook_url" { variable "shoc_consumer_role_arn" { type = string - description = "Exact IAM role ARN allowed to GetSecretValue / kms:Decrypt the SHOC HMAC secret (cross-account consumer). Live pin today is arn:aws:iam::396287094661:role/shoc-backend-dev per docs/shoc-webhook-contract.md." + description = "Exact IAM role ARN allowed to GetSecretValue / kms:Decrypt the SHOC HMAC secret and invoke the read API (cross-account consumer). Default is the live pin per docs/shoc-webhook-contract.md; changing it is a deliberate cross-family IAM review." + default = "arn:aws:iam::396287094661:role/shoc-backend-dev" } diff --git a/tests/test_cross_account_principal_pin.py b/tests/test_cross_account_principal_pin.py index fce6c88..1b80b1c 100644 --- a/tests/test_cross_account_principal_pin.py +++ b/tests/test_cross_account_principal_pin.py @@ -2,11 +2,14 @@ The SHOC integration deliberately trusts EXACTLY ONE foreign principal: ``arn:aws:iam::396287094661:role/shoc-backend-dev`` (API resource policy in -api.tf, HMAC secret + KMS grants in wo_shoc.tf; documented literal in -variables.tf + terraform.tfvars.example). Future shoc-backend-staging/-prod -roles are each a deliberate, individually-reviewed policy addition — so any -new foreign account id or role ARN appearing under terraform/ must consciously -update this pin (and go through the mandatory GPT-4.1 cross-family IAM review). +api.tf, HMAC secret + KMS grants in wo_shoc.tf). The exact ARN is pinned as +the ``shoc_consumer_role_arn`` variable default and in +``terraform.tfvars.example``; grant sites consume ``local.shoc_consumer_role_arn`` +only. Future shoc-backend-staging/-prod roles are each a deliberate, +individually-reviewed policy addition — so any new foreign account id or role +ARN appearing under terraform/, or any change to the default/example pin, must +consciously update this test (and go through the mandatory GPT-4.1 cross-family +IAM review). Raised as a QUESTION in the 2026-07-24 cross-family review of the webhook emitter policy surface: "how is the exact-one-principal @@ -18,33 +21,73 @@ from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[1] TF_DIR = REPO_ROOT / "terraform" +VARIABLES_TF = TF_DIR / "variables.tf" +TFVARS_EXAMPLE = TF_DIR / "terraform.tfvars.example" -# The one foreign principal the app may reference as a literal ARN, and the -# only files allowed to embed that literal (grants use the variable). +# The one foreign principal the app may trust. ALLOWED_FOREIGN_PRINCIPAL = "arn:aws:iam::396287094661:role/shoc-backend-dev" + +# Files allowed to embed that ARN as a literal (default + example). Grant +# sites must use local.shoc_consumer_role_arn instead. ALLOWED_LITERAL_FILES = {"variables.tf", "terraform.tfvars.example"} -# Grant sites must keep referencing the variable so dropping one half of the -# secret/KMS/API trust surface fails this pin. -GRANT_FILES = { - "api.tf": "shoc_consumer_role_arn", - "wo_shoc.tf": "shoc_consumer_role_arn", -} +# Grant sites that must reference the local; dropping one half fails the pin. +GRANT_FILES = ("api.tf", "wo_shoc.tf") # Accounts that are not "foreign": seahaven-prod (the deploy target). HOME_ACCOUNTS = {"011934824531"} _IAM_ARN_RE = re.compile(r"arn:aws:iam::(\d{12}):\S*?(?=[\"'\s])") +_VAR_DEFAULT_RE = re.compile( + r'variable\s+"shoc_consumer_role_arn"\s*\{(.*?)^\}', + re.DOTALL | re.MULTILINE, +) +_DEFAULT_VALUE_RE = re.compile(r'default\s*=\s*"([^"]+)"') +_TFVARS_VALUE_RE = re.compile( + r'^shoc_consumer_role_arn\s*=\s*"([^"]+)"\s*$', + re.MULTILINE, +) +_LOCAL_REF = "local.shoc_consumer_role_arn" def _terraform_sources(): paths = sorted(TF_DIR.glob("*.tf")) - example = TF_DIR / "terraform.tfvars.example" - if example.exists(): - paths.append(example) + if TFVARS_EXAMPLE.exists(): + paths.append(TFVARS_EXAMPLE) return paths +def test_variable_default_is_the_pinned_foreign_principal(): + """The Terraform default must equal the exact trusted ARN. + + Grant sites consume the variable via local.shoc_consumer_role_arn. Pinning + the default restores the exact-principal invariant the former CDK scan + enforced: widening trust requires editing this default (and this test). + """ + block = _VAR_DEFAULT_RE.search(VARIABLES_TF.read_text()) + assert block, "variables.tf must declare variable shoc_consumer_role_arn" + default = _DEFAULT_VALUE_RE.search(block.group(1)) + assert default, ( + "variable shoc_consumer_role_arn must set default = " + f'"{ALLOWED_FOREIGN_PRINCIPAL}" so the exact principal is pinned in-repo' + ) + assert default.group(1) == ALLOWED_FOREIGN_PRINCIPAL, ( + f"shoc_consumer_role_arn default is {default.group(1)!r}, expected " + f"{ALLOWED_FOREIGN_PRINCIPAL!r}" + ) + + +def test_tfvars_example_is_the_pinned_foreign_principal(): + match = _TFVARS_VALUE_RE.search(TFVARS_EXAMPLE.read_text()) + assert match, ( + "terraform.tfvars.example must set shoc_consumer_role_arn to the pinned ARN" + ) + assert match.group(1) == ALLOWED_FOREIGN_PRINCIPAL, ( + f"terraform.tfvars.example shoc_consumer_role_arn is {match.group(1)!r}, " + f"expected {ALLOWED_FOREIGN_PRINCIPAL!r}" + ) + + def test_only_the_pinned_foreign_principal_appears_in_terraform_sources(): findings = [] for path in _terraform_sources(): @@ -64,14 +107,30 @@ def test_only_the_pinned_foreign_principal_appears_in_terraform_sources(): "cross-account trust addition must update this pin deliberately: " f"{unexpected}" ) - # Both documentation/example sites must still name the pinned role. + # Both default/example sites must still name the pinned role as a literal. assert {name for name, _ in findings} == ALLOWED_LITERAL_FILES -def test_grant_files_reference_shoc_consumer_role_arn(): - for filename, needle in GRANT_FILES.items(): +def test_grant_files_use_local_shoc_consumer_role_arn_only(): + """Grant sites must consume the local — never a hardcoded foreign ARN. + + api.tf (API resource policy) and wo_shoc.tf (KMS + secret policy) are the + two halves of the trust surface. Each must reference + local.shoc_consumer_role_arn so dropping one half fails CI, and neither + may embed a raw foreign IAM ARN (that would bypass the variable default pin). + """ + for filename in GRANT_FILES: text = (TF_DIR / filename).read_text() - assert needle in text, ( - f"{filename} must reference {needle} so the SHOC cross-account " + assert _LOCAL_REF in text, ( + f"{filename} must reference {_LOCAL_REF} so the SHOC cross-account " "grant surface cannot silently drop one half of the trust pair" ) + foreign = [ + m.group(0) + for m in _IAM_ARN_RE.finditer(text) + if m.group(1) not in HOME_ACCOUNTS + ] + assert not foreign, ( + f"{filename} must not hardcode foreign IAM ARNs " + f"(use {_LOCAL_REF}): {foreign}" + )