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"