diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0d65a66..ff062325 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,6 +69,11 @@ jobs: # ui/ SPA. The fake LLM/GitHub/Slack boundaries need no secrets. - name: Run E2E working-directory: tests/e2e + # Playwright's globalSetup runs the real `bun run build`, whose vite bundle + # exceeds Node's default ~2 GB heap (same OOM fixed in build-artifacts.yml). + # The runner has ~16 GB, so lift the heap cap. + env: + NODE_OPTIONS: "--max-old-space-size=8192" run: npx playwright test - name: Upload Playwright report if: ${{ !cancelled() }} diff --git a/.security-review/suppressions.json b/.security-review/suppressions.json index 722fc906..574fb0f7 100644 --- a/.security-review/suppressions.json +++ b/.security-review/suppressions.json @@ -9,6 +9,16 @@ "suppression_justification": "PRE-EXISTING and NOT introduced or worsened by the T7+T19 change (the assets bucket / app-role PutObject / SSM deploy doc). This is the known single-account-wide CDK cfn-exec residual already documented in infra/lib/config.ts:31-34 and the github-deploy-roles.ts construct comment, accepted at the T4 GPT-4.1 IAM cross-review and the v5 plan-review. WHO can assume each env's infra role is exact-subject scoped (StringEquals on the dev ref / prod environment); the residual is the shared account-wide cfn-exec-role that every env's infra role can reach. The tracked fix is per-env CDK bootstrap qualifiers so each env's infra role assumes its own env-scoped cfn-exec-role. Suppressed for THIS change's gate because it is out-of-diff and unchanged; surfaced to Adam for scheduling the per-env-bootstrap remediation.", "owner": "adam@seahavenind.com", "added": "2026-06-26" + }, + { + "id": "OSWE-IAC-SECRETS-LIST-01", + "title": "EC2 instance role can enumerate Secrets Manager secret NAMES/metadata account-wide via ListSecrets \"*\"", + "file": "infra/lib/constructs/instance-role.ts", + "severity": "low", + "status": "confirmed", + "suppression_justification": "ACCEPTED LOW residual, metadata-only. fetch-config.sh materializes the .env via `batch-get-secret-value --filters Key=name,Values=open-swe-/`, and the AWS API requires secretsmanager:ListSecrets for the filtered batch call. ListSecrets has resource type 'none' in the IAM reference and supports NO resource-level or name-condition scoping, so the `*` is structural, not a fixable scoping miss — it fits this role's stated exception for actions that genuinely lack resource-level scoping. Secret VALUES remain strictly prefix-scoped (GetSecretValue/BatchGetSecretValue on secret:open-swe-/*), so cross-env VALUE isolation is preserved; only NAMES/tags/descriptions are enumerable, within Sea Haven's own single-tenant account 328440206208. Confirmed by the T-bringup GPT-4.1 IAM cross-review (BLOCK: none) and the iac-iam detector (one low residual, no critical/high). Future hardening to eliminate the `*` entirely: switch fetch-config.sh to an explicit SecretIdList sourced from a non-sensitive SSM manifest param (no --filters -> no ListSecrets).", + "owner": "adam@seahavenind.com", + "added": "2026-06-26" } ] } diff --git a/infra/lib/constructs/instance-role.ts b/infra/lib/constructs/instance-role.ts index 7f5eab96..2dcfdbb4 100644 --- a/infra/lib/constructs/instance-role.ts +++ b/infra/lib/constructs/instance-role.ts @@ -61,15 +61,37 @@ export class InstanceRole extends Construct { ); // Read secrets from Secrets Manager under open-swe-/*. Secret ARNs carry - // a random 6-char suffix, hence the trailing `*`. + // a random 6-char suffix, hence the trailing `*`. fetch-config.sh materializes + // the .env via `batch-get-secret-value` (one call instead of N), so the role + // needs BatchGetSecretValue (a DISTINCT action from GetSecretValue) in addition + // to the per-secret GetSecretValue/DescribeSecret. All three are scoped to the + // env prefix — the box can only read its own env's secret VALUES. this.role.addToPolicy( new iam.PolicyStatement({ sid: "ReadSecrets", - actions: ["secretsmanager:GetSecretValue", "secretsmanager:DescribeSecret"], + actions: [ + "secretsmanager:GetSecretValue", + "secretsmanager:BatchGetSecretValue", + "secretsmanager:DescribeSecret", + ], resources: [`arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:${p}/*`], }), ); + // batch-get-secret-value with a name-prefix `--filters` additionally requires + // ListSecrets, which the AWS API does NOT support scoping by resource (it is a + // list-style action) — so it is `*`, matching this role's stated exception for + // actions that genuinely have no resource-level scoping. This grants only the + // ability to ENUMERATE secret names; reading a secret's VALUE still requires the + // prefix-scoped GetSecretValue above, so cross-env value isolation is preserved. + this.role.addToPolicy( + new iam.PolicyStatement({ + sid: "ListSecretsForBatchFilter", + actions: ["secretsmanager:ListSecrets"], + resources: ["*"], + }), + ); + // NOTE (T11): SSM SecureString + Secrets Manager here are assumed to use the // AWS-managed keys (alias/aws/ssm, alias/aws/secretsmanager) for which the // service grants Decrypt implicitly — so NO kms:Decrypt is granted. If T11