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