mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-05 20:02:11 +00:00
fix: BatchGetSecretValue must be on * for the filtered batch call (#23)
The prior fix scoped secretsmanager:BatchGetSecretValue to the env-prefixed secret ARN, but the live box still got AccessDenied: batch-get-secret-value invoked WITH a name --filters is a COLLECTION call that AWS authorizes against * (a per-secret ARN does not satisfy it). Split the statement: - GetSecretValue + DescribeSecret stay PREFIX-scoped (secret:open-swe-<env>/*) — this is what gates which secret VALUES the box can read (checked per-secret in the batch). - BatchGetSecretValue + ListSecrets move to a * operation-level statement (the filtered collection call + the list action; neither is resource-scopable for this usage). VALUE isolation preserved (dev box still cannot read prod secret values); only secret NAME/metadata enumeration is widened. GPT-4.1 IAM cross-review: BLOCK none, FIX none. Suppression OSWE-IAC-SECRETS-LIST-01 updated; future hardening (explicit --secret-id-list to drop both * grants) tracked there.
This commit is contained in:
parent
cfbdcda9b7
commit
cdcb6625b9
2 changed files with 23 additions and 22 deletions
|
|
@ -12,11 +12,11 @@
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"id": "OSWE-IAC-SECRETS-LIST-01",
|
"id": "OSWE-IAC-SECRETS-LIST-01",
|
||||||
"title": "EC2 instance role can enumerate Secrets Manager secret NAMES/metadata account-wide via ListSecrets \"*\"",
|
"title": "EC2 instance role grants BatchGetSecretValue + ListSecrets on \"*\" (operation-level; secret-NAME enumeration account-wide)",
|
||||||
"file": "infra/lib/constructs/instance-role.ts",
|
"file": "infra/lib/constructs/instance-role.ts",
|
||||||
"severity": "low",
|
"severity": "low",
|
||||||
"status": "confirmed",
|
"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).",
|
"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>/`. With a name FILTER, both BatchGetSecretValue (a collection call) and ListSecrets are authorized by AWS against `*`, NOT a per-secret ARN — a prefix-scoped ARN AccessDenies the call (confirmed empirically on i-0af4e03e8bf70e6c3). So the two `*` grants are operation-level, not value-level. Secret VALUES remain strictly gated by the PREFIX-scoped GetSecretValue/DescribeSecret on secret:open-swe-<env>/* (GetSecretValue is checked per-secret even within the batch), so cross-env VALUE isolation is preserved; only NAMES/tags/descriptions are enumerable, within Sea Haven's own single-tenant account 328440206208. Confirmed by GPT-4.1 IAM cross-review (BLOCK: none) and the iac-iam detector (one low residual, no critical/high). Future hardening to eliminate BOTH `*` grants: switch fetch-config.sh to an explicit `--secret-id-list` (no filter), which lets BatchGetSecretValue be prefix-scoped and needs no ListSecrets.",
|
||||||
"owner": "adam@seahavenind.com",
|
"owner": "adam@seahavenind.com",
|
||||||
"added": "2026-06-26"
|
"added": "2026-06-26"
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -60,34 +60,35 @@ export class InstanceRole extends Construct {
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|
||||||
// Read secrets from Secrets Manager under open-swe-<env>/*. Secret ARNs carry
|
// VALUE access — Secrets Manager under open-swe-<env>/*. Secret ARNs carry a
|
||||||
// a random 6-char suffix, hence the trailing `*`. fetch-config.sh materializes
|
// random 6-char suffix, hence the trailing `*`. This is the statement that
|
||||||
// the .env via `batch-get-secret-value` (one call instead of N), so the role
|
// actually gates which secret VALUES the box can read: prefix-scoped, so the
|
||||||
// needs BatchGetSecretValue (a DISTINCT action from GetSecretValue) in addition
|
// dev box can never read prod secret values (and vice versa). GetSecretValue is
|
||||||
// to the per-secret GetSecretValue/DescribeSecret. All three are scoped to the
|
// checked per-secret even when the value is returned via the batch call below.
|
||||||
// env prefix — the box can only read its own env's secret VALUES.
|
|
||||||
this.role.addToPolicy(
|
this.role.addToPolicy(
|
||||||
new iam.PolicyStatement({
|
new iam.PolicyStatement({
|
||||||
sid: "ReadSecrets",
|
sid: "ReadSecretValues",
|
||||||
actions: [
|
actions: ["secretsmanager:GetSecretValue", "secretsmanager:DescribeSecret"],
|
||||||
"secretsmanager:GetSecretValue",
|
|
||||||
"secretsmanager:BatchGetSecretValue",
|
|
||||||
"secretsmanager:DescribeSecret",
|
|
||||||
],
|
|
||||||
resources: [`arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:${p}/*`],
|
resources: [`arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:${p}/*`],
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|
||||||
// batch-get-secret-value with a name-prefix `--filters` additionally requires
|
// OPERATION-level grants for fetch-config.sh's prefix-FILTERED batch read
|
||||||
// ListSecrets, which the AWS API does NOT support scoping by resource (it is a
|
// (`batch-get-secret-value --filters Key=name,Values=open-swe-<env>/`). Both of
|
||||||
// list-style action) — so it is `*`, matching this role's stated exception for
|
// these are collection operations that AWS authorizes against `*`, NOT a
|
||||||
// actions that genuinely have no resource-level scoping. This grants only the
|
// per-secret ARN: BatchGetSecretValue with a filter is a collection call (a
|
||||||
// ability to ENUMERATE secret names; reading a secret's VALUE still requires the
|
// prefix-scoped ARN does NOT satisfy it — it AccessDenies), and ListSecrets is a
|
||||||
// prefix-scoped GetSecretValue above, so cross-env value isolation is preserved.
|
// list action with no resource-level scoping at all. Neither returns or widens
|
||||||
|
// VALUE access: a secret's value is still only returned when the prefix-scoped
|
||||||
|
// GetSecretValue above allows it, so cross-env VALUE isolation is preserved. The
|
||||||
|
// residual is metadata-only (the box can ENUMERATE secret names account-wide).
|
||||||
|
// Future hardening to drop both `*` grants: switch fetch-config to an explicit
|
||||||
|
// `--secret-id-list` (no filter), which lets BatchGetSecretValue be prefix-scoped
|
||||||
|
// and needs no ListSecrets. Tracked as OSWE-IAC-SECRETS-LIST-01.
|
||||||
this.role.addToPolicy(
|
this.role.addToPolicy(
|
||||||
new iam.PolicyStatement({
|
new iam.PolicyStatement({
|
||||||
sid: "ListSecretsForBatchFilter",
|
sid: "SecretsBatchListOps",
|
||||||
actions: ["secretsmanager:ListSecrets"],
|
actions: ["secretsmanager:BatchGetSecretValue", "secretsmanager:ListSecrets"],
|
||||||
resources: ["*"],
|
resources: ["*"],
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue