From cc2db2c04fcfed6f2c84bd41aa9a2bbf626319e3 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 18 Sep 2026 14:20:44 -0400 Subject: [PATCH] fix(terraform): grant shoc-backend-staging HMAC and API access (PLAT-211) Terraform still pinned only shoc-backend-dev, so the next ingest apply would drop the live staging HMAC grant. --- README.md | 4 +- docs/shoc-webhook-contract.md | 2 +- terraform/api.tf | 4 +- terraform/locals.tf | 2 +- terraform/terraform.tfvars.example | 5 +- terraform/variables.tf | 19 ++++- terraform/wo_shoc.tf | 6 +- tests/test_cross_account_principal_pin.py | 93 +++++++++++++---------- 8 files changed, 82 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index bec2640..da6a0c2 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ Amazon APM work order emails (from Hexagon EAM / HxGN SmartCloud) are received a - **Ordering/retry semantics.** `parallelization_factor=1`, `bisect_batch_on_error=False`, `retry_attempts=-1`, `maximum_record_age=24h`: a retryable failure (429/5xx/timeout/connection error) blocks the shard and retries from the failed record — per-work-order commit order is the guarantee, and blocking is the intended behavior when SHOC is down. `report_batch_item_failures` keeps earlier in-batch successes from being re-delivered. Records that exhaust the 24h age are parked as ESM **failure metadata** (not full records) on `workorder-shoc-emitter-failures` (`on_failure` destination); a non-retryable 4xx (a contract bug, never worth blocking the shard for 24h) parks the **full `{envelope, response_status}` payload** on `workorder-shoc-emitter-rejected` and the loop continues. Both queues: 14-day retention, SSL-enforced, alarmed (see [CloudWatch alarms](#cloudwatch-alarms)); recovery is `scripts/replay_shoc_webhooks.py` (see [Scripts](#scripts)). - **Secret + KMS.** The HMAC signing keys live in Secrets Manager secret `workorder-ingest/shoc-webhook-hmac` (value `{"keys": [{"kid", "secret"}, ...]}`, newest first, max 2), encrypted with the dedicated CMK `workorder-ingest-shoc-webhook-kms` — deliberately **not** `alias/seahaven-dynamodb`, so the SHOC cross-account grant's decrypt reach covers exactly this one secret and nothing else. The secret's removal policy is **`DESTROY`, deliberately not `RETAIN`**: the value is machine-generated HMAC material, fully regenerable by a single rotation, so `RETAIN` buys nothing and would expose the fixed-name RETAIN-orphan deadlock (a failed create orphans an empty shell holding the global name; every later create fails `AlreadyExists`). **Accidental-deletion recovery runbook:** redeploy to recreate the secret, force a rotation (`aws secretsmanager rotate-secret --secret-id workorder-ingest/shoc-webhook-hmac`), notify the SHOC team — receivers re-fetch within their ≤5-minute cache TTL, so no coordination window is needed — then watch the `-failures` queue and replay the gap with the replay script. - **Rotation.** `workorder-shoc-hmac-rotator` runs on a 30-day schedule: it prepends a fresh 64-hex-char key as `keys[0]` and truncates the list to 2 entries (one overlap cycle). `kid` format is `YYYY-MM-DDTHH`. The emitter always signs with `keys[0]` behind a 5-minute TTL cache; the receiver accepts any listed `kid` and re-fetches on an unknown one — there is no delivery window in which signatures can't verify. -- **Cross-account grants (exact ARN only):** `arn:aws:iam::396287094661:role/shoc-backend-dev` is granted `secretsmanager:GetSecretValue` on the secret's resource policy **and** `kms:Decrypt` on the CMK's key policy — both halves are required; either one alone fails silently at the receiver. Future staging/prod receiver roles are each a deliberate, individually-reviewed policy addition — no wildcard/prefix trust. +- **Cross-account grants (exact ARN only):** `arn:aws:iam::396287094661:role/shoc-backend-dev` and `arn:aws:iam::396287094661:role/shoc-backend-staging` are granted `secretsmanager:GetSecretValue` on the secret's resource policy **and** `kms:Decrypt` on the CMK's key policy — both halves are required; either one alone fails silently at the receiver. A future prod receiver role is a deliberate, individually-reviewed policy addition — no wildcard/prefix trust. ### Procurement API (`procurement-api` stack) @@ -110,7 +110,7 @@ A read-only REST API (API Gateway + the `procurement-api` Lambda, `lambdas/api/` | `GET /docs`, `GET /openapi.json` | shared docs token (`X-Auth-Token` header or `?token=` in a browser) | Redoc reference docs (vendored offline, no CDN) with the spec inlined / the committed spec | - **Spec is source of truth:** `lambdas/api/openapi.json` (OpenAPI 3.1). Its top-level `webhooks` section documents the outbound SHOC work-order feed, so one page describes both directions (call + be-called). `tests/test_api_spec_drift.py` pins the spec's paths to the router table, so spec and implementation cannot drift. -- **Auth:** data routes use API Gateway `AWS_IAM` (SigV4) plus a resource policy allowing exactly `arn:aws:iam::396287094661:role/shoc-backend-dev` on `GET/*`; same-account admin callers authorize via identity policy (Postman signs SigV4 natively). Docs routes are auth `NONE` at the gateway (resource-policy carve-out for exactly those two GETs) but the handler fails closed on the shared token (`lambdas/shared/web_ui_auth.py`, secret `procurement-ingest/web-ui-auth-token`) — not an unauthenticated data path (INFRA-74 posture). +- **Auth:** data routes use API Gateway `AWS_IAM` (SigV4) plus a resource policy allowing exactly `arn:aws:iam::396287094661:role/shoc-backend-dev` and `arn:aws:iam::396287094661:role/shoc-backend-staging` on `GET/*`; same-account admin callers authorize via identity policy (Postman signs SigV4 natively). Docs routes are auth `NONE` at the gateway (resource-policy carve-out for exactly those two GETs) but the handler fails closed on the shared token (`lambdas/shared/web_ui_auth.py`, secret `procurement-ingest/web-ui-auth-token`) — not an unauthenticated data path (INFRA-74 posture). - **Custom domain:** `https://procurement-api.seahaven.com` (REGIONAL API Gateway domain, TLS 1.2, empty base-path mapping to the `prod` stage, so callers hit `/work-orders` with no `/prod` segment). The stable SHOC-facing endpoint; the raw `*.execute-api.us-east-1.amazonaws.com/prod` URL still works. **Cross-account DNS:** the `seahaven.com` public zone is in the mgmt account (`328440206208`), so the ACM cert's validation record and the A-alias are added there out of band — `scripts/setup_procurement_api_domain.sh cert` issues the cert (DNS-validated against the mgmt zone) and writes its ARN to prod SSM `/procurement-api/custom-domain/certificate-arn`, which Terraform reads (`data.aws_ssm_parameter` in `terraform/api.tf`); after an HCP apply that owns the API Gateway DomainName, `scripts/setup_procurement_api_domain.sh alias` adds the A-alias from API Gateway `get-domain-name` (`regionalDomainName` / `regionalHostedZoneId`). SigV4 is unaffected (same underlying API id + resource policy); the docs page injects whichever host served the request into `servers[0].url`. - **KMS:** `purchase-orders` is CMK-encrypted; the imported-by-name table doesn't carry the key association, so the stack grants `kms:Decrypt`/`DescribeKey` on the CMK from SSM `/seahaven/dynamodb/cmk-arn` explicitly (the INFRA-104 failure class). - **No access logging in v1** (keeps the `?token=` shim out of any log and avoids the account-level API Gateway CloudWatch role); rotate the docs token before ever enabling it. No CORS (server-to-server + Postman callers). diff --git a/docs/shoc-webhook-contract.md b/docs/shoc-webhook-contract.md index 526aeec..c1ee1e0 100644 --- a/docs/shoc-webhook-contract.md +++ b/docs/shoc-webhook-contract.md @@ -114,7 +114,7 @@ Verification requirements (fail closed — reject with `401` on any failure): ### 6.1 Key material and rotation -The secret lives in **seahaven-prod (011934824531)** Secrets Manager: **`workorder-ingest/shoc-webhook-hmac`**, encrypted with a dedicated customer-managed KMS key so it is readable cross-account. The SHOC backend role (`arn:aws:iam::396287094661:role/shoc-backend-dev`) is granted `secretsmanager:GetSecretValue` + `kms:Decrypt` via resource policies — exact role ARN only; future `shoc-backend-staging`/`-prod` roles are each a deliberate, individually-reviewed policy addition (no wildcard/prefix trust). Secret value shape: +The secret lives in **seahaven-prod (011934824531)** Secrets Manager: **`workorder-ingest/shoc-webhook-hmac`**, encrypted with a dedicated customer-managed KMS key so it is readable cross-account. The SHOC backend roles (`arn:aws:iam::396287094661:role/shoc-backend-dev` and `arn:aws:iam::396287094661:role/shoc-backend-staging`) are granted `secretsmanager:GetSecretValue` + `kms:Decrypt` via resource policies — exact role ARNs only; a future `shoc-backend-prod` role is a deliberate, individually-reviewed policy addition (no wildcard/prefix trust). Secret value shape: ```json { diff --git a/terraform/api.tf b/terraform/api.tf index 72a50b1..80173e5 100644 --- a/terraform/api.tf +++ b/terraform/api.tf @@ -3,10 +3,10 @@ locals { Version = "2012-10-17" Statement = [ { - Sid = "ShocBackendDevDataRead" + Sid = "ShocBackendDataRead" Effect = "Allow" Principal = { - AWS = local.shoc_consumer_role_arn + AWS = local.shoc_consumer_role_arns } Action = "execute-api:Invoke" Resource = [ diff --git a/terraform/locals.tf b/terraform/locals.tf index a8f3932..be498e1 100644 --- a/terraform/locals.tf +++ b/terraform/locals.tf @@ -4,7 +4,7 @@ locals { bedrock_model_id = "us.anthropic.claude-haiku-4-5-20251001-v1:0" - shoc_consumer_role_arn = var.shoc_consumer_role_arn + shoc_consumer_role_arns = var.shoc_consumer_role_arns po_email_bucket_name = "po-ingest-emails-${local.account_id}" wo_email_bucket_name = "workorder-ingest-emails-${local.account_id}" diff --git a/terraform/terraform.tfvars.example b/terraform/terraform.tfvars.example index b8c8002..62148aa 100644 --- a/terraform/terraform.tfvars.example +++ b/terraform/terraform.tfvars.example @@ -6,4 +6,7 @@ shoc_hmac_secret_arn = "arn:aws:secretsmanager:us-east-1:011934824531:se shoc_webhook_url = "https://api.dev.seahaven.com/api/webhooks/work-orders" # 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" +shoc_consumer_role_arns = [ + "arn:aws:iam::396287094661:role/shoc-backend-dev", + "arn:aws:iam::396287094661:role/shoc-backend-staging", +] diff --git a/terraform/variables.tf b/terraform/variables.tf index 48d9300..9b4e927 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -26,8 +26,19 @@ variable "shoc_webhook_url" { description = "SHOC webhook HTTPS endpoint URL (required; no default — set explicitly in HCP workspace vars)" } -variable "shoc_consumer_role_arn" { - type = string - 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" +variable "shoc_consumer_role_arns" { + type = list(string) + description = "Exact IAM role ARNs allowed to GetSecretValue / kms:Decrypt the SHOC HMAC secret and invoke the read API (cross-account consumers). Default is the live pin per docs/shoc-webhook-contract.md; each addition is a deliberate cross-family IAM review. No wildcards." + default = [ + "arn:aws:iam::396287094661:role/shoc-backend-dev", + "arn:aws:iam::396287094661:role/shoc-backend-staging", + ] + + validation { + condition = toset(var.shoc_consumer_role_arns) == toset([ + "arn:aws:iam::396287094661:role/shoc-backend-dev", + "arn:aws:iam::396287094661:role/shoc-backend-staging", + ]) + error_message = "shoc_consumer_role_arns must be exactly the shoc-backend-dev and shoc-backend-staging role ARNs in account 396287094661; no wildcards, omissions, or extra principals." + } } diff --git a/terraform/wo_shoc.tf b/terraform/wo_shoc.tf index a445270..b4ff359 100644 --- a/terraform/wo_shoc.tf +++ b/terraform/wo_shoc.tf @@ -12,14 +12,14 @@ data "aws_iam_policy_document" "shoc_webhook_kms" { } statement { - sid = "ShocBackendDevDecrypt" + sid = "ShocBackendDecrypt" effect = "Allow" actions = ["kms:Decrypt"] resources = ["*"] principals { type = "AWS" - identifiers = [local.shoc_consumer_role_arn] + identifiers = local.shoc_consumer_role_arns } condition { @@ -146,7 +146,7 @@ resource "aws_secretsmanager_secret_policy" "shoc_webhook_hmac" { { Effect = "Allow" Principal = { - AWS = local.shoc_consumer_role_arn + AWS = local.shoc_consumer_role_arns } Action = [ "secretsmanager:GetSecretValue", diff --git a/tests/test_cross_account_principal_pin.py b/tests/test_cross_account_principal_pin.py index 1b80b1c..d8e4dec 100644 --- a/tests/test_cross_account_principal_pin.py +++ b/tests/test_cross_account_principal_pin.py @@ -1,11 +1,12 @@ """Pin the cross-account principal surface of the Terraform app. -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). 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, +The SHOC integration trusts EXACTLY these foreign principals: +``arn:aws:iam::396287094661:role/shoc-backend-dev`` and +``arn:aws:iam::396287094661:role/shoc-backend-staging`` (API resource policy in +api.tf, HMAC secret + KMS grants in wo_shoc.tf). The exact ARNs are pinned as +the ``shoc_consumer_role_arns`` variable default and in +``terraform.tfvars.example``; grant sites consume ``local.shoc_consumer_role_arns`` +only. Future shoc-backend-prod (or any other) 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 @@ -13,7 +14,8 @@ 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 -invariant enforced over time?" — this test is the answer. +invariant enforced over time?" — this test is the answer, now an +exact-allowlist rather than a single ARN. """ import re @@ -24,11 +26,15 @@ 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 trust. -ALLOWED_FOREIGN_PRINCIPAL = "arn:aws:iam::396287094661:role/shoc-backend-dev" +ALLOWED_FOREIGN_PRINCIPALS = frozenset( + { + "arn:aws:iam::396287094661:role/shoc-backend-dev", + "arn:aws:iam::396287094661:role/shoc-backend-staging", + } +) -# Files allowed to embed that ARN as a literal (default + example). Grant -# sites must use local.shoc_consumer_role_arn instead. +# Files allowed to embed those ARNs as literals (default + example). Grant +# sites must use local.shoc_consumer_role_arns instead. ALLOWED_LITERAL_FILES = {"variables.tf", "terraform.tfvars.example"} # Grant sites that must reference the local; dropping one half fails the pin. @@ -39,15 +45,16 @@ 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*\{(.*?)^\}', + r'variable\s+"shoc_consumer_role_arns"\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, +_DEFAULT_LIST_RE = re.compile(r"default\s*=\s*\[(.*?)\]", re.DOTALL) +_QUOTED_RE = re.compile(r'"([^"]+)"') +_TFVARS_LIST_RE = re.compile( + r"^shoc_consumer_role_arns\s*=\s*\[(.*?)\]", + re.DOTALL | re.MULTILINE, ) -_LOCAL_REF = "local.shoc_consumer_role_arn" +_LOCAL_REF = "local.shoc_consumer_role_arns" def _terraform_sources(): @@ -57,38 +64,45 @@ def _terraform_sources(): return paths -def test_variable_default_is_the_pinned_foreign_principal(): - """The Terraform default must equal the exact trusted ARN. +def _quoted_arns(block: str) -> frozenset[str]: + return frozenset(_QUOTED_RE.findall(block)) - Grant sites consume the variable via local.shoc_consumer_role_arn. Pinning + +def test_variable_default_is_the_pinned_foreign_principals(): + """The Terraform default must equal the exact trusted ARN set. + + Grant sites consume the variable via local.shoc_consumer_role_arns. Pinning the default restores the exact-principal invariant the former CDK scan - enforced: widening trust requires editing this default (and this test). + enforced: widening or shrinking 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 block, "variables.tf must declare variable shoc_consumer_role_arns" + default = _DEFAULT_LIST_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' + "variable shoc_consumer_role_arns must set default = [...] so the " + "exact principals are 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}" + arns = _quoted_arns(default.group(1)) + assert arns == ALLOWED_FOREIGN_PRINCIPALS, ( + f"shoc_consumer_role_arns default is {sorted(arns)!r}, expected " + f"{sorted(ALLOWED_FOREIGN_PRINCIPALS)!r}" ) -def test_tfvars_example_is_the_pinned_foreign_principal(): - match = _TFVARS_VALUE_RE.search(TFVARS_EXAMPLE.read_text()) +def test_tfvars_example_is_the_pinned_foreign_principals(): + match = _TFVARS_LIST_RE.search(TFVARS_EXAMPLE.read_text()) assert match, ( - "terraform.tfvars.example must set shoc_consumer_role_arn to the pinned ARN" + "terraform.tfvars.example must set shoc_consumer_role_arns to the pinned ARNs" ) - 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}" + arns = _quoted_arns(match.group(1)) + assert arns == ALLOWED_FOREIGN_PRINCIPALS, ( + f"terraform.tfvars.example shoc_consumer_role_arns is {sorted(arns)!r}, " + f"expected {sorted(ALLOWED_FOREIGN_PRINCIPALS)!r}" ) -def test_only_the_pinned_foreign_principal_appears_in_terraform_sources(): +def test_only_the_pinned_foreign_principals_appear_in_terraform_sources(): findings = [] for path in _terraform_sources(): for match in _IAM_ARN_RE.finditer(path.read_text()): @@ -100,23 +114,24 @@ def test_only_the_pinned_foreign_principal_appears_in_terraform_sources(): unexpected = [ (name, arn) for name, arn in findings - if arn != ALLOWED_FOREIGN_PRINCIPAL or name not in ALLOWED_LITERAL_FILES + if arn not in ALLOWED_FOREIGN_PRINCIPALS or name not in ALLOWED_LITERAL_FILES ] assert not unexpected, ( "Unexpected foreign IAM principal(s) under terraform/ — every " "cross-account trust addition must update this pin deliberately: " f"{unexpected}" ) - # Both default/example sites must still name the pinned role as a literal. + # Both default/example sites must still name the pinned roles as literals. assert {name for name, _ in findings} == ALLOWED_LITERAL_FILES + assert {arn for _, arn in findings} == ALLOWED_FOREIGN_PRINCIPALS -def test_grant_files_use_local_shoc_consumer_role_arn_only(): +def test_grant_files_use_local_shoc_consumer_role_arns_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 + local.shoc_consumer_role_arns 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: