Scope secrets fetch to --secret-id-list; drop ListSecrets grant (#48)

Switch fetch-config.sh from a name-prefix batch-get-secret-value
--filters scan to an explicit --secret-id-list (the 28 SECRET_VARS,
chunked at the 20/call cap). An id-list batch authorizes per-secret
ARN, so the instance role's BatchGetSecretValue moves from Resource:*
to the open-swe-<env>/* prefix and the account-wide ListSecrets grant
is dropped entirely. The box can no longer enumerate secret names
account-wide; cross-env value isolation is unchanged (GetSecretValue
was already prefix-scoped). Resolves OSWE-IAC-SECRETS-LIST-01.

Also capture each chunk response into a variable and consume the
producer via command substitution so a failed AWS call aborts under
set -e instead of being swallowed by process substitution and
misreported as a missing required var.

Refs: OSWE-IAC-SECRETS-LIST-01

deploy/seahaven/fetch-config.sh

@@ -158,34 +158,64 @@ while IFS=$'\t' read -r nb vb; do
 done < <(jq -r '.Parameters[] | (.Name|@base64) + "\t" + (.Value|@base64)' <<<"$ssm_json")
 log "loaded ${ssm_count} config param(s) from SSM"
 
-# 2) Secrets Manager (sensitive values). batch-get-secret-value filters by name
-#    prefix; one secret per env var, SecretString = the value. The AWS CLI does NOT
-#    auto-paginate this operation (unlike list/get-parameters-by-path), and a single
-#    response caps well under the full set (~10 items/page) — so we MUST follow
-#    NextToken ourselves or secrets on later pages are silently dropped (they then
-#    surface as FAIL-FAST "missing required var"). `--no-cli-pager` only disables the
-#    OUTPUT pager, not API pagination. Each page's records are base64(name<TAB>value).
+# 2) Secrets Manager (sensitive values). We request secrets by EXPLICIT id
+#    (`batch-get-secret-value --secret-id-list ...`) rather than a name-prefix
+#    `--filters` collection scan (OSWE-IAC-SECRETS-LIST-01). Two wins:
+#      (a) Least privilege — an explicit id list lets the instance role scope
+#          BatchGetSecretValue to the per-secret ARN prefix and DROP the account-wide
+#          `secretsmanager:ListSecrets` grant that a filtered scan unavoidably forces
+#          (ListSecrets has no resource-level scoping). A filtered batch call also only
+#          authorizes against `*`; an id-list call authorizes per-secret ARN.
+#      (b) Deterministic set — the value set is the fixed SECRET_VARS shells created by
+#          the CDK ConfigStore, so we no longer depend on a list scan returning every
+#          page. Value-less shells and ids with no current value come back in the
+#          response `.Errors[]` (ResourceNotFound), never in `.SecretValues[]`, so a
+#          genuinely-missing REQUIRED secret is still caught by the FAIL-FAST check
+#          below — an absent optional secret is simply skipped.
+#    `--secret-id-list` is capped at 20 ids per call, so we chunk it. `--no-cli-pager`
+#    disables only the OUTPUT pager. Each record is base64(name<TAB>value).
+#
+# SOURCE OF TRUTH for this list: infra/lib/constructs/config-store.ts `SECRET_VARS`.
+# Keep the two in lockstep — a new secret shell created there must be added here or it
+# will never be fetched into the .env.
+SECRET_VARS=(
+  ANTHROPIC_API_KEY CORRIDOR_API_TOKEN CORRIDOR_MCP_TOKEN CORRIDOR_TOKEN
+  DASHBOARD_JWT_SECRET DAYTONA_API_KEY EXA_API_KEY FIREWORKS_API_KEY
+  GITHUB_APP_CLIENT_SECRET GITHUB_APP_PRIVATE_KEY GITHUB_PAT GITHUB_WEBHOOK_SECRET
+  GOOGLE_API_KEY GROQ_API_KEY JUDGE_ANTHROPIC_API_KEY LANGSMITH_API_KEY
+  LANGSMITH_API_KEY_PROD LANGCHAIN_API_KEY LINEAR_API_KEY LINEAR_WEBHOOK_SECRET
+  OPENAI_API_KEY RUNLOOP_API_KEY SLACK_BOT_TOKEN SLACK_CLIENT_SECRET
+  SLACK_SIGNING_SECRET TOKEN_ENCRYPTION_KEY USER_ID_API_KEY_MAP X_SERVICE_AUTH_JWT_SECRET
+)
+
 batch_get_secrets_tsv() {
-  local token="" page
-  while :; do
-    if [ -n "$token" ]; then
-      page="$(aws secretsmanager batch-get-secret-value \
-        --filters "Key=name,Values=${SECRET_PREFIX}" \
-        --region "$REGION" --no-cli-pager --output json --next-token "$token")"
-    else
-      page="$(aws secretsmanager batch-get-secret-value \
-        --filters "Key=name,Values=${SECRET_PREFIX}" \
-        --region "$REGION" --no-cli-pager --output json)"
-    fi
+  local -a ids=()
+  local v
+  for v in "${SECRET_VARS[@]}"; do ids+=("${SECRET_PREFIX}${v}"); done
+
+  local i page
+  local -a chunk
+  for ((i = 0; i < ${#ids[@]}; i += 20)); do
+    chunk=("${ids[@]:i:20}")
+    # Capture the response into a variable FIRST so a non-zero `aws` exit (throttle,
+    # AccessDenied, KMS DecryptionFailure) aborts under set -e instead of being
+    # silently swallowed — then we'd FAIL-FAST below as "missing secret" with a wrong
+    # root cause. (Value-less / absent shells come back in .Errors[], not .SecretValues[].)
+    page="$(aws secretsmanager batch-get-secret-value \
+      --secret-id-list "${chunk[@]}" \
+      --region "$REGION" --no-cli-pager --output json)"
     printf '%s' "$page" \
       | jq -r '.SecretValues[] | select(.SecretString != null) | (.Name|@base64) + "\t" + (.SecretString|@base64)'
-    token="$(printf '%s' "$page" | jq -r '.NextToken // empty')"
-    [ -n "$token" ] || break
   done
 }
 
 log "reading secrets under ${SECRET_PREFIX} ..."
 secret_count=0
+# Capture into a variable (NOT `done < <(...)` process substitution) so a non-zero
+# exit from batch_get_secrets_tsv propagates under set -e — process substitution hides
+# the producer's exit status from the parent shell, which would let a failed AWS call
+# fall through to a misleading "missing required var" FAIL-FAST. Mirrors the SSM read.
+secrets_tsv="$(batch_get_secrets_tsv)"
 while IFS=$'\t' read -r nb vb; do
   [ -n "$nb" ] || continue
   name="$(printf '%s' "$nb" | b64d)"
@@ -198,7 +228,7 @@ while IFS=$'\t' read -r nb vb; do
   [ -n "$key" ] || continue
   accept_var "$key" "$value" "Secrets"
   secret_count=$((secret_count + 1))
-done < <(batch_get_secrets_tsv)
+done <<<"$secrets_tsv"
 log "loaded ${secret_count} secret(s) from Secrets Manager"
 
 # --- Sea Haven DEFAULT_REPO_OWNER guard --------------------------------------

infra/lib/constructs/instance-role.ts

@@ -64,35 +64,28 @@ export class InstanceRole extends Construct {
     // random 6-char suffix, hence the trailing `*`. This is the statement that
     // actually gates which secret VALUES the box can read: prefix-scoped, so the
     // dev box can never read prod secret values (and vice versa). GetSecretValue is
-    // checked per-secret even when the value is returned via the batch call below.
+    // checked per-secret even when the value is returned via BatchGetSecretValue.
+    //
+    // BatchGetSecretValue is included here (prefix-scoped, NOT `*`) because
+    // fetch-config.sh now fetches by EXPLICIT `--secret-id-list` rather than a name
+    // `--filters` scan (OSWE-IAC-SECRETS-LIST-01). An id-list batch call authorizes
+    // against each referenced secret's ARN, so the prefix ARN satisfies it — and we
+    // no longer need the account-wide `secretsmanager:ListSecrets` grant that a
+    // filtered collection scan unavoidably forced (ListSecrets has no resource-level
+    // scoping). Net effect: the box can read open-swe-<env>/* secret values and
+    // nothing else — it can no longer enumerate secret names account-wide.
     this.role.addToPolicy(
       new iam.PolicyStatement({
         sid: "ReadSecretValues",
-        actions: ["secretsmanager:GetSecretValue", "secretsmanager:DescribeSecret"],
+        actions: [
+          "secretsmanager:GetSecretValue",
+          "secretsmanager:BatchGetSecretValue",
+          "secretsmanager:DescribeSecret",
+        ],
         resources: [`arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:${p}/*`],
       }),
     );
 
-    // OPERATION-level grants for fetch-config.sh's prefix-FILTERED batch read
-    // (`batch-get-secret-value --filters Key=name,Values=open-swe-<env>/`). Both of
-    // these are collection operations that AWS authorizes against `*`, NOT a
-    // per-secret ARN: BatchGetSecretValue with a filter is a collection call (a
-    // prefix-scoped ARN does NOT satisfy it — it AccessDenies), and ListSecrets is a
-    // 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(
-      new iam.PolicyStatement({
-        sid: "SecretsBatchListOps",
-        actions: ["secretsmanager:BatchGetSecretValue", "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