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