mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 12:43:16 +00:00
fix: instance role BatchGetSecretValue + ListSecrets for .env materialization (#22)
* fix(infra): grant instance role BatchGetSecretValue + ListSecrets for .env materialization fetch-config.sh materializes the box's .env via `secretsmanager batch-get-secret-value --filters Key=name,Values=open-swe-<env>/`, but the instance role only granted GetSecretValue/DescribeSecret. BatchGetSecretValue is a distinct IAM action, so the call was AccessDenied and open-swe.service crash-looped (no .env written -> ExecStartPre exit 1). - Add secretsmanager:BatchGetSecretValue to the prefix-scoped ReadSecrets statement. - Add secretsmanager:ListSecrets on * (required by the name-prefix filtered batch call; the API has no resource-level scoping for the list action — fits the role's stated exception). Secret VALUES stay prefix-scoped; only names are enumerable. Reviews: GPT-4.1 IAM cross-review BLOCK=none; /sh-security-review iac-iam one LOW metadata residual (no critical/high), recorded as OSWE-IAC-SECRETS-LIST-01. Refs T7/T19 dev bring-up. * ci: lift Node heap cap for Playwright E2E build (vite OOM) The E2E job's Playwright globalSetup runs the real `bun run build`, whose vite bundle exceeds Node's default ~2 GB heap and OOMs (JavaScript heap out of memory) — the same failure fixed for build-artifacts.yml in #19. Set NODE_OPTIONS=--max-old-space-size=8192 on the Run E2E step.
This commit is contained in:
parent
5fa132205b
commit
cfbdcda9b7
3 changed files with 39 additions and 2 deletions
5
.github/workflows/ci.yml
vendored
5
.github/workflows/ci.yml
vendored
|
|
@ -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() }}
|
||||
|
|
|
|||
|
|
@ -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-<env>/`, 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-<env>/*), 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"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -61,15 +61,37 @@ export class InstanceRole extends Construct {
|
|||
);
|
||||
|
||||
// Read secrets from Secrets Manager under open-swe-<env>/*. 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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue