From 274f93349555485639945bb166d67861cd624ad9 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 17:46:37 -0400 Subject: [PATCH 1/8] refactor(iam): scope Lambda execution boundary to per-workload prefixes Re-scope seahaven-lambda-execution-boundary in the prod/dev copy of deploy-substrate.template.yaml from account-wide wildcards to per-workload resource prefixes drawn from the template's own permission-source block. This is the PROD/DEV HALF of INFRA-186. What was scoped (wildcard -> per-workload prefix): - dynamodb table/* + table/*/index/* -> afterhours-*, front-*, meal-order-manager-*, PaymentsDashboard*, payments-dashboard-* (a trailing * after each prefix also covers the /index/* GSI ARNs, so the separate table/*/index/* entry is deleted rather than replaced) - s3 *-${AccountId} -> meal-order-manager-*-${AccountId} (read/write) and seahaven-payments-* / seahaven-payroll-emails-* (read-only). The removed pattern was not an ownership check at all: S3 ARNs carry no account field, so it was a bare name-suffix filter that matched 8 of 9 buckets in prod -- including the org's own Config and VPC-flow-log buckets -- with PutObject and DeleteObject. - secretsmanager secret:* -> five / prefixes + the legacy bare afi-api-key-*. The wildcard reached workorder-ingest's HMAC signing key, i.e. a webhook-forgery primitive. - ssm parameter/* -> afterhours-shift-manager and meal-order-manager, each as both the bare path ARN and /* (GetParametersByPath authorises against the path, not the leaf) - sqs :* -> payments-* - lambda function:* -> afterhours-*, meal-order-manager-*, payments-*. Highest-leverage fix here: an invoked function runs under its OWN role, and every non-SAM function in prod is CDK-deployed with no boundary, so function:* was a boundary-escape primitive, not just lateral movement. - ses identity/* + configuration-set/* -> the two verified prod identities; configuration-set dropped (zero exist) - logs split into a scoped write half (/aws/lambda*) and a wildcard describe half (DescribeLogGroups is a collection action AWS authorises against "*" regardless of the ARN supplied) Deliberately NOT tightened, each with written justification on the statement: CloudWatchLogsDescribe, XRay and Ec2Eni name runtime-created resources or use actions that support no resource-level permissions. KMS keeps key/* -- key ARNs carry UUID key ids, not workload names -- and is constrained by a kms:ViaService condition instead, which inherits the per-workload scoping of the services above for free. No runtime risk. PermissionsBoundaryUsageCount is 0 in BOTH accounts this file deploys to (seahaven-prod 011934824531 and seahaven-dev 710827005802, verified 2026-07-30 via aws iam get-policy), so no live Lambda can break. Adam scoped the handoff to prod/dev for exactly this reason. Since usage is 0, a boundary that is slightly too tight is recoverable -- the migrating stack widens it in its own PR before its first deploy -- whereas leaving it loose perpetuates the exposure. The widening path and its ordering hazard are documented in the template. mgmt is DELIBERATELY UNTOUCHED and the two copies are now DIVERGENT. The management account (328440206208) uses a separate copy in Sea-Haven-Industries/.github/oidc-deploy-roles.yaml and has 26 LIVE boundary-carrying roles, where tightening is a production change with a silent, deploy-time-invisible failure mode; it needs its own validated rollout and is explicitly out of scope. The header's parity rule is therefore now SCOPED, not global: SamCfnIamManagementPolicy and SamCfnExecutionRole stay byte-identical and must still change together, while LambdaExecutionBoundary must NOT be reconciled in either direction. A DELIBERATE DIVERGENCE block records this so a future mechanical drift check does not "fix" it away, following the same pattern terraform-substrate.template.yaml uses for its divergences. Content-only change: ManagedPolicyName, the policy ARN and the logical id LambdaExecutionBoundary are unchanged. Eight StringEquals iam:PermissionsBoundary conditions across this file and terraform-substrate.template.yaml pin the boundary by literal name, and a rename fails SILENTLY -- an IAM condition naming a non-existent policy simply never matches. Verification: - npx tsc --noEmit: clean - npx cdk synth deploy-substrate-prod deploy-substrate-dev: succeeds - synthesized resource diff vs main: LambdaExecutionBoundary is the ONLY changed resource; GitHubOIDCProvider, SamCfnExecutionRole and SamCfnIamManagementPolicy are byte-identical - policy document 4,060 chars / 6,144 cap (2,084 headroom), 13 statements, identical in both accounts - iam simulate-custom-policy against live prod, every deny re-checked against an Allow */* positive control: 11/11 cross-tenant denies are real (Config and flow-log buckets, proposal-system-uploads, proposal-system/db-credentials, workorder-ingest/shoc-webhook-hmac, proposal-system-api, proposal-system-jobs, WorkOrders, /seahaven/dynamodb/cmk-arn, the flow-log group, seahavenind.com) and 23/23 enumerated workload resources still allow Checkov suppressions re-keyed: CKV_AWS_111 still fires on the boundary because three statements legitimately retain Resource:"*", so the suppression is still required. All three line-keyed ids shifted (139->291, 329->713, 805->1189); new ids added, superseded ids retained, and the boundary justification's stale "OPEN follow-up: tighten to per-workload prefixes" sentence rewritten to CLOSED since this commit is what closes it. Scanners: RESULT PASS. Refs: INFRA-186 --- .../deploy-substrate.template.yaml | 532 +++++++++++++++--- 1 file changed, 458 insertions(+), 74 deletions(-) diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index ce16245..e8a59d3 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -39,16 +39,98 @@ Description: >- # added. The mgmt copy was remediated 2026-07-27 (.github PRs #95 Phase A # + #98 Phase B); DenySelfMutation and the widened policy/seahaven-* # DenyBoundaryPolicyEdit scope were then ported back here, so the two -# copies' statement sets are reconciled as of that date — every IAM -# statement in LambdaExecutionBoundary, SamCfnIamManagementPolicy and -# SamCfnExecutionRole is byte-identical across the two files; the only -# remaining delta is the DependsOn line above, which is ordering, not -# permission. If a substrate statement changes again, change BOTH files -# in the same piece of work. +# 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 +# between them is the DependsOn line above, which is ordering, not +# permission. LambdaExecutionBoundary is NO LONGER byte-identical — see +# DELIBERATE DIVERGENCE below. +# +# DELIBERATE DIVERGENCE — LambdaExecutionBoundary (INFRA-186, 2026-07-30) +# The parity rule above is SCOPED, not global. LambdaExecutionBoundary in THIS +# file is DELIBERATELY STRICTER than the mgmt copy in +# Sea-Haven-Industries/.github/oidc-deploy-roles.yaml. Do not "reconcile" the two +# by copying mgmt's statements back over these, or vice versa; the divergence is +# load-bearing. A future mechanical drift check WILL read it as drift — it is not. +# +# 1. WHAT DIVERGED. Every scopable Resource pattern in LambdaExecutionBoundary +# was re-scoped from account-wide wildcards (table/*, table/*/index/*, +# secret:*, parameter/*, sqs :*, function:*, ses identity/* + +# configuration-set/*, and an s3:::*- pattern that was a bare +# name-suffix filter rather than an ownership check) to per-workload +# prefixes drawn from the permission-source block above. The three genuinely +# unscopable statements (CloudWatchLogsDescribe, XRay, Ec2Eni) keep +# Resource "*" with written justification on each. CloudWatch Logs was split +# into a scoped write half and a wildcard describe half. KMS keeps key/* and +# is scoped by a kms:ViaService condition instead, because key ARNs carry +# UUID key ids that cannot be prefix-scoped. +# +# 2. WHY MGMT'S RATIONALE IS LEGITIMATE THERE. The superset framing this file +# used to carry ("being slightly broad is the correct trade-off; a boundary +# that is too tight will break Lambda functions at runtime AFTER deploy") is +# a real constraint in the management account: 328440206208 has 26 LIVE +# roles carrying seahaven-lambda-execution-boundary, across all five SAM +# stacks. Tightening there is a production change to running workloads with +# a silent, deploy-time-invisible failure mode. +# +# 3. WHY IT DOES NOT TRANSFER HERE. This file deploys ONLY to seahaven-prod +# (011934824531) and seahaven-dev (710827005802), where +# PermissionsBoundaryUsageCount is 0 and 0 respectively (aws iam get-policy, +# verified 2026-07-30; corroborated by list-roles returning no role carrying +# any permissions boundary in either account). No live Lambda can break, so +# the risk that justifies mgmt's breadth is absent — while the exposure is +# strictly WORSE here than in mgmt, because prod is multi-tenant: the old +# wildcards reached proposal-system's, procurement-ingest's and +# workorder-ingest's CDK-owned tables, buckets, secrets and queues, the org's +# own Config and VPC-flow-log buckets, and — via function:* — CDK Lambdas +# that carry no boundary at all. +# +# 4. RECONCILIATION OBLIGATION, RESTATED. For LambdaExecutionBoundary the two +# copies are now INTENTIONALLY DIFFERENT and must NOT be synchronised: +# - A change to the per-workload Resource patterns in THIS file does NOT +# propagate to mgmt. +# - A change to mgmt's boundary does NOT propagate here. +# - Any change to the ACTION lists, or any new statement, is a substrate +# semantic change and DOES still require the same review in both copies. +# For SamCfnIamManagementPolicy and SamCfnExecutionRole the original rule is +# unchanged: change BOTH files in the same piece of work. +# +# KNOWN OPEN ITEM (deferred, not closed by INFRA-186): mgmt 328440206208 still +# carries the account-wide patterns. Tightening it needs its own validated +# rollout — enumerate what the 26 live roles actually call, stage it, and be +# ready to roll back — and is explicitly OUT OF SCOPE of INFRA-186. Until that +# lands, the two copies stay divergent and that is the intended state. +# +# COUPLING: the boundary's ManagedPolicyName and ARN are UNCHANGED and must stay +# unchanged. Four Conditions in SamCfnIamManagementPolicy below, and four more in +# HcptfIamManagementPolicy in lib/terraform-substrate/terraform-substrate.template.yaml, +# pin arn:aws:iam:::policy/seahaven-lambda-execution-boundary by literal +# string inside StringEquals iam:PermissionsBoundary. A rename fails SILENTLY — an +# IAM condition naming a non-existent policy simply never matches, so the +# escalation control would evaporate rather than error — and would additionally +# force a CloudFormation REPLACEMENT that any role carrying the boundary would +# block. INFRA-186 is a CONTENT-ONLY change for exactly this reason; +# terraform-substrate.template.yaml already records that expectation and is +# correctly left untouched. +# +# SIZE BUDGET: an attached managed policy document is capped at 6,144 characters +# (whitespace excluded). LambdaExecutionBoundary measures 4,060 characters across +# 13 statements as of 2026-07-30 (was 2,507 across 11 — TIGHTENING COSTS +# CHARACTERS). Measure before widening — +# len(json.dumps(doc,separators=(',',':'))) on the synthesized PolicyDocument with +# ${AWS::AccountId} resolved. A typical new workload costs ~450 characters, so +# roughly five more fit. CRITICAL: unlike the 2026-07-27 inline-limit incident, +# there is NO restructure available when this cap is reached — a role has exactly +# ONE permissions boundary, so statements cannot be spilled into a second attached +# managed policy. At the cap the only levers are prefix consolidation and dropping +# unused actions. # # This template is deployed via lib/deploy-substrate-stack.ts -# (cloudformation-include) as stack seahaven-deploy-substrate, once per -# member account that hosts SAM workloads. +# (cloudformation-include) as stack seahaven-deploy-substrate, once per member +# account that hosts SAM workloads (currently seahaven-prod 011934824531 and +# seahaven-dev 710827005802 via bin/app.ts instances deploy-substrate-prod / +# deploy-substrate-dev; NEVER mgmt — 328440206208 is served by the .github copy +# named above until its stacks migrate out). Parameters: CreateOIDCProvider: @@ -83,7 +165,7 @@ Resources: UpdateReplacePolicy: Retain # --------------------------------------------------------------------------- - # Lambda execution permissions boundary (INFRA-103) + # Lambda execution permissions boundary (INFRA-103, re-scoped by INFRA-186) # # This managed policy is the CEILING for every Lambda execution role that the # five SAM stacks auto-generate via AWS::Serverless::Function. Applying it as @@ -91,49 +173,119 @@ Resources: # intersection of the role's own policies and this boundary, so a misconfigured # SAM role can never exceed what is listed here. # - # The boundary is intentionally a SUPERSET of the union of all runtime - # permissions currently granted across the five stacks. Being slightly broad - # is the correct trade-off at this stage — a boundary that is too tight will - # break Lambda functions at runtime after deploy, which is worse than a slightly - # loose boundary that is tightened in a follow-up. + # SCOPING RULE (INFRA-186, 2026-07-30). This boundary is a CLOSED ENUMERATION + # of per-workload resource prefixes. It was previously a deliberate SUPERSET + # with account-wide wildcards (table/*, secret:*, sqs :*, function:*, + # parameter/*, and an s3:::*- pattern that was a name-suffix filter, + # not an ownership check). That trade-off was made when the only account + # carrying this policy hosted nothing but the five SAM stacks. It does not + # survive multi-tenancy: seahaven-prod now hosts CDK-owned tenants + # (proposal-system, procurement-ingest, workorder-ingest) whose tables, buckets, + # secrets and queues those wildcards reached, and whose Lambdas carry NO + # permissions boundary at all — making function:* a boundary-escape primitive. # - # Permission sources per stack: + # Tightening here carries ZERO runtime risk and was sequenced deliberately: + # PermissionsBoundaryUsageCount is 0 in BOTH accounts this template deploys to + # (seahaven-prod 011934824531 and seahaven-dev 710827005802, verified + # 2026-07-30), so no live Lambda can break. A boundary that is slightly TOO + # TIGHT is recoverable here — the migrating stack widens it in its own PR before + # its first deploy — whereas leaving it loose perpetuates the exposure. Prefer + # tighter; the widening path is below. # - # afterhours-shift-manager + # A resource pattern that genuinely CANNOT be scoped keeps its wildcard WITH a + # written justification on the statement: CloudWatchLogsDescribe, XRay and + # Ec2Eni name runtime-created resources or use actions AWS authorises against + # "*" regardless of the ARN supplied. Do not "tighten" those. + # + # 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: + # 1. Add the workload's prefixes to the relevant per-service Resource lists + # above — never add a new per-workload statement (a duplicated action list + # costs ~250 characters for zero new actions; an extra ARN costs ~60). + # Add its entry to the permission-source block below in the same edit: that + # block is the sanctioned scope source, and a prefix added without one will + # be "reconciled" away later. + # 2. Measure. See SIZE BUDGET in the header. There is NO escape hatch. + # 3. Both review gates run and neither discharges the other: the GPT-4.1 + # cross-family review against the real diff, and /sh-security-review + # (IaC/IAM is on the mandatory surface). CLI down = review outstanding. + # 4. Merge and let CI deploy deploy-substrate-prod / deploy-substrate-dev to + # UPDATE_COMPLETE, THEN deploy the workload stack. + # 5. Check the managed-policy VERSION budget first: 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. + # ORDERING IS NOT ENFORCED BY CLOUDFORMATION AND THIS IS THE MOST IMPORTANT + # SENTENCE HERE: the workload's deploy SUCCEEDS even against a stale boundary, + # because seahaven-cfn-exec-iam-management's gate checks that the boundary ARN + # is attached, never its contents. The failure surfaces later, at first invoke, + # as AccessDenied. A stale boundary is a silent deploy-time pass and a loud + # 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 + # 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 + # policy with no synth-time signal. Revisit only if a widening ever has to + # re-broaden a per-service Resource list back toward a wildcard. + # + # 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. + # + # Permission sources per stack (verified live 2026-07-30; this block is the + # sanctioned source for every Resource pattern above — keep it accurate): + # + # afterhours-shift-manager (functions: afterhours-*, 6 live) # - DynamoDB CRUD (afterhours-shifts table) # - secretsmanager:GetSecretValue (afterhours-shift-manager/*) + # - ssm:GetParameter (/afterhours-shift-manager/*: channel-id, + # slack-bot-token, slack-signing-secret) # - ses:SendEmail (SES identity) + # - lambda:InvokeFunction (ReleaseNotifyInvokeRole, + # HolidaySchedulerExecutionRole — these two carry the boundary and need + # ONLY this action) # - CloudWatch Logs (all functions) + # - UNRESOLVED: /3cx-scheduler/* ownership (this stack vs. the retired + # standalone 3CX scheduler). Deliberately NOT granted. Resolve at migration. # - # payments-dashboard - # - DynamoDB CRUD / Read (PaymentsDashboard table) - # - S3 GetObject (payroll-emails, payments-csv buckets) + # payments-dashboard (functions: payments-*) + # - DynamoDB CRUD / Read (PaymentsDashboard table — legacy PascalCase) + # - S3 GetObject ONLY (seahaven-payments-csv-*, seahaven-payments-boa-raw-*, + # seahaven-payroll-emails-*) — no write intent enumerated # - secretsmanager:GetSecretValue (payments-dashboard/*) - # - sqs:SendMessage + sqs:ReceiveMessage + sqs:DeleteMessage etc. - # (PayrollBatchQueue + DLQs) - # - lambda:InvokeFunction (ExpenseReceiver → ExpenseProcessor) - # - ec2:CreateNetworkInterface / DescribeNetworkInterfaces / - # DeleteNetworkInterface (VPC-attached functions) + # - sqs Send/Receive/Delete etc. (payments-payroll-batch + DLQs) + # - lambda:InvokeFunction (ExpenseReceiver -> ExpenseProcessor) + # - ec2 ENI lifecycle (VPC-attached functions) + # - KMS via dynamodb (table CMK) — no SSM, no SES # - CloudWatch Logs # - # meal-order-manager + # meal-order-manager (functions: meal-order-manager-*, 7 live) # - DynamoDB CRUD / Read (meal-order-manager-orders table) - # - S3 CRUD (ReportsBucket) + s3:GetObject (ReportsBucket presigned URLs) + # - S3 CRUD (meal-order-manager-reports-*, meal-order-manager-form-*) # - secretsmanager:GetSecretValue (meal-order-manager/*) # - ssm:GetParameter (/meal-order-manager/*) - # - lambda:InvokeFunction (submit-order → slack-notifier, - # close-form → aggregate-orders) + # - lambda:InvokeFunction (submit-order -> slack-notifier, + # close-form -> aggregate-orders, plus AdminAuthorizerInvokeRole) # - ses:SendRawEmail # - CloudWatch Logs # - # front-integrations + # front-integrations (functions: front-*) # - DynamoDB CRUD (front-sla-alerts table) - # - secretsmanager:GetSecretValue (by ARN, various) + # - secretsmanager:GetSecretValue (front-integrations/*) # - CloudWatch Logs + # - no S3 / SQS / SSM / SES / KMS / VPC # - # afi-backup-monitor - # - secretsmanager:GetSecretValue (by ARN) + # afi-backup-monitor (functions: afi-*) + # - secretsmanager:GetSecretValue (afi-backup-monitor/slack-webhook-url AND + # the bare, unprefixed afi-api-key — see the SecretsManager statement) # - CloudWatch Logs + # - nothing else # # --------------------------------------------------------------------------- LambdaExecutionBoundary: @@ -149,18 +301,69 @@ Resources: Version: "2012-10-17" Statement: - # ── CloudWatch Logs (every Lambda) ────────────────────────────────── - - Sid: CloudWatchLogs + # ── CloudWatch Logs — write (every Lambda) ────────────────────────── + # Scoped to the Lambda log-group namespace. Every SAM function's group + # is /aws/lambda/, and the trailing * also covers the + # :log-stream: suffix PutLogEvents authorises against, so one ARN + # serves CreateLogGroup, CreateLogStream, PutLogEvents and + # DescribeLogStreams. The * is deliberately NOT after a trailing slash: + # /aws/lambda* also matches the /aws/lambda-insights groups the Lambda + # Insights extension writes to, which /aws/lambda/* would have denied. + # VERIFICATION PROVENANCE, stated precisely (2026-07-30). What + # iam simulate-custom-policy DOES confirm: this pattern allows + # logs:CreateLogGroup / logs:PutLogEvents on the bare group ARN + # log-group:/aws/lambda/ (and /aws/lambda//), and DENIES + # log-group:seahaven-prod-vpc-flow-logs — the latter re-checked against + # an Allow */* positive control, which allows it, so the deny is real + # policy behaviour and not a simulator artifact. + # What the simulator CANNOT evaluate, so do NOT claim it was verified: + # log-stream-qualified ARNs (log-group::log-stream:) and the bare + # /aws/lambda-insights group both return implicitDeny EVEN UNDER an + # Allow */* policy. That is a simulator resource-parsing limitation, not + # a denial. Coverage of those two rests on documented IAM wildcard + # semantics — "*" matches any sequence of characters including ":" and + # "/" — which is why the * is deliberately NOT placed after a trailing + # slash. If this ever needs true end-to-end proof, it must come from a + # real invoke in dev, not from the simulator. + # + # NOT scoped per workload, deliberately. A per-stack prefix + # (/aws/lambda/payments-* etc.) was considered and rejected: a Lambda + # denied PutLogEvents does not fail — it keeps running and silently + # produces no logs. Log denial is the one failure class in this policy + # that is NOT loud, so it must not depend on function-name discipline. + # ACCEPTED RESIDUAL RISK: a SAM Lambda can write into another tenant's + # /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. + - Sid: CloudWatchLogsWrite Effect: Allow Action: - logs:CreateLogGroup - logs:CreateLogStream - logs:PutLogEvents - - logs:DescribeLogGroups - logs:DescribeLogStreams + Resource: + - !Sub "arn:aws:logs:us-east-1:${AWS::AccountId}:log-group:/aws/lambda*" + + # ── CloudWatch Logs — describe (UNSCOPABLE, kept "*" deliberately) ── + # logs:DescribeLogGroups is a COLLECTION action: AWS authorises it + # against "*" regardless of any resource ARN supplied. Scoping it would + # produce a policy that reads tighter and denies at runtime, so it is + # split into its own statement and keeps the wildcard. Read-only + # metadata; it cannot mutate anything or return log content. + - Sid: CloudWatchLogsDescribe + Effect: Allow + Action: + - logs:DescribeLogGroups Resource: "*" - # ── X-Ray tracing (standard Lambda execution) ──────────────────── + # ── X-Ray tracing (UNSCOPABLE, kept "*" deliberately) ─────────────── + # xray:PutTraceSegments / PutTelemetryRecords support no resource-level + # permissions — X-Ray exposes no ARN for them, which is why the AWS + # managed AWSXRayDaemonWriteAccess also uses "*". Any ARN written here + # would be inert and would falsely imply a control exists. Write-only + # into this account's own trace store; no cross-tenant read is + # expressible with this action set. - Sid: XRay Effect: Allow Action: @@ -168,10 +371,27 @@ Resources: - xray:PutTelemetryRecords Resource: "*" - # ── VPC / ENI management (payments-dashboard VPC functions) ──────── - # Matches AWSLambdaVPCAccessExecutionRole exactly. - # AssignPrivateIpAddresses / UnassignPrivateIpAddresses are for EFA - # and secondary IPs — not part of the Lambda ENI lifecycle — omitted. + # ── VPC / ENI management (UNSCOPABLE, kept "*" deliberately) ──────── + # Matches AWSLambdaVPCAccessExecutionRole exactly, and for the same + # reasons. The three 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 + # Lambda service at attach time with an id that cannot exist when this + # policy is written. Nothing here is scopable by resource name. + # 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): + # 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. - Sid: Ec2Eni Effect: Allow Action: @@ -183,9 +403,20 @@ Resources: - ec2:DescribeVpcs Resource: "*" - # ── DynamoDB (afterhours, payments, meal-order, front-integrations) ─ - # Table/* covers base-table operations; table/*/index/* is required for - # Query/Scan on Global Secondary Indexes. + # ── DynamoDB — one prefix per enumerated workload ─────────────────── + # Replaces table/* + table/*/index/*, which reached every table in the + # account. A trailing * on each workload prefix covers the base table + # AND its /index/* GSI ARNs in a single entry (IAM wildcards match "/"), + # verified 2026-07-30 with iam simulate-custom-policy against + # table/afterhours-shifts/index/gsi1 — so the separate table/*/index/* + # line is deleted, not replaced. Prod's CDK-owned tables (WorkOrders, + # WorkOrderComments, purchase-orders, verified-sites, pending-site-review) + # match none of these prefixes, which is the point. + # PaymentsDashboard is a legacy PascalCase table exempt from renaming + # under handbook naming-conventions.md; the kebab-case prefix is listed + # alongside it so a future rename cannot silently lock the stack out. + # Table names are NOT stack names (afterhours-shifts, front-sla-alerts) — + # scoping naively off stack name would deny at runtime. - Sid: DynamoDB Effect: Allow Action: @@ -200,11 +431,35 @@ Resources: - dynamodb:DescribeTable - dynamodb:ConditionCheckItem Resource: - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/*" - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/*/index/*" + - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/afterhours-*" + - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/front-*" + - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/meal-order-manager-*" + - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/PaymentsDashboard*" + - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/payments-dashboard-*" - # ── S3 (payments-dashboard read, meal-order-manager CRUD) ────────── - - Sid: S3 + # ── S3 — write access (meal-order-manager only) ───────────────────── + # THE LARGEST SECURITY WIN IN INFRA-186. The removed + # arn:aws:s3:::*-${AWS::AccountId} was not a per-workload scope at all: + # S3 ARNs carry no account field, so it was a bare NAME-SUFFIX FILTER + # matching every bucket in the account whose name ends in the account id. + # In seahaven-prod today that was 8 of 9 buckets — all four + # proposal-system-*, both ingest-email buckets, AND the org's own + # seahaven-prod-config-* and seahaven-prod-vpc-flow-logs-* — with + # PutObject and DeleteObject. That is anti-forensics capability over the + # org's own security telemetry, handed to the SAM fleet's ceiling. + # Verified 2026-07-30 with iam simulate-custom-policy: the patterns below + # deny seahaven-prod-config-*, seahaven-prod-vpc-flow-logs-* and + # proposal-system-uploads-*, and allow meal-order-manager-reports-*. + # The four meal-order-manager-*-${AWS::AccountId} entries previously + # listed here were strictly redundant — every one ends in - and + # was already matched by the wildcard above them; their comment claiming a + # "non-AccountId suffix pattern" was contradicted by the ARNs beneath it. + # Split by DIRECTION of access: meal-order-manager is the only stack the + # permission-source block gives write intent to (ReportsBucket / + # FormBucket CRUD). Both the bucket and object ARN forms are listed in + # each statement because s3:ListBucket authorises against the bucket ARN + # and s3:GetObject against the object ARN. + - Sid: S3WorkloadReadWrite Effect: Allow Action: - s3:GetObject @@ -216,24 +471,77 @@ Resources: - s3:GetObjectTagging - s3:PutObjectTagging Resource: - - !Sub "arn:aws:s3:::*-${AWS::AccountId}" - - !Sub "arn:aws:s3:::*-${AWS::AccountId}/*" - # meal-order-manager ReportsBucket (non-AccountId suffix pattern) - - !Sub "arn:aws:s3:::meal-order-manager-reports-${AWS::AccountId}" - - !Sub "arn:aws:s3:::meal-order-manager-reports-${AWS::AccountId}/*" - - !Sub "arn:aws:s3:::meal-order-manager-form-${AWS::AccountId}" - - !Sub "arn:aws:s3:::meal-order-manager-form-${AWS::AccountId}/*" + - !Sub "arn:aws:s3:::meal-order-manager-*-${AWS::AccountId}" + - !Sub "arn:aws:s3:::meal-order-manager-*-${AWS::AccountId}/*" - # ── Secrets Manager (all stacks) ────────────────────────────────── + # ── S3 — read-only access (payments-dashboard) ────────────────────── + # payments-dashboard's enumerated need is s3:GetObject only, so a + # compromised payments function cannot delete payroll-email evidence. + # Its buckets carry the org-wide seahaven- prefix rather than a + # payments- one (seahaven-payments-csv-*, seahaven-payments-boa-raw-*, + # seahaven-payroll-emails-*), which is why two prefixes are needed here + # and why bucket names cannot be derived from stack names. + - Sid: S3WorkloadReadOnly + Effect: Allow + Action: + - s3:GetObject + - s3:GetObjectVersion + - s3:ListBucket + - s3:GetBucketLocation + Resource: + - !Sub "arn:aws:s3:::seahaven-payments-*-${AWS::AccountId}" + - !Sub "arn:aws:s3:::seahaven-payments-*-${AWS::AccountId}/*" + - !Sub "arn:aws:s3:::seahaven-payroll-emails-${AWS::AccountId}" + - !Sub "arn:aws:s3:::seahaven-payroll-emails-${AWS::AccountId}/*" + + # ── Secrets Manager — one / prefix per workload ────────────── + # Replaces secret:*, which in seahaven-prod today reads + # proposal-system/db-credentials, proposal-system/bedrock-user, + # procurement-ingest/web-ui-auth-token and workorder-ingest/shoc-webhook-hmac + # — the last of which is an HMAC SIGNING key, so the wildcard was a + # webhook-forgery primitive against the SHOC integration. + # The trailing * after each / is MANDATORY, not decorative: Secrets + # Manager appends a random 6-character suffix to every ARN, so an + # exact-name ARN never matches. Verified 2026-07-30 with + # iam simulate-custom-policy against a suffixed ARN. + # afi-api-key is a bare, unprefixed, account-root secret that predates the + # / convention (afi-backup-monitor's OTHER secret, + # afi-backup-monitor/slack-webhook-url, is correctly prefixed). It is + # listed explicitly because omitting it would encode a KNOWN-WRONG scope + # that fails silently months from now at migration. MIGRATION OBLIGATION: + # afi-backup-monitor's migration PR renames it to + # afi-backup-monitor/api-key and deletes this line. + # secretsmanager:ListSecrets and BatchGetSecretValue are deliberately + # ABSENT and must stay absent — AWS authorises both against "*" regardless + # of any resource list, so adding either would silently reinstate + # account-wide read. (Precedent: BatchGetSecretValue synthed clean, passed + # review, then AccessDenied in production and crash-looped open-swe.) - Sid: SecretsManager Effect: Allow Action: - secretsmanager:GetSecretValue - secretsmanager:DescribeSecret Resource: - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:*" + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afterhours-shift-manager/*" + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-backup-monitor/*" + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-api-key-*" + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:front-integrations/*" + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:meal-order-manager/*" + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:payments-dashboard/*" - # ── SSM Parameter Store (meal-order-manager, afterhours) ────────── + # ── SSM Parameter Store (meal-order-manager, afterhours) ──────────── + # Replaces parameter/*, which in seahaven-prod reads + # /procurement-api/custom-domain/certificate-arn and + # /seahaven/dynamodb/cmk-arn. These are exactly the two hierarchies the + # statement's own heading already claimed to serve. + # The bare-path entries alongside the /* entries are REQUIRED, not + # duplicates: ssm:GetParametersByPath authorises against the PATH ARN, + # not the leaf, so a /*-only grant can deny the recursive read. + # Note the ARN form drops the parameter's leading slash. + # /3cx-scheduler/* is deliberately EXCLUDED: ownership between + # afterhours-shift-manager and the retired standalone 3CX ring-group + # scheduler is unresolved, and inventing a scope is not permitted. + # Resolve at afterhours' migration and widen then if it is genuinely ours. - Sid: SSMParameterRead Effect: Allow Action: @@ -241,9 +549,23 @@ Resources: - ssm:GetParameters - ssm:GetParametersByPath Resource: - - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/*" + - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/afterhours-shift-manager" + - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/afterhours-shift-manager/*" + - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/meal-order-manager" + - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/meal-order-manager/*" - # ── SQS (payments-dashboard batch queues) ───────────────────────── + # ── SQS (payments-dashboard batch queues + async DLQs) ────────────── + # payments-dashboard is the only enumerated stack with a queue + # dependency, and all of its live queues are payments-prefixed. Replacing + # :* costs four characters and removes sqs:ReceiveMessage / + # sqs:DeleteMessage on proposal-system-jobs, workorder-shoc-emitter-failures + # and workorder-shoc-emitter-rejected, where the wildcard was a silent + # message-drain (data-loss) primitive against another tenant's pipeline. + # Verified denied 2026-07-30 with iam simulate-custom-policy. + # A prefix rather than four literals is deliberate: SAM auto-names async + # DLQs from the function name, so a new payments-* function's DLQ is + # covered without a boundary edit. + # sqs:ListQueues is absent and must stay absent (authorised against "*"). - Sid: SQS Effect: Allow Action: @@ -254,35 +576,89 @@ Resources: - sqs:GetQueueUrl - sqs:ChangeMessageVisibility Resource: - - !Sub "arn:aws:sqs:us-east-1:${AWS::AccountId}:*" + - !Sub "arn:aws:sqs:us-east-1:${AWS::AccountId}:payments-*" - # ── Lambda invocation (payments, meal-order inter-function calls) ── + # ── Lambda invocation — HIGHEST-LEVERAGE FIX IN THIS CHANGE ───────── + # function:* was a BOUNDARY-ESCAPE primitive, not merely lateral + # movement: an invoked function executes under ITS OWN execution role, + # and every non-SAM function in seahaven-prod (proposal-system-*, + # procurement-api, workorder-*, po-*) is CDK-deployed and carries NO + # permissions boundary at all. A bounded SAM Lambda could therefore reach, + # by proxy, capability this ceiling exists to deny. Verified 2026-07-30 + # with iam simulate-custom-policy: proposal-system-api is now denied, + # payments-expenseProcessor still allowed. + # Scoped to the three workloads with enumerated inter-function calls: + # payments-dashboard (ExpenseReceiver -> ExpenseProcessor), + # meal-order-manager (submit-order -> slack-notifier, close-form -> + # aggregate-orders, plus AdminAuthorizerInvokeRole), and + # afterhours-shift-manager, whose boundary-carrying ReleaseNotifyInvokeRole + # and HolidaySchedulerExecutionRole exist solely to invoke. + # front-integrations and afi-backup-monitor have no enumerated invoke need + # and are deliberately absent. Prefixes (not literal function ARNs) are + # used because they also match the : / : qualified form. - Sid: LambdaInvoke Effect: Allow Action: - lambda:InvokeFunction Resource: - - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:*" + - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:afterhours-*" + - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:meal-order-manager-*" + - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:payments-*" - # ── SES (afterhours weekly-post, meal-order email-report) ────────── + # ── SES — pinned to account inventory, NOT per-workload scoping ───── + # Labelled honestly: an SES identity is a shared DOMAIN, so no + # per-workload prefix exists to scope to. These are seahaven-prod's two + # verified identities (re-verified live 2026-07-30); seahaven-dev has + # none, so both entries are simply inert there. The win is bounded but + # real: identity/* would let the SAM fleet send as ANY identity ever added + # to the account, including a customer or partner domain, from a + # legitimately-authenticated sender. This cannot break anything that would + # otherwise work — sending from an unverified identity fails with + # MessageRejected regardless of IAM — so the failure mode is LOUD. + # configuration-set/* is DROPPED: zero configuration sets exist in prod or + # dev and no enumerated stack uses one. + # MIGRATION-BLOCKING QUESTION: mgmt additionally has seahavenind.com, + # apfacilities.org, adam@seahaven.com and payroll@seahaven.com verified; + # prod does NOT. Each migrating stack must confirm its actual Source + # address, verify that domain in the target account, and add the identity + # ARN here in the same PR. + # A ses:FromAddress condition would be stronger but is not available: the + # permission-source block names payroll@seahavenind.com, which is not a + # verified identity in prod — writing that condition would invent a scope. - Sid: SES Effect: Allow Action: - ses:SendEmail - ses:SendRawEmail Resource: - - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/*" - - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:configuration-set/*" + - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/int.seahaven.com" + - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/seahaven.com" - # ── KMS (CMK-encrypted resources) ───────────────────────────────── - # Required for Lambda functions that read/write CMK-encrypted AWS - # resources. Verified live state: - # - PaymentsDashboard DynamoDB table: CMK key/0b660af3 (KMS:ENABLED) - # - payments-dashboard CloudWatch log groups: CMK key/b748750c - # Secrets Manager + SQS queues in these stacks use AWS-managed keys - # (aws/secretsmanager, aws/sqs) which do not require explicit kms:* - # actions in the execution role policy. The CMK keys are scoped to - # this account to prevent cross-account KMS calls. + # ── KMS — key/* RETAINED, scoped by condition instead ─────────────── + # KMS is the one high-value data plane that CANNOT be scoped by resource + # name: key ARNs carry UUID key ids, not workload names, and alias ARNs + # are not valid in a Resource for these actions (alias scoping needs a + # kms:RequestAlias condition). The two key ids named in this statement's + # previous comment (key/0b660af3, key/b748750c) are MGMT keys that do not + # exist in seahaven-prod or seahaven-dev; hardcoding them — or prod's + # three live CMKs — would encode one account's inventory into a template + # shared by two, and would break on any key replacement. + # So Resource stays key/* and the scope is derived from kms:ViaService: + # the fleet may use a CMK ONLY as part of a request one of these services + # makes on its behalf. Decrypting a DynamoDB item or an S3 object still + # works; a direct kms:Decrypt on arbitrary ciphertext lifted from anywhere + # in the account is denied — the actual escalation path key/* opened. + # Because those services are themselves prefix-scoped above, the effective + # KMS scope INHERITS the per-workload scoping for free, with no key ids + # and no per-account parameterisation. + # logs.us-east-1.amazonaws.com is included as belt-and-braces: CloudWatch + # Logs is believed to decrypt log-group CMKs under its own service grant + # rather than the execution role's credentials, which would make this entry + # a no-op — but a KMS denial on the logging path would be SILENT, and the + # entry cannot grant anything meaningful on its own, so the insurance is + # bought deliberately. + # If a specific key must ever be named, use a kms:RequestAlias condition — + # never a literal key id. - Sid: KMS Effect: Allow Action: @@ -291,6 +667,14 @@ Resources: - kms:DescribeKey Resource: - !Sub "arn:aws:kms:us-east-1:${AWS::AccountId}:key/*" + Condition: + StringEquals: + kms:ViaService: + - dynamodb.us-east-1.amazonaws.com + - logs.us-east-1.amazonaws.com + - s3.us-east-1.amazonaws.com + - secretsmanager.us-east-1.amazonaws.com + - sqs.us-east-1.amazonaws.com # --------------------------------------------------------------------------- # Shared CloudFormation execution role (SAM stacks) — INFRA-97 scoped From b264f74f01f61478c25e377e77b1781e04569a71 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 18:03:40 -0400 Subject: [PATCH 2/8] fix(iam): correct two boundary-scoping defects found in review Post-implementation verification of the INFRA-186 prod/dev scoping found two functional defects that would have denied permissions the migrating stacks actually need. Neither is live today (prod/dev boundary usage is 0), but both would have surfaced as AccessDenied at first migration. - KMS: the ViaService list omitted ssm., while SSMParameterRead in the same policy grants ssm:GetParameter*. A SecureString read decrypts via the SSM service principal, so the boundary denied reads it also granted. - S3: payments-dashboard was classified read-only from the template's own permission-source comment, but that enumeration is incomplete -- the real stack grants s3:PutObject on BoaRawBucket (template.yaml:272, 1098-1099). Write is now allowed on seahaven-payments-boa-raw-* only; payroll-emails and payments-csv stay read-only, preserving the evidence-deletion protection. The seahaven-payments-* wildcard is replaced by the three literal bucket names, verified against payments-dashboard/template.yaml. Not changed: SES configuration-set/*. Review claimed dropping it rested on a false premise; verified live -- prod and dev both have ZERO configuration sets and member-baseline-stack.ts:44 excludes SES monitoring. The drop is correct. README: the 'substrate changes must edit both files' rule is now false for the boundary specifically, and said so uniformly. Corrected to distinguish the deliberately divergent boundary from the still-at-parity substrate resources. --- README.md | 21 +++++++--- .../deploy-substrate.template.yaml | 42 +++++++++++++++---- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index d8f168c..627f686 100644 --- a/README.md +++ b/README.md @@ -133,11 +133,22 @@ account-level deploy plumbing: only for an account that does not already have one — one provider per URL per account). -The template is a verbatim extraction of the substrate section of -`Sea-Haven-Industries/.github/oidc-deploy-roles.yaml` (see the provenance -header in the template — mgmt's copy remains source of truth for 328440206208 -until its stacks migrate out; substrate changes while both are live must edit -both files). Per-repo `githubdeploy-*` deploy roles are deliberately NOT part +The template began as a verbatim extraction of the substrate section of +`Sea-Haven-Industries/.github/oidc-deploy-roles.yaml`, which remains the source +of truth for mgmt (328440206208) until its stacks migrate out. + +**The two copies are no longer at parity, and the old "edit both files" rule no +longer applies uniformly.** Under INFRA-186, `seahaven-lambda-execution-boundary` +in *this* copy was scoped to per-workload prefixes for prod and dev (where +boundary usage was 0, so no live Lambda could break), while mgmt's copy keeps +the account-wide wildcards pending its own separately validated rollout across +26 live boundary-carrying roles. So: **the boundary resource is deliberately +divergent**; every *other* substrate resource (`github-cfn-execution-role`, +`seahaven-cfn-exec-iam-management`) is still expected to change in both files +together. The template's provenance header records which is which — read it +before assuming either parity or divergence. + +Per-repo `githubdeploy-*` deploy roles are deliberately NOT part of the substrate — they are provisioned per repo at migration/onboarding time so an account never carries trust relationships for repos that do not deploy to it. diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index e8a59d3..66bec29 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -474,13 +474,30 @@ Resources: - !Sub "arn:aws:s3:::meal-order-manager-*-${AWS::AccountId}" - !Sub "arn:aws:s3:::meal-order-manager-*-${AWS::AccountId}/*" - # ── S3 — read-only access (payments-dashboard) ────────────────────── - # payments-dashboard's enumerated need is s3:GetObject only, so a - # compromised payments function cannot delete payroll-email evidence. - # Its buckets carry the org-wide seahaven- prefix rather than a - # payments- one (seahaven-payments-csv-*, seahaven-payments-boa-raw-*, - # seahaven-payroll-emails-*), which is why two prefixes are needed here - # and why bucket names cannot be derived from stack names. + # ── S3 — payments-dashboard ───────────────────────────────────────── + # Split by direction, but note the source of truth: the template + # comment above enumerates "S3 GetObject" for payments-dashboard, and + # that enumeration is INCOMPLETE. Verified against the real stack + # 2026-07-30 — payments-dashboard/template.yaml grants s3:PutObject on + # BoaRawBucket (lines 272 and 1098-1099), so the fetchBoaTransactions + # path writes raw BoA payloads. A read-only grant here would deny that + # write at migration time. Write is therefore allowed on the boa-raw + # bucket ONLY; payroll-emails and payments-csv stay read-only, so a + # compromised payments function still cannot delete payroll evidence. + # Buckets carry the org-wide seahaven- prefix rather than a payments- + # one, which is why bucket names cannot be derived from stack names. + - Sid: S3PaymentsBoaRawWrite + Effect: Allow + Action: + - s3:GetObject + - s3:PutObject + - s3:GetObjectVersion + - s3:ListBucket + - s3:GetBucketLocation + Resource: + - !Sub "arn:aws:s3:::seahaven-payments-boa-raw-${AWS::AccountId}" + - !Sub "arn:aws:s3:::seahaven-payments-boa-raw-${AWS::AccountId}/*" + - Sid: S3WorkloadReadOnly Effect: Allow Action: @@ -489,8 +506,8 @@ Resources: - s3:ListBucket - s3:GetBucketLocation Resource: - - !Sub "arn:aws:s3:::seahaven-payments-*-${AWS::AccountId}" - - !Sub "arn:aws:s3:::seahaven-payments-*-${AWS::AccountId}/*" + - !Sub "arn:aws:s3:::seahaven-payments-csv-${AWS::AccountId}" + - !Sub "arn:aws:s3:::seahaven-payments-csv-${AWS::AccountId}/*" - !Sub "arn:aws:s3:::seahaven-payroll-emails-${AWS::AccountId}" - !Sub "arn:aws:s3:::seahaven-payroll-emails-${AWS::AccountId}/*" @@ -674,6 +691,13 @@ Resources: - logs.us-east-1.amazonaws.com - s3.us-east-1.amazonaws.com - secretsmanager.us-east-1.amazonaws.com + # REQUIRED for parity with SSMParameterRead above: a + # SecureString parameter decrypts via the SSM service + # principal, so omitting this denies reads that this same + # policy grants — a self-inconsistency caught by the + # 2026-07-30 review. Any ssm:GetParameter* grant in this + # boundary must keep this entry. + - ssm.us-east-1.amazonaws.com - sqs.us-east-1.amazonaws.com # --------------------------------------------------------------------------- From 59852eff3415decd26d9e7219ef1af9e405bc3f9 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 18:17:16 -0400 Subject: [PATCH 3/8] fix(iam): restore four permissions the boundary would have denied at migration /sh-security-review (6 detectors + verifier) found four HIGH findings, all the same defect class: the template's permission-source comment block was used as the sanctioned scope source, but it is an incomplete and in places invented secondary record. Each was verified against the real stack template before fixing. None is live today (prod/dev boundary usage is 0); all four would have been AccessDenied at first migration, three of them SILENTLY. - SES configuration-set/seahaven-email-events restored. afterhours-shift-manager template.yaml:178-181 grants it with an in-repo comment stating the send is denied without it. An earlier revision dropped it after checking whether any config set exists in prod/dev today (none do) -- the wrong test. The right question is whether an enumerated stack's own IAM policy names it. - SES identity/seahavenind.com added. meal-order-manager's SenderEmail defaults to adam@seahavenind.com (template.yaml:20-22) and email_report sends with it. The prior 'unverified identity fails loudly anyway' argument holds only until the migration verifies the domain, which the migration procedure requires. - scheduler:Create/Delete/GetSchedule + iam:PassRole (scheduler.amazonaws.com only) added. afterhours template.yaml:110-120 needs both; the block omitted them entirely. Failure is silent -- app.py wraps create_schedule in a bare except, so the Slack command reports success and no schedule exists. - secret:afi-slack-webhook-* added. The block named 'afi-backup-monitor/slack-webhook-url', which does not exist; both afi secret ARNs are deploy parameters, so the real names live only in that repo's README:48-49 (afi-api-key, afi-slack-webhook). Also corrected, all comment-only: - The permission-source block itself, at each of the four points it was wrong, with the correction and its evidence recorded inline. - The false claim that SAM auto-names async DLQs (it does not -- all four payments queues are hand-written with explicit QueueNames). Replaced with the real invariant: any queue a boundary-carrying function sends to must be payments-* or the boundary widens in the same PR; a denied destination write is silent. - SIZE BUDGET: was 13 statements / 4,060 chars, actually 16 / 5,457 after these fixes. Headroom is 687 chars, roughly ONE more workload -- not the five the header claimed. Flagged per-workload boundaries as the realistic next move. Verified unchanged: logical id LambdaExecutionBoundary and ManagedPolicyName seahaven-lambda-execution-boundary, so all eight pinning conditions across both guardrail policies still resolve. --- .../deploy-substrate.template.yaml | 120 +++++++++++++++--- 1 file changed, 104 insertions(+), 16 deletions(-) diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index 66bec29..0a569f9 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -114,12 +114,20 @@ Description: >- # correctly left untouched. # # SIZE BUDGET: an attached managed policy document is capped at 6,144 characters -# (whitespace excluded). LambdaExecutionBoundary measures 4,060 characters across -# 13 statements as of 2026-07-30 (was 2,507 across 11 — TIGHTENING COSTS -# CHARACTERS). Measure before widening — -# len(json.dumps(doc,separators=(',',':'))) on the synthesized PolicyDocument with -# ${AWS::AccountId} resolved. A typical new workload costs ~450 characters, so -# roughly five more fit. CRITICAL: unlike the 2026-07-27 inline-limit incident, +# (whitespace excluded). LambdaExecutionBoundary measures 5,457 characters across +# 16 statements as of 2026-07-30 (was 2,507 across 11 before this scoping pass — +# TIGHTENING COSTS CHARACTERS, and the review corrections cost more). 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 once already inside a single branch). +# +# ⚠ HEADROOM IS 687 CHARACTERS — roughly ONE more workload at ~450 each, NOT the +# "five" an earlier revision of this header claimed. The next stack to migrate is +# likely to exhaust it. Read the note below before assuming there is room: the +# realistic next move is per-workload boundaries +# (seahaven-lambda-execution-boundary-), which is also the durable fix +# for the shared-ceiling residual documented in the SCOPING RULE. +# CRITICAL: unlike the 2026-07-27 inline-limit incident, # there is NO restructure available when this cap is reached — a role has exactly # ONE permissions boundary, so statements cannot be spilled into a second attached # managed policy. At the cap the only levers are prefix consolidation and dropping @@ -244,9 +252,18 @@ Resources: # afterhours-shift-manager (functions: afterhours-*, 6 live) # - DynamoDB CRUD (afterhours-shifts table) # - secretsmanager:GetSecretValue (afterhours-shift-manager/*) - # - ssm:GetParameter (/afterhours-shift-manager/*: channel-id, - # slack-bot-token, slack-signing-secret) - # - ses:SendEmail (SES identity) + # - ses:SendEmail on the identity AND on + # configuration-set/seahaven-email-events (template.yaml:178-181 — the + # send is DENIED without the config-set ARN when the identity has a + # default configuration set) + # - scheduler:CreateSchedule/DeleteSchedule/GetSchedule on + # schedule/default/holiday-* + iam:PassRole to scheduler.amazonaws.com + # (template.yaml:110-120). CORRECTED 2026-07-30: this block previously + # omitted both, and the omission is SILENT at runtime (bare except). + # - NO ssm. CORRECTED 2026-07-30: this block previously credited + # ssm:GetParameter to this stack; `grep -c 'ssm:' template.yaml` = 0. + # Its slack tokens come from Secrets Manager and the channel id from a + # CloudFormation parameter. # - lambda:InvokeFunction (ReleaseNotifyInvokeRole, # HolidaySchedulerExecutionRole — these two carry the boundary and need # ONLY this action) @@ -256,8 +273,12 @@ Resources: # # payments-dashboard (functions: payments-*) # - DynamoDB CRUD / Read (PaymentsDashboard table — legacy PascalCase) - # - S3 GetObject ONLY (seahaven-payments-csv-*, seahaven-payments-boa-raw-*, - # seahaven-payroll-emails-*) — no write intent enumerated + # - S3 GetObject on seahaven-payments-csv-* and seahaven-payroll-emails-*; + # GetObject + PutObject on seahaven-payments-boa-raw-* + # (template.yaml:272 and 1097-1099, fetchBoaTransactions raw archive). + # CORRECTED 2026-07-30: this block previously said "GetObject ONLY — + # no write intent enumerated", which was false and would have denied + # the raw-archive write at migration. # - secretsmanager:GetSecretValue (payments-dashboard/*) # - sqs Send/Receive/Delete etc. (payments-payroll-batch + DLQs) # - lambda:InvokeFunction (ExpenseReceiver -> ExpenseProcessor) @@ -282,8 +303,12 @@ Resources: # - no S3 / SQS / SSM / SES / KMS / VPC # # afi-backup-monitor (functions: afi-*) - # - secretsmanager:GetSecretValue (afi-backup-monitor/slack-webhook-url AND - # the bare, unprefixed afi-api-key — see the SecretsManager statement) + # - secretsmanager:GetSecretValue on TWO bare, unprefixed secrets: + # afi-api-key and afi-slack-webhook. CORRECTED 2026-07-30: this block + # previously named "afi-backup-monitor/slack-webhook-url", which does + # not exist. Both ARNs are deploy PARAMETERS in that stack + # (AfiApiKeySecretArn / SlackWebhookSecretArn), so no name is + # discoverable from its template — the names are in its README:48-49. # - CloudWatch Logs # - nothing else # @@ -542,6 +567,14 @@ Resources: - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afterhours-shift-manager/*" - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-backup-monitor/*" - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-api-key-*" + # CORRECTION (2026-07-30 review): the permission-source block + # named this secret "afi-backup-monitor/slack-webhook-url", which + # does not exist. afi-backup-monitor/template.yaml takes both + # secret ARNs as deploy PARAMETERS, so no name is discoverable + # from the template; the real names are in that repo's README + # (afi-api-key and afi-slack-webhook, both bare/unprefixed). Same + # rename-at-migration obligation as afi-api-key applies. + - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-slack-webhook-*" - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:front-integrations/*" - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:meal-order-manager/*" - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:payments-dashboard/*" @@ -579,9 +612,17 @@ Resources: # and workorder-shoc-emitter-rejected, where the wildcard was a silent # message-drain (data-loss) primitive against another tenant's pipeline. # Verified denied 2026-07-30 with iam simulate-custom-policy. - # A prefix rather than four literals is deliberate: SAM auto-names async - # DLQs from the function name, so a new payments-* function's DLQ is - # covered without a boundary edit. + # A prefix rather than four literals is deliberate, BUT the original + # justification for it was wrong and is corrected here: SAM does NOT + # auto-create or auto-name async DLQs — all four payments queues are + # hand-written AWS::SQS::Queue resources with explicit QueueNames, and + # OnFailure destinations take an explicit ARN. The real invariant is + # therefore a naming rule, not a framework behaviour: ANY queue a + # boundary-carrying function sends to — including async OnFailure + # destinations and DeadLetterQueue targets — must be named payments-*, + # or the boundary must be widened in the SAME PR. A denied destination + # write is SILENT: the async event is discarded with no caller to + # error, no Errors datapoint and no DLQ contents. # sqs:ListQueues is absent and must stay absent (authorised against "*"). - Sid: SQS Effect: Allow @@ -642,6 +683,22 @@ Resources: # A ses:FromAddress condition would be stronger but is not available: the # permission-source block names payroll@seahavenind.com, which is not a # verified identity in prod — writing that condition would invent a scope. + # CORRECTION (2026-07-30 review): an earlier revision DROPPED + # configuration-set/* on the reasoning that zero configuration sets + # exist in prod or dev today. That test was the wrong one. SES + # authorizes SendEmail against the CONFIGURATION-SET resource in + # addition to the identity whenever the identity has a default + # configuration set, and afterhours-shift-manager/template.yaml:178-181 + # grants exactly that ARN with an in-repo comment recording that + # omitting it DENIES the send. The correct question is not "does the + # resource exist in the target account yet" but "does an enumerated + # stack's own IAM policy name it". seahavenind.com is listed because + # meal-order-manager's SenderEmail parameter defaults to + # adam@seahavenind.com (meal-order-manager/template.yaml:20-22) and its + # email_report handler sends with that Source; the earlier "sending + # from an unverified identity fails loudly anyway" argument only holds + # until the migration verifies the domain, which the migration + # procedure itself requires. - Sid: SES Effect: Allow Action: @@ -650,6 +707,37 @@ Resources: Resource: - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/int.seahaven.com" - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/seahaven.com" + - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/seahavenind.com" + - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:configuration-set/seahaven-email-events" + + # ── EventBridge Scheduler (afterhours-shift-manager holiday routing) ─ + # Omitted from the permission-source block entirely, which the block's + # "verified live" header did not catch. afterhours-shift-manager + # template.yaml:110-120 grants these three scheduler actions plus + # iam:PassRole on its holiday-scheduler execution role. Without them + # the failure is SILENT: src/slack-bot/app.py wraps create_schedule in + # a bare `except Exception`, so the Slack command returns success, the + # holiday record is written with no schedule, and no alarm fires. + - Sid: EventBridgeScheduler + Effect: Allow + Action: + - scheduler:CreateSchedule + - scheduler:DeleteSchedule + - scheduler:GetSchedule + Resource: + - !Sub "arn:aws:scheduler:us-east-1:${AWS::AccountId}:schedule/default/holiday-*" + + # PassRole is confined to the scheduler service principal, so this + # cannot be used to hand a role to Lambda or any other service. + - Sid: SchedulerPassRole + Effect: Allow + Action: + - iam:PassRole + Resource: + - !Sub "arn:aws:iam::${AWS::AccountId}:role/afterhours-shift-manager-*" + Condition: + StringEquals: + "iam:PassedToService": "scheduler.amazonaws.com" # ── KMS — key/* RETAINED, scoped by condition instead ─────────────── # KMS is the one high-value data plane that CANNOT be scoped by resource From 32f06e74eb6a5a58811fc9ccac65b234ce08ab67 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 19:07:09 -0400 Subject: [PATCH 4/8] refactor(iam): reduce boundary to the fleet-wide floor, defer per-workload scope Adam's call after review: the security win of INFRA-186 comes from DELETING the account-wide wildcards, not from enumerating replacements. Per-workload prefixes add no security -- they only keep a workload functional -- and widening a boundary is the safe direction (adding a resource never breaks a running Lambda; only tightening does). So the per-workload scope moves to each migration PR, which has the stack's real template open in front of it. Removed all nine per-workload data-plane statements (DynamoDB, S3 x3, Secrets Manager, SSM, SQS, Lambda invoke, SES, scheduler x2, KMS). Kept the fleet-wide floor: CloudWatchLogsWrite (/aws/lambda*), CloudWatchLogsDescribe, XRay, Ec2Eni -- the statements every Lambda needs regardless of workload, and also the silent-failure classes, which is why they belong in the floor. KMS dropped entirely: both accounts have ZERO CMK-encrypted log groups (verified). A workload bringing a CMK adds the statement plus the matching kms:ViaService principal in its own PR. Why not keep the enumeration: it required predicting five stacks' needs from this file's own permission-source comment block, and /sh-security-review found SIX errors in the result -- three silent. The block is a secondary record, not an authority. Deriving scope per-migration from the owning template removes the whole error class. Effect on the security objective: unchanged. secret:*, table/*, function:*, sqs:* and the s3:::*- name-suffix filter are gone either way, so the amplifier is closed identically. Size: 5,457 chars / 16 statements -> 703 / 4. Headroom 687 -> 5,441, so the cap stops being a forcing function. Header, SCOPING RULE and WIDENING PATH all updated to match; widening path now leads with 'read the stack's own template', names the silent-failure classes to check, and moves the version-budget check to a precondition instead of a trailing step. Verified unchanged: logical id and ManagedPolicyName, so all eight pinning conditions across both guardrail policies still resolve. Both accounts synth identically at 703 chars. --- .../deploy-substrate.template.yaml | 505 ++++-------------- 1 file changed, 108 insertions(+), 397 deletions(-) diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index 0a569f9..9a91de2 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -53,17 +53,21 @@ Description: >- # by copying mgmt's statements back over these, or vice versa; the divergence is # load-bearing. A future mechanical drift check WILL read it as drift — it is not. # -# 1. WHAT DIVERGED. Every scopable Resource pattern in LambdaExecutionBoundary -# was re-scoped from account-wide wildcards (table/*, table/*/index/*, -# secret:*, parameter/*, sqs :*, function:*, ses identity/* + -# configuration-set/*, and an s3:::*- pattern that was a bare -# name-suffix filter rather than an ownership check) to per-workload -# prefixes drawn from the permission-source block above. The three genuinely -# unscopable statements (CloudWatchLogsDescribe, XRay, Ec2Eni) keep -# Resource "*" with written justification on each. CloudWatch Logs was split -# into a scoped write half and a wildcard describe half. KMS keeps key/* and -# is scoped by a kms:ViaService condition instead, because key ARNs carry -# UUID key ids that cannot be prefix-scoped. +# 1. WHAT DIVERGED. Every per-workload data-plane statement was REMOVED from +# LambdaExecutionBoundary in this file, leaving only the fleet-wide floor: +# CloudWatchLogsWrite (scoped to /aws/lambda*), CloudWatchLogsDescribe, +# XRay and Ec2Eni. The account-wide wildcards mgmt still carries — table/*, +# table/*/index/*, secret:*, parameter/*, sqs :*, function:*, ses +# identity/* + configuration-set/*, kms key/*, and an s3:::*- +# pattern that was a bare name-suffix filter rather than an ownership +# check — are simply GONE here rather than re-scoped. +# The security win is the deletion: it is what closes the amplifier whereby +# a principal able to write an inline policy onto a boundary-carrying role +# could read every secret in the account. Per-workload prefixes add no +# security — they only keep a workload functional — so they are added by +# each migration PR, from that stack's own template, when the stack +# actually lands. See the note on the boundary resource for the full +# rationale and the six errors that the pre-loaded approach produced. # # 2. WHY MGMT'S RATIONALE IS LEGITIMATE THERE. The superset framing this file # used to carry ("being slightly broad is the correct trade-off; a boundary @@ -114,19 +118,21 @@ Description: >- # correctly left untouched. # # SIZE BUDGET: an attached managed policy document is capped at 6,144 characters -# (whitespace excluded). LambdaExecutionBoundary measures 5,457 characters across -# 16 statements as of 2026-07-30 (was 2,507 across 11 before this scoping pass — -# TIGHTENING COSTS CHARACTERS, and the review corrections cost more). Measure -# before widening — len(json.dumps(doc,separators=(',',':'))) on the synthesized +# (whitespace excluded). LambdaExecutionBoundary measures 703 characters across +# 4 statements as of 2026-07-30 — 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 once already inside a single branch). +# the same edit (they went stale twice inside this branch alone). # -# ⚠ HEADROOM IS 687 CHARACTERS — roughly ONE more workload at ~450 each, NOT the -# "five" an earlier revision of this header claimed. The next stack to migrate is -# likely to exhaust it. Read the note below before assuming there is room: the -# realistic next move is per-workload boundaries -# (seahaven-lambda-execution-boundary-), which is also the durable fix -# for the shared-ceiling residual documented in the SCOPING RULE. +# Headroom is 5,441 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 +# migrated. Deferring per-workload scope to each migration PR (see the note on +# the boundary itself) removed that pressure entirely. If the budget tightens +# again as workloads land, the end-state fix is per-workload boundaries +# (seahaven-lambda-execution-boundary-), which also resolves the +# shared-ceiling residual — tracked as its own ticket, do not improvise it. # CRITICAL: unlike the 2026-07-27 inline-limit incident, # there is NO restructure available when this cap is reached — a role has exactly # ONE permissions boundary, so statements cannot be spilled into a second attached @@ -181,8 +187,11 @@ Resources: # intersection of the role's own policies and this boundary, so a misconfigured # SAM role can never exceed what is listed here. # - # SCOPING RULE (INFRA-186, 2026-07-30). This boundary is a CLOSED ENUMERATION - # of per-workload resource prefixes. It was previously a deliberate SUPERSET + # SCOPING RULE (INFRA-186, 2026-07-30). This boundary is the FLEET-WIDE FLOOR + # ONLY: what every Lambda execution role needs regardless of workload. It + # carries NO per-workload data-plane statements — each migrating stack adds its + # own, from its own template, in its own PR (see the note on the boundary + # resource and the WIDENING PATH below). It was previously a deliberate SUPERSET # with account-wide wildcards (table/*, secret:*, sqs :*, function:*, # parameter/*, and an s3:::*- pattern that was a name-suffix filter, # not an ownership check). That trade-off was made when the only account @@ -209,21 +218,30 @@ Resources: # 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: - # 1. Add the workload's prefixes to the relevant per-service Resource lists - # above — never add a new per-workload statement (a duplicated action list - # costs ~250 characters for zero new actions; an extra ARN costs ~60). - # Add its entry to the permission-source block below in the same edit: that - # block is the sanctioned scope source, and a prefix added without one will - # be "reconciled" away later. - # 2. Measure. See SIZE BUDGET in the header. There is NO escape hatch. - # 3. Both review gates run and neither discharges the other: the GPT-4.1 - # cross-family review against the real diff, and /sh-security-review - # (IaC/IAM is on the mandatory surface). CLI down = review outstanding. - # 4. Merge and let CI deploy deploy-substrate-prod / deploy-substrate-dev to - # UPDATE_COMPLETE, THEN deploy the workload stack. - # 5. Check the managed-policy VERSION budget first: max 5 versions, both + # 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. + # 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: + # the /sh-security-review pass on 2026-07-30 found SIX places where it was + # incomplete or simply invented a resource name, three of which would have + # failed silently. Update that block in the same edit with what you find. + # 2. Add the workload's statements. Group by service so a second workload can + # extend a Resource list rather than duplicate an action list (~250 + # characters for zero new actions; an extra ARN costs ~60). Check for the + # 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. + # 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 + # (IaC/IAM is on the mandatory surface). CLI down = review outstanding. + # 5. Merge and let CI deploy deploy-substrate-prod / deploy-substrate-dev to + # UPDATE_COMPLETE, THEN deploy the workload stack. # ORDERING IS NOT ENFORCED BY CLOUDFORMATION AND THIS IS THE MOST IMPORTANT # SENTENCE HERE: the workload's deploy SUCCEEDS even against a stale boundary, # because seahaven-cfn-exec-iam-management's gate checks that the boundary ARN @@ -428,366 +446,59 @@ Resources: - ec2:DescribeVpcs Resource: "*" - # ── DynamoDB — one prefix per enumerated workload ─────────────────── - # Replaces table/* + table/*/index/*, which reached every table in the - # account. A trailing * on each workload prefix covers the base table - # AND its /index/* GSI ARNs in a single entry (IAM wildcards match "/"), - # verified 2026-07-30 with iam simulate-custom-policy against - # table/afterhours-shifts/index/gsi1 — so the separate table/*/index/* - # line is deleted, not replaced. Prod's CDK-owned tables (WorkOrders, - # WorkOrderComments, purchase-orders, verified-sites, pending-site-review) - # match none of these prefixes, which is the point. - # PaymentsDashboard is a legacy PascalCase table exempt from renaming - # under handbook naming-conventions.md; the kebab-case prefix is listed - # alongside it so a future rename cannot silently lock the stack out. - # Table names are NOT stack names (afterhours-shifts, front-sla-alerts) — - # scoping naively off stack name would deny at runtime. - - Sid: DynamoDB - Effect: Allow - Action: - - dynamodb:GetItem - - dynamodb:PutItem - - dynamodb:UpdateItem - - dynamodb:DeleteItem - - dynamodb:Query - - dynamodb:Scan - - dynamodb:BatchGetItem - - dynamodb:BatchWriteItem - - dynamodb:DescribeTable - - dynamodb:ConditionCheckItem - Resource: - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/afterhours-*" - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/front-*" - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/meal-order-manager-*" - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/PaymentsDashboard*" - - !Sub "arn:aws:dynamodb:us-east-1:${AWS::AccountId}:table/payments-dashboard-*" - - # ── S3 — write access (meal-order-manager only) ───────────────────── - # THE LARGEST SECURITY WIN IN INFRA-186. The removed - # arn:aws:s3:::*-${AWS::AccountId} was not a per-workload scope at all: - # S3 ARNs carry no account field, so it was a bare NAME-SUFFIX FILTER - # matching every bucket in the account whose name ends in the account id. - # In seahaven-prod today that was 8 of 9 buckets — all four - # proposal-system-*, both ingest-email buckets, AND the org's own - # seahaven-prod-config-* and seahaven-prod-vpc-flow-logs-* — with - # PutObject and DeleteObject. That is anti-forensics capability over the - # org's own security telemetry, handed to the SAM fleet's ceiling. - # Verified 2026-07-30 with iam simulate-custom-policy: the patterns below - # deny seahaven-prod-config-*, seahaven-prod-vpc-flow-logs-* and - # proposal-system-uploads-*, and allow meal-order-manager-reports-*. - # The four meal-order-manager-*-${AWS::AccountId} entries previously - # listed here were strictly redundant — every one ends in - and - # was already matched by the wildcard above them; their comment claiming a - # "non-AccountId suffix pattern" was contradicted by the ARNs beneath it. - # Split by DIRECTION of access: meal-order-manager is the only stack the - # permission-source block gives write intent to (ReportsBucket / - # FormBucket CRUD). Both the bucket and object ARN forms are listed in - # each statement because s3:ListBucket authorises against the bucket ARN - # and s3:GetObject against the object ARN. - - Sid: S3WorkloadReadWrite - Effect: Allow - Action: - - s3:GetObject - - s3:PutObject - - s3:DeleteObject - - s3:ListBucket - - s3:GetBucketLocation - - s3:GetObjectVersion - - s3:GetObjectTagging - - s3:PutObjectTagging - Resource: - - !Sub "arn:aws:s3:::meal-order-manager-*-${AWS::AccountId}" - - !Sub "arn:aws:s3:::meal-order-manager-*-${AWS::AccountId}/*" - - # ── S3 — payments-dashboard ───────────────────────────────────────── - # Split by direction, but note the source of truth: the template - # comment above enumerates "S3 GetObject" for payments-dashboard, and - # that enumeration is INCOMPLETE. Verified against the real stack - # 2026-07-30 — payments-dashboard/template.yaml grants s3:PutObject on - # BoaRawBucket (lines 272 and 1098-1099), so the fetchBoaTransactions - # path writes raw BoA payloads. A read-only grant here would deny that - # write at migration time. Write is therefore allowed on the boa-raw - # bucket ONLY; payroll-emails and payments-csv stay read-only, so a - # compromised payments function still cannot delete payroll evidence. - # Buckets carry the org-wide seahaven- prefix rather than a payments- - # one, which is why bucket names cannot be derived from stack names. - - Sid: S3PaymentsBoaRawWrite - Effect: Allow - Action: - - s3:GetObject - - s3:PutObject - - s3:GetObjectVersion - - s3:ListBucket - - s3:GetBucketLocation - Resource: - - !Sub "arn:aws:s3:::seahaven-payments-boa-raw-${AWS::AccountId}" - - !Sub "arn:aws:s3:::seahaven-payments-boa-raw-${AWS::AccountId}/*" - - - Sid: S3WorkloadReadOnly - Effect: Allow - Action: - - s3:GetObject - - s3:GetObjectVersion - - s3:ListBucket - - s3:GetBucketLocation - Resource: - - !Sub "arn:aws:s3:::seahaven-payments-csv-${AWS::AccountId}" - - !Sub "arn:aws:s3:::seahaven-payments-csv-${AWS::AccountId}/*" - - !Sub "arn:aws:s3:::seahaven-payroll-emails-${AWS::AccountId}" - - !Sub "arn:aws:s3:::seahaven-payroll-emails-${AWS::AccountId}/*" - - # ── Secrets Manager — one / prefix per workload ────────────── - # Replaces secret:*, which in seahaven-prod today reads - # proposal-system/db-credentials, proposal-system/bedrock-user, - # procurement-ingest/web-ui-auth-token and workorder-ingest/shoc-webhook-hmac - # — the last of which is an HMAC SIGNING key, so the wildcard was a - # webhook-forgery primitive against the SHOC integration. - # The trailing * after each / is MANDATORY, not decorative: Secrets - # Manager appends a random 6-character suffix to every ARN, so an - # exact-name ARN never matches. Verified 2026-07-30 with - # iam simulate-custom-policy against a suffixed ARN. - # afi-api-key is a bare, unprefixed, account-root secret that predates the - # / convention (afi-backup-monitor's OTHER secret, - # afi-backup-monitor/slack-webhook-url, is correctly prefixed). It is - # listed explicitly because omitting it would encode a KNOWN-WRONG scope - # that fails silently months from now at migration. MIGRATION OBLIGATION: - # afi-backup-monitor's migration PR renames it to - # afi-backup-monitor/api-key and deletes this line. - # secretsmanager:ListSecrets and BatchGetSecretValue are deliberately - # ABSENT and must stay absent — AWS authorises both against "*" regardless - # of any resource list, so adding either would silently reinstate - # account-wide read. (Precedent: BatchGetSecretValue synthed clean, passed - # review, then AccessDenied in production and crash-looped open-swe.) - - Sid: SecretsManager - Effect: Allow - Action: - - secretsmanager:GetSecretValue - - secretsmanager:DescribeSecret - Resource: - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afterhours-shift-manager/*" - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-backup-monitor/*" - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-api-key-*" - # CORRECTION (2026-07-30 review): the permission-source block - # named this secret "afi-backup-monitor/slack-webhook-url", which - # does not exist. afi-backup-monitor/template.yaml takes both - # secret ARNs as deploy PARAMETERS, so no name is discoverable - # from the template; the real names are in that repo's README - # (afi-api-key and afi-slack-webhook, both bare/unprefixed). Same - # rename-at-migration obligation as afi-api-key applies. - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:afi-slack-webhook-*" - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:front-integrations/*" - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:meal-order-manager/*" - - !Sub "arn:aws:secretsmanager:us-east-1:${AWS::AccountId}:secret:payments-dashboard/*" - - # ── SSM Parameter Store (meal-order-manager, afterhours) ──────────── - # Replaces parameter/*, which in seahaven-prod reads - # /procurement-api/custom-domain/certificate-arn and - # /seahaven/dynamodb/cmk-arn. These are exactly the two hierarchies the - # statement's own heading already claimed to serve. - # The bare-path entries alongside the /* entries are REQUIRED, not - # duplicates: ssm:GetParametersByPath authorises against the PATH ARN, - # not the leaf, so a /*-only grant can deny the recursive read. - # Note the ARN form drops the parameter's leading slash. - # /3cx-scheduler/* is deliberately EXCLUDED: ownership between - # afterhours-shift-manager and the retired standalone 3CX ring-group - # scheduler is unresolved, and inventing a scope is not permitted. - # Resolve at afterhours' migration and widen then if it is genuinely ours. - - Sid: SSMParameterRead - Effect: Allow - Action: - - ssm:GetParameter - - ssm:GetParameters - - ssm:GetParametersByPath - Resource: - - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/afterhours-shift-manager" - - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/afterhours-shift-manager/*" - - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/meal-order-manager" - - !Sub "arn:aws:ssm:us-east-1:${AWS::AccountId}:parameter/meal-order-manager/*" - - # ── SQS (payments-dashboard batch queues + async DLQs) ────────────── - # payments-dashboard is the only enumerated stack with a queue - # dependency, and all of its live queues are payments-prefixed. Replacing - # :* costs four characters and removes sqs:ReceiveMessage / - # sqs:DeleteMessage on proposal-system-jobs, workorder-shoc-emitter-failures - # and workorder-shoc-emitter-rejected, where the wildcard was a silent - # message-drain (data-loss) primitive against another tenant's pipeline. - # Verified denied 2026-07-30 with iam simulate-custom-policy. - # A prefix rather than four literals is deliberate, BUT the original - # justification for it was wrong and is corrected here: SAM does NOT - # auto-create or auto-name async DLQs — all four payments queues are - # hand-written AWS::SQS::Queue resources with explicit QueueNames, and - # OnFailure destinations take an explicit ARN. The real invariant is - # therefore a naming rule, not a framework behaviour: ANY queue a - # boundary-carrying function sends to — including async OnFailure - # destinations and DeadLetterQueue targets — must be named payments-*, - # or the boundary must be widened in the SAME PR. A denied destination - # write is SILENT: the async event is discarded with no caller to - # error, no Errors datapoint and no DLQ contents. - # sqs:ListQueues is absent and must stay absent (authorised against "*"). - - Sid: SQS - Effect: Allow - Action: - - sqs:SendMessage - - sqs:ReceiveMessage - - sqs:DeleteMessage - - sqs:GetQueueAttributes - - sqs:GetQueueUrl - - sqs:ChangeMessageVisibility - Resource: - - !Sub "arn:aws:sqs:us-east-1:${AWS::AccountId}:payments-*" - - # ── Lambda invocation — HIGHEST-LEVERAGE FIX IN THIS CHANGE ───────── - # function:* was a BOUNDARY-ESCAPE primitive, not merely lateral - # movement: an invoked function executes under ITS OWN execution role, - # and every non-SAM function in seahaven-prod (proposal-system-*, - # procurement-api, workorder-*, po-*) is CDK-deployed and carries NO - # permissions boundary at all. A bounded SAM Lambda could therefore reach, - # by proxy, capability this ceiling exists to deny. Verified 2026-07-30 - # with iam simulate-custom-policy: proposal-system-api is now denied, - # payments-expenseProcessor still allowed. - # Scoped to the three workloads with enumerated inter-function calls: - # payments-dashboard (ExpenseReceiver -> ExpenseProcessor), - # meal-order-manager (submit-order -> slack-notifier, close-form -> - # aggregate-orders, plus AdminAuthorizerInvokeRole), and - # afterhours-shift-manager, whose boundary-carrying ReleaseNotifyInvokeRole - # and HolidaySchedulerExecutionRole exist solely to invoke. - # front-integrations and afi-backup-monitor have no enumerated invoke need - # and are deliberately absent. Prefixes (not literal function ARNs) are - # used because they also match the : / : qualified form. - - Sid: LambdaInvoke - Effect: Allow - Action: - - lambda:InvokeFunction - Resource: - - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:afterhours-*" - - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:meal-order-manager-*" - - !Sub "arn:aws:lambda:us-east-1:${AWS::AccountId}:function:payments-*" - - # ── SES — pinned to account inventory, NOT per-workload scoping ───── - # Labelled honestly: an SES identity is a shared DOMAIN, so no - # per-workload prefix exists to scope to. These are seahaven-prod's two - # verified identities (re-verified live 2026-07-30); seahaven-dev has - # none, so both entries are simply inert there. The win is bounded but - # real: identity/* would let the SAM fleet send as ANY identity ever added - # to the account, including a customer or partner domain, from a - # legitimately-authenticated sender. This cannot break anything that would - # otherwise work — sending from an unverified identity fails with - # MessageRejected regardless of IAM — so the failure mode is LOUD. - # configuration-set/* is DROPPED: zero configuration sets exist in prod or - # dev and no enumerated stack uses one. - # MIGRATION-BLOCKING QUESTION: mgmt additionally has seahavenind.com, - # apfacilities.org, adam@seahaven.com and payroll@seahaven.com verified; - # prod does NOT. Each migrating stack must confirm its actual Source - # address, verify that domain in the target account, and add the identity - # ARN here in the same PR. - # A ses:FromAddress condition would be stronger but is not available: the - # permission-source block names payroll@seahavenind.com, which is not a - # verified identity in prod — writing that condition would invent a scope. - # CORRECTION (2026-07-30 review): an earlier revision DROPPED - # configuration-set/* on the reasoning that zero configuration sets - # exist in prod or dev today. That test was the wrong one. SES - # authorizes SendEmail against the CONFIGURATION-SET resource in - # addition to the identity whenever the identity has a default - # configuration set, and afterhours-shift-manager/template.yaml:178-181 - # grants exactly that ARN with an in-repo comment recording that - # omitting it DENIES the send. The correct question is not "does the - # resource exist in the target account yet" but "does an enumerated - # stack's own IAM policy name it". seahavenind.com is listed because - # meal-order-manager's SenderEmail parameter defaults to - # adam@seahavenind.com (meal-order-manager/template.yaml:20-22) and its - # email_report handler sends with that Source; the earlier "sending - # from an unverified identity fails loudly anyway" argument only holds - # until the migration verifies the domain, which the migration - # procedure itself requires. - - Sid: SES - Effect: Allow - Action: - - ses:SendEmail - - ses:SendRawEmail - Resource: - - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/int.seahaven.com" - - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/seahaven.com" - - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:identity/seahavenind.com" - - !Sub "arn:aws:ses:us-east-1:${AWS::AccountId}:configuration-set/seahaven-email-events" - - # ── EventBridge Scheduler (afterhours-shift-manager holiday routing) ─ - # Omitted from the permission-source block entirely, which the block's - # "verified live" header did not catch. afterhours-shift-manager - # template.yaml:110-120 grants these three scheduler actions plus - # iam:PassRole on its holiday-scheduler execution role. Without them - # the failure is SILENT: src/slack-bot/app.py wraps create_schedule in - # a bare `except Exception`, so the Slack command returns success, the - # holiday record is written with no schedule, and no alarm fires. - - Sid: EventBridgeScheduler - Effect: Allow - Action: - - scheduler:CreateSchedule - - scheduler:DeleteSchedule - - scheduler:GetSchedule - Resource: - - !Sub "arn:aws:scheduler:us-east-1:${AWS::AccountId}:schedule/default/holiday-*" - - # PassRole is confined to the scheduler service principal, so this - # cannot be used to hand a role to Lambda or any other service. - - Sid: SchedulerPassRole - Effect: Allow - Action: - - iam:PassRole - Resource: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/afterhours-shift-manager-*" - Condition: - StringEquals: - "iam:PassedToService": "scheduler.amazonaws.com" - - # ── KMS — key/* RETAINED, scoped by condition instead ─────────────── - # KMS is the one high-value data plane that CANNOT be scoped by resource - # name: key ARNs carry UUID key ids, not workload names, and alias ARNs - # are not valid in a Resource for these actions (alias scoping needs a - # kms:RequestAlias condition). The two key ids named in this statement's - # previous comment (key/0b660af3, key/b748750c) are MGMT keys that do not - # exist in seahaven-prod or seahaven-dev; hardcoding them — or prod's - # three live CMKs — would encode one account's inventory into a template - # shared by two, and would break on any key replacement. - # So Resource stays key/* and the scope is derived from kms:ViaService: - # the fleet may use a CMK ONLY as part of a request one of these services - # makes on its behalf. Decrypting a DynamoDB item or an S3 object still - # works; a direct kms:Decrypt on arbitrary ciphertext lifted from anywhere - # in the account is denied — the actual escalation path key/* opened. - # Because those services are themselves prefix-scoped above, the effective - # KMS scope INHERITS the per-workload scoping for free, with no key ids - # and no per-account parameterisation. - # logs.us-east-1.amazonaws.com is included as belt-and-braces: CloudWatch - # Logs is believed to decrypt log-group CMKs under its own service grant - # rather than the execution role's credentials, which would make this entry - # a no-op — but a KMS denial on the logging path would be SILENT, and the - # entry cannot grant anything meaningful on its own, so the insurance is - # bought deliberately. - # If a specific key must ever be named, use a kms:RequestAlias condition — - # never a literal key id. - - Sid: KMS - Effect: Allow - Action: - - kms:Decrypt - - kms:GenerateDataKey - - kms:DescribeKey - Resource: - - !Sub "arn:aws:kms:us-east-1:${AWS::AccountId}:key/*" - Condition: - StringEquals: - kms:ViaService: - - dynamodb.us-east-1.amazonaws.com - - logs.us-east-1.amazonaws.com - - s3.us-east-1.amazonaws.com - - secretsmanager.us-east-1.amazonaws.com - # REQUIRED for parity with SSMParameterRead above: a - # SecureString parameter decrypts via the SSM service - # principal, so omitting this denies reads that this same - # policy grants — a self-inconsistency caught by the - # 2026-07-30 review. Any ssm:GetParameter* grant in this - # boundary must keep this entry. - - ssm.us-east-1.amazonaws.com - - sqs.us-east-1.amazonaws.com - + # ── NO PER-WORKLOAD DATA-PLANE STATEMENTS — BY DESIGN ─────────────── + # The statements above are the FLEET-WIDE FLOOR: what every Lambda + # execution role needs regardless of which workload it belongs to. + # There are deliberately NO DynamoDB, S3, Secrets Manager, SSM, SQS, + # SES, KMS, lambda:InvokeFunction or scheduler statements here. + # + # WHY (decided 2026-07-30, Adam): + # The security win of INFRA-186 comes from DELETION, not enumeration. + # Removing the account-wide secret:*, table/*, function:* and sqs:* + # wildcards is what closes the amplifier — the ability of a principal + # who can write an inline policy onto a boundary-carrying role to read + # every secret in the account. Per-workload prefixes add no security; + # they exist only to keep a workload FUNCTIONAL once it arrives. + # + # An earlier revision of this branch PRE-LOADED prefixes for all five + # mgmt SAM stacks before any of them had migrated. That required + # predicting five stacks' permission needs from the permission-source + # comment block above, and the /sh-security-review pass found SIX + # errors in the result — three of which would have failed SILENTLY at + # first migration (afterhours' SES config-set, its holiday scheduler + # behind a bare except, and afi's webhook secret under an invented + # name). The block is a secondary record and is not a substitute for + # reading the owning repo's template. + # + # WIDENING IS THE SAFE DIRECTION. Adding a resource to a boundary can + # never break a running Lambda; only tightening can. So there is no + # cost to deferring per-workload scope to the migration PR that has + # the real template open in front of it — and a large cost to + # guessing it years ahead of the migration. + # + # 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 + # at first invoke. The permission-source block above is the starting + # point, NOT the authority — verify every entry against the stack. + # + # Prod/dev boundary usage is 0 (verified 2026-07-30), so this floor + # currently constrains nothing that exists. Fleet-wide statements that + # genuinely cannot be scoped (Logs, X-Ray, ENI) stay above with their + # 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 + # 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 + # cold-start INIT. + # + # The end-state fix for the shared-ceiling residual (one boundary = + # every SAM workload reaches every other's data plane once they land) + # is per-workload boundaries — tracked separately, see the header. # --------------------------------------------------------------------------- # Shared CloudFormation execution role (SAM stacks) — INFRA-97 scoped # From e791005af019515a30a7acd57100353467893ca0 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 19:09:27 -0400 Subject: [PATCH 5/8] docs(iam): cite INFRA-187 as the per-workload boundary end state Replaces the placeholder 'tracked as its own ticket' references with the real key, and records the load-bearing constraint inline so the next reader does not rediscover it: both guardrail policies pin ONE literal boundary ARN inside StringEquals iam:PermissionsBoundary, and loosening that to a wildcard weakens the gate rather than merely relaxing it. --- lib/deploy-substrate/deploy-substrate.template.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index 9a91de2..a7a756b 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -132,7 +132,7 @@ Description: >- # the boundary itself) removed that pressure entirely. If the budget tightens # again as workloads land, the end-state fix is per-workload boundaries # (seahaven-lambda-execution-boundary-), which also resolves the -# shared-ceiling residual — tracked as its own ticket, do not improvise it. +# shared-ceiling residual — tracked as INFRA-187, do not improvise it. # CRITICAL: unlike the 2026-07-27 inline-limit incident, # there is NO restructure available when this cap is reached — a role has exactly # ONE permissions boundary, so statements cannot be spilled into a second attached @@ -498,7 +498,10 @@ Resources: # # The end-state fix for the shared-ceiling residual (one boundary = # every SAM workload reaches every other's data plane once they land) - # is per-workload boundaries — tracked separately, see the header. + # is per-workload boundaries — tracked as INFRA-187. Do not improvise + # it: the load-bearing problem there is that both guardrail policies + # pin ONE literal boundary ARN inside StringEquals conditions, and + # loosening that to a wildcard weakens the gate. # --------------------------------------------------------------------------- # Shared CloudFormation execution role (SAM stacks) — INFRA-97 scoped # From a1086e04fbab5011b37ca5236c4026ca4eb3fcd8 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 31 Jul 2026 13:23:20 -0400 Subject: [PATCH 6/8] docs(readme): describe the boundary floor, not the superseded prefix design --- README.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 627f686..abf1f9c 100644 --- a/README.md +++ b/README.md @@ -139,14 +139,17 @@ of truth for mgmt (328440206208) until its stacks migrate out. **The two copies are no longer at parity, and the old "edit both files" rule no longer applies uniformly.** Under INFRA-186, `seahaven-lambda-execution-boundary` -in *this* copy was scoped to per-workload prefixes for prod and dev (where -boundary usage was 0, so no live Lambda could break), while mgmt's copy keeps -the account-wide wildcards pending its own separately validated rollout across -26 live boundary-carrying roles. So: **the boundary resource is deliberately -divergent**; every *other* substrate resource (`github-cfn-execution-role`, -`seahaven-cfn-exec-iam-management`) is still expected to change in both files -together. The template's provenance header records which is which — read it -before assuming either parity or divergence. +in *this* copy was reduced to a fleet-wide floor for prod and dev (where +boundary usage was 0, so no live Lambda could break): CloudWatch Logs write on +`/aws/lambda*`, log-group describe, X-Ray, and ENI lifecycle — nothing else. +Each migrating stack adds its own data-plane statements, derived from its own +template, in its own PR (per-workload boundaries are the INFRA-187 end state). +mgmt's copy keeps the account-wide wildcards pending its own separately +validated rollout across 26 live boundary-carrying roles. So: **the boundary +resource is deliberately divergent**; every *other* substrate resource +(`github-cfn-execution-role`, `seahaven-cfn-exec-iam-management`) is still +expected to change in both files together. The template's provenance header +records which is which — read it before assuming either parity or divergence. Per-repo `githubdeploy-*` deploy roles are deliberately NOT part of the substrate — they are provisioned per repo at migration/onboarding time From 08a1d41b05682b0e086e1b5da582bc0d44859441 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 31 Jul 2026 13:44:21 -0400 Subject: [PATCH 7/8] 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. --- README.md | 14 ++- .../deploy-substrate.template.yaml | 102 ++++++++++++------ .../terraform-substrate.template.yaml | 10 +- 3 files changed, 92 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index abf1f9c..9aaf5f9 100644 --- a/README.md +++ b/README.md @@ -272,14 +272,24 @@ split this substrate exists to enforce. `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 + + 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-` 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 diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index a7a756b..f966b9d 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -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 " 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:::policy/seahaven-lambda-execution-boundary + # aws iam delete-policy-version --version-id v ... # 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. diff --git a/lib/terraform-substrate/terraform-substrate.template.yaml b/lib/terraform-substrate/terraform-substrate.template.yaml index 538f6da..4875cd7 100644 --- a/lib/terraform-substrate/terraform-substrate.template.yaml +++ b/lib/terraform-substrate/terraform-substrate.template.yaml @@ -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. From 16a82c2a360f94224290ff0fa6ecbce2f4c91cbd Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 31 Jul 2026 13:46:29 -0400 Subject: [PATCH 8/8] docs(iam): qualify mgmt-only verification claims per cross-review round 2 --- .../deploy-substrate.template.yaml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/deploy-substrate/deploy-substrate.template.yaml b/lib/deploy-substrate/deploy-substrate.template.yaml index f966b9d..301af15 100644 --- a/lib/deploy-substrate/deploy-substrate.template.yaml +++ b/lib/deploy-substrate/deploy-substrate.template.yaml @@ -284,8 +284,13 @@ Resources: # 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; starting point - # only — verify every entry against the owning repo before use): + # Permission sources per stack (verified live 2026-07-30 IN MGMT — these + # stacks and resources do not exist in prod/dev yet, so nothing below is a + # prod/dev observation; starting point only — verify every entry against + # the owning repo before use. PARAMETERIZED resources (ARNs passed as + # deploy parameters) must be re-derived from the live stack configuration + # at migration time, as exact ARNs — never inferred from these names into + # broad patterns like secret:afi-*): # # afterhours-shift-manager (functions: afterhours-*, 6 live) # - DynamoDB CRUD (afterhours-shifts table) @@ -307,7 +312,9 @@ Resources: # ONLY this action) # - CloudWatch Logs (all functions) # - UNRESOLVED: /3cx-scheduler/* ownership (this stack vs. the retired - # standalone 3CX scheduler). Deliberately NOT granted. Resolve at migration. + # standalone 3CX scheduler). Deliberately NOT granted. Resolve at + # migration in that stack's own repo — do NOT add any 3cx-scheduler + # resource here until ownership is resolved. # # payments-dashboard (functions: payments-*) # - DynamoDB CRUD / Read (PaymentsDashboard table — legacy PascalCase) @@ -338,7 +345,8 @@ Resources: # - DynamoDB CRUD (front-sla-alerts table) # - secretsmanager:GetSecretValue (front-integrations/*) # - CloudWatch Logs - # - no S3 / SQS / SSM / SES / KMS / VPC + # - no IAM permissions for S3 / SQS / SSM / SES / KMS / VPC in this + # stack's template as of 2026-07-30 # # afi-backup-monitor (functions: afi-*) # - secretsmanager:GetSecretValue on TWO bare, unprefixed secrets: