From ea27635ef203c24d85657d175dfb0a12e180793d Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 16:31:34 -0400 Subject: [PATCH 1/2] feat(iac): add per-account HCP Terraform deploy substrate for prod and dev New stack seahaven-terraform-substrate (instances terraform-substrate-prod + terraform-substrate-dev): app.terraform.io OIDC provider and the shared boundary-gated guardrail policy seahaven-hcptf-iam-management that per-workspace Terraform apply roles attach at migration time. No roles are pre-provisioned (accumulator pattern, parallel to githubdeploy-*). Guardrail statements mirror seahaven-cfn-exec-iam-management byte-identically except DenySelfMutation, whose scope extends to hcptf-* alongside the GitHub-substrate principals. Explicit stack dependency on the same-account deploy-substrate stack (boundary ARN appears only in Condition strings, so CFN infers no edge). --- .github/workflows/deploy.yaml | 4 +- README.md | 78 +++++ bin/app.ts | 38 ++- lib/terraform-substrate-stack.ts | 42 +++ .../terraform-substrate.template.yaml | 266 ++++++++++++++++++ 5 files changed, 424 insertions(+), 4 deletions(-) create mode 100644 lib/terraform-substrate-stack.ts create mode 100644 lib/terraform-substrate/terraform-substrate.template.yaml diff --git a/.github/workflows/deploy.yaml b/.github/workflows/deploy.yaml index ab2bb9f..748e135 100644 --- a/.github/workflows/deploy.yaml +++ b/.github/workflows/deploy.yaml @@ -47,7 +47,7 @@ jobs: uses: Sea-Haven-Industries/.github/.github/workflows/cd-cdk.yaml@0170a57c0d99b542cfafd1f3e1d369c32643f486 # v1.0.2 with: node-version: "24" - stacks: "dev-baseline deploy-substrate-dev" + stacks: "dev-baseline deploy-substrate-dev terraform-substrate-dev" stack-name: "seahaven-dev-baseline" secrets: deploy-role-arn: ${{ secrets.AWS_DEPLOY_ROLE_ARN_DEV }} @@ -56,7 +56,7 @@ jobs: uses: Sea-Haven-Industries/.github/.github/workflows/cd-cdk.yaml@0170a57c0d99b542cfafd1f3e1d369c32643f486 # v1.0.2 with: node-version: "24" - stacks: "prod-baseline dynamodb-cmk-prod alarm-topic-prod deploy-substrate-prod" + stacks: "prod-baseline dynamodb-cmk-prod alarm-topic-prod deploy-substrate-prod terraform-substrate-prod" stack-name: "seahaven-prod-baseline" secrets: deploy-role-arn: ${{ secrets.AWS_DEPLOY_ROLE_ARN_PROD }} diff --git a/README.md b/README.md index 5a3a0cf..23a8536 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,84 @@ created with the boundary already attached. 4. Merge; the substrate deploys via CD. Per-repo deploy roles and app stacks follow the cross-account migration playbook from there. +### Terraform deploy substrate (per account) + +`lib/terraform-substrate-stack.ts` + `lib/terraform-substrate/terraform-substrate.template.yaml` +deploy `seahaven-terraform-substrate` into each member account that hosts +Terraform-managed workloads (currently seahaven-prod and seahaven-dev; never +mgmt — mgmt stays SAM until its stacks migrate out). It contains only the +shared account-level plumbing: + +- the `app.terraform.io` OIDC identity provider (audience + `aws.workload.identity`; Retain — it is the federation anchor for every + future `hcptf-*` role), +- the `seahaven-hcptf-iam-management` guardrail policy: the boundary-gated + IAM role lifecycle (conditioned on `seahaven-lambda-execution-boundary`, + owned by the deploy-substrate stack — hence the explicit stack dependency + in `bin/app.ts`) plus the `DenyBoundaryTampering` / `DenyBoundaryPolicyEdit` + / `DenySelfMutation` backstops, mirroring `seahaven-cfn-exec-iam-management`. + `DenySelfMutation` here covers `hcptf-*`, `github-cfn-execution-role` and + `githubdeploy-*` — the Terraform path can never mutate either substrate's + principals. + +Per-workspace roles (`hcptf-` apply + `hcptf--plan`) are +deliberately NOT pre-provisioned — they are appended to the template at each +stack's migration time so an account never carries trust for workspaces that +do not deploy to it. + +**HCP Terraform layout (org-level setup, console):** one org `seahaven` +(free tier: 500 managed resources, 1 concurrent run); one HCP **project per +AWS account** (`seahaven-prod`, `seahaven-dev`); one **workspace per stack** +(`-`, one state file = one blast radius). Default execution mode +Remote. Never use HCP's "Quick setup AWS dynamic credentials" button — it +writes the single `TFC_AWS_RUN_ROLE_ARN`, which collapses the plan/apply role +split this substrate exists to enforce. + +**Migration checklist (per stack, in order):** + +1. Create the workspace in the target account's HCP project + (`-`). Apply method **Manual**; automatic speculative plans on + if VCS-connected (CLI `terraform plan` runs are inherently speculative). +2. PR to this repo appending `hcptf--plan` (read-only, + ViewOnlyAccess-class, **no** IAM writes, **no** guardrail-policy attach) + and `hcptf-` (attaches `seahaven-hcptf-iam-management` + stack-scoped + service statements) to the substrate template. Trust: this account's + `app.terraform.io` provider; `StringEquals` on + `app.terraform.io:aud` = `aws.workload.identity` and on + `app.terraform.io:sub` = + `organization:seahaven:project:seahaven-:workspace::run_phase:plan` + (or `:apply`). Exact `StringEquals` only — never `StringLike`, never a + wildcarded `run_phase` (a speculative PR plan must never hold write + credentials). IAM roles = mandatory GPT-4.1 cross-review + + `/sh-security-review` on the diff. +3. After deploy, verify: both roles exist; `hcptf-` lists + `seahaven-hcptf-iam-management` in `list-attached-role-policies`; trust + subs match the live org/project/workspace names byte-for-byte; simulate + the apply role against a `hcptf-*` ARN (expect `explicitDeny` from + `DenySelfMutation`) and against a normal stack role name (expect + `allowed`). +4. Set **workspace-level** variables `TFC_AWS_PLAN_ROLE_ARN` + + `TFC_AWS_APPLY_ROLE_ARN` (category env) to the verified role ARNs, plus + `TFC_AWS_PROVIDER_AUTH=true`. Never project-scoped variable sets — the + trust is pinned per workspace, so a shared set breaks every other + workspace. +5. Auto-apply stays OFF until the stack is sealed. + +**Rollback (proven in mgmt 2026-07-30):** delete any `hcptf-*` roles first +(they reference the provider), then the stack. The provider and guardrail +policy are Retain — after a stack delete, remove the orphaned provider with +`aws iam delete-open-id-connect-provider` and the policy with +`aws iam delete-policy` once nothing references them. Workspaces with state +must be migrated or destroyed HCP-side first; an OIDC provider deletion +strands them mid-run, it does not clean them up. + +**Verification of record for the guardrail policy** is mechanical +reconciliation — tag-preserving YAML load of the template vs +`get-policy-version` readback, sorted `json.dumps` compare per statement — +same discipline as the deploy-substrate reconciliation (2026-07-27), not +header-reading. The managed-policy document budget is 6,144 characters; +measure before appending statements. + ### CloudTrail (audit finding C-1) | Resource | Logical ID | Notes | diff --git a/bin/app.ts b/bin/app.ts index 4b65c4c..af49987 100644 --- a/bin/app.ts +++ b/bin/app.ts @@ -7,6 +7,7 @@ import { BackupOffsiteStack } from "../lib/backup-offsite-stack"; import { BackupStack } from "../lib/backup-stack"; import { RegionalBaselineStack } from "../lib/regional-baseline-stack"; import { DeploySubstrateStack } from "../lib/deploy-substrate-stack"; +import { TerraformSubstrateStack } from "../lib/terraform-substrate-stack"; import { DynamoDbCmkStack } from "../lib/dynamodb-cmk-stack"; import { MemberBaselineStack } from "../lib/member-baseline-stack"; import { OrgGovernanceStack } from "../lib/org-governance-stack"; @@ -171,18 +172,51 @@ new MemberBaselineStack(app, "prod-baseline", { // seahaven-lambda-execution-boundary policy both returned NoSuchEntity in // 011934824531 AND 710827005802, so the named creates cannot collide with // out-of-band copies. -new DeploySubstrateStack(app, "deploy-substrate-prod", { +const deploySubstrateProd = new DeploySubstrateStack(app, "deploy-substrate-prod", { stackName: "seahaven-deploy-substrate", env: { account: PROD_ACCOUNT, region: "us-east-1" }, createOidcProvider: false, }); -new DeploySubstrateStack(app, "deploy-substrate-dev", { +const deploySubstrateDev = new DeploySubstrateStack(app, "deploy-substrate-dev", { stackName: "seahaven-deploy-substrate", env: { account: DEV_ACCOUNT, region: "us-east-1" }, createOidcProvider: false, }); +// ── Per-account HCP Terraform deploy substrate ─────────────────────────────── +// The Terraform analog of the GitHub Actions substrate above: app.terraform.io +// OIDC provider + the shared boundary-gated guardrail policy +// (seahaven-hcptf-iam-management) that per-workspace apply roles attach. +// Per-workspace hcptf-* roles are appended to the template at each stack's +// migration time, never here. prod/dev ONLY — mgmt stays SAM (Terraform POC +// decision 2026-07-30; the mgmt POC substrate was rolled back the same day). +// The guardrail policy names the seahaven-lambda-execution-boundary ARN only +// inside Condition strings, so CFN infers no creation edge — the explicit +// dependency below guarantees the deploy-substrate stack (which owns the +// boundary) lands first in any future account onboarding. First-create +// precondition verified 2026-07-30: no app.terraform.io provider and no +// hcptf-* roles in either account. +const terraformSubstrateProd = new TerraformSubstrateStack( + app, + "terraform-substrate-prod", + { + stackName: "seahaven-terraform-substrate", + env: { account: PROD_ACCOUNT, region: "us-east-1" }, + }, +); +terraformSubstrateProd.addStackDependency(deploySubstrateProd); + +const terraformSubstrateDev = new TerraformSubstrateStack( + app, + "terraform-substrate-dev", + { + stackName: "seahaven-terraform-substrate", + env: { account: DEV_ACCOUNT, region: "us-east-1" }, + }, +); +terraformSubstrateDev.addStackDependency(deploySubstrateDev); + // ── Shared DynamoDB CMK (INFRA-95 / M-3) ───────────────────────────────────── // Dedicated, standalone stack so the customer-managed key for sensitive // finance/PII DynamoDB tables is an independent shared dependency for the owning diff --git a/lib/terraform-substrate-stack.ts b/lib/terraform-substrate-stack.ts new file mode 100644 index 0000000..f0d131c --- /dev/null +++ b/lib/terraform-substrate-stack.ts @@ -0,0 +1,42 @@ +import * as cdk from "aws-cdk-lib"; +import * as cfninc from "aws-cdk-lib/cloudformation-include"; +import * as path from "path"; +import { Construct } from "constructs"; + +/** + * Per-account HCP Terraform deploy substrate: the shared account-level + * resources every Terraform workspace pipeline needs - + * - app.terraform.io OIDC identity provider (always created; Phase-0 + * checks confirmed no account has one), and + * - `seahaven-hcptf-iam-management`, the shared boundary-gated IAM + * guardrail policy every per-workspace APPLY role attaches. + * + * Deliberately NOT here: per-workspace hcptf- / hcptf--plan + * roles. Those are appended to the template at each stack's migration time + * (accumulator pattern, parallel to per-repo githubdeploy-* roles) so an + * account never accumulates trust for workspaces that do not deploy to it. + * + * The IAM guardrail statements mirror seahaven-cfn-exec-iam-management in + * lib/deploy-substrate/deploy-substrate.template.yaml - see the provenance + * header in lib/terraform-substrate/terraform-substrate.template.yaml for + * the reconciliation rule and the boundary-ARN coupling to the + * seahaven-deploy-substrate stack (bin/app.ts carries the explicit + * addStackDependency; the ARN reference alone creates no CFN edge). + */ +export class TerraformSubstrateStack extends cdk.Stack { + constructor(scope: Construct, id: string, props?: cdk.StackProps) { + super(scope, id, props); + + new cfninc.CfnInclude(this, "Substrate", { + templateFile: path.join( + __dirname, + "terraform-substrate", + "terraform-substrate.template.yaml", + ), + }); + + cdk.Tags.of(this).add("Project", "account-baseline"); + cdk.Tags.of(this).add("Owner", "adam@seahavenind.com"); + cdk.Tags.of(this).add("ManagedBy", "cdk"); + } +} diff --git a/lib/terraform-substrate/terraform-substrate.template.yaml b/lib/terraform-substrate/terraform-substrate.template.yaml new file mode 100644 index 0000000..61cbe9b --- /dev/null +++ b/lib/terraform-substrate/terraform-substrate.template.yaml @@ -0,0 +1,266 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: >- + Per-account HCP Terraform deploy substrate for Sea Haven Industries: + the app.terraform.io OIDC identity provider and the shared boundary-gated + IAM guardrail policy that every per-workspace Terraform APPLY role attaches. + Per-workspace hcptf-* roles are NOT pre-provisioned — they are appended to + this template at each stack's migration time. + +# PROVENANCE / DESIGN SOURCE +# Authored fresh 2026-07-30 (the mgmt Terraform POC's CLI-created provider and +# hcptf-* roles were rolled back the same day, so there is no deployed source +# to vendor). The IAM statement set in HcptfIamManagementPolicy MIRRORS the +# reviewed seahaven-cfn-exec-iam-management pattern in +# lib/deploy-substrate/deploy-substrate.template.yaml (boundary-gated +# CreateRole/AttachRolePolicy/PutRolePolicy/PutRolePermissionsBoundary + +# DenyBoundaryTampering / DenyBoundaryPolicyEdit / DenySelfMutation). If that +# pattern changes in either file, reconcile BOTH in the same piece of work and +# verify mechanically (tag-preserving YAML load + sorted JSON compare per +# statement), never by reading headers. +# +# COUPLING: the boundary ARN referenced in the Conditions below is +# seahaven-lambda-execution-boundary, created by the seahaven-deploy-substrate +# stack in the same account. The reference is a literal !Sub string inside +# Condition values, so CloudFormation infers NO ordering edge from it — +# bin/app.ts carries an explicit addStackDependency on the same-account +# deploy-substrate stack instead. The coupling is by NAME: if the boundary +# policy is ever renamed or replaced, every Condition below (and the +# deploy-substrate copy) must change in the same piece of work. INFRA-186 +# (per-workload boundary scoping) changes the boundary's CONTENT, not its ARN, +# and does not touch this file. +# +# SIZE BUDGET: an attached managed policy document is capped at 6,144 +# characters (whitespace excluded). The statement set below is ~2.5 KB. +# Measure before adding statements — len(json.dumps(doc,separators=(',',':'))) +# on the synthesized PolicyDocument — the same wall the role INLINE limit +# (10,240 bytes) put the first deploy-substrate deploy into on 2026-07-27. +# +# PER-WORKSPACE ROLE ACCUMULATOR +# At each stack's migration, a PR appends to this template: +# - hcptf--plan: read-only (ViewOnlyAccess-class), trust sub +# organization:seahaven:project:seahaven-:workspace::run_phase:plan +# - hcptf-: apply role attaching HcptfIamManagementPolicy plus +# stack-scoped service statements, trust sub ...run_phase:apply +# All subs are exact StringEquals (never StringLike, never a wildcarded +# run_phase — a speculative PR plan must never hold write credentials); +# audience is aws.workload.identity. IAM role additions here are a mandatory +# GPT-4.1 cross-review + /sh-security-review trigger. See the README +# "Terraform substrate" section for the full migration checklist and the +# rollback runbook. +# +# This template is deployed via lib/terraform-substrate-stack.ts +# (cloudformation-include) as stack seahaven-terraform-substrate, once per +# member account that hosts Terraform-managed workloads (currently +# seahaven-prod 011934824531 and seahaven-dev 710827005802; NEVER mgmt — +# mgmt stays SAM until its stacks migrate out). + +Resources: + + # --------------------------------------------------------------------------- + # HCP Terraform OIDC provider + # + # Always created: Phase-0 checks (2026-07-30) confirmed neither prod nor dev + # has an app.terraform.io provider (the mgmt POC's copy was deleted in the + # same-day rollback and never existed in the member accounts). An account + # holds exactly ONE provider per URL. + # --------------------------------------------------------------------------- + TerraformCloudOIDCProvider: + Type: AWS::IAM::OIDCProvider + Properties: + Url: https://app.terraform.io + ClientIdList: + # Default audience of HCP Terraform dynamic provider credentials + # (TFC_AWS_WORKLOAD_IDENTITY_AUDIENCE). Trust policies pin this via + # StringEquals on app.terraform.io:aud. + - aws.workload.identity + ThumbprintList: + # AWS ignores thumbprints for issuers signed by a trusted root CA + # (app.terraform.io qualifies) and secures trust via the CA bundle; + # the property is populated because CloudFormation requires a value. + # This is the thumbprint HashiCorp's own AWS setup documentation uses. + - 9e99a48a9960b14926bb7f3b02e22da2b0ab7280 + # Every future hcptf-* role trusts this provider. Retain so deleting the + # stack can never delete the account's Terraform federation anchor out + # from under live workspaces. + DeletionPolicy: Retain + UpdateReplacePolicy: Retain + + # --------------------------------------------------------------------------- + # Shared boundary-gated IAM guardrail policy (attached managed policy) + # + # Attached by every per-workspace Terraform APPLY role (hcptf-); + # NEVER by plan roles (hcptf--plan are read-only and hold no IAM + # writes at all). Defined once here so all apply roles carry the identical + # reviewed escalation control instead of per-role copies that can drift. + # + # PRIMARY ESCALATION CONTROL (same design as INFRA-97 on the SAM side): + # every iam:CreateRole / AttachRolePolicy / PutRolePolicy is conditioned on + # the target role carrying seahaven-lambda-execution-boundary, so a role + # created by a Terraform apply can never exceed the boundary ceiling. The + # POC security review confirmed the unconditioned alternative is critical: + # iam:PutRolePolicy on Lambda exec roles + lambda:UpdateFunctionCode reads + # every secret in the account. + # --------------------------------------------------------------------------- + HcptfIamManagementPolicy: + Type: AWS::IAM::ManagedPolicy + Properties: + # Fixed name: future hcptf-* roles reference it by ARN, and a rename + # would detach-and-replace mid-update. Treat a rename as a coordinated + # migration, not an edit. + ManagedPolicyName: seahaven-hcptf-iam-management + Description: >- + Boundary-gated IAM role lifecycle for per-workspace Terraform apply + roles (hcptf-*), plus the explicit Deny backstops that keep the + permissions boundary from being detached or rewritten and the deploy + substrates' own principals from being mutated. Mirrors + seahaven-cfn-exec-iam-management; reconcile changes across both. + PolicyDocument: + Version: "2012-10-17" + Statement: + # Create role — MUST attach boundary + - Sid: IAMCreateRoleWithBoundary + Effect: Allow + Action: + - iam:CreateRole + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + Condition: + StringEquals: + "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" + + # Attach managed policies — MUST have boundary already on role + - Sid: IAMAttachPolicyWithBoundary + Effect: Allow + Action: + - iam:AttachRolePolicy + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + Condition: + StringEquals: + "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" + + # Put inline policy — MUST have boundary already on role + - Sid: IAMPutRolePolicyWithBoundary + Effect: Allow + Action: + - iam:PutRolePolicy + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + Condition: + StringEquals: + "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" + + # Boundary management — SET only, never DELETE. For a delete, the + # iam:PermissionsBoundary condition key resolves to the boundary + # CURRENTLY on the target role, so a StringEquals grant would match + # exactly the roles the gate protects and self-defeat it (verified + # live against the mgmt SAM copy 2026-07-27). Terraform never needs + # the delete: it SETS the boundary on roles it creates, and destroy + # calls DeleteRole. + - Sid: IAMPutPermissionsBoundary + Effect: Allow + Action: + - iam:PutRolePermissionsBoundary + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + Condition: + StringEquals: + "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" + + # Explicit Deny backstop (AWS's NoBoundaryPolicyEdit/NoBoundaryDelete + # delegation pattern). A Deny is required, not merely omitting the + # Allow — any future Allow added to an apply role silently reopens + # the escalation otherwise. + - Sid: DenyBoundaryTampering + Effect: Deny + Action: + - iam:DeleteRolePermissionsBoundary + - iam:DeleteUserPermissionsBoundary + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + - !Sub "arn:aws:iam::${AWS::AccountId}:user/*" + + # Whole seahaven-* policy family: this policy carries the Denies, so + # it is a higher-value target than the boundary it protects. Safe to + # scope broadly — no Terraform stack manages a seahaven-* managed + # policy, and apply roles hold no iam:CreatePolicy. + - Sid: DenyBoundaryPolicyEdit + Effect: Deny + Action: + - iam:CreatePolicyVersion + - iam:SetDefaultPolicyVersion + - iam:DeletePolicyVersion + - iam:DeletePolicy + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-*" + + # Self-protection for BOTH deploy substrates' principals. Without + # this the control is one API call from being undone — + # IAMRoleReadAndDelete below grants iam:DetachRolePolicy on + # Resource "*" unconditioned, so an apply role could detach this + # very policy from itself. Scope covers the Terraform substrate's + # own roles (hcptf-*) AND the GitHub Actions substrate's + # (github-cfn-execution-role, githubdeploy-*): a Terraform apply + # never legitimately manages any of them — hcptf-* roles are + # managed by THIS stack via the CDK bootstrap execution role, the + # GitHub-side roles by their own substrate/onboarding — so the Deny + # costs nothing operationally and closes the same + # UpdateAssumeRolePolicy-on-* repoint risk the SAM-side review + # flagged, for every substrate principal reachable from this path. + - Sid: DenySelfMutation + Effect: Deny + Action: + - iam:AttachRolePolicy + - iam:DeleteRole + - iam:DeleteRolePolicy + - iam:DeleteRolePermissionsBoundary + - iam:DetachRolePolicy + - iam:PutRolePolicy + - iam:PutRolePermissionsBoundary + - iam:UpdateAssumeRolePolicy + - iam:UpdateRole + - iam:UpdateRoleDescription + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/hcptf-*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/github-cfn-execution-role" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/githubdeploy-*" + + # Read / tag / delete role and policy — no boundary condition needed + # (delete cannot be boundary-conditioned, see IAMPutPermissionsBoundary; + # DenySelfMutation above is the backstop). Parity with the SAM copy. + - Sid: IAMRoleReadAndDelete + Effect: Allow + Action: + - iam:DeleteRole + - iam:DeleteRolePolicy + - iam:DetachRolePolicy + - iam:GetRole + - iam:GetRolePolicy + - iam:ListAttachedRolePolicies + - iam:ListRolePolicies + - iam:ListRoles + - iam:TagRole + - iam:UntagRole + - iam:UpdateRole + - iam:UpdateRoleDescription + - iam:UpdateAssumeRolePolicy + - iam:GetPolicy + - iam:GetPolicyVersion + - iam:ListPolicies + - iam:ListPolicyVersions + Resource: "*" + + # PassRole — Terraform passes stack-created execution roles to the + # Lambda service. Other target services (e.g. scheduler.amazonaws.com, + # apigateway.amazonaws.com) are NOT granted here: a stack that needs + # one adds a scoped PassRole statement on its own apply role at + # migration time. + - Sid: IAMPassRole + Effect: Allow + Action: + - iam:PassRole + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + Condition: + StringEquals: + "iam:PassedToService": "lambda.amazonaws.com" From 981960433f0b47cb8114116239f7466978bb395c Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 16:55:45 -0400 Subject: [PATCH 2/2] fix(iam): scope Terraform guardrail role writes to a Terraform-owned path Security review (6 detectors + proof-or-kill verifier) confirmed 1 critical and 1 high in the first revision, both inherited by mirroring the SAM copy's Resource "*" role grants: - C1 (critical): iam:UpdateAssumeRolePolicy on "*" with DenySelfMutation covering only three name patterns lets the principal repoint the AdministratorAccess CDK bootstrap role's trust policy to an external account. - C2 (high): the SAM justification for role/* (SAM auto-roles land at path / with no settable RolePath) does not transfer -- Terraform's aws_iam_role supports path. Fixes, closing the class at the root rather than by denylist: - All role writes, boundary sets and PassRole confined to role/tf-managed/*; reads split into a separate statement that keeps Resource "*". - DenySelfMutation extended to cdk-hnb659fds-*, OrganizationAccountAccessRole and seahaven-* as defense in depth. - OIDC provider made conditional (CreateOIDCProvider), mirroring the sibling substrate, so a first-create rollback is recoverable rather than wedging the stack in ROLLBACK_COMPLETE against a Retained orphan. - README corrected: the guardrail policy is NOT Retain (only the provider is), so the Deny backstops do not survive a stack delete. checkov CKV_AWS_109 no longer fires on this template, so no suppression is needed. The template header records every divergence from the SAM copy. --- README.md | 79 +++++++-- lib/terraform-substrate-stack.ts | 40 ++++- .../terraform-substrate.template.yaml | 161 ++++++++++++++---- 3 files changed, 228 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 23a8536..d8f168c 100644 --- a/README.md +++ b/README.md @@ -202,10 +202,29 @@ shared account-level plumbing: IAM role lifecycle (conditioned on `seahaven-lambda-execution-boundary`, owned by the deploy-substrate stack — hence the explicit stack dependency in `bin/app.ts`) plus the `DenyBoundaryTampering` / `DenyBoundaryPolicyEdit` - / `DenySelfMutation` backstops, mirroring `seahaven-cfn-exec-iam-management`. - `DenySelfMutation` here covers `hcptf-*`, `github-cfn-execution-role` and - `githubdeploy-*` — the Terraform path can never mutate either substrate's - principals. + / `DenySelfMutation` backstops. + +**This policy derives from `seahaven-cfn-exec-iam-management` but is +deliberately stricter — it is not a mirror.** The 2026-07-30 security review +confirmed the SAM copy's `Resource: "*"` role grants as a critical escalation +primitive (`iam:UpdateAssumeRolePolicy` on `*` repoints the AdministratorAccess +CDK bootstrap role's trust policy to an external account), and its justification +for the wildcard — SAM auto-generates execution roles at path `/` with no +settable `RolePath` — does not transfer, because Terraform's `aws_iam_role` +supports `path`. So here: + +- every role **write** (create, delete, detach, `UpdateAssumeRolePolicy`, + boundary set) and `iam:PassRole` is confined to the Terraform-owned path + `role/tf-managed/*`; reads stay on `*` for data sources, +- **Terraform configs must set `path = "/tf-managed/"` on every + `aws_iam_role`** — a role created anywhere else is denied, +- `DenySelfMutation` additionally covers `cdk-hnb659fds-*`, + `OrganizationAccountAccessRole` and `seahaven-*` (detective-control roles, + which no prod/nonprod SCP shields from `iam:DeleteRole`). + +Do not "reconcile" the two files by copying statements between them. The +durable org-level fix for the same class is extending the existing +`ProtectPrivilegedRoles` SCP (currently security-OU only) to prod and nonprod. Per-workspace roles (`hcptf-` apply + `hcptf--plan`) are deliberately NOT pre-provisioned — they are appended to the template at each @@ -225,8 +244,12 @@ split this substrate exists to enforce. 1. Create the workspace in the target account's HCP project (`-`). Apply method **Manual**; automatic speculative plans on if VCS-connected (CLI `terraform plan` runs are inherently speculative). -2. PR to this repo appending `hcptf--plan` (read-only, - ViewOnlyAccess-class, **no** IAM writes, **no** guardrail-policy attach) +2. PR to this repo appending `hcptf--plan` (read-only — + `arn:aws:iam::aws:policy/job-function/ViewOnlyAccess`, never + `ReadOnlyAccess`, which grants `secretsmanager:GetSecretValue`, + `s3:GetObject` and `kms:Decrypt` and would let any PR-triggered speculative + plan render secret values into HCP run output; **no** IAM writes, **no** + guardrail-policy attach) and `hcptf-` (attaches `seahaven-hcptf-iam-management` + stack-scoped service statements) to the substrate template. Trust: this account's `app.terraform.io` provider; `StringEquals` on @@ -250,13 +273,43 @@ split this substrate exists to enforce. workspace. 5. Auto-apply stays OFF until the stack is sealed. -**Rollback (proven in mgmt 2026-07-30):** delete any `hcptf-*` roles first -(they reference the provider), then the stack. The provider and guardrail -policy are Retain — after a stack delete, remove the orphaned provider with -`aws iam delete-open-id-connect-provider` and the policy with -`aws iam delete-policy` once nothing references them. Workspaces with state -must be migrated or destroyed HCP-side first; an OIDC provider deletion -strands them mid-run, it does not clean them up. +**HCP-side authority is AWS authority.** AWS exposes only `aud`, `sub` and +`amr` as trust-policy condition keys for a generic OIDC provider — HCP's +immutable `terraform_workspace_id` / `terraform_project_id` claims are *not* +usable in an IAM condition (AWS's provider-specific claim validation covers +Google, GitHub, CircleCI and OCI only). The `sub` pin therefore rests on HCP +display names, so whoever can create, rename, move or delete a workspace in the +`seahaven-prod` project effectively holds prod deploy authority. Restrict that +HCP team permission to the same people, and when a workspace is retired, delete +its `hcptf-*` roles in the same change so a reused name cannot inherit them. + +**Terraform state is secret-bearing.** HCP-hosted state records sensitive +attributes in full and lives outside the AWS accounts, readable by any HCP +principal with workspace read. Per the handbook's secrets-and-config rule, +secrets stay in Secrets Manager / SSM and are referenced by ARN: do not manage +secret *values* in Terraform (create the secret shell, populate out of band or +via write-only/ephemeral arguments) so no value enters state. + +**Rollback (proven in mgmt 2026-07-30):** delete any `hcptf-*` roles first — +they reference the provider, and while any of them still attaches the guardrail +policy the stack delete cannot remove it. Then delete the stack. Only the +**provider** is `Retain`: it survives as an orphan and is removed with +`aws iam delete-open-id-connect-provider`. The **guardrail policy is deleted +with the stack** — do not expect it to persist, and note that every +`DenySelfMutation` / `DenyBoundaryTampering` backstop goes with it, so an +`hcptf-*` role recreated out of band afterwards is *not* gated. Workspaces +holding state must be migrated or destroyed HCP-side first; deleting the OIDC +provider strands them mid-run rather than cleaning them up. + +**First-create rollback trap.** The provider is `Retain`, so if any other +resource in this stack fails on first create, CloudFormation rolls back, the +provider survives untracked, and the stack lands in `ROLLBACK_COMPLETE` — which +cannot be updated, and cannot be recreated because an account holds exactly one +provider per URL. Recovery: delete the stack, then either remove the orphaned +provider with the command above before retrying, or redeploy with +`createOidcProvider: false`. Note `cd-cdk`'s pre-flight and health check probe +only the job's single `stack-name` input (the account baseline), so a wedged +substrate stack does not show up there — check it directly. **Verification of record for the guardrail policy** is mechanical reconciliation — tag-preserving YAML load of the template vs diff --git a/lib/terraform-substrate-stack.ts b/lib/terraform-substrate-stack.ts index f0d131c..b7a1e58 100644 --- a/lib/terraform-substrate-stack.ts +++ b/lib/terraform-substrate-stack.ts @@ -3,11 +3,26 @@ import * as cfninc from "aws-cdk-lib/cloudformation-include"; import * as path from "path"; import { Construct } from "constructs"; +export interface TerraformSubstrateStackProps extends cdk.StackProps { + /** + * Create the app.terraform.io OIDC identity provider in this account. + * Defaults to true - Phase-0 checks (2026-07-30) confirmed neither prod nor + * dev has one. Set false for an account that already has the provider: an + * account holds exactly ONE provider per URL, so a duplicate create fails. + * + * This flag also makes a first-create rollback recoverable. The provider is + * Retain, so if any other resource in this stack fails on FIRST create the + * provider survives as an orphan while the stack lands in ROLLBACK_COMPLETE + * (which cannot be updated). Recovery is to delete the stack and either + * remove the orphaned provider or redeploy with this false. + */ + createOidcProvider?: boolean; +} + /** * Per-account HCP Terraform deploy substrate: the shared account-level * resources every Terraform workspace pipeline needs - - * - app.terraform.io OIDC identity provider (always created; Phase-0 - * checks confirmed no account has one), and + * - app.terraform.io OIDC identity provider (conditional, see props), and * - `seahaven-hcptf-iam-management`, the shared boundary-gated IAM * guardrail policy every per-workspace APPLY role attaches. * @@ -16,15 +31,23 @@ import { Construct } from "constructs"; * (accumulator pattern, parallel to per-repo githubdeploy-* roles) so an * account never accumulates trust for workspaces that do not deploy to it. * - * The IAM guardrail statements mirror seahaven-cfn-exec-iam-management in - * lib/deploy-substrate/deploy-substrate.template.yaml - see the provenance - * header in lib/terraform-substrate/terraform-substrate.template.yaml for - * the reconciliation rule and the boundary-ARN coupling to the + * The IAM guardrail statements DERIVE FROM seahaven-cfn-exec-iam-management in + * lib/deploy-substrate/deploy-substrate.template.yaml but are deliberately + * STRICTER (role writes and PassRole confined to the tf-managed path, wider + * DenySelfMutation) - the SAM copy's Resource "*" grants were confirmed a + * critical escalation primitive by the 2026-07-30 security review, and its + * justification for them does not transfer to Terraform. See the provenance + * header in lib/terraform-substrate/terraform-substrate.template.yaml for the + * full divergence list, and for the boundary-ARN coupling to the * seahaven-deploy-substrate stack (bin/app.ts carries the explicit * addStackDependency; the ARN reference alone creates no CFN edge). */ export class TerraformSubstrateStack extends cdk.Stack { - constructor(scope: Construct, id: string, props?: cdk.StackProps) { + constructor( + scope: Construct, + id: string, + props?: TerraformSubstrateStackProps, + ) { super(scope, id, props); new cfninc.CfnInclude(this, "Substrate", { @@ -33,6 +56,9 @@ export class TerraformSubstrateStack extends cdk.Stack { "terraform-substrate", "terraform-substrate.template.yaml", ), + parameters: { + CreateOIDCProvider: props?.createOidcProvider === false ? "false" : "true", + }, }); cdk.Tags.of(this).add("Project", "account-baseline"); diff --git a/lib/terraform-substrate/terraform-substrate.template.yaml b/lib/terraform-substrate/terraform-substrate.template.yaml index 61cbe9b..538f6da 100644 --- a/lib/terraform-substrate/terraform-substrate.template.yaml +++ b/lib/terraform-substrate/terraform-substrate.template.yaml @@ -9,14 +9,53 @@ Description: >- # PROVENANCE / DESIGN SOURCE # Authored fresh 2026-07-30 (the mgmt Terraform POC's CLI-created provider and # hcptf-* roles were rolled back the same day, so there is no deployed source -# to vendor). The IAM statement set in HcptfIamManagementPolicy MIRRORS the -# reviewed seahaven-cfn-exec-iam-management pattern in +# to vendor). HcptfIamManagementPolicy DERIVES FROM the reviewed +# seahaven-cfn-exec-iam-management pattern in # lib/deploy-substrate/deploy-substrate.template.yaml (boundary-gated # CreateRole/AttachRolePolicy/PutRolePolicy/PutRolePermissionsBoundary + -# DenyBoundaryTampering / DenyBoundaryPolicyEdit / DenySelfMutation). If that -# pattern changes in either file, reconcile BOTH in the same piece of work and -# verify mechanically (tag-preserving YAML load + sorted JSON compare per -# statement), never by reading headers. +# DenyBoundaryTampering / DenyBoundaryPolicyEdit / DenySelfMutation) but is +# DELIBERATELY STRICTER — it is NOT a byte-identical mirror. Do not "reconcile" +# the two by copying this file's statements back, or vice versa; the divergences +# below are load-bearing and were required by the 2026-07-30 security review +# (findings C1-C5, one confirmed critical + one high): +# +# 1. ROLE PATH SCOPING (review finding C2). The SAM copy's Resource +# `role/*` on the boundary-gated statements is justified there by SAM +# auto-generating execution roles at path / with no settable RolePath — +# a path condition would break every SAM deploy. THAT RATIONALE DOES NOT +# TRANSFER: Terraform's aws_iam_role supports `path` and `name_prefix`. +# So every role-WRITE statement here is scoped to the Terraform-owned path +# `role/tf-managed/*`. Terraform configs MUST set path = "/tf-managed/" on +# every role they create; a role created anywhere else is denied. The path +# is deliberately NOT `hcptf-*`, which would collide with the substrate's +# own hcptf-* apply/plan roles under DenySelfMutation's wildcard. +# 2. READ AND WRITE SPLIT (review findings C1, C3, C4). The SAM copy's +# IAMRoleReadAndDelete grants iam:UpdateAssumeRolePolicy / DeleteRole / +# DetachRolePolicy / DeleteRolePolicy / UpdateRole on Resource "*" +# unconditioned — a confirmed privilege-escalation primitive (repoint the +# AdministratorAccess CDK bootstrap role's trust policy, then assume it +# cross-account) that DenySelfMutation's three name patterns do not cover. +# Here those actions are split: reads stay on "*" (Terraform data sources +# need them), every destructive/mutating action is confined to +# `role/tf-managed/*`. This closes the escalation at the root instead of +# chasing it with a denylist. +# 3. PASSROLE SCOPING (review finding C5). The SAM copy passes any role to +# Lambda (its comment claims SAM-role scoping the Resource does not +# express). Here PassRole is confined to `role/tf-managed/*`, so one +# workspace cannot attach another workspace's execution role to a function +# it controls — that path performs no IAM write and would otherwise evade +# every boundary gate and Deny in this document. +# 4. DENYSELFMUTATION SCOPE. Extended beyond the substrate's own principals to +# cdk-hnb659fds-* (AdministratorAccess bootstrap roles), +# OrganizationAccountAccessRole, and seahaven-* (detective-control roles +# such as the Config recorder role, which no SCP on prod/nonprod protects +# from iam:DeleteRole). Defense in depth behind the path scoping above. +# +# The SAM copy retains its adjudicated accepted risks because SAM's constraints +# are real; this file has no such excuse. KNOWN OPEN ITEM (pre-existing, not +# introduced here): the org's ProtectPrivilegedRoles SCP encodes exactly the +# protection in (4) but is attached ONLY to the security OU — extending it to +# prod/nonprod is the durable org-level fix and is tracked separately. # # COUPLING: the boundary ARN referenced in the Conditions below is # seahaven-lambda-execution-boundary, created by the seahaven-deploy-substrate @@ -54,18 +93,40 @@ Description: >- # seahaven-prod 011934824531 and seahaven-dev 710827005802; NEVER mgmt — # mgmt stays SAM until its stacks migrate out). +Parameters: + CreateOIDCProvider: + Type: String + Default: "true" + AllowedValues: ["true", "false"] + Description: >- + Set to false if the app.terraform.io OIDC provider already exists in this + account. An account holds exactly ONE provider per URL, so an unconditional + create collides. Because the provider is Retain, a FIRST-create rollback + (caused by any other resource in this stack failing) leaves the provider + behind as an orphan and the stack in ROLLBACK_COMPLETE — which cannot be + updated. Recovery: delete the stack, then either + `aws iam delete-open-id-connect-provider --open-id-connect-provider-arn + arn:aws:iam:::oidc-provider/app.terraform.io` before retrying, or + redeploy with this parameter false. Same idempotency affordance the sibling + deploy-substrate template carries for the GitHub provider. + +Conditions: + ShouldCreateOIDCProvider: !Equals [!Ref CreateOIDCProvider, "true"] + Resources: # --------------------------------------------------------------------------- # HCP Terraform OIDC provider # - # Always created: Phase-0 checks (2026-07-30) confirmed neither prod nor dev - # has an app.terraform.io provider (the mgmt POC's copy was deleted in the + # Created by default: Phase-0 checks (2026-07-30) confirmed neither prod nor + # dev has an app.terraform.io provider (the mgmt POC's copy was deleted in the # same-day rollback and never existed in the member accounts). An account - # holds exactly ONE provider per URL. + # holds exactly ONE provider per URL — see the parameter above for the + # first-create rollback trap this condition exists to make recoverable. # --------------------------------------------------------------------------- TerraformCloudOIDCProvider: Type: AWS::IAM::OIDCProvider + Condition: ShouldCreateOIDCProvider Properties: Url: https://app.terraform.io ClientIdList: @@ -117,13 +178,16 @@ Resources: PolicyDocument: Version: "2012-10-17" Statement: - # Create role — MUST attach boundary + # Create role — MUST attach boundary AND land on the Terraform-owned + # path. Two independent gates: the boundary caps what the role can do, + # the path caps which roles this policy can touch at all. Terraform + # configs set path = "/tf-managed/" on every aws_iam_role. - Sid: IAMCreateRoleWithBoundary Effect: Allow Action: - iam:CreateRole Resource: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/tf-managed/*" Condition: StringEquals: "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" @@ -134,7 +198,7 @@ Resources: Action: - iam:AttachRolePolicy Resource: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/tf-managed/*" Condition: StringEquals: "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" @@ -145,7 +209,7 @@ Resources: Action: - iam:PutRolePolicy Resource: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/tf-managed/*" Condition: StringEquals: "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" @@ -157,12 +221,18 @@ Resources: # live against the mgmt SAM copy 2026-07-27). Terraform never needs # the delete: it SETS the boundary on roles it creates, and destroy # calls DeleteRole. + # Path-scoped as well as boundary-pinned: the condition constrains WHICH + # boundary may be set, not WHICH role receives it. Unscoped (as in the + # SAM copy) this is a one-way denial-of-service — applying the Lambda + # runtime boundary to the CDK bootstrap execution role collapses its + # permissions, and DenyBoundaryTampering below then blocks removal by + # this same principal (2026-07-30 review finding C3). - Sid: IAMPutPermissionsBoundary Effect: Allow Action: - iam:PutRolePermissionsBoundary Resource: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/tf-managed/*" Condition: StringEquals: "iam:PermissionsBoundary": !Sub "arn:aws:iam::${AWS::AccountId}:policy/seahaven-lambda-execution-boundary" @@ -224,43 +294,70 @@ Resources: - !Sub "arn:aws:iam::${AWS::AccountId}:role/hcptf-*" - !Sub "arn:aws:iam::${AWS::AccountId}:role/github-cfn-execution-role" - !Sub "arn:aws:iam::${AWS::AccountId}:role/githubdeploy-*" + # Extended beyond the SAM copy's three patterns (2026-07-30 review + # findings C1/C3/C4). cdk-hnb659fds-* carries AdministratorAccess + # and deploys this very stack; OrganizationAccountAccessRole is the + # org break-glass path; seahaven-* covers detective-control roles + # (e.g. the Config recorder role) that the protect-security-baseline + # SCP does NOT shield from iam:DeleteRole. Defense in depth — the + # path scoping on the write statements is the primary control. + - !Sub "arn:aws:iam::${AWS::AccountId}:role/cdk-hnb659fds-*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/OrganizationAccountAccessRole" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/seahaven-*" - # Read / tag / delete role and policy — no boundary condition needed - # (delete cannot be boundary-conditioned, see IAMPutPermissionsBoundary; - # DenySelfMutation above is the backstop). Parity with the SAM copy. - - Sid: IAMRoleReadAndDelete + # READ-ONLY on every role/policy in the account. Terraform data sources + # and refresh legitimately need to read arbitrary roles; none of these + # actions can modify anything, so Resource "*" is safe here. + - Sid: IAMReadOnly Effect: Allow Action: - - iam:DeleteRole - - iam:DeleteRolePolicy - - iam:DetachRolePolicy - iam:GetRole - iam:GetRolePolicy - iam:ListAttachedRolePolicies - iam:ListRolePolicies - iam:ListRoles - - iam:TagRole - - iam:UntagRole - - iam:UpdateRole - - iam:UpdateRoleDescription - - iam:UpdateAssumeRolePolicy - iam:GetPolicy - iam:GetPolicyVersion - iam:ListPolicies - iam:ListPolicyVersions Resource: "*" - # PassRole — Terraform passes stack-created execution roles to the - # Lambda service. Other target services (e.g. scheduler.amazonaws.com, - # apigateway.amazonaws.com) are NOT granted here: a stack that needs - # one adds a scoped PassRole statement on its own apply role at - # migration time. + # DESTRUCTIVE / MUTATING role actions — confined to the Terraform-owned + # path. The SAM copy grants these on Resource "*" unconditioned, which + # the 2026-07-30 review confirmed as a critical escalation primitive + # (finding C1): iam:UpdateAssumeRolePolicy on "*" lets the principal + # repoint the AdministratorAccess CDK bootstrap role's trust policy to + # an external account and assume it. Path scoping closes that at the + # root rather than enumerating protected names. + - Sid: IAMRoleWriteScoped + Effect: Allow + Action: + - iam:DeleteRole + - iam:DeleteRolePolicy + - iam:DetachRolePolicy + - iam:TagRole + - iam:UntagRole + - iam:UpdateRole + - iam:UpdateRoleDescription + - iam:UpdateAssumeRolePolicy + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/tf-managed/*" + + # PassRole — Terraform passes the execution roles it created (which are + # on the tf-managed path, boundary-gated above) to the Lambda service. + # Path-scoped, not role/*: unscoped, one workspace's apply role could + # attach ANOTHER workspace's or a SAM stack's execution role to a + # function it controls and run arbitrary code as that identity — a path + # that performs no IAM write and so evades every boundary gate and Deny + # in this document (2026-07-30 review finding C5). Other target services + # (scheduler, apigateway, ...) are NOT granted: a stack that needs one + # adds a scoped PassRole statement to its own apply role at migration. - Sid: IAMPassRole Effect: Allow Action: - iam:PassRole Resource: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/*" + - !Sub "arn:aws:iam::${AWS::AccountId}:role/tf-managed/*" Condition: StringEquals: "iam:PassedToService": "lambda.amazonaws.com"