diff --git a/docs/guides/cloud-instance-ssm-vs-ssh.md b/docs/guides/cloud-instance-ssm-vs-ssh.md index 47f8f006503..9b92f0e78a4 100644 --- a/docs/guides/cloud-instance-ssm-vs-ssh.md +++ b/docs/guides/cloud-instance-ssm-vs-ssh.md @@ -11,6 +11,9 @@ older SSH-over-`ProxyCommand` workaround. > inbound port, and no `~/.ssh/config` edits**. Reachability is an IAM decision > (grant/revoke `ssm:StartSession`, CloudTrail-audited), not a networking decision > gated by a key someone holds. +> +> New here? [remote-crew-on-ec2.md](remote-crew-on-ec2.md#deploy-your-first-kiro-crew-on-ec2-step-by-step) +> is the step-by-step first-launch walkthrough; this page is the transport deep-dive. ## The two transports diff --git a/docs/guides/remote-crew-on-ec2.md b/docs/guides/remote-crew-on-ec2.md index 6aeb1810c6e..bf96911c213 100644 --- a/docs/guides/remote-crew-on-ec2.md +++ b/docs/guides/remote-crew-on-ec2.md @@ -24,6 +24,63 @@ plus the EC2-specific gotchas people actually hit (from `kirocrew doctor`). Both end at the same loopback gateway and both are managed identically once registered in **Settings → Remote Instances**. +## Deploy your first Kiro Crew on EC2 (step by step) + +This is the shortest path from a fresh laptop to a working cloud Kiro Crew, using +the one-command launcher (`kirocrew cloud`). Plan on ~10 minutes plus one browser +sign-in. + +**Before you start (laptop prerequisites):** + +1. **AWS CLI installed and configured.** `aws sts get-caller-identity` must print + your account — if it errors, run `aws configure` (or `aws sso login`) first. +2. **Session Manager plugin** — `kirocrew cloud doctor` offers to install it, or + see [cloud-instance-ssm-vs-ssh.md](cloud-instance-ssm-vs-ssh.md). +3. **npm on your laptop (recommended)** — with `npm` installed (Node.js 18+), + the launcher builds the dashboard frontend automatically from the packaged + source (`npm ci && npm run build` in an isolated temporary tree — your + checkout's own `website/dist` and `static/dist` are never read or touched) + and ships the result, so the box skips the slow, failure-prone on-box npm + build. `kirocrew cloud doctor` reports whether the frontend will ship. + Without npm — or if the local build fails — the box builds the frontend + itself. +4. **A git checkout of this repo** with Kiro Crew installed from it — the launcher + packages your local source and uploads it to S3 (private-repo safe), it does + not clone GitHub on the box. + +Then run the checks and launch: + +```bash +kirocrew cloud doctor # verifies AWS CLI, credentials, plugin, frontend build +kirocrew cloud launch # interactive: picks the recommended tier, region, etc. +``` + +- Accept the recommended **balanced** tier. The **light** tier is sized under + Kiro Crew's working set — fine for a quick look, but expect pressure under + real workloads. (`kirocrew cloud launch` prints each tier's current instance + type and memory; the source of truth is `src/kiro_crew/cloud/sizes.py`.) +- Launch blocks until the box is **actually serving the dashboard** (the stack's + health check verifies the SPA loads, not just that something answers on the + port). Typical wait: 10–20 minutes on a cold box. +- When it finishes it prints a dashboard URL and opens it. **Sign in to Kiro in + the browser window** that appears (device-code flow) — chats don't work until + this is done. If you launched with `--yes` (non-interactive), run + `kirocrew cloud login` afterwards to do this step. + +**Coming back later:** + +```bash +kirocrew cloud connect # reopens the dashboard tunnel (alias: tunnel) +kirocrew cloud status # instance state +kirocrew cloud stop # stop the box (EBS kept; cheap) +kirocrew cloud start # start it again +kirocrew cloud destroy # tear everything down (stack + uploaded source) +``` + +If `connect` ever prints a "stale dashboard" warning, the box is missing its +frontend assets — see +[Dashboard HTML not found](#dashboard-html-not-found-after-launch) below. + ## Way 1 — SSH tunnel 1. On your laptop, forward the gateway's port over SSH (use the **real** gateway @@ -59,6 +116,44 @@ port, no SSH key. These map to warnings in `kirocrew doctor`. +### "Dashboard HTML not found" after launch + +The gateway is up but has no frontend assets (`src/kiro_crew/static/dist` is +missing) — `kirocrew cloud connect` also prints a "stale dashboard" warning in +this state. Fixes, cheapest first: + +1. On the box: `sudo systemctl restart kirocrew` — picks up assets staged after + the gateway started. +2. Still missing? The on-box npm build failed at install time. On a cloud box + the install is fail-closed (`KIROCREW_REQUIRE_FRONTEND=1` plus the stack's + own dist check), so re-running `kirocrew cloud launch` (with Node/npm on + your laptop, so the frontend ships pre-built) is the reliable fix. +3. To inspect by hand: `kirocrew cloud connect` keeps working for SSH/SSM access + (`aws ssm start-session --target i-…`); the install log is + `/var/log/kirocrew-install.log` on the box. + +### Launch fails during package install / pip output scrolls by + +What looks like "python testing" is pip's dependency resolver on a cold box. The +template already retries the install once for transient mirror flakes; if the +stack still rolls back, just re-run `kirocrew cloud launch` — a second run reuses +the warm dnf/pip caches and almost always gets through. Persistent failures point +at the instance tier: use **balanced**, not light. + +### Stack rolls back with a WaitCondition timeout + +The box didn't report healthy within 25 minutes. Read the actual failure reason: + +```bash +aws cloudformation describe-stack-events \ + --stack-name kirocrew- \ + --query 'StackEvents[?ResourceStatus==`CREATE_FAILED`].ResourceStatusReason' +``` + +The reason carries the last lines of the on-box install log, which names the +failing step (package install, npm build, gateway start). Fix that step, then +re-run `kirocrew cloud launch`. + ### MCP tools all fail: "Sandbox backend unavailable … `allow_unsandboxed_exec` is not set" On Linux, agent subprocesses run inside a **user-namespace sandbox**. Many hardened diff --git a/docs/system-specs/modules/cloud.md b/docs/system-specs/modules/cloud.md index a053750e9e2..660de80c5e3 100644 --- a/docs/system-specs/modules/cloud.md +++ b/docs/system-specs/modules/cloud.md @@ -73,7 +73,7 @@ claim that a hostile in-process agent is fully contained. | `ssm.py` | SSM `send-command` run-and-poll (base64-wrapped remote scripts) + `start-session` port-forward; `open_port_forward()` directly spawns the streaming `aws ssm start-session` child because `run_aws` captures output, and calls `aws.assert_human_action()` before doing so; `port_is_free` / `wait_for_local_port`. | | `login.py` | `kiro-cli` device-code / social sign-in on the box over SSM, plus `logout` — the account switch. `login` short-circuits on an existing session, so `logout` is what makes a different Kiro account reachable without a hand-run SSM command. It kills any still-polling background `kiro-cli login` **and** any live `kiro-cli acp` runtime **before** signing out (otherwise the login re-authenticates the old account, and an ACP runtime keeps serving the old account's in-memory credential until its next 401), removes the login log/PID/FIFO (they hold the previous device-code URL + code, which must never be re-shown as a fresh prompt), and confirms the result with `is_logged_in` rather than the exit code — `kiro-cli logout` exits non-zero when there was no session to drop, which is still the requested state. That confirmation fails CLOSED: it requires a positive signed-out sentinel (`__NOAUTH__`), so an SSM timeout or transport error — where the session may still be active — reports failure rather than a false "signed out". The same fail-closed applies to the cleanup command itself: if that SSM invocation doesn't return `Success`, the kills it was meant to do can't be trusted and logout reports failure without probing. The CLI warns the operator that in-flight chats/cron sessions are stopped (their runtimes are killed). | | `connect.py` | SSM port-forward + token mint + open browser; Instances-registry integration; `redact_token`. `is_launched_instance()` prevents the generic instance PATCH endpoint from rewriting a correlated launch’s connection method, SSM target, AWS profile, or region, so Stop/Start/Delete retain the stack address and a running billable instance is not stranded. | -| `source.py` | Detect and package an editable local checkout (`git archive`, tarfile fallback) and upload it to a per-account S3 bucket; packaged installs instead use the template's public-repo clone fallback. The secret-excluding filter is shared by both packaging paths. Also **`ensure_instance_boundary`** — creates the shared, immutable `kirocrew-ec2-boundary` managed policy once (create-if-not-exists, never re-versioned) and returns its ARN; `delete_instance_boundary` for admin cleanup. | +| `source.py` | Detect and package an editable local checkout (`git archive`, tarfile fallback) and upload it to a per-account S3 bucket; packaged installs instead use the template's public-repo clone fallback. The secret-excluding filter is shared by both packaging paths. `_inject_dist` extracts only `website/` from that already-filtered archive into an isolated temporary root, runs lockfile-exact `npm ci --ignore-scripts` plus the stock build there, and appends the resulting frontend. It never reads the gitignored checkout `static/dist`, so a bundle left by another branch cannot override the source being shipped; edition composition variables are removed so an external composition root cannot enter the build. The built dist must be a real tree contained by the isolated root. Members are then admitted through four gates: the secret filter (`allow_dist_under` exempts only the literal destination `dist` component for that exact prefix — every other excluded dir still refuses the member; `.env`/`.pem`/credential-name checks still apply), a **type allowlist** (`_DIST_ALLOWED_SUFFIXES` — only build-artifact extensions ship), the hardened nolink read gate (`hooks.safe_read_file_bytes_nolink`: O_NOFOLLOW + same-fd fstat rejects symlinks/hardlinks/non-regular files race-free, fd-real-path containment), and a bounded text-content scan using the shared credential redactor as a detector. The content scan covers UTF-8 HTML, JavaScript, CSS, source maps, JSON/manifests, and SVG; it blocks distinctive/plaintext and encoded credentials plus exact 40-character bare AWS secret keys, while ignoring the redactor's bare-secret warning for longer base64-shaped runs because real minified bundles and data URIs trigger that heuristic. Shipping is **atomic over one hardened-read byte snapshot**: the scanner and index-reference check inspect the same bytes written to the archive, and any build, containment, read/content, or referenced-chunk rejection aborts injection entirely — the original archive ships unchanged and the box uses its required npm-build fallback, so a partial dist never rides. Diagnostics repr-escape untrusted filenames and index references before logging or terminal display. Also **`ensure_instance_boundary`** — creates the shared, immutable `kirocrew-ec2-boundary` managed policy once (create-if-not-exists, never re-versioned) and returns its ARN; `delete_instance_boundary` for admin cleanup. | | `config.py` | Persisted profile / region / tag (**never credentials**); `load()` tolerates a hand-edited/corrupt `cloud.json` — bad JSON *or* a non-object shape falls back to defaults rather than crashing every cloud command. | | `sizes.py` | arm64/Graviton size tiers (16 GB default `t4g.xlarge`). | | `ui.py` / `wizard.py` | Terminal UI + the interactive launch flow. `_deploy_with_progress` runs the blocking deploy on a daemon thread and captures the `aws cloudformation deploy` child via a `proc_sink`, so a Ctrl+C on the main (poll) thread terminates it instead of orphaning it (~1800s). An unknown `--size`/`size_key` on the public `launch()` entrypoint yields a clean rc=1 + message, not an uncaught `KeyError`. Resuming a saved stack (`launch` after `stop`) first calls `_ensure_running_and_ssm_ready` — starts a `stopped` instance and waits for SSM `Online` before sign-in/tunnel (which are SSM-only and would otherwise fail); a `terminated` instance fails clean pointing at `--new`. `last_tag` is persisted (`cfg.save()`) **only after** a deploy confirms healthy — a failed first launch leaves no saved pointer, so the next `launch` retries clean instead of resuming a rolled-back/instance-less stack; `_saved_launch_is_usable` additionally ignores a stale saved tag (from an older build) whose stack is in a `_FAILED_STATES` status or has no instance. | @@ -84,8 +84,12 @@ claim that a hostile in-process agent is fully contained. CloudFormation stack, one `aws cloudformation deploy` (change-set based), atomic rollback, one-command `delete-stack` teardown. AMI resolves from the public `resolve:ssm` Amazon-Linux-2023 alias per arch (no hardcoded AMI ids). A -`WaitCondition` + `cfn-signal` blocks the deploy until the gateway is healthy; a -failed bootstrap folds the on-box setup-log tail into the signal reason so the cause survives the rollback. Bootstrap failure reasons are normalized to printable ASCII before CloudFormation receives them; otherwise CloudFormation replaces the setup error with a charset error and masks it during rollback (`test_cloud_ec2.py::test_failure_reason_is_filtered_to_printable_ascii`). +`WaitCondition` + `cfn-signal` blocks the deploy until the gateway is healthy — +where healthy means the root URL answers **and** its body lacks the gateway's +"Dashboard HTML not found" marker (the dashboard-less fallback page is still +HTTP 200, so a bare status check would bless a frontend-less box). A failed +bootstrap folds the on-box setup-log tail into the signal reason so the cause +survives the rollback. Bootstrap failure reasons are normalized to printable ASCII before CloudFormation receives them; otherwise CloudFormation replaces the setup error with a charset error and masks it during rollback (`test_cloud_ec2.py::test_failure_reason_is_filtered_to_printable_ascii`). Bootstrap is reboot-resilient. UserData performs no package or build work directly: it first copies its already-rendered script to @@ -113,7 +117,17 @@ role (`s3:GetObject` scoped to the single object). Wheel and desktop installs have no checkout to package, so `ec2.deploy` omits `SourceBucket` by default and the template clones the public repository/ref instead. An explicit `ship_source=True` remains fail-closed rather than packaging an unrelated -`site-packages` ancestor. +`site-packages` ancestor. Packaging builds the stock frontend from the exact +filtered source archive in a temporary root, with dependency lifecycle scripts +disabled, and injects only the admitted result. It never reads or mutates the +checkout's live `website/dist` or gitignored `static/dist`; this binds shipped +same-origin JavaScript to the source tarball even after a branch switch. If that +isolated build fails, the original archive ships unchanged and the box builds +with npm. `install.sh` skips the on-box npm build only when the source-bound +bundle is present. A cloud box that still ends up without a dist is failed by +the template's existing gates (`KIROCREW_REQUIRE_FRONTEND=1` makes an on-box +build failure fatal, and the bootstrap's dist check fails the stack before the +gateway starts); local installs stay non-fatal. `discover_network` is **egress-kind-aware**, not just "has a default route": `_subnet_egress_kinds` classifies each subnet's effective route table (explicit diff --git a/install.sh b/install.sh index 26f483f6d9a..fb5ae94c1c7 100755 --- a/install.sh +++ b/install.sh @@ -411,7 +411,40 @@ cd "$KIROCREW_APP_DIR" # ── Frontend (npm + vite) ── # Vite emits to website/dist; we stage it into src/kiro_crew/static/dist # where setup.py copies it into the package at install time. -if has node && [ -d "$KIROCREW_APP_DIR/website" ]; then +# A launch that ships the prebuilt bundle (kirocrew cloud launch injects it +# into the source tarball) skips this step entirely — the on-box npm build is +# the fallback path. The skip is gated on KIROCREW_REQUIRE_FRONTEND=1 (exported +# by the CFn template) so it only fires on cloud boxes: a LOCAL checkout also +# has a staged static/dist (a symlink on source installs), and skipping there +# would freeze the dashboard on whatever was built last. index.html alone does +# not prove a usable bundle: a torn staging or interrupted earlier build can +# leave an index whose hashed chunks are missing, and an install RETRY that +# trusted it would skip the rebuild and serve a shell whose every chunk 404s. +# So the skip additionally requires every /assets/ chunk the index references +# to exist (the same completeness signal frontend._incomplete_bundle_reason +# uses); an incomplete tree falls through to the npm rebuild below. A cloud box +# that ends up with no dist is failed by the existing gates: +# KIROCREW_REQUIRE_FRONTEND=1 makes a build failure fatal below, and the +# template's own DIST_INDEX check fails the stack before the gateway starts. +# Local installs stay non-fatal. +_dist_bundle_complete() { + # Complete = index.html exists and every /assets/*.js|.css it references + # is present on disk. Gateway-served routes (/manifest.js) are not under + # /assets/ and are deliberately not matched. + local _dist_dir _refs _ref + _dist_dir="$1" + [ -f "$_dist_dir/index.html" ] || return 1 + _refs="$(grep -oE '(src|href)="/assets/[^"?#]+\.(js|css)"' \ + "$_dist_dir/index.html" 2>/dev/null | sed -E 's/^[^"]*"//; s/"$//')" + for _ref in $_refs; do + [ -f "$_dist_dir$_ref" ] || return 1 + done + return 0 +} +if [ "${KIROCREW_REQUIRE_FRONTEND:-0}" = "1" ] \ + && _dist_bundle_complete "$KIROCREW_APP_DIR/src/kiro_crew/static/dist"; then + ok "Frontend shipped pre-built — skipping npm build" +elif has node && [ -d "$KIROCREW_APP_DIR/website" ]; then info "Building frontend (website/)…" _fe_log="$(mktemp)" ( diff --git a/src/kiro_crew/cli_cloud.py b/src/kiro_crew/cli_cloud.py index d72cc03d563..1af3d1a7cf2 100644 --- a/src/kiro_crew/cli_cloud.py +++ b/src/kiro_crew/cli_cloud.py @@ -18,7 +18,7 @@ from kiro_crew.cloud import connect as connect_mod from kiro_crew.cloud import ec2, iam from kiro_crew.cloud import login as login_mod -from kiro_crew.cloud import sizes, ssm, ui, wizard +from kiro_crew.cloud import sizes, source, ssm, ui, wizard from kiro_crew.cloud.aws import AWSError, CloudActionDenied from kiro_crew.cloud.config import DEFAULT_REGION, CloudConfig from kiro_crew.deploy.engine import resolve_aws_bin @@ -118,6 +118,14 @@ def _cloud_connect(args: argparse.Namespace) -> int: ui.fail(str(exc)) return 1 if conn.ready and conn.url: + # Warn early if the box serves the "Dashboard HTML not found" page (no + # static/dist) instead of letting the user open a broken dashboard. + # Deferred import: cli_server pulls the gateway's heavy module chain + # (vector_memory → numpy), which must not load at CLI import time + # (test_cli_lazy_imports, issue #3504). + from kiro_crew.cli_server import _probe_dashboard_health + + _probe_dashboard_health(conn.local_port) if not conn.token: # Tunnel is up but the token mint failed — the URL will hit the # dashboard's login wall. Say so instead of implying it's ready. @@ -402,6 +410,19 @@ def _cloud_doctor(args: argparse.Namespace) -> int: else: ui.fail("AWS not reachable") ui.detail(reach.get("note", "")) + # A launch builds the stock frontend from the exact source archive in an + # isolated temporary tree, so a residual gitignored static/dist can never + # influence what ships. Doctor checks only the local prerequisites; the + # packaging path remains fail-closed and falls back to the box build. + try: + reason = source.dist_ineligible_reason(source.repo_root()) + except Exception as exc: + reason = f"could not inspect the checkout ({exc})" + if not reason: + ui.ok("frontend source ready (launch builds an isolated prebuilt bundle)") + else: + ui.warn(f"frontend cannot be prebuilt for launch: {reason}") + ui.detail("The box will use its required npm-build fallback.") return 0 diff --git a/src/kiro_crew/cli_server.py b/src/kiro_crew/cli_server.py index 8c8fd494797..aa4cdd4c75c 100644 --- a/src/kiro_crew/cli_server.py +++ b/src/kiro_crew/cli_server.py @@ -134,7 +134,8 @@ def _probe_dashboard_health(port: int) -> None: print( "⚠️ Warning: gateway is serving a stale dashboard " "(assets missing — likely an update pruned the " - "running install). Restart the gateway to fix.", + "running install). Restart the gateway to fix " + "(on a cloud box: sudo systemctl restart kirocrew).", file=sys.stderr, ) except Exception: diff --git a/src/kiro_crew/cloud/ec2.py b/src/kiro_crew/cloud/ec2.py index efd7392c4e0..7373b1a8f4d 100644 --- a/src/kiro_crew/cloud/ec2.py +++ b/src/kiro_crew/cloud/ec2.py @@ -668,6 +668,11 @@ def deploy( # access needed). Fall back to a git clone only if source shipping is off. source_bucket = source_key = "" if ship_source: + # Build the stock frontend from website/ inside the exact filtered source + # archive, then inject its admitted bytes. The temporary build never + # mutates the checkout's live website/dist or trusts its ignored + # static/dist; failure leaves the source archive unchanged so the box + # uses its required npm-build fallback. source_bucket, source_key = source_mod.upload_source(tag, profile, region) def _cleanup_uploaded_source() -> None: diff --git a/src/kiro_crew/cloud/source.py b/src/kiro_crew/cloud/source.py index 213d1bcd866..329e6c672e8 100644 --- a/src/kiro_crew/cloud/source.py +++ b/src/kiro_crew/cloud/source.py @@ -12,14 +12,20 @@ from __future__ import annotations +import base64 +import binascii +import io import logging import os +import re +import shutil import subprocess import tarfile import tempfile from pathlib import Path from typing import Optional +from kiro_crew import frontend, hooks, security from kiro_crew.cloud import aws from kiro_crew.sel import sel from kiro_crew.subprocess_utf8 import UTF8_TEXT @@ -123,10 +129,154 @@ def repo_root() -> Path: ) -def _exclude_filter(ti: tarfile.TarInfo) -> Optional[tarfile.TarInfo]: - """Shared tar member filter: drop excluded dirs + credential-shaped files.""" +# Repo-relative destination for the source-bound frontend bundle. ``static/dist`` +# is git-ignored (and "dist" is in ``_EXCLUDE_DIRS``), so the source archive never +# carries a residual checkout bundle; ``_inject_dist`` builds from the archived +# ``website/`` bytes and appends only the admitted result. +_DIST_PREFIX = "src/kiro_crew/static/dist" + +# Extension allowlist for injected dist members. Vite/Rollup output (and the +# ``website/public/`` files it copies verbatim) is entirely static web assets, +# so admission is type-level: only build-artifact-shaped files ship. A stray +# ``secrets.yaml`` / ``id_rsa`` / extensionless credential dropped into +# ``website/public/`` is refused by TYPE, independent of the name/suffix +# denylists in :func:`_exclude_filter` — an allowlist cannot be extended by an +# attacker naming a file cleverly, where a denylist can be sidestepped. +_DIST_ALLOWED_SUFFIXES = frozenset( + { + # documents + code + ".html", + ".js", + ".mjs", + ".cjs", + ".css", + ".map", + ".json", + ".webmanifest", + ".wasm", + # images + ".svg", + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".ico", + ".avif", + # fonts + ".woff", + ".woff2", + ".ttf", + } +) + +# Allowed asset types whose bytes must be valid UTF-8 and credential-free. Binary +# formats bypass text decoding but still pass the name, type and hardened-read +# gates. SVG is XML text and source maps/manifests are JSON text. +# A `data:/;base64,` URI is an embedded resource that +# DECLARES itself as one in its own syntax. Vite inlines small images and fonts +# this way, and those payloads are long base64 runs that trip the generic +# bare-secret sliding-window heuristic (a real 231-char PNG run from this +# project's built editor CSS does; pinned by +# test_real_bundled_data_uri_does_not_false_positive). +# +# Masking the payload before the credential scan removes the false-positive +# SOURCE, which is what lets every remaining bare-secret warning be treated as +# actionable. The alternative -- exempting the warning whenever the run is not +# exactly 40 chars -- also discarded the true positives, because +# `security._contains_bare_secret` exists precisely to catch a genuine 40-char +# secret glued to adjacent base64 characters ("X" + key, key + "ABC", +# key + "X" + key), all of which report a 41+ char run. +# +# Only the payload is masked, and only for the scan: the bytes shipped in the +# tarball are always the unmodified originals. A credential baked into a bundle +# by a build lands in ordinary JS/CSS, not inside a data: URI, so the leak this +# gate exists to stop is still caught. +_DATA_URI_B64_RE = re.compile(r"data:[\w.+-]+/[\w.+-]+;base64,([A-Za-z0-9+/=]+)") + +#: Prefix of the generic entropy-heuristic warning `redact_credentials` emits. +_BARE_SECRET_WARNING = "Redacted bare secret key" + +#: Container magic numbers for the asset types a bundler inlines as data URIs. +#: The exemption below is keyed on CONTENT, not position: a payload is only +#: treated as media if it actually decodes to one of these. Masking purely by +#: position would let a bare key parked where a payload goes escape the scan. +_MEDIA_MAGIC = ( + b"\x89PNG\r\n\x1a\n", # png + b"\xff\xd8\xff", # jpeg + b"GIF87a", + b"GIF89a", + b"RIFF", # webp + b"wOFF", + b"wOF2", + b"\x00\x01\x00\x00", # ttf + b"OTTO", + b"true", + b"ttcf", + b"\x00\x00\x01\x00", # ico + b" bool: + """True if a base64 data-URI payload decodes to a recognized media container.""" + for pad in ("", "=", "=="): + try: + raw = base64.b64decode(payload + pad, validate=True) + except (binascii.Error, ValueError): + continue + return raw.startswith(_MEDIA_MAGIC) + return False + + +def _mask_inlined_media(text: str) -> str: + """Blank out data-URI payloads that are genuinely inlined media.""" + + def replace(match: "re.Match[str]") -> str: + return "data:masked" if _data_uri_payload_is_media(match.group(1)) else match.group(0) + + return _DATA_URI_B64_RE.sub(replace, text) + + +_DIST_TEXT_SUFFIXES = frozenset( + {".html", ".js", ".mjs", ".cjs", ".css", ".map", ".json", ".webmanifest", ".svg"} +) + + +def _dist_suffix_allowed(arcname: str) -> bool: + """Whether an injected dist member's extension is an allowed asset type.""" + suffix = Path(arcname).suffix.lower() + return suffix in _DIST_ALLOWED_SUFFIXES + + +def _exclude_filter( + ti: tarfile.TarInfo, allow_dist_under: Optional[str] = None +) -> Optional[tarfile.TarInfo]: + """Shared tar member filter: drop excluded dirs + credential-shaped files. + + ``allow_dist_under`` names the ONE repo-relative prefix whose members + exempt only the ``dist`` component of ``_EXCLUDE_DIRS`` — used by + :func:`_inject_dist` for the prebuilt ``static/dist`` bundle, which + otherwise trips on the ``dist`` dir name. Every other excluded dir (``.aws``, + ``.ssh``, …) and the suffix / ``.env`` / credential-name checks still apply + to every member, so a ``static/dist/.aws/credentials`` or + ``static/dist/.env`` is still refused. Default ``None`` keeps every other + caller byte-identical. + """ + # Pure-posix join: injected arcnames are already posix and the allow-prefix + # comparison must not vary with the host separator. + name = "/".join(Path(ti.name).parts) parts = set(Path(ti.name).parts) - if parts & _EXCLUDE_DIRS: # excluded / secret-bearing directory anywhere in path + excluded = parts & _EXCLUDE_DIRS + if allow_dist_under is not None and ( + name == allow_dist_under or name.startswith(allow_dist_under + "/") + ): + # Only the prefix's own `dist` component is exempt; every other excluded + # dir (secret-bearing or not) still refuses the member — a staged + # ``static/dist/.aws/credentials`` must never ship. + excluded -= {"dist"} + if excluded: # excluded / secret-bearing dir anywhere return None if ti.name.endswith(_EXCLUDE_SUFFIXES): # credential file suffixes return None @@ -330,24 +480,214 @@ def _excluded(rel: str) -> bool: return Path(out.name) +class _AbortInjection(Exception): + """Internal: discard the injected tarball and ship the original archive.""" + + +def _read_dist_bundle(dist: Path, dist_resolved: Path) -> list[tuple[str, bytes]]: + """Return one policy-checked, immutable snapshot of the dist bundle. + + Every candidate is read exactly once through the no-link gate. Text assets + are decoded strictly and scanned for credential-shaped content without + rewriting their bytes. ``index.html`` is parsed from that same pinned byte + snapshot, so completeness checks cannot follow a swapped path or inspect + content different from what the tarball receives. + """ + admitted: list[tuple[str, bytes]] = [] + admitted_arcnames: set[str] = set() + index_html: Optional[str] = None + + for file_path in sorted(dist.rglob("*")): + if not file_path.is_file() or file_path.is_symlink(): + continue + arcname = f"{_DIST_PREFIX}/{file_path.relative_to(dist).as_posix()}" + ti = tarfile.TarInfo(arcname) + if _exclude_filter(ti, allow_dist_under=_DIST_PREFIX) is None: + logger.info("excluding %r from source tarball", arcname) + continue + if not _dist_suffix_allowed(arcname): + logger.info( + "excluding %r (extension not in the dist asset allowlist)", + arcname, + ) + continue + + try: + data = hooks.safe_read_file_bytes_nolink(str(file_path), within_root=str(dist_resolved)) + except hooks.FileTooLargeError as exc: + raise _AbortInjection(f"{arcname!r} exceeds the hardened read limit") from exc + if data is None: + raise _AbortInjection( + f"{arcname!r} rejected by the hardened read gate " + "(hardlink/symlink/non-regular or escaped the tree)" + ) + + suffix = Path(arcname).suffix.lower() + if suffix in _DIST_TEXT_SUFFIXES: + try: + decoded = data.decode("utf-8") + except UnicodeDecodeError as exc: + raise _AbortInjection(f"text asset {arcname!r} is not valid UTF-8") from exc + # The data-URI exemption applies to the GENERIC bare-secret + # heuristic ONLY -- that is the single check with a false-positive + # problem on built assets -- and only to a payload that actually + # DECODES to media (see _mask_inlined_media). Exempting by position + # alone would let a bare key parked where a payload goes escape. Distinctive and encoded-credential + # matches have reliable token structure, so they stay actionable + # everywhere, including inside a data: URI payload (an attacker + # cannot launder a key by base64-ing it into an inlined image). + # + # Every bare-secret warning on the masked text is actionable, + # including runs longer than 40 chars: that is what a genuine key + # glued to adjacent base64 characters looks like, and it is + # precisely what `security._contains_bare_secret` exists to catch. + _, full_warnings = security.redact_credentials(decoded) + _, masked_warnings = security.redact_credentials(_mask_inlined_media(decoded)) + actionable = [ + warning for warning in full_warnings if not warning.startswith(_BARE_SECRET_WARNING) + ] + [warning for warning in masked_warnings if warning.startswith(_BARE_SECRET_WARNING)] + if actionable: + raise _AbortInjection(f"text asset {arcname!r} contains credential-shaped content") + if arcname == f"{_DIST_PREFIX}/index.html": + index_html = decoded + + admitted.append((arcname, data)) + admitted_arcnames.add(arcname) + + if index_html is None: + raise _AbortInjection("index.html was not admitted by the hardened read gate") + + unshipped = [ + ref + for ref in frontend._index_asset_refs(index_html) + if f"{_DIST_PREFIX}{ref}" not in admitted_arcnames + ] + if unshipped: + raise _AbortInjection( + f"index.html references {unshipped[0]!r} which was refused admission " + "— a partial dist must never ship" + ) + return admitted + + +def _build_archived_dist(archive: Path) -> list[tuple[str, bytes]]: + """Build and admit a stock frontend from the exact archived source bytes.""" + with tempfile.TemporaryDirectory(prefix="kirocrew-frontend-") as temp: + build_root = Path(temp).resolve(strict=True) + website_root = build_root / "website" + with tarfile.open(archive, "r:gz") as src: + for member in src: + try: + rel = Path(member.name).relative_to("website") + except ValueError: + continue + if not member.isreg() or not rel.parts or ".." in rel.parts: + continue + target = website_root / rel + target.parent.mkdir(parents=True, exist_ok=True) + source_fh = src.extractfile(member) + if source_fh is None: + raise _AbortInjection(f"could not read archived {member.name!r}") + with source_fh: + target.write_bytes(source_fh.read()) + + messages: list[str] = [] + try: + dist = frontend.build_stock_frontend(build_root, log=messages.append) + except OSError as exc: + # `build_stock_frontend` resolves npm with shutil.which and reports a + # MISSING one by returning None, but the spawn itself is unguarded: + # an npm that resolves yet cannot be executed (non-executable, broken + # symlink, unlinked between the check and the spawn, EPERM) raises + # here. Without this branch it would reach `except BaseException: + # raise` below and abort the launch with a traceback -- exactly the + # outcome the archive-unchanged fallback exists to prevent. Converted + # to a refusal so the box builds its own frontend, which is what + # every other build failure already does. + raise _AbortInjection(f"could not run the frontend build ({exc!r})") from exc + if dist is None: + reason = messages[-1].strip() if messages else "frontend build failed" + raise _AbortInjection(reason) + try: + dist.lstat() + dist_resolved = dist.resolve(strict=True) + except OSError as exc: + raise _AbortInjection(f"could not resolve built frontend ({exc!r})") from exc + if dist.is_symlink() or build_root not in dist_resolved.parents: + raise _AbortInjection("built frontend resolves outside the isolated source tree") + return _read_dist_bundle(dist, dist_resolved) + + +def dist_ineligible_reason(root: Path) -> str: + """Why a source-bound frontend cannot be built for a cloud launch ("" = eligible).""" + website = root / "website" + if not website.is_dir(): + return "website source is not present in this checkout" + if not (website / "package-lock.json").is_file(): + return "website/package-lock.json is missing" + if not shutil.which("npm"): + return "npm is not installed" + return "" + + +def _inject_dist(archive: Path, root: Path) -> Path: + """Build from archived ``website/`` source and append the admitted dist. + + The ignored checkout ``static/dist`` is never trusted: build scripts can + produce arbitrary same-origin JavaScript, so no marker can safely bind a + residual bundle to the source being shipped. Instead the stock frontend is + rebuilt in a temporary root populated only from the already-filtered source + archive. Any build or admission refusal returns that original archive + unchanged so the box builds with npm. + """ + injected = tempfile.NamedTemporaryFile( # noqa: SIM115 - handed to caller + prefix="kirocrew-src-", suffix=".tar.gz", delete=False + ) + injected.close() + try: + admitted = _build_archived_dist(archive) + with tarfile.open(archive, "r:gz") as src, tarfile.open(injected.name, "w:gz") as dst: + for member in src: + fh = src.extractfile(member) if member.isreg() else None + dst.addfile(member, fh) + for arcname, data in admitted: + ti = tarfile.TarInfo(arcname) + ti.size = len(data) + dst.addfile(ti, io.BytesIO(data)) + except _AbortInjection as exc: + logger.warning("not shipping %s: %s — the box will build with npm", _DIST_PREFIX, exc) + Path(injected.name).unlink(missing_ok=True) + return archive + except BaseException: + Path(injected.name).unlink(missing_ok=True) + raise + archive.unlink(missing_ok=True) + logger.info("built and injected frontend %s from archived source", _DIST_PREFIX) + return Path(injected.name) + + def build_source_tarball(root: Optional[Path] = None) -> Path: """Package the local source tree into a gzip tarball; return its path. Uses ``git archive HEAD`` (fast) for a clean checkout, but if the tracked working tree is DIRTY (uncommitted edits to tracked files) it uses the ``git ls-files`` tar path instead — otherwise the launch would silently ship - stale last-commit code. Both paths ship only tracked files. + stale last-commit code. Both paths ship only tracked files. ``_inject_dist`` + then builds the stock frontend from that exact archive in an isolated + temporary tree and appends only its admitted byte snapshot. A residual + gitignored checkout bundle is never read; a failed build leaves the original + archive unchanged so the box builds its own frontend. """ root = root or repo_root() if _tracked_tree_is_dirty(root): logger.info("working tree has uncommitted tracked changes; packaging the working tree") - return _tar_fallback(root) + return _inject_dist(_tar_fallback(root), root) archive = _use_git_archive(root) if archive is not None: logger.info("packaged source via git archive: %s", archive) - return archive + return _inject_dist(archive, root) logger.info("git archive unavailable; using tracked-file tarfile fallback") - return _tar_fallback(root) + return _inject_dist(_tar_fallback(root), root) def _account_id(profile: str, region: str) -> str: diff --git a/src/kiro_crew/cloud/templates/kirocrew-ec2.yaml b/src/kiro_crew/cloud/templates/kirocrew-ec2.yaml index 8036b78caa5..a883953d2a1 100644 --- a/src/kiro_crew/cloud/templates/kirocrew-ec2.yaml +++ b/src/kiro_crew/cloud/templates/kirocrew-ec2.yaml @@ -283,6 +283,14 @@ Resources: # EXPANDED size is guarded by test_cloud_ec2.py's UserData size test; # when it trips, trim the script or move knowledge here, never delete it. # + # "Dashboard health": the WaitCondition gate accepts the box only when + # the root URL answers AND its body lacks the gateway's + # DASHBOARD_HTML_NOT_FOUND_MARKER (dashboard/handlers/core.py). The + # gateway serves that guidance page with HTTP 200 when static/dist is + # missing, so a bare status check would bless a dashboard-less box. + # install.sh fails closed on a missing/incomplete bundle first; this is + # the backstop at the serving layer, for a box whose assets vanish + # after install already succeeded. # "Reboot resume": State Manager runs wildcard associations when a new # managed node first comes online. An account-level # AWS-RunPatchBaseline association can therefore patch and reboot the @@ -680,13 +688,19 @@ Resources: systemctl enable kirocrew.service || fail "could not enable kirocrew.service" systemctl start kirocrew.service || fail "could not start kirocrew.service" - echo "--- waiting for the gateway to answer on 127.0.0.1:${DashboardPort} ---" + echo "--- waiting for the gateway to serve the dashboard on 127.0.0.1:${DashboardPort} ---" + # see "Dashboard health" ok=0 for i in $(seq 1 60); do - if curl -fsS -o /dev/null http://127.0.0.1:${DashboardPort}/ 2>/dev/null; then ok=1; break; fi + if curl -fsS http://127.0.0.1:${DashboardPort}/ 2>/dev/null \ + | grep -q 'Dashboard HTML not found'; then + : + elif curl -fsS -o /dev/null http://127.0.0.1:${DashboardPort}/ 2>/dev/null; then + ok=1; break + fi sleep 5 done - [ "$ok" = "1" ] || fail "gateway did not become healthy within 5 minutes" + [ "$ok" = "1" ] || fail "gateway did not serve the dashboard within 5 minutes" echo "=== KiroCrew bootstrap complete $(date -u) ===" signal_ok=0 @@ -717,6 +731,8 @@ Resources: Properties: Handle: !Ref WaitHandle # 25 min: cold boot + dnf + NodeSource + npm build (vite) + pip install. + # The vite build time only applies when the launch shipped no prebuilt + # static/dist (the usual path skips it); keep the budget for that fallback. Timeout: "1500" Count: 1 diff --git a/src/kiro_crew/frontend.py b/src/kiro_crew/frontend.py index 206266b43e7..7ab5119092b 100644 --- a/src/kiro_crew/frontend.py +++ b/src/kiro_crew/frontend.py @@ -21,7 +21,7 @@ import subprocess import tempfile from pathlib import Path -from typing import Callable, Iterator, Optional +from typing import Callable, Iterator, List, Optional from kiro_crew import platform_compat from kiro_crew.executors import subprocess_executor @@ -240,6 +240,18 @@ def ensure_dev_dist_symlink() -> Optional[Path]: return candidate +def _index_asset_refs(html: str) -> List[str]: + """The ``/assets/`` chunk paths ``index.html`` references. + + Vite emits the content-hashed chunks under ``/assets/``, so these are the + completeness signal shared by :func:`_incomplete_bundle_reason` (chunks + exist on disk) and the source-shipping injector (chunks made it into the + tarball). Gateway-served routes (``/manifest.js``) are deliberately not + matched. + """ + return re.findall(r'(?:src|href)="(/assets/[^"?#]+\.(?:js|css))', html) + + def _incomplete_bundle_reason(tree: Path) -> str: """Why ``tree`` is not a complete built frontend, or ``""`` if it is. @@ -260,7 +272,7 @@ def _incomplete_bundle_reason(tree: Path) -> str: html = index.read_text(encoding="utf-8", errors="replace") except OSError as exc: return f"index.html is unreadable ({exc})" - refs = re.findall(r'(?:src|href)="(/assets/[^"?#]+\.(?:js|css))', html) + refs = _index_asset_refs(html) missing = [ref for ref in refs if not (tree / ref.lstrip("/")).is_file()] if missing: return f"{len(missing)} referenced asset(s) missing, e.g. {missing[0]}" @@ -295,62 +307,125 @@ def _staging_lock(static_parent: Path) -> Iterator[None]: yield -def _npm_build_and_stage_locked( +def _run_npm_step( website_dir: Path, - proj_path: Path, npm: str, + args: list[str], + timeout: int, log: Callable[[str], None], + label: str, + *, + env: Optional[dict[str, str]] = None, ) -> bool: - """Run ``npm run build`` then stage it. Caller holds the staging lock. - - The build is spawned in its own process group and the whole tree is reaped - on timeout. ``npm run build`` is ``tsc -b && vite build``, so killing only - npm would leave vite writing ``website/dist`` after this function returns - and the lock releases — a surviving writer makes the lock's exclusion - meaningless, since a peer could then stage a tree vite is still rewriting. - """ + """Run one npm step and reap its whole process tree on timeout.""" proc = subprocess.Popen( - [npm, "run", "build"], - env=_edition_build_env(), + [npm, *args], + env=env, cwd=str(website_dir), # DEVNULL, not PIPE: nothing reads the build's output, and pipes would # make the post-kill drain block until every grandchild closes its - # inherited write handle — inside the lock holder, which would then - # never release it. + # inherited write handle. stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, start_new_session=platform_compat.IS_POSIX, creationflags=platform_compat.CREATE_NEW_PROCESS_GROUP, ) try: - proc.wait(timeout=_BUILD_TIMEOUT) + proc.wait(timeout=timeout) except subprocess.TimeoutExpired: # Enumerate BEFORE killing: the kill reparents survivors to init and # erases the PPID links that identify them. The group kill alone misses - # a descendant that started its own session, and such an escapee keeps - # rewriting website/dist after this holder releases the staging lock — - # the mixed-bundle publication this lock exists to prevent. + # a descendant that started its own session. descendants = platform_compat.process_descendants(proc.pid) try: platform_compat.kill_process_tree(proc.pid, platform_compat.SIGKILL) except (ProcessLookupError, OSError, ValueError) as exc: - log(f" ⚠️ Could not reap the timed-out frontend build: {exc}") + log(f" ⚠️ Could not reap the timed-out frontend {label}: {exc}") for child in descendants: try: platform_compat.kill_process_tree(child, platform_compat.SIGKILL) except (ProcessLookupError, OSError, ValueError): - # Already reaped by the group kill, or no longer signalable. continue - # Reap the direct child so it is not left a zombie. Bounded, so a - # survivor cannot hold the staging lock open indefinitely. try: proc.wait(timeout=_BUILD_KILL_GRACE) except subprocess.TimeoutExpired: - log(" ⚠️ Frontend build did not die after SIGKILL") - log(" ⚠️ Frontend build timed out — dashboard may be stale") + log(f" ⚠️ Frontend {label} did not die after SIGKILL") + log(f" ⚠️ Frontend {label} timed out — dashboard may be stale") return False if proc.returncode != 0: - log(" ⚠️ Frontend build failed — dashboard may be stale") + log(f" ⚠️ Frontend {label} failed — dashboard may be stale") + return False + return True + + +def build_stock_frontend( + proj_path: Path, + npm: str | None = None, + log: Callable[[str], None] = print, +) -> Optional[Path]: + """Build a stock dist from an isolated source tree without staging it. + + Cloud source packaging extracts ``website/`` from the exact source archive + into a temporary root and calls this helper. Edition variables are removed: + an inherited composition root points outside that archive and would break + the guarantee that the shipped JavaScript derives only from packaged source. + """ + website_dir = proj_path / _DIR_NAME + lockfile = website_dir / "package-lock.json" + if not website_dir.is_dir() or not lockfile.is_file(): + log(" ⚠️ Archived website source or package-lock.json is missing") + return None + npm_bin = npm or shutil.which("npm") + if not npm_bin: + log(" ⚠️ npm not found — cannot build the archived frontend") + return None + env = dict(os.environ) + env.pop(_EDITION_DIR_ENV, None) + env.pop(_EDITION_OPT_IN_ENV, None) + if not _run_npm_step( + website_dir, + npm_bin, + ["ci", "--ignore-scripts", "--no-audit", "--no-fund"], + _INSTALL_TIMEOUT, + log, + "npm install", + env=env, + ): + return None + if not _run_npm_step( + website_dir, + npm_bin, + ["run", "build"], + _BUILD_TIMEOUT, + log, + "build", + env=env, + ): + return None + dist = website_dir / "dist" + reason = _incomplete_bundle_reason(dist) + if reason: + log(f" ⚠️ Archived frontend build is incomplete ({reason})") + return None + return dist + + +def _npm_build_and_stage_locked( + website_dir: Path, + proj_path: Path, + npm: str, + log: Callable[[str], None], +) -> bool: + """Run ``npm run build`` then stage it. Caller holds the staging lock.""" + if not _run_npm_step( + website_dir, + npm, + ["run", "build"], + _BUILD_TIMEOUT, + log, + "build", + env=_edition_build_env(), + ): return False static_dist = proj_path / "src" / "kiro_crew" / "static" / "dist" return _stage_dist_locked(website_dir / "dist", static_dist, log) diff --git a/src/kiro_crew/security_posture.py b/src/kiro_crew/security_posture.py index 4789cd7d955..4cab3a8c92c 100644 --- a/src/kiro_crew/security_posture.py +++ b/src/kiro_crew/security_posture.py @@ -1424,6 +1424,10 @@ class PostureControl: # run twice. Nothing redacted here is written or shown — the ledger and # the transcript keep the original text. "dashboard/steer_settle.py", + # DETECTOR, not a redactor: scans candidate frontend bytes and discards the + # redacted output. Admission either preserves those original bytes unchanged + # or aborts the dist injection, so this module owns no redacted egress. + "cloud/source.py", # DETECTOR, not a redactor: the pre-push content scan calls both scanners only # to COUNT findings and then refuses the push. It deliberately discards the # cleaned text — rewriting a code diff would corrupt the very fix the gate diff --git a/test/test_cloud_cli.py b/test/test_cloud_cli.py index fdb184df413..ebb06d28375 100644 --- a/test/test_cloud_cli.py +++ b/test/test_cloud_cli.py @@ -205,6 +205,70 @@ def test_connect_not_ready_returns_failure(self, monkeypatch, capsys): assert rc == 1 assert "Dashboard tunnel did not become ready" in capsys.readouterr().out + def test_connect_probes_dashboard_health_on_local_port(self, monkeypatch): + # When the tunnel is ready, connect warns (via cli_server's probe) if the + # box serves "Dashboard HTML not found" instead of a working SPA. + monkeypatch.setattr(cli_cloud.ssm, "session_manager_plugin_installed", lambda: True) + monkeypatch.setattr( + ec2, "describe", lambda *a, **k: {"exists": True, "instance_id": "i-0abc"} + ) + monkeypatch.setattr( + connect_mod, + "connect", + lambda *a, **k: connect_mod.Connection( + instance_id="i-0abc", + local_port=5599, + remote_port=5476, + token="tok", + url="http://127.0.0.1:5599/?token=tok", + ready=True, + process=None, # .wait() skipped + ), + ) + probed = {} + # cli_cloud defers the probe import into _cloud_connect (CLI lazy-import + # ratchet, issue #3504), so patch it at its source module. + import kiro_crew.cli_server as cli_server + + monkeypatch.setattr( + cli_server, + "_probe_dashboard_health", + lambda port: probed.setdefault("port", port), + ) + rc = cli_cloud._cloud_connect(_args(profile="", region="", tag="kc-1")) + assert rc == 0 + assert probed.get("port") == 5599 + + def test_doctor_reports_frontend_build_prerequisites(self, monkeypatch, capsys, tmp_path): + # Doctor reports whether the launcher can build from archived source; an + # ignored checkout dist is deliberately irrelevant to provenance. + monkeypatch.setattr(cli_cloud, "_resolve", lambda a: ("", "us-east-1")) + monkeypatch.setattr( + "shutil.which", lambda name, **kw: "/usr/bin/npm" if name == "npm" else None + ) + monkeypatch.setattr(cli_cloud.ssm, "session_manager_plugin_installed", lambda: False) + monkeypatch.setattr(cli_cloud.ssm, "session_manager_plugin_install_hint", lambda: "") + monkeypatch.setattr( + cli_cloud.iam, + "reachability_check", + lambda *a, **k: {"reachable": False, "note": "offline"}, + ) + import kiro_crew.cloud.source as source_mod + + monkeypatch.setattr(source_mod, "repo_root", lambda: tmp_path) + assert cli_cloud._cloud_doctor(_args(profile="", region="")) == 0 + assert "cannot be prebuilt" in capsys.readouterr().out + + website = tmp_path / "website" + website.mkdir() + (website / "package-lock.json").write_text('{"lockfileVersion": 3}\n') + assert cli_cloud._cloud_doctor(_args(profile="", region="")) == 0 + assert "frontend source ready" in capsys.readouterr().out + + (website / "package-lock.json").unlink() + assert cli_cloud._cloud_doctor(_args(profile="", region="")) == 0 + assert "package-lock.json is missing" in capsys.readouterr().out + class TestDestroy: def test_destroy_dry_run(self, monkeypatch, capsys): diff --git a/test/test_cloud_ec2.py b/test/test_cloud_ec2.py index 605fb84d4ab..e8221ed406a 100644 --- a/test/test_cloud_ec2.py +++ b/test/test_cloud_ec2.py @@ -715,9 +715,18 @@ def test_dry_run_defaults_to_public_clone_without_checkout(self, monkeypatch): class TestDeployShipsSource: def test_deploy_uploads_source_and_passes_params(self, monkeypatch): import kiro_crew.cloud.source as source_mod + from kiro_crew import frontend as frontend_mod monkeypatch.setattr(ec2, "find_stack", lambda *a, **k: None) monkeypatch.setattr(source_mod, "ensure_instance_boundary", lambda *a, **k: _BOUNDARY_ARN) + + # Launch must NEVER rebuild the frontend: on a source install + # static/dist is the LIVE tree the local gateway serves, and an + # in-launch npm build would empty/replace it mid-serve. + def _never_build(*a, **k): + raise AssertionError("ec2.deploy must not call frontend.build_and_stage") + + monkeypatch.setattr(frontend_mod, "build_and_stage", _never_build) monkeypatch.setattr( source_mod, "upload_source", diff --git a/test/test_cloud_source.py b/test/test_cloud_source.py index 420e9c2f7c8..1b42bddd29b 100644 --- a/test/test_cloud_source.py +++ b/test/test_cloud_source.py @@ -2,6 +2,9 @@ from __future__ import annotations +import base64 +import io +import os import tarfile from pathlib import Path @@ -304,12 +307,543 @@ def _archive_must_not_run(root): # pragma: no cover - must not be called tarball.unlink() def test_clean_tree_prefers_git_archive(self, monkeypatch, tmp_path): - # The fast path (git archive) is still used when the tree is clean. + # The fast path (git archive) is still used when the tree is clean — + # the tarball is the git-archive output, passed through _inject_dist + # (a no-op without a local static/dist, which tmp_path lacks). monkeypatch.setattr(source, "_tracked_tree_is_dirty", lambda root: False) - sentinel = tmp_path / "archive.tar.gz" - sentinel.write_bytes(b"x") - monkeypatch.setattr(source, "_use_git_archive", lambda root: sentinel) - assert source.build_source_tarball(tmp_path) == sentinel + + def _fake_archive(root): + out = tmp_path / "archive.tar.gz" + with tarfile.open(out, "w:gz") as tf: + payload = tmp_path / "app.py" + payload.write_text("x=1\n") + tf.add(payload, arcname="app.py") + return out + + monkeypatch.setattr(source, "_use_git_archive", _fake_archive) + tarball = source.build_source_tarball(tmp_path) + try: + with tarfile.open(tarball) as tf: + assert "app.py" in tf.getnames() + finally: + tarball.unlink() + + +class TestBuildArchivedDist: + @staticmethod + def _archive(tmp_path: Path, members: dict[str, bytes]) -> Path: + archive = tmp_path / "source.tar.gz" + with tarfile.open(archive, "w:gz") as tf: + for name, data in members.items(): + ti = tarfile.TarInfo(name) + ti.size = len(data) + tf.addfile(ti, io.BytesIO(data)) + return archive + + def test_build_uses_archived_source_not_residual_checkout(self, tmp_path, monkeypatch): + archive = self._archive( + tmp_path, + { + "website/package-lock.json": b'\n{"lockfileVersion": 3}\n', + "website/source-marker.txt": b"archived source\n", + }, + ) + residual = tmp_path / "src" / "kiro_crew" / "static" / "dist" + (residual / "assets").mkdir(parents=True) + (residual / "index.html").write_text("residual\n") + (residual / "assets" / "app.js").write_text("// malicious residual\n") + + def _build(build_root, **_kwargs): + website = build_root / "website" + assert (website / "source-marker.txt").read_text( + encoding="utf-8" + ) == "archived source\n" + dist = website / "dist" + (dist / "assets").mkdir(parents=True) + (dist / "index.html").write_bytes(b'\n') + (dist / "assets" / "app.js").write_bytes(b"// archive-bound bundle\n") + return dist + + monkeypatch.setattr(source.frontend, "build_stock_frontend", _build) + + admitted = dict(source._build_archived_dist(archive)) + + assert admitted[f"{source._DIST_PREFIX}/assets/app.js"] == (b"// archive-bound bundle\n") + assert b"malicious residual" not in b"".join(admitted.values()) + + def test_build_refuses_output_outside_isolated_tree(self, tmp_path, monkeypatch): + archive = self._archive( + tmp_path, {"website/package-lock.json": b'{"lockfileVersion": 3}\n'} + ) + outside = tmp_path / "outside-dist" + (outside / "assets").mkdir(parents=True) + (outside / "index.html").write_text("outside\n") + (outside / "assets" / "app.js").write_text("// outside\n") + monkeypatch.setattr(source.frontend, "build_stock_frontend", lambda *_a, **_kw: outside) + + with pytest.raises(source._AbortInjection, match="outside the isolated"): + source._build_archived_dist(archive) + + def test_unexecutable_npm_refuses_instead_of_raising(self, tmp_path, monkeypatch): + """A resolvable-but-unrunnable npm must refuse, not raise. + + ``build_stock_frontend`` reports a MISSING npm by returning None, but + the spawn itself is unguarded: an npm that resolves via ``shutil.which`` + yet cannot be executed (non-executable, broken symlink, unlinked between + the check and the spawn, EPERM) raises OSError. That has to become an + ``_AbortInjection`` so the caller ships the original archive and the box + builds its own frontend -- otherwise the launch dies with a traceback, + which is exactly what the fallback exists to prevent. + """ + archive = self._archive( + tmp_path, {"website/package-lock.json": b'{"lockfileVersion": 3}\n'} + ) + # A real file that resolves but cannot be executed -> Popen raises. + fake_npm = tmp_path / "npm" + fake_npm.write_text("not executable\n") + fake_npm.chmod(0o644) + monkeypatch.setattr("shutil.which", lambda name, **kw: str(fake_npm)) + + with pytest.raises(source._AbortInjection, match="could not run the frontend build"): + source._build_archived_dist(archive) + + +class TestInjectDist: + """``_inject_dist`` ships only a frontend built from archived source.""" + + @staticmethod + def _make_archive(tmp_path: Path, names: list[str]) -> Path: + out = tmp_path / "in.tar.gz" + with tarfile.open(out, "w:gz") as tf: + for name in names: + payload = tmp_path / "payload.txt" + payload.write_text("x=1\n") + tf.add(payload, arcname=name) + return out + + @staticmethod + def _make_dist(root: Path) -> Path: + dist = root / "src" / "kiro_crew" / "static" / "dist" + (dist / "assets").mkdir(parents=True) + (dist / "index.html").write_text("spa\n") + (dist / "assets" / "app.js").write_text("// bundle\n") + return dist + + @pytest.fixture(autouse=True) + def _build_local_dist(self, monkeypatch, tmp_path): + """Keep admission tests focused; provenance has dedicated tests below.""" + + def _build(archive): + dist = archive.parent / "src" / "kiro_crew" / "static" / "dist" + if not dist.exists(): + raise source._AbortInjection("frontend build failed") + return source._read_dist_bundle(dist, dist.resolve(strict=True)) + + monkeypatch.setattr(source, "_build_archived_dist", _build) + + def test_dist_members_injected(self, tmp_path): + self._make_dist(tmp_path) + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + names = tf.getnames() + assert "install.sh" in names + assert f"{source._DIST_PREFIX}/index.html" in names + assert f"{source._DIST_PREFIX}/assets/app.js" in names + finally: + out.unlink(missing_ok=True) + + def test_no_dist_returns_archive_unchanged(self, tmp_path): + archive = self._make_archive(tmp_path, ["install.sh"]) + assert source._inject_dist(archive, tmp_path) == archive + assert archive.exists() # not rewritten/removed + + def test_dist_symlinks_never_shipped(self, tmp_path): + dist = self._make_dist(tmp_path) + outside = tmp_path / "outside.txt" + outside.write_text("secret beyond the tree\n") + link = dist / "linked.txt" + try: + link.symlink_to(outside) + except OSError: + pytest.skip("symlink creation not permitted here") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + names = tf.getnames() + assert not any("linked.txt" in n for n in names) + assert not any("outside.txt" in n for n in names) + finally: + out.unlink(missing_ok=True) + + def test_dist_secret_files_still_filtered(self, tmp_path): + # allow_dist_under skips only the _EXCLUDE_DIRS ("dist") check — the + # credential checks (.env, .npmrc, .pem) still apply inside the bundle. + dist = self._make_dist(tmp_path) + (dist / ".env").write_text("TOKEN=x\n") + (dist / ".npmrc").write_text("//registry/:_authToken=x\n") + (dist / "cert.pem").write_text("-----BEGIN-----\n") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + names = tf.getnames() + assert f"{source._DIST_PREFIX}/index.html" in names + assert not any(n.endswith(".env") for n in names) + assert not any(n.endswith(".npmrc") for n in names) + assert not any(n.endswith(".pem") for n in names) + finally: + out.unlink(missing_ok=True) + + def test_dist_secret_dirs_still_filtered(self, tmp_path): + # The allow exempts only the literal "dist" component: a secret-bearing + # DIRECTORY staged inside the bundle (e.g. a copied static/dist/.aws) + # must still be refused — the credential checks are name/suffix-based + # and would not catch .aws/credentials on their own. + dist = self._make_dist(tmp_path) + aws_dir = dist / ".aws" + aws_dir.mkdir() + (aws_dir / "credentials").write_text("[default]\naws_access_key_id=x\n") + ssh_dir = dist / ".ssh" + ssh_dir.mkdir() + (ssh_dir / "id_ed25519").write_text("OPENSSH PRIVATE KEY\n") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + names = tf.getnames() + assert f"{source._DIST_PREFIX}/index.html" in names + assert not any("/.aws/" in n for n in names) + assert not any("/.ssh/" in n for n in names) + finally: + out.unlink(missing_ok=True) + + def test_exclude_filter_default_still_drops_dist(self): + # The prefix allow is opt-in: without allow_dist_under, "dist" members + # (e.g. node_modules/pkg/dist/junk.js) stay excluded as before. + ti = tarfile.TarInfo("node_modules/pkg/dist/junk.js") + assert source._exclude_filter(ti) is None + assert source._exclude_filter(ti, allow_dist_under=source._DIST_PREFIX) is None + ok = tarfile.TarInfo(f"{source._DIST_PREFIX}/index.html") + assert source._exclude_filter(ok) is None # default unchanged + assert source._exclude_filter(ok, allow_dist_under=source._DIST_PREFIX) is ok + + def test_allow_dist_under_does_not_allow_sibling_prefix(self): + # The allow is an exact prefix: a path under "dist-evil" is NOT under + # "dist", so the allow must not rescue it. This one has no literal "dist" + # part so it passes either way — the discriminating case is below. + not_under = tarfile.TarInfo("src/kiro_crew/static/dist-evil/x.js") + assert source._exclude_filter(not_under, allow_dist_under=source._DIST_PREFIX) is not_under + # A member that IS under a "dist" dir but NOT the exact allowed prefix + # must stay excluded even when the allow is active. + other_dist = tarfile.TarInfo("website/node_modules/pkg/dist/junk.js") + assert source._exclude_filter(other_dist, allow_dist_under=source._DIST_PREFIX) is None + + def test_hardlinked_credential_in_dist_refused(self, tmp_path): + # A HARDLINK planted in dist aliasing a credential file has a path + # inside the checkout and is not a symlink — only the nolink read gate + # (fstat st_nlink > 1 on the opened descriptor) catches it. A planted + # filesystem object has no legitimate shape inside a Vite build tree, + # so ONE rejection aborts the WHOLE injection: nothing ships, the box + # builds with npm. + dist = self._make_dist(tmp_path) + secret = tmp_path / "outside-secret.txt" + secret.write_text("AKIAIOSFODNN7EXAMPLE\n") + try: + os.link(secret, dist / "assets" / "vendor-abc123.js") + except (OSError, NotImplementedError): + pytest.skip("hardlinks not supported on this filesystem") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + assert out == archive # aborted: original archive, no dist at all + with tarfile.open(archive) as tf: + assert tf.getnames() == ["install.sh"] + + def test_oversized_asset_aborts_atomically(self, tmp_path, monkeypatch): + dist = self._make_dist(tmp_path) + archive = self._make_archive(tmp_path, ["install.sh"]) + original = archive.read_bytes() + real_read = source.hooks.safe_read_file_bytes_nolink + created: list[str] = [] + real_ntf = source.tempfile.NamedTemporaryFile + + def _oversized(path, *args, **kwargs): + if Path(path) == dist / "assets" / "app.js": + raise source.hooks.FileTooLargeError("over the per-file cap") + return real_read(path, *args, **kwargs) + + def _capturing_ntf(*args, **kwargs): + temp = real_ntf(*args, **kwargs) + created.append(temp.name) + return temp + + monkeypatch.setattr(source.hooks, "safe_read_file_bytes_nolink", _oversized) + monkeypatch.setattr(source.tempfile, "NamedTemporaryFile", _capturing_ntf) + + assert source._inject_dist(archive, tmp_path) == archive + assert archive.read_bytes() == original + assert created and not any(Path(path).exists() for path in created) + + def test_credential_in_text_asset_aborts_injection(self, tmp_path): + dist = self._make_dist(tmp_path) + (dist / "assets" / "config.js").write_text( + 'window.config={accessKey:"AKIAIOSFODNN7EXAMPLE"};\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + original = archive.read_bytes() + + out = source._inject_dist(archive, tmp_path) + + assert out == archive + assert archive.read_bytes() == original + with tarfile.open(archive) as tf: + assert tf.getnames() == ["install.sh"] + + def test_exact_bare_aws_secret_in_text_asset_aborts_injection(self, tmp_path): + dist = self._make_dist(tmp_path) + (dist / "assets" / "config.js").write_text( + 'window.secret="wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY";\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + + assert source._inject_dist(archive, tmp_path) == archive + + @pytest.mark.parametrize( + "run", + [ + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEYA", + "XwJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEYABC", + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEYXwJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + ], + ids=["trailing", "leading", "trailing-multi", "two-keys"], + ) + def test_glued_bare_aws_secret_in_text_asset_aborts_injection(self, tmp_path, run): + """A real secret glued to adjacent base64 chars must not ship. + + ``security._contains_bare_secret`` exists to catch exactly this: a + genuine 40-char key with no delimiter on one side reports a 41+ char + run. An admission gate that only acted on the exact ``(40 chars)`` + warning discarded that signal and shipped the key verbatim into an + unauthenticated ``/assets/`` file. + """ + dist = self._make_dist(tmp_path) + (dist / "assets" / "config.js").write_text(f'window.s="{run}";\n') + archive = self._make_archive(tmp_path, ["install.sh"]) + original = archive.read_bytes() + + assert source._inject_dist(archive, tmp_path) == archive + assert archive.read_bytes() == original + with tarfile.open(archive) as tf: + assert tf.getnames() == ["install.sh"] + + def test_data_uri_payload_cannot_hide_a_credential_elsewhere(self, tmp_path): + """Masking a data: URI must not blind the scan to the rest of the file.""" + secret = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + dist = self._make_dist(tmp_path) + (dist / "assets" / "editor.css").write_text( + '.i{background:url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUg")}\n' + f'.j{{content:"{secret}"}}\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + + assert source._inject_dist(archive, tmp_path) == archive + + @pytest.mark.parametrize( + "payload", + [ + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEYABC", + ], + ids=["exact-40", "glued"], + ) + def test_bare_secret_parked_as_a_data_uri_payload_aborts(self, tmp_path, payload): + """A key where a payload goes is not media, so it is not exempt. + + The exemption exists for genuinely inlined assets, which decode to a + recognized container. Keying it on POSITION instead of CONTENT would + turn ``data:image/png;base64,`` into a laundering prefix: writing the + raw key there would discard the bare-secret warning and ship it. + """ + dist = self._make_dist(tmp_path) + (dist / "assets" / "editor.css").write_text( + f'.i{{background:url("data:image/png;base64,{payload}")}}\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + original = archive.read_bytes() + + assert source._inject_dist(archive, tmp_path) == archive + assert archive.read_bytes() == original + + def test_encoded_credential_inside_data_uri_aborts_injection(self, tmp_path): + """The data-URI exemption covers the entropy heuristic, not real keys. + + Only the generic bare-secret heuristic is exempted inside a ``data:`` + payload, because that is the one check built assets false-positive on. + A distinctive credential base64'd into an inlined image must still + abort -- otherwise the exemption would be a laundering channel. + """ + payload = base64.b64encode(b'{"accessKey":"AKIAIOSFODNN7EXAMPLE"}').decode() + dist = self._make_dist(tmp_path) + (dist / "assets" / "editor.css").write_text( + f'.i{{background:url("data:image/png;base64,{payload}")}}\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + original = archive.read_bytes() + + assert source._inject_dist(archive, tmp_path) == archive + assert archive.read_bytes() == original + + def test_real_bundled_data_uri_does_not_false_positive(self, tmp_path): + dist = self._make_dist(tmp_path) + # This 231-character PNG data run comes from Kiro Crew's built editor + # CSS and trips the generic bare-secret sliding-window heuristic. Long + # minified runs are not an asset-safe signal; distinctive credentials + # and exact 40-character bare AWS secrets remain blocked. + data = ( + "iVBORw0KGgoAAAANSUhEUgAAAAQAAAAECAYAAACp8Z5+AAAAAXNSR0IArs4c6Q" + "AAAARnQU1BAACxjwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAAZdEVYdFNv" + "ZnR3YXJlAHBhaW50Lm5ldCA0LjAuMTZEaa/1AAAAHUlEQVQYV2PYvXu3JAi7u" + "LiAMaYAjAGTQBPYLQkAa/0Zef3qRswAAAAASUVORK5CYII" + ) + (dist / "assets" / "editor.css").write_text( + f'.icon{{background:url("data:image/png;base64,{data}")}}\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + assert f"{source._DIST_PREFIX}/assets/editor.css" in tf.getnames() + finally: + out.unlink(missing_ok=True) + + def test_symlinked_index_is_not_read(self, tmp_path): + dist = tmp_path / "src" / "kiro_crew" / "static" / "dist" + (dist / "assets").mkdir(parents=True) + outside = tmp_path / "outside-index.html" + outside.write_text("outside\n") + try: + (dist / "index.html").symlink_to(outside) + except OSError: + pytest.skip("symlink creation not permitted here") + archive = self._make_archive(tmp_path, ["install.sh"]) + + assert source._inject_dist(archive, tmp_path) == archive + + def test_control_char_in_ref_is_escaped_in_diagnostic(self, tmp_path): + dist = tmp_path / "src" / "kiro_crew" / "static" / "dist" + dist.mkdir(parents=True) + (dist / "index.html").write_text('\n') + + with pytest.raises(source._AbortInjection) as exc_info: + source._read_dist_bundle(dist, dist.resolve(strict=True)) + reason = str(exc_info.value) + + assert "\x1b" not in reason + assert r"\x1b" in reason + + def test_non_asset_extension_refused_by_allowlist(self, tmp_path): + # Vite copies website/public/* into dist verbatim, so an untracked + # secrets.yaml (or an extensionless credential) parked there would + # ship to S3. The type allowlist refuses it independent of the + # name/suffix denylists, while the legit members still ship. + dist = self._make_dist(tmp_path) + (dist / "secrets.yaml").write_text("aws_secret_access_key: hunter2\n") + (dist / "id_rsa_backup").write_text("PRIVATE KEY\n") + (dist / "notes.txt").write_text("internal deploy notes\n") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + names = tf.getnames() + assert f"{source._DIST_PREFIX}/index.html" in names + assert f"{source._DIST_PREFIX}/assets/app.js" in names + assert not any(n.endswith("secrets.yaml") for n in names) + assert not any(n.endswith("id_rsa_backup") for n in names) + assert not any(n.endswith("notes.txt") for n in names) + finally: + out.unlink(missing_ok=True) + + def test_allowlist_admits_typical_vite_output(self, tmp_path): + # The allowlist must not refuse real build output: hashed chunks, + # sourcemaps, the webmanifest, fonts, images and wasm all ship. + dist = self._make_dist(tmp_path) + for name in ( + "assets/chunk-ab12cd34.js", + "assets/style-ef56ab78.css", + "assets/app.js.map", + "manifest.json", + "site.webmanifest", + "icon-192.png", + "logo.svg", + "fonts/inter.woff2", + "vendor/react.mjs", + "pcm.wasm", + ): + p = dist / name + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text("x\n") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + try: + with tarfile.open(out) as tf: + names = set(tf.getnames()) + for name in ( + "assets/chunk-ab12cd34.js", + "assets/style-ef56ab78.css", + "assets/app.js.map", + "manifest.json", + "site.webmanifest", + "icon-192.png", + "logo.svg", + "fonts/inter.woff2", + "vendor/react.mjs", + "pcm.wasm", + ): + assert f"{source._DIST_PREFIX}/{name}" in names + finally: + out.unlink(missing_ok=True) + + def test_refused_referenced_chunk_aborts_injection(self, tmp_path): + # Atomicity: if a chunk index.html references is refused admission + # (here: the chunk on disk is a SYMLINK, which the walk skips), the + # bundle must not ship AT ALL — shipping the index without its chunk + # would make install.sh skip the on-box build and the box serve a + # shell whose chunk 404s. + dist = tmp_path / "src" / "kiro_crew" / "static" / "dist" + (dist / "assets").mkdir(parents=True) + (dist / "index.html").write_text( + '\n' + ) + real = tmp_path / "elsewhere.js" + real.write_text("// real chunk kept outside the tree\n") + try: + (dist / "assets" / "app-deadbeef.js").symlink_to(real) + except OSError: + pytest.skip("symlink creation not permitted here") + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + assert out == archive # aborted: partial dist never rides + with tarfile.open(archive) as tf: + assert tf.getnames() == ["install.sh"] + + def test_incomplete_dist_not_shipped(self, tmp_path): + # An index whose referenced hashed chunks are missing (torn tree from a + # failed/partial build) must NOT ship: install.sh would skip the on-box + # npm build because index.html exists, and the box would serve a shell + # whose every chunk 404s. + dist = tmp_path / "src" / "kiro_crew" / "static" / "dist" + dist.mkdir(parents=True) + (dist / "index.html").write_text( + '\n' + ) + archive = self._make_archive(tmp_path, ["install.sh"]) + out = source._inject_dist(archive, tmp_path) + assert out == archive # unchanged: incomplete bundle refused + with tarfile.open(archive) as tf: + assert tf.getnames() == ["install.sh"] class TestBucketNaming: diff --git a/test/test_frontend_dist_resolve.py b/test/test_frontend_dist_resolve.py index 3d6ed6903c3..9c2405d3549 100644 --- a/test/test_frontend_dist_resolve.py +++ b/test/test_frontend_dist_resolve.py @@ -720,6 +720,51 @@ def test_stage_built_dist_sweeps_residue_even_when_refusing(tmp_path): assert not orphan.exists(), "residue survived a refused stage" +def test_build_stock_frontend_uses_lockfile_without_lifecycle_scripts(tmp_path, monkeypatch): + website = tmp_path / "website" + website.mkdir() + (website / "package-lock.json").write_text('{"lockfileVersion": 3}\n') + monkeypatch.setenv("KIROCREW_EDITION_DIR", "/private/edition") + monkeypatch.setenv("KIROCREW_ALLOW_EDITION", "1") + calls: list[tuple[list[str], dict[str, str]]] = [] + + def _run(website_dir, npm, args, timeout, log, label, *, env=None): + assert website_dir == website + assert npm == "/usr/bin/npm" + assert env is not None + calls.append((args, env)) + if args == ["run", "build"]: + _make_dist(website / "dist") + return True + + monkeypatch.setattr(frontend, "_run_npm_step", _run) + + assert frontend.build_stock_frontend(tmp_path, npm="/usr/bin/npm") == (website / "dist") + assert [args for args, _env in calls] == [ + ["ci", "--ignore-scripts", "--no-audit", "--no-fund"], + ["run", "build"], + ] + for _args, env in calls: + assert "KIROCREW_EDITION_DIR" not in env + assert "KIROCREW_ALLOW_EDITION" not in env + + +def test_build_stock_frontend_stops_when_install_fails(tmp_path, monkeypatch): + website = tmp_path / "website" + website.mkdir() + (website / "package-lock.json").write_text('{"lockfileVersion": 3}\n') + calls: list[list[str]] = [] + + def _run(_website, _npm, args, *_rest, **_kwargs): + calls.append(args) + return False + + monkeypatch.setattr(frontend, "_run_npm_step", _run) + + assert frontend.build_stock_frontend(tmp_path, npm="/usr/bin/npm") is None + assert calls == [["ci", "--ignore-scripts", "--no-audit", "--no-fund"]] + + def test_build_and_stage_holds_the_lock_across_the_build(tmp_path): """The lock must be held while `npm run build` runs, not just while copying. diff --git a/test/test_spawn_audit.py b/test/test_spawn_audit.py index 8e6f21db749..3671df76997 100644 --- a/test/test_spawn_audit.py +++ b/test/test_spawn_audit.py @@ -1077,11 +1077,11 @@ def _is_bundled_skill_asset(path: Path) -> bool: # module's own location, never agent input) when no node resolves. Same # class as cli.py::_ensure_node, which invokes the identical script. "env.py::ensure_node", - # Fixed argv (`npm run build`) in the operator's own checkout. The npm - # binary and project path arrive from the caller: the Dev Fleet sync - # resolves npm via its trusted-bin allowlist and the path from the - # operator-registered worktree, never from agent input. - "frontend.py::_npm_build_and_stage_locked", + # Fixed argv (`npm ci`, `npm run build`) in the operator's own checkout + # or isolated archive-build tree. The npm binary and project path arrive + # from the caller: the Dev Fleet sync and cloud source packager resolve + # npm via its trusted-bin allowlist / which, never from agent input. + "frontend.py::_run_npm_step", "frontend.py::build_frontend_async", "frontend.py::build_frontend_sync", # _write_build_source_fingerprint stamps the built bundle's source