From f0bf3f906368276c66eb8b12d6851aab7f8a3e14 Mon Sep 17 00:00:00 2001 From: CrysisDeu Date: Sat, 24 Oct 2026 03:44:36 +0000 Subject: [PATCH] feat: remote crew sign-in via Crew's vault on the KAS bundle route On the KAS bundle route (agent.acp_backend kas with agent.kas_bundle on) the engine's only credential is Crew's own vault and kiro-cli is not installed, yet two remote sign-in surfaces still drove kiro-cli on the remote machine: the cloud instance sign-in over SSM and the aws_control crew container's kiro-cli store seed. The box now decides which program owns the sign-in. New verbs `kirocrew kas login | whoami | logout | route` drive KasLoginService from a shell in kiro-cli's own vocabulary (the device prompt lines, the `whoami --format json` document, the exit codes), and every remote script `kirocrew cloud login|logout` runs asks the box's `kirocrew kas route` before choosing its branch; only the exact answer `kas-bundle` takes the vault verbs, every other answer (an older Kiro Crew included) runs the kiro-cli commands it always ran. The local reading of an identity shares one parser for both, and an OIDC sign-in records its start URL so an Identity Center vault identity can be matched to its organisation. The launching machine's own identity is read from its vault when it runs the bundle route. The container supervisor decides after writing the backend config: on the bundle route it skips kiro-cli's sentinel store seed and requires the installed engine instead; elsewhere the seed runs as before. Builder ID device authorization through the vault was run against the live issuer up to the device prompt; the approval half needs a human and is recorded as not yet validated. Refs #21843 --- docs/system-specs/modules/aws-control.md | 18 +- docs/system-specs/modules/cli.md | 1 + docs/system-specs/modules/cloud.md | 9 +- docs/system-specs/modules/kas-auth.md | 66 +- .../runtime/container/supervisor/__main__.py | 64 +- .../container/supervisor/kiro_login.py | 55 ++ .../crew/runtime/container_tests/conftest.py | 2 +- .../test_supervisor_kiro_login.py | 56 ++ .../container_tests/test_supervisor_main.py | 43 ++ src/kiro_crew/auth/refresh.py | 3 + src/kiro_crew/auth/service.py | 196 +++-- src/kiro_crew/auth/store.py | 80 ++ src/kiro_crew/auth/whoami.py | 171 +++++ src/kiro_crew/cli_cloud.py | 18 +- src/kiro_crew/cli_kas.py | 13 +- src/kiro_crew/cli_kas_login.py | 316 ++++++++ src/kiro_crew/cli_parser_admin.py | 4 +- src/kiro_crew/cli_parser_gateway.py | 7 +- src/kiro_crew/cloud/login.py | 178 ++++- src/kiro_crew/cloud/login_target.py | 49 ++ src/kiro_crew/cloud/login_vault.py | 219 ++++++ src/kiro_crew/cloud/wizard.py | 4 +- src/kiro_crew/dashboard/handlers/kas_login.py | 14 + test/spawn_audit_helpers.py | 9 + test/test_cli_kas_login.py | 331 ++++++++ test/test_cloud_login_target.py | 5 +- test/test_cloud_login_vault.py | 715 ++++++++++++++++++ test/test_kas_auth_whoami.py | 277 +++++++ test/test_kas_login_api.py | 15 + test/test_kas_login_signout_generation.py | 256 +++++++ 30 files changed, 3082 insertions(+), 112 deletions(-) create mode 100644 src/kiro_crew/auth/whoami.py create mode 100644 src/kiro_crew/cli_kas_login.py create mode 100644 src/kiro_crew/cloud/login_vault.py create mode 100644 test/test_cli_kas_login.py create mode 100644 test/test_cloud_login_vault.py create mode 100644 test/test_kas_auth_whoami.py create mode 100644 test/test_kas_login_signout_generation.py diff --git a/docs/system-specs/modules/aws-control.md b/docs/system-specs/modules/aws-control.md index 096ec60f9d0..5a918e0385d 100644 --- a/docs/system-specs/modules/aws-control.md +++ b/docs/system-specs/modules/aws-control.md @@ -2684,8 +2684,10 @@ Seeding the vault is necessary and not sufficient. `kiro-cli acp` validates its credential store before it offers an ACP handshake, so on a store it has never signed into it exits `rc=1` "You are not logged in" and that token request is never reached: the container answers `/health` 200 and every dashboard turn with -`503 kiro_prerequisite_required`. So `kiro_login.seed_kiro_cli_login` runs immediately -after `require_model_identity`. It empties that store's `auth_kv` table +`503 kiro_prerequisite_required`. So `kiro_login.seed_kiro_cli_login` runs after +`require_model_identity` and after `write_backend_config` -- which program owns the +sign-in is a property of the WRITTEN config (a crew bundle may ship one), so the seed +reads that file first (`kiro_login.kas_bundle_route`). It empties that store's `auth_kv` table (`DELETE FROM auth_kv`; `$XDG_DATA_HOME/kiro-cli/data.sqlite3`, falling back to `$HOME/.local/share/...`, plain JSON), writes one row, and then requires `kiro-cli whoami` to accept it; a non-zero exit is a startup refusal. Emptying the @@ -2713,6 +2715,18 @@ internal-only: a desktop host signs kiro-cli in by itself. Every failure inside a startup refusal, because the alternative is the 503 above with a healthy-looking task in front of it. +**On the KAS bundle route none of this runs.** When the written config names the KAS +harness with `agent.kas_bundle` on -- positive on both halves, the shape +`kiro_prerequisite_service._kas_bundle_route` reads -- the backend spawns the installed +engine directly: there is no kiro-cli in the task, no login check in front of the +engine's credential request, and so no store to seed. The vault `seed_model_identity` +fills is the engine's only credential there, exactly as on a bundle-route desktop +([kas-auth.md](kas-auth.md)). Whether the engine is installed is not re-checked here: +the gateway's own readiness gate (`kiro_prerequisite_service`) names a missing engine on +every turn, and the image this spec describes pins kiro-cli and ships no engine, so the +route is reachable today only by a crew bundle or data home that carries one. A config +that cannot be read answers "relay route", the one the image was measured on. + **That is defence in depth, not a licence to drop the sandbox.** What decides whether an auto-approved worker is safe is whether it can REACH a credential, not whether one is resident in its own environment, and the vault is a route the container cannot diff --git a/docs/system-specs/modules/cli.md b/docs/system-specs/modules/cli.md index 94f726d288b..2c131896d5d 100644 --- a/docs/system-specs/modules/cli.md +++ b/docs/system-specs/modules/cli.md @@ -382,6 +382,7 @@ choice blob makes the usage line unreadable. | `kirocrew eval [scenarios…] [--all] [--judge]` | Run multi-session evaluation scenarios; a bare invocation is the ~30s smoke test — see [knowledge.md](knowledge.md). | | `kirocrew sandbox install-profile/status/remove-profile` | Manage the AppArmor profile the agent sandbox needs (Linux) — see [security.md](security.md). | | `kirocrew kas install/status/remove` | Install, inspect or delete Kiro Crew's own KAS engine bundle (`/engines/kas//`: a sha256-pinned `@kiro/agent` build plus its Node runtime). `install` installs this release's pinned version (`acp/kas_pin.py`) and refuses a manifest that does not hash to the pin; it takes `--version` and `--unpinned` (another version, for testing), `--url` (an https base; default `agent.kas_bundle_url`), `--from DIR` (a local manifest and its artifacts, for offline installs) and `--force`. A version the data home already holds is a success with no fetch, so re-running `install.sh --kas` passes; `--force` replaces it. That check is by version name only: a copy an earlier `--unpinned` install laid down under the pin's name also counts, and `--force` is how to swap it for the pinned manifest's engine. `status` shows the pin, the version that runs and every version on disk; `remove --version V` deletes one. Operator-facing like `doctor`: it runs outside the sandbox, which is the only place the `engines` leaf is writable. The route itself is `agent.kas_bundle` — see [acp-client.md](acp-client.md) § KAS bundle route. | +| `kirocrew kas login/whoami/logout/route` | Crew's own Kiro sign-in from a shell (`cli_kas_login.py`), for a host with no browser and no dashboard session -- the remote crew signed in over SSM by `kirocrew cloud login`, a headless host reached by ssh. `login [--provider builder_id\|idc\|google\|github] [--start-url U] [--region R]` runs the device-code flow and prints `Open ` + `code: XXXX-XXXX`, polling until approved (exit 0 once the vault holds the credential; `already signed in`, exit 0, over a usable stored identity). `whoami [--format json]` exits 0 with the identity (the `kiro-cli whoami --format json` document, never a token) or 1 when signed out. `logout` removes every stored identity; a gateway on the host retires its agent processes on its next turn. `route` prints `kas-bundle` when `agent.acp_backend` is `kas` AND `agent.kas_bundle` is on, `kiro-cli` otherwise -- the remote sign-in scripts read it on the box. Not an MCP tool: a sign-in moves a credential and the agent must never choose whose. [kas-auth.md](kas-auth.md) § Sign-in from a shell. | | `kirocrew tailnet status/up/down` | Publish the dashboard on your tailnet (Tailscale) and trust its origin, or stop publishing — see [remote-and-mobile](../../guides/remote-and-mobile.md). | | `kirocrew telemetry status/disable/enable` | Inspect exactly what the anonymous beacon sends, or turn it off permanently — see [metrics.md](metrics.md) and [governance.md](governance.md). | diff --git a/docs/system-specs/modules/cloud.md b/docs/system-specs/modules/cloud.md index 01085b7cc43..5d4759ab619 100644 --- a/docs/system-specs/modules/cloud.md +++ b/docs/system-specs/modules/cloud.md @@ -4,8 +4,9 @@ `src/kiro_crew/cloud/` runs KiroCrew on the user's **own** AWS EC2 instance with a single command. It provisions a CloudFormation stack, ships an available local -checkout (or clones the public repo from packaged installs), signs `kiro-cli` in -over SSM, and opens the dashboard through an SSM port-forward. Command surface +checkout (or clones the public repo from packaged installs), signs the box's Kiro +backend in over SSM (`kiro-cli`, or Crew's own vault on the KAS bundle route), and +opens the dashboard through an SSM port-forward. Command surface (wired in `cli_cloud.py`, invoked via `kirocrew cloud `): @@ -72,7 +73,9 @@ claim that a hostile in-process agent is fully contained. | `ec2.py` | `deploy`/`status`/`stop`/`start`/`destroy` via `aws cloudformation` + `ec2`; AZ- **and egress-**aware `discover_network` + `resolve_explicit_subnet` (`--subnet` pin, same guarantees); tag-based stateless discovery; `_validate_cidr`. `find_stack` verifies BOTH `kirocrew:managed=true` AND `kirocrew:instance==` before status/stop/start/destroy touch a stack — so a same-prefix managed stack with a different instance tag can't be acted on by the wrong `--tag`. | | `iam.py` | Least-privilege launcher policy generator (applied by the user, never by KiroCrew) + read-only reachability check + the **content-fixed instance permissions-boundary document** (`boundary_policy_document`/`boundary_arn`) and its constants (`BOUNDARY_NAME`). | | `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 `cancel_device_login` — which stops a login this crew started and removes the files holding its code, WITHOUT dropping the box's session, because a cancelled attempt must not sign the crew in later and must not take an older valid session with it — and `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). | +| `login.py` | Device-code / social sign-in on the box over SSM -- `kiro-cli`'s, or Crew's own vault's on the KAS bundle route (every remote script asks the box which, see `login_vault.py`) -- plus `cancel_device_login` — which stops a login this crew started and removes the files holding its code, WITHOUT dropping the box's session, because a cancelled attempt must not sign the crew in later and must not take an older valid session with it — and `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). | +| `login_vault.py` | The on-box half of the sign-in when Crew's own vault owns the credential. The local `kirocrew cloud` command cannot read the instance's `config.json`, so **the box decides**: every remote script (`login.py`'s check / whoami / login / logout / callback / cancel) splices in `route_prelude()`, which resolves the box's `kirocrew` and runs its `kirocrew kas route`; only the exact answer `kas-bundle` (`agent.acp_backend == "kas"` AND `agent.kas_bundle` on, the positive shape `kiro_prerequisite_service._kas_bundle_route` uses) takes the vault branch -- an empty answer (a box whose Kiro Crew predates the verb, or no `kirocrew` at all) and every other answer take the kiro-cli branch, whose `kiro-cli` invocations are the ones the scripts always ran. Before either branch, `require_sign_in_program()` refuses readably in two cases. First, when the probe RAN and could not decide -- `kas route` exited `ROUTE_UNKNOWN_EXIT` (1: unreadable config, or a crash) -- it echoes `__KIROCREW_ROUTE_UNKNOWN__`, the probe's output and the remedy, and touches NEITHER program whether or not kiro-cli is installed: a box whose config just broke may be on the bundle route, and `kiro-cli logout` there would report a sign-out while the vault credential stays usable. Argparse's 2 (a `kirocrew` too old for the verb) and the shell's 127 (no `kirocrew`) are NOT that case: they are "no route answer" and keep the kiro-cli branch. Second, when the box has NEITHER program: the route was not `kas-bundle` (the probe's exit code, its stdout and its stderr are kept apart in `$KC_ROUTE_RC` / `$KC_ROUTE_OUT` / `$KC_ROUTE_ERR`; only stdout is compared, since the config loader warns on stderr while the verb still answers `kas-bundle`) and `kiro-cli` did not resolve. It echoes `__KIROCREW_NO_SIGNIN_PROGRAM__`, the probe's output (the `kas route` error for an unreadable config, or the shell's "not found" for a missing `kirocrew`) and the remedy, so the operator never reads `kiro-cli: command not found` for what is a bundle-route box whose config broke; the local side reads either sentinel as UNKNOWN (`_auth_probe` None, `remote_identity` None, `logout` False) and as the prompt's error, never as signed out and never as a reason to start a flow. On the vault branch the scripts drive `kirocrew kas whoami [--format json]`, `kirocrew kas login [--provider idc --start-url U --region R]` (one background launch for every target: the verb prompts for nothing, so the kiro-cli IdC pty driver has no counterpart) and `kirocrew kas logout` ([kas-auth.md](kas-auth.md) § Sign-in from a shell); their output is read by the SAME capture loop, `parse_login_output` and `parse_whoami_output`, because the verbs print kiro-cli's vocabulary (`https://…user_code=…`, `code: XXXX-XXXX`, the `accountType`/`startUrl` document) and use its exit codes. The device-login script echoes `__KIROCREW_VAULT_ROUTE__` on that branch so `start_device_login` never degrades a failed vault sign-in into the kiro-cli social-callback flow (there is no kiro-cli on that box); the callback script itself refuses with `__KIROCREW_VAULT_NO_CALLBACK__`. Cancel and replace `pkill` both command-line patterns (`kiro-cli login`, `kirocrew kas login`) and confirm the stop only when both are gone. The vault logout kills no engine process: the vault is part of the gateway's identity fingerprint and its pre-turn identity-change check retires the agent processes on the next turn. Nothing crosses SSM but the prompt the user must open; the device code and the token stay between the box's `kirocrew` and the issuer. | +| `login_target.py` | `KiroLoginTarget`, the durable identity a managed launch signs in as (Builder ID default, or an Identity Center start URL + region), the `whoami` parser both routes share, `identity_matches_target`, and `discover_local_identity` -- the launching machine's own sign-in, inherited by `cloud launch` / `setup`. On a local KAS bundle route that reading comes from Crew's own vault (`local_vault_identity`, rendered through `auth/whoami.whoami_document` and read back through the same parser), with the same three answers as the kiro-cli reading: an identity, `{}` for none to inherit, `None` for unknown. An explicitly supplied `kiro_bin` still asks kiro-cli. | | `connect.py` | SSM port-forward + token mint + open browser; Instances-registry integration; `redact_token`. **The Fargate lane has no connect verb here.** A Fargate crew is reached by the instances layer's `fargate` connection method, surfaced as **Settings > Remote Crew**, whose card opens the forward and shows the task's turn API URL. What this module contributes to that lane is `FARGATE_TURN_PATH`, the path that forward dials, spelled here rather than imported from an image's source; `FARGATE_HEALTH_PATH` beside it is dialled from nowhere in the gateway and exists so the container-contract test can assert the pair. Nothing mints for a Fargate task: it runs the crew container, whose only listener is a front proxy serving a JSON turn API with the backend loopback-only and every control path authorisation-gated and then 404, so there is no mint route to call and no browser to open. There is no CLI counterpart either, and the asymmetry is the reason -- `cloud connect` addresses a crew by EC2 tag, and `cloud launch` cannot create a Fargate crew at all, because that engine is reachable only through the dashboard's provisioner API -- so `cloud connect`'s no-instance failure names Settings > Remote Crew rather than a verb that would dial a crew the CLI cannot make. `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, pinned to this build's release tag by `ec2.resolve_public_ref` (see Provisioning shape and source). 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. **`ensure_crew_boundary`** and **`ensure_crew_exec_boundary`** do the same for the two Fargate ceilings (`kirocrew-crew-boundary`, `kirocrew-crew-exec-boundary`), and all three route through one `_ensure_boundary` sequence whose ORDER is the security property: an existing policy is verified against the expected content-fixed document BEFORE it is reused, and a lost create race is verified on the way back, so a permissive policy seeded at either name is refused rather than trusted to cap nothing. | | `config.py` | Persisted profile / region / tag **plus the optional `fargate` and `microvm` blocks** (**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. The `fargate` and `microvm` fields hold each block **exactly as read**, and `fargate_config()` / `microvm_config()` are what judge them (see "The MicroVM lane" below for the second). **This module has no writer:** no `save()`, no `apply_update()`, no lock. `profile` / `region` / `last_tag` are still READ here so an install whose pointer predates `launch_state.py` keeps resuming, and `launch_state.py` is where those three are written now. See "The Fargate lane's configuration home" and "Where launch state lives" below. | diff --git a/docs/system-specs/modules/kas-auth.md b/docs/system-specs/modules/kas-auth.md index 265ae873833..e3f14fd80f4 100644 --- a/docs/system-specs/modules/kas-auth.md +++ b/docs/system-specs/modules/kas-auth.md @@ -332,8 +332,69 @@ access token. - `KIRO_API_KEY` remains a supported headless path (matches KAS `EnvAuthProvider`), and its ApiKey identity must fail-fast on usage/credit APIs (existing lesson). +## Sign-in from a shell, and on a remote crew + +The dashboard card is one driver of `KasLoginService`; `kirocrew kas login | whoami | +logout | route` (`cli_kas_login.py`) is the other, for a host with no browser and no +dashboard session. It exists because on the bundle route the vault is the engine's +ONLY credential and kiro-cli is not installed, so `kiro-cli login/whoami/logout` have +nothing to act on there. The verbs deliberately speak kiro-cli's vocabulary: + +| Verb | Does | Output / exit | +|---|---|---| +| `kas login [--provider builder_id\|idc\|google\|github] [--start-url] [--region]` | device-code sign-in (the one transport that needs no callback to reach the host), polls until approved, saves to the vault | prints `Open ` and `code: XXXX-XXXX`, exits 0 once stored; on a vault that already holds a usable identity prints `already signed in` and exits 0 (switching is `logout` then `login`) | +| `kas whoami [--format json]` | who the vault is signed in as: `KasToken.is_usable` (the predicate the spawn reads) AND no refresh-rejected marker on the slot, so a sign-in doctor and the card call "expired -- sign in again" reads as signed out here too and `kas login` replaces it | exit 0 + the `kiro-cli whoami --format json` document (`auth/whoami.whoami_document`: `accountType`, and `startUrl` when recorded -- nothing else, never a token or e-mail); exit 1 when signed out | +| `kas logout` | removes every stored identity (`KasLoginService.logout`) and bumps the store's **sign-out generation** (below) | a running gateway retires its agent processes on the next turn: the vault is in its identity fingerprint and the pre-turn identity-change check runs the same sweep the dashboard's sign-out runs; a dashboard login pending at that moment is refused when its code is approved | +| `kas route` | which program owns the Kiro sign-in on this host | `kas-bundle` when `agent.acp_backend == "kas"` AND `agent.kas_bundle` is on, `kiro-cli` otherwise; a config that cannot be read -- a load that raised, OR one the loader returned as defaults with `_base_unreadable` / `_overlay_unreadable` set (`auth/whoami.load_sign_in_owner_config`) -- is a plain-language error naming the path and the fault, exit 1: the owner is then UNKNOWN, never kiro-cli, so a remote script neither runs a kiro-cli branch on a bundle-route box nor signs the wrong store out. The predicate itself is `auth/whoami.agent_owns_sign_in(agent)`, the one in-process spelling | + +`accountType` is spelled as kiro-cli spells it (`BuilderId`, `IamIdentityCenter`, +`ExternalIdP`, `Social`), so `cloud/login_target.parse_whoami_output` and +`identity_matches_target` read a vault identity and a kiro-cli one through one parser. +An SSO-OIDC sign-in records the start URL it was begun against on the stored token +(`KasToken.extra["start_url"]`, written by `_poll_oidc`): it is configuration, not a +credential, and it is the one field that tells one organisation's Identity Center +sign-in from another's. Every refresh (`auth/refresh.py`'s three builders) carries +`extra` onto the renewed token, so the claim survives the token it was recorded on. Tokens stored before it was recorded carry none, and a +`whoami` without it is read as "an Identity Center sign-in of an unknown +organisation", never as Builder ID. + +**Sign-out generation.** The service's in-memory epoch stops a login begun in THIS +process from re-filling a vault a sign-out in this process emptied. `kirocrew kas +logout` is a second process over the same vault, so the store carries a marker beside +the refresh-rejected ones (`TokenStore.signout_generation` / `bump_signout_generation`, +`kas/signed-out`, a fresh random value per sign-out): every `begin_*` snapshots it, every +sign-out bumps it after the slots are deleted, and `_persist_and_finish` compares the +two before it writes. Compare-and-save (with the `replaces` slot deletions of an +account switch) and delete-all-and-bump each run under one store-wide flock (`TokenStore.with_signout_lock`, `kas/signout.lock`, taken outside any +identity's refresh lock), so a sign-out cannot land between a login's compare and its +write. `kirocrew kas logout` runs the same sign-out over an empty vault too, so the +generation moves even when nothing was stored. A mismatch is terminal for that login +(`{"status": "error", "code": "signed_out_during_login"}`, the entry is dropped); +`POST /api/kas-login/poll` answers it as the same coded 409 the begin handlers answer +for the in-flight case, so the card ends the sign-in and offers Start over instead of +restarting the provider on the device flow as it does for an ordinary terminal loopback +answer. Unlike the refresh-rejected marker this one is not best-effort: it is the +fence itself, so a bump that cannot be written raises `TokenStoreError` and the +sign-out reports failure rather than publishing "signed out" while a pending approval +can still undo it. + +The remote crew uses exactly these verbs: `kirocrew cloud login|logout` and the +launch wizard run their sign-in scripts on the instance over SSM, and every script +asks the box's own `kirocrew kas route` which program to drive +([cloud.md](cloud.md) § `login_vault.py`). The local machine never reads the remote +config and never holds the remote credential. The aws_control crew container has no +sign-in surface of its own -- its identity arrives from Secrets Manager into the +vault -- and on the bundle route it skips kiro-cli's store seed and requires the +engine instead ([aws-control.md](aws-control.md)). + ## Validation status +- Builder ID **device-code** flow through the vault, from a shell (`kirocrew kas login + --provider builder_id` in an isolated data home, no kiro-cli): `RegisterClient` and + `StartDeviceAuthorization` were accepted by the live `oidc.us-east-1.amazonaws.com` + and the verification URL + user code were printed. The approval half (a human + signing in to the printed URL) and the resulting `CreateToken` have NOT been run + live; IdC and External IdP remain unit-tested only. - Social **device-code** flow: **validated end-to-end** — a stdlib-only Python client (no kiro-cli) obtained a real Google `{accessToken, refreshToken, profileArn}` via `/oauth/device/authorization` → user browser approval → `/oauth/device/poll`. Confirms @@ -351,8 +412,9 @@ access token. ## Not yet done -- Live-endpoint validation of Builder ID / IdC / External IdP (only social device flow - is proven against the real service). +- Live-endpoint validation of the approval and token half of Builder ID, and of IdC / + External IdP end to end (only the social device flow is proven against the real + service; Builder ID is proven up to the device prompt). - The loopback flow has a dashboard driver: `POST /api/kas-login/loopback` (`api_kas_login_begin_loopback`) starts the PKCE loopback sign-in and returns the portal URL plus the polling handle. A coded 409 `loopback_unavailable` (the install diff --git a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/__main__.py b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/__main__.py index edf9b8188f2..437bdfd713f 100644 --- a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/__main__.py +++ b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/__main__.py @@ -962,32 +962,6 @@ def run(settings: Settings, *, wait_for_shutdown=None) -> int: "which would authenticate the task as another account without saying so." ) backend_mod.require_model_identity(settings) - # Then satisfy kiro-cli's OWN login check, which is a separate question from - # whether this task has an identity. - # - # `kiro-cli acp` validates its own credential store before it offers an ACP - # handshake, so on a store it has never signed into it exits rc=1 "You are not - # logged in" and the `_kiro/auth/getAccessToken` request the vault answers is - # never reached: the container serves /health 200 and answers every dashboard - # turn with a 503. The row written here is a NON-SECRET sentinel, not a copy of - # the credential -- Crew is the auth owner and the engine asks the host for the - # token it uses, so what the store needs is the answer to "has this been signed - # into", which carries nothing worth reading. - # - # AFTER the vault check, because the order is what makes each refusal say the - # right thing: no identity is that check's verdict, and this one's is that - # kiro-cli would refuse to start. Both are startup refusals, because the thing - # they prevent is a container that starts and then 503s every turn. - # - # Handed `env`, NOT this process's own environment. The step runs kiro-cli to ask - # its login check a question, and this process still holds the delivered - # credential in its environment at this point -- the pop below has not run yet, - # and cannot run earlier because `build_backend_env` above is what reads it. An - # inherited copy would put the credential in a child that kiro-cli may outlive - # through a helper, readable by the later same-uid model worker. `env` is the - # dictionary `build_backend_env` already scrubbed of both credential shapes, and - # it carries the same HOME, so the store resolves to the same path either way. - kiro_login_mod.seed_kiro_cli_login(env=env) # Now drop BOTH credential shapes from this process's own environment. The front is # spawned with no env argument and so inherits this one whole, and # `build_backend_env` only ever cleaned the COPY handed to the backend -- so without @@ -1023,7 +997,43 @@ def run(settings: Settings, *, wait_for_shutdown=None) -> int: # bundle may ship config, and this has to win on the keys it sets) and before the # backend, which reads this file at boot: a transport it starts there is already # connected by the time anything else could object. - backend_mod.write_backend_config(settings) + config_path = backend_mod.write_backend_config(settings) + # Then satisfy the SIGN-IN PROGRAM's own check, which is a separate question + # from whether this task has an identity, and which program that is depends on + # the config just written -- hence after it, not after the vault check. + # + # On the KAS bundle route (`agent.acp_backend` kas AND `agent.kas_bundle` on) the + # backend spawns the installed engine directly: there is no kiro-cli, no login + # check in front of the engine's credential request, and nothing to seed. Whether + # the engine is installed is the gateway's own readiness gate's question + # (`kiro_prerequisite_service`), which names a missing engine on every turn. + # + # Everywhere else `kiro-cli acp` validates its own credential store before it + # offers an ACP handshake, so on a store it has never signed into it exits rc=1 + # "You are not logged in" and the `_kiro/auth/getAccessToken` request the vault + # answers is never reached: the container serves /health 200 and answers every + # dashboard turn with a 503. The row written here is a NON-SECRET sentinel, not a + # copy of the credential -- Crew is the auth owner and the engine asks the host + # for the token it uses, so what the store needs is the answer to "has this been + # signed into", which carries nothing worth reading. + # + # AFTER the vault check, because the order is what makes each refusal say the + # right thing: no identity is that check's verdict, and this one's is that the + # sign-in program would refuse to start. All are startup refusals, because the + # thing they prevent is a container that starts and then 503s every turn. + # + # Handed `env`, NOT this process's own environment. The kiro-cli step spawns + # kiro-cli to ask its login check a question; `env` is the dictionary + # `build_backend_env` scrubbed of both credential shapes, and it carries the same + # HOME, so the store resolves to the same path either way. (This process's own + # environment has been scrubbed too by now, above; handing the backend's copy + # keeps the seam the same whatever runs before it.) + if kiro_login_mod.kas_bundle_route(config_path): + log.info( + "KAS bundle route: kiro-cli's login store is not seeded (no kiro-cli on this route)" + ) + else: + kiro_login_mod.seed_kiro_cli_login(env=env) # 1. Claim writer ownership in shared storage, THEN restore, THEN the backend. The # claim is first because restoration takes the authority pair before any writer diff --git a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/kiro_login.py b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/kiro_login.py index 0373f5c4283..f4ea6ceabf9 100644 --- a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/kiro_login.py +++ b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container/supervisor/kiro_login.py @@ -79,6 +79,15 @@ remove, arrived at a second way. With it, that drift is loud and names the version. Every failure here is a startup refusal, for the same reason. + +ON THE KAS BUNDLE ROUTE NONE OF THIS RUNS. When the written config names the KAS +harness with ``agent.kas_bundle`` on (:func:`kas_bundle_route`), the backend spawns +the installed engine directly: there is no kiro-cli in the task, no login check in +front of the engine's credential request, and so no store to seed. The vault seeded +by ``backend.seed_model_identity`` is the engine's only credential there, exactly as it +is on a bundle-route desktop. Whether that engine is installed is the gateway's own +readiness gate's question (``kiro_prerequisite_service``), which names a missing +engine on every turn; this module does not duplicate it. """ from __future__ import annotations @@ -343,3 +352,49 @@ def seed_kiro_cli_login( _require_accepted(binary, path, run=run, env=env) log.info("kiro-cli accepts its login store at %s (row %s)", path, key) return path + + +# ── the bundle route: no kiro-cli, so no store to seed ─────────────────────── + +#: The ``agent.acp_backend`` value that selects the KAS harness, spelled as +#: ``backend.FORCED_AGENT_SETTINGS`` writes it. Read from that mapping rather than +#: repeated, so the two cannot drift. +_KAS_BACKEND_KEY: str = "acp_backend" +_KAS_BUNDLE_KEY: str = "kas_bundle" + + +def kas_bundle_route(config_path: Path | None) -> bool: + """Whether the config the backend boots with runs Crew's OWN engine. + + True only when the written ``config.json`` names the KAS harness AND turns + ``agent.kas_bundle`` on -- positive on both halves, the same shape the + gateway's readiness gate reads (``kiro_prerequisite_service._kas_bundle_route``). + On that route the backend spawns the installed engine directly, kiro-cli is + not part of the task, and the engine's credential request reaches the vault + with no kiro-cli login check in front of it: the sentinel row this module + writes has nothing to satisfy, and a kiro-cli that is not installed cannot be + asked to accept it. Everywhere else (the relay route, the container's default) + the seed runs exactly as before. + + Read from the FILE the backend reads, after ``backend.write_backend_config`` has + written it, so the answer is about the config that boots and not about an + earlier draft a crew bundle may have changed. A file that cannot be read or + parsed answers False: the relay route is the one the image was measured on, + and its seed is what refuses loudly when something is wrong. + """ + from . import backend # sibling; imported here to keep the standalone import tree flat + + if not isinstance(config_path, Path): + # Nothing was written (a stubbed writer answers None): no route to read. + return False + try: + raw = json.loads(config_path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return False + agent = raw.get("agent") if isinstance(raw, dict) else None + if not isinstance(agent, dict): + return False + return ( + agent.get(_KAS_BACKEND_KEY) == backend.FORCED_AGENT_SETTINGS[_KAS_BACKEND_KEY] + and agent.get(_KAS_BUNDLE_KEY) is True + ) diff --git a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/conftest.py b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/conftest.py index 00c235b0321..a6682fcbaf3 100644 --- a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/conftest.py +++ b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/conftest.py @@ -118,7 +118,7 @@ # long before anyone notices it drifted. It may be LOWERED only alongside a deliberate # deletion of tests, in the same commit, and never to make a red lane green: a floor # edited down to meet the measurement measures nothing. -_MIN_COLLECTED = 836 +_MIN_COLLECTED = 847 # Not collected off Linux. This suite's SUBJECT is the source of a Linux container # image, built by the deploy driver and run on Fargate -- not part of the application diff --git a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_kiro_login.py b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_kiro_login.py index fa4f71678bc..3b0f3bb9637 100644 --- a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_kiro_login.py +++ b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_kiro_login.py @@ -428,3 +428,59 @@ def test_a_row_under_another_key_is_removed_too(tmp_path, monkeypatch): with sqlite3.connect(db) as conn: rows = conn.execute(f"SELECT key FROM {AUTH_TABLE}").fetchall() assert [key for (key,) in rows] == [SENTINEL_ROW_KEY] + + +# --- the KAS bundle route: no kiro-cli, so no store to seed ------------------ + + +def _write_config(path: Path, agent: dict | None) -> Path: + path.write_text(json.dumps({"agent": agent} if agent is not None else {}), encoding="utf-8") + return path + + +@pytest.mark.parametrize( + "agent, expected", + [ + ({"acp_backend": "kas", "kas_bundle": True}, True), + ({"acp_backend": "kas", "kas_bundle": False}, False), + ({"acp_backend": "kas"}, False), + ({"acp_backend": "kiro", "kas_bundle": True}, False), + ({"acp_backend": "kas", "kas_bundle": "true"}, False), + (None, False), + ], +) +def test_the_bundle_route_is_positive_on_both_halves_of_the_written_config( + tmp_path, agent, expected +): + cfg = _write_config(tmp_path / "config.json", agent) + assert kiro_login_mod.kas_bundle_route(cfg) is expected + + +def test_the_bundle_route_reads_the_config_the_container_actually_writes(tmp_path): + """The predicate compares against the backend name `build_backend_config` forces, + read from the file that function writes, so a respelling there cannot leave this + reading an old name -- and a bundle flag the arriving config carries survives the + forced merge, which is the only way the container ever takes the route.""" + from container.supervisor import backend + + written = backend.build_backend_config({"agent": {"kas_bundle": True}}) + cfg = tmp_path / "config.json" + cfg.write_text(json.dumps(written), encoding="utf-8") + assert kiro_login_mod.kas_bundle_route(cfg) is True + plain = tmp_path / "plain.json" + plain.write_text(json.dumps(backend.build_backend_config(None)), encoding="utf-8") + assert kiro_login_mod.kas_bundle_route(plain) is False + + +def test_an_unreadable_or_malformed_config_reads_as_the_relay_route(tmp_path): + assert kiro_login_mod.kas_bundle_route(tmp_path / "missing.json") is False + bad = tmp_path / "bad.json" + bad.write_text("{not json", encoding="utf-8") + assert kiro_login_mod.kas_bundle_route(bad) is False + (tmp_path / "list.json").write_text("[]", encoding="utf-8") + assert kiro_login_mod.kas_bundle_route(tmp_path / "list.json") is False + + +def test_no_written_config_reads_as_the_relay_route(): + """A writer that produced nothing (None) has no route to read; the seed runs.""" + assert kiro_login_mod.kas_bundle_route(None) is False diff --git a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_main.py b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_main.py index 7c51de7ea38..b511607f030 100644 --- a/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_main.py +++ b/src/kiro_crew/apps/builtins/aws_control/crew/runtime/container_tests/test_supervisor_main.py @@ -536,3 +536,46 @@ def recording_seed(**kw): # the time run() returns it has popped both names from this process's own # environment, which is the later half of the same interlock. backend_mod.require_model_identity(make_settings(tmp_path)) + + +# --- run() picks the sign-in program from the config it just wrote ----------- + + +def test_the_login_seed_runs_after_the_config_it_decides_on_is_written( + wired, tmp_path, monkeypatch +): + """Which program owns the sign-in is a property of the WRITTEN config (a crew + bundle may ship one), so the kiro-cli seed must follow `write_backend_config`, + and still precede the backend.""" + order: list[str] = [] + real_write = backend_mod.write_backend_config + + def recording_write(settings): + order.append("write_backend_config") + return real_write(settings) + + def recording_seed(**kw): + order.append("seed_kiro_cli_login") + wired.append("seed_kiro_cli_login") + + monkeypatch.setattr(backend_mod, "write_backend_config", recording_write) + monkeypatch.setattr(entry.kiro_login_mod, "seed_kiro_cli_login", recording_seed) + entry.run(make_settings(tmp_path), wait_for_shutdown=lambda c: "signal") + assert order == ["write_backend_config", "seed_kiro_cli_login"] + assert wired.index("seed_kiro_cli_login") < wired.index("start_backend") + + +def test_on_the_bundle_route_the_kiro_cli_store_is_not_seeded(wired, tmp_path, monkeypatch): + """A bundle-route task has no kiro-cli: nothing to seed, and the backend still starts.""" + settings = make_settings(tmp_path) + settings.config_dir.mkdir(parents=True, exist_ok=True) + (settings.config_dir / "config.json").write_text( + json.dumps({"agent": {"kas_bundle": True}}), encoding="utf-8" + ) + + def never_seed(**kw): + raise AssertionError("kiro-cli's store must not be seeded on the bundle route") + + monkeypatch.setattr(entry.kiro_login_mod, "seed_kiro_cli_login", never_seed) + entry.run(settings, wait_for_shutdown=lambda c: "signal") + assert "start_backend" in wired diff --git a/src/kiro_crew/auth/refresh.py b/src/kiro_crew/auth/refresh.py index cc885a70ef8..76e0b4fa82a 100644 --- a/src/kiro_crew/auth/refresh.py +++ b/src/kiro_crew/auth/refresh.py @@ -228,6 +228,7 @@ async def _refresh_social(token: KasToken, *, session: aiohttp.ClientSession) -> refresh_token=data.get("refreshToken") or token.refresh_token, profile_arn=profile_arn, region=token.region, + extra=dict(token.extra), ) @@ -255,6 +256,7 @@ async def _refresh_sso_oidc(token: KasToken, *, session: aiohttp.ClientSession) region=region, client_id=token.client_id, client_secret=token.client_secret, + extra=dict(token.extra), ) @@ -285,6 +287,7 @@ async def _refresh_external_idp(token: KasToken, *, session: aiohttp.ClientSessi auth_method="external_idp", client_id=token.client_id, token_endpoint=token.token_endpoint, + extra=dict(token.extra), ) diff --git a/src/kiro_crew/auth/service.py b/src/kiro_crew/auth/service.py index eb2a629b7bb..a166d98257c 100644 --- a/src/kiro_crew/auth/service.py +++ b/src/kiro_crew/auth/service.py @@ -39,6 +39,7 @@ TokenStore, TokenStoreError, ) +from kiro_crew.auth.whoami import START_URL_EXTRA_KEY logger = logging.getLogger(__name__) @@ -136,6 +137,11 @@ class _PendingOidcLogin: identity: str provider: str resolve_profile: bool + # The start URL the authorization was begun against, recorded on the stored + # token (``KasToken.extra["start_url"]``) so a later ``whoami`` can say WHICH + # organisation an Identity Center sign-in belongs to. Configuration, not a + # credential. + start_url: str = "" # Same budget as _PendingLogin.transport_failures, for the SSO-OIDC poll. transport_failures: int = 0 @@ -235,6 +241,13 @@ def __init__( # credential lands (see _persist_and_finish). Kept beside, not inside, the # pending entries: it is a property of the request, not of the flow. self._replaces: dict[str, str] = {} + # login_id -> the store's sign-out generation when that login began. Checked + # again before the credential is persisted: a different value means a + # sign-out landed meanwhile in SOME process on this host (this gateway or a + # `kirocrew kas logout` in a shell), and the credential must not re-fill + # the vault that sign-out emptied. The epoch below is the same guard for + # this process alone, read without a file. + self._signout_gen: dict[str, str] = {} # Bumped by every sign-out. A begin reads it before its first await and # hands it to _register_pending, which refuses to register a login that # was started before a sign-out: otherwise a begin already talking to the @@ -316,6 +329,7 @@ async def begin_device( # Before the first await: a sign-out that lands anywhere after this line # invalidates this begin (see _register_pending). epoch = self._epoch + signout_gen = await asyncio.to_thread(self._store.signout_generation) replaces = _validate_replaces(replaces) normalized = (provider_str or "").strip().lower() if normalized in ("builder_id", "idc"): @@ -332,6 +346,7 @@ async def begin_device( resolve_profile=False, replaces=replaces, epoch=epoch, + signout_gen=signout_gen, ) if normalized == "idc": cleaned = (start_url or "").strip() @@ -345,6 +360,7 @@ async def begin_device( resolve_profile=True, replaces=replaces, epoch=epoch, + signout_gen=signout_gen, ) provider = _parse_provider(provider_str) session = await self._http() @@ -356,6 +372,7 @@ async def begin_device( expires_at=auth.expires_at, replaces=replaces, epoch=epoch, + signout_gen=signout_gen, ) async def _begin_oidc( @@ -368,6 +385,7 @@ async def _begin_oidc( resolve_profile: bool, replaces: str = "", epoch: int, + signout_gen: str = "", ) -> dict[str, Any]: """Register a fresh SSO-OIDC client and start its device authorization.""" session = await self._http() @@ -383,12 +401,14 @@ async def _begin_oidc( identity=identity, provider=provider, resolve_profile=resolve_profile, + start_url=start_url, ), user_code=auth.user_code, verification_uri_complete=auth.verification_uri_complete, expires_at=auth.expires_at, replaces=replaces, epoch=epoch, + signout_gen=signout_gen, ) async def _register_pending( @@ -400,6 +420,7 @@ async def _register_pending( expires_at: datetime, replaces: str = "", epoch: int, + signout_gen: str = "", ) -> dict[str, Any]: # Opaque handle so the deviceCode (the secret half of the flow) never # travels back to the browser; the poll endpoint accepts only this id. @@ -417,9 +438,11 @@ async def _register_pending( now = datetime.now(timezone.utc) expired = [lid for lid, entry in self._pending.items() if _expires_at(entry) <= now] for lid in expired: - _release(self._pending.pop(lid)) - self._replaces.pop(lid, None) + entry = self._drop_records(lid) + if entry is not None: + _release(entry) self._pending[login_id] = pending + self._signout_gen[login_id] = signout_gen if replaces: self._replaces[login_id] = replaces return { @@ -577,6 +600,8 @@ async def _poll_oidc(self, login_id: str, pending: _PendingOidcLogin) -> dict[st await self._clear_transport_failures(login_id) if token is None: return {"status": "pending"} + if pending.start_url: + token.extra[START_URL_EXTRA_KEY] = pending.start_url if pending.resolve_profile: # An IdC token is unusable without a profile ARN (the store itself drops # it), so resolution failures must end the login loudly, not save junk. @@ -620,51 +645,94 @@ async def _persist_and_finish(self, login_id: str, token: KasToken) -> dict[str, async with self._lock: if login_id not in self._pending: raise UnknownLoginError(login_id) - result = await self._save_token(token) - self._pending.pop(login_id, None) - replaces = self._replaces.pop(login_id, "") - if result.get("status") == "authorized" and replaces: - # An explicit account switch. The store resolves by fixed slot - # priority (external_idp > builder_id > identity_center > social), - # not by recency, so ANY other stored slot that outranks the new one - # -- the one the user named, or one they had forgotten about -- would - # leave the agents on a different account while the card claimed - # the switch happened. A switch therefore means "this is the one - # account": every other slot is removed, only now, after the new - # credential is on disk, so a failed sign-in never leaves the vault - # empty. The delete is unconditional (a no-op on an empty slot): - # `load` only decides what to REPORT as replaced, never whether to - # delete, because a slot whose read failed transiently is exactly - # the one that would otherwise survive and resolve later. A removal - # that fails is reported, not hidden: the re-read status will still - # show the account that won, and Sign out clears it by hand. - removed: list[str] = [] - failed = False + # A sign-out in ANOTHER process (`kirocrew kas logout`, or a second + # gateway on this data home) cannot bump this process's epoch, so the + # store's own sign-out generation is compared before the write -- and + # the compare and the write run under the store-wide sign-out flock, + # the same one every sign-out holds across its deletes and its bump, + # so a sign-out cannot land between the two. A login begun before that + # sign-out does not re-fill the vault it emptied. Terminal for this + # login, as an in-process sign-out is; the poll handler answers it as + # the same 409 a begin does, so the card ends the sign-in rather than + # retrying it on another transport. + result = await self._save_token( + token, + begun_gen=self._signout_gen.get(login_id, ""), + replaces=self._replaces.get(login_id, ""), + ) + if result.get("code") == "signed_out_during_login": + entry = self._drop_records(login_id) + if entry is not None: + _release(entry) + logger.info( + "login %s not persisted: a sign-out landed while it was pending", login_id + ) + return result + self._drop_records(login_id) + return result + + async def _save_token( + self, token: KasToken, *, begun_gen: str | None = None, replaces: str = "" + ) -> dict[str, Any]: + """Write an approved token to the store and shape the poll's terminal answer. + + Takes no asyncio lock, so ``_persist_and_finish`` can run it while holding + ``self._lock`` to keep the login cancellable until the write lands. With + *begun_gen* (the sign-out generation the login began under) the write runs + under the store's sign-out flock and is skipped -- answering + ``signed_out_during_login`` -- when the generation has moved: that is the + cross-process half of the sign-out guard (``TokenStore.signout_generation``). + + *replaces* names an explicit account switch. The store resolves by fixed + slot priority (external_idp > builder_id > identity_center > social), not + by recency, so ANY other stored slot that outranks the new one -- the one + the user named, or one they had forgotten about -- would leave the agents + on a different account while the card claimed the switch happened. A + switch therefore means "this is the one account": every other slot is + removed, only after the new credential is on disk (so a failed sign-in + never leaves the vault empty) and INSIDE the same flock as the save, so a + second login finishing in another process (the shell verb beside the + dashboard) cannot interleave its own save and deletes with these and + leave the vault holding neither credential. The delete is unconditional + (a no-op on an empty slot): ``load`` only decides what to REPORT as + replaced, never whether to delete, because a slot whose read failed + transiently is exactly the one that would otherwise survive and resolve + later. A removal that fails is reported, not hidden: the re-read status + will still show the account that won, and Sign out clears it by hand. + """ + + def _locked_save() -> tuple[bool, list[str], bool]: + saved = False + removed: list[str] = [] + failed = False + + def _compare_save_replace() -> None: + nonlocal saved, failed + if begun_gen is not None and self._store.signout_generation() != begun_gen: + return + self._store.save(token) + saved = True + if not replaces: + return for other in KNOWN_IDENTITIES: if other == token.identity: continue try: - held = await asyncio.to_thread(self._store.load, other) is not None - await asyncio.to_thread(self._store.delete, other) + held = self._store.load(other) is not None + self._store.delete(other) if held: removed.append(other) except TokenStoreError as err: logger.warning("could not remove replaced KAS identity %s: %s", other, err) failed = True - if removed: - result["replaced"] = removed - if failed: - result["code"] = "previous_identity_not_removed" - return result - async def _save_token(self, token: KasToken) -> dict[str, Any]: - """Write an approved token to the store and shape the poll's terminal answer. + self._store.with_signout_lock(_compare_save_replace) + return saved, removed, failed - Takes no lock, so ``_persist_and_finish`` can run it while holding - ``self._lock`` to keep the login cancellable until the write lands. - """ try: - await asyncio.to_thread(self._store.save, token) + saved, removed, failed = await asyncio.to_thread(_locked_save) + if not saved: + return {"status": "error", "code": "signed_out_during_login"} except TokenStoreError as err: # Disk full / read-only store: the login was approved but we cannot # persist it. Report error (not authorized); the caller drops the @@ -674,7 +742,12 @@ async def _save_token(self, token: KasToken) -> dict[str, Any]: # The one poll error a dashboard must NOT retry on another transport: # the store itself is unwritable, so every flavor would fail the same way. return {"status": "error", "code": "token_store_failed"} - return {"status": "authorized", "provider": token.provider} + result: dict[str, Any] = {"status": "authorized", "provider": token.provider} + if removed: + result["replaced"] = removed + if failed: + result["code"] = "previous_identity_not_removed" + return result async def begin_loopback(self, provider_str: str, *, replaces: str = "") -> dict[str, Any]: """Start a loopback (PKCE) social login; returns the portal URL to open. @@ -691,6 +764,7 @@ async def begin_loopback(self, provider_str: str, *, replaces: str = "") -> dict between this call's start and its registration. """ epoch = self._epoch # before the first await; see _register_pending + signout_gen = await asyncio.to_thread(self._store.signout_generation) replaces = _validate_replaces(replaces) provider = _parse_provider(provider_str) if select_transport() is not Transport.LOOPBACK: @@ -716,6 +790,7 @@ async def begin_loopback(self, provider_str: str, *, replaces: str = "") -> dict expires_at=expires_at, replaces=replaces, epoch=epoch, + signout_gen=signout_gen, ) # The dashboard opens this URL itself: the browser is the user's, not ours. result["auth_url"] = auth_url @@ -781,13 +856,11 @@ async def _poll_loopback(self, login_id: str, pending: _PendingLoopbackLogin) -> if self._pending.get(login_id) is not pending: raise UnknownLoginError(login_id) if pending.task.cancelled(): - self._pending.pop(login_id, None) - self._replaces.pop(login_id, None) + self._drop_records(login_id) return {"status": "error", "code": "loopback_cancelled"} err = pending.task.exception() if err is not None: - self._pending.pop(login_id, None) - self._replaces.pop(login_id, None) + self._drop_records(login_id) if err is None: return await self._persist_and_finish(login_id, pending.task.result()) if isinstance(err, portal.PortalTimeoutError): @@ -833,21 +906,50 @@ async def logout(self, identity: str) -> None: entries = list(self._pending.values()) self._pending.clear() self._replaces.clear() + self._signout_gen.clear() + # Released BEFORE the store is touched: a delete that raises must not + # leave a loopback listener holding its port with no entry left to + # find it by. for entry in entries: _release(entry) - # The named slot first: an unknown kind raises before any write. - await asyncio.to_thread(self._store.delete, identity) - for other in KNOWN_IDENTITIES: - if other != identity: - await asyncio.to_thread(self._store.delete, other) + # An unknown kind raises before any write -- checked here against the + # store's own list, with no filesystem call on the serving loop. + if identity not in KNOWN_IDENTITIES: + raise ValueError(f"unknown identity kind: {identity!r}") + + def _delete_all_and_bump() -> None: + self._store.delete(identity) + for other in KNOWN_IDENTITIES: + if other != identity: + self._store.delete(other) + # Then tell every OTHER process on this host: a login pending in + # the gateway while this runs in a shell (or the reverse) compares + # this before it persists, and refuses. Deletes and bump sit under + # one sign-out flock with that compare-and-save, so no login can + # slip a write between them. + self._store.bump_signout_generation() + + await asyncio.to_thread(self._store.with_signout_lock, _delete_all_and_bump) # Nothing may resolve after a sign-out. A slot that survived is a # failure to sign out, not a detail. if await asyncio.to_thread(self._store.resolve) is not None: raise TokenStoreError("a stored Kiro identity survived sign-out") + def _drop_records(self, login_id: str) -> _PendingKind | None: + """Remove every per-login record (caller holds ``self._lock``); returns the entry. + + The three tables are keyed by one id and must leave together: a login that + drops out of ``_pending`` but keeps its ``_signout_gen`` row is never swept + again (the eviction in ``_register_pending`` walks ``_pending`` only) and + the table grows by one row per abandoned login. + """ + entry = self._pending.pop(login_id, None) + self._replaces.pop(login_id, None) + self._signout_gen.pop(login_id, None) + return entry + async def _forget(self, login_id: str) -> None: async with self._lock: - entry = self._pending.pop(login_id, None) - self._replaces.pop(login_id, None) + entry = self._drop_records(login_id) if entry is not None: _release(entry) diff --git a/src/kiro_crew/auth/store.py b/src/kiro_crew/auth/store.py index 617610cc8ea..f2435667319 100644 --- a/src/kiro_crew/auth/store.py +++ b/src/kiro_crew/auth/store.py @@ -22,6 +22,7 @@ import json import logging import os +import secrets from collections.abc import Callable from dataclasses import asdict, dataclass, field from datetime import datetime, timezone @@ -224,6 +225,42 @@ def lock_path(self, identity: str) -> Path: make_owner_only_dir(self._kas_dir) return self._kas_dir / f"refresh-{identity}.lock" + def signout_lock_path(self) -> Path: + """Path to the store-wide sign-out lock (owner-only ``kas`` dir). + + One lock for the whole store, taken OUTSIDE any identity's refresh lock: + a sign-out holds it across every slot's delete and the generation bump, + and a login holds it across its generation compare and its save, so the + two cannot interleave between the compare and the write. Order is always + sign-out lock, then an identity's refresh lock, never the reverse. + """ + self._assert_unlinked() + make_owner_only_dir(self._kas_dir) + return self._kas_dir / "signout.lock" + + def with_signout_lock(self, action: Callable[[], None]) -> None: + """Run ``action`` while holding the store-wide sign-out lock. + + Same flock discipline as :meth:`_with_refresh_lock`; lock failures are + ``TokenStoreError``. ``action`` may take identity refresh locks inside + (``save`` and ``delete`` do), never the other way round. + """ + try: + fd = os.open(str(self.signout_lock_path()), os.O_RDWR | os.O_CREAT, 0o600) + except OSError as err: + raise TokenStoreError("could not take the sign-out lock") from err + try: + try: + acquire_lock(fd, exclusive=True) + except OSError as err: + raise TokenStoreError("could not take the sign-out lock") from err + try: + action() + finally: + release_lock(fd) + finally: + os.close(fd) + def _with_refresh_lock(self, identity: str, action: Callable[[], None]) -> None: """Run ``action`` while holding the identity's refresh lock (:meth:`lock_path`). @@ -415,6 +452,49 @@ def refresh_rejected(self, identity: str) -> datetime | None: except ValueError: return None + # ── sign-out generation: one marker every process that empties the vault bumps ── + + def _signout_generation_path(self) -> Path: + return self._kas_dir / "signed-out" + + def signout_generation(self) -> str: + """An opaque value that changes on every sign-out, ``""`` before the first. + + A login reads it when it begins and again, under :meth:`with_signout_lock`, + right before it persists its credential + (:meth:`KasLoginService._persist_and_finish`); a different value means a + sign-out landed in between -- from ANY process on this host, the gateway's + own sign-out or a ``kirocrew kas logout`` in a shell -- and the credential + must not be written into the vault that sign-out just emptied. The gateway's + in-memory epoch covers only its own pending logins; this is the same guard + across processes. An unreadable marker reads as ``""``, so a login that began + before the marker existed still refuses once one is written. + """ + try: + return self._signout_generation_path().read_text(encoding="utf-8").strip() + except (OSError, ValueError): + return "" + + def bump_signout_generation(self) -> None: + """Record a sign-out so a login begun before it refuses to persist. + + Called by every sign-out path after the slots are deleted, inside + :meth:`with_signout_lock`. NOT best-effort, unlike the refresh-rejected + marker: the marker IS the fence that keeps a login pending in another + process from re-filling the vault this sign-out emptied, so a marker that + cannot be written raises ``TokenStoreError`` and the sign-out reports + failure -- the slots are already gone, but "signed out" must not be + published while a pending approval can still undo it. A fresh random + value rather than a counter, so two processes bumping at once cannot land + on one value a pending login already read. + """ + try: + self._assert_unlinked() + make_owner_only_dir(self._kas_dir) + self._signout_generation_path().write_text(secrets.token_hex(16), encoding="utf-8") + except OSError as err: + raise TokenStoreError("could not record the sign-out generation") from err + def resolve(self) -> KasToken | None: """Return the highest-priority stored token (External > Builder > Social). diff --git a/src/kiro_crew/auth/whoami.py b/src/kiro_crew/auth/whoami.py new file mode 100644 index 00000000000..5099bb2dd5d --- /dev/null +++ b/src/kiro_crew/auth/whoami.py @@ -0,0 +1,171 @@ +"""Who Crew's own vault is signed in as, in the vocabulary ``kiro-cli whoami`` set. + +On the KAS bundle route (``agent.acp_backend == "kas"`` with ``agent.kas_bundle`` +on) the vault under ``/kas`` is the engine's only credential, so the +question "who is this machine signed in as" is answered here and not by +``kiro-cli whoami``, which that route does not install. Every reader of a remote +or local identity -- the cloud login target (:mod:`kiro_crew.cloud.login_target`), +the on-box ``kirocrew kas whoami`` the cloud sign-in runs over SSM -- already +parses ``kiro-cli whoami --format json`` (``accountType``, ``startUrl``, ``email``), +so the vault's answer is rendered in that same document shape and goes through the +same parser. One vocabulary, two credential owners, no second code path on the +reading side. + +What the vault can say is the KIND of account: Builder ID, Identity Center, +company SSO, Google or GitHub. It stores no e-mail, so no ``email`` key is ever +written -- an empty string there would read as "known to be blank" to the identity +comparisons on the kiro-cli path, which share this shape. The Identity Center +start URL is recorded on the token at sign-in (``KasToken.extra["start_url"]``), +because it is the one field that tells one organisation's sign-in from another's +(:func:`kiro_crew.cloud.login_target.identity_matches_target`). + +Nothing here returns, logs or prints a token value. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from kiro_crew.auth.store import KasToken, TokenStore + +#: ``KasToken.extra`` key under which an SSO-OIDC sign-in records the start URL it +#: was begun against. Written by :class:`kiro_crew.auth.service.KasLoginService`, +#: read here. Absent on tokens stored before it was recorded, and on every +#: non-OIDC identity. +START_URL_EXTRA_KEY = "start_url" + +#: The vault's store slot -> the ``accountType`` kiro-cli prints for that kind of +#: sign-in. The social slot is spelled from its provider instead (below), as +#: ``Social`` -- the same family :func:`is_personal_account_type` accepts. +_ACCOUNT_TYPE_BY_SLOT: Mapping[str, str] = { + "builder_id": "BuilderId", + "identity_center": "IamIdentityCenter", + "external_idp": "ExternalIdP", +} + + +class ConfigUnreadable(Exception): + """The config could not be read, so which program owns the sign-in is unknown. + + Raised by :func:`load_sign_in_owner_config` for a load that raised AND for a + load that came back degraded (``config.json`` or ``config.local.json`` present + but unparseable, so the agent section is a default rather than the user's). + The two look different to the loader and the same to a sign-in: a default + agent section says "kiro-cli" about a host that may well be on the bundle + route, and acting on it signs the wrong store out. + """ + + +def agent_owns_sign_in(agent: Any) -> bool: + """THE bundle-route predicate over a loaded ``agent`` section. + + Positive on both halves: ``agent.acp_backend == "kas"`` AND ``agent.kas_bundle`` + on -- the shape ``kiro_prerequisite_service._kas_bundle_route`` uses for the + readiness gate. The bundle flag alone says nothing when another harness is + selected, and the backend alone is the kiro-cli relay route, where kiro-cli + owns the sign-in. + """ + from kiro_crew.agent_sdk.backends import ACP_BACKEND_KAS + + return getattr(agent, "acp_backend", "") == ACP_BACKEND_KAS and bool( + getattr(agent, "kas_bundle", False) + ) + + +def load_sign_in_owner_config() -> Any: + """Load the config for a sign-in-owner decision, refusing a degraded read. + + Imports the config loader at call time so this module stays cheap for + ``kirocrew``'s argument parser. Raises :class:`ConfigUnreadable` when the load + raised or came back with ``_base_unreadable`` / ``_overlay_unreadable`` set. + """ + from kiro_crew.config.loader import KiroCrewConfig + + try: + cfg = KiroCrewConfig.load() + except Exception as exc: # noqa: BLE001 - every failure is the same refusal + raise ConfigUnreadable(f"{type(exc).__name__}: {exc}") from exc + unreadable = [ + name + for name, flag in ( + ("config.json", getattr(cfg, "_base_unreadable", False)), + ("config.local.json", getattr(cfg, "_overlay_unreadable", False)), + ) + if flag + ] + if unreadable: + raise ConfigUnreadable( + f"{' and '.join(unreadable)} present but unparseable, so the settings read are " + "defaults, not yours" + ) + return cfg + + +def vault_owns_sign_in() -> bool: + """Whether THIS host runs the KAS bundle route, so Crew's vault owns the sign-in. + + :func:`agent_owns_sign_in` over :func:`load_sign_in_owner_config`. A config + that cannot be read (or read only as defaults) answers False here, the answer + that keeps kiro-cli as the owner everywhere it is today -- right for an + in-process reader that has a kiro-cli fallback, wrong for ``kirocrew kas route``, + which therefore reports the failure instead (``cli_kas_login._route``). + """ + try: + return agent_owns_sign_in(load_sign_in_owner_config().agent) + except ConfigUnreadable: + return False + + +def account_type_for(identity: str, provider: str) -> str: + """Map a vault slot and its provider class onto whoami's ``accountType``. + + ``identity`` is the store slot (one of ``kiro_crew.auth.store.KNOWN_IDENTITIES``); + ``provider`` the governance class stored beside it. ``""`` when neither names a + kind this vocabulary has a word for. + """ + if identity == "social": + return f"Social{provider}" if provider else "" + return _ACCOUNT_TYPE_BY_SLOT.get(identity, "") + + +def whoami_document(token: KasToken) -> dict[str, str]: + """The ``kiro-cli whoami --format json`` document the stored *token* stands for. + + Keys are kiro-cli's own camelCase, and only the two the parser reads: + ``accountType`` always, ``startUrl`` when the sign-in recorded one. Never the + token itself, never the refresh token, never the profile ARN. + """ + doc = {"accountType": account_type_for(token.identity, token.provider)} + start_url = token.extra.get(START_URL_EXTRA_KEY) if isinstance(token.extra, dict) else None + if isinstance(start_url, str) and start_url: + doc["startUrl"] = start_url + return doc + + +def resolve_signed_in(store: TokenStore) -> KasToken | None: + """The identity *store* is signed in as, or ``None`` when it is not signed in. + + "Signed in" is :meth:`KasToken.is_usable` -- the one predicate the spawn-time + owner decision, ``kirocrew doctor`` and the dashboard card read -- AND no + refresh-rejected marker on the slot: an entry whose refresh the issuer has + refused still carries a refresh token, so ``is_usable`` stays true while every + credential callback fails. ``kirocrew doctor`` and the sign-in card already call + that "sign-in expired -- sign in again", and this answers the same, so + ``kas login`` goes on to a fresh sign-in that replaces the slot instead of + printing "already signed in" over a dead one. A lapsed entry with nothing to + renew it answers ``None`` too, exactly as ``kiro-cli whoami`` exits non-zero on a + session it cannot renew. A vault that cannot be read answers ``None`` as well: + the caller is asking whether a sign-in is needed, and "cannot tell" must not + read as "signed in". + """ + try: + token = store.resolve() + if token is None or not token.is_usable(): + return None + if store.refresh_rejected(token.identity) is not None: + return None + except Exception: # noqa: BLE001 - a status read never raises + return None + return token diff --git a/src/kiro_crew/cli_cloud.py b/src/kiro_crew/cli_cloud.py index 665a007dde1..9ae6834e91e 100644 --- a/src/kiro_crew/cli_cloud.py +++ b/src/kiro_crew/cli_cloud.py @@ -248,7 +248,11 @@ def _cloud_connect(args: argparse.Namespace) -> int: def _cloud_login(args: argparse.Namespace) -> int: - """Sign kiro-cli into the instance (device-code) — the backend chats need this. + """Sign the instance's Kiro backend in (device-code) — the backend chats need this. + + The box decides which program that is: ``kiro-cli`` on the relay route, Crew's + own vault (``kirocrew kas login``) on the KAS bundle route, which installs no + kiro-cli. The prompt, the identity check and the sign-out read the same here. Standalone re-entry for the wizard's sign-in step: after a non-interactive launch (``--yes``) nobody approved the browser code, so kiro-cli is logged @@ -327,7 +331,7 @@ def _cloud_login(args: argparse.Namespace) -> int: def _cloud_logout(args: argparse.Namespace) -> int: - """Sign kiro-cli out on the instance — the way to switch Kiro accounts. + """Sign the instance's Kiro backend out — the way to switch Kiro accounts. ``cloud login`` short-circuits when a session already exists, so switching accounts otherwise means an SSM console round-trip to run ``kiro-cli @@ -340,16 +344,14 @@ def _cloud_logout(args: argparse.Namespace) -> int: ui.fail(f"No running instance for tag '{tag}'.") return 1 - with ui.Spinner("Signing kiro-cli out on the instance…"): + with ui.Spinner("Signing out on the instance…"): signed_out = login_mod.logout(st["instance_id"], profile, region) if not signed_out: ui.fail("Could not confirm the instance is signed out.") ui.detail("The session may still be active — retry, or check with: kirocrew cloud connect") return 1 ui.ok("Signed out on the instance.") - ui.detail( - "Any in-flight chats/cron sessions were stopped (their kiro-cli runtimes were killed)." - ) + ui.detail("Any in-flight chats/cron sessions were stopped or will stop at their next turn.") ui.detail("Sign in with another account: kirocrew cloud login") return 0 @@ -679,8 +681,8 @@ def handle_cloud(args: argparse.Namespace) -> int: ui.detail("status Show one instance's state") ui.detail("tunnel Open the dashboard SSM tunnel (alias: connect)") ui.detail("connect Open the dashboard over an SSM tunnel") - ui.detail("login Sign kiro-cli in on the instance (fixes chat errors)") - ui.detail("logout Sign kiro-cli out on the instance (switch Kiro account)") + ui.detail("login Sign Kiro in on the instance (fixes chat errors)") + ui.detail("logout Sign Kiro out on the instance (switch Kiro account)") ui.detail("stop|start Pause / resume (save cost)") ui.detail("destroy Remove everything from AWS") ui.detail("iam-policy Print the least-privilege IAM policy to apply") diff --git a/src/kiro_crew/cli_kas.py b/src/kiro_crew/cli_kas.py index b21b1ec5937..6332931b704 100644 --- a/src/kiro_crew/cli_kas.py +++ b/src/kiro_crew/cli_kas.py @@ -75,6 +75,12 @@ def add_kas_parser(parser: argparse.ArgumentParser) -> None: "--version", default=None, help="Delete only this version (default: every version)" ) remove.add_argument("--yes", action="store_true", help="Do not ask for confirmation") + # The sign-in verbs (login / whoami / logout / route) live beside the engine + # verbs because they share the route: on the bundle route the vault is the + # engine's only credential. Their wording and flow are cli_kas_login's. + from kiro_crew.cli_kas_login import add_kas_login_parsers + + add_kas_login_parsers(sub) def _progress_printer(): @@ -270,5 +276,10 @@ def kas_cmd(args: argparse.Namespace) -> int: return _status(args) if action == "remove": return _remove(args) - print("usage: kirocrew kas {install,status,remove}", file=sys.stderr) + from kiro_crew.cli_kas_login import kas_login_cmd + + handled = kas_login_cmd(args) + if handled is not None: + return handled + print("usage: kirocrew kas {install,status,remove,login,whoami,logout,route}", file=sys.stderr) return 2 diff --git a/src/kiro_crew/cli_kas_login.py b/src/kiro_crew/cli_kas_login.py new file mode 100644 index 00000000000..dbd7a12160a --- /dev/null +++ b/src/kiro_crew/cli_kas_login.py @@ -0,0 +1,316 @@ +"""``kirocrew kas login | whoami | logout | route`` -- Crew's own Kiro sign-in from a terminal. + +The dashboard's Kiro sign-in card drives :class:`kiro_crew.auth.service.KasLoginService` +over HTTP. These verbs drive the same service from a shell, which is what a machine +with no browser and no dashboard session needs: the remote crew ``kirocrew cloud +login`` signs in over SSM, and any headless host an operator reaches by ssh. They +exist because on the KAS bundle route (``agent.kas_bundle`` on) the vault is the +engine's ONLY credential and kiro-cli is not installed, so ``kiro-cli login`` / +``whoami`` / ``logout`` have nothing to act on there. + +The verbs speak the vocabulary the cloud sign-in already reads from kiro-cli: + +- ``login`` prints the verification URL and the ``code: XXXX-XXXX`` line, then keeps + polling until the user approves in a browser on any device (the device-code flow, + the one transport that needs no callback to reach this machine), and exits 0 + once the credential is in the vault. On a vault that already holds a usable + identity it prints ``already signed in`` and exits 0, as ``kiro-cli login`` does + over a live session, so switching accounts is ``logout`` then ``login``. +- ``whoami`` exits 0 and prints the identity when signed in, exits 1 when not; + ``--format json`` prints the ``kiro-cli whoami --format json`` document + (:func:`kiro_crew.auth.whoami.whoami_document`). +- ``logout`` removes every stored identity. A gateway on this host notices on its + next turn: the vault is part of its identity fingerprint, and the pre-turn + identity-change check retires the running agent processes, so nothing keeps + serving the signed-out account past that turn. +- ``route`` prints which program owns the Kiro sign-in on this host -- + ``kas-bundle`` when the next spawn runs Crew's own engine, ``kiro-cli`` otherwise, + and a plain-language error with exit 1 when the config cannot be read (the owner + is then unknown, not kiro-cli). The cloud sign-in scripts read it on the box to + pick the sign-in program; it is the answer to "how does a remote command know + the remote config". + +Operator-facing like ``kirocrew kas install``: it runs outside the agent sandbox, +the only place the ``kas`` vault leaf is writable. Not an LLM-facing command and +deliberately not an MCP tool: a sign-in moves a credential, and the agent must +never be able to choose whose. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import sys +from typing import Any + +#: What ``kirocrew kas route`` prints when Crew's own engine runs the agents and the +#: vault is the only sign-in. The cloud sign-in's remote scripts compare against this +#: exact spelling (``cloud/login_vault.ROUTE_KAS_BUNDLE`` carries the same constant on +#: the reading side; ``test_cli_kas_login.py`` pins the two equal). +ROUTE_KAS_BUNDLE = "kas-bundle" +#: Printed for every other configuration: the kiro-cli backend, the KAS relay route +#: and any harness that signs in through kiro-cli. It names the program that owns the +#: sign-in, so the cloud scripts keep running ``kiro-cli`` exactly as before. +ROUTE_KIRO_CLI = "kiro-cli" +#: ``kas route``'s exit status when the config could not be read, so the owner is +#: UNKNOWN. Distinct from the shell's 127 (no ``kirocrew`` on the box) and argparse's +#: 2 (a ``kirocrew`` too old to have the verb), which the cloud scripts read as "no +#: route answer, kiro-cli as always"; this one they read as "do not touch either +#: program" (``cloud/login_vault.route_prelude``). A Python crash exits 1 too, and +#: is rightly read the same way: an answer that cannot be trusted is not kiro-cli. +ROUTE_UNKNOWN_EXIT = 1 + +#: Seconds between two polls of a pending device authorization. kiro-cli's own +#: device flow polls at the issuer's suggested interval, which is five seconds for +#: both SSO-OIDC and the Kiro social service; one constant here keeps the CLI inside +#: both issuers' ``slow_down`` thresholds. +POLL_INTERVAL_SECS = 5.0 + +_PROVIDERS = ("builder_id", "idc", "google", "github") + + +def add_kas_login_parsers(sub: Any) -> None: + """Attach ``login`` / ``whoami`` / ``logout`` / ``route`` to the ``kas`` subparsers.""" + login = sub.add_parser( + "login", + help="Sign Kiro Crew's own vault in with a device code (no browser needed here)", + ) + login.add_argument( + "--provider", + default="builder_id", + choices=_PROVIDERS, + help="Which identity to sign in with (default: builder_id)", + ) + login.add_argument( + "--start-url", + default="", + help="IAM Identity Center start URL (required with --provider idc)", + ) + login.add_argument( + "--region", + default="", + help="IAM Identity Center region (default: the Builder ID region)", + ) + whoami = sub.add_parser("whoami", help="Show which Kiro identity the vault is signed in as") + whoami.add_argument("--format", default="text", choices=("text", "json"), help="Output format") + sub.add_parser("logout", help="Remove every Kiro identity from Kiro Crew's vault") + sub.add_parser( + "route", + help=f"Print which program owns the Kiro sign-in here: {ROUTE_KAS_BUNDLE} or {ROUTE_KIRO_CLI}", + ) + + +def _route(_args: argparse.Namespace) -> int: + from kiro_crew.auth.whoami import ( + ConfigUnreadable, + agent_owns_sign_in, + load_sign_in_owner_config, + ) + from kiro_crew.config.paths import data_home + + # Not ``vault_owns_sign_in``: that answers "kiro-cli" for a config it cannot + # read, the right fallback for an in-process caller but, printed from this + # verb, it would send a bundle-route box down a kiro-cli branch it cannot run + # (or, with kiro-cli installed, sign the wrong store out). A config that + # cannot be read -- raised, or read only as defaults -- is reported with the + # path and the fault, exit 1, so the remote scripts know the owner is UNKNOWN. + try: + cfg = load_sign_in_owner_config() + except ConfigUnreadable as exc: + print( + f"❌ could not read the Kiro Crew config under {data_home()}: {exc}. Which program " + "owns the Kiro sign-in here is unknown; fix the config, then run this again.", + file=sys.stderr, + ) + return ROUTE_UNKNOWN_EXIT + print(ROUTE_KAS_BUNDLE if agent_owns_sign_in(cfg.agent) else ROUTE_KIRO_CLI) + return 0 + + +def _whoami(args: argparse.Namespace) -> int: + from kiro_crew.auth.bridge import default_token_store, describe_vault_identity + from kiro_crew.auth.whoami import resolve_signed_in, whoami_document + + token = resolve_signed_in(default_token_store()) + if token is None: + print("Not signed in to Kiro Crew.", file=sys.stderr) + return 1 + if args.format == "json": + print(json.dumps(whoami_document(token))) + return 0 + doc = whoami_document(token) + print(f"Signed in to Kiro Crew: {doc['accountType']} ({describe_vault_identity()})") + if doc.get("startUrl"): + print(f" start URL: {doc['startUrl']}") + return 0 + + +def _logout(_args: argparse.Namespace) -> int: + from kiro_crew.auth.bridge import default_token_store + from kiro_crew.auth.service import KasLoginService + from kiro_crew.auth.store import KNOWN_IDENTITIES, TokenStoreError + + store = default_token_store() + token = store.resolve() + # The service's sign-out runs even over an empty vault: besides the deletes it + # advances the store's sign-out generation, which is what stops a dashboard + # login pending in the gateway at this moment from filling the vault once its + # code is approved. An empty vault has nothing to delete, but that login still + # has to be refused. + identity = token.identity if token is not None else KNOWN_IDENTITIES[0] + try: + asyncio.run(KasLoginService(store).logout(identity)) + except TokenStoreError as exc: + print(f"❌ sign-out did not complete: {exc}", file=sys.stderr) + return 1 + if token is None: + print("Not signed in to Kiro Crew; nothing to sign out.") + else: + print("Signed out of Kiro Crew.") + return 0 + + +def _login(args: argparse.Namespace) -> int: + from kiro_crew.auth.bridge import default_token_store, describe_vault_identity + from kiro_crew.auth.whoami import resolve_signed_in + + store = default_token_store() + if resolve_signed_in(store) is not None: + # The same answer kiro-cli gives over a live session, in the words the + # cloud sign-in's parser reads as "already signed in": switching accounts + # is `kirocrew kas logout` first, never a second identity stacked on top. + print( + f"Already signed in to Kiro Crew ({describe_vault_identity()}). " + "Run `kirocrew kas logout` first to switch accounts." + ) + return 0 + if args.provider == "idc" and not (args.start_url or "").strip(): + print("❌ --provider idc requires --start-url", file=sys.stderr) + return 2 + # A lapsed entry in another slot would outrank a fresh lower-priority sign-in + # at resolve time (the store picks by fixed slot order, not recency), so it is + # named as the slot this login replaces: the service removes every other slot + # once the new credential has landed, never before. Read here, off any loop. + stale = store.resolve() + try: + return asyncio.run( + _run_device_login( + store, + provider=args.provider, + start_url=args.start_url, + region=args.region, + replaces=stale.identity if stale is not None else "", + ) + ) + except KeyboardInterrupt: + print("\nSign-in cancelled; nothing was stored.", file=sys.stderr) + return 130 + + +async def _run_device_login( + store: Any, *, provider: str, start_url: str, region: str, replaces: str = "" +) -> int: + """Begin a device-code login, print its prompt, poll until it ends.""" + # Imported here, not at module scope: ``kirocrew``'s argument parser imports + # this module while it is built, and aiohttp and the auth subsystem (with the + # ``cryptography`` wheel behind the vault) must not be the price of `--help`. + import aiohttp + + from kiro_crew.auth.login.builder_id import BuilderIdAuthError + from kiro_crew.auth.login.device import DeviceAuthError + from kiro_crew.auth.service import ( + InvalidRegionError, + KasLoginService, + MissingStartUrlError, + SignedOutDuringLoginError, + UnknownLoginError, + ) + + service = KasLoginService(store) + try: + try: + begun = await service.begin_device( + provider, + start_url=start_url, + region=region, + replaces=replaces, + ) + except MissingStartUrlError: + print("❌ --provider idc requires --start-url", file=sys.stderr) + return 2 + except InvalidRegionError as exc: + print(f"❌ {exc}", file=sys.stderr) + return 2 + except (BuilderIdAuthError, DeviceAuthError) as exc: + print(f"❌ the sign-in service refused to start a sign-in: {exc}", file=sys.stderr) + return 1 + except (aiohttp.ClientError, asyncio.TimeoutError) as exc: + print(f"❌ could not reach the sign-in service: {exc}", file=sys.stderr) + return 1 + # Line-buffered on purpose: the cloud sign-in reads these two lines out of + # a log this process writes under nohup, where stdout is block-buffered. + print(f"Open {begun['verification_uri_complete']} in a browser on any device", flush=True) + print(f"code: {begun['user_code']}", flush=True) + # Worded so the only "code: ..." in this output is the line above: the + # cloud sign-in's parser takes the first match. + print(f"Waiting for approval; this prompt expires at {begun['expires_at']} ...", flush=True) + while True: + await asyncio.sleep(POLL_INTERVAL_SECS) + try: + result = await service.poll_device(begun["login_id"]) + except (aiohttp.ClientError, asyncio.TimeoutError) as exc: + # The service absorbs a few dropped polls itself; past its budget + # the outage is real, and the user gets a line, not a traceback. + print( + f"❌ lost contact with the sign-in service while waiting for approval: " + f"{exc}; nothing was stored. Run the command again.", + file=sys.stderr, + ) + return 1 + except (UnknownLoginError, SignedOutDuringLoginError): + print( + "❌ this sign-in was cancelled by a sign-out before it was approved; " + "nothing was stored.", + file=sys.stderr, + ) + return 1 + status = result.get("status") + if status == "pending": + continue + if status == "authorized": + print( + f"Signed in to Kiro Crew as {result.get('provider') or provider}.", flush=True + ) + if result.get("code") == "previous_identity_not_removed": + print( + "⚠ an earlier identity could not be removed from the vault; " + "run `kirocrew kas logout` and sign in again if the wrong account answers", + file=sys.stderr, + ) + return 0 + if status == "expired": + print( + "❌ the code expired before it was approved; run the command again.", + file=sys.stderr, + ) + return 1 + code = result.get("code") or "error" + print(f"❌ sign-in failed ({code}); nothing was stored.", file=sys.stderr) + return 1 + finally: + await service.close() + + +def kas_login_cmd(args: argparse.Namespace) -> int | None: + """Dispatch the sign-in verbs; ``None`` when *args* names none of them.""" + action = getattr(args, "kas_action", None) + if action == "login": + return _login(args) + if action == "whoami": + return _whoami(args) + if action == "logout": + return _logout(args) + if action == "route": + return _route(args) + return None diff --git a/src/kiro_crew/cli_parser_admin.py b/src/kiro_crew/cli_parser_admin.py index ded359f924f..f16d63486d1 100644 --- a/src/kiro_crew/cli_parser_admin.py +++ b/src/kiro_crew/cli_parser_admin.py @@ -653,7 +653,7 @@ def _tunnel_opts(p: "argparse.ArgumentParser") -> None: ) _tunnel_opts(_c_tunnel) _c_login = cloud_sub.add_parser( - "login", help="Sign kiro-cli in on the instance (fixes 'not logged in' chat errors)" + "login", help="Sign Kiro in on the instance (fixes 'not logged in' chat errors)" ) _cloud_common(_c_login) _c_login.add_argument( @@ -661,7 +661,7 @@ def _tunnel_opts(p: "argparse.ArgumentParser") -> None: ) _cloud_identity_opts(_c_login) _c_logout = cloud_sub.add_parser( - "logout", help="Sign kiro-cli out on the instance (to switch Kiro account)" + "logout", help="Sign Kiro out on the instance (to switch Kiro account)" ) _cloud_common(_c_logout) _c_stop = cloud_sub.add_parser("stop", help="Stop the instance (pause billing)") diff --git a/src/kiro_crew/cli_parser_gateway.py b/src/kiro_crew/cli_parser_gateway.py index b9d0e8a2546..6a23661a3c1 100644 --- a/src/kiro_crew/cli_parser_gateway.py +++ b/src/kiro_crew/cli_parser_gateway.py @@ -257,9 +257,14 @@ def register_kas_parser(sub: _Commands) -> None: kirocrew kas install --from ./bundle # offline: manifest.json + artifacts in a directory kirocrew kas status kirocrew kas remove + kirocrew kas login # sign Kiro Crew's vault in with a device code (Builder ID) + kirocrew kas login --provider idc --start-url https://my-org.awsapps.com/start --region us-east-1 + kirocrew kas whoami --format json # who the vault is signed in as; exit 1 when signed out + kirocrew kas logout + kirocrew kas route # kas-bundle | kiro-cli: which program owns the sign-in here Then: kirocrew config set agent.kas_bundle true && kirocrew restart -The engine signs in through Kiro Crew's own login (dashboard → Sign in). +The engine signs in through Kiro Crew's own login (dashboard → Sign in, or `kas login`). """, formatter_class=argparse.RawDescriptionHelpFormatter, ) diff --git a/src/kiro_crew/cloud/login.py b/src/kiro_crew/cloud/login.py index df481b01d5c..a86a2308edc 100644 --- a/src/kiro_crew/cloud/login.py +++ b/src/kiro_crew/cloud/login.py @@ -1,12 +1,22 @@ -"""Sign the ``kiro-cli`` backend in on the EC2 instance, over SSM. +"""Sign the Kiro backend in on the EC2 instance, over SSM. Backend auth can't be baked into user-data (it needs a human to approve in a browser and, possibly, to buy a Kiro subscription). We drive it after the box is -up: run ``kiro-cli login`` on the instance over SSM, scrape the device-code URL -+ code, and open that URL in the user's *local* browser. KiroCrew stores **no** -Kiro credentials — they live in kiro-cli's own store on the instance. - -Two remote-login shapes: +up: run the box's sign-in program on the instance over SSM, scrape the +device-code URL + code, and open that URL in the user's *local* browser. The +local machine stores **no** Kiro credentials — they live on the instance, in +kiro-cli's own store or in Crew's own vault. + +Which program that is, the BOX decides, inside every remote script +(:func:`kiro_crew.cloud.login_vault.route_prelude`): on the KAS bundle route +(``agent.acp_backend == "kas"`` with ``agent.kas_bundle`` on) kiro-cli is not +installed and the scripts drive ``kirocrew kas login|whoami|logout`` against +Crew's vault; on every other route they drive ``kiro-cli login|whoami|logout`` +exactly as they always have. The local Python reads one set of outputs and +sentinels whichever program produced them. + +Two remote-login shapes on the kiro-cli route (the vault route has only the +first, since its ``login`` verb always prints a device code): - **Device code.** Per ``docs/reference/kiro-cli/authentication.md`` the device flow covers Builder ID, IAM Identity Center, Google and GitHub: kiro-cli @@ -30,6 +40,7 @@ from dataclasses import dataclass, field from typing import Optional +from kiro_crew.cloud import login_vault as vault from kiro_crew.cloud import ssm from kiro_crew.cloud.aws import AWSError from kiro_crew.cloud.login_target import ( @@ -143,6 +154,14 @@ _CALLBACK_LOGIN_CAPTURE_SLEEP = 1 _TOKEN_PRESENT_SENTINEL = "__KIRO_AUTH_TOKEN_PRESENT__" _NOAUTH_SENTINEL = "__NOAUTH__" +# Reported when a vault-route box produced no device-code prompt. There is no +# kiro-cli callback flow to degrade into on that route, so the remedy names the +# box's own sign-in verb and its log. +_VAULT_NO_DEVICE_CODE_ERROR = ( + "the instance signs in through Kiro Crew's own vault (KAS bundle route) and its " + "`kirocrew kas login` produced no device-code prompt; retry, or run " + "`kirocrew cloud connect` and `kirocrew kas login` on the instance to see its error." +) # Resolve the kiro-cli binary to an absolute path, PATH-independent. Under SSM's # `echo | base64 -d | sudo -u -i bash` wrapper (script on stdin, @@ -157,6 +176,36 @@ ) +_BOX_REFUSAL_SENTINELS = (vault.ROUTE_UNKNOWN_SENTINEL, vault.NO_SIGNIN_PROGRAM_SENTINEL) +_BOX_REFUSAL_FALLBACK = ( + "the instance refused before touching either sign-in program (its route could not be " + "read, or it has no kiro-cli and is not on the KAS bundle route)." +) + + +def _box_refused(stdout: str) -> bool: + """Did the remote script stop before running either sign-in program?""" + return any(s in stdout for s in _BOX_REFUSAL_SENTINELS) + + +def _box_refusal_error(stdout: str) -> str: + """The box's own explanation, from the lines after its refusal sentinel. + + The remote script already wrote the plain-language cause and remedy (see + :func:`login_vault.require_sign_in_program`); this returns those lines as the + prompt's error so the CLI shows the box's words rather than a second guess. + """ + lines = stdout.splitlines() + try: + start = next( + i for i, ln in enumerate(lines) if any(s in ln for s in _BOX_REFUSAL_SENTINELS) + ) + except StopIteration: + return _BOX_REFUSAL_FALLBACK + text = "\n".join(ln for ln in lines[start + 1 :] if ln.strip()) + return text or _BOX_REFUSAL_FALLBACK + + def _login_dir_guard() -> str: """Snippet that establishes the private per-user login directory. @@ -293,6 +342,10 @@ def _auth_probe(instance_id: str, profile: str = "", region: str = "") -> Option total_wait=60, ) out = (res.stdout or "").strip() + if _box_refused(out): + # The box could not decide its route, or has no kiro-cli and no vault + # route: nothing was asked, so the state is unknown -- not "signed out". + return None if _TOKEN_PRESENT_SENTINEL in out: return True if _NOAUTH_SENTINEL in out: @@ -361,6 +414,8 @@ def remote_identity( out = (res.stdout or "").strip() if not res.ok and not out: return None + if _box_refused(out): + return None if _NOAUTH_SENTINEL in out: return {} return parse_whoami_output(out) @@ -409,6 +464,11 @@ def logout(instance_id: str, profile: str = "", region: str = "") -> bool: # that the kills landed — hence the status check, not `res.ok`.) if res.status != "Success": return False + if vault.VAULT_LOGOUT_FAILED_SENTINEL in (res.stdout or ""): + # The vault verb exited non-zero: its sign-out did not complete (the slots + # may be gone, but the fence a pending login checks was not written), and + # the empty-vault probe below would read as a clean sign-out anyway. + return False return _auth_probe(instance_id, profile, region) is False @@ -481,6 +541,11 @@ def start_device_login( total_wait=90, ) prompt = parse_login_output(res.stdout or res.stderr or "", ignore_url=resolved.start_url) + if _box_refused(res.stdout or ""): + # The box refused before touching either program and printed why and the + # remedy. Hand that text on as the error -- there is no flow to fall back to. + prompt.error = _box_refusal_error(res.stdout or "") + return prompt if prompt.already_logged_in: # kiro-cli ignores a login over a LIVE session and prints "already # logged in" whatever that session's identity is. The caller asked for @@ -494,6 +559,15 @@ def start_device_login( prompt.browser_opened = _open_browser(prompt.url) return prompt + if vault.VAULT_ROUTE_SENTINEL in (res.stdout or ""): + # The box took the vault branch: its `kirocrew kas login` prompts for + # nothing and prints a device code for every target, so no prompt means + # the sign-in itself failed to start, and the box has no kiro-cli whose + # social callback flow could stand in. Report what the log says. + if not prompt.error: + prompt.error = _VAULT_NO_DEVICE_CODE_ERROR + return prompt + if resolved.is_identity_center: # Pinned: stay on this identity. Report, do not degrade. if _DRIVER_SETUP_FAILED_SENTINEL in (res.stdout or ""): @@ -580,8 +654,10 @@ def cancel_device_login(instance_id: str, profile: str = "", region: str = "") - cancelled it. Deliberately not :func:`logout` — the box may hold an older, valid session that the cancelled attempt was never meant to drop. - **Blast radius: every ``kiro-cli login`` under the same uid on that instance.** - The kill is `pkill -u "$(id -u)" -f "kiro-cli login"` — a command-line match, + **Blast radius: every ``kiro-cli login`` and every ``kirocrew kas login`` under + the same uid on that instance.** + The kill is `pkill -u "$(id -u)" -f "kiro-cli login"` (and the same for the + vault sign-in's command line) — a command-line match, not a PID this function tracked, because the PID file sits at a predictable path and honouring it would let whoever can write there pick what gets killed. So an operator who is running their own ``kiro-cli login`` in a terminal on @@ -664,6 +740,7 @@ def _cancel_login_command() -> str: killed=0 if command -v pkill >/dev/null 2>&1; then pkill -u "$(id -u)" -f "{_LOGIN_PROCESS_PATTERN}" 2>/dev/null || true + pkill -u "$(id -u)" -f "{vault.VAULT_LOGIN_PROCESS_PATTERN}" 2>/dev/null || true killed=1 fi {_login_dir_guard()} @@ -678,7 +755,10 @@ def _cancel_login_command() -> str: # Re-probe, and report UNCONFIRMED rather than a clean stop. if command -v pgrep >/dev/null 2>&1; then for _ in 1 2 3 4 5; do - pgrep -u "$(id -u)" -f "{_LOGIN_PROCESS_PATTERN}" >/dev/null 2>&1 || exit 0 + if ! pgrep -u "$(id -u)" -f "{_LOGIN_PROCESS_PATTERN}" >/dev/null 2>&1 \\ + && ! pgrep -u "$(id -u)" -f "{vault.VAULT_LOGIN_PROCESS_PATTERN}" >/dev/null 2>&1; then + exit 0 + fi sleep 1 done echo "{_CANCEL_UNCONFIRMED_SENTINEL}" @@ -702,7 +782,13 @@ def _login_check_command() -> str: return f""" set +e {_KIRO_BIN_RESOLVE} -out="$("$KIRO" whoami < /dev/null 2>&1)" +{vault.route_prelude()} +{vault.require_sign_in_program()} +if {vault.is_vault_route_test()}; then + out="$({vault.whoami_line(as_json=False)})" +else + out="$("$KIRO" whoami < /dev/null 2>&1)" +fi rc=$? echo "$out" if [ "$rc" -eq 0 ]; then @@ -725,7 +811,13 @@ def _whoami_json_command() -> str: return f""" set +e {_KIRO_BIN_RESOLVE} -out="$("$KIRO" whoami --format json < /dev/null 2>&1)" +{vault.route_prelude()} +{vault.require_sign_in_program()} +if {vault.is_vault_route_test()}; then + out="$({vault.whoami_line(as_json=True)})" +else + out="$("$KIRO" whoami --format json < /dev/null 2>&1)" +fi rc=$? if [ "$rc" -ne 0 ]; then echo "{_NOAUTH_SENTINEL}" @@ -751,17 +843,38 @@ def _logout_command() -> str: The log/PID/FIFO hold the previous device-code URL + code, so they are removed too — a stale prompt must never be shown as if it were a fresh one. + + On the vault route the box runs :func:`login_vault.logout_lines` instead: + stop a polling ``kirocrew kas login``, then ``kirocrew kas logout``. No engine + process is killed there -- the gateway's pre-turn identity-change check + retires them on the next turn once the vault's fingerprint has changed. That + verb's exit status IS meaningful (non-zero = the sign-out's cross-process + fence was not written, so a pending dashboard login could still land), so + unlike ``kiro-cli logout``'s it is propagated: the script echoes + :data:`login_vault.VAULT_LOGOUT_FAILED_SENTINEL` and exits 1, and + :func:`logout` reports failure without probing. """ return f""" set +e {_KIRO_BIN_RESOLVE} +{vault.route_prelude()} +{vault.require_sign_in_program()} {_login_dir_guard()} +KC_LOGOUT_RC=0 +if {vault.is_vault_route_test()}; then +{vault.logout_lines()} +else if command -v pkill >/dev/null 2>&1; then pkill -u "$(id -u)" -f "{_LOGIN_PROCESS_PATTERN}" 2>/dev/null || true pkill -u "$(id -u)" -f "kiro-cli acp" 2>/dev/null || true fi "$KIRO" logout < /dev/null 2>&1 +fi rm -f "{_LOGIN_LOG_PATH}" "{_LOGIN_PID_PATH}" "{_LOGIN_FIFO_PATH}" +if [ "$KC_LOGOUT_RC" -ne 0 ]; then + echo "{vault.VAULT_LOGOUT_FAILED_SENTINEL}" + exit 1 +fi exit 0 """.strip() @@ -773,15 +886,24 @@ def _device_login_command( license_: str = "", idp_region: str = "", ) -> str: - """Build the remote command that starts login and captures its prompt.""" + """Build the remote command that starts login and captures its prompt. + + The box picks the sign-in program (``$KC_ROUTE``, see + :func:`login_vault.route_prelude`): ``kirocrew kas login`` against Crew's vault + on the KAS bundle route, ``kiro-cli login --use-device-flow`` everywhere else. + Both write the verification URL + code to the same private log, which the + shared capture loop below reads. + """ replace = "" if replace_existing: - # The same constant the cancel uses: a box may be running the social-login - # shape (`kiro-cli login`, no device flag), and a replacement that misses it - # leaves that poller alive beside the new one. + # The same constants the cancel uses: a box may be running the social-login + # shape (`kiro-cli login`, no device flag) or a vault sign-in (`kirocrew kas + # login`), and a replacement that misses either leaves that poller alive + # beside the new one. replace = f""" if command -v pkill >/dev/null 2>&1; then pkill -u "$(id -u)" -f "{_LOGIN_PROCESS_PATTERN}" 2>/dev/null || true + pkill -u "$(id -u)" -f "{vault.VAULT_LOGIN_PROCESS_PATTERN}" 2>/dev/null || true fi """.strip() # Build optional kiro-cli flags for enterprise / IAM Identity Center login. @@ -840,9 +962,18 @@ def _device_login_command( nohup "$KIRO" login --use-device-flow{extra_flags} >"{_LOGIN_LOG_PATH}" 2>&1 "{_LOGIN_PID_PATH}" for _ in $(seq 1 {_DEVICE_LOGIN_CAPTURE_ATTEMPTS}); do if [ -s "{_LOGIN_LOG_PATH}" ] && grep -Eiq "https://|verification code|user_code=|already .*logged in|already .*signed in" "{_LOGIN_LOG_PATH}"; then @@ -882,6 +1017,13 @@ def _start_callback_login( total_wait=90, ) prompt = parse_login_output(res.stdout or res.stderr or "") + if _box_refused(res.stdout or ""): + prompt.error = _box_refusal_error(res.stdout or "") + return prompt + if vault.VAULT_NO_CALLBACK_SENTINEL in (res.stdout or ""): + # The box signs in through Crew's vault; the callback flow is kiro-cli's. + prompt.error = _VAULT_NO_DEVICE_CODE_ERROR + return prompt if prompt.already_logged_in: return prompt if not prompt.ports: @@ -955,6 +1097,12 @@ def _callback_login_command() -> str: return f""" set +e {_KIRO_BIN_RESOLVE} +{vault.route_prelude()} +{vault.require_sign_in_program()} +if {vault.is_vault_route_test()}; then + echo "{vault.VAULT_NO_CALLBACK_SENTINEL}" + exit 1 +fi {_login_dir_guard()} if command -v pkill >/dev/null 2>&1; then pkill -u "$(id -u)" -f "kiro-cli login" 2>/dev/null || true diff --git a/src/kiro_crew/cloud/login_target.py b/src/kiro_crew/cloud/login_target.py index 87f585d654f..67a552ae521 100644 --- a/src/kiro_crew/cloud/login_target.py +++ b/src/kiro_crew/cloud/login_target.py @@ -295,6 +295,47 @@ def target_from_whoami(identity: Mapping[str, Any], *, region: str = "") -> Kiro return None +def _local_vault_owns_sign_in() -> bool: + """Whether THIS machine runs the KAS bundle route (:func:`auth.whoami.vault_owns_sign_in`).""" + from kiro_crew.auth.whoami import vault_owns_sign_in + + return vault_owns_sign_in() + + +def local_vault_identity() -> dict[str, str] | None: + """The identity Crew's own vault on THIS machine is signed in as. + + Same three answers as the kiro-cli reading: the parsed identity when the vault + holds a usable sign-in, ``{}`` when it is proven to hold none (nothing to + inherit), and ``None`` when the vault could not be read, or holds an entry that + could not be read -- an identity of unknown kind, never Builder ID. The document is the one + ``kirocrew kas whoami --format json`` prints + (:func:`kiro_crew.auth.whoami.whoami_document`) and it is read through + :func:`parse_whoami_output`, so the local and the remote reading share one + vocabulary and one parser. ``kiro_crew.auth`` is imported here, at call time: + it brings the ``cryptography`` wheel, which is not a price every ``kirocrew + cloud`` verb should pay. + """ + from kiro_crew.auth.bridge import default_token_store + from kiro_crew.auth.whoami import whoami_document + + try: + store = default_token_store() + token = store.resolve() + if token is None: + # `resolve` reports a vault it cannot read or decrypt as empty. Only a + # vault PROVEN to hold no entry is "nothing to inherit"; an entry that + # would not read is an unknown identity, which the caller must not + # resolve to Builder ID (the same rule as a kiro-cli whoami that failed). + return {} if store.holds_no_identity() else None + except Exception: # noqa: BLE001 - discovery is best-effort + logger.debug("local vault could not be read", exc_info=True) + return None + if not token.is_usable(): + return {} + return parse_whoami_output(json.dumps(whoami_document(token))) + + def discover_local_identity( kiro_bin: str | None = None, *, timeout: float = 15.0 ) -> dict[str, str] | None: @@ -312,8 +353,16 @@ def discover_local_identity( Used by the CLI launch paths to inherit the launching machine's identity. The dashboard has its own async, sandbox-tiered fetch; this one is the plain synchronous form for ``kirocrew setup`` / ``kirocrew cloud launch``. + + On the KAS bundle route (``agent.acp_backend == "kas"`` with + ``agent.kas_bundle`` on) this machine's Kiro sign-in is Crew's own vault, not + kiro-cli's store -- the route installs no kiro-cli -- so the answer comes from + :func:`local_vault_identity`, rendered in the same ``whoami`` document shape + and read through the same parser, with the same three answers. """ if kiro_bin is None: + if _local_vault_owns_sign_in(): + return local_vault_identity() try: kiro_bin = resolve_kiro_cli() except Exception: # noqa: BLE001 - discovery is best-effort diff --git a/src/kiro_crew/cloud/login_vault.py b/src/kiro_crew/cloud/login_vault.py new file mode 100644 index 00000000000..e4f785930e7 --- /dev/null +++ b/src/kiro_crew/cloud/login_vault.py @@ -0,0 +1,219 @@ +"""The on-box half of the cloud sign-in when Crew's own vault owns the credential. + +:mod:`kiro_crew.cloud.login` signs the remote crew in by running shell scripts on +the instance over SSM. Which PROGRAM those scripts should drive depends on the +instance's own configuration: on the KAS bundle route (``agent.acp_backend == +"kas"`` with ``agent.kas_bundle`` on) Crew spawns its own engine, kiro-cli is not +installed, and the engine's only credential is Crew's vault; everywhere else the +remote ``kiro-cli`` owns the sign-in exactly as it always has. + +The local ``kirocrew cloud`` command cannot read that configuration -- it lives in +the instance's ``config.json`` -- so the decision is made ON THE BOX, by the box's +own ``kirocrew kas route``, inside every script that needs it. This module holds +that prelude and the vault-route lines the scripts splice in; the kiro-cli lines +they keep are :mod:`kiro_crew.cloud.login`'s and are not repeated here. The two +halves share the capture loop, the private login directory and the sentinels, so +the local Python reads one set of outputs whichever program produced them. + +Every vault-route verb is a ``kirocrew kas`` subcommand (:mod:`kiro_crew.cli_kas_login`) +that speaks ``kiro-cli``'s vocabulary on purpose: ``login`` prints the same +verification URL + ``code:`` lines the capture loop greps for, ``whoami --format +json`` prints the document :func:`kiro_crew.cloud.login_target.parse_whoami_output` +already parses, and each exits 0 / non-zero where ``kiro-cli`` does. + +Nothing here reads, prints or carries a credential; the device code and the token +stay between the box's ``kirocrew`` process and the issuer. +""" + +from __future__ import annotations + +import shlex + +# The one spelling of the route answer and the one exit code for "unknown", owned by +# the verb that prints them; the scripts compare against the same objects, so there +# is nothing to drift. +from kiro_crew.cli_kas_login import ROUTE_KAS_BUNDLE, ROUTE_UNKNOWN_EXIT + +__all__ = ["ROUTE_KAS_BUNDLE", "ROUTE_UNKNOWN_EXIT"] + +#: Echoed by the device-login script when it took the vault branch, so the local +#: :func:`kiro_crew.cloud.login.start_device_login` knows the box has no kiro-cli +#: to fall back to (the social callback flow is kiro-cli's) and reports the vault +#: sign-in's own outcome instead. +VAULT_ROUTE_SENTINEL = "__KIROCREW_VAULT_ROUTE__" + +#: Echoed (and the command ended) by the callback-login script on a vault-route box: +#: there is no ``kiro-cli login`` callback flow to continue there. +VAULT_NO_CALLBACK_SENTINEL = "__KIROCREW_VAULT_NO_CALLBACK__" + +#: Echoed (and the script ended non-zero) when the box's ``kirocrew kas route`` ran +#: and could NOT decide (exit :data:`ROUTE_UNKNOWN_EXIT`: an unreadable config, or a +#: crash). Neither program is touched then, kiro-cli installed or not: a box whose +#: config just broke may well be on the bundle route, and running ``kiro-cli logout`` +#: there would report "signed out" while the vault credential stays usable. The +#: probe's own output follows the sentinel. +ROUTE_UNKNOWN_SENTINEL = "__KIROCREW_ROUTE_UNKNOWN__" + +#: Echoed (and the script ended non-zero) when the box has NO sign-in program: its +#: ``kirocrew kas route`` did not answer ``kas-bundle`` and it has no ``kiro-cli`` +#: either. Without this the kiro-cli branch would run on a box that has none and the +#: operator would read ``kiro-cli: command not found`` where the real cause is a +#: bundle-route box whose route probe failed (an unreadable config, a ``kirocrew`` +#: too old for the verb). The script prints the probe's own output after the +#: sentinel so the local side can show it. +NO_SIGNIN_PROGRAM_SENTINEL = "__KIROCREW_NO_SIGNIN_PROGRAM__" + +#: Echoed (and the logout script ended non-zero) when the box's ``kirocrew kas +#: logout`` exited non-zero: the slots may be gone, but the sign-out's cross-process +#: fence (the sign-out generation) was not written, so a login pending in the +#: box's gateway could still land. The local side must not report "signed out". +VAULT_LOGOUT_FAILED_SENTINEL = "__KIROCREW_VAULT_LOGOUT_FAILED__" + +#: The command-line fragment that identifies a vault sign-in this code started, for +#: the same uid-scoped ``pkill -f`` the kiro-cli pattern is used with. ``$KIROCREW`` +#: resolves to a path whose basename is ``kirocrew`` (see :data:`KIROCREW_BIN_RESOLVE`), +#: so the launched command line contains this fragment whatever flags follow. It +#: does not match ``kirocrew kas logout``, ``kirocrew kas whoami`` or the gateway. +VAULT_LOGIN_PROCESS_PATTERN = "kirocrew kas login" + +#: Resolve the box's ``kirocrew`` to an absolute path, PATH-independent, for the same +#: reason ``login._KIRO_BIN_RESOLVE`` does it for kiro-cli: under SSM's login-shell +#: wrapper PATH is sometimes unreliable. The EC2 template installs it under +#: ``$HOME/.local/bin`` (``install.sh``) and runs the gateway from there. +# Shell text that runs on a Linux instance; the candidate paths are the box's, not +# this process's, which is why no pathlib call shapes them. +KIROCREW_BIN_RESOLVE = ( + 'KIROCREW="$(command -v kirocrew 2>/dev/null || true)"; ' + '[ -n "$KIROCREW" ] || for c in "$HOME/.local/bin/kirocrew" /usr/local/bin/kirocrew' + ' /usr/bin/kirocrew; do [ -x "$c" ] && KIROCREW="$c" && break; done; ' + '[ -n "$KIROCREW" ] || KIROCREW=kirocrew' +) + + +def route_prelude() -> str: + """Shell that sets ``$KIROCREW``, ``$KC_ROUTE`` and ``$KC_ROUTE_OUT`` on the box. + + ``$KC_ROUTE`` is what the box's ``kirocrew kas route`` printed on STDOUT when it + exited 0, or empty otherwise. Three exit codes mean three things, and the + scripts keep them apart: ``0`` is an answer; :data:`ROUTE_UNKNOWN_EXIT` (``1``) is + the verb saying it could not decide (``$KC_ROUTE_UNKNOWN=1``, and + :func:`require_sign_in_program` then refuses before either program runs); + anything else -- argparse's ``2`` from a ``kirocrew`` too old to have the verb, + the shell's ``127`` for no ``kirocrew`` at all -- is "no route answer", and the + script takes its kiro-cli branch, the behaviour every box had before the verb + existed. Only the exact spelling :data:`ROUTE_KAS_BUNDLE` selects the vault + branch, so the vault is never chosen by an absence. Stderr is captured APART (``$KC_ROUTE_ERR``, through a + private temp file) and never joins the compared value: the config loader warns + there about a loose ``config.local.json`` mode while the verb still answers + ``kas-bundle`` on stdout, and a merged stream would fail the exact comparison + on a working bundle-route box. ``$KC_ROUTE_OUT`` and ``$KC_ROUTE_ERR`` are both + shown by :func:`require_sign_in_program` when the probe did not answer. + """ + return ( + f"{KIROCREW_BIN_RESOLVE}\n" + 'KC_ROUTE_ERR_FILE="$(mktemp 2>/dev/null || echo /dev/null)"\n' + 'KC_ROUTE_OUT="$("$KIROCREW" kas route < /dev/null 2>"$KC_ROUTE_ERR_FILE")"; KC_ROUTE_RC=$?\n' + 'KC_ROUTE_ERR="$(cat "$KC_ROUTE_ERR_FILE" 2>/dev/null)"; ' + '[ "$KC_ROUTE_ERR_FILE" = /dev/null ] || rm -f "$KC_ROUTE_ERR_FILE"\n' + 'KC_ROUTE=""; KC_ROUTE_UNKNOWN=0\n' + 'if [ "$KC_ROUTE_RC" -eq 0 ]; then KC_ROUTE="$KC_ROUTE_OUT"; ' + f'elif [ "$KC_ROUTE_RC" -eq {ROUTE_UNKNOWN_EXIT} ]; then KC_ROUTE_UNKNOWN=1; fi' + ) + + +def require_sign_in_program() -> str: + """Shell that refuses, readably, when the box has neither sign-in program. + + Spliced in right after :func:`route_prelude` and before the ``$KIRO`` / + ``$KIROCREW`` branch of every script. Two refusals: + + - the probe RAN and could not decide (``$KC_ROUTE_UNKNOWN=1``): neither program + is touched, whether or not kiro-cli is installed -- a box whose config broke + may be on the bundle route, and signing kiro-cli's store out there would + report a sign-out the vault never saw; + - the route is not ``kas-bundle`` AND ``kiro-cli`` did not resolve: the kiro-cli + branch would only print ``command not found``. + + Each names the real situation instead -- a box with no kiro-cli whose + route probe did not say ``kas-bundle`` -- echoes the probe's own output (the + ``kas route`` error text, or the shell's "not found" for a ``kirocrew`` the box + lacks), and names the remedy. The sentinel comes first so the local side can + classify the answer without parsing the prose. ``$KC_ROUTE_OUT`` and + ``$KC_ROUTE_ERR`` are both printed here (blank lines dropped), since a failed + probe's explanation is on stderr while a shell's "not found" may land on either. + """ + return f""" +if [ "$KC_ROUTE_UNKNOWN" = 1 ]; then + echo "{ROUTE_UNKNOWN_SENTINEL}" + echo "This instance's 'kirocrew kas route' could not tell which program owns the Kiro sign-in (exit $KC_ROUTE_RC), so nothing was changed on it:" + printf '%s\n%s\n' "$KC_ROUTE_OUT" "$KC_ROUTE_ERR" | sed '/^$/d; s/^/ /' + echo "Fix the instance's Kiro Crew config.json (run 'kirocrew kas route' on the instance to see the error), then retry." + exit 1 +fi +if ! {is_vault_route_test()} && [ ! -x "$KIRO" ] && ! command -v "$KIRO" >/dev/null 2>&1; then + echo "{NO_SIGNIN_PROGRAM_SENTINEL}" + echo "This instance has no kiro-cli, and its 'kirocrew kas route' did not answer {ROUTE_KAS_BUNDLE} (exit $KC_ROUTE_RC):" + printf '%s\n%s\n' "$KC_ROUTE_OUT" "$KC_ROUTE_ERR" | sed '/^$/d; s/^/ /' + echo "If this instance runs the KAS bundle route, fix its Kiro Crew config.json (run 'kirocrew kas route' on the instance to see the error); otherwise install kiro-cli on it." + exit 1 +fi +""".strip() + + +def is_vault_route_test() -> str: + """The ``[ ... ]`` test the scripts branch on.""" + return f'[ "$KC_ROUTE" = "{ROUTE_KAS_BUNDLE}" ]' + + +def whoami_line(*, as_json: bool) -> str: + """The vault-route replacement for ``"$KIRO" whoami [--format json]``.""" + fmt = " --format json" if as_json else "" + return f'"$KIROCREW" kas whoami{fmt} < /dev/null 2>&1' + + +def logout_lines() -> str: + """The vault-route replacement for the kiro-cli kill-then-logout block. + + Stops a vault sign-in still polling (it would otherwise re-fill the vault the + moment it was emptied), then removes every stored identity. No engine process + is killed here: the vault is part of the gateway's identity fingerprint, and + the gateway's pre-turn identity-change check retires the running agent + processes on the next turn (``dashboard/handlers/kas_login._retire_runtimes_after_sign_out`` + documents the same sweep), so no process keeps serving the signed-out account + past that turn. + + The verb's exit status is kept in ``$KC_LOGOUT_RC`` for the caller's script + to act on: a non-zero one means the sign-out did not complete (its fence was + not written), and ``kiro-cli logout``'s "non-zero when there was nothing to + drop" convention does not apply to it, so the script must not fold it into + the unconditional ``exit 0`` the kiro-cli branch ends with. + """ + return f""" +if command -v pkill >/dev/null 2>&1; then + pkill -u "$(id -u)" -f "{VAULT_LOGIN_PROCESS_PATTERN}" 2>/dev/null || true +fi +"$KIROCREW" kas logout < /dev/null 2>&1 +KC_LOGOUT_RC=$? +""".strip() + + +def login_launch(*, log_path: str, identity_provider: str = "", idp_region: str = "") -> str: + """The vault-route background launch of ``kirocrew kas login``, writing to *log_path*. + + One shape for every target: Builder ID is the flagless default, an Identity + Center target adds ``--provider idc`` with its start URL and region. The verb + takes its answers from flags, prompts for nothing and prints the verification + URL + code itself, so no pseudo-terminal driver is needed -- the kiro-cli + branch's IdC pty dance has no counterpart here. Each value is shell-quoted + because it flows from a CLI argument into a script run on the instance. The + kiro-cli ``--license`` has no vault equivalent: the identity decides the tier. + """ + flags = "" + if identity_provider: + flags += f" --provider idc --start-url {shlex.quote(identity_provider)}" + if idp_region: + flags += f" --region {shlex.quote(idp_region)}" + return f""" +echo "{VAULT_ROUTE_SENTINEL}" +nohup "$KIROCREW" kas login{flags} >"{log_path}" 2>&1 web.Response: {"error": "Kiro auth service is unreachable.", "code": "auth_service_unreachable"}, status=502, ) + if result.get("code") == "signed_out_during_login": + # A sign-out landed (in this gateway or from `kirocrew kas logout` in a + # shell) while this login was pending, so its credential was not + # persisted. The same 409 the begin handlers answer for the in-flight + # case: the card ends this sign-in and offers Start over, and never + # restarts the provider on another transport as it would for a plain + # terminal poll answer. + return web.json_response( + { + "error": "Signed out while the sign-in was pending.", + "code": "signed_out_during_login", + }, + status=409, + ) # Only the authorized transition persists a credential — that is the state # change worth an audit line; a pending tick is just a read. if result.get("status") == "authorized": diff --git a/test/spawn_audit_helpers.py b/test/spawn_audit_helpers.py index 4340c2f8102..28150b7762a 100644 --- a/test/spawn_audit_helpers.py +++ b/test/spawn_audit_helpers.py @@ -471,6 +471,15 @@ # ``_cleanup_app_crons_from_scheduler`` above. No child process is # created; the sole input is the operator-typed app name. "cli_commands_app.py::_register_app_crons_to_scheduler", + # NOT a subprocess spawn: the AST heuristic matches ``asyncio.run`` (attr + # ``run`` on base ``asyncio``) driving ``KasLoginService``'s device-code + # sign-in and sign-out coroutines from the loop-less ``kirocrew kas + # login`` / ``kas logout`` verbs. No child process is created: the + # inputs are operator-typed flags (provider, start URL, region) that go + # into HTTPS requests to the issuer and into Crew's own vault, never + # into an argv. Same classification as ``cli_doctor.py::_doctor``. + "cli_kas_login.py::_login", + "cli_kas_login.py::_logout", "cli_doctor.py::_doctor", # NOT a subprocess spawn: the AST heuristic matches ``asyncio.run`` (attr # ``run`` on base ``asyncio``) driving the async Discord diff --git a/test/test_cli_kas_login.py b/test/test_cli_kas_login.py new file mode 100644 index 00000000000..b08d54ebe69 --- /dev/null +++ b/test/test_cli_kas_login.py @@ -0,0 +1,331 @@ +"""``kirocrew kas login | whoami | logout | route``: the vault sign-in from a shell. + +These verbs are what the cloud sign-in runs on a bundle-route box over SSM, so their +OUTPUT is a contract: the device prompt must parse through ``parse_login_output`` +(URL + ``code:`` line), ``whoami --format json`` through ``parse_whoami_output``, +``route`` must spell the constant the remote scripts compare against, and the exit +codes must follow kiro-cli's. The login service is faked at its class seam; nothing +here reaches an issuer. +""" + +from __future__ import annotations + +import argparse +from datetime import datetime, timezone + +import pytest + +from kiro_crew import cli_kas, cli_kas_login +from kiro_crew.auth.store import KasToken, TokenStore +from kiro_crew.cloud import login_vault +from kiro_crew.cloud.login import parse_login_output +from kiro_crew.cloud.login_target import parse_whoami_output + +_FUTURE = datetime(2099, 1, 1, tzinfo=timezone.utc) + + +def _parse(*argv: str) -> argparse.Namespace: + parser = argparse.ArgumentParser(prog="kirocrew kas") + cli_kas.add_kas_parser(parser) + return parser.parse_args(list(argv)) + + +def _token(identity="builder_id", provider="BuilderId") -> KasToken: + return KasToken( + access_token="atk-secret", + expires_at=_FUTURE, + provider=provider, + identity=identity, + refresh_token="rtk-secret", + profile_arn="arn:aws:kiro:us-east-1:1:profile/p1", + ) + + +@pytest.fixture +def home_store(tmp_path, monkeypatch) -> TokenStore: + """Point the verbs' ``default_token_store`` at a vault under tmp_path.""" + from kiro_crew.auth import bridge + + store = TokenStore(tmp_path) + monkeypatch.setattr(bridge, "default_token_store", lambda: store) + return store + + +class TestParser: + def test_the_sign_in_verbs_are_reachable_under_kas(self): + args = _parse("login", "--provider", "idc", "--start-url", "https://a.b/start") + assert (args.kas_action, args.provider, args.start_url) == ( + "login", + "idc", + "https://a.b/start", + ) + assert _parse("login").provider == "builder_id" + assert _parse("whoami", "--format", "json").format == "json" + assert _parse("logout").kas_action == "logout" + assert _parse("route").kas_action == "route" + + def test_the_usage_line_names_every_verb(self, capsys): + assert cli_kas.kas_cmd(argparse.Namespace(kas_action=None)) == 2 + err = capsys.readouterr().err + for verb in ("login", "whoami", "logout", "route"): + assert verb in err + + +class TestRoute: + def test_the_spelling_the_remote_scripts_compare_against(self): + """The box prints it, the SSM script tests ``$KC_ROUTE`` against it: one constant.""" + assert cli_kas_login.ROUTE_KAS_BUNDLE == login_vault.ROUTE_KAS_BUNDLE + assert cli_kas_login.ROUTE_KAS_BUNDLE != cli_kas_login.ROUTE_KIRO_CLI + # The "unknown" exit must stay apart from argparse's 2 (verb absent) and the + # shell's 127 (kirocrew absent), which the remote scripts read as "no answer". + assert cli_kas_login.ROUTE_UNKNOWN_EXIT == login_vault.ROUTE_UNKNOWN_EXIT + assert cli_kas_login.ROUTE_UNKNOWN_EXIT not in (0, 2, 127) + # A no-action `kas` call is argparse's 2 territory, never the unknown exit. + assert cli_kas.kas_cmd(argparse.Namespace(kas_action=None)) == 2 + + @pytest.mark.parametrize( + "backend, bundle, expected", + [("kas", True, "kas-bundle"), ("kas", False, "kiro-cli"), ("kiro", True, "kiro-cli")], + ) + def test_route_prints_the_owner(self, monkeypatch, capsys, backend, bundle, expected): + from kiro_crew.config.loader import KiroCrewConfig + + class _Agent: + acp_backend = backend + kas_bundle = bundle + + class _Cfg: + agent = _Agent() + + monkeypatch.setattr(KiroCrewConfig, "load", classmethod(lambda cls: _Cfg())) + assert cli_kas.kas_cmd(_parse("route")) == 0 + assert capsys.readouterr().out.strip() == expected + + def test_an_unreadable_config_is_a_readable_error_not_kiro_cli(self, monkeypatch, capsys): + """Printed from this verb, a silent `kiro-cli` would send a bundle-route box + down a branch it cannot run; the owner is UNKNOWN and the verb says why.""" + from kiro_crew.config.loader import KiroCrewConfig + + def _boom(cls): + raise ValueError("Expecting value: line 1 column 1") + + monkeypatch.setattr(KiroCrewConfig, "load", classmethod(_boom)) + assert cli_kas.kas_cmd(_parse("route")) == cli_kas_login.ROUTE_UNKNOWN_EXIT == 1 + out = capsys.readouterr() + assert out.out == "" + assert "could not read the Kiro Crew config" in out.err + assert "Expecting value" in out.err + assert "fix the config" in out.err + + @pytest.mark.parametrize("flag", ["_base_unreadable", "_overlay_unreadable"]) + def test_a_degraded_load_is_refused_the_same_way(self, monkeypatch, capsys, flag): + """The loader does not raise on a malformed config.json: it returns DEFAULTS + with a flag set. Defaults say "kiro-cli" about a host that may be on the + bundle route, and a sign-out acting on that would empty the wrong store.""" + from kiro_crew.config.loader import KiroCrewConfig + + class _Agent: + acp_backend = "kas" + kas_bundle = True + + class _Cfg: + agent = _Agent() + _base_unreadable = False + _overlay_unreadable = False + + cfg = _Cfg() + setattr(cfg, flag, True) + monkeypatch.setattr(KiroCrewConfig, "load", classmethod(lambda cls: cfg)) + assert cli_kas.kas_cmd(_parse("route")) == 1 + out = capsys.readouterr() + assert out.out == "" + assert "unparseable" in out.err + assert ("config.json" if flag == "_base_unreadable" else "config.local.json") in out.err + + +class TestWhoami: + def test_signed_out_exits_one_like_kiro_cli(self, home_store, capsys): + assert cli_kas.kas_cmd(_parse("whoami", "--format", "json")) == 1 + out = capsys.readouterr() + assert out.out == "" + assert "Not signed in" in out.err + + def test_json_parses_through_the_cloud_whoami_parser(self, home_store, capsys): + home_store.save(_token()) + assert cli_kas.kas_cmd(_parse("whoami", "--format", "json")) == 0 + out = capsys.readouterr().out + assert parse_whoami_output(out) == {"account_type": "BuilderId"} + assert "atk-secret" not in out and "rtk-secret" not in out + + def test_text_names_the_kind_and_never_the_token(self, home_store, capsys): + home_store.save(_token("social", "Google")) + assert cli_kas.kas_cmd(_parse("whoami")) == 0 + out = capsys.readouterr().out + assert "SocialGoogle" in out + assert "atk-secret" not in out and "rtk-secret" not in out + + +class TestLogout: + def test_signed_out_is_a_clean_no_op_that_still_moves_the_generation(self, home_store, capsys): + """A dashboard login may be pending over an empty vault; this sign-out must + still refuse it when its code is approved, so the generation moves.""" + before = home_store.signout_generation() + assert cli_kas.kas_cmd(_parse("logout")) == 0 + assert "nothing to sign out" in capsys.readouterr().out + assert home_store.signout_generation() != before + + def test_logout_empties_every_slot(self, home_store, capsys): + home_store.save(_token()) + home_store.save(_token("social", "Google")) + assert cli_kas.kas_cmd(_parse("logout")) == 0 + assert home_store.resolve() is None + assert "Signed out" in capsys.readouterr().out + + +class _FakeService: + """Scripted stand-in for KasLoginService: begin answers once, poll replays a list.""" + + polls: list[dict] = [] + begun: list[dict] = [] + closed = 0 + + def __init__(self, store): + self.store = store + + async def begin_device(self, provider, *, start_url="", region="", replaces=""): + _FakeService.begun.append( + {"provider": provider, "start_url": start_url, "region": region, "replaces": replaces} + ) + return { + "login_id": "lid-1", + "user_code": "WXYZ-1234", + "verification_uri_complete": "https://device.sso.us-east-1.amazonaws.com/?user_code=WXYZ-1234", + "expires_at": "2099-01-01T00:00:00+00:00", + } + + async def poll_device(self, login_id): + assert login_id == "lid-1" + return _FakeService.polls.pop(0) + + async def close(self): + _FakeService.closed += 1 + + +@pytest.fixture +def fake_service(monkeypatch): + from kiro_crew.auth import service + + _FakeService.polls = [] + _FakeService.begun = [] + _FakeService.closed = 0 + monkeypatch.setattr(service, "KasLoginService", _FakeService) + # No real waiting between polls: the cadence is the issuer's concern, not this test's. + monkeypatch.setattr(cli_kas_login, "POLL_INTERVAL_SECS", 0) + return _FakeService + + +class TestLogin: + def test_the_prompt_parses_like_kiro_cli_output(self, home_store, fake_service, capsys): + fake_service.polls = [ + {"status": "pending"}, + {"status": "authorized", "provider": "BuilderId"}, + ] + assert cli_kas.kas_cmd(_parse("login")) == 0 + out = capsys.readouterr().out + prompt = parse_login_output(out) + assert prompt.url == "https://device.sso.us-east-1.amazonaws.com/?user_code=WXYZ-1234" + assert prompt.code == "WXYZ-1234" + assert not prompt.already_logged_in + assert "Signed in to Kiro Crew" in out + assert fake_service.closed == 1 + assert fake_service.begun == [ + {"provider": "builder_id", "start_url": "", "region": "", "replaces": ""} + ] + + def test_an_idc_login_forwards_its_start_url_and_region(self, home_store, fake_service): + fake_service.polls = [{"status": "authorized", "provider": "Enterprise"}] + args = _parse( + "login", + "--provider", + "idc", + "--start-url", + "https://acme.awsapps.com/start", + "--region", + "eu-west-1", + ) + assert cli_kas.kas_cmd(args) == 0 + assert fake_service.begun[0]["start_url"] == "https://acme.awsapps.com/start" + assert fake_service.begun[0]["region"] == "eu-west-1" + + def test_an_idc_login_without_a_start_url_is_a_usage_error(self, home_store, fake_service): + assert cli_kas.kas_cmd(_parse("login", "--provider", "idc")) == 2 + assert fake_service.begun == [] + + def test_a_live_sign_in_answers_already_signed_in(self, home_store, fake_service, capsys): + """kiro-cli ignores a login over a live session; so does this, in words the + cloud sign-in reads as ``already_logged_in`` and then verifies the identity of.""" + home_store.save(_token()) + assert cli_kas.kas_cmd(_parse("login")) == 0 + out = capsys.readouterr().out + assert parse_login_output(out).already_logged_in + assert fake_service.begun == [] + + def test_a_lapsed_slot_is_named_as_the_one_this_login_replaces(self, home_store, fake_service): + """A stale higher-priority slot would outrank the fresh sign-in at resolve time.""" + lapsed = KasToken( + access_token="old", + expires_at=datetime(2000, 1, 1, tzinfo=timezone.utc), + provider="ExternalIdp", + identity="external_idp", + refresh_token=None, + ) + home_store.save(lapsed) + fake_service.polls = [{"status": "authorized", "provider": "BuilderId"}] + assert cli_kas.kas_cmd(_parse("login")) == 0 + assert fake_service.begun[0]["replaces"] == "external_idp" + + def test_an_expired_code_is_a_failure_with_the_remedy(self, home_store, fake_service, capsys): + fake_service.polls = [{"status": "expired"}] + assert cli_kas.kas_cmd(_parse("login")) == 1 + assert "expired" in capsys.readouterr().err + + def test_a_lost_connection_while_polling_is_a_line_not_a_traceback( + self, home_store, fake_service, capsys, monkeypatch + ): + import aiohttp + + async def _drop(self, login_id): + raise aiohttp.ClientConnectionError("connection reset") + + monkeypatch.setattr(fake_service, "poll_device", _drop) + assert cli_kas.kas_cmd(_parse("login")) == 1 + err = capsys.readouterr().err + assert "lost contact with the sign-in service" in err + assert "Traceback" not in err + + def test_a_sign_out_during_the_login_is_a_line_not_a_traceback( + self, home_store, fake_service, capsys, monkeypatch + ): + from kiro_crew.auth.service import UnknownLoginError + + async def _gone(self, login_id): + raise UnknownLoginError(login_id) + + monkeypatch.setattr(fake_service, "poll_device", _gone) + assert cli_kas.kas_cmd(_parse("login")) == 1 + err = capsys.readouterr().err + assert "cancelled by a sign-out" in err + assert "Traceback" not in err + + def test_a_store_failure_is_a_failure_naming_its_code(self, home_store, fake_service, capsys): + fake_service.polls = [{"status": "error", "code": "token_store_failed"}] + assert cli_kas.kas_cmd(_parse("login")) == 1 + assert "token_store_failed" in capsys.readouterr().err + + def test_only_one_code_line_is_printed(self, home_store, fake_service, capsys): + """``parse_login_output`` takes the FIRST ``code: ...`` match, so the waiting + line must not read as a second code.""" + fake_service.polls = [{"status": "authorized", "provider": "BuilderId"}] + cli_kas.kas_cmd(_parse("login")) + out = capsys.readouterr().out + assert out.lower().count("code:") == 1 diff --git a/test/test_cloud_login_target.py b/test/test_cloud_login_target.py index 6dd29cffc5d..d15d27a6f73 100644 --- a/test/test_cloud_login_target.py +++ b/test/test_cloud_login_target.py @@ -406,7 +406,10 @@ def test_pinned_identity_login_runs_under_a_pty_driver(self): # The directory is reaped once the login process exits. assert 'rm -rf "$(dirname "$0")"' in pinned plain = login._device_login_command(replace_existing=False) - assert "python3" not in plain and "nohup stdbuf" in plain and "mktemp" not in plain + # No driver staging for Builder ID: no python3, no pty directory. (The route + # prelude's own one-line `mktemp` for the probe's stderr is not that.) + assert "python3" not in plain and "nohup stdbuf" in plain + assert "kirocrew-login-pty" not in plain and "mktemp -d" not in plain def test_driver_staging_failure_is_reported_by_name(self, monkeypatch): _ssm_script( diff --git a/test/test_cloud_login_vault.py b/test/test_cloud_login_vault.py new file mode 100644 index 00000000000..21ac2638235 --- /dev/null +++ b/test/test_cloud_login_vault.py @@ -0,0 +1,715 @@ +"""The cloud sign-in on a box whose Crew runs the KAS bundle route. + +Every remote script decides ON THE BOX which program owns the sign-in, by the +box's own ``kirocrew kas route``: the vault verbs when it prints ``kas-bundle``, +the kiro-cli verbs it always ran otherwise -- byte for byte, including when the +probe prints nothing at all (a box whose Kiro Crew predates the verb). The local +Python reads one set of sentinels whichever branch ran. Half of these tests read +the generated scripts; the other half run them under bash against stub +``kirocrew`` / ``kiro-cli`` programs, one isolated HOME per route, and never start +a background login (the device-login script is exercised only on its text). +""" + +from __future__ import annotations + +import json +import os +import shutil +import stat +import subprocess +from pathlib import Path + +import pytest + +from kiro_crew.cloud import login, login_vault, ssm +from kiro_crew.cloud.login_target import KiroLoginTarget + +_IDC = KiroLoginTarget.from_fields( + license="pro", start_url="https://example.awsapps.com/start", region="us-east-1" +) + +_SCRIPTS = { + "check": login._login_check_command(), + "whoami": login._whoami_json_command(), + "logout": login._logout_command(), + "device_default": login._device_login_command(replace_existing=True), + "device_idc": login._device_login_command(replace_existing=False, **_IDC.login_kwargs()), + "callback": login._callback_login_command(), + "resume": login._resume_login_command(), +} + +_VAULT_VERBS = { + "check": '"$KIROCREW" kas whoami < /dev/null', + "whoami": '"$KIROCREW" kas whoami --format json < /dev/null', + "logout": '"$KIROCREW" kas logout < /dev/null', + "device_default": 'nohup "$KIROCREW" kas login >', + "device_idc": ( + 'nohup "$KIROCREW" kas login --provider idc --start-url ' + "https://example.awsapps.com/start --region us-east-1 >" + ), + "resume": 'nohup "$KIROCREW" kas login >', +} + +_KIRO_CLI_VERBS = { + "check": '"$KIRO" whoami < /dev/null', + "whoami": '"$KIRO" whoami --format json < /dev/null', + "logout": '"$KIRO" logout < /dev/null', + "device_default": '"$KIRO" login --use-device-flow >', + "device_idc": '"$KIRO" login --use-device-flow --identity-provider https://example.awsapps.com/start', + "resume": '"$KIRO" login --use-device-flow >', +} + + +class TestScriptText: + @pytest.mark.parametrize("name", sorted(_SCRIPTS)) + def test_every_script_asks_the_box_for_its_route(self, name): + cmd = _SCRIPTS[name] + assert login_vault.KIROCREW_BIN_RESOLVE in cmd + assert '"$KIROCREW" kas route' in cmd + assert login_vault.is_vault_route_test() in cmd + + @pytest.mark.parametrize("name", sorted(_VAULT_VERBS)) + def test_both_programs_are_present_and_the_vault_one_is_the_if_branch(self, name): + cmd = _SCRIPTS[name] + test_at = cmd.index(login_vault.is_vault_route_test()) + vault_at = cmd.index(_VAULT_VERBS[name]) + cli_at = cmd.index(_KIRO_CLI_VERBS[name]) + assert test_at < vault_at < cli_at, name + + def test_the_route_is_chosen_only_by_the_exact_spelling(self): + """An empty or foreign answer takes the kiro-cli branch: the vault is never + selected by an absence.""" + assert login_vault.is_vault_route_test() == '[ "$KC_ROUTE" = "kas-bundle" ]' + assert "|| true" in login_vault.route_prelude() + + def test_the_vault_launch_echoes_the_route_sentinel_for_the_local_side(self): + for name in ("device_default", "device_idc", "resume"): + cmd = _SCRIPTS[name] + sentinel_at = cmd.index(f'echo "{login_vault.VAULT_ROUTE_SENTINEL}"') + launch_at = cmd.index(_VAULT_VERBS[name]) + assert sentinel_at < launch_at, name + + def test_the_vault_login_takes_its_identity_from_flags_and_needs_no_pty(self): + """kiro-cli's IdC login prompts and needs the pty driver; the vault verb does + not, so the driver staging sits in the kiro-cli branch only.""" + cmd = _SCRIPTS["device_idc"] + else_at = cmd.index("\nelse\n", cmd.index(_VAULT_VERBS["device_idc"])) + assert cmd.index("KC_DRIVER_DIR=") > else_at + + def test_the_vault_login_flags_are_shell_quoted(self): + evil = "$(touch /tmp/pwned)" + launch = login_vault.login_launch( + log_path="$KC_LOGIN_DIR/x.log", identity_provider=evil, idp_region="us east" + ) + assert f"--start-url {evil}" not in launch + assert "--start-url '$(touch /tmp/pwned)'" in launch + assert "--region 'us east'" in launch + + def test_the_default_target_adds_no_identity_flags_to_the_vault_login(self): + launch = login_vault.login_launch(log_path="$KC_LOGIN_DIR/x.log") + assert "--provider" not in launch + assert "--start-url" not in launch + assert "--region" not in launch + + def test_the_callback_flow_refuses_on_the_vault_route(self): + cmd = _SCRIPTS["callback"] + refusal = cmd.index(f'echo "{login_vault.VAULT_NO_CALLBACK_SENTINEL}"') + assert refusal < cmd.index("mkfifo") + # It exits before touching the login directory: nothing to cancel later. + assert refusal < cmd.index("KC_LOGIN_DIR=") + + def test_cancel_and_replace_stop_a_vault_login_too(self): + for cmd in ( + login._cancel_login_command(), + login._device_login_command(replace_existing=True), + ): + assert f'pkill -u "$(id -u)" -f "{login._LOGIN_PROCESS_PATTERN}"' in cmd + assert f'pkill -u "$(id -u)" -f "{login_vault.VAULT_LOGIN_PROCESS_PATTERN}"' in cmd + # The stop is only confirmed once BOTH command lines are gone. + cancel = login._cancel_login_command() + assert f'pgrep -u "$(id -u)" -f "{login_vault.VAULT_LOGIN_PROCESS_PATTERN}"' in cancel + + def test_the_vault_login_pattern_matches_what_the_box_launches(self): + import re + + pat = re.compile(login_vault.VAULT_LOGIN_PROCESS_PATTERN) + for cmdline in ( + "/srv/crew/.local/bin/kirocrew kas login", + "python3 /srv/crew/.local/bin/kirocrew kas login --provider idc --start-url https://x", + ): + assert pat.search(cmdline), cmdline + for other in ( + "kirocrew kas logout", + "kirocrew kas whoami --format json", + "kirocrew kas route", + "kirocrew gateway", + "kiro-cli login", + ): + assert not pat.search(other), other + + def test_the_vault_logout_kills_no_engine_process(self): + """The gateway's pre-turn identity-change check retires them; the kiro-cli + branch's `pkill kiro-cli acp` has no vault counterpart on purpose.""" + lines = login_vault.logout_lines() + assert "kas logout" in lines + assert "acp" not in lines + assert "kiro-cli" not in lines + + +# --------------------------------------------------------------------------- +# The local Python's reading of the vault branch's outputs. +# --------------------------------------------------------------------------- + + +def _result(stdout: str, status: str = "Success", exit_code: int = 0) -> ssm.CommandResult: + return ssm.CommandResult(status, stdout, "", exit_code) + + +class TestLocalSide: + def test_a_vault_prompt_is_read_exactly_like_a_kiro_cli_one(self, monkeypatch): + monkeypatch.setattr(login, "is_logged_in", lambda *a, **k: False) + monkeypatch.setattr( + ssm, + "run_command", + lambda *a, **k: _result( + f"{login_vault.VAULT_ROUTE_SENTINEL}\n" + "Open https://device.sso.us-east-1.amazonaws.com/?user_code=WXYZ-1234 in a browser\n" + "code: WXYZ-1234\n" + "Waiting for approval; this prompt expires at 2099-01-01T00:00:00+00:00 ...\n" + ), + ) + prompt = login.start_device_login("i-0abc", "dev", "us-east-1", open_browser=False) + assert prompt.url == "https://device.sso.us-east-1.amazonaws.com/?user_code=WXYZ-1234" + assert prompt.code == "WXYZ-1234" + assert prompt.actionable and not prompt.error + + def test_no_prompt_on_the_vault_route_never_falls_back_to_the_callback_flow(self, monkeypatch): + monkeypatch.setattr(login, "is_logged_in", lambda *a, **k: False) + monkeypatch.setattr( + ssm, + "run_command", + lambda *a, **k: _result( + f"{login_vault.VAULT_ROUTE_SENTINEL}\n❌ could not reach the sign-in service: boom\n" + ), + ) + + def _never(*a, **k): + raise AssertionError("the kiro-cli callback flow must not run on a vault-route box") + + monkeypatch.setattr(login, "_start_callback_login", _never) + prompt = login.start_device_login("i-0abc", "dev", "us-east-1", open_browser=False) + assert not prompt.actionable + assert "Kiro Crew's own vault" in prompt.error + assert "kirocrew kas login" in prompt.error + + def test_the_callback_refusal_sentinel_is_an_error_not_a_silent_prompt(self, monkeypatch): + monkeypatch.setattr( + ssm, + "run_command", + lambda *a, **k: _result(login_vault.VAULT_NO_CALLBACK_SENTINEL, "Success", 1), + ) + prompt = login._start_callback_login("i-0abc", "dev", "us-east-1", open_browser=False) + assert prompt.error and not prompt.actionable and not prompt.ports + + def test_a_vault_whoami_document_is_the_remote_identity(self, monkeypatch): + doc = json.dumps( + { + "accountType": "IamIdentityCenter", + "identity": "identity_center", + "startUrl": "https://example.awsapps.com/start", + "expiresAt": "2099-01-01T00:00:00+00:00", + } + ) + monkeypatch.setattr(ssm, "run_command", lambda *a, **k: _result(doc + "\n")) + assert login.remote_identity("i-0abc", "dev", "us-east-1") == { + "account_type": "IamIdentityCenter", + "start_url": "https://example.awsapps.com/start", + } + assert login.remote_identity_state("i-0abc", "dev", "us-east-1", target=_IDC) == "match" + assert ( + login.remote_identity_state("i-0abc", "dev", "us-east-1", target=KiroLoginTarget()) + == "mismatch" + ) + + +class TestLocalIdentityOnTheBundleRoute: + def test_the_local_vault_answers_when_this_machine_runs_the_bundle_route(self, monkeypatch): + from kiro_crew.cloud import login_target + + monkeypatch.setattr(login_target, "_local_vault_owns_sign_in", lambda: True) + monkeypatch.setattr( + login_target, "local_vault_identity", lambda: {"account_type": "BuilderId"} + ) + + def _never(): + raise AssertionError("kiro-cli must not be resolved on the bundle route") + + monkeypatch.setattr(login_target, "resolve_kiro_cli", _never) + assert login_target.discover_local_identity() == {"account_type": "BuilderId"} + + def test_an_explicit_binary_still_asks_kiro_cli(self, monkeypatch): + from kiro_crew.cloud import login_target + + monkeypatch.setattr(login_target, "_local_vault_owns_sign_in", lambda: True) + seen = {} + + class _Proc: + returncode = 0 + stdout = '{"accountType": "BuilderId"}' + stderr = "" + + def _run(argv, **kw): + seen["argv"] = argv + return _Proc() + + monkeypatch.setattr(login_target.subprocess, "run", _run) + assert login_target.discover_local_identity("kiro-cli-explicit") == { + "account_type": "BuilderId" + } + assert seen["argv"][0] == "kiro-cli-explicit" + + def test_local_vault_identity_has_the_three_answers(self, tmp_path, monkeypatch): + from datetime import datetime, timezone + + from kiro_crew.auth import bridge + from kiro_crew.auth.store import KasToken, TokenStore + from kiro_crew.cloud import login_target + + store = TokenStore(tmp_path) + monkeypatch.setattr(bridge, "default_token_store", lambda: store) + # Nothing stored: nothing to inherit. + assert login_target.local_vault_identity() == {} + store.save( + KasToken( + access_token="atk", + expires_at=datetime(2099, 1, 1, tzinfo=timezone.utc), + provider="BuilderId", + identity="builder_id", + ) + ) + assert login_target.local_vault_identity() == {"account_type": "BuilderId"} + + def _broken(): + raise OSError("vault unreadable") + + monkeypatch.setattr(bridge, "default_token_store", _broken) + assert login_target.local_vault_identity() is None + + def test_an_entry_that_will_not_read_is_an_unknown_identity_not_builder_id( + self, tmp_path, monkeypatch + ): + """`resolve` hides a read or decryption failure as "empty"; inheriting + Builder ID from an Identity Center entry that merely failed to read would + launch the wrong sign-in. Only a vault proven empty answers `{}`.""" + from kiro_crew.auth import bridge + from kiro_crew.auth.store import TokenStore + from kiro_crew.cloud import login_target + + store = TokenStore(tmp_path) + monkeypatch.setattr(bridge, "default_token_store", lambda: store) + monkeypatch.setattr(store, "resolve", lambda: None) + monkeypatch.setattr(store, "holds_no_identity", lambda: False) + assert login_target.local_vault_identity() is None + + def _unreadable(): + raise OSError("secrets.enc unreadable") + + monkeypatch.setattr(store, "holds_no_identity", _unreadable) + assert login_target.local_vault_identity() is None + + +# --------------------------------------------------------------------------- +# The scripts under bash, against stub programs, one HOME per route. +# --------------------------------------------------------------------------- + +#: The stub's `kas route` line: a warning on stderr (as the real config loader +#: emits for a loose config.local.json) and the answer on stdout. Variants below +#: swap this one line, and assert that they did. +_STUB_ROUTE_LINE = ( + ' route) echo "config.local.json is group-writable; tightening is advised" >&2; ' + 'echo "{route}"; exit 0;;' +) + +_STUB_KIROCREW = ( + """#!/bin/bash +echo "kirocrew $*" >> "$HOME/calls.log" +case "$2" in +""" + + _STUB_ROUTE_LINE + + """ + whoami) if [ -f "$HOME/vault" ]; then + if [ "$3" = "--format" ]; then echo '{{"accountType": "BuilderId", "identity": "builder_id"}}'; else echo "Signed in to Kiro Crew: BuilderId"; fi + exit 0 + fi + echo "Not signed in to Kiro Crew." >&2; exit 1;; + logout) rm -f "$HOME/vault"; echo "Signed out of Kiro Crew."; exit 0;; +esac +exit 2 +""" +) + +_STUB_KIRO_CLI = """#!/bin/bash +echo "kiro-cli $*" >> "$HOME/calls.log" +case "$1" in + whoami) if [ -f "$HOME/clistore" ]; then echo '{{"email":"x@y","accountType":"BuilderId"}}'; exit 0; fi + echo "Not logged in"; exit 1;; + logout) rm -f "$HOME/clistore"; exit 0;; +esac +exit 2 +""" + + +_STUB_PKILL = """#!/bin/bash +echo "pkill $*" >> "$HOME/calls.log" +exit 1 +""" + +_STUB_PGREP = """#!/bin/bash +echo "pgrep $*" >> "$HOME/calls.log" +exit 1 +""" + + +# The generated scripts run on a Linux instance: the login-directory guard needs +# POSIX mode bits (``mkdir -m 0700``, ``stat -c %a``) and the stubs need an +# executable bit, none of which Git Bash on Windows provides -- so the bash-backed +# tests below run only where a POSIX bash exists, like ``test_cloud_login_paths``. +_BASH = shutil.which("bash") if os.name == "posix" else None +_needs_posix_bash = pytest.mark.skipif( + _BASH is None, reason="requires a POSIX bash to execute the remote scripts" +) + + +@pytest.fixture +def box(tmp_path): + """A fake instance: ``$HOME`` with stub programs on a PATH of its own. + + The PATH puts the stub directory first, so the generated scripts resolve + ``kirocrew``, ``kiro-cli``, ``pkill`` and ``pgrep`` to the stubs and nothing + here touches a real process or a real store. + """ + if _BASH is None: + pytest.skip("requires a POSIX bash to execute the remote scripts") + + def make( + route: str, *, vault: bool = False, clistore: bool = False, kiro_cli: bool = True + ) -> dict[str, str]: + home = tmp_path / f"home-{route or 'none'}" + (home / "bin").mkdir(parents=True) + (home / "bin" / "kirocrew").write_text(_STUB_KIROCREW.format(route=route)) + if kiro_cli: + (home / "bin" / "kiro-cli").write_text(_STUB_KIRO_CLI) + # The logout and cancel scripts run uid-scoped `pkill -f` / `pgrep -f` on the + # box. Here the box is this machine, so both are shadowed by stubs that only + # log the pattern they were given: the test must never reach a real sign-in + # or agent process running under the developer's or the runner's uid. + (home / "bin" / "pkill").write_text(_STUB_PKILL) + (home / "bin" / "pgrep").write_text(_STUB_PGREP) + for stub in ("kirocrew", "kiro-cli", "pkill", "pgrep"): + if (home / "bin" / stub).exists(): + os.chmod(home / "bin" / stub, stat.S_IRWXU) # owner-only, executable + if vault: + (home / "vault").write_text("x") + if clistore: + (home / "clistore").write_text("x") + return {"HOME": str(home), "PATH": f"{home / 'bin'}:/usr/bin:/bin"} + + return make + + +def _bash(script: str, env: dict[str, str]) -> subprocess.CompletedProcess: + # cwd is the fake box's HOME: the scripts and their stub children write + # relative to nowhere in particular, and nothing of theirs may land in the + # checkout. + return subprocess.run( + ["bash", "-c", script], + cwd=env["HOME"], + env=env, + capture_output=True, + encoding="utf-8", + errors="replace", + stdin=subprocess.DEVNULL, + timeout=60, + ) + + +def _calls(env: dict[str, str]) -> list[str]: + path = Path(env["HOME"]) / "calls.log" + return path.read_text().splitlines() if path.exists() else [] + + +@_needs_posix_bash +@pytest.mark.parametrize("route", ["kas-bundle", "kiro-cli", ""]) +def test_the_check_runs_the_route_owner_and_answers_with_the_shared_sentinels(box, route): + env = box(route) + out = _bash(login._login_check_command(), env) + assert login._NOAUTH_SENTINEL in out.stdout and out.returncode == 1 + program = "kirocrew kas whoami" if route == "kas-bundle" else "kiro-cli whoami" + assert program in _calls(env) + other = "kiro-cli whoami" if route == "kas-bundle" else "kirocrew kas whoami" + assert other not in _calls(env) + + +@_needs_posix_bash +def test_a_signed_in_vault_box_reads_as_signed_in(box): + env = box("kas-bundle", vault=True) + out = _bash(login._login_check_command(), env) + assert login._TOKEN_PRESENT_SENTINEL in out.stdout and out.returncode == 0 + who = _bash(login._whoami_json_command(), env) + assert who.returncode == 0 + from kiro_crew.cloud.login_target import parse_whoami_output + + assert parse_whoami_output(who.stdout) == {"account_type": "BuilderId"} + + +@_needs_posix_bash +def test_logout_on_the_vault_route_runs_the_vault_verb_and_the_box_reads_signed_out(box): + env = box("kas-bundle", vault=True) + out = _bash(login._logout_command(), env) + assert out.returncode == 0 + assert "kirocrew kas logout" in _calls(env) + assert "kiro-cli logout" not in _calls(env) + # The kill reached the stub, scoped to this uid and the vault login's command line. + kills = [c for c in _calls(env) if c.startswith("pkill ")] + assert kills and all(login_vault.VAULT_LOGIN_PROCESS_PATTERN in k for k in kills) + assert all("-u " in k for k in kills) + assert login._NOAUTH_SENTINEL in _bash(login._login_check_command(), env).stdout + + +@_needs_posix_bash +def test_logout_on_the_kiro_cli_route_is_unchanged(box): + env = box("kiro-cli", clistore=True) + out = _bash(login._logout_command(), env) + assert out.returncode == 0 + assert "kiro-cli logout" in _calls(env) + assert "kirocrew kas logout" not in _calls(env) + + +@_needs_posix_bash +def test_the_callback_flow_refuses_on_a_vault_route_box(box): + env = box("kas-bundle") + out = _bash(login._callback_login_command(), env) + assert login_vault.VAULT_NO_CALLBACK_SENTINEL in out.stdout + assert out.returncode == 1 + assert not (Path(env["HOME"]) / ".kirocrew").exists() + + +_STUB_KIROCREW_LOGOUT_FAILS = _STUB_KIROCREW.replace( + 'logout) rm -f "$HOME/vault"; echo "Signed out of Kiro Crew."; exit 0;;', + 'logout) rm -f "$HOME/vault"; echo "sign-out did not complete" >&2; exit 1;;', +) + + +@_needs_posix_bash +def test_a_failed_vault_logout_is_reported_not_folded_into_exit_zero(box, monkeypatch): + """`kiro-cli logout` exits non-zero when there was nothing to drop, so the kiro-cli + branch ends `exit 0` and re-probes. The vault verb's non-zero means its fence was + not written; that must reach the local side as a failed sign-out, even though the + empty-vault probe would read clean.""" + env = box("kas-bundle", vault=True) + stub = Path(env["HOME"]) / "bin" / "kirocrew" + stub.write_text(_STUB_KIROCREW_LOGOUT_FAILS.format(route="kas-bundle")) + out = _bash(login._logout_command(), env) + assert out.returncode == 1 + assert login_vault.VAULT_LOGOUT_FAILED_SENTINEL in out.stdout + # And the local reading: a Success status with the sentinel is a failed logout, + # with no probe sent. + sent: list[str] = [] + + def _run(instance_id, command, *a, **k): + sent.append(command) + return ssm.CommandResult("Success", out.stdout, "", 0) + + monkeypatch.setattr(ssm, "run_command", _run) + assert login.logout("i-0abc", "dev", "us-east-1") is False + assert len(sent) == 1 + + +@_needs_posix_bash +def test_a_kiro_cli_logout_still_ends_zero_and_reprobes(box, monkeypatch): + env = box("kiro-cli") # nothing stored: kiro-cli logout "fails", as it does + out = _bash(login._logout_command(), env) + assert out.returncode == 0 + assert login_vault.VAULT_LOGOUT_FAILED_SENTINEL not in out.stdout + + +# --------------------------------------------------------------------------- +# A box with NO sign-in program: kiro-cli absent and the route probe not kas-bundle. +# --------------------------------------------------------------------------- + +#: A ``kirocrew`` whose ``kas route`` fails the way the real verb does on an +#: unreadable config: a plain message on stderr and exit 1. +_STUB_KIROCREW_ROUTE_FAILS = _STUB_KIROCREW.replace( + _STUB_ROUTE_LINE, + ' route) echo "could not read the Kiro Crew config under /srv/crew/.kiro/crew: ' + "ValueError: Expecting value. Which program owns the Kiro sign-in here is unknown; " + 'fix the config, then run this again." >&2; exit 1;;', +) +assert _STUB_KIROCREW_ROUTE_FAILS != _STUB_KIROCREW + +#: A Kiro Crew too old to have the verb: argparse's exit 2, not the unknown exit. +_STUB_KIROCREW_NO_VERB = _STUB_KIROCREW.replace( + _STUB_ROUTE_LINE, + " route) echo \"kirocrew kas: error: argument kas_action: invalid choice: 'route'\" >&2; exit 2;;", +) +assert _STUB_KIROCREW_NO_VERB != _STUB_KIROCREW + + +class TestNoSignInProgram: + def test_every_script_refuses_before_running_either_program(self): + for name, cmd in _SCRIPTS.items(): + if name == "cancel": + continue + refusal = cmd.index(login_vault.NO_SIGNIN_PROGRAM_SENTINEL) + # The resolve lines and the route probe sit before the refusal by design; + # what must come AFTER it is every invocation of either sign-in program. + assert cmd.index(login_vault.KIROCREW_BIN_RESOLVE) < refusal, name + assert cmd.index('"$KIROCREW" kas route') < refusal, name + first_program = min( + i + for i in (cmd.find('"$KIRO" ', refusal), cmd.find('"$KIROCREW" kas ', refusal)) + if i != -1 + ) + assert refusal < first_program, name + # And nothing but the probe invoked kirocrew before the refusal (the + # refusal's own `command -v "$KIRO"` test is a lookup, not a verb). + before = cmd[:refusal] + assert before.count('"$KIROCREW" kas ') == 1, name + assert '"$KIRO" whoami' not in before and '"$KIRO" login' not in before, name + assert '"$KIRO" logout' not in before, name + + def test_the_refusal_names_the_cause_and_the_remedy(self): + snippet = login_vault.require_sign_in_program() + assert "no kiro-cli" in snippet + assert "kirocrew kas route" in snippet + assert "fix its Kiro Crew config.json" in snippet + assert "install kiro-cli" in snippet + # The probe's own output is shown, so the operator reads the real error. + assert '"$KC_ROUTE_OUT"' in snippet + + @_needs_posix_bash + def test_a_bundle_route_box_whose_route_probe_fails_reads_as_unknown(self, box, monkeypatch): + """The box is on the bundle route but its config is unreadable: `kas route` + exits 1, so the vault branch is not chosen -- and with no kiro-cli either, + the script says so instead of `kiro-cli: command not found`.""" + env = box("kas-bundle", kiro_cli=False) + stub = Path(env["HOME"]) / "bin" / "kirocrew" + stub.write_text(_STUB_KIROCREW_ROUTE_FAILS.format(route="kas-bundle")) + out = _bash(login._login_check_command(), env) + assert out.returncode == 1 + assert login_vault.ROUTE_UNKNOWN_SENTINEL in out.stdout + assert "could not read the Kiro Crew config" in out.stdout + assert "nothing was changed on it" in out.stdout + assert "command not found" not in out.stdout + out.stderr + # Only the probe ran on the box: neither sign-in program was asked anything. + assert _calls(env) == ["kirocrew kas route"] + monkeypatch.setattr(ssm, "run_command", lambda *a, **k: _result(out.stdout, "Success", 1)) + # Unknown, not signed out: no sign-in is started, no verdict is given. + assert login._auth_probe("i-0abc", "dev", "us-east-1") is None + assert login.remote_identity("i-0abc", "dev", "us-east-1") is None + + @_needs_posix_bash + def test_a_box_with_no_kirocrew_and_no_kiro_cli_reads_the_same_way(self, box): + env = box("", kiro_cli=False) + (Path(env["HOME"]) / "bin" / "kirocrew").unlink() + out = _bash(login._whoami_json_command(), env) + assert out.returncode == 1 + assert login_vault.NO_SIGNIN_PROGRAM_SENTINEL in out.stdout + assert "not found" in out.stdout # the shell's own word for the missing kirocrew + + @_needs_posix_bash + def test_a_box_with_kiro_cli_is_untouched_by_the_check(self, box): + env = box("") + out = _bash(login._login_check_command(), env) + assert login_vault.NO_SIGNIN_PROGRAM_SENTINEL not in out.stdout + assert "kiro-cli whoami" in _calls(env) + + def test_the_local_side_shows_the_boxs_words_as_the_error(self, monkeypatch): + stdout = ( + f"{login_vault.NO_SIGNIN_PROGRAM_SENTINEL}\n" + "This instance has no kiro-cli, and its 'kirocrew kas route' did not answer kas-bundle (exit 1):\n" + " could not read the Kiro Crew config under /srv/crew/.kiro/crew: boom\n" + "If this instance runs the KAS bundle route, fix its Kiro Crew config.json; otherwise install kiro-cli on it.\n" + ) + monkeypatch.setattr(login, "is_logged_in", lambda *a, **k: False) + monkeypatch.setattr(ssm, "run_command", lambda *a, **k: _result(stdout, "Success", 1)) + + def _never(*a, **k): + raise AssertionError("no fallback flow on a box with no sign-in program") + + monkeypatch.setattr(login, "_start_callback_login", _never) + prompt = login.start_device_login("i-0abc", "dev", "us-east-1", open_browser=False) + assert not prompt.actionable + assert "no kiro-cli" in prompt.error + assert "could not read the Kiro Crew config" in prompt.error + assert "fix its Kiro Crew config.json" in prompt.error + assert login_vault.NO_SIGNIN_PROGRAM_SENTINEL not in prompt.error + + @_needs_posix_bash + def test_an_unknown_route_refuses_even_with_kiro_cli_installed(self, box): + """GPT's case: vault identity + kiro-cli installed + config just broke. The + probe exits 1; the script must NOT fall into the kiro-cli branch and sign + the wrong store out while the vault credential stays usable.""" + env = box("kas-bundle", vault=True, clistore=True) + stub = Path(env["HOME"]) / "bin" / "kirocrew" + stub.write_text(_STUB_KIROCREW_ROUTE_FAILS.format(route="kas-bundle")) + out = _bash(login._logout_command(), env) + assert out.returncode == 1 + assert login_vault.ROUTE_UNKNOWN_SENTINEL in out.stdout + assert "could not read the Kiro Crew config" in out.stdout + # Neither store was touched. + assert _calls(env) == ["kirocrew kas route"] + assert (Path(env["HOME"]) / "vault").exists() + assert (Path(env["HOME"]) / "clistore").exists() + # The local side: logout reports failure, nothing reads as signed out. + import kiro_crew.cloud.login as login_mod + + def _run(instance_id, command, *a, **k): + return ssm.CommandResult("Success", out.stdout, "", 1) + + orig = ssm.run_command + ssm.run_command = _run + try: + assert login_mod.logout("i-0abc", "dev", "us-east-1") is False + assert login_mod._auth_probe("i-0abc", "dev", "us-east-1") is None + finally: + ssm.run_command = orig + + @_needs_posix_bash + def test_a_kirocrew_without_the_verb_still_takes_the_kiro_cli_branch(self, box): + """An older Kiro Crew answers argparse's exit 2 to `kas route`: that is "no + route answer", and the box keeps its kiro-cli commands exactly as before.""" + env = box("", clistore=True) + stub = Path(env["HOME"]) / "bin" / "kirocrew" + stub.write_text(_STUB_KIROCREW_NO_VERB.format(route="")) + out = _bash(login._login_check_command(), env) + assert out.returncode == 0 + assert login._TOKEN_PRESENT_SENTINEL in out.stdout + assert login_vault.ROUTE_UNKNOWN_SENTINEL not in out.stdout + assert "kiro-cli whoami" in _calls(env) + + def test_the_callback_flow_reads_the_refusal_as_an_error_too(self, monkeypatch): + stdout = f"{login_vault.NO_SIGNIN_PROGRAM_SENTINEL}\nThis instance has no kiro-cli.\n" + monkeypatch.setattr(ssm, "run_command", lambda *a, **k: _result(stdout, "Success", 1)) + callback = login._start_callback_login("i-0abc", "dev", "us-east-1", open_browser=False) + assert callback.error == "This instance has no kiro-cli." + assert not callback.actionable and not callback.ports + + +@_needs_posix_bash +def test_a_warning_on_stderr_does_not_change_the_route(box): + """The config loader warns on stderr about a loose config.local.json while the + verb still answers on stdout; only stdout is compared.""" + env = box("kas-bundle", vault=True) + out = _bash(login._login_check_command(), env) + assert out.returncode == 0 + assert login._TOKEN_PRESENT_SENTINEL in out.stdout + assert "kirocrew kas whoami" in _calls(env) + assert login_vault.NO_SIGNIN_PROGRAM_SENTINEL not in out.stdout + + +def test_the_prelude_keeps_stderr_apart_from_the_compared_answer(): + prelude = login_vault.route_prelude() + assert '2>"$KC_ROUTE_ERR_FILE"' in prelude + assert "2>&1" not in prelude + assert 'KC_ROUTE="$KC_ROUTE_OUT"' in prelude diff --git a/test/test_kas_auth_whoami.py b/test/test_kas_auth_whoami.py new file mode 100644 index 00000000000..12f260425a3 --- /dev/null +++ b/test/test_kas_auth_whoami.py @@ -0,0 +1,277 @@ +"""``auth/whoami.py``: the vault's identity in ``kiro-cli whoami``'s vocabulary. + +The cloud sign-in reads a remote identity through ``parse_whoami_output`` whichever +program printed it, so the document the vault renders must parse there into the +same fields kiro-cli's does, carry no e-mail (the vault stores none), and never a +token. The OIDC sign-in records the start URL it was begun against, which is the +one field that tells one organisation's Identity Center sign-in from another's. +""" + +from __future__ import annotations + +import json +from datetime import datetime, timedelta, timezone + +import pytest + +from kiro_crew.auth.login import builder_id, control_plane +from kiro_crew.auth.store import KasToken, TokenStore +from kiro_crew.auth.whoami import ( + START_URL_EXTRA_KEY, + account_type_for, + resolve_signed_in, + vault_owns_sign_in, + whoami_document, +) +from kiro_crew.cloud.login_target import ( + KiroLoginTarget, + identity_matches_target, + is_personal_account_type, + parse_whoami_output, + target_from_whoami, +) + +_FUTURE = datetime(2099, 1, 1, tzinfo=timezone.utc) + + +def _token(identity: str, provider: str, **extra) -> KasToken: + return KasToken( + access_token="atk-secret-value", + expires_at=_FUTURE, + provider=provider, + identity=identity, + refresh_token="rtk-secret-value", + profile_arn="arn:aws:kiro:us-east-1:1:profile/p1", + extra=dict(extra), + ) + + +@pytest.mark.parametrize( + "identity, provider, expected", + [ + ("builder_id", "BuilderId", "BuilderId"), + ("identity_center", "Enterprise", "IamIdentityCenter"), + ("external_idp", "ExternalIdp", "ExternalIdP"), + ("social", "Google", "SocialGoogle"), + ("social", "Github", "SocialGithub"), + ("social", "", ""), + ("not-a-slot", "BuilderId", ""), + ], +) +def test_account_type_spells_each_slot_the_way_kiro_cli_does(identity, provider, expected): + assert account_type_for(identity, provider) == expected + + +def test_the_document_parses_through_the_cloud_whoami_parser(): + doc = whoami_document(_token("builder_id", "BuilderId")) + parsed = parse_whoami_output(json.dumps(doc)) + assert parsed == {"account_type": "BuilderId"} + # A Builder ID vault sign-in satisfies the default login target, as kiro-cli's does. + assert identity_matches_target(parsed, KiroLoginTarget()) + assert is_personal_account_type(parsed["account_type"]) + + +def test_an_identity_center_document_carries_the_recorded_start_url(): + doc = whoami_document( + _token("identity_center", "Enterprise", start_url="https://acme.awsapps.com/start") + ) + parsed = parse_whoami_output(json.dumps(doc)) + assert parsed["account_type"] == "IamIdentityCenter" + assert parsed["start_url"] == "https://acme.awsapps.com/start" + target = KiroLoginTarget.from_fields( + start_url="https://acme.awsapps.com/start", region="us-east-1" + ) + assert identity_matches_target(parsed, target) + other = KiroLoginTarget.from_fields( + start_url="https://other.awsapps.com/start", region="us-east-1" + ) + assert not identity_matches_target(parsed, other) + assert target_from_whoami(parsed, region="us-east-1") == target + + +def test_an_identity_center_document_without_a_start_url_is_an_unknown_organisation(): + # A token stored before the start URL was recorded: the parser sees the type and + # no URL, and the target derivation answers None (not Builder ID), as it does for + # a kiro-cli whoami that omitted it. + doc = whoami_document(_token("identity_center", "Enterprise")) + assert "startUrl" not in doc + assert target_from_whoami(parse_whoami_output(json.dumps(doc))) is None + + +def test_the_document_never_carries_a_secret_or_an_email(): + token = _token("social", "Google", start_url="https://ignored.example") + doc = whoami_document(token) + serialised = json.dumps(doc) + assert token.access_token not in serialised + assert token.refresh_token not in serialised + assert token.profile_arn not in serialised + assert "email" not in doc + # The whoami parser must not read a blank e-mail as "known to be blank". + assert "email" not in parse_whoami_output(serialised) + + +def test_a_non_string_start_url_extra_is_not_rendered(): + doc = whoami_document(_token("builder_id", "BuilderId", **{START_URL_EXTRA_KEY: 42})) + assert "startUrl" not in doc + + +def test_resolve_signed_in_is_the_usable_predicate(tmp_path): + store = TokenStore(tmp_path) + assert resolve_signed_in(store) is None + lapsed = KasToken( + access_token="atk", + expires_at=datetime.now(timezone.utc) - timedelta(hours=1), + provider="BuilderId", + identity="builder_id", + refresh_token=None, + ) + store.save(lapsed) + # Stored, but nothing can renew it: signed out for every reader that asks. + assert resolve_signed_in(store) is None + store.save(_token("builder_id", "BuilderId")) + found = resolve_signed_in(store) + assert found is not None and found.identity == "builder_id" + + +def test_resolve_signed_in_answers_none_for_an_unreadable_vault(): + class _Broken: + def resolve(self): + raise OSError("vault unreadable") + + assert resolve_signed_in(_Broken()) is None # type: ignore[arg-type] + + +def test_agent_owns_sign_in_is_the_one_predicate(): + from kiro_crew.auth.whoami import agent_owns_sign_in + + class _A: + def __init__(self, b, k): + self.acp_backend, self.kas_bundle = b, k + + assert agent_owns_sign_in(_A("kas", True)) is True + assert agent_owns_sign_in(_A("kas", False)) is False + assert agent_owns_sign_in(_A("kiro", True)) is False + assert agent_owns_sign_in(object()) is False + + +def test_a_degraded_config_load_is_refused_for_the_owner_decision(monkeypatch): + from kiro_crew.auth.whoami import ConfigUnreadable, load_sign_in_owner_config + from kiro_crew.config.loader import KiroCrewConfig + + class _Cfg: + agent = object() + _base_unreadable = True + _overlay_unreadable = False + + monkeypatch.setattr(KiroCrewConfig, "load", classmethod(lambda cls: _Cfg())) + with pytest.raises(ConfigUnreadable, match="config.json"): + load_sign_in_owner_config() + # The in-process reader keeps its kiro-cli fallback over the same refusal. + assert vault_owns_sign_in() is False + + +def test_vault_owns_sign_in_is_positive_on_both_halves(monkeypatch): + from kiro_crew.config.loader import KiroCrewConfig + + class _Agent: + def __init__(self, backend, bundle): + self.acp_backend = backend + self.kas_bundle = bundle + + class _Cfg: + def __init__(self, backend, bundle): + self.agent = _Agent(backend, bundle) + + cases = { + ("kas", True): True, + ("kas", False): False, + ("kiro", True): False, + ("claude", True): False, + } + for (backend, bundle), expected in cases.items(): + monkeypatch.setattr( + KiroCrewConfig, "load", classmethod(lambda cls, b=backend, k=bundle: _Cfg(b, k)) + ) + assert vault_owns_sign_in() is expected, (backend, bundle) + + def _boom(cls): + raise OSError("config unreadable") + + monkeypatch.setattr(KiroCrewConfig, "load", classmethod(_boom)) + assert vault_owns_sign_in() is False + + +# --------------------------------------------------------------------------- +# The service records the start URL an OIDC sign-in was begun against. +# --------------------------------------------------------------------------- + + +def _oidc_auth() -> builder_id.DeviceAuthorization: + return builder_id.DeviceAuthorization( + device_code="oidc-dc-1", + user_code="WXYZ-1234", + verification_uri="https://device.sso.us-east-1.amazonaws.com/", + verification_uri_complete="https://device.sso.us-east-1.amazonaws.com/?user_code=WXYZ-1234", + expires_at=datetime.now(timezone.utc) + timedelta(seconds=300), + interval_secs=0.0, + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "provider, kwargs, expected_url", + [ + ("builder_id", {}, "https://view.awsapps.com/start"), + ( + "idc", + {"start_url": "https://acme.awsapps.com/start", "region": "eu-west-1"}, + "https://acme.awsapps.com/start", + ), + ], +) +async def test_an_oidc_sign_in_records_its_start_url_on_the_stored_token( + tmp_path, monkeypatch, provider, kwargs, expected_url +): + from kiro_crew.auth.service import KasLoginService + + async def _fake_register(region, *, session): + return builder_id.RegisteredClient(client_id="cid-1", client_secret="csec-1") + + async def _fake_start(client, *, region, start_url, session): + return _oidc_auth() + + async def _fake_poll(client, auth, *, region, identity, provider, session): + return KasToken( + access_token="at-1", + expires_at=_FUTURE, + provider=provider, + identity=identity, + refresh_token="rt-1", + region=region, + client_id=client.client_id, + client_secret=client.client_secret, + ) + + async def _fake_profiles(access_token, *, region, session): + return [ + control_plane.KiroProfile(arn="arn:aws:kiro:eu-west-1:1:profile/p1", profile_name="P1") + ] + + monkeypatch.setattr(builder_id, "register_client", _fake_register) + monkeypatch.setattr(builder_id, "start_device_authorization", _fake_start) + monkeypatch.setattr(builder_id, "poll_token_once", _fake_poll) + monkeypatch.setattr(control_plane, "list_available_profiles", _fake_profiles) + + store = TokenStore(tmp_path) + service = KasLoginService(store) + try: + begun = await service.begin_device(provider, **kwargs) + result = await service.poll_device(begun["login_id"]) + finally: + await service.close() + assert result["status"] == "authorized" + saved = store.resolve() + assert saved is not None + assert saved.extra[START_URL_EXTRA_KEY] == expected_url + doc = whoami_document(saved) + assert doc["startUrl"] == expected_url diff --git a/test/test_kas_login_api.py b/test/test_kas_login_api.py index cab59736d61..865aa1be286 100644 --- a/test/test_kas_login_api.py +++ b/test/test_kas_login_api.py @@ -237,6 +237,21 @@ async def test_poll_ok_and_missing_and_unknown(): assert _body(resp)["code"] == "unknown_login_id" +@pytest.mark.asyncio +async def test_poll_answers_a_sign_out_during_the_login_as_the_begin_409(): + """The code the service returns when a sign-out (this gateway's or a shell's) + landed while the login was pending must not reach the card as a plain + terminal answer -- that path restarts the provider on the device flow.""" + + class _SignedOut(_StubService): + async def poll_device(self, login_id): + return {"status": "error", "code": "signed_out_during_login"} + + resp = await api_kas_login_poll(_FakeRequest(_SignedOut(), {"login_id": "lid-1"})) + assert resp.status == 409 + assert _body(resp)["code"] == "signed_out_during_login" + + @pytest.mark.asyncio async def test_logout_ok_and_invalid_identity(): resp = await api_kas_login_logout(_FakeRequest(_StubService(), {"identity": "social"})) diff --git a/test/test_kas_login_signout_generation.py b/test/test_kas_login_signout_generation.py new file mode 100644 index 00000000000..4cc80b08b8f --- /dev/null +++ b/test/test_kas_login_signout_generation.py @@ -0,0 +1,256 @@ +"""A sign-out in one process stops a login pending in another from re-filling the vault. + +The gateway's in-memory epoch covers its own pending logins. ``kirocrew kas logout`` +runs in a shell, in a second process, with a second ``KasLoginService`` over the same +vault; its sign-out must still stop a dashboard login that was pending in the gateway +from persisting once the user approves the code. The store carries a sign-out +generation every sign-out bumps; a login snapshots it when it begins and compares it +before it writes. These tests drive two services over one store, the way the two +processes do, and also pin the two sibling fixes the same round brought: a refresh +keeps the token's recorded claims, and a refresh-rejected slot reads as signed out. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import pytest + +from kiro_crew.auth.refresh import ensure_fresh +from kiro_crew.auth.service import KasLoginService +from kiro_crew.auth.store import KasToken, TokenStore +from kiro_crew.auth.whoami import START_URL_EXTRA_KEY, resolve_signed_in + + +class _FakeResp: + def __init__(self, status: int, payload): + self.status = status + self._payload = payload + + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + async def json(self, *, content_type: str | None = "application/json"): + return self._payload + + async def text(self): + return str(self._payload) + + +class _FakeSession: + def __init__(self, responses): + self._responses = list(responses) + + def post(self, url, *, json=None, data=None, headers=None): # noqa: A002 + return self._responses.pop(0) + + @property + def closed(self): + return False + + async def close(self): + return None + + +def _device_auth(): + from kiro_crew.auth.login import device + + return device.DeviceAuthorization( + device_code="dc-1", + user_code="WXYZ-1234", + verification_uri="https://app.kiro.dev/device", + verification_uri_complete="https://app.kiro.dev/device?user_code=WXYZ-1234", + expires_at=datetime.now(timezone.utc) + timedelta(seconds=300), + interval_secs=0.0, + ) + + +async def _begin_social(service: KasLoginService, monkeypatch) -> str: + from kiro_crew.auth.login import device + + async def _fake_initiate(provider, *, session): + return _device_auth() + + monkeypatch.setattr(device, "initiate_device_authorization", _fake_initiate) + return (await service.begin_device("google"))["login_id"] + + +_AUTHORIZED = { + "status": "authorized", + "accessToken": "at-1", + "refreshToken": "rt-1", + "expiresIn": 3600, + "profileArn": "arn:aws:kiro:us-east-1:1:profile/p1", +} + + +@pytest.mark.asyncio +async def test_a_sign_out_in_another_process_stops_a_pending_login_from_persisting( + tmp_path, monkeypatch +): + store = TokenStore(tmp_path) + gateway = KasLoginService(store, session=_FakeSession([_FakeResp(200, _AUTHORIZED)])) + shell = KasLoginService(TokenStore(tmp_path)) + try: + login_id = await _begin_social(gateway, monkeypatch) + # The shell signs out while the dashboard's code is still awaiting approval. + store.save( + KasToken( + access_token="old", + expires_at=datetime.now(timezone.utc) + timedelta(hours=1), + provider="BuilderId", + identity="builder_id", + ) + ) + await shell.logout("builder_id") + assert store.resolve() is None + # The user approves the code; the gateway's poll must not re-fill the vault. + result = await gateway.poll_device(login_id) + finally: + await gateway.close() + await shell.close() + assert result == {"status": "error", "code": "signed_out_during_login"} + assert store.resolve() is None + # Terminal for that login: it is gone from the pending table. + from kiro_crew.auth.service import UnknownLoginError + + with pytest.raises(UnknownLoginError): + await gateway.poll_device(login_id) + + +@pytest.mark.asyncio +async def test_a_login_with_no_sign_out_in_between_persists(tmp_path, monkeypatch): + store = TokenStore(tmp_path) + gateway = KasLoginService(store, session=_FakeSession([_FakeResp(200, _AUTHORIZED)])) + try: + login_id = await _begin_social(gateway, monkeypatch) + result = await gateway.poll_device(login_id) + finally: + await gateway.close() + assert result["status"] == "authorized" + saved = store.resolve() + assert saved is not None and saved.identity == "social" + + +def test_every_sign_out_moves_the_generation_and_an_absent_marker_reads_empty(tmp_path): + store = TokenStore(tmp_path) + assert store.signout_generation() == "" + store.bump_signout_generation() + first = store.signout_generation() + assert first + store.bump_signout_generation() + assert store.signout_generation() not in ("", first) + + +@pytest.mark.asyncio +async def test_a_refresh_keeps_the_recorded_start_url(tmp_path): + store = TokenStore(tmp_path) + stale = KasToken( + access_token="old", + expires_at=datetime.now(timezone.utc) + timedelta(seconds=10), + provider="Enterprise", + identity="identity_center", + refresh_token="rt", + profile_arn="arn:aws:kiro:eu-west-1:1:profile/p1", + region="eu-west-1", + client_id="cid", + client_secret="csec", + extra={START_URL_EXTRA_KEY: "https://acme.awsapps.com/start"}, + ) + store.save(stale) + out = await ensure_fresh( + store, + stale, + session=_FakeSession([_FakeResp(200, {"accessToken": "new", "expiresIn": 3600})]), + ) + assert out.access_token == "new" + assert out.extra[START_URL_EXTRA_KEY] == "https://acme.awsapps.com/start" + reread = store.resolve() + assert ( + reread is not None and reread.extra[START_URL_EXTRA_KEY] == "https://acme.awsapps.com/start" + ) + + +def test_a_refresh_rejected_slot_reads_as_signed_out(tmp_path): + store = TokenStore(tmp_path) + token = KasToken( + access_token="atk", + expires_at=datetime.now(timezone.utc) - timedelta(minutes=1), + provider="Enterprise", + identity="identity_center", + refresh_token="rt", + profile_arn="arn:aws:kiro:us-east-1:1:profile/p1", + ) + store.save(token) + assert token.is_usable() # a refresh token keeps the spawn predicate true + assert resolve_signed_in(store) is not None + store.mark_refresh_rejected("identity_center") + # Doctor and the card call this "sign-in expired"; so does the shell verb. + assert resolve_signed_in(store) is None + + +def test_a_sign_out_whose_fence_cannot_be_written_reports_failure(tmp_path, monkeypatch): + """The marker is the cross-process fence; a sign-out must not publish success + while a pending approval elsewhere could still re-fill the vault.""" + import asyncio + + from kiro_crew.auth.store import TokenStoreError + + store = TokenStore(tmp_path) + store.save( + KasToken( + access_token="atk", + expires_at=datetime.now(timezone.utc) + timedelta(hours=1), + provider="BuilderId", + identity="builder_id", + ) + ) + + # A marker path under a FILE: the write fails with NotADirectoryError on every + # platform, with no permission bits to depend on. + blocker = tmp_path / "blocker" + blocker.write_text("x", encoding="utf-8") + monkeypatch.setattr(store, "_signout_generation_path", lambda: blocker / "signed-out") + with pytest.raises(TokenStoreError, match="sign-out generation"): + asyncio.run(KasLoginService(store).logout("builder_id")) + + +@pytest.mark.asyncio +async def test_a_loopback_login_that_fails_leaves_no_generation_row(tmp_path, monkeypatch): + """Every per-login table is keyed by one id and must leave together: a row left + in ``_signout_gen`` after the entry drops out of ``_pending`` is never swept.""" + import asyncio + + from kiro_crew.auth.login import portal + from kiro_crew.auth.service import _PendingLoopbackLogin + from kiro_crew.auth.store import SocialProvider + + store = TokenStore(tmp_path) + service = KasLoginService(store) + + async def _fail() -> KasToken: + raise portal.PortalTimeoutError("nobody came back") + + task = asyncio.ensure_future(_fail()) + try: + await task + except portal.PortalTimeoutError: + pass + entry = _PendingLoopbackLogin( + task=task, + provider=SocialProvider.GOOGLE, + expires_at=datetime.now(timezone.utc) + timedelta(seconds=300), + ) + service._pending["lid-x"] = entry + service._signout_gen["lid-x"] = "" + try: + result = await service.poll_device("lid-x") + finally: + await service.close() + assert result["code"] == "loopback_timeout" + assert "lid-x" not in service._pending + assert "lid-x" not in service._signout_gen + assert "lid-x" not in service._replaces