From be48b877ea254235e8ef7c3ea181a3e45b41d20b Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Tue, 25 Aug 2026 14:46:03 +0000 Subject: [PATCH 01/11] feat: add default VPC BPA exclusion Create or reuse a VPC-wide allow-bidirectional BPA exclusion for every LowKey pack, disclose the behavior in the final deployment review, and cover the installer and CloudFormation wiring with focused tests. --- deploy/cloudformation/template.yaml | 45 ++++++++- deploy/test-templates.sh | 3 + install.sh | 49 +++++++++- tests/test-vpc-bpa.sh | 145 ++++++++++++++++++++++++++++ 4 files changed, 239 insertions(+), 3 deletions(-) create mode 100755 tests/test-vpc-bpa.sh diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index 3179f3a..5cc1aa3 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -1,9 +1,11 @@ AWSTemplateFormatVersion: '2010-09-09' Description: > - OpenClaw Instance - Deploys a fully configured OpenClaw AI assistant on EC2 - within its own VPC. Designed for StackSet deployment across AWS Organization accounts. + LowKey agent host on EC2 - provisions public VPC connectivity and a + bidirectional VPC Block Public Access exclusion for reliable bootstrap and ingress. Metadata: + AWSToolsMetrics: + AWSAgentToolkit: aws-cloudformation@2 AWS::CloudFormation::Interface: ParameterGroups: - Label: @@ -53,6 +55,7 @@ Metadata: - ExistingVpcId - ExistingSubnetId - ExistingSubnetId2 + - CreateVpcBpaExclusion - SSHAllowedCidr - KeyPairName - Label: @@ -368,6 +371,12 @@ Parameters: Description: "Second public subnet ID in a different AZ. Required for KiroCrew pack (ALB needs 2 AZs) when ExistingVpcId is set." AllowedPattern: '^(subnet-[a-z0-9]+)?$' + CreateVpcBpaExclusion: + Type: String + Default: 'true' + AllowedValues: ['true', 'false'] + Description: "Create a VPC-wide allow-bidirectional Block Public Access exclusion. The LowKey wizard sets this to false only when the reused VPC already has an active bidirectional exclusion." + RepoBranch: Type: String Default: 'main' @@ -463,6 +472,7 @@ Conditions: IsApiKey: !Equals [!Ref ModelMode, 'api-key'] IsBedrock: !Equals [!Ref ModelMode, 'bedrock'] CreateNewVpc: !Equals [!Ref ExistingVpcId, ''] + ShouldCreateVpcBpaExclusion: !Equals [!Ref CreateVpcBpaExclusion, 'true'] IsBuilder: !Equals [!Ref ProfileName, 'builder'] IsNotBuilder: !Not [!Condition IsBuilder] IsAccountAssistant: !Equals [!Ref ProfileName, 'account_assistant'] @@ -506,6 +516,30 @@ Resources: - Key: loki:pack Value: !Ref PackName + # LowKey currently uses an IGW-backed public VPC for bootstrap and optional + # public endpoints. Keep the VPC reachable when regional BPA is enabled. + VpcBpaExclusion: + Type: AWS::EC2::VPCBlockPublicAccessExclusion + Condition: ShouldCreateVpcBpaExclusion + Metadata: + com.aws.cloudformation.Context: + why: Keep LowKey's IGW-backed VPC reachable when regional VPC BPA is enabled. + must: + - VPC-wide allow-bidirectional; bootstrap needs internet egress and public endpoints need ingress. + mutable: review-required + Properties: + InternetGatewayExclusionMode: allow-bidirectional + VpcId: !If [CreateNewVpc, !Ref VPC, !Ref ExistingVpcId] + Tags: + - Key: Name + Value: !Sub '${EnvironmentName}-vpc-bpa-exclusion' + - Key: loki:managed + Value: 'true' + - Key: loki:watermark + Value: !Ref LokiWatermark + - Key: loki:pack + Value: !Ref PackName + InternetGateway: Type: AWS::EC2::InternetGateway Condition: CreateNewVpc @@ -1838,6 +1872,13 @@ Resources: # -------------------------------------------------------------------------- Instance: Type: AWS::EC2::Instance + Metadata: + # Optional dependency: when this stack creates the exclusion, wait for it + # before UserData attempts internet downloads. Existing exclusions need no wait. + VpcBpaExclusionDependency: !If + - ShouldCreateVpcBpaExclusion + - !Ref VpcBpaExclusion + - existing CreationPolicy: ResourceSignal: Timeout: PT30M diff --git a/deploy/test-templates.sh b/deploy/test-templates.sh index 575ab54..ec39356 100644 --- a/deploy/test-templates.sh +++ b/deploy/test-templates.sh @@ -45,6 +45,9 @@ check_contains "$CFN_TEMPLATE" "git clone --depth 1" "CFN: UserData uses git clo check_contains "$CFN_TEMPLATE" "deploy/bootstrap.sh" "CFN: UserData calls bootstrap.sh" check_contains "$CFN_TEMPLATE" "--pack" "CFN: UserData passes --pack flag" check_contains "$CFN_TEMPLATE" "Deployed agent pack" "CFN: PackName in Outputs" +check_contains "$CFN_TEMPLATE" "AWS::EC2::VPCBlockPublicAccessExclusion" "CFN: VPC BPA exclusion resource defined" +check_contains "$CFN_TEMPLATE" "InternetGatewayExclusionMode: allow-bidirectional" "CFN: VPC BPA exclusion allows bidirectional internet traffic" +check_contains "$CFN_TEMPLATE" "CreateVpcBpaExclusion" "CFN: BPA exclusion creation parameter defined" echo "" diff --git a/install.sh b/install.sh index 503f014..1ecf930 100755 --- a/install.sh +++ b/install.sh @@ -1558,6 +1558,43 @@ _check_codex_model_access() { fi } +# Resolve whether CloudFormation must create the default VPC-wide BPA +# exclusion. New VPCs always need one. For reused VPCs, avoid creating a +# duplicate when an active allow-bidirectional exclusion already covers it. +resolve_vpc_bpa_exclusion() { + CREATE_VPC_BPA_EXCLUSION="true" + VPC_BPA_EXCLUSION_STATUS="will be created" + + [[ -n "${EXISTING_VPC_ID:-}" ]] || return 0 + + local check_region="${DEPLOY_REGION:-$REGION}" + local exclusions_json + if ! exclusions_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$check_region" --max-results 100 --output json 2>&1); then + fail "Could not inspect VPC BPA exclusions for ${EXISTING_VPC_ID} in ${check_region}. Refusing to risk a duplicate exclusion. AWS said: ${exclusions_json}" + fi + + local target_suffix=":vpc/${EXISTING_VPC_ID}" + local active_mode + active_mode=$(printf '%s' "$exclusions_json" | jq -r --arg suffix "$target_suffix" ' + [.VpcBlockPublicAccessExclusions[]? + | select((.ResourceArn // "") | endswith($suffix)) + | select(.State == "create-in-progress" or .State == "create-complete" + or .State == "update-in-progress" or .State == "update-complete") + | .InternetGatewayExclusionMode][0] // empty + ') + + case "$active_mode" in + allow-bidirectional) + CREATE_VPC_BPA_EXCLUSION="false" + VPC_BPA_EXCLUSION_STATUS="already exists" + ;; + allow-egress) + fail "VPC ${EXISTING_VPC_ID} has an egress-only BPA exclusion. LowKey requires allow-bidirectional so ingress can reach this VPC. Update or remove the existing exclusion, then rerun the wizard." + ;; + esac +} + check_vpc_quota() { local check_region="${DEPLOY_REGION:-$REGION}" echo "" @@ -1620,6 +1657,7 @@ check_permissions() { if aws iam simulate-principal-policy \ --policy-source-arn "$CALLER_ARN" \ --action-names "cloudformation:CreateStack" "iam:CreateRole" "ec2:CreateVpc" \ + "ec2:CreateVpcBlockPublicAccessExclusion" "ec2:DescribeVpcBlockPublicAccessExclusions" \ --query 'EvaluationResults[?EvalDecision!=`allowed`].EvalActionName' \ --output text 2>/dev/null | grep -q "."; then warn "Some permissions may be missing." @@ -2420,7 +2458,7 @@ collect_security_config() { # Parameter source-of-truth: single mapping for CFN Console and CFN CLI # ============================================================================ # ⚠ KEEP THESE TWO ARRAYS IN SYNC — same order, same count -PARAM_CFN_NAMES=(EnvironmentName PackName ProfileName InstanceType DefaultModel ModelMode BedrockRegion LokiWatermark EnableBedrockForm EnableSecurityHub EnableGuardDuty EnableInspector EnableAccessAnalyzer EnableConfigRecorder ExistingVpcId ExistingSubnetId ExistingSubnetId2 RepoBranch KiroFromSecret TelegramBotTokenSecret TelegramUser Primary DailyDriver CodexModel EnableWebUIAuth WebUIAdminEmail EdgeLambdaVersionArn EdgeConfigSecretName EdgeConfigSecretArn SigningKeySecretName SigningKeySecretArn KirocrewTgBotTokenSecret KirocrewTgUserId) +PARAM_CFN_NAMES=(EnvironmentName PackName ProfileName InstanceType DefaultModel ModelMode BedrockRegion LokiWatermark EnableBedrockForm EnableSecurityHub EnableGuardDuty EnableInspector EnableAccessAnalyzer EnableConfigRecorder ExistingVpcId ExistingSubnetId ExistingSubnetId2 CreateVpcBpaExclusion RepoBranch KiroFromSecret TelegramBotTokenSecret TelegramUser Primary DailyDriver CodexModel EnableWebUIAuth WebUIAdminEmail EdgeLambdaVersionArn EdgeConfigSecretName EdgeConfigSecretArn SigningKeySecretName SigningKeySecretArn KirocrewTgBotTokenSecret KirocrewTgUserId) PARAM_VALUES=() # populated by build_deploy_params() # Per-pack default model (passed to CFN DefaultModel / bootstrap.sh --model). @@ -2473,6 +2511,7 @@ build_deploy_params() { "${EXISTING_VPC_ID:-}" "${EXISTING_SUBNET_ID:-}" "${EXISTING_SUBNET_ID2:-}" + "${CREATE_VPC_BPA_EXCLUSION:-true}" "$REPO_BRANCH" "${KIRO_FROM_SECRET:-}" "${TELEGRAM_BOT_TOKEN_SECRET:-}" @@ -2571,6 +2610,7 @@ show_summary() { summary+="Bedrock ${BEDROCK_REGION} (cross-region inference)\n" fi [[ -n "${EXISTING_VPC_ID:-}" ]] && summary+="VPC reuse ${EXISTING_VPC_ID}\n" + summary+="BPA exclusion ${VPC_BPA_EXCLUSION_STATUS:-will be created} (allows internet ingress to this VPC)\n" summary+="Security ${security_summary}\n" summary+="Environment ${ENV_NAME}" @@ -2808,6 +2848,8 @@ PACK_NAME="openclaw" # Default pack; overridden by collect_config EXISTING_VPC_ID="" EXISTING_SUBNET_ID="" EXISTING_SUBNET_ID2="" # KiroCrew ALB needs a 2nd AZ subnet on existing-VPC path +CREATE_VPC_BPA_EXCLUSION="true" +VPC_BPA_EXCLUSION_STATUS="will be created" # ============================================================================ # Ensure Lowkey-Session SSM document exists (instance-scoped, not account-wide) @@ -3355,6 +3397,11 @@ run_config_and_review() { check_existing_deployments fi + # Every LowKey VPC is bidirectionally excluded from regional VPC BPA. + # Resolve an existing exclusion before the final review so the summary can + # say whether CloudFormation will create it or it is already present. + resolve_vpc_bpa_exclusion + # VPC quota check (skip if reusing) if [[ -z "${EXISTING_VPC_ID:-}" ]]; then check_vpc_quota diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh new file mode 100755 index 0000000..e8d740c --- /dev/null +++ b/tests/test-vpc-bpa.sh @@ -0,0 +1,145 @@ +#!/usr/bin/env bash +# tests/test-vpc-bpa.sh — VPC BPA exclusion detection and template wiring +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +INSTALL_SH="${REPO_ROOT}/install.sh" +TEMPLATE="${REPO_ROOT}/deploy/cloudformation/template.yaml" + +PASS=0 +FAIL=0 +pass() { printf ' ✓ %s\n' "$1"; PASS=$((PASS + 1)); } +fail_test() { printf ' ✗ %s\n' "$1"; FAIL=$((FAIL + 1)); } +assert_eq() { + local description="$1" expected="$2" actual="$3" + if [[ "$expected" == "$actual" ]]; then + pass "$description" + else + fail_test "$description (expected: $expected, actual: $actual)" + fi +} +assert_contains() { + local description="$1" needle="$2" haystack="$3" + [[ "$haystack" == *"$needle"* ]] && pass "$description" || fail_test "$description (missing: $needle)" +} + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT +cat > "${TMPDIR}/functions.sh" <<'STUBS' +set -euo pipefail +EXISTING_VPC_ID="" +DEPLOY_REGION="us-east-1" +CREATE_VPC_BPA_EXCLUSION="true" +VPC_BPA_EXCLUSION_STATUS="will be created" +fail() { printf '%s\n' "$*" >&2; exit 1; } +STUBS +sed -n '/^resolve_vpc_bpa_exclusion() {/,/^}/p' "$INSTALL_SH" >> "${TMPDIR}/functions.sh" + +printf '── VPC BPA resolution ──\n' + +test_new_vpc_creates_exclusion() { + source "${TMPDIR}/functions.sh" + aws() { fail "AWS must not be called for a new VPC"; } + resolve_vpc_bpa_exclusion + assert_eq "new VPC creates exclusion" "true" "$CREATE_VPC_BPA_EXCLUSION" + assert_eq "new VPC review status" "will be created" "$VPC_BPA_EXCLUSION_STATUS" +} +test_new_vpc_creates_exclusion + +test_existing_bidirectional_exclusion_is_reused() { + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { + cat <<'JSON' +{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-1","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-0123456789abcdef0","State":"create-complete"}]} +JSON + } + resolve_vpc_bpa_exclusion + assert_eq "existing bidirectional exclusion is reused" "false" "$CREATE_VPC_BPA_EXCLUSION" + assert_eq "existing exclusion review status" "already exists" "$VPC_BPA_EXCLUSION_STATUS" +} +test_existing_bidirectional_exclusion_is_reused + +test_other_vpc_exclusion_is_ignored() { + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { + cat <<'JSON' +{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-other","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-fffffffffffffffff","State":"create-complete"}]} +JSON + } + resolve_vpc_bpa_exclusion + assert_eq "other VPC exclusion does not suppress creation" "true" "$CREATE_VPC_BPA_EXCLUSION" + assert_eq "missing target exclusion review status" "will be created" "$VPC_BPA_EXCLUSION_STATUS" +} +test_other_vpc_exclusion_is_ignored + +if ( + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { + cat <<'JSON' +{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-egress","InternetGatewayExclusionMode":"allow-egress","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-0123456789abcdef0","State":"create-complete"}]} +JSON + } + resolve_vpc_bpa_exclusion +) >/dev/null 2>&1; then + fail_test "egress-only exclusion is rejected because it does not allow ingress" +else + pass "egress-only exclusion is rejected because it does not allow ingress" +fi + +if ( + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { return 1; } + resolve_vpc_bpa_exclusion +) >/dev/null 2>&1; then + fail_test "existing VPC API failure stops before duplicate creation" +else + pass "existing VPC API failure stops before duplicate creation" +fi + +printf '\n── Installer and CloudFormation wiring ──\n' +summary_body="$(sed -n '/^show_summary() {/,/^}/p' "$INSTALL_SH")" +run_config_body="$(sed -n '/^run_config_and_review() {/,/^}/p' "$INSTALL_SH")" +assert_contains "summary shows BPA status" 'BPA exclusion ${VPC_BPA_EXCLUSION_STATUS:-will be created}' "$summary_body" +assert_contains "summary explains ingress effect" 'allows internet ingress to this VPC' "$summary_body" +assert_contains "config resolves BPA before review" 'resolve_vpc_bpa_exclusion' "$run_config_body" +assert_contains "installer passes BPA creation parameter" 'CreateVpcBpaExclusion' "$(grep '^PARAM_CFN_NAMES=' "$INSTALL_SH")" + +if python3 - "$TEMPLATE" <<'PY' +import sys, yaml +class Loader(yaml.SafeLoader): + pass +Loader.add_multi_constructor( + '!', + lambda loader, tag, node: loader.construct_scalar(node) + if isinstance(node, yaml.ScalarNode) + else loader.construct_sequence(node) + if isinstance(node, yaml.SequenceNode) + else loader.construct_mapping(node), +) +with open(sys.argv[1]) as stream: + doc = yaml.load(stream, Loader=Loader) +param = doc['Parameters']['CreateVpcBpaExclusion'] +assert param['Default'] == 'true' +assert param['AllowedValues'] == ['true', 'false'] +condition = doc['Conditions']['ShouldCreateVpcBpaExclusion'] +resource = doc['Resources']['VpcBpaExclusion'] +assert resource['Type'] == 'AWS::EC2::VPCBlockPublicAccessExclusion' +assert resource['Condition'] == 'ShouldCreateVpcBpaExclusion' +assert resource['Properties']['InternetGatewayExclusionMode'] == 'allow-bidirectional' +assert 'VpcId' in resource['Properties'] +instance_dependency = doc['Resources']['Instance']['Metadata']['VpcBpaExclusionDependency'] +assert 'VpcBpaExclusion' in repr(instance_dependency) +assert doc['Metadata']['AWSToolsMetrics']['AWSAgentToolkit'] == 'aws-cloudformation@2' +PY +then + pass "template creates a conditional VPC-wide bidirectional exclusion" +else + fail_test "template BPA exclusion wiring is invalid" +fi + +printf '\nPassed: %d Failed: %d\n' "$PASS" "$FAIL" +[[ "$FAIL" -eq 0 ]] From 80d6799f35557b7f1a9b9a0c408c75602b46db7c Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Tue, 25 Aug 2026 14:56:59 +0000 Subject: [PATCH 02/11] fix: enforce BPA readiness before bootstrap Reject reused exclusions until they are complete, force exclusion creation for new VPCs, make IAM simulation failures visible, include exclusion lifecycle permissions, and expand regression coverage. --- deploy/cloudformation/template.yaml | 5 ++- install.sh | 66 ++++++++++++++++++++--------- tests/test-vpc-bpa.sh | 54 ++++++++++++++++------- 3 files changed, 88 insertions(+), 37 deletions(-) diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index 5cc1aa3..bf16454 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -472,7 +472,10 @@ Conditions: IsApiKey: !Equals [!Ref ModelMode, 'api-key'] IsBedrock: !Equals [!Ref ModelMode, 'bedrock'] CreateNewVpc: !Equals [!Ref ExistingVpcId, ''] - ShouldCreateVpcBpaExclusion: !Equals [!Ref CreateVpcBpaExclusion, 'true'] + # New VPCs always require the exclusion; reused VPCs may supply an existing one. + ShouldCreateVpcBpaExclusion: !Or + - !Condition CreateNewVpc + - !Equals [!Ref CreateVpcBpaExclusion, 'true'] IsBuilder: !Equals [!Ref ProfileName, 'builder'] IsNotBuilder: !Not [!Condition IsBuilder] IsAccountAssistant: !Equals [!Ref ProfileName, 'account_assistant'] diff --git a/install.sh b/install.sh index 1ecf930..6b4ce9a 100755 --- a/install.sh +++ b/install.sh @@ -1575,22 +1575,38 @@ resolve_vpc_bpa_exclusion() { fi local target_suffix=":vpc/${EXISTING_VPC_ID}" - local active_mode - active_mode=$(printf '%s' "$exclusions_json" | jq -r --arg suffix "$target_suffix" ' + local exclusion_json + exclusion_json=$(printf '%s' "$exclusions_json" | jq -c --arg suffix "$target_suffix" ' [.VpcBlockPublicAccessExclusions[]? - | select((.ResourceArn // "") | endswith($suffix)) - | select(.State == "create-in-progress" or .State == "create-complete" - or .State == "update-in-progress" or .State == "update-complete") - | .InternetGatewayExclusionMode][0] // empty + | select((.ResourceArn // "") | endswith($suffix))][0] // empty ') - - case "$active_mode" in - allow-bidirectional) - CREATE_VPC_BPA_EXCLUSION="false" - VPC_BPA_EXCLUSION_STATUS="already exists" + [[ -n "$exclusion_json" ]] || return 0 + + local exclusion_state exclusion_mode exclusion_reason + exclusion_state=$(printf '%s' "$exclusion_json" | jq -r '.State // "unknown"') + exclusion_mode=$(printf '%s' "$exclusion_json" | jq -r '.InternetGatewayExclusionMode // "unknown"') + exclusion_reason=$(printf '%s' "$exclusion_json" | jq -r '.Reason // empty') + + case "$exclusion_state" in + create-complete|update-complete) + case "$exclusion_mode" in + allow-bidirectional) + CREATE_VPC_BPA_EXCLUSION="false" + VPC_BPA_EXCLUSION_STATUS="already exists" + ;; + allow-egress) + fail "VPC ${EXISTING_VPC_ID} has an egress-only BPA exclusion. LowKey requires allow-bidirectional so ingress can reach this VPC. Update or remove the existing exclusion, then rerun the wizard." + ;; + *) + fail "VPC ${EXISTING_VPC_ID} has a BPA exclusion with unsupported mode '${exclusion_mode}'. LowKey requires allow-bidirectional." + ;; + esac ;; - allow-egress) - fail "VPC ${EXISTING_VPC_ID} has an egress-only BPA exclusion. LowKey requires allow-bidirectional so ingress can reach this VPC. Update or remove the existing exclusion, then rerun the wizard." + create-in-progress|update-in-progress) + fail "VPC ${EXISTING_VPC_ID} has a BPA exclusion still in state '${exclusion_state}'. Wait for it to complete, then rerun the wizard so no pack starts before the exclusion is active." + ;; + *) + fail "VPC ${EXISTING_VPC_ID} has a BPA exclusion in unusable state '${exclusion_state}'${exclusion_reason:+: ${exclusion_reason}}. Resolve that exclusion, then rerun the wizard." ;; esac } @@ -1654,13 +1670,21 @@ check_vpc_quota() { check_permissions() { echo "" info "Checking permissions..." - if aws iam simulate-principal-policy \ - --policy-source-arn "$CALLER_ARN" \ - --action-names "cloudformation:CreateStack" "iam:CreateRole" "ec2:CreateVpc" \ - "ec2:CreateVpcBlockPublicAccessExclusion" "ec2:DescribeVpcBlockPublicAccessExclusions" \ - --query 'EvaluationResults[?EvalDecision!=`allowed`].EvalActionName' \ - --output text 2>/dev/null | grep -q "."; then - warn "Some permissions may be missing." + local denied_actions + if ! denied_actions=$(aws iam simulate-principal-policy \ + --policy-source-arn "$CALLER_ARN" \ + --action-names "cloudformation:CreateStack" "iam:CreateRole" "ec2:CreateVpc" \ + "ec2:CreateVpcBlockPublicAccessExclusion" "ec2:DescribeVpcBlockPublicAccessExclusions" \ + "ec2:ModifyVpcBlockPublicAccessExclusion" "ec2:DeleteVpcBlockPublicAccessExclusion" \ + --query 'EvaluationResults[?EvalDecision!=`allowed`].EvalActionName' \ + --output text 2>&1); then + warn "Could not verify deployment permissions: ${denied_actions}" + confirm_or_abort "Continue without verified permissions?" + return 0 + fi + + if [[ -n "$denied_actions" ]]; then + warn "Some permissions may be missing: ${denied_actions}" confirm_or_abort "Continue anyway?" else ok "Permissions verified" @@ -2610,7 +2634,7 @@ show_summary() { summary+="Bedrock ${BEDROCK_REGION} (cross-region inference)\n" fi [[ -n "${EXISTING_VPC_ID:-}" ]] && summary+="VPC reuse ${EXISTING_VPC_ID}\n" - summary+="BPA exclusion ${VPC_BPA_EXCLUSION_STATUS:-will be created} (allows internet ingress to this VPC)\n" + summary+="BPA exclusion: ${VPC_BPA_EXCLUSION_STATUS:-will be created} (allows internet ingress to this VPC)\n" summary+="Security ${security_summary}\n" summary+="Environment ${ENV_NAME}" diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index e8d740c..6a15082 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -74,20 +74,38 @@ JSON } test_other_vpc_exclusion_is_ignored -if ( - source "${TMPDIR}/functions.sh" - EXISTING_VPC_ID="vpc-0123456789abcdef0" - aws() { - cat <<'JSON' -{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-egress","InternetGatewayExclusionMode":"allow-egress","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-0123456789abcdef0","State":"create-complete"}]} -JSON - } - resolve_vpc_bpa_exclusion -) >/dev/null 2>&1; then - fail_test "egress-only exclusion is rejected because it does not allow ingress" -else - pass "egress-only exclusion is rejected because it does not allow ingress" -fi +assert_reused_exclusion_rejected() { + local state="$1" mode="$2" description="$3" reason="${4:-}" + if ( + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { + printf '{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-test","InternetGatewayExclusionMode":"%s","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-0123456789abcdef0","State":"%s","Reason":"%s"}]}\n' \ + "$mode" "$state" "$reason" + } + resolve_vpc_bpa_exclusion + ) >/dev/null 2>&1; then + fail_test "$description" + else + pass "$description" + fi +} + +assert_reused_exclusion_rejected \ + "create-in-progress" "allow-bidirectional" \ + "in-progress exclusion is rejected so bootstrap cannot start early" +assert_reused_exclusion_rejected \ + "update-in-progress" "allow-bidirectional" \ + "updating exclusion is rejected so bootstrap cannot start early" +assert_reused_exclusion_rejected \ + "create-failed" "allow-bidirectional" \ + "failed exclusion is rejected instead of attempting a duplicate" "service rejected request" +assert_reused_exclusion_rejected \ + "delete-in-progress" "allow-bidirectional" \ + "deleting exclusion is rejected instead of attempting a duplicate" +assert_reused_exclusion_rejected \ + "create-complete" "allow-egress" \ + "egress-only exclusion is rejected because it does not allow ingress" if ( source "${TMPDIR}/functions.sh" @@ -103,10 +121,14 @@ fi printf '\n── Installer and CloudFormation wiring ──\n' summary_body="$(sed -n '/^show_summary() {/,/^}/p' "$INSTALL_SH")" run_config_body="$(sed -n '/^run_config_and_review() {/,/^}/p' "$INSTALL_SH")" -assert_contains "summary shows BPA status" 'BPA exclusion ${VPC_BPA_EXCLUSION_STATUS:-will be created}' "$summary_body" +permissions_body="$(sed -n '/^check_permissions() {/,/^}/p' "$INSTALL_SH")" +assert_contains "summary shows exact BPA status label" 'BPA exclusion: ${VPC_BPA_EXCLUSION_STATUS:-will be created}' "$summary_body" assert_contains "summary explains ingress effect" 'allows internet ingress to this VPC' "$summary_body" assert_contains "config resolves BPA before review" 'resolve_vpc_bpa_exclusion' "$run_config_body" assert_contains "installer passes BPA creation parameter" 'CreateVpcBpaExclusion' "$(grep '^PARAM_CFN_NAMES=' "$INSTALL_SH")" +assert_contains "permission check includes BPA modification" 'ec2:ModifyVpcBlockPublicAccessExclusion' "$permissions_body" +assert_contains "permission check includes BPA deletion" 'ec2:DeleteVpcBlockPublicAccessExclusion' "$permissions_body" +assert_contains "permission simulation failure is handled separately" 'if ! denied_actions=$(aws iam simulate-principal-policy' "$permissions_body" if python3 - "$TEMPLATE" <<'PY' import sys, yaml @@ -126,6 +148,8 @@ param = doc['Parameters']['CreateVpcBpaExclusion'] assert param['Default'] == 'true' assert param['AllowedValues'] == ['true', 'false'] condition = doc['Conditions']['ShouldCreateVpcBpaExclusion'] +assert 'CreateNewVpc' in repr(condition) +assert 'CreateVpcBpaExclusion' in repr(condition) resource = doc['Resources']['VpcBpaExclusion'] assert resource['Type'] == 'AWS::EC2::VPCBlockPublicAccessExclusion' assert resource['Condition'] == 'ShouldCreateVpcBpaExclusion' From 6902f9301f25d95cb6b78306b114155128417a59 Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Tue, 25 Aug 2026 14:59:34 +0000 Subject: [PATCH 03/11] docs: clarify VPC BPA exclusion scope Disclose the VPC-wide bidirectional effect, internet egress, and external ownership of reused exclusions in the final deployment review. --- install.sh | 4 +++- tests/test-vpc-bpa.sh | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/install.sh b/install.sh index 6b4ce9a..f767c92 100755 --- a/install.sh +++ b/install.sh @@ -2634,7 +2634,9 @@ show_summary() { summary+="Bedrock ${BEDROCK_REGION} (cross-region inference)\n" fi [[ -n "${EXISTING_VPC_ID:-}" ]] && summary+="VPC reuse ${EXISTING_VPC_ID}\n" - summary+="BPA exclusion: ${VPC_BPA_EXCLUSION_STATUS:-will be created} (allows internet ingress to this VPC)\n" + local bpa_management_note="" + [[ "${CREATE_VPC_BPA_EXCLUSION:-true}" == "false" ]] && bpa_management_note="; external, not managed by this stack" + summary+="BPA exclusion: ${VPC_BPA_EXCLUSION_STATUS:-will be created} (VPC-wide bidirectional; allows internet ingress to this VPC and internet egress${bpa_management_note})\n" summary+="Security ${security_summary}\n" summary+="Environment ${ENV_NAME}" diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index 6a15082..9d6fd5e 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -123,7 +123,10 @@ summary_body="$(sed -n '/^show_summary() {/,/^}/p' "$INSTALL_SH")" run_config_body="$(sed -n '/^run_config_and_review() {/,/^}/p' "$INSTALL_SH")" permissions_body="$(sed -n '/^check_permissions() {/,/^}/p' "$INSTALL_SH")" assert_contains "summary shows exact BPA status label" 'BPA exclusion: ${VPC_BPA_EXCLUSION_STATUS:-will be created}' "$summary_body" +assert_contains "summary explains VPC-wide bidirectional scope" 'VPC-wide bidirectional' "$summary_body" assert_contains "summary explains ingress effect" 'allows internet ingress to this VPC' "$summary_body" +assert_contains "summary explains egress effect" 'and internet egress' "$summary_body" +assert_contains "summary identifies reused exclusion as external" 'external, not managed by this stack' "$summary_body" assert_contains "config resolves BPA before review" 'resolve_vpc_bpa_exclusion' "$run_config_body" assert_contains "installer passes BPA creation parameter" 'CreateVpcBpaExclusion' "$(grep '^PARAM_CFN_NAMES=' "$INSTALL_SH")" assert_contains "permission check includes BPA modification" 'ec2:ModifyVpcBlockPublicAccessExclusion' "$permissions_body" From 688f54f6292e7ba72c39096a25f3bee420dd4c9a Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Tue, 25 Aug 2026 15:07:10 +0000 Subject: [PATCH 04/11] fix: retain BPA exclusions for reused VPCs Separate new-VPC and reused-VPC exclusion resources so shared exclusions survive deletion or replacement of the LowKey stack that originally created them. --- deploy/cloudformation/template.yaml | 52 +++++++++++++++++++++++------ deploy/test-templates.sh | 3 ++ tests/test-vpc-bpa.sh | 21 ++++++++---- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index bf16454..b88f454 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -472,9 +472,10 @@ Conditions: IsApiKey: !Equals [!Ref ModelMode, 'api-key'] IsBedrock: !Equals [!Ref ModelMode, 'bedrock'] CreateNewVpc: !Equals [!Ref ExistingVpcId, ''] - # New VPCs always require the exclusion; reused VPCs may supply an existing one. - ShouldCreateVpcBpaExclusion: !Or - - !Condition CreateNewVpc + # New VPCs always get a stack-owned exclusion. When LowKey must create one + # for a reused VPC, retain it independently of this individual deployment. + CreateExistingVpcBpaExclusion: !And + - !Not [!Condition CreateNewVpc] - !Equals [!Ref CreateVpcBpaExclusion, 'true'] IsBuilder: !Equals [!Ref ProfileName, 'builder'] IsNotBuilder: !Not [!Condition IsBuilder] @@ -519,11 +520,10 @@ Resources: - Key: loki:pack Value: !Ref PackName - # LowKey currently uses an IGW-backed public VPC for bootstrap and optional - # public endpoints. Keep the VPC reachable when regional BPA is enabled. + # New LowKey VPCs own their exclusion: stack deletion removes both in order. VpcBpaExclusion: Type: AWS::EC2::VPCBlockPublicAccessExclusion - Condition: ShouldCreateVpcBpaExclusion + Condition: CreateNewVpc Metadata: com.aws.cloudformation.Context: why: Keep LowKey's IGW-backed VPC reachable when regional VPC BPA is enabled. @@ -532,7 +532,34 @@ Resources: mutable: review-required Properties: InternetGatewayExclusionMode: allow-bidirectional - VpcId: !If [CreateNewVpc, !Ref VPC, !Ref ExistingVpcId] + VpcId: !Ref VPC + Tags: + - Key: Name + Value: !Sub '${EnvironmentName}-vpc-bpa-exclusion' + - Key: loki:managed + Value: 'true' + - Key: loki:watermark + Value: !Ref LokiWatermark + - Key: loki:pack + Value: !Ref PackName + + # An exclusion on a reused VPC is VPC-wide shared infrastructure. Retain it + # when this deployment is deleted or replaced so another LowKey stack using + # the same VPC cannot lose internet connectivity with the owning stack. + ExistingVpcBpaExclusion: + Type: AWS::EC2::VPCBlockPublicAccessExclusion + Condition: CreateExistingVpcBpaExclusion + DeletionPolicy: Retain + UpdateReplacePolicy: Retain + Metadata: + com.aws.cloudformation.Context: + why: Give a reused VPC a BPA exclusion whose lifetime is independent of one LowKey deployment. + must: + - Retain on stack deletion or replacement because other stacks may share this VPC-wide exclusion. + mutable: review-required + Properties: + InternetGatewayExclusionMode: allow-bidirectional + VpcId: !Ref ExistingVpcId Tags: - Key: Name Value: !Sub '${EnvironmentName}-vpc-bpa-exclusion' @@ -1876,12 +1903,15 @@ Resources: Instance: Type: AWS::EC2::Instance Metadata: - # Optional dependency: when this stack creates the exclusion, wait for it - # before UserData attempts internet downloads. Existing exclusions need no wait. + # Wait for whichever exclusion this stack creates before UserData starts. + # A complete exclusion supplied by a reused VPC needs no stack dependency. VpcBpaExclusionDependency: !If - - ShouldCreateVpcBpaExclusion + - CreateNewVpc - !Ref VpcBpaExclusion - - existing + - !If + - CreateExistingVpcBpaExclusion + - !Ref ExistingVpcBpaExclusion + - existing CreationPolicy: ResourceSignal: Timeout: PT30M diff --git a/deploy/test-templates.sh b/deploy/test-templates.sh index ec39356..ce34acf 100644 --- a/deploy/test-templates.sh +++ b/deploy/test-templates.sh @@ -48,6 +48,9 @@ check_contains "$CFN_TEMPLATE" "Deployed agent pack" "CFN: PackName in Outputs" check_contains "$CFN_TEMPLATE" "AWS::EC2::VPCBlockPublicAccessExclusion" "CFN: VPC BPA exclusion resource defined" check_contains "$CFN_TEMPLATE" "InternetGatewayExclusionMode: allow-bidirectional" "CFN: VPC BPA exclusion allows bidirectional internet traffic" check_contains "$CFN_TEMPLATE" "CreateVpcBpaExclusion" "CFN: BPA exclusion creation parameter defined" +check_contains "$CFN_TEMPLATE" "ExistingVpcBpaExclusion:" "CFN: reused VPC BPA exclusion has independent lifecycle" +check_contains "$CFN_TEMPLATE" "DeletionPolicy: Retain" "CFN: reused VPC BPA exclusion retained on stack deletion" +check_contains "$CFN_TEMPLATE" "UpdateReplacePolicy: Retain" "CFN: reused VPC BPA exclusion retained on replacement" echo "" diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index 9d6fd5e..ba496a7 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -150,16 +150,25 @@ with open(sys.argv[1]) as stream: param = doc['Parameters']['CreateVpcBpaExclusion'] assert param['Default'] == 'true' assert param['AllowedValues'] == ['true', 'false'] -condition = doc['Conditions']['ShouldCreateVpcBpaExclusion'] +condition = doc['Conditions']['CreateExistingVpcBpaExclusion'] assert 'CreateNewVpc' in repr(condition) assert 'CreateVpcBpaExclusion' in repr(condition) -resource = doc['Resources']['VpcBpaExclusion'] -assert resource['Type'] == 'AWS::EC2::VPCBlockPublicAccessExclusion' -assert resource['Condition'] == 'ShouldCreateVpcBpaExclusion' -assert resource['Properties']['InternetGatewayExclusionMode'] == 'allow-bidirectional' -assert 'VpcId' in resource['Properties'] +new_vpc_resource = doc['Resources']['VpcBpaExclusion'] +assert new_vpc_resource['Type'] == 'AWS::EC2::VPCBlockPublicAccessExclusion' +assert new_vpc_resource['Condition'] == 'CreateNewVpc' +assert new_vpc_resource['Properties']['InternetGatewayExclusionMode'] == 'allow-bidirectional' +assert new_vpc_resource['Properties']['VpcId'] == 'VPC' +assert 'DeletionPolicy' not in new_vpc_resource +existing_vpc_resource = doc['Resources']['ExistingVpcBpaExclusion'] +assert existing_vpc_resource['Type'] == 'AWS::EC2::VPCBlockPublicAccessExclusion' +assert existing_vpc_resource['Condition'] == 'CreateExistingVpcBpaExclusion' +assert existing_vpc_resource['Properties']['InternetGatewayExclusionMode'] == 'allow-bidirectional' +assert existing_vpc_resource['Properties']['VpcId'] == 'ExistingVpcId' +assert existing_vpc_resource['DeletionPolicy'] == 'Retain' +assert existing_vpc_resource['UpdateReplacePolicy'] == 'Retain' instance_dependency = doc['Resources']['Instance']['Metadata']['VpcBpaExclusionDependency'] assert 'VpcBpaExclusion' in repr(instance_dependency) +assert 'ExistingVpcBpaExclusion' in repr(instance_dependency) assert doc['Metadata']['AWSToolsMetrics']['AWSAgentToolkit'] == 'aws-cloudformation@2' PY then From a092a7d36ac8bee966b0a4556fe5f28adada10da Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Tue, 25 Aug 2026 15:11:29 +0000 Subject: [PATCH 05/11] fix: verify VPC BPA before pack bootstrap Revalidate a complete bidirectional exclusion from the instance before cloning or running any pack, covering direct CloudFormation callers and external exclusion drift. --- deploy/cloudformation/template.yaml | 42 +++++++++++++++++++++++++++++ tests/test-vpc-bpa.sh | 7 +++++ 2 files changed, 49 insertions(+) diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index b88f454..e1b0143 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -914,6 +914,16 @@ Resources: - secretsmanager:GetSecretValue - secretsmanager:DescribeSecret Resource: !Sub 'arn:aws:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:${KirocrewTgBotTokenSecret}*' + # Revalidate the VPC-wide prerequisite immediately before bootstrap. + # Inline attachment avoids an IAM propagation race at instance launch. + - PolicyName: !Sub '${EnvironmentName}-vpc-bpa-read' + PolicyDocument: + Version: '2012-10-17' + Statement: + - Sid: VerifyVpcBpaExclusion + Effect: Allow + Action: ec2:DescribeVpcBlockPublicAccessExclusions + Resource: '*' Tags: - Key: Name Value: !Sub '${EnvironmentName}-role' @@ -2007,6 +2017,38 @@ Resources: aws cloudformation signal-resource --stack-name "${!STACK_NAME}" --logical-resource-id Instance --unique-id "$_INSTANCE_ID" --status FAILURE --region "$REGION" 2>/dev/null || true fi ' ERR + # Fail closed before any pack code starts. This revalidates exclusions + # supplied by reused VPCs and protects direct CloudFormation callers + # that bypass the installer's pre-deployment check. + _IMDS_TOKEN=$(curl -sf -X PUT http://169.254.169.254/latest/api/token \ + -H "X-aws-ec2-metadata-token-ttl-seconds: 60") + _PRIMARY_MAC=$(curl -sf -H "X-aws-ec2-metadata-token: $_IMDS_TOKEN" \ + http://169.254.169.254/latest/meta-data/network/interfaces/macs/ | head -1) + _TARGET_VPC_ID=$(curl -sf -H "X-aws-ec2-metadata-token: $_IMDS_TOKEN" \ + "http://169.254.169.254/latest/meta-data/network/interfaces/macs/$_PRIMARY_MAC/vpc-id") + if [[ -z "$_TARGET_VPC_ID" ]]; then + echo "FATAL: could not resolve the instance VPC before BPA validation" >&2 + exit 1 + fi + + _BPA_READY=false + for _attempt in 1 2 3 4 5 6; do + _BPA_MATCHES=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$REGION" --max-results 100 \ + --query "length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')])" \ + --output text 2>/dev/null || echo 0) + if [[ "$_BPA_MATCHES" =~ ^[1-9][0-9]*$ ]]; then + _BPA_READY=true + break + fi + echo "VPC BPA exclusion not ready (attempt $_attempt/6); retrying in 10s..." + sleep 10 + done + if [[ "$_BPA_READY" != "true" ]]; then + echo "FATAL: VPC $_TARGET_VPC_ID lacks a complete allow-bidirectional BPA exclusion; refusing to start pack bootstrap" >&2 + exit 1 + fi + # Ensure git is available (not present on all AMIs) command -v git &>/dev/null || dnf install -y git 2>/dev/null || yum install -y git # Clone repo with retry (GitHub blips shouldn't kill bootstrap) diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index ba496a7..cd224d1 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -145,8 +145,15 @@ Loader.add_multi_constructor( if isinstance(node, yaml.SequenceNode) else loader.construct_mapping(node), ) +template_text = open(sys.argv[1]).read() with open(sys.argv[1]) as stream: doc = yaml.load(stream, Loader=Loader) +assert 'ec2:DescribeVpcBlockPublicAccessExclusions' in template_text +bpa_check = template_text.index('aws ec2 describe-vpc-block-public-access-exclusions') +git_clone = template_text.index('git clone --depth 1') +pack_bootstrap = template_text.index('bash /tmp/lowkey/deploy/bootstrap.sh') +assert bpa_check < git_clone < pack_bootstrap +assert 'refusing to start pack bootstrap' in template_text param = doc['Parameters']['CreateVpcBpaExclusion'] assert param['Default'] == 'true' assert param['AllowedValues'] == ['true', 'false'] From 915a22bbabbcc14e00c2e831b3127ab6a20869ce Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Tue, 25 Aug 2026 15:14:14 +0000 Subject: [PATCH 06/11] fix: aggregate paginated BPA validation Use JSON output so AWS CLI combines all exclusion pages before evaluating the pre-bootstrap count. --- deploy/cloudformation/template.yaml | 2 +- tests/test-vpc-bpa.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index e1b0143..e3528db 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -2036,7 +2036,7 @@ Resources: _BPA_MATCHES=$(aws ec2 describe-vpc-block-public-access-exclusions \ --region "$REGION" --max-results 100 \ --query "length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')])" \ - --output text 2>/dev/null || echo 0) + --output json 2>/dev/null || echo 0) if [[ "$_BPA_MATCHES" =~ ^[1-9][0-9]*$ ]]; then _BPA_READY=true break diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index cd224d1..4703735 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -149,6 +149,7 @@ template_text = open(sys.argv[1]).read() with open(sys.argv[1]) as stream: doc = yaml.load(stream, Loader=Loader) assert 'ec2:DescribeVpcBlockPublicAccessExclusions' in template_text +assert '--output json 2>/dev/null || echo 0' in template_text bpa_check = template_text.index('aws ec2 describe-vpc-block-public-access-exclusions') git_clone = template_text.index('git clone --depth 1') pack_bootstrap = template_text.index('bash /tmp/lowkey/deploy/bootstrap.sh') From d884fab712ef1cffd59532aea9dee03a94bbdfe3 Mon Sep 17 00:00:00 2001 From: "Bode (Kiro Crew Issue Radar)" Date: Tue, 25 Aug 2026 15:40:17 +0000 Subject: [PATCH 07/11] fix: paginate BPA lookups and order bootstrap networking Follow NextToken in the installer and instance checks, make EC2 wait for public routing before UserData, and document the exclusion parameter, VPC-wide scope, and retention behavior. --- deploy/cloudformation/README.md | 10 ++++++ deploy/cloudformation/template.yaml | 38 +++++++++++++++++++--- docs/reference/cloudformation.mdx | 26 +++++++++++++++ install.sh | 42 +++++++++++++++++------- tests/test-vpc-bpa.sh | 50 ++++++++++++++++++++++++++++- 5 files changed, 149 insertions(+), 17 deletions(-) diff --git a/deploy/cloudformation/README.md b/deploy/cloudformation/README.md index ce6ba16..63f1c1b 100644 --- a/deploy/cloudformation/README.md +++ b/deploy/cloudformation/README.md @@ -77,6 +77,16 @@ aws cloudformation create-stack-instances \ - The `CreationPolicy` with `ResourceSignal` ensures the stack only completes when the instance is fully bootstrapped - Requires `CAPABILITY_NAMED_IAM` due to named IAM roles and users +## VPC Block Public Access exclusion + +The stack creates a VPC-wide `allow-bidirectional` VPC Block Public Access exclusion so bootstrap egress and public endpoints keep working when BPA is enabled. This exempts the **entire VPC** for internet ingress and egress, not only the LowKey instance. + +- New VPC: always created and owned by the stack (`CreateVpcBpaExclusion` is ignored). +- Reused VPC with `CreateVpcBpaExclusion=true`: created with `DeletionPolicy: Retain` / `UpdateReplacePolicy: Retain`, so it survives stack deletion. +- Reused VPC with `CreateVpcBpaExclusion=false`: nothing is created; the VPC must already have a complete `allow-bidirectional` exclusion. + +Reusing a VPC that already has one **requires** `CreateVpcBpaExclusion=false`, or the stack fails trying to create a duplicate. UserData revalidates the exclusion before running any pack and aborts if it is missing. + ## Next Steps See [Next Steps After Deployment](../README.md#next-steps-after-deployment) for bootstrap scripts setup. diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index e3528db..fad4466 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -1922,6 +1922,12 @@ Resources: - CreateExistingVpcBpaExclusion - !Ref ExistingVpcBpaExclusion - existing + # On the new-VPC path the public internet route must exist before UserData + # makes its first AWS call; a reused VPC brings its own routing. + PublicRoutingDependency: !If + - CreateNewVpc + - !Sub '${VPCGatewayAttachment}|${PublicRoute}|${PublicSubnetRouteTableAssociation}' + - existing CreationPolicy: ResourceSignal: Timeout: PT30M @@ -2033,11 +2039,33 @@ Resources: _BPA_READY=false for _attempt in 1 2 3 4 5 6; do - _BPA_MATCHES=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$REGION" --max-results 100 \ - --query "length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')])" \ - --output json 2>/dev/null || echo 0) - if [[ "$_BPA_MATCHES" =~ ^[1-9][0-9]*$ ]]; then + # This API has no CLI paginator, so walk NextToken explicitly: one + # request per page, each returning " ". + _BPA_TOKEN="None" + _BPA_FOUND=0 + while :; do + if [[ "$_BPA_TOKEN" == "None" ]]; then + _BPA_PAGE=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$REGION" --max-results 100 \ + --query "[length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')]), NextToken]" \ + --output text 2>/dev/null || echo "") + else + _BPA_PAGE=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$REGION" --max-results 100 --starting-token "$_BPA_TOKEN" \ + --query "[length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')]), NextToken]" \ + --output text 2>/dev/null || echo "") + fi + [[ -n "$_BPA_PAGE" ]] || break + _BPA_COUNT=$(printf '%s' "$_BPA_PAGE" | awk '{print $1}') + _BPA_NEXT=$(printf '%s' "$_BPA_PAGE" | awk '{print $2}') + if [[ "$_BPA_COUNT" =~ ^[1-9][0-9]*$ ]]; then + _BPA_FOUND=1 + break + fi + [[ -n "$_BPA_NEXT" && "$_BPA_NEXT" != "None" ]] || break + _BPA_TOKEN="$_BPA_NEXT" + done + if [[ "$_BPA_FOUND" == "1" ]]; then _BPA_READY=true break fi diff --git a/docs/reference/cloudformation.mdx b/docs/reference/cloudformation.mdx index 1a20093..ededc20 100644 --- a/docs/reference/cloudformation.mdx +++ b/docs/reference/cloudformation.mdx @@ -39,6 +39,7 @@ You rarely set these by hand — `install.sh` computes them from your pack + pro | `EnableConfigRecorder` | `true` | Enable Config recorder | | `ExistingVpcId` | `vpc-0abc123` | Reuse an existing VPC instead of creating one | | `ExistingSubnetId` | `subnet-0def456` | Public subnet in the existing VPC | +| `CreateVpcBpaExclusion` | `true` | Create the VPC-wide `allow-bidirectional` Block Public Access exclusion. Set to `false` only when reusing a VPC that already has a complete one | | `RepoBranch` | `main` | Git branch of the lowkey repo to clone | | `SSHAllowedCidr` | `127.0.0.1/32` | CIDR allowed to SSH (default disables SSH) | | `KiroFromSecret` | `/lowkey/kiro-api-key` | Secrets Manager id/arn for Kiro API key (kiro-cli only) | @@ -49,6 +50,24 @@ You rarely set these by hand — `install.sh` computes them from your pack + pro `LiteLLMApiKey` and `ProviderApiKey` are `NoEcho: true` — they won't appear in stack history or describe-stacks output, but they **do** pass through UserData (which is Base64-encoded in stack metadata). For long-lived secrets, prefer the `--kiro-from-secret` / Secrets Manager pattern which flows only a reference. +## VPC Block Public Access exclusion + +Every deployment needs its VPC exempt from [VPC Block Public Access](https://docs.aws.amazon.com/vpc/latest/userguide/security-vpc-bpa.html): bootstrap downloads require internet egress, and public endpoints (such as the KiroCrew ALB) require ingress. The stack creates a VPC-wide `allow-bidirectional` exclusion to guarantee this. + + +`allow-bidirectional` exempts the **entire VPC**, allowing internet ingress *and* egress for every resource in it — not just the LowKey instance. Deploy only into a VPC where that is acceptable. + + +| `ExistingVpcId` | `CreateVpcBpaExclusion` | Behavior | +|---|---|---| +| empty (new VPC) | `true` or `false` | Always creates a stack-owned exclusion; the parameter is ignored because a new VPC always needs one | +| set | `true` | Creates an exclusion with `DeletionPolicy: Retain` and `UpdateReplacePolicy: Retain`, so the shared VPC-wide exclusion outlives this stack | +| set | `false` | Creates nothing; the VPC must already have a complete `allow-bidirectional` exclusion | + +Reusing a VPC that already has a complete exclusion **requires** `CreateVpcBpaExclusion=false` — otherwise CloudFormation attempts a duplicate exclusion and the stack fails. `install.sh` detects this and sets the parameter for you. + +Before cloning the repo or running any pack, UserData verifies its own VPC has a complete `allow-bidirectional` exclusion and aborts if it does not, so a misconfigured direct deployment fails closed instead of bootstrapping without connectivity. + ## Deploying the template directly ```bash @@ -105,3 +124,10 @@ The stack owns the VPC, EC2, IAM — everything vanishes together. Security serv If you reused an existing VPC via `ExistingVpcId`, the VPC is **not** deleted (you brought it, you keep it). + + +Two BPA exceptions to "everything vanishes together": + +- An exclusion this stack created for a **reused** VPC is `Retain`ed and survives deletion. Remove it with `aws ec2 delete-vpc-block-public-access-exclusion --exclusion-id ` once no deployment needs that VPC exempt. +- An exclusion created for a **new** VPC is deleted with the stack. If another LowKey deployment was later pointed at that same VPC, it loses the exemption — recreate one, or redeploy that stack with `CreateVpcBpaExclusion=true`. + diff --git a/install.sh b/install.sh index f767c92..a475d45 100755 --- a/install.sh +++ b/install.sh @@ -1568,18 +1568,38 @@ resolve_vpc_bpa_exclusion() { [[ -n "${EXISTING_VPC_ID:-}" ]] || return 0 local check_region="${DEPLOY_REGION:-$REGION}" - local exclusions_json - if ! exclusions_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$check_region" --max-results 100 --output json 2>&1); then - fail "Could not inspect VPC BPA exclusions for ${EXISTING_VPC_ID} in ${check_region}. Refusing to risk a duplicate exclusion. AWS said: ${exclusions_json}" - fi - local target_suffix=":vpc/${EXISTING_VPC_ID}" - local exclusion_json - exclusion_json=$(printf '%s' "$exclusions_json" | jq -c --arg suffix "$target_suffix" ' - [.VpcBlockPublicAccessExclusions[]? - | select((.ResourceArn // "") | endswith($suffix))][0] // empty - ') + local exclusion_json="" next_token="" page_json page_match + + # This API is not auto-paginated by the AWS CLI, so follow NextToken + # explicitly. Missing a later page would hide an existing exclusion and + # cause a duplicate-create attempt. + while :; do + if [[ -n "$next_token" ]]; then + page_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$check_region" --max-results 100 --starting-token "$next_token" \ + --output json 2>&1) || page_json="" + else + page_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$check_region" --max-results 100 --output json 2>&1) || page_json="" + fi + if [[ -z "$page_json" ]] || ! printf '%s' "$page_json" | jq -e . >/dev/null 2>&1; then + fail "Could not inspect VPC BPA exclusions for ${EXISTING_VPC_ID} in ${check_region}. Refusing to risk a duplicate exclusion. AWS said: ${page_json}" + fi + + page_match=$(printf '%s' "$page_json" | jq -c --arg suffix "$target_suffix" ' + [.VpcBlockPublicAccessExclusions[]? + | select((.ResourceArn // "") | endswith($suffix))][0] // empty + ') + if [[ -n "$page_match" ]]; then + exclusion_json="$page_match" + break + fi + + next_token=$(printf '%s' "$page_json" | jq -r '.NextToken // empty') + [[ -n "$next_token" ]] || break + done + [[ -n "$exclusion_json" ]] || return 0 local exclusion_state exclusion_mode exclusion_reason diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index 4703735..cb0b1b8 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -74,6 +74,51 @@ JSON } test_other_vpc_exclusion_is_ignored +test_exclusion_on_second_page_is_found() { + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { + local starting_token="" + while [[ $# -gt 0 ]]; do + [[ "$1" == "--starting-token" ]] && starting_token="$2" + shift + done + if [[ -z "$starting_token" ]]; then + cat <<'JSON' +{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-other","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-fffffffffffffffff","State":"create-complete"}],"NextToken":"page2"} +JSON + else + cat <<'JSON' +{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-page2","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-0123456789abcdef0","State":"create-complete"}]} +JSON + fi + } + resolve_vpc_bpa_exclusion + assert_eq "exclusion on a later page is found" "false" "$CREATE_VPC_BPA_EXCLUSION" + assert_eq "paginated exclusion review status" "already exists" "$VPC_BPA_EXCLUSION_STATUS" +} +test_exclusion_on_second_page_is_found + +test_pagination_ends_without_match() { + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { + local starting_token="" + while [[ $# -gt 0 ]]; do + [[ "$1" == "--starting-token" ]] && starting_token="$2" + shift + done + if [[ -z "$starting_token" ]]; then + printf '{"VpcBlockPublicAccessExclusions":[],"NextToken":"page2"}\n' + else + printf '{"VpcBlockPublicAccessExclusions":[]}\n' + fi + } + resolve_vpc_bpa_exclusion + assert_eq "exhausted pages without a match still creates" "true" "$CREATE_VPC_BPA_EXCLUSION" +} +test_pagination_ends_without_match + assert_reused_exclusion_rejected() { local state="$1" mode="$2" description="$3" reason="${4:-}" if ( @@ -149,7 +194,7 @@ template_text = open(sys.argv[1]).read() with open(sys.argv[1]) as stream: doc = yaml.load(stream, Loader=Loader) assert 'ec2:DescribeVpcBlockPublicAccessExclusions' in template_text -assert '--output json 2>/dev/null || echo 0' in template_text +assert '--starting-token "$_BPA_TOKEN"' in template_text bpa_check = template_text.index('aws ec2 describe-vpc-block-public-access-exclusions') git_clone = template_text.index('git clone --depth 1') pack_bootstrap = template_text.index('bash /tmp/lowkey/deploy/bootstrap.sh') @@ -177,6 +222,9 @@ assert existing_vpc_resource['UpdateReplacePolicy'] == 'Retain' instance_dependency = doc['Resources']['Instance']['Metadata']['VpcBpaExclusionDependency'] assert 'VpcBpaExclusion' in repr(instance_dependency) assert 'ExistingVpcBpaExclusion' in repr(instance_dependency) +routing_dependency = repr(doc['Resources']['Instance']['Metadata']['PublicRoutingDependency']) +for required in ('VPCGatewayAttachment', 'PublicRoute', 'PublicSubnetRouteTableAssociation'): + assert required in routing_dependency assert doc['Metadata']['AWSToolsMetrics']['AWSAgentToolkit'] == 'aws-cloudformation@2' PY then From d4f98d90e5156d3c62e8e4219d32feb0c8273f48 Mon Sep 17 00:00:00 2001 From: "Bode (Kiro Crew Issue Radar)" Date: Tue, 25 Aug 2026 15:49:46 +0000 Subject: [PATCH 08/11] fix: correct BPA pagination flag and IMDS path Use the API --next-token flag instead of the paginator-only --starting-token, strip the IMDS trailing slash so UserData cannot abort before validation, guard against repeating pagination tokens, and document all four deployment cases. --- deploy/cloudformation/README.md | 20 +++++++++++++++--- deploy/cloudformation/template.yaml | 17 ++++++++++++--- docs/reference/cloudformation.mdx | 7 ++++--- install.sh | 19 +++++++++++++---- tests/test-vpc-bpa.sh | 32 ++++++++++++++++++++++------- 5 files changed, 75 insertions(+), 20 deletions(-) diff --git a/deploy/cloudformation/README.md b/deploy/cloudformation/README.md index 63f1c1b..5b346ae 100644 --- a/deploy/cloudformation/README.md +++ b/deploy/cloudformation/README.md @@ -81,12 +81,26 @@ aws cloudformation create-stack-instances \ The stack creates a VPC-wide `allow-bidirectional` VPC Block Public Access exclusion so bootstrap egress and public endpoints keep working when BPA is enabled. This exempts the **entire VPC** for internet ingress and egress, not only the LowKey instance. -- New VPC: always created and owned by the stack (`CreateVpcBpaExclusion` is ignored). -- Reused VPC with `CreateVpcBpaExclusion=true`: created with `DeletionPolicy: Retain` / `UpdateReplacePolicy: Retain`, so it survives stack deletion. -- Reused VPC with `CreateVpcBpaExclusion=false`: nothing is created; the VPC must already have a complete `allow-bidirectional` exclusion. +| `ExistingVpcId` | `CreateVpcBpaExclusion` | Behavior | +|---|---|---| +| empty (new VPC) | `true` | Stack-owned exclusion, deleted with the stack | +| empty (new VPC) | `false` | Same as `true` — ignored, because a new VPC always needs one | +| set (reused VPC) | `true` | Created with `DeletionPolicy: Retain` / `UpdateReplacePolicy: Retain`, so it survives stack deletion | +| set (reused VPC) | `false` | Nothing created; the VPC must already have a complete `allow-bidirectional` exclusion | Reusing a VPC that already has one **requires** `CreateVpcBpaExclusion=false`, or the stack fails trying to create a duplicate. UserData revalidates the exclusion before running any pack and aborts if it is missing. +Cleanup, once no deployment needs that VPC exempt: + +```bash +aws ec2 describe-vpc-block-public-access-exclusions \ + --query 'VpcBlockPublicAccessExclusions[].[ExclusionId,ResourceArn,State]' --output text +aws ec2 delete-vpc-block-public-access-exclusion --exclusion-id +``` + +> **Warning** +> A **new-VPC** exclusion is stack-owned and deleted with its stack. If another LowKey deployment was later pointed at that same VPC, deleting the first stack removes the exemption the second one depends on. Recreate an exclusion, or redeploy the remaining stack with `CreateVpcBpaExclusion=true`. + ## Next Steps See [Next Steps After Deployment](../README.md#next-steps-after-deployment) for bootstrap scripts setup. diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index fad4466..1535026 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -2030,6 +2030,9 @@ Resources: -H "X-aws-ec2-metadata-token-ttl-seconds: 60") _PRIMARY_MAC=$(curl -sf -H "X-aws-ec2-metadata-token: $_IMDS_TOKEN" \ http://169.254.169.254/latest/meta-data/network/interfaces/macs/ | head -1) + # IMDS directory listings return each entry with a trailing slash; + # leaving it in would build a rejected double-slash vpc-id path. + _PRIMARY_MAC="${!_PRIMARY_MAC%/}" _TARGET_VPC_ID=$(curl -sf -H "X-aws-ec2-metadata-token: $_IMDS_TOKEN" \ "http://169.254.169.254/latest/meta-data/network/interfaces/macs/$_PRIMARY_MAC/vpc-id") if [[ -z "$_TARGET_VPC_ID" ]]; then @@ -2039,9 +2042,12 @@ Resources: _BPA_READY=false for _attempt in 1 2 3 4 5 6; do - # This API has no CLI paginator, so walk NextToken explicitly: one - # request per page, each returning " ". + # This API has no CLI paginator, so walk NextToken explicitly with + # the API's own --next-token flag: one request per page, each + # returning " ". _BPA_TOKEN="None" + _BPA_PREV="" + _BPA_PAGES=0 _BPA_FOUND=0 while :; do if [[ "$_BPA_TOKEN" == "None" ]]; then @@ -2051,7 +2057,7 @@ Resources: --output text 2>/dev/null || echo "") else _BPA_PAGE=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$REGION" --max-results 100 --starting-token "$_BPA_TOKEN" \ + --region "$REGION" --max-results 100 --next-token "$_BPA_TOKEN" \ --query "[length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')]), NextToken]" \ --output text 2>/dev/null || echo "") fi @@ -2063,6 +2069,11 @@ Resources: break fi [[ -n "$_BPA_NEXT" && "$_BPA_NEXT" != "None" ]] || break + # Stop on a repeating or endless token instead of looping forever. + [[ "$_BPA_NEXT" != "$_BPA_PREV" ]] || break + _BPA_PAGES=$((_BPA_PAGES + 1)) + [[ "$_BPA_PAGES" -lt 50 ]] || break + _BPA_PREV="$_BPA_TOKEN" _BPA_TOKEN="$_BPA_NEXT" done if [[ "$_BPA_FOUND" == "1" ]]; then diff --git a/docs/reference/cloudformation.mdx b/docs/reference/cloudformation.mdx index ededc20..3d50c76 100644 --- a/docs/reference/cloudformation.mdx +++ b/docs/reference/cloudformation.mdx @@ -60,9 +60,10 @@ Every deployment needs its VPC exempt from [VPC Block Public Access](https://doc | `ExistingVpcId` | `CreateVpcBpaExclusion` | Behavior | |---|---|---| -| empty (new VPC) | `true` or `false` | Always creates a stack-owned exclusion; the parameter is ignored because a new VPC always needs one | -| set | `true` | Creates an exclusion with `DeletionPolicy: Retain` and `UpdateReplacePolicy: Retain`, so the shared VPC-wide exclusion outlives this stack | -| set | `false` | Creates nothing; the VPC must already have a complete `allow-bidirectional` exclusion | +| empty (new VPC) | `true` | Creates a stack-owned exclusion, deleted with the stack | +| empty (new VPC) | `false` | Same as `true` — the parameter is ignored, because a new VPC always needs an exclusion | +| set (reused VPC) | `true` | Creates an exclusion with `DeletionPolicy: Retain` and `UpdateReplacePolicy: Retain`, so the shared VPC-wide exclusion outlives this stack | +| set (reused VPC) | `false` | Creates nothing; the VPC must already have a complete `allow-bidirectional` exclusion | Reusing a VPC that already has a complete exclusion **requires** `CreateVpcBpaExclusion=false` — otherwise CloudFormation attempts a duplicate exclusion and the stack fails. `install.sh` detects this and sets the parameter for you. diff --git a/install.sh b/install.sh index a475d45..f236ef8 100755 --- a/install.sh +++ b/install.sh @@ -1569,15 +1569,17 @@ resolve_vpc_bpa_exclusion() { local check_region="${DEPLOY_REGION:-$REGION}" local target_suffix=":vpc/${EXISTING_VPC_ID}" - local exclusion_json="" next_token="" page_json page_match + local exclusion_json="" next_token="" prev_token="" page_json page_match + local page_count=0 - # This API is not auto-paginated by the AWS CLI, so follow NextToken - # explicitly. Missing a later page would hide an existing exclusion and + # This API has no AWS CLI paginator, so follow NextToken explicitly using the + # API's own --next-token flag (--starting-token only exists for paginated + # operations). Missing a later page would hide an existing exclusion and # cause a duplicate-create attempt. while :; do if [[ -n "$next_token" ]]; then page_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$check_region" --max-results 100 --starting-token "$next_token" \ + --region "$check_region" --max-results 100 --next-token "$next_token" \ --output json 2>&1) || page_json="" else page_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ @@ -1596,8 +1598,17 @@ resolve_vpc_bpa_exclusion() { break fi + prev_token="$next_token" next_token=$(printf '%s' "$page_json" | jq -r '.NextToken // empty') [[ -n "$next_token" ]] || break + # Fail closed rather than looping forever on a repeating or endless token. + if [[ "$next_token" == "$prev_token" ]]; then + fail "VPC BPA exclusion lookup in ${check_region} returned a repeating pagination token. Refusing to loop; rerun the wizard." + fi + page_count=$((page_count + 1)) + if [[ "$page_count" -ge 50 ]]; then + fail "VPC BPA exclusion lookup in ${check_region} exceeded 50 pages without a result. Refusing to loop; rerun the wizard." + fi done [[ -n "$exclusion_json" ]] || return 0 diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index cb0b1b8..7a6290c 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -78,12 +78,14 @@ test_exclusion_on_second_page_is_found() { source "${TMPDIR}/functions.sh" EXISTING_VPC_ID="vpc-0123456789abcdef0" aws() { - local starting_token="" + local next_token="" while [[ $# -gt 0 ]]; do - [[ "$1" == "--starting-token" ]] && starting_token="$2" + # Reject the paginator-only flag: this operation has no CLI paginator. + [[ "$1" == "--starting-token" ]] && return 252 + [[ "$1" == "--next-token" ]] && next_token="$2" shift done - if [[ -z "$starting_token" ]]; then + if [[ -z "$next_token" ]]; then cat <<'JSON' {"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-other","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-fffffffffffffffff","State":"create-complete"}],"NextToken":"page2"} JSON @@ -103,12 +105,13 @@ test_pagination_ends_without_match() { source "${TMPDIR}/functions.sh" EXISTING_VPC_ID="vpc-0123456789abcdef0" aws() { - local starting_token="" + local next_token="" while [[ $# -gt 0 ]]; do - [[ "$1" == "--starting-token" ]] && starting_token="$2" + [[ "$1" == "--starting-token" ]] && return 252 + [[ "$1" == "--next-token" ]] && next_token="$2" shift done - if [[ -z "$starting_token" ]]; then + if [[ -z "$next_token" ]]; then printf '{"VpcBlockPublicAccessExclusions":[],"NextToken":"page2"}\n' else printf '{"VpcBlockPublicAccessExclusions":[]}\n' @@ -119,6 +122,18 @@ test_pagination_ends_without_match() { } test_pagination_ends_without_match +# A service returning the same token forever must not hang the wizard. +if ( + source "${TMPDIR}/functions.sh" + EXISTING_VPC_ID="vpc-0123456789abcdef0" + aws() { printf '{"VpcBlockPublicAccessExclusions":[],"NextToken":"same"}\n'; } + resolve_vpc_bpa_exclusion +) >/dev/null 2>&1; then + fail_test "repeating pagination token fails closed instead of looping" +else + pass "repeating pagination token fails closed instead of looping" +fi + assert_reused_exclusion_rejected() { local state="$1" mode="$2" description="$3" reason="${4:-}" if ( @@ -194,7 +209,10 @@ template_text = open(sys.argv[1]).read() with open(sys.argv[1]) as stream: doc = yaml.load(stream, Loader=Loader) assert 'ec2:DescribeVpcBlockPublicAccessExclusions' in template_text -assert '--starting-token "$_BPA_TOKEN"' in template_text +assert '--next-token "$_BPA_TOKEN"' in template_text +assert '--starting-token' not in template_text +assert '"$_BPA_NEXT" != "$_BPA_PREV"' in template_text +assert '${!_PRIMARY_MAC%/}' in template_text bpa_check = template_text.index('aws ec2 describe-vpc-block-public-access-exclusions') git_clone = template_text.index('git clone --depth 1') pack_bootstrap = template_text.index('bash /tmp/lowkey/deploy/bootstrap.sh') From 32f13460826c13c5712c4c3f2db450c11fc6a683 Mon Sep 17 00:00:00 2001 From: "Bode (Kiro Crew Issue Radar)" Date: Tue, 25 Aug 2026 15:56:54 +0000 Subject: [PATCH 09/11] refactor: drop BPA pagination for a documented 100-exclusion limit Inspect only the first 100 exclusions in both callers, keep the malformed-response and IMDS fixes, and document the limit and its failure modes for direct CloudFormation users. --- deploy/cloudformation/README.md | 11 ++++++ deploy/cloudformation/template.yaml | 42 ++++---------------- docs/reference/cloudformation.mdx | 4 ++ install.sh | 57 ++++++++------------------- tests/test-vpc-bpa.sh | 61 ++++++++--------------------- 5 files changed, 55 insertions(+), 120 deletions(-) diff --git a/deploy/cloudformation/README.md b/deploy/cloudformation/README.md index 5b346ae..2ade65f 100644 --- a/deploy/cloudformation/README.md +++ b/deploy/cloudformation/README.md @@ -101,6 +101,17 @@ aws ec2 delete-vpc-block-public-access-exclusion --exclusion-id > **Warning** > A **new-VPC** exclusion is stack-owned and deleted with its stack. If another LowKey deployment was later pointed at that same VPC, deleting the first stack removes the exemption the second one depends on. Recreate an exclusion, or redeploy the remaining stack with `CreateVpcBpaExclusion=true`. +### Limitation: first 100 exclusions only + +Exclusion discovery is deliberately not paginated. Both the installer and the instance-side check inspect only the **first 100** BPA exclusions in the region (`--max-results 100`). The default quota is well under that, so this is an accepted edge case for now. + +If an account holds more than 100 exclusions and the target VPC's exclusion falls outside that first page: + +- The installer treats it as absent and passes `CreateVpcBpaExclusion=true`, so CloudFormation attempts a duplicate and the stack fails with a create error. +- The instance-side check likewise does not see it and refuses to start pack bootstrap, so the deployment fails closed rather than running without internet access. + +Workaround: pass `CreateVpcBpaExclusion=false` explicitly when you know the VPC already has a complete `allow-bidirectional` exclusion, or reduce the number of exclusions in the region. + ## Next Steps See [Next Steps After Deployment](../README.md#next-steps-after-deployment) for bootstrap scripts setup. diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index 1535026..d6380e5 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -2042,41 +2042,13 @@ Resources: _BPA_READY=false for _attempt in 1 2 3 4 5 6; do - # This API has no CLI paginator, so walk NextToken explicitly with - # the API's own --next-token flag: one request per page, each - # returning " ". - _BPA_TOKEN="None" - _BPA_PREV="" - _BPA_PAGES=0 - _BPA_FOUND=0 - while :; do - if [[ "$_BPA_TOKEN" == "None" ]]; then - _BPA_PAGE=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$REGION" --max-results 100 \ - --query "[length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')]), NextToken]" \ - --output text 2>/dev/null || echo "") - else - _BPA_PAGE=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$REGION" --max-results 100 --next-token "$_BPA_TOKEN" \ - --query "[length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')]), NextToken]" \ - --output text 2>/dev/null || echo "") - fi - [[ -n "$_BPA_PAGE" ]] || break - _BPA_COUNT=$(printf '%s' "$_BPA_PAGE" | awk '{print $1}') - _BPA_NEXT=$(printf '%s' "$_BPA_PAGE" | awk '{print $2}') - if [[ "$_BPA_COUNT" =~ ^[1-9][0-9]*$ ]]; then - _BPA_FOUND=1 - break - fi - [[ -n "$_BPA_NEXT" && "$_BPA_NEXT" != "None" ]] || break - # Stop on a repeating or endless token instead of looping forever. - [[ "$_BPA_NEXT" != "$_BPA_PREV" ]] || break - _BPA_PAGES=$((_BPA_PAGES + 1)) - [[ "$_BPA_PAGES" -lt 50 ]] || break - _BPA_PREV="$_BPA_TOKEN" - _BPA_TOKEN="$_BPA_NEXT" - done - if [[ "$_BPA_FOUND" == "1" ]]; then + # Known limitation: only the first 100 exclusions in the region are + # inspected; see the BPA section in deploy/cloudformation/README.md. + _BPA_COUNT=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$REGION" --max-results 100 \ + --query "length(VpcBlockPublicAccessExclusions[?ends_with(ResourceArn, ':vpc/$_TARGET_VPC_ID') && InternetGatewayExclusionMode == 'allow-bidirectional' && (State == 'create-complete' || State == 'update-complete')])" \ + --output text 2>/dev/null || echo 0) + if [[ "$_BPA_COUNT" =~ ^[1-9][0-9]*$ ]]; then _BPA_READY=true break fi diff --git a/docs/reference/cloudformation.mdx b/docs/reference/cloudformation.mdx index 3d50c76..9abe663 100644 --- a/docs/reference/cloudformation.mdx +++ b/docs/reference/cloudformation.mdx @@ -69,6 +69,10 @@ Reusing a VPC that already has a complete exclusion **requires** `CreateVpcBpaEx Before cloning the repo or running any pack, UserData verifies its own VPC has a complete `allow-bidirectional` exclusion and aborts if it does not, so a misconfigured direct deployment fails closed instead of bootstrapping without connectivity. + +**Limitation — first 100 exclusions only.** Discovery is not paginated: the installer and the instance-side check inspect only the first 100 BPA exclusions in the region. The default quota is far below that, so this is an accepted edge case. In a region holding more, an exclusion outside that first page reads as absent — the stack then fails on a duplicate create, and bootstrap refuses to start rather than running unexempted. Pass `CreateVpcBpaExclusion=false` explicitly when you know the VPC already has one. + + ## Deploying the template directly ```bash diff --git a/install.sh b/install.sh index f236ef8..0842abf 100755 --- a/install.sh +++ b/install.sh @@ -1569,47 +1569,22 @@ resolve_vpc_bpa_exclusion() { local check_region="${DEPLOY_REGION:-$REGION}" local target_suffix=":vpc/${EXISTING_VPC_ID}" - local exclusion_json="" next_token="" prev_token="" page_json page_match - local page_count=0 - - # This API has no AWS CLI paginator, so follow NextToken explicitly using the - # API's own --next-token flag (--starting-token only exists for paginated - # operations). Missing a later page would hide an existing exclusion and - # cause a duplicate-create attempt. - while :; do - if [[ -n "$next_token" ]]; then - page_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$check_region" --max-results 100 --next-token "$next_token" \ - --output json 2>&1) || page_json="" - else - page_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ - --region "$check_region" --max-results 100 --output json 2>&1) || page_json="" - fi - if [[ -z "$page_json" ]] || ! printf '%s' "$page_json" | jq -e . >/dev/null 2>&1; then - fail "Could not inspect VPC BPA exclusions for ${EXISTING_VPC_ID} in ${check_region}. Refusing to risk a duplicate exclusion. AWS said: ${page_json}" - fi - - page_match=$(printf '%s' "$page_json" | jq -c --arg suffix "$target_suffix" ' - [.VpcBlockPublicAccessExclusions[]? - | select((.ResourceArn // "") | endswith($suffix))][0] // empty - ') - if [[ -n "$page_match" ]]; then - exclusion_json="$page_match" - break - fi - - prev_token="$next_token" - next_token=$(printf '%s' "$page_json" | jq -r '.NextToken // empty') - [[ -n "$next_token" ]] || break - # Fail closed rather than looping forever on a repeating or endless token. - if [[ "$next_token" == "$prev_token" ]]; then - fail "VPC BPA exclusion lookup in ${check_region} returned a repeating pagination token. Refusing to loop; rerun the wizard." - fi - page_count=$((page_count + 1)) - if [[ "$page_count" -ge 50 ]]; then - fail "VPC BPA exclusion lookup in ${check_region} exceeded 50 pages without a result. Refusing to loop; rerun the wizard." - fi - done + local exclusions_json + + # Known limitation: only the first 100 exclusions in the region are + # inspected. Accounts holding more are an accepted edge case for now — see + # the BPA section in deploy/cloudformation/README.md. + if ! exclusions_json=$(aws ec2 describe-vpc-block-public-access-exclusions \ + --region "$check_region" --max-results 100 --output json 2>&1) \ + || ! printf '%s' "$exclusions_json" | jq -e . >/dev/null 2>&1; then + fail "Could not inspect VPC BPA exclusions for ${EXISTING_VPC_ID} in ${check_region}. Refusing to risk a duplicate exclusion. AWS said: ${exclusions_json}" + fi + + local exclusion_json + exclusion_json=$(printf '%s' "$exclusions_json" | jq -c --arg suffix "$target_suffix" ' + [.VpcBlockPublicAccessExclusions[]? + | select((.ResourceArn // "") | endswith($suffix))][0] // empty + ') [[ -n "$exclusion_json" ]] || return 0 diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index 7a6290c..f52b41d 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -74,64 +74,37 @@ JSON } test_other_vpc_exclusion_is_ignored -test_exclusion_on_second_page_is_found() { +test_exclusion_beyond_first_page_is_not_inspected() { source "${TMPDIR}/functions.sh" EXISTING_VPC_ID="vpc-0123456789abcdef0" + # Documented limitation: only the first 100 exclusions are inspected, so a + # target hidden behind NextToken is treated as absent rather than paginated. aws() { - local next_token="" while [[ $# -gt 0 ]]; do - # Reject the paginator-only flag: this operation has no CLI paginator. - [[ "$1" == "--starting-token" ]] && return 252 - [[ "$1" == "--next-token" ]] && next_token="$2" + # No continuation flag may be used by either caller. + case "$1" in + --next-token|--starting-token) return 252 ;; + esac shift done - if [[ -z "$next_token" ]]; then - cat <<'JSON' -{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-other","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-fffffffffffffffff","State":"create-complete"}],"NextToken":"page2"} -JSON - else - cat <<'JSON' -{"VpcBlockPublicAccessExclusions":[{"ExclusionId":"vpcbpa-excl-page2","InternetGatewayExclusionMode":"allow-bidirectional","ResourceArn":"arn:aws:ec2:us-east-1:123456789012:vpc/vpc-0123456789abcdef0","State":"create-complete"}]} -JSON - fi - } - resolve_vpc_bpa_exclusion - assert_eq "exclusion on a later page is found" "false" "$CREATE_VPC_BPA_EXCLUSION" - assert_eq "paginated exclusion review status" "already exists" "$VPC_BPA_EXCLUSION_STATUS" -} -test_exclusion_on_second_page_is_found - -test_pagination_ends_without_match() { - source "${TMPDIR}/functions.sh" - EXISTING_VPC_ID="vpc-0123456789abcdef0" - aws() { - local next_token="" - while [[ $# -gt 0 ]]; do - [[ "$1" == "--starting-token" ]] && return 252 - [[ "$1" == "--next-token" ]] && next_token="$2" - shift - done - if [[ -z "$next_token" ]]; then - printf '{"VpcBlockPublicAccessExclusions":[],"NextToken":"page2"}\n' - else - printf '{"VpcBlockPublicAccessExclusions":[]}\n' - fi + printf '{"VpcBlockPublicAccessExclusions":[],"NextToken":"page2"}\n' } resolve_vpc_bpa_exclusion - assert_eq "exhausted pages without a match still creates" "true" "$CREATE_VPC_BPA_EXCLUSION" + assert_eq "single-page lookup does not paginate" "true" "$CREATE_VPC_BPA_EXCLUSION" + assert_eq "single-page lookup review status" "will be created" "$VPC_BPA_EXCLUSION_STATUS" } -test_pagination_ends_without_match +test_exclusion_beyond_first_page_is_not_inspected -# A service returning the same token forever must not hang the wizard. +# Malformed API output must fail closed rather than silently creating a duplicate. if ( source "${TMPDIR}/functions.sh" EXISTING_VPC_ID="vpc-0123456789abcdef0" - aws() { printf '{"VpcBlockPublicAccessExclusions":[],"NextToken":"same"}\n'; } + aws() { printf 'not json\n'; } resolve_vpc_bpa_exclusion ) >/dev/null 2>&1; then - fail_test "repeating pagination token fails closed instead of looping" + fail_test "malformed exclusion output stops before duplicate creation" else - pass "repeating pagination token fails closed instead of looping" + pass "malformed exclusion output stops before duplicate creation" fi assert_reused_exclusion_rejected() { @@ -209,9 +182,9 @@ template_text = open(sys.argv[1]).read() with open(sys.argv[1]) as stream: doc = yaml.load(stream, Loader=Loader) assert 'ec2:DescribeVpcBlockPublicAccessExclusions' in template_text -assert '--next-token "$_BPA_TOKEN"' in template_text assert '--starting-token' not in template_text -assert '"$_BPA_NEXT" != "$_BPA_PREV"' in template_text +assert '--next-token' not in template_text +assert '--max-results 100' in template_text assert '${!_PRIMARY_MAC%/}' in template_text bpa_check = template_text.index('aws ec2 describe-vpc-block-public-access-exclusions') git_clone = template_text.index('git clone --depth 1') From 8ab9a836aa6d88c6afacb0d2ff72bdedb7f49f0a Mon Sep 17 00:00:00 2001 From: "Bode (Kiro Crew Issue Radar)" Date: Tue, 25 Aug 2026 16:01:51 +0000 Subject: [PATCH 10/11] fix: report BPA validation failures through the ERR trap Fail with a failing command instead of exit so the UserData ERR trap publishes SSM status and signals CloudFormation, avoiding a 30-minute CreationPolicy timeout. --- deploy/cloudformation/template.yaml | 7 +++++-- tests/test-vpc-bpa.sh | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index d6380e5..4081b4a 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -2037,7 +2037,9 @@ Resources: "http://169.254.169.254/latest/meta-data/network/interfaces/macs/$_PRIMARY_MAC/vpc-id") if [[ -z "$_TARGET_VPC_ID" ]]; then echo "FATAL: could not resolve the instance VPC before BPA validation" >&2 - exit 1 + # Fail via a failing command, not 'exit': only that triggers the ERR + # trap above, which publishes SSM status and signals CFN promptly. + false fi _BPA_READY=false @@ -2057,7 +2059,8 @@ Resources: done if [[ "$_BPA_READY" != "true" ]]; then echo "FATAL: VPC $_TARGET_VPC_ID lacks a complete allow-bidirectional BPA exclusion; refusing to start pack bootstrap" >&2 - exit 1 + # Fail via a failing command, not 'exit', so the ERR trap reports it. + false fi # Ensure git is available (not present on all AMIs) diff --git a/tests/test-vpc-bpa.sh b/tests/test-vpc-bpa.sh index f52b41d..01e27b3 100755 --- a/tests/test-vpc-bpa.sh +++ b/tests/test-vpc-bpa.sh @@ -186,6 +186,14 @@ assert '--starting-token' not in template_text assert '--next-token' not in template_text assert '--max-results 100' in template_text assert '${!_PRIMARY_MAC%/}' in template_text +# Failures in the BPA block must reach the ERR trap (which publishes SSM status +# and signals CloudFormation); a bare 'exit' skips it and stalls until timeout. +bpa_block = template_text[ + template_text.index('# Fail closed before any pack code starts'): + template_text.index('# Ensure git is available') +] +assert 'exit 1' not in bpa_block +assert bpa_block.count('false') >= 2 bpa_check = template_text.index('aws ec2 describe-vpc-block-public-access-exclusions') git_clone = template_text.index('git clone --depth 1') pack_bootstrap = template_text.index('bash /tmp/lowkey/deploy/bootstrap.sh') From c4e60079fe9d233d2d3c9cc29b15d84842323228 Mon Sep 17 00:00:00 2001 From: "Bode (Kiro Crew Issue Radar)" Date: Tue, 25 Aug 2026 16:06:11 +0000 Subject: [PATCH 11/11] docs: correct the BPA exclusion limit workaround CreateVpcBpaExclusion=false only avoids the duplicate create; the instance-side check reads the same first 100 results and still aborts, so document that the count must be reduced instead. --- deploy/cloudformation/README.md | 2 +- docs/reference/cloudformation.mdx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy/cloudformation/README.md b/deploy/cloudformation/README.md index 2ade65f..06a7a63 100644 --- a/deploy/cloudformation/README.md +++ b/deploy/cloudformation/README.md @@ -110,7 +110,7 @@ If an account holds more than 100 exclusions and the target VPC's exclusion fall - The installer treats it as absent and passes `CreateVpcBpaExclusion=true`, so CloudFormation attempts a duplicate and the stack fails with a create error. - The instance-side check likewise does not see it and refuses to start pack bootstrap, so the deployment fails closed rather than running without internet access. -Workaround: pass `CreateVpcBpaExclusion=false` explicitly when you know the VPC already has a complete `allow-bidirectional` exclusion, or reduce the number of exclusions in the region. +Workaround: none that keeps the deployment working. Setting `CreateVpcBpaExclusion=false` only avoids the duplicate-create failure — the instance-side check reads the same first 100 results, so bootstrap still refuses to start. Deploying into such a region requires bringing the region's exclusion count back under 100, so the target VPC's exclusion appears in the first page of results. ## Next Steps diff --git a/docs/reference/cloudformation.mdx b/docs/reference/cloudformation.mdx index 9abe663..c0aed20 100644 --- a/docs/reference/cloudformation.mdx +++ b/docs/reference/cloudformation.mdx @@ -70,7 +70,7 @@ Reusing a VPC that already has a complete exclusion **requires** `CreateVpcBpaEx Before cloning the repo or running any pack, UserData verifies its own VPC has a complete `allow-bidirectional` exclusion and aborts if it does not, so a misconfigured direct deployment fails closed instead of bootstrapping without connectivity. -**Limitation — first 100 exclusions only.** Discovery is not paginated: the installer and the instance-side check inspect only the first 100 BPA exclusions in the region. The default quota is far below that, so this is an accepted edge case. In a region holding more, an exclusion outside that first page reads as absent — the stack then fails on a duplicate create, and bootstrap refuses to start rather than running unexempted. Pass `CreateVpcBpaExclusion=false` explicitly when you know the VPC already has one. +**Limitation — first 100 exclusions only.** Discovery is not paginated: the installer and the instance-side check each inspect only the first 100 BPA exclusions in the region. The default quota is far below that, so this is an accepted edge case. In a region holding more, an exclusion outside that first page reads as absent — the stack fails on a duplicate create, and bootstrap refuses to start rather than running unexempted. Setting `CreateVpcBpaExclusion=false` is **not** a workaround: it avoids the duplicate create, but the instance-side check reads the same first 100 results and still aborts. Deploying into such a region requires bringing the region's exclusion count back under 100. ## Deploying the template directly