Commit graph

6 commits

Author SHA1 Message Date
b264f74f01
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.
2026-07-30 18:03:40 -04:00
274f933495
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 <stack>/ 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
2026-07-30 17:46:37 -04:00
61a94da4fc
fix(iam): reconcile the remaining substrate divergences from the mgmt copy
Review of the DenySelfMutation port found the header's 'reconciled' claim
was not yet true: mgmt Phase A also added the CloudWatch Logs
metric-filter actions (afterhours-shift-manager creates an
AWS::Logs::MetricFilter through this role), and without them a migrating
SAM stack fails mid-deploy with AccessDenied. Ports those three actions
and corrects two stale header notes. Every IAM statement in the three
shared resources is now byte-identical across both files, verified
programmatically; the only delta left is the DependsOn ordering line.
2026-07-27 18:55:10 -04:00
62f6a76e6c
fix(iam): port DenySelfMutation self-protection into the prod/dev deploy substrate
The seahaven-cfn-exec-iam-management policy in prod and dev carried only
DenyBoundaryTampering + DenyBoundaryPolicyEdit: the mgmt Phase A review
later showed a Deny-in-a-managed-policy control is self-detachable
(iam:DetachRolePolicy on * is unconditioned), so without DenySelfMutation
the exec role can detach the very policy carrying the Denies and
reinstate the boundary-removal escalation. Latent today (no PassRole
grants, zero SAM stacks in prod/dev) but must be closed before the first
SAM workload migrates.

Ports verbatim from .github/oidc-deploy-roles.yaml (mgmt, PRs #95/#98):
- DenySelfMutation over role/github-cfn-execution-role + githubdeploy-*
- DenyBoundaryPolicyEdit widened to policy/seahaven-*

Statement set verified byte-identical to the mgmt copy (9 sids);
provenance header updated - the two copies are reconciled.
2026-07-27 18:37:39 -04:00
2cfc122269
fix(deploy-substrate): move boundary-gated IAM policy off the role's inline budget
The first deploy of seahaven-deploy-substrate failed in both prod and dev
with ServiceLimitExceeded: 'Maximum policy size of 10240 bytes exceeded
for role github-cfn-execution-role'. The role's inline policies already
sat ~94 bytes under IAM's hard 10,240-byte per-role limit, so the two
Deny statements added to close the boundary-removal escalation did not
fit (10,656 total).

Moves the whole boundary-gated IAM block (6 Allow + 2 Deny statements)
into an attached managed policy, which carries its own separate
6,144-byte budget. Inline drops to 8,285 with ~1.9 KB of headroom;
the managed policy sits at 2,371.

Effective permissions are unchanged: the union of role statements
(inline + attached) is byte-identical as a sorted set before and after
the move (27 statements both sides), identity policies are unioned, and
an explicit Deny still wins. Boundary and trust policy untouched.

Both failed stacks rolled back cleanly with zero orphaned resources and
were deleted before this retry.
2026-07-27 16:43:15 -04:00
d6bea33436
feat(deploy-substrate): per-account GitHub Actions deploy substrate for prod/dev
SAM repos migrating off the frozen management account need the shared
deploy plumbing (permissions boundary + github-cfn-execution-role) in
their target account; none of it existed outside mgmt, so there was no
OIDC SAM deploy path into seahaven-prod or seahaven-dev at all.

Adds a templated, per-account substrate stack so onboarding a future
account is one bin/app.ts instance plus one CD job, not a hand-rolled
copy. Per-repo githubdeploy-* roles stay out by design: they are
provisioned per repo at migration time so an account never accumulates
trust for repos that do not deploy to it.

The template is a verbatim extraction of the reviewed mgmt substrate,
with deliberate, documented divergences — notably the removal of
iam:DeleteRolePermissionsBoundary plus explicit Deny backstops, which
closes a confirmed privilege-escalation path (see PR body).
2026-07-27 16:24:09 -04:00