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.
This commit is contained in:
Adam Moussa 2026-07-30 16:55:45 -04:00
parent ea27635ef2
commit 981960433f
No known key found for this signature in database
3 changed files with 228 additions and 52 deletions

View file

@ -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-<stack>` apply + `hcptf-<stack>-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
(`<stack>-<env>`). Apply method **Manual**; automatic speculative plans on
if VCS-connected (CLI `terraform plan` runs are inherently speculative).
2. PR to this repo appending `hcptf-<stack>-plan` (read-only,
ViewOnlyAccess-class, **no** IAM writes, **no** guardrail-policy attach)
2. PR to this repo appending `hcptf-<stack>-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-<stack>` (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

View file

@ -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");

View file

@ -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::<acct>: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"