feat(cloud): fail launch preflight when a private hosted zone shadows a bootstrap download host

[timwukp]

Problem / Motivation

kirocrew cloud launch can fail deterministically, ~4 minutes in, with an error that names the wrong layer:

'kiro-cli did not install (the chat backend would not work) :: ...brotli-1.0.9-4.amzn2023.0.2.aarch64  |  nodejs-1:18.20.8-

The real error, visible only in the untruncated reason:

--- installing kiro-cli ---
curl: (6) Could not resolve host: desktop-release.q.us-east-1.amazonaws.com

A private hosted zone owns its apex and every subdomain. An interface VPC endpoint with private DNS enabled creates one — Amazon Q's com.amazonaws.<region>.q endpoint creates a zone for q.<region>.amazonaws.com. kiro-cli is downloaded from desktop-release.q.us-east-1.amazonaws.com, which sits inside that subtree, so the lookup is answered by the private zone, finds no record, and returns NXDOMAIN without falling through to public DNS.

Nothing in the preflight looks at the resolver path. cloud doctor and [1/6]–[2/6] validate the aws CLI, session-manager-plugin, and EC2/CloudFormation/SSM reachability — all of which go green — and then the launch spends ~4 minutes creating an IAM role, security group, instance profile and EC2 instance before dying on a condition that was knowable from one API call up front.

Why it matters

discover_network() targets the account's default VPC by design (isDefault=true, ec2.py), so a Q endpoint in the default VPC means remote crew cannot launch at all — deterministically, on every attempt. Q endpoints via PrivateLink are common in enterprise and regulated accounts, and such an account is more likely to have one, not less.

The cost is not just the wasted provisioning, it is the misdirection. In the reported case this took six launches to identify, and produced two confidently wrong root causes on the way:

1. The fail() reason budget (~1 KB) was filled with successful dnf transaction output — brotli, nodejs 18, python3.11-setuptools — while the actual error was truncated away. Those RPM names look exactly like a failing package install, so the first three attempts chased a "transient dnf mirror race".
2. Once the real error was visible, the absent --retry on that curl looked like the cause. Adding --retry 5 --retry-delay 2 --retry-connrefused and relaunching produced 6 consecutive failures over ~10 seconds — proving the failure is deterministic and that retries cannot help.

Meanwhile cdn.amazonlinux.com and nodejs.org resolve fine in the same run, which is what makes this look impossible until you inspect the VPC's resolver config.

What changed (motivation → approach → change)

Goal: turn a deterministic, misattributed, post-provisioning failure into a pre-launch error with a working suggestion.

Approach. Detect the general condition rather than special-casing Amazon Q: "is any private hosted zone bound to the selected VPC authoritative for a host the bootstrap must download from?" One read-only call, no new infrastructure, and it catches future shadowing by any endpoint or user-created zone. Rejected alternatives: resolving the name from the launching machine (wrong vantage point — the machine outside the VPC resolves it fine), and probing from the instance (too late, the instance is the thing we are trying not to create).

What was built:

• _BOOTSTRAP_DOWNLOAD_HOSTS — the hosts the UserData fetches. The kiro-cli URL is pinned to us-east-1 in the template regardless of launch region, so the constant is literal to match rather than interpolating the region.
• _zone_shadows_host() — pure, label-boundary suffix match. A plain endswith would report xq.us-east-1.amazonaws.com as shadowing desktop-release.q.us-east-1.amazonaws.com; this does not.
• shadowed_download_hosts() — one route53:ListHostedZonesByVPC call, returns (host, zone) pairs.
• assert_download_hosts_resolvable() — raises aws.AWSError naming the zone, the host, and the --subnet remedy.
• Wired at the single point where the --subnet and auto-discovery paths converge, so both are covered by one call site, inside the existing try that cleans up the uploaded source on failure.
• iam.py: new Route53DnsPreflight statement. ListHostedZonesByVPC does not support resource-level permissions, hence Resource: "*".

A missing permission is deliberately non-fatal. iam.py is an explicit action allowlist with no describe wildcard, so anyone running an older launch policy has no route53:ListHostedZonesByVPC. On AWSError the check logs and returns empty. Losing the early warning is acceptable; breaking a launch that would otherwise succeed is not.

Design question for reviewers

A positive finding is currently fatal. The tradeoff: it can false-positive. If someone runs a private zone for nodejs.org pointing at a legitimate internal mirror, resolution works today and this check would start blocking them.

Alternatives if you would rather not take that risk:

• warn instead of raise (keeps the diagnosis, loses the guarantee);
• only raise for hosts in an AWS service-endpoint namespace (*.amazonaws.com), warn for the rest — the collision this addresses is in that namespace, a third-party internal mirror never is;
• confirm the record is genuinely absent via ListResourceRecordSets before raising. This needs another permission and does not always work: the Q endpoint's zone is service-managed and returned AccessDenied when enumerated.

Happy to switch to any of these — say which and I will update.

Tests

pytest test/test_cloud_ec2.py test/test_cloud_iam.py → 139 passed.

7 new tests in TestDnsPreflight (test/test_cloud_ec2.py), mocking AWS at the cloud.aws.checked_json chokepoint per the file's existing convention:

Test	Locks in
test_zone_shadows_subdomain_and_apex	a zone shadows both its apex and any subdomain
test_match_is_on_label_boundaries	xq.… / notnodejs.org do not match — the endswith false positive
test_empty_zone_never_shadows	empty and root (.) zone names are inert
test_detects_q_endpoint_zone	the real case: q.us-east-1.amazonaws.com flags the kiro-cli host, efs.… does not
test_clean_vpc_has_no_hits	an unrelated private zone produces no finding
test_missing_permission_is_not_fatal	AccessDenied returns empty and the assert wrapper stays quiet — the backwards-compat guarantee
test_assert_raises_with_actionable_text	the raised message contains both the zone and --subnet, so the user gets a way forward and not just a diagnosis

Extended test_covers_core_launch_actions (test/test_cloud_iam.py) so route53:ListHostedZonesByVPC is guarded by a test rather than only present in the policy.

Manual verification

Verified against a live account, as a controlled experiment — same instance size, region, AMI and template, with only the VPC changed:

	Default VPC (has com.amazonaws.us-east-1.q, private DNS on)	VPC without that endpoint
Result	5/5 launches failed, same NXDOMAIN on desktop-release.q.us-east-1.amazonaws.com	CREATE_COMPLETE, signed in, dashboard up

The read-only calls this change relies on were exercised by hand against both VPCs: route53 list-hosted-zones-by-vpc returns q.us-east-1.amazonaws.com. for the failing VPC and no shadowing zone for the working one — i.e. the check would have raised on the first and passed on the second, before any resource was created.

One limit worth stating: the private zone's record set could not be enumerated (AccessDenied — it is service-managed by the endpoint), so "the zone has no desktop-release record" is inferred from the observed NXDOMAIN rather than read directly. The A/B narrows the alternatives considerably but does not independently rule out some other property of that VPC.

Why no screenshot: backend-only change — two Python modules under src/kiro_crew/cloud/ and their tests. No frontend path is touched and nothing renders differently.

Related Issues

Fixes #7522

Scope of that Fixes, stated explicitly. This PR resolves what #7522 reports — a launch that dies ~4 minutes into provisioning with a failure reason naming the wrong step — by detecting the shadowing before any resource is created. It does not remove the underlying namespace collision. That durable fix (serve the kiro-cli artifact from a domain no customer-createable private hosted zone can be authoritative for) is tracked separately in #7822, so it survives this merge instead of closing with it.

I originally withheld the closing keyword to keep the scope honest. In practice that was the wrong call: with no linked PR, closedByPullRequestsReferences stayed empty, automated triage lanes screened #7522 as uncovered and re-selected it repeatedly, and one of those passes built a diagnosis on the retracted root cause still sitting in the issue body. The body now carries a retraction banner, and splitting the durable fix into #7822 keeps the tracker accurate without leaving the reported defect unlinked.

Also unaddressed here, and carried into #7822: fail()'s ~1 KB reason budget keeping successful dnf output while truncating the actual error away, and the failure message naming the install rather than the download.

Pattern harvest

Rule candidate: review-prompt
Pattern: a host the bootstrap must reach lives under a domain that a VPC-endpoint private hosted zone can be authoritative for — any such host is unreachable inside that VPC regardless of public DNS, and no retry helps.

The generalizable form is the check itself (suffix-match every required download host against the VPC's private zones) rather than a lint rule, which is why it ships as a preflight instead of a static check. The narrower reviewable invariant: a new hard-coded download host in the bootstrap should be added to _BOOTSTRAP_DOWNLOAD_HOSTS in the same change, or the preflight silently stops covering it.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A: no user-facing doc change; the new failure text is self-describing and names the --subnet remedy inline
• No secrets, credentials, or internal references in the diff

CI status after rebase

Rebased onto current main (af861783). The four checks previously red here were failing identically on the old base 5603ae7 (test_security_posture.py::…test_no_new_gate_side_log_line_reads_the_baseline_redactor, on dashboard/handlers/memory.py). That has since been fixed on main, and the rebase cleared them:

Check	on old base 5603ae7	after rebase onto af861783
Backend Tests (3.10, 3)	FAIL	pass
Backend Tests (3.12, 3)	FAIL	pass
Backend Tests (Windows) (3)	FAIL	flake, see below
Coverage Gate	FAIL (backend-test=failure)	pass

The rebase reused this branch's four file blobs on top of main's current tree, so the diff is byte-identical (+197/−0, same four files) and the branch is now behind_by: 0, MERGEABLE.

One remaining red is a pre-existing test-isolation bug, not this change

Backend Tests (Windows) (3) now fails on a different, unrelated test — a rate limiter tripping under parallel execution:

test/test_session_control.py::test_the_audit_write_does_not_run_on_the_event_loop
test/test_session_control.py::test_the_created_agent_name_is_sanitized_before_storage
  SessionControlError: too many sessions created recently; retry shortly

The census test no longer appears in that job's annotations, none of this PR's files are implicated, and session-creation rate limiting has no path to the route53:ListHostedZonesByVPC call this PR adds. retry shortly is the limiter's own transient message. The same shard passes on 3.10 and 3.12.

Root-caused since, and it is not actually a flake — the fix is up as #7852. create_rate_limit._buckets is process-wide module state keyed (verb, caller_key). Every test in test/test_session_control.py builds its caller as _slot(state, "chat-1"), so all 36 create_session( call sites share one bucket key against a budget of 20 per 300 s; the file runs in ~1.5 s, so the creates accumulate and the 21st onward is refused. Running the file whole fails deterministically (2 failed, 140 passed); pytest-split distributing it across 4 groups is what makes it look intermittent, and is also why the same shard number passes on 3.10/3.12.

main shows the same two tests failing in 2 of the 9 runs that completed that job in a ~5.5-hour window (33604886863, 33599188871), with byte-identical annotations — full table in this comment.

So please don't re-run this job on my behalf — it is a ~78% coin flip that fixes nothing and will re-redden another PR later. Merging #7852 (one import, one autouse reset fixture, matching test_create_rate_limit.py and test_chat_folder_cap.py) clears it here and everywhere. Failing that, this red is safe to accept as pre-existing: this PR's diff has no path to dashboard/session_control.py, and a route53:ListHostedZonesByVPC call cannot reach a session-creation rate limiter.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[timwukp]

Disposition: PR-template warning — fixed

The description bot's warning above is stale. All four named sections were added, using the repo template's exact heading strings (including the unicode arrows in ## What changed (motivation → approach → change), which the bot's message abbreviates to ## What changed):

Section	Status
## Problem / Motivation	present
## Why it matters	present
## What changed (motivation → approach → change)	present
## Tests	present

Also present: ## Manual verification, ## Related Issues, ## Pattern harvest, ## Checklist. ## Screenshots / video is deliberately omitted (backend-only diff) with the <!-- no-visual-delta --> marker plus a justification line.

Reading the template surfaced two things the bot didn't flag, both now corrected:

• Title was not Conventional Commits — it was cloud: …, a scope rather than one of the required types. Now feat(cloud): …, and Code Review / PR Hygiene is green.
• Commit count was 4, limit is 2 — squashed to 1, diff byte-identical afterwards (+197/−0, same four files).

Two gates left, both maintainer-side

1. The AI review lanes are skipped, not passing. On this fork PR the secret-backed lanes skip and the Stage-2 fork-*-review.yml pipeline hasn't fired, so GPT 5.6 Review, Opus 4.8 Review, Design Review, UX Review and First Principles Review have all reported skipped rather than a verdict. Nothing has been reviewed by them yet — worth knowing before reading the green count as substantive. I believe this needs a maintainer to label/approve the fork PR; happy to act on whatever they post once it runs.

2. The four red checks are pre-existing on this PR's base and will not clear from anything done on this branch. See the Base-branch CI status section in the description: Backend Tests (3.10, 3), (3.12, 3), (Windows, 3) and Coverage Gate all fail identically on 5603ae7 with the same annotation, on dashboard/handlers/memory.py — a file not in this diff. PR Readiness aggregates CI, so it inherits that red and will stay red until the base is fixed, regardless of review outcome.

The _BASELINE_LOG_SITE_CENSUS entry for that file is what needs updating. I've deliberately kept it out of this diff rather than widening scope — glad to open a separate PR for it if that's useful.

[timwukp]

The one remaining red here is not this PR, and I can now show that rather than assert it — plus the fix is up separately, which changes what I'm asking for.

main fails the same two tests, right now. In the 9 ci.yml runs on main that completed Backend Tests (Windows) (3) over a ~5.5-hour window, 2 failed:

head	run	Windows (3)
8ebf6a87	33618034073	success
ca45f1c4	33617595177	success
4525771a	33607228235	success
be2ee947	33604886863	failure
35fcdbc4	33602120993	success
aaad3571	33599188871	failure
73318b28	33596377179	success
a71114e8	33595131662	success
63a043a7	33591434675	success

Both main failures carry byte-identical annotations to this PR's: the same two tests, the same SessionControlError: too many sessions created recently; retry shortly.

It is not actually a flake, and that matters for what you do about it. create_rate_limit._buckets is process-wide module state keyed (verb, caller_key). Every test in test/test_session_control.py builds its caller as _slot(state, "chat-1"), so they all share one bucket key, and the file has 36 create_session( call sites against a budget of 20 per 300 s. The file runs in ~1.5 s, so the creates accumulate and the 21st onward is refused. Running the file whole fails deterministically — I reproduced it locally on the first try:

$ pytest test/test_session_control.py      # on main @ 8ebf6a87
2 failed, 140 passed in 4.69s

What makes it look intermittent in CI is pytest-split distributing the file across 4 groups: whether a group carries more than 20 creates depends on the split. That also explains the platform asymmetry — the same shard number passes on 3.10/3.12 not because Linux is different but because a different set of tests lands together.

So I'm withdrawing my earlier request to re-run this job. A re-run is a ~78% coin flip that fixes nothing and will re-redden someone else's PR later today. I've put the actual fix up instead: #7852 — one import plus one autouse reset fixture in that test file, matching what test_create_rate_limit.py and test_chat_folder_cap.py already do. Before: 2 failed / 140 passed. After: 142 passed, all four CI shards green.

What would help here, in preference order:

1. Merge fix(test): reset the create-rate-limit bucket between session-control tests #7852. It clears this red for this PR and for every other PR hitting the ~22%, and it needs no re-run of anything.
2. Failing that, accept this red as pre-existing. The table above is verifiable in 30 seconds and this PR's diff (cloud/ec2.py, cloud/iam.py, and their two tests) has no path to dashboard/session_control.py — the route53:ListHostedZonesByVPC call it adds cannot reach a session-creation rate limiter.

Also still outstanding and not something I can move: the five AI review lanes are skipped on this head, which pr-readiness.yml treats as pending rather than passed, so this PR has had no AI review verdict at all. I previously said in this thread that a maintainer label gates those — that was wrong, and I corrected it on #7522: fork-pr-label.yml only adds a cosmetic label, and the real path is the Stage-2 fork-*-review.yml pipeline firing via workflow_run.

Everything else is green.

[timwukp]

Closing: this is redundant. The preflight it adds is on main as of 516aa45 (#7888), which also closed #7522.

For the record, since a silently closed PR reads as a withdrawal: #7888's 197 added lines are byte-identical to this PR's, across the same four files. Both added-line sets hash to 9872cd4d9667391d0953faaea61bac6b…, and git diff pr7553 516aa4507 is empty for cloud/ec2.py, cloud/iam.py and test/test_cloud_iam.py. This PR was opened 2026-09-01T07:29:02Z; #7888 was opened 2026-09-02T14:42:47Z and merged 3.5 hours later. This PR received 0 reviews in that window, and its five AI review lanes only ever posted skipped — which pr-readiness.yml treats as pending, so it could not reach a passing readiness verdict; #7888's lanes all ran and passed.

I've written that up as a process question in #8046 rather than leaving it implied here, including what I am explicitly not claiming — I am not alleging misconduct by anyone, and @aniruddhaadak80 has several of their own fork PRs closed unmerged in the same window, so they are in the same position I am rather than an advantaged one. The useful outcome is PR-level de-duplication and a fix for the Stage-2 fork-review gap, not attention on these two PRs.

The investigation behind this change is in #7522 (root cause, the controlled A/B) and the durable fix — serving the kiro-cli artifact from a domain no customer-attachable private hosted zone can claim — remains open and unaddressed in #7822. #7888 landed detection, which is the same scope this PR had; it does not remove the namespace collision.

Thanks to whoever reviewed and merged #7888 — the check is better in the tree than in an open PR, which is why I'm not asking for this one to be reopened.