mirror of
https://github.com/Sea-Haven-Industries/seahaven-org-baseline.git
synced 2026-09-30 05:43:17 +00:00
docs(iam): resolve confirmed review findings from both INFRA-186 gates
Cross-family round 1 plus the /sh-security-review verifier confirmed 11 findings on the floor reduction, all documentation defects; no policy statement changes. The one HIGH: the Terraform migration checklist never widened the boundary, so a Lambda-bearing Terraform migration would deploy green and lose every data-plane call at first invoke. Checklist step 2 now carries the widening requirement, step 3 verifies deployed boundary content, and the terraform-substrate header no longer reads as 'Terraform path unaffected'. Also corrected: Description is a REPLACEMENT property (a Description edit wedges the custom-named policy and CFN's remedy is the forbidden rename), the sanctioned-source contradiction, the false AWSLambdaVPCAccessExecutionRole parity claim, the KMS log-group category error, stale size numbers (691/5,453), the same-PR widening contradiction, per-workload residue text, a LoggingConfig silent-log-loss note, the us-east-1 region pin rationale, and ENI DoS deferral now tracked as INFRA-200.
This commit is contained in:
parent
a1086e04fb
commit
08a1d41b05
3 changed files with 92 additions and 34 deletions
14
README.md
14
README.md
|
|
@ -272,14 +272,24 @@ split this substrate exists to enforce.
|
|||
`organization:seahaven:project:seahaven-<env>:workspace:<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 +
|
||||
credentials). **If the stack creates Lambda execution roles, this same PR
|
||||
must also widen `seahaven-lambda-execution-boundary`** per the WIDENING
|
||||
PATH in `lib/deploy-substrate/deploy-substrate.template.yaml`: the
|
||||
guardrail forces every Terraform-created role to carry that boundary, and
|
||||
it is a fleet-wide floor with zero data-plane permissions until widened —
|
||||
an unwidened migration deploys green, then every data-plane call is denied
|
||||
at first invoke and async/DLQ writes are discarded silently. IAM roles and
|
||||
boundary widenings = mandatory cross-family review +
|
||||
`/sh-security-review` on the diff.
|
||||
3. After deploy, verify: both roles exist; `hcptf-<stack>` 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`).
|
||||
`allowed`); and if step 2 widened the boundary, confirm the deployed
|
||||
default version carries the stack's data-plane statements
|
||||
(`aws iam get-policy-version`) — role verification alone never checks
|
||||
boundary content.
|
||||
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
|
||||
|
|
|
|||
|
|
@ -40,8 +40,11 @@ Description: >-
|
|||
# + #98 Phase B); DenySelfMutation and the widened policy/seahaven-*
|
||||
# DenyBoundaryPolicyEdit scope were then ported back here, so the two
|
||||
# copies' GUARDRAIL statement sets were reconciled as of that date.
|
||||
# SamCfnIamManagementPolicy and SamCfnExecutionRole remain byte-identical
|
||||
# across the two files and MUST still be changed together; the only delta
|
||||
# SamCfnIamManagementPolicy and SamCfnExecutionRole remain at parity on
|
||||
# their IAM STATEMENT SETS across the two files and MUST still be changed
|
||||
# together. Parity covers statements, not surrounding comments — a comment
|
||||
# may diverge where it describes boundary content, which now differs
|
||||
# between the files. The only functional delta
|
||||
# between them is the DependsOn line above, which is ordering, not
|
||||
# permission. LambdaExecutionBoundary is NO LONGER byte-identical — see
|
||||
# DELIBERATE DIVERGENCE below.
|
||||
|
|
@ -118,13 +121,13 @@ Description: >-
|
|||
# correctly left untouched.
|
||||
#
|
||||
# SIZE BUDGET: an attached managed policy document is capped at 6,144 characters
|
||||
# (whitespace excluded). LambdaExecutionBoundary measures 703 characters across
|
||||
# 4 statements as of 2026-07-30 — the fleet-wide floor only. Measure before
|
||||
# (whitespace excluded). LambdaExecutionBoundary measures 691 characters across
|
||||
# 4 statements as of 2026-07-31 — the fleet-wide floor only. Measure before
|
||||
# widening — len(json.dumps(doc,separators=(',',':'))) on the synthesized
|
||||
# PolicyDocument with ${AWS::AccountId} resolved, and UPDATE THESE TWO NUMBERS in
|
||||
# the same edit (they went stale twice inside this branch alone).
|
||||
#
|
||||
# Headroom is 5,441 characters, roughly TWELVE workloads at ~450 each. That is a
|
||||
# Headroom is 5,453 characters, roughly TWELVE workloads at ~450 each. That is a
|
||||
# deliberate outcome, not luck: an earlier revision of this branch pre-loaded
|
||||
# per-workload prefixes for all five mgmt SAM stacks and reached 5,457 characters
|
||||
# with 687 left — about one workload of room — before any stack had actually
|
||||
|
|
@ -214,14 +217,27 @@ Resources:
|
|||
# Ec2Eni name runtime-created resources or use actions AWS authorises against
|
||||
# "*" regardless of the ARN supplied. Do not "tighten" those.
|
||||
#
|
||||
# Every "verified <date>" annotation in this file is a POINT-IN-TIME
|
||||
# observation, not live state. Re-validate (usage counts, log-group CMK state,
|
||||
# per-stack permission sources) before citing one as justification for a
|
||||
# future change.
|
||||
#
|
||||
# WIDENING PATH — read this before migrating a stack into prod or dev.
|
||||
# The boundary is never widened by the person who hits the AccessDenied. It is
|
||||
# widened by the migrating stack's owner, in THIS repo, BEFORE the workload's
|
||||
# first deploy into the target account:
|
||||
# 0. PRECONDITION — check the managed-policy VERSION budget BEFORE merging:
|
||||
# max 5 versions, both
|
||||
# accounts are on v1 today. Every widening — and every Description-only
|
||||
# edit — burns one. Delete the oldest non-default version if at 5.
|
||||
# accounts are on v1 today. Every widening (PolicyDocument edit) burns
|
||||
# one. Description, ManagedPolicyName and Path are REPLACEMENT
|
||||
# properties per the CFN resource reference — CloudFormation cannot
|
||||
# replace a custom-named policy, so a Description-only edit FAILS the
|
||||
# stack update, and the error's suggested remedy (rename) is exactly
|
||||
# the forbidden rename in the COUPLING note above. Never edit those
|
||||
# three properties. Delete the oldest non-default version if at 5:
|
||||
# aws iam list-policy-versions --policy-arn \
|
||||
# arn:aws:iam::<acct>:policy/seahaven-lambda-execution-boundary
|
||||
# aws iam delete-policy-version --version-id v<oldest-non-default> ...
|
||||
# 1. Derive the workload's needs from ITS OWN TEMPLATE — open the stack's
|
||||
# template.yaml and read the actual IAM policy statements. The
|
||||
# permission-source block below is a STARTING POINT, NOT THE AUTHORITY:
|
||||
|
|
@ -234,8 +250,11 @@ Resources:
|
|||
# SILENT classes specifically: a denied SQS destination/DLQ write discards
|
||||
# the async event with no error and no alarm; a denied scheduler call may
|
||||
# sit behind a bare except; a denied KMS decrypt for env-var encryption
|
||||
# fails at cold-start INIT; and any CMK-encrypted resource needs the
|
||||
# matching kms:ViaService principal, not just the kms action.
|
||||
# fails at cold-start INIT; any CMK-encrypted resource needs the
|
||||
# matching kms:ViaService principal, not just the kms action; and a
|
||||
# function using LoggingConfig with a custom log-group name outside
|
||||
# /aws/lambda* silently loses ALL logs — add a scoped logs statement
|
||||
# for the custom group or keep the default group name.
|
||||
# 3. Measure. See SIZE BUDGET in the header. There is NO escape hatch.
|
||||
# 4. Both review gates run and neither discharges the other: the GPT-4.1
|
||||
# cross-family review against the real diff, and /sh-security-review
|
||||
|
|
@ -250,7 +269,7 @@ Resources:
|
|||
# production failure.
|
||||
#
|
||||
# CONSIDERED AND REJECTED: a Deny statement reserving the seahaven-* namespace.
|
||||
# With the Allow set now enumerated per workload it is fully redundant (verified
|
||||
# With the Allow set reduced to the fleet-wide floor it is fully redundant (verified
|
||||
# 2026-07-30: seahaven-prod-config-* and seahaven-prod-vpc-flow-logs-* are
|
||||
# already denied by the Allow set alone), and a Deny inside a BOUNDARY is the
|
||||
# hardest failure mode in the estate to debug — it beats every Allow in every
|
||||
|
|
@ -260,12 +279,13 @@ Resources:
|
|||
# NOTE ON WHAT THESE PREFIXES ARE. All five stacks below currently live in the
|
||||
# MANAGEMENT account and none of their resources exists in seahaven-prod or
|
||||
# seahaven-dev yet. These are MIGRATION-CANDIDATE prefixes for the accounts this
|
||||
# template deploys to, not an inventory of what is deployed there. They are the
|
||||
# sanctioned scope source because they are the enumerated permission sources of
|
||||
# the stacks this boundary exists to cap.
|
||||
# template deploys to, not an inventory of what is deployed there. They are a
|
||||
# SECONDARY RECORD and a starting point for widening PRs — the authority is
|
||||
# each stack's own template (WIDENING PATH step 1). No Resource pattern in
|
||||
# the floor above derives from this block.
|
||||
#
|
||||
# Permission sources per stack (verified live 2026-07-30; this block is the
|
||||
# sanctioned source for every Resource pattern above — keep it accurate):
|
||||
# Permission sources per stack (verified live 2026-07-30; starting point
|
||||
# only — verify every entry against the owning repo before use):
|
||||
#
|
||||
# afterhours-shift-manager (functions: afterhours-*, 6 live)
|
||||
# - DynamoDB CRUD (afterhours-shifts table)
|
||||
|
|
@ -378,6 +398,13 @@ Resources:
|
|||
# /aws/lambda/* group (log poisoning). No read action is granted here, so
|
||||
# this is not an exfiltration path. Tighten only once every
|
||||
# boundary-carrying function is confirmed to set an explicit FunctionName.
|
||||
# REGION IS PINNED TO us-east-1 DELIBERATELY: every Sea Haven workload
|
||||
# deploys to us-east-1, and this template itself only ever deploys there.
|
||||
# ${AWS::Region} would resolve to the identical string, so it would
|
||||
# document nothing. A future stack in another region carries this
|
||||
# boundary but CANNOT write its logs (the silent class above) — so a
|
||||
# cross-region migration MUST add region-scoped statements in its
|
||||
# widening PR, same as any other data-plane need.
|
||||
- Sid: CloudWatchLogsWrite
|
||||
Effect: Allow
|
||||
Action:
|
||||
|
|
@ -415,8 +442,10 @@ Resources:
|
|||
Resource: "*"
|
||||
|
||||
# ── VPC / ENI management (UNSCOPABLE, kept "*" deliberately) ────────
|
||||
# Matches AWSLambdaVPCAccessExecutionRole exactly, and for the same
|
||||
# reasons. The three ec2:Describe* actions do not support resource-level
|
||||
# Derived from AWSLambdaVPCAccessExecutionRole, NOT an exact match:
|
||||
# DescribeSecurityGroups and DescribeVpcs exceed that managed policy
|
||||
# (kept for CFN/SAM VpcConfig validation; read-only). The four
|
||||
# ec2:Describe* actions do not support resource-level
|
||||
# permissions AT ALL — an ARN in Resource is ignored and the call is
|
||||
# authorised against "*" — so narrowing them is cosmetic. The ENI in
|
||||
# CreateNetworkInterface / DeleteNetworkInterface is created by the
|
||||
|
|
@ -425,16 +454,20 @@ Resources:
|
|||
# AssignPrivateIpAddresses / UnassignPrivateIpAddresses are for EFA and
|
||||
# secondary IPs — not part of the Lambda ENI lifecycle — omitted.
|
||||
#
|
||||
# KNOWN OPEN ITEM (pre-existing, NOT introduced by INFRA-186):
|
||||
# KNOWN OPEN ITEM (pre-existing, NOT introduced by INFRA-186; tracked
|
||||
# as INFRA-200):
|
||||
# ec2:DeleteNetworkInterface on "*" lets a bounded Lambda delete any ENI
|
||||
# in the account, including NAT / VPC-endpoint / RDS ENIs — a
|
||||
# denial-of-service primitive inherited from the AWS managed policy. The
|
||||
# durable fix is a Condition on ec2:Subnet / ec2:Vpc naming the VPC the
|
||||
# Lambda fleet attaches to. That VPC does not exist in seahaven-prod or
|
||||
# seahaven-dev today (payments-dashboard's 10.20.0.0/16 VPC is in mgmt),
|
||||
# so writing the condition now would encode an mgmt resource into a
|
||||
# prod/dev template. Whoever brings the VPC across in payments-dashboard's
|
||||
# migration PR adds the condition in the same PR.
|
||||
# durable fix is a Condition on ec2:Subnet / ec2:Vpc naming THE SET OF
|
||||
# VPCs that boundary-carrying Lambdas attach to — not a single VPC id;
|
||||
# the list must be extended whenever a workload introduces a new VPC
|
||||
# (tag-based conditions are the alternative if the set churns). No such
|
||||
# VPC exists in seahaven-prod or seahaven-dev today (payments-dashboard's
|
||||
# 10.20.0.0/16 VPC is in mgmt), so writing the condition now would encode
|
||||
# an mgmt resource into a prod/dev template. Whoever brings the first VPC
|
||||
# across in payments-dashboard's migration PR adds the condition in the
|
||||
# same PR.
|
||||
- Sid: Ec2Eni
|
||||
Effect: Allow
|
||||
Action:
|
||||
|
|
@ -478,8 +511,11 @@ Resources:
|
|||
#
|
||||
# CONSEQUENCE FOR EVERY MIGRATION PR (mandatory, see WIDENING PATH in
|
||||
# the header): a stack landing in prod or dev MUST add its own
|
||||
# data-plane statements here, derived from ITS OWN template, in the
|
||||
# same PR that deploys it. Without them its Lambdas get AccessDenied
|
||||
# data-plane statements here, derived from ITS OWN template, in its
|
||||
# own PR to THIS repo, deployed to UPDATE_COMPLETE before the
|
||||
# workload's first deploy from its own repo (the workload PR and the
|
||||
# widening PR cannot be the same PR — they live in different repos).
|
||||
# Without them its Lambdas get AccessDenied
|
||||
# at first invoke. The permission-source block above is the starting
|
||||
# point, NOT the authority — verify every entry against the stack.
|
||||
#
|
||||
|
|
@ -489,8 +525,13 @@ Resources:
|
|||
# justifications; they are also the SILENT-failure classes, which is
|
||||
# why they belong in the floor rather than in per-workload widenings.
|
||||
#
|
||||
# KMS is absent deliberately: both accounts have ZERO CMK-encrypted
|
||||
# log groups today (verified 2026-07-30). A workload bringing a
|
||||
# KMS is absent deliberately: zero boundary-carrying roles exist, so
|
||||
# nothing bounded decrypts anything yet. (Log-group CMKs are the one
|
||||
# KMS case that never hits an execution role — CloudWatch Logs
|
||||
# decrypts via the key policy's grant to the Logs service principal —
|
||||
# so log-group CMK state is not the gate here. seahaven-prod already
|
||||
# hosts the seahaven-dynamodb-cmk table key; the first migrating
|
||||
# stack touching that table must cover it.) A workload bringing a
|
||||
# CMK-encrypted resource adds a KMS statement with the matching
|
||||
# kms:ViaService principal in its own migration PR — a missing
|
||||
# ViaService entry denies, and for env-var encryption it fails at
|
||||
|
|
@ -999,8 +1040,9 @@ Resources:
|
|||
# conditioned on iam:PermissionsBoundary StringEquals the
|
||||
# seahaven-lambda-execution-boundary ARN. That condition means
|
||||
# any role this execution role creates must have the boundary
|
||||
# applied, so it can never exceed what the boundary allows
|
||||
# (which is scoped to the services the five stacks actually use).
|
||||
# applied, so it can never exceed what the boundary allows (in
|
||||
# THIS file the fleet-wide floor plus per-migration widenings;
|
||||
# in mgmt's copy the five SAM stacks' service wildcards).
|
||||
#
|
||||
# iam:PassRole is also included here so CloudFormation can pass
|
||||
# the auto-generated Lambda execution role to the Lambda service.
|
||||
|
|
|
|||
|
|
@ -65,8 +65,14 @@ Description: >-
|
|||
# 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.
|
||||
# (boundary reduced to a fleet-wide floor; per-workload boundaries are
|
||||
# INFRA-187) changed the boundary's CONTENT, not its ARN, so this file is
|
||||
# textually untouched — but the Terraform path IS affected: the Conditions
|
||||
# below FORCE every role a Terraform apply creates onto that boundary, and
|
||||
# the floor carries zero data-plane permissions. A migrating stack that
|
||||
# creates Lambda execution roles must widen the boundary per the WIDENING
|
||||
# PATH in lib/deploy-substrate/deploy-substrate.template.yaml, deployed
|
||||
# before its first apply (README migration checklist step 2).
|
||||
#
|
||||
# SIZE BUDGET: an attached managed policy document is capped at 6,144
|
||||
# characters (whitespace excluded). The statement set below is ~2.5 KB.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue