From f3cddd2900a6787b411375c19fd8bc69a386c8af Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Tue, 8 Sep 2026 00:25:35 +0100 Subject: [PATCH 1/2] docs: add the persistent-aws-creds change proposal --- .../persistent-aws-creds/.openspec.yaml | 2 + .../changes/persistent-aws-creds/design.md | 102 ++++++++++ .../changes/persistent-aws-creds/proposal.md | 35 ++++ .../specs/endpoint-provisioning/spec.md | 45 +++++ .../specs/remote-auth/spec.md | 97 +++++++++ .../specs/remote-endpoint/spec.md | 188 ++++++++++++++++++ .../changes/persistent-aws-creds/tasks.md | 34 ++++ 7 files changed, 503 insertions(+) create mode 100644 openspec/changes/persistent-aws-creds/.openspec.yaml create mode 100644 openspec/changes/persistent-aws-creds/design.md create mode 100644 openspec/changes/persistent-aws-creds/proposal.md create mode 100644 openspec/changes/persistent-aws-creds/specs/endpoint-provisioning/spec.md create mode 100644 openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md create mode 100644 openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md create mode 100644 openspec/changes/persistent-aws-creds/tasks.md diff --git a/openspec/changes/persistent-aws-creds/.openspec.yaml b/openspec/changes/persistent-aws-creds/.openspec.yaml new file mode 100644 index 00000000..2e24cfa4 --- /dev/null +++ b/openspec/changes/persistent-aws-creds/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-07 diff --git a/openspec/changes/persistent-aws-creds/design.md b/openspec/changes/persistent-aws-creds/design.md new file mode 100644 index 00000000..1b91b215 --- /dev/null +++ b/openspec/changes/persistent-aws-creds/design.md @@ -0,0 +1,102 @@ +## Context + +Every `spinloop remote` call signs with the caller's ambient AWS credentials: Lambda Function URL calls through `sign()` (`internal/remote/remote.go:608`) and direct SDK calls (STS, CloudFormation, EC2, CloudWatch Logs, Pricing) through `LoadAWSConfig` (`internal/remote/aws.go:23`). Both funnel into `awsconfig.LoadDefaultConfig`, so there are exactly two choke points where a stored credential can be introduced. The control-plane stack (`remote/lib/llm-stack.ts`) currently creates only machine principals — `InstanceRole`, `SeedRole`, and the Lambdas' execution roles — and no human-facing principal. Deployment identity is (account, region): the stack name is fixed (`cloud-vm-llm`) and environments live within it. See proposal.md for the motivation. + +## Goals / Non-Goals + +**Goals:** + +- A long-lived AWS credential that survives between SSO log-ins, stored only in the OS keystore (or an owner-only file where no keystore exists), created and removed by `spinloop remote auth`. +- The stored key covers the day-to-day `remote` commands only; `bootstrap` and `bake` keep requiring ambient administrator credentials. +- `--store` doubles as rotation and can run with the stored key alone, so no admin login is needed after the first store. +- Zero behaviour change for anyone who never stores a key: the standard chain stays the fallback. + +**Non-Goals:** + +- No SSO device flow or AWS login inside spinloop — the first store still uses whatever credentials the user already has configured. +- No per-environment credential scoping: one entry per region, covering all environments in the account. +- No automatic expiry: access keys do not expire; the ~90-day lifetime is a documented rotation expectation. +- No change to the Lambda authorisation model (Function URLs stay `AWS_IAM`, any principal in the account with the grant can invoke). + +## Decisions + +### 1. Principal: an IAM user with an inline policy, not a role + +AWS access keys attach to IAM users only; a role yields at most 12-hour assumed sessions, which would not meet the ~90-day goal. The CDK stack therefore creates an IAM user, `cloud-vm-llm-remote-cli` (fixed name, not deployment-specific, so the public-repo identifier check is unaffected), with an inline policy attached at synth: + +- `lambda:InvokeFunctionUrl` on the seven function-URL ARNs (`startUrl.functionUrlArn` et al.). +- `logs:DescribeLogStreams`, `logs:FilterLogEvents`, `logs:GetLogEvents` on the runner and boot log-group ARNs (`remote logs`). +- `cloudformation:DescribeStacks` on `this.stackArn` (control-plane discovery by `deploy` and `remote auth`). +- `pricing:GetProducts` (resource `*` — the Price List API has no resource-level scoping) so `status --cost` works with the stored key. +- `iam:GetUser`, `iam:ListAccessKeys`, `iam:CreateAccessKey`, `iam:DeleteAccessKey` on the user's own ARN — self-service rotation. + +Alternatives considered: a customer-managed policy (a named, reusable policy is nicer in the IAM console, but it outlives stack deletion unless specially handled — an inline policy deletes with the stack, which is what a re-deploy of an older template version should do); having the user assume a role per call (defeats the ~90-day goal). + +The user name is a constant on the Go side, not a stack output: `remote auth --store` confirms the user exists with `iam:GetUser` before doing anything, which is also the failure path for control planes deployed before this change. + +### 2. Keystore backend: `99designs/keyring`, with a file fallback + +The OS keystore is accessed through the `99designs/keyring` library (Keychain on macOS, Credential Manager on Windows, Secret Service on Linux). Where the platform keystore is unavailable (headless Linux without a D-Bus Secret Service), the entry is stored in an owner-only file `/keystore/remote-.json` (0600 in a 0700 directory, the same treatment the repo already gives files that may hold secrets). The entry records: access key id, secret, account, user name, region, stored-at timestamp, and which store holds it. `--store` and the no-flag report say which store was used. + +The entry is keyed by region (`spinloop-remote-` in the keyring). Keying by (account, region) would be more precise but is a chicken-and-egg: resolving the account requires credentials, which is exactly what the lookup is for. One control plane per account per region is the norm; if two accounts are bootstrapped in the same region, the last stored entry wins and the account recorded in the entry makes the mismatch visible in the report. + +Alternatives: `zalando/go-keyring` (less actively maintained); hand-rolled Security.framework/wincred wrappers (cross-platform burden for nothing). + +### 3. Precedence at the two choke points + +`LoadAWSConfig` becomes the single decision point: + +1. If the process environment carries explicit AWS credentials (`AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` set) or an explicit profile selection (`AWS_PROFILE`, `AWS_SHARED_CREDENTIALS_FILE`, `AWS_CONFIG_FILE`), load the default chain exactly as today — these win over a stored key. This preserves the existing behaviour where a Spinloop's `.env`/`ENV` injects credentials into the process environment before AWS work (`applySpinloopEnv`), and it lets an operator override the stored key for debugging. +2. Otherwise, if a stored entry exists for the region, load the default config with an explicit static credentials provider for it (`awsconfig.WithCredentialsProvider`). This skips the rest of the chain, so the stored key beats shared config, SSO sessions, and IMDS. +3. Otherwise, today's behaviour: plain default chain. + +`sign()` already calls `LoadAWSConfig`, so Function URL signing, log reading, discovery, and bake polling all inherit this with no per-caller changes. `bootstrap` and `bake` are the exception: they call an ambient-only variant (the current `LoadAWSConfig` body), never consulting the keystore. + +The expired-credentials hint is source-aware: when the stored key was the credential in use, the "refresh" guidance says `spinloop remote auth --store` instead of "refresh your SSO session". + +### 4. The `auth` command + +`cmd/spinloop/remote_auth.go`, registered under `remote` alongside `bootstrap` and `bake`. It takes no Spinloop path; it takes a `--region` flag resolved with the same precedence as bootstrap's (`resolveRegion`), plus `--store` and `--clear`, which are mutually exclusive. + +`--store`: + +1. Resolve the region; load ambient credentials (never the keystore). +2. `iam:GetUser` for the control-plane user; absent → "re-run `spinloop remote bootstrap` first". +3. If an entry is already stored for the region: rotation mode — the stored credential creates the replacement key (a separate IAM client configured with the stored static credentials), the entry is swapped, and the superseded key is deleted. Otherwise the ambient credentials create the first key. +4. Respect IAM's two-keys-per-user cap: if the user already has two keys and the store is not a rotation, fail and say so (the other key is likely on another machine — `--clear` there first). +5. Verify the new key with STS `GetCallerIdentity`: it must resolve to the same account as the caller, or it is not stored. +6. Write the entry and confirm: account, region, user, access key id, store used. The secret is never printed. + +`--clear`: + +1. Resolve the region and look up the entry; none → say so and exit 0 (idempotent). +2. Delete the access key on the AWS side using the stored credential, best effort: on failure the local entry is still removed and the command reports that the key may still exist on the AWS side. +3. Remove the local entry. + +No flag: list every stored entry (account, region, user, access key id, stored-at, store) from the local store only — no AWS call, no secret. Nothing stored → say so and name `--store`. + +Following the repo's existing test pattern (package-variable seams as in `remote_bootstrap.go`), the keyring operations and the IAM/STS calls are behind seams so the flows are unit-testable without AWS, a network, or a real keystore. + +### 5. Existing deployments + +A control plane deployed before this change has no such user. Re-running `spinloop remote bootstrap` is idempotent and adds it. Until then, `remote auth --store` fails with that guidance, and every other command behaves exactly as before (no stored key, standard chain). + +## Risks / Trade-offs + +- [A long-lived unattended key in a keystore is a standing credential] → the policy is scoped to day-to-day control only (no stack create/delete, no bake, no instance creation); `--clear` deletes the AWS-side key; rotation is one command. +- [Keyring access can prompt for the OS password on some platforms (e.g. a locked macOS Keychain item, Linux Secret Service unlock)] → the prompt comes from the OS, not spinloop; the file fallback avoids keystore access entirely where no keystore exists, and the report says which store is in use. +- [Two keys per user cap with multiple machines] → the cap is checked before creating; the error names the fix (`--clear` on the other machine, or delete via the console). Rotation on one machine deletes only the key that machine's entry held. +- [Same region, two accounts] → last stored entry wins; the recorded account in the no-flag report makes this visible. +- [`99designs/keyring` is a new dependency] → it is the only new Go module, used in one place (`internal/remote`); its Linux path needs D-Bus only when actually used. +- [CDK rollback: an older template re-deployed deletes the user and its keys] → expected behaviour; stored entries then 403 and the hint says to re-run bootstrap and `--store`. + +## Migration Plan + +1. Ship the CDK change and the CLI change together (one release). The CLI is backward-compatible: with no stored key, behaviour is unchanged. +2. Users re-run `spinloop remote bootstrap` (idempotent) to add the user, then `spinloop remote auth --store` with their existing admin credentials. +3. No data migration: the registry and `remote.json` formats are untouched. +4. Rollback: redeploying an older CDK template removes the user and its keys; reverting the CLI restores ambient-only resolution. Stored file-fallback entries are inert in either case. + +## Open Questions + +None — the remaining unknowns (exact keyring library quirks per platform, the `--region` flag's default when nothing else resolves) are implementation details that do not change the specs, the approach, or the task breakdown. diff --git a/openspec/changes/persistent-aws-creds/proposal.md b/openspec/changes/persistent-aws-creds/proposal.md new file mode 100644 index 00000000..cafa9016 --- /dev/null +++ b/openspec/changes/persistent-aws-creds/proposal.md @@ -0,0 +1,35 @@ +# Persistent AWS Credentials + +## Why + +Every `spinloop remote` call is signed with the caller's ambient AWS credentials — an SSO session, a profile, or environment variables. SSO sessions expire, so operating a remote endpoint means logging back into AWS on a regular cycle (issue #172: "avoid regular log-ins and token expiration"). Spinloop stores no credentials of its own today, so there is no way to keep a remote endpoint reachable between log-ins. + +## What Changes + +- A new `spinloop remote auth` subcommand: + - `--store` creates an AWS access key for a control-plane IAM user (created by the CDK stack) and stores it in the OS keystore — Keychain on macOS, Credential Manager on Windows, Secret Service on Linux — keyed by region. When a key is already stored, `--store` rotates: the stored key creates the replacement, the new key is verified against the same account, the entry is swapped, and the old key is deleted — no administrator credentials needed. + - `--clear` removes the stored entry and deletes the access key on the AWS side (best effort, so a cleared key does not linger). + - With no flag it reports what is stored — account, region, user, access key id, when stored — never the secret. +- The CDK control-plane stack (`remote/lib/llm-stack.ts`) gains an IAM user (`cloud-vm-llm-remote-cli`) carrying a narrow policy: invoke the seven control-plane function URLs, read the `/cloud-vm-llm/*` CloudWatch log groups, `cloudformation:DescribeStacks` on the stack, and manage its own access keys. Access keys attach to IAM users, not roles, so this is the principal a long-lived keypair can belong to. +- Credential resolution for `spinloop remote` (and fleet's remote nodes, which share the same client) consults the OS keystore for the target region. Explicit AWS environment credentials or a named profile override the stored key; otherwise the stored key takes precedence over the rest of the standard chain (shared config, SSO sessions, IMDS), which remains the fallback. +- `bootstrap` and `bake` keep using ambient administrator credentials and explicitly ignore any stored key: they provision the control plane itself, and the stored key's policy does not cover them. +- A control plane deployed before this change has no such user; `remote auth --store` then says to re-run `spinloop remote bootstrap` first. +- Access keys do not expire. The ~90-day lifetime is a rotation expectation: `--store` is the rotation, and it can be run with the stored key alone. + +## Capabilities + +### New Capabilities + +- `remote-auth`: the `spinloop remote auth` command and the stored long-lived control-plane credential — keystore storage, lookup precedence, self-rotation, and clearing. + +### Modified Capabilities + +- `remote-endpoint`: the "Authenticated control requests" requirement — resolution from the standard chain and "Spinloop SHALL NOT store AWS credentials of its own" — changes so that an OS-keystore-stored key is a credential source, overridable by explicit environment credentials or a profile; the "Remote command group" requirement gains the `auth` subcommand. +- `endpoint-provisioning`: the stack bootstrap deploys gains the control-plane IAM user and its policy, and the bootstrap plan and report name it. + +## Impact + +- Go: `internal/remote` gains a keystore-backed credential source consulted at the two choke points (`LoadAWSConfig`, `sign`); new `cmd/spinloop/remote_auth.go`; `bootstrap` and `bake` explicitly bypass the stored key. New Go dependency: an OS keyring library (e.g. `99designs/keyring`), with an owner-only file under spinloop's config directory as the fallback where no keystore exists (headless Linux). +- CDK: `remote/lib/llm-stack.ts` adds the IAM user, its policy, and a stack output naming the user; existing deployments need a re-bootstrap before `remote auth --store` works. +- Docs: `docs/commands/remote.md`, `README.md`, `remote/README.md` — the credential story moves from "ambient only" to "stored key, ambient as fallback". +- Public-repo constraint unchanged: the user name is fixed, not deployment-specific, so `scripts/check-no-cloud-identifiers.sh` has nothing new to catch. diff --git a/openspec/changes/persistent-aws-creds/specs/endpoint-provisioning/spec.md b/openspec/changes/persistent-aws-creds/specs/endpoint-provisioning/spec.md new file mode 100644 index 00000000..fe16a513 --- /dev/null +++ b/openspec/changes/persistent-aws-creds/specs/endpoint-provisioning/spec.md @@ -0,0 +1,45 @@ +## MODIFIED Requirements + +### Requirement: Bootstrap deploys the control plane + +The system SHALL provide `spinloop remote bootstrap`, which deploys the +account-level control plane that every remote environment reuses — the EC2 +Image Builder pipelines, the environment-aware lifecycle Lambdas and their IAM, +and the shared S3 weights bucket, IAM roles and VPC, and the IAM user that +holds the long-lived control-plane credential (see the Remote Auth +specification) together with its policy — by obtaining the CDK project shipped +in `remote/` and driving its deploy of the control-plane stack. Bootstrap SHALL +NOT start any AMI bake; the bake is a separate +`spinloop remote bake` step. Bootstrap SHALL NOT create any Elastic IP or EC2 +instance, and SHALL NOT register an environment; those belong to +`spinloop remote deploy`. Bootstrap SHALL NOT reimplement the infrastructure; +it SHALL orchestrate the existing CDK project. On success, bootstrap SHALL +signpost `spinloop remote bake` as the next step, ahead of +`spinloop remote deploy`. + +#### Scenario: A successful bootstrap yields the control plane + +- **WHEN** `spinloop remote bootstrap` completes +- **THEN** the control-plane stack is deployed — Image Builder pipelines, the + lifecycle Lambdas, and the shared bucket/roles/VPC — with no Elastic IP or + instance created and no AMI bake started + +#### Scenario: The control-plane credential user is deployed + +- **WHEN** `spinloop remote bootstrap` completes +- **THEN** the control-plane IAM user exists with a policy covering the + day-to-day remote commands only — invoking the control URLs, reading the + control-plane log groups, describing the control-plane stack, and managing + its own access keys — and no permission to deploy, bake, or otherwise + provision AWS resources + +#### Scenario: Bootstrap signposts the bake + +- **WHEN** `spinloop remote bootstrap` completes +- **THEN** its output names `spinloop remote bake` as the next step, ahead of + `spinloop remote deploy` + +#### Scenario: Orchestration stops on a failed step + +- **WHEN** any step in the sequence fails +- **THEN** bootstrap stops and reports which step failed rather than continuing diff --git a/openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md b/openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md new file mode 100644 index 00000000..5c7d9df4 --- /dev/null +++ b/openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md @@ -0,0 +1,97 @@ +## Purpose + +Define how `spinloop remote auth` stores a long-lived control-plane AWS credential in the OS keystore, how that credential resolves in preference to other sources, and how it is reported, rotated, and cleared. + +## ADDED Requirements + +### Requirement: Storing a control-plane credential + +`spinloop remote auth --store` SHALL create an AWS access key for the control-plane IAM user created by the control-plane stack and store it in the OS keystore (Keychain, Credential Manager, or Secret Service), keyed by the target region. The stored entry SHALL record the access key id, the secret, the AWS account, the user name, the region, and when it was stored. Before storing, the command SHALL verify that the new key resolves to the same AWS account as the caller's credentials, and SHALL NOT store a key that resolves to a different account. + +When a credential is already stored for the region, `--store` SHALL rotate rather than create a second entry: it SHALL create the replacement key with the stored credential itself, so that no administrator or other ambient credential is required, verify it, replace the stored entry, and delete the superseded access key on the AWS side. + +When the control-plane IAM user does not exist — a control plane deployed before this capability — the command SHALL fail, naming `spinloop remote bootstrap` as the step to re-run first. + +The secret SHALL never be printed in any output. Where no OS keystore is available on the machine, the entry MAY instead be stored in an owner-only file under the user's spinloop config directory, and the command SHALL say which store it used. + +#### Scenario: First store + +- **WHEN** the user runs `spinloop remote auth --store` in an account with a bootstrapped control plane and no stored credential for the region +- **THEN** an access key is created for the control-plane user and stored in the OS keystore for that region, and the confirmation names the account, region, and access key id without printing the secret + +#### Scenario: A key that resolves elsewhere is not stored + +- **WHEN** the newly created key resolves to a different AWS account than the caller's credentials +- **THEN** the key is not stored and the command fails saying so + +#### Scenario: Rotation needs no administrator credential + +- **WHEN** a credential is already stored for the region, no other AWS credential is configured, and the user runs `spinloop remote auth --store` +- **THEN** the stored credential is used to create the replacement key, the stored entry is swapped to the new key, and the superseded access key is deleted on the AWS side + +#### Scenario: A control plane without the user + +- **WHEN** the user runs `spinloop remote auth --store` against a control plane deployed before the control-plane user existed +- **THEN** the command fails, naming `spinloop remote bootstrap` as the step to re-run first + +#### Scenario: No keystore on the machine + +- **WHEN** the user runs `spinloop remote auth --store` on a machine with no OS keystore available +- **THEN** the entry is stored in an owner-only file under the user's spinloop config directory instead, and the report says where + +### Requirement: Stored credentials resolve for control calls + +For any `spinloop remote` subcommand that resolves AWS credentials for a target region, a stored credential for that region SHALL be used when no explicit AWS environment credentials and no explicit profile selection are present, and SHALL take precedence over the remaining standard credential sources — shared config files, SSO sessions, and instance metadata. Explicit AWS environment credentials (access key id, secret, and session token set in the process environment) or an explicit profile selection SHALL override the stored credential. When no credential is stored for the region, resolution SHALL fall back to the standard credential chain as before this capability. + +`spinloop remote bootstrap` and `spinloop remote bake` SHALL NOT consult a stored credential: they provision the control plane itself and SHALL resolve from ambient sources only. + +#### Scenario: The stored key signs between log-ins + +- **WHEN** a credential is stored for the region, no AWS environment credential or profile is set, and the ambient SSO session is absent or expired +- **THEN** `spinloop remote status` signs with the stored credential and succeeds + +#### Scenario: Explicit environment credentials win + +- **WHEN** an AWS access key id and secret are set in the process environment and a credential is also stored for the region +- **THEN** the command signs with the environment credentials, not the stored one + +#### Scenario: A named profile wins + +- **WHEN** an explicit profile is selected and a credential is stored for the region +- **THEN** the command signs with the profile's credentials, not the stored one + +#### Scenario: No stored key falls back as before + +- **WHEN** no credential is stored for the region +- **THEN** credential resolution behaves exactly as it did before this capability + +#### Scenario: Bootstrap ignores the stored key + +- **WHEN** a credential is stored for the region and the user runs `spinloop remote bootstrap` +- **THEN** bootstrap resolves its credentials from ambient sources only, not from the stored credential + +### Requirement: Reporting and clearing stored credentials + +`spinloop remote auth` with no flag SHALL report every stored credential — the account, region, user name, access key id, and when it was stored — without contacting AWS and without printing the secret. When nothing is stored, it SHALL say so and name `--store` as the way to store one. + +`spinloop remote auth --clear` SHALL remove the stored credential for the target region and SHALL additionally delete the access key on the AWS side, using the stored credential, so that a cleared key does not linger in the account. If the AWS-side deletion cannot be made, the local entry SHALL still be removed and the failure reported, with a note that the key may still exist on the AWS side. + +#### Scenario: Status reports what is stored + +- **WHEN** a credential is stored and the user runs `spinloop remote auth` +- **THEN** the output lists the account, region, user name, access key id, and when it was stored, no AWS call is made, and the secret is not printed + +#### Scenario: Status with nothing stored + +- **WHEN** no credential is stored and the user runs `spinloop remote auth` +- **THEN** the output says none is stored and names `--store` as the way to store one + +#### Scenario: Clear removes both sides + +- **WHEN** the user runs `spinloop remote auth --clear` and a credential is stored +- **THEN** the stored entry is removed and the access key is deleted on the AWS side + +#### Scenario: Clear still removes locally when the AWS deletion fails + +- **WHEN** the user runs `spinloop remote auth --clear` and the AWS-side deletion fails +- **THEN** the local entry is still removed, and the command reports that the key may still exist on the AWS side diff --git a/openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md b/openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md new file mode 100644 index 00000000..d7903dac --- /dev/null +++ b/openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md @@ -0,0 +1,188 @@ +## MODIFIED Requirements + +### Requirement: Remote command group + +The system SHALL provide a `remote` command group with the subcommands +`bootstrap`, `bake`, `auth`, `start`, `stop`, `restart`, `status`, `deploy`, +`ls`, `metrics`, and `keep`. `start`, `stop`, `restart`, `status`, `metrics` and +`deploy` each take an optional Spinloop path: +`start` SHALL boot the endpoint and block until it is serving, then perform a +quick TCP probe of the inference endpoint — if the probe fails, a warning is +printed to stderr explaining the network mismatch (see the Remote Start Probe +specification) — and finally print the base URL and API key as shell exports; +`start` SHALL also accept a `--keep DURATION` flag that sets the instance +retention deadline to `now + DURATION`, preventing the idle sweep from +terminating it before that time (see the Remote Keep specification); +`stop` SHALL stop it immediately rather than waiting for its idle timer; +`restart` SHALL stop the endpoint in the manner of a pause — without +terminating it, so its boot disk, its weights and its stable address are +preserved — and SHALL immediately start it again, blocking until it is serving +and reporting progress as `start` does (see the Reporting a start in progress +specification); `restart` SHALL accept a `--force` flag with a `-F` short form +that, when set, performs the stop without first asking the engine to shut down +(see the Endpoint Lifecycle specification for forced stops); +`status` SHALL report instance state and endpoint health without side effects +and SHALL NOT perform any TCP probe, and SHALL include the `Retain-Until` +deadline when the instance has an active retention tag; +`keep` SHALL set the `Retain-Until` tag on the environment's instance for the +given duration, without starting or stopping the instance (see the Remote Keep +specification); `metrics` SHALL report instance state, token usage, resource +consumption, and GPU information for a running instance; `deploy` SHALL set +what the endpoint serves. `ls` SHALL list the registered remote environments +(see the Remote Environments specification). `bootstrap` SHALL stand up the +account-level AWS control plane (once per account) by obtaining and driving the +CDK project, and takes its own flags rather than a Spinloop path (see the +Endpoint Provisioning specification). `bake` SHALL start an AMI bake for each +runner named, and takes runner names rather than a Spinloop path (see the +Endpoint Provisioning specification). `auth` SHALL store, report, and clear the +long-lived control-plane credential, and takes its own flags rather than a +Spinloop path (see the Remote Auth specification). An unrecognised subcommand +SHALL fail naming the accepted ones. + +#### Scenario: Starting the endpoint + +- **WHEN** the user runs `spinloop remote start` and the endpoint reports ready +- **THEN** the base URL and API key are printed as `export` lines + +#### Scenario: Starting warns when the network is not admitted + +- **WHEN** the user runs `spinloop remote start` and the endpoint reports ready + but the TCP probe to the inference port fails +- **THEN** a warning is printed to stderr with a remediation command, and the + command still exits 0 + +#### Scenario: Starting with a keep flag + +- **WHEN** the user runs `spinloop remote start --keep 4h` and the endpoint reports ready +- **THEN** the base URL and API key are printed as `export` lines, and the + instance retention deadline is set to 4 hours from now + +#### Scenario: Waiting through a cold start + +- **WHEN** the endpoint reports that it is still starting +- **THEN** the command waits and retries until it is ready or the timeout + passes, rather than failing on the first attempt + +#### Scenario: Restarting the endpoint + +- **WHEN** the user runs `spinloop remote restart` for a running environment and + the endpoint reports ready again +- **THEN** the instance was stopped and re-woken without being terminated, the + command blocked until the model was serving again, and the environment's + address is the one its configuration records + +#### Scenario: Forcing a restart skips the engine stop + +- **WHEN** the user runs `spinloop remote restart --force` (or `-F`) +- **THEN** the instance is stopped without the engine being asked to shut down + first, and the command then blocks until the model is serving again + +#### Scenario: Restarting a stopped endpoint starts it + +- **WHEN** the user runs `spinloop remote restart` for an environment whose instance is already stopped +- **THEN** the instance is re-woken rather than replaced, and the command blocks + until the model is serving again, as with a plain start + +#### Scenario: A failed re-wake says how to recover + +- **WHEN** the stop half of a restart has taken effect but the wake fails +- **THEN** the command fails saying the instance is stopped and that + `spinloop remote start` will bring it back + +#### Scenario: Listing environments + +- **WHEN** the user runs `spinloop remote ls` +- **THEN** the registered environments are listed rather than any endpoint being + contacted + +#### Scenario: Setting a keep deadline + +- **WHEN** the user runs `spinloop remote keep 2h` +- **THEN** the instance retention tag is set and the deadline is reported + +#### Scenario: Metrics reports instance figures + +- **WHEN** the user runs `spinloop remote metrics` with a running instance +- **THEN** token counts, resource usage, and GPU information are displayed + +#### Scenario: Bootstrap is a recognised subcommand + +- **WHEN** the user runs `spinloop remote bootstrap` +- **THEN** the command is dispatched to the provisioning flow rather than + reported as unknown + +#### Scenario: Bake is a recognised subcommand + +- **WHEN** the user runs `spinloop remote bake llamacpp` +- **THEN** the command is dispatched to the bake flow rather than + reported as unknown + +#### Scenario: Auth is a recognised subcommand + +- **WHEN** the user runs `spinloop remote auth` +- **THEN** the command is dispatched to the credential store, report, and clear + flow rather than reported as unknown + +#### Scenario: Unknown subcommand + +- **WHEN** the user runs `spinloop remote frobnicate` +- **THEN** the command fails listing the accepted subcommands, which include + `bootstrap`, `bake`, `metrics`, and `keep` + +### Requirement: Authenticated control requests + +Requests to the control URLs SHALL be signed with the caller's own AWS +credentials, resolved in this order: explicit AWS environment credentials or an +explicit profile selection, then a stored control-plane credential for the +target region (see the Remote Auth specification), then the remaining standard +credential sources — shared config files, SSO sessions, and instance metadata. +Requests SHALL carry the hash of the request body so that a request with a +payload is signed over that payload. The only credentials Spinloop stores of its +own are the stored control-plane credentials, held in the OS keystore (or, +where no keystore exists, an owner-only file under the user's config directory) +and created or removed by `spinloop remote auth` (see the Remote Auth +specification). + +Every control subcommand — `start`, `stop`, `status`, `deploy`, and `metrics` — +SHALL treat a non-success reply from the control endpoint as a failure: it SHALL +return an error and a non-zero exit, and SHALL NOT print an empty or partial +result as though the call succeeded. + +A rejected request SHALL be reported with an actionable cause. When the request +is rejected because the caller's AWS credentials are expired or invalid, the +command SHALL say to refresh them (env credentials, a profile, or an SSO +session; `spinloop remote auth --store` where a stored credential was in use), +distinct from the case where the credentials are resolvable but may lack +permission to invoke the endpoint. + +#### Scenario: A request carrying a body is signed over it + +- **WHEN** `spinloop remote deploy` sends a configuration +- **THEN** the request is signed including the body's hash, not as an empty + payload + +#### Scenario: Credentials are missing + +- **WHEN** no AWS credentials can be resolved +- **THEN** the command fails saying how to configure them + +#### Scenario: Credentials are expired + +- **WHEN** `spinloop remote status` runs with expired or invalid AWS credentials + and the control endpoint rejects the signed request +- **THEN** the command fails with a non-zero exit and a message saying to + refresh the AWS credentials, rather than printing a blank state + +#### Scenario: The endpoint rejects a control request + +- **WHEN** any control subcommand receives a non-success HTTP reply from the + control endpoint +- **THEN** the command reports the failure with its status and cause, and does + not present the empty reply as a successful result + +#### Scenario: The stored credential signs when the ambient chain has none + +- **WHEN** a control-plane credential is stored for the region, no AWS + environment credential or profile is set, and no other standard-chain source + is available +- **THEN** the control request is signed with the stored credential diff --git a/openspec/changes/persistent-aws-creds/tasks.md b/openspec/changes/persistent-aws-creds/tasks.md new file mode 100644 index 00000000..8aae2549 --- /dev/null +++ b/openspec/changes/persistent-aws-creds/tasks.md @@ -0,0 +1,34 @@ +## 1. Keystore entry and storage backend + +- [ ] 1.1 Add the stored-credential entry type (access key id, secret, account, user name, region, stored-at, store used) and the keyring operations (put, get, delete, list) in `internal/remote`, keyed by region with a `spinloop-remote-` name, backed by `99designs/keyring` with an owner-only file fallback (`/keystore/remote-.json`, 0600 in a 0700 dir) where no OS keystore exists; verify with unit tests covering put/get/delete/list on a fake keyring, region keying, and the file fallback's modes (go test ./internal/remote) +- [ ] 1.2 Add the `99designs/keyring` dependency to go.mod; verify `go mod tidy` and `go build ./...` succeed + +## 2. Credential resolution at the choke points + +- [ ] 2.1 Split `LoadAWSConfig` (`internal/remote/aws.go`) into an ambient-only loader (today's body) and a loader that applies the precedence: explicit AWS env credentials or explicit profile selection → default chain as today; else a stored entry for the region → default config with a static credentials provider for it; else default chain; verify with unit tests for the full precedence matrix, including env-over-stored, profile-over-stored, stored-over-SSO/shared-config, and no-stored-falls-back +- [ ] 2.2 Make the expired-credential hint in `sign()` (`internal/remote/remote.go`) source-aware: when the stored key was the credential in use, the refresh guidance says `spinloop remote auth --store`; verify with unit tests on both hint variants +- [ ] 2.3 Point `bootstrap` and `bake` credential preflights (`cmd/spinloop/remote_bootstrap.go`) at the ambient-only loader so they never consult the keystore; verify with a unit test that a stored entry is not used by the bootstrap preflight and that the existing preflight tests still pass + +## 3. CDK: control-plane IAM user and policy + +- [ ] 3.1 Add the `cloud-vm-llm-remote-cli` IAM user to `remote/lib/llm-stack.ts` with an inline policy granting: `lambda:InvokeFunctionUrl` on the seven function-URL ARNs, `logs:DescribeLogStreams`/`FilterLogEvents`/`GetLogEvents` on the runner and boot log-group ARNs, `cloudformation:DescribeStacks` on the stack ARN, `pricing:GetProducts`, and `iam:GetUser`/`ListAccessKeys`/`CreateAccessKey`/`DeleteAccessKey` on the user's own ARN; verify by extending `remote/test/stack.test.ts` to assert the user, each grant, and the absence of any provisioning permission (no cloudformation:CreateStack, no imagebuilder, no ec2:RunInstances) and running `pnpm test` in `remote/` +- [ ] 3.2 Confirm the fixed user name introduces no deployment identifier; verify `scripts/check-no-cloud-identifiers.sh` passes + +## 4. The `remote auth` command + +- [ ] 4.1 Register `auth` under the `remote` command group in `cmd/spinloop/remote_auth.go` with mutually exclusive `--store`/`--clear` flags and a `--region` flag resolved like bootstrap's; verify `spinloop remote auth --help` renders per the cli-ux conventions and that `spinloop remote --help` lists `auth` +- [ ] 4.2 Implement `--store` first-store: load ambient credentials only, confirm the control-plane user with `iam:GetUser` (absent → fail naming `spinloop remote bootstrap`), create the access key, verify it with STS `GetCallerIdentity` resolves to the caller's account (mismatch → not stored, fail), write the entry, confirm with account/region/user/key-id/store and never the secret; verify with unit tests through seams for the happy path, the missing-user path, and the account-mismatch path +- [ ] 4.3 Implement `--store` rotation: when an entry exists for the region, create the replacement with the stored credential (separate IAM client, no ambient credential required), swap the entry, and delete the superseded key; check IAM's two-keys-per-user cap when not rotating and fail naming the fix; verify with unit tests including rotation with no ambient credentials configured +- [ ] 4.4 Implement `--clear`: look up the entry (none → say so, exit 0), delete the access key on the AWS side with the stored credential best-effort (on failure still remove locally and report the key may linger), remove the local entry; verify with unit tests for the happy path and the AWS-deletion-failure path +- [ ] 4.5 Implement the no-flag report: list every stored entry (account, region, user, access key id, stored-at, store) from the local store only, no AWS call, no secret; nothing stored → say so and name `--store`; verify with unit tests for both cases +- [ ] 4.6 Add `auth` to tab completion where the `remote` subcommands are completed (`cmd/spinloop/complete.go`); verify the existing completion tests pass and `__complete` never errors + +## 5. Docs + +- [ ] 5.1 Update `docs/commands/remote.md`: an `auth` section (store, report, clear, rotate) and the credentials section — stored key with ambient as fallback, explicit env/profile override, bootstrap/bake stay admin-only; verify the claims match the implemented behaviour by reading the final code +- [ ] 5.2 Update `README.md` (the credentials paragraph) and `remote/README.md` (prerequisites: bootstrap still admin; day-to-day commands can use the stored key; existing control planes need a re-bootstrap before `--store`; rotate roughly every 90 days); verify by reading that no claim contradicts the implementation or the identifier check + +## 6. Verification + +- [ ] 6.1 Run the full suite with coverage and the linters; verify `go test ./... -cover` keeps total coverage >= 80%, `go vet ./...` and `gofmt -l .` are clean, and `pnpm test` passes in `remote/` +- [ ] 6.2 End-to-end against a real account: re-run `spinloop remote bootstrap`, run `spinloop remote auth --store`, log out of SSO, and verify `spinloop remote status` succeeds with the stored key, that `spinloop remote auth` reports the entry, that `--store` again rotates without admin credentials, and that `--clear` removes both the entry and the AWS-side key From bf9b7c55122bdd42fe9ddee4fba6699207d2834e Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Tue, 8 Sep 2026 21:13:01 +0100 Subject: [PATCH 2/2] feat(remote): add auth --store for a persistent control-plane credential The control plane stack now creates an IAM user with a stack-owned managed policy scoped to day-to-day control, and spinloop remote auth --store keeps an access key for it in the OS keystore (an owner-only file where no keystore is reachable). Resolution is per region: explicit environment credentials or a profile first, then the stored key, then the standard chain. Control planes deployed before this change need a re-bootstrap to gain the user. --- README.md | 13 +- cmd/spinloop/commands.go | 1 + cmd/spinloop/remote_auth.go | 285 +++++++++++ cmd/spinloop/remote_auth_test.go | 471 ++++++++++++++++++ cmd/spinloop/remote_bootstrap.go | 8 +- cmd/spinloop/remote_bootstrap_test.go | 19 + docs/commands/remote.md | 73 ++- docs/env-vars.md | 3 +- docs/internals.md | 9 +- go.mod | 8 +- go.sum | 18 +- internal/fleet/remote_node_test.go | 50 ++ internal/remote/aws.go | 145 +++++- internal/remote/aws_test.go | 167 +++++++ internal/remote/keystore.go | 314 ++++++++++++ internal/remote/keystore_test.go | 270 ++++++++++ internal/remote/logs.go | 15 +- internal/remote/logs_test.go | 22 +- internal/remote/remote.go | 55 +- internal/remote/remote_test.go | 66 +++ internal/remote/seed.go | 2 +- .../.openspec.yaml | 0 .../design.md | 35 +- .../proposal.md | 6 +- .../specs/endpoint-provisioning/spec.md | 0 .../specs/remote-auth/spec.md | 14 +- .../specs/remote-endpoint/spec.md | 3 + .../2026-09-08-persistent-aws-creds/tasks.md | 37 ++ .../changes/persistent-aws-creds/tasks.md | 34 -- openspec/specs/endpoint-provisioning/spec.md | 17 +- openspec/specs/remote-auth/spec.md | 109 ++++ openspec/specs/remote-endpoint/spec.md | 48 +- remote/README.md | 16 +- remote/lib/llm-stack.ts | 68 +++ remote/test/stack.test.ts | 144 ++++++ remote/vitest.config.ts | 5 +- 36 files changed, 2416 insertions(+), 134 deletions(-) create mode 100644 cmd/spinloop/remote_auth.go create mode 100644 cmd/spinloop/remote_auth_test.go create mode 100644 internal/remote/keystore.go create mode 100644 internal/remote/keystore_test.go rename openspec/changes/{persistent-aws-creds => archive/2026-09-08-persistent-aws-creds}/.openspec.yaml (100%) rename openspec/changes/{persistent-aws-creds => archive/2026-09-08-persistent-aws-creds}/design.md (56%) rename openspec/changes/{persistent-aws-creds => archive/2026-09-08-persistent-aws-creds}/proposal.md (71%) rename openspec/changes/{persistent-aws-creds => archive/2026-09-08-persistent-aws-creds}/specs/endpoint-provisioning/spec.md (100%) rename openspec/changes/{persistent-aws-creds => archive/2026-09-08-persistent-aws-creds}/specs/remote-auth/spec.md (76%) rename openspec/changes/{persistent-aws-creds => archive/2026-09-08-persistent-aws-creds}/specs/remote-endpoint/spec.md (97%) create mode 100644 openspec/changes/archive/2026-09-08-persistent-aws-creds/tasks.md delete mode 100644 openspec/changes/persistent-aws-creds/tasks.md create mode 100644 openspec/specs/remote-auth/spec.md diff --git a/README.md b/README.md index 20fff4ad..847709a7 100644 --- a/README.md +++ b/README.md @@ -643,11 +643,14 @@ the environment; deploying [`remote/`](remote/) yourself prints the same values: Spinloop wins if you do set one. Every URL and the region can be overridden with the matching -[`SPINLOOP_REMOTE_*`](docs/env-vars.md) environment variable. The commands use -your AWS credentials (environment, profile or SSO — the standard chain), which -need `lambda:InvokeFunctionUrl` allowed. A cold `start` takes a few minutes -while the instance boots and loads the model; `--timeout` (default 15m) caps -the wait. +[`SPINLOOP_REMOTE_*`](docs/env-vars.md) environment variable. The commands +sign with an AWS credential resolved per region: explicit environment +credentials or a named profile first, then the stored control-plane credential +from [`spinloop remote auth --store`](docs/commands/remote.md#credentials), +then the standard chain (config files, SSO sessions, instance metadata). The +credential needs `lambda:InvokeFunctionUrl` allowed. A cold `start` takes a +few minutes while the instance boots and loads the model; `--timeout` +(default 15m) caps the wait. The AWS credentials, region and `SPINLOOP_REMOTE_*` overrides can all travel with the Spinloop, in the `.env` beside it. A value already set in your shell wins over the `.env`. To pin a value diff --git a/cmd/spinloop/commands.go b/cmd/spinloop/commands.go index e54217d8..6998c804 100644 --- a/cmd/spinloop/commands.go +++ b/cmd/spinloop/commands.go @@ -353,6 +353,7 @@ names a file — falling back to the default environment. Each subcommand's } remote.AddCommand( remoteBootstrapCmd(), + remoteAuthCmd(), remoteBakeCmd(), remoteStartCmd(), remotePauseCmd(), diff --git a/cmd/spinloop/remote_auth.go b/cmd/spinloop/remote_auth.go new file mode 100644 index 00000000..c358b026 --- /dev/null +++ b/cmd/spinloop/remote_auth.go @@ -0,0 +1,285 @@ +package main + +import ( + "context" + "fmt" + "os" + "os/signal" + "strings" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/spf13/cobra" + "github.com/spinloop-ai/spinloop/internal/remote" +) + +// Seams: package variables so tests drive the auth flow without AWS. The IAM +// and STS functions take the config they resolve from, so a test passes an +// ambient config for a first store and a stored-credential config for a +// rotation, and records which credential each call resolved with. +var ( + iamUserExistsFn = remote.IAMUserExists + iamUserAccessKeysFn = remote.IAMUserAccessKeyIDs + iamCreateAccessKeyFn = remote.IAMCreateAccessKey + iamDeleteAccessKeyFn = remote.IAMDeleteAccessKey + authCallerIdentityFn = remote.CallerIdentity +) + +// remoteAuthCmd is `spinloop remote auth`: store, report, or clear the +// long-lived control-plane credential this machine signs day-to-day commands +// with. With no flag it reports what is stored, from the local store only. +func remoteAuthCmd() *cobra.Command { + var ( + store bool + clear bool + region string + ) + c := &cobra.Command{ + Use: "auth", + Short: "store, report, or clear the control-plane credential", + Long: `stores a long-lived control-plane credential in this machine's OS +keystore (keychain, credential manager, or secret service) so the day-to-day +remote commands sign without a fresh SSO log-in. With no flag it reports what +is stored; --store stores the credential for a region, rotating it when one +is already stored; --clear removes it and deletes the access key.`, + Args: cobra.ArbitraryArgs, + SilenceErrors: true, + SilenceUsage: true, + ValidArgsFunction: noPositionals, + RunE: func(c *cobra.Command, _ []string) error { + resolve(c) + return runRemoteAuth(store, clear, region) + }, + } + fs := c.Flags() + fs.BoolVar(&store, "store", false, "store the credential for the region, rotating it when one is stored") + fs.BoolVar(&clear, "clear", false, "remove the stored credential and delete its access key") + fs.StringVar(®ion, "region", "", "AWS region (default: AWS_REGION or us-east-1)") + c.MarkFlagsMutuallyExclusive("store", "clear") + fs.SetInterspersed(false) + return c +} + +// runRemoteAuth is the body of `spinloop remote auth`. +func runRemoteAuth(store, clear bool, regionFlag string) error { + region := resolveRegion(regionFlag) + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) + defer stop() + switch { + case clear: + return runRemoteAuthClear(ctx, region) + case store: + return runRemoteAuthStore(ctx, region) + default: + return runRemoteAuthReport() + } +} + +// runRemoteAuthReport lists every stored credential from the local store. It +// makes no AWS call: the report is this machine's own book, and it must work +// on a machine whose credentials are expired or absent — which is the point +// of the stored key. +func runRemoteAuthReport() error { + creds, err := remote.ListStoredCredentials() + if err != nil { + return fmt.Errorf("reading the stored credentials: %w", err) + } + if len(creds) == 0 { + fmt.Println("No stored credential. Store one with `spinloop remote auth --store`.") + return nil + } + w := os.Stdout + fmt.Fprintln(w, "region\taccount\tuser\tkey id\tstored at\tstore") + for _, c := range creds { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n", + c.Region, c.Account, c.User, c.AccessKeyID, c.StoredAt.UTC().Format(time.RFC3339), c.Store) + } + return nil +} + +// runRemoteAuthStore stores a credential for the region. With nothing stored +// it is a first store: it runs on the caller's ambient credentials, which are +// the administrator's, and verifies the new key against the caller's account. +// With an entry already stored it is a rotation: it runs on the stored +// credential alone — no administrator or other ambient credential required — +// and deletes the superseded key on the AWS side once the new one is verified +// and swapped in. +func runRemoteAuthStore(ctx context.Context, region string) error { + existing, rotating := remote.LookupStoredCredential(region) + + var cfg aws.Config + var expectedAccount string + if rotating { + cfg = remote.ConfigFromStored(existing) + expectedAccount = existing.Account + } else { + var err error + cfg, err = loadCreds(ctx, region) + if err != nil { + return fmt.Errorf("resolving AWS credentials: %w (configure env credentials, a profile or an SSO session)", err) + } + var err2 error + expectedAccount, err2 = authCallerIdentityFn(ctx, cfg) + if err2 != nil { + return fmt.Errorf("confirming the AWS account: %w", err2) + } + } + + exists, err := iamUserExistsFn(ctx, cfg, remote.ControlPlaneUserName) + if err != nil { + return fmt.Errorf("checking the control-plane user: %w", err) + } + if !exists { + return fmt.Errorf("the control-plane user %q does not exist in this account — re-run `spinloop remote bootstrap` to create it", + remote.ControlPlaneUserName) + } + + if !rotating { + // IAM allows two access keys per user. A first store would be the + // third, so check before creating rather than after a failure. + keys, err := iamUserAccessKeysFn(ctx, cfg, remote.ControlPlaneUserName) + if err != nil { + return fmt.Errorf("listing the control-plane user's access keys: %w", err) + } + if len(keys) >= 2 { + return fmt.Errorf("the control-plane user already has two access keys — delete one and run `spinloop remote auth --store` again") + } + } + + keyID, secret, err := iamCreateAccessKeyFn(ctx, cfg, remote.ControlPlaneUserName) + if err != nil { + return fmt.Errorf("creating the access key: %w", err) + } + + // Verify the new key resolves to the expected account before anything is + // stored. A key that cannot be verified, or that resolves elsewhere, is + // deleted on the AWS side rather than left behind. + probe := remote.ConfigFromStored(remote.StoredCredential{AccessKeyID: keyID, SecretAccessKey: secret, Region: region}) + account, err := verifyNewKey(ctx, probe) + if err != nil { + deleteStrayKey(ctx, cfg, keyID) + return fmt.Errorf("verifying the new access key: %w", err) + } + if account != expectedAccount { + deleteStrayKey(ctx, cfg, keyID) + return fmt.Errorf("the new access key resolves to account %s, not %s — it was not stored and has been deleted on the AWS side", + account, expectedAccount) + } + + cred := remote.StoredCredential{ + AccessKeyID: keyID, + SecretAccessKey: secret, + Account: account, + User: remote.ControlPlaneUserName, + Region: region, + StoredAt: time.Now().UTC(), + } + if err := remote.StoreCredential(cred); err != nil { + return fmt.Errorf("storing the credential: %w", err) + } + // Read the entry back: it carries the store kind the write recorded, and + // a store that cannot hand back what it was given has not stored it. + cred, ok := remote.LookupStoredCredential(region) + if !ok { + return fmt.Errorf("the credential was stored but cannot be read back") + } + + w := os.Stderr + fmt.Fprintf(w, "Stored the control-plane credential for %s:\n", region) + fmt.Fprintf(w, " Account: %s\n", account) + fmt.Fprintf(w, " Region: %s\n", region) + fmt.Fprintf(w, " User: %s\n", remote.ControlPlaneUserName) + fmt.Fprintf(w, " Key id: %s\n", keyID) + fmt.Fprintf(w, " Store: %s\n", cred.Store) + if rotating { + if err := iamDeleteAccessKeyFn(ctx, probe, remote.ControlPlaneUserName, existing.AccessKeyID); err != nil { + fmt.Fprintf(w, "The superseded key %s could not be deleted on the AWS side (%v) — it may still exist.\n", + existing.AccessKeyID, err) + } else { + fmt.Fprintf(w, "The superseded key %s was deleted on the AWS side.\n", existing.AccessKeyID) + } + } + return nil +} + +// verifyAttempts is how many times the identity probe runs while verifying a +// freshly created key. +const verifyAttempts = 6 + +// verifyProbeBackoff is the wait before the first retry of a verification +// that has not succeeded yet; each later retry waits twice as long, up to +// verifyProbeMaxBackoff. Tests set it to zero, which zeroes every wait. +var verifyProbeBackoff = time.Second + +// verifyProbeMaxBackoff caps the exponential growth, so the whole retry +// window stays around half a minute. +const verifyProbeMaxBackoff = 16 * time.Second + +// verifyNewKey resolves the account of the freshly created key. It retries +// while STS rejects the key id itself: after IAM issues a key, STS can lag +// several seconds before it resolves the key, and in that window every call +// reports the key as an invalid client token. Any other failure returns at +// once — the caller deletes the key, as with a verification that never +// succeeds. +func verifyNewKey(ctx context.Context, probe aws.Config) (string, error) { + backoff := verifyProbeBackoff + for attempt := 1; ; attempt++ { + account, err := authCallerIdentityFn(ctx, probe) + if err == nil || !invalidClientToken(err) || attempt >= verifyAttempts { + return account, err + } + fmt.Fprintf(os.Stderr, "The new access key is not resolvable yet — retry %d of %d...\n", attempt, verifyAttempts-1) + select { + case <-ctx.Done(): + return "", ctx.Err() + case <-time.After(backoff): + } + backoff *= 2 + if backoff > verifyProbeMaxBackoff { + backoff = verifyProbeMaxBackoff + } + } +} + +// invalidClientToken reports whether the error is STS rejecting the access +// key id itself — the shape of the failure while a freshly issued key is +// still propagating. The pinned SDK version has no typed error for it, so +// the match is on the API error code in the message. +func invalidClientToken(err error) bool { + return err != nil && strings.Contains(err.Error(), "InvalidClientTokenId") +} + +// deleteStrayKey removes an access key this command created and then failed +// to verify, so a failed store leaves no live key on the AWS side. A failure +// here is reported, not returned: the store has already failed. +func deleteStrayKey(ctx context.Context, cfg aws.Config, keyID string) { + if err := iamDeleteAccessKeyFn(ctx, cfg, remote.ControlPlaneUserName, keyID); err != nil { + fmt.Fprintf(os.Stderr, "The new key %s could not be deleted on the AWS side (%v) — delete it manually.\n", keyID, err) + } +} + +// runRemoteAuthClear removes the stored credential for the region and deletes +// its access key on the AWS side, using the stored credential, so a cleared +// key does not linger in the account. If the AWS-side deletion cannot be +// made, the local entry is still removed and the failure reported. +func runRemoteAuthClear(ctx context.Context, region string) error { + cred, ok := remote.LookupStoredCredential(region) + if !ok { + fmt.Printf("No stored credential for %s.\n", region) + return nil + } + cfg := remote.ConfigFromStored(cred) + deleteErr := iamDeleteAccessKeyFn(ctx, cfg, cred.User, cred.AccessKeyID) + if err := remote.DeleteStoredCredential(region); err != nil { + return fmt.Errorf("removing the stored credential: %w", err) + } + w := os.Stderr + if deleteErr != nil { + fmt.Fprintf(w, "Removed the stored credential for %s, but deleting its access key %s on the AWS side failed (%v) — the key may still exist.\n", + region, cred.AccessKeyID, deleteErr) + } else { + fmt.Fprintf(w, "Removed the stored credential for %s and deleted its access key %s on the AWS side.\n", + region, cred.AccessKeyID) + } + return nil +} diff --git a/cmd/spinloop/remote_auth_test.go b/cmd/spinloop/remote_auth_test.go new file mode 100644 index 00000000..47000557 --- /dev/null +++ b/cmd/spinloop/remote_auth_test.go @@ -0,0 +1,471 @@ +package main + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/spinloop-ai/spinloop/internal/remote" +) + +// authSeamState is what the stubbed IAM/STS seams answer with and record. +type authSeamState struct { + userExists bool + existsErr error + + accessKeys []string + accessErr error + + newKeyID string + newSecret string + createErr error + createdFor string // the user name the create ran for + + deleteErr error + deleted []string // the key ids delete ran for + + account string // default account for identity calls + accounts map[string]string // per resolved access key id, when a key resolves elsewhere + identity []string // the access key id each identity call resolved with + identityErrFor string // the access key id whose identity calls return identityErrs + identityErrs []error // returned in order, then the call answers as usual + existsKey string // the access key id the user check resolved with +} + +func stubAuthSeams(t *testing.T) *authSeamState { + t.Helper() + state := &authSeamState{accounts: map[string]string{}} + + origExists, origKeys := iamUserExistsFn, iamUserAccessKeysFn + origCreate, origDelete := iamCreateAccessKeyFn, iamDeleteAccessKeyFn + origIdentity := authCallerIdentityFn + origBackoff := verifyProbeBackoff + verifyProbeBackoff = 0 + t.Cleanup(func() { + iamUserExistsFn, iamUserAccessKeysFn = origExists, origKeys + iamCreateAccessKeyFn, iamDeleteAccessKeyFn = origCreate, origDelete + authCallerIdentityFn = origIdentity + verifyProbeBackoff = origBackoff + }) + + iamUserExistsFn = func(ctx context.Context, cfg aws.Config, _ string) (bool, error) { + if creds, err := cfg.Credentials.Retrieve(ctx); err == nil { + state.existsKey = creds.AccessKeyID + } + return state.userExists, state.existsErr + } + iamUserAccessKeysFn = func(context.Context, aws.Config, string) ([]string, error) { + return state.accessKeys, state.accessErr + } + iamCreateAccessKeyFn = func(_ context.Context, _ aws.Config, user string) (string, string, error) { + state.createdFor = user + return state.newKeyID, state.newSecret, state.createErr + } + iamDeleteAccessKeyFn = func(_ context.Context, _ aws.Config, _ string, keyID string) error { + state.deleted = append(state.deleted, keyID) + return state.deleteErr + } + authCallerIdentityFn = func(ctx context.Context, cfg aws.Config) (string, error) { + creds, err := cfg.Credentials.Retrieve(ctx) + if err != nil { + return "", err + } + state.identity = append(state.identity, creds.AccessKeyID) + if state.identityErrFor == creds.AccessKeyID && len(state.identityErrs) > 0 { + e := state.identityErrs[0] + state.identityErrs = state.identityErrs[1:] + return "", e + } + if acct, ok := state.accounts[creds.AccessKeyID]; ok { + return acct, nil + } + return state.account, nil + } + return state +} + +// invalidClientTokenErr is what STS reports while a freshly issued key has +// not propagated yet: the pinned SDK version has no typed error for it, so +// the seam returns the wire message. +var invalidClientTokenErr = errors.New("operation error STS: GetCallerIdentity, https response error StatusCode: 403, api error InvalidClientTokenId: The security token included in the request is invalid.") + +// countKey is how many times the named access key id appears in the recorded +// identity calls — i.e. how many attempts ran with that key. +func countKey(calls []string, keyID string) int { + n := 0 + for _, k := range calls { + if k == keyID { + n++ + } + } + return n +} + +// authStoreEnv points the credential store at a temp file store, so the tests +// never touch the machine's real keystore or config. +func authStoreEnv(t *testing.T) { + t.Helper() + isolateConfig(t) + t.Setenv("SPINLOOP_CONFIG_DIR", "") + t.Setenv("SPINLOOP_REMOTE_KEYSTORE", "file") +} + +// noAmbientCreds pins the process to have no ambient AWS credential at all: +// no env credentials, no profile, no config files, no instance metadata. +func noAmbientCreds(t *testing.T) { + t.Helper() + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", filepath.Join(t.TempDir(), "no-such-file")) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join(t.TempDir(), "no-such-file")) + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") +} + +func seedStoredCred(t *testing.T, region, keyID, secret, account string) { + t.Helper() + if err := remote.StoreCredential(remote.StoredCredential{ + AccessKeyID: keyID, + SecretAccessKey: secret, + Account: account, + User: remote.ControlPlaneUserName, + Region: region, + StoredAt: time.Now().UTC(), + }); err != nil { + t.Fatal(err) + } +} + +func TestRemoteAuthStoreFirstStore(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = true + st.accessKeys = []string{"AKIAEXISTSALREADY01"} + st.newKeyID = "AKIANEWNEWNEWNEW01" + st.newSecret = "new-secret" + st.account = "1" + st.accounts["AKIANEWNEWNEWNEW01"] = "1" + + out := captureStderr(t, func() { + if err := runRemoteAuth(true, false, "ap-southeast-2"); err != nil { + t.Fatalf("runRemoteAuth --store: %v", err) + } + }) + + cred, ok := remote.LookupStoredCredential("ap-southeast-2") + if !ok { + t.Fatal("the credential was not stored") + } + if cred.AccessKeyID != "AKIANEWNEWNEWNEW01" || cred.SecretAccessKey != "new-secret" { + t.Errorf("stored key = %q/%q", cred.AccessKeyID, cred.SecretAccessKey) + } + if cred.Account != "1" || cred.Region != "ap-southeast-2" || cred.User != remote.ControlPlaneUserName { + t.Errorf("stored entry = %+v", cred) + } + if cred.Store != "file" { + t.Errorf("store kind = %q, want file", cred.Store) + } + if st.existsKey != "AKIATESTTESTTESTTEST" { + t.Errorf("the user check ran with %q, want the ambient credential", st.existsKey) + } + if st.createdFor != remote.ControlPlaneUserName { + t.Errorf("the key was created for %q", st.createdFor) + } + if len(st.deleted) != 0 { + t.Errorf("a failed-free store deleted keys: %v", st.deleted) + } + for _, want := range []string{"ap-southeast-2", "1", remote.ControlPlaneUserName, "AKIANEWNEWNEWNEW01", "file"} { + if !strings.Contains(out, want) { + t.Errorf("confirmation missing %q:\n%s", want, out) + } + } + if strings.Contains(out, "new-secret") { + t.Errorf("the confirmation printed the secret:\n%s", out) + } +} + +func TestRemoteAuthStoreMissingUser(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = false + + err := runRemoteAuth(true, false, "ap-southeast-2") + if err == nil || !strings.Contains(err.Error(), "spinloop remote bootstrap") { + t.Fatalf("error = %v, want it naming spinloop remote bootstrap", err) + } + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); ok { + t.Error("a credential was stored despite the missing user") + } +} + +func TestRemoteAuthStoreAccountMismatch(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = true + st.newKeyID = "AKIANEWNEWNEWNEW01" + st.newSecret = "new-secret" + st.account = "1" + st.accounts["AKIANEWNEWNEWNEW01"] = "2" + + err := runRemoteAuth(true, false, "ap-southeast-2") + if err == nil || !strings.Contains(err.Error(), "resolves to account 2") { + t.Fatalf("error = %v, want the account mismatch", err) + } + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); ok { + t.Error("a key that resolves elsewhere was stored") + } + if len(st.deleted) != 1 || st.deleted[0] != "AKIANEWNEWNEWNEW01" { + t.Errorf("the stray key was not deleted: %v", st.deleted) + } +} + +func TestRemoteAuthStoreRetriesUnpropagatedKey(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = true + st.newKeyID = "AKIANEWNEWNEWNEW01" + st.newSecret = "new-secret" + st.account = "1" + st.accounts["AKIANEWNEWNEWNEW01"] = "1" + st.identityErrFor = "AKIANEWNEWNEWNEW01" + st.identityErrs = []error{invalidClientTokenErr, invalidClientTokenErr} + + out := captureStderr(t, func() { + if err := runRemoteAuth(true, false, "ap-southeast-2"); err != nil { + t.Fatalf("a key that propagates late must still be stored: %v", err) + } + }) + if n := countKey(st.identity, "AKIANEWNEWNEWNEW01"); n != 3 { + t.Errorf("probe attempts with the new key = %d, want the two failed and the successful", n) + } + if len(st.deleted) != 0 { + t.Errorf("a late-propagating key was deleted on the AWS side: %v", st.deleted) + } + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); !ok { + t.Error("the credential was not stored") + } + if !strings.Contains(out, "not resolvable yet") { + t.Errorf("the retry did not say what it was doing:\n%s", out) + } +} + +func TestRemoteAuthStoreVerifyExhausted(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = true + st.newKeyID = "AKIANEWNEWNEWNEW01" + st.newSecret = "new-secret" + st.account = "1" + st.identityErrFor = "AKIANEWNEWNEWNEW01" + st.identityErrs = make([]error, verifyAttempts) + for i := range st.identityErrs { + st.identityErrs[i] = invalidClientTokenErr + } + + err := runRemoteAuth(true, false, "ap-southeast-2") + if err == nil || !strings.Contains(err.Error(), "verifying the new access key") { + t.Fatalf("error = %v, want the exhausted verification", err) + } + if n := countKey(st.identity, "AKIANEWNEWNEWNEW01"); n != verifyAttempts { + t.Errorf("probe attempts with the new key = %d, want %d", n, verifyAttempts) + } + if len(st.deleted) != 1 || st.deleted[0] != "AKIANEWNEWNEWNEW01" { + t.Errorf("the unverifiable key was not deleted: %v", st.deleted) + } + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); ok { + t.Error("an unverifiable key was stored") + } +} + +func TestRemoteAuthStoreVerifyOtherErrorNoRetry(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = true + st.newKeyID = "AKIANEWNEWNEWNEW01" + st.newSecret = "new-secret" + st.account = "1" + st.identityErrFor = "AKIANEWNEWNEWNEW01" + st.identityErrs = []error{errors.New("operation error STS: GetCallerIdentity, api error AccessDenied: not authorised")} + + err := runRemoteAuth(true, false, "ap-southeast-2") + if err == nil || !strings.Contains(err.Error(), "verifying the new access key") { + t.Fatalf("error = %v, want the verification failure", err) + } + if n := countKey(st.identity, "AKIANEWNEWNEWNEW01"); n != 1 { + t.Errorf("probe attempts with the new key = %d, want one: only the unpropagated-key shape retries", n) + } + if len(st.deleted) != 1 || st.deleted[0] != "AKIANEWNEWNEWNEW01" { + t.Errorf("the unverifiable key was not deleted: %v", st.deleted) + } +} + +func TestRemoteAuthStoreTwoKeys(t *testing.T) { + authStoreEnv(t) + stubAWSEnv(t) + st := stubAuthSeams(t) + st.userExists = true + st.accessKeys = []string{"AKIAFIRSTKEYKEYKEY01", "AKIASECONDKEYKEY01"} + + err := runRemoteAuth(true, false, "ap-southeast-2") + if err == nil || !strings.Contains(err.Error(), "two access keys") { + t.Fatalf("error = %v, want the two-key cap named", err) + } + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); ok { + t.Error("a credential was stored at the two-key cap") + } +} + +func TestRemoteAuthStoreRotationNoAmbient(t *testing.T) { + authStoreEnv(t) + noAmbientCreds(t) + seedStoredCred(t, "ap-southeast-2", "AKIAOLDOLDOLDOLD01", "old-secret", "1") + st := stubAuthSeams(t) + st.userExists = true + st.newKeyID = "AKIANEWNEWNEWNEW01" + st.newSecret = "new-secret" + st.accounts["AKIANEWNEWNEWNEW01"] = "1" + + out := captureStderr(t, func() { + if err := runRemoteAuth(true, false, "ap-southeast-2"); err != nil { + t.Fatalf("rotation without ambient credentials: %v", err) + } + }) + + if st.existsKey != "AKIAOLDOLDOLDOLD01" { + t.Errorf("the user check ran with %q, want the stored credential", st.existsKey) + } + if len(st.identity) != 1 || st.identity[0] != "AKIANEWNEWNEWNEW01" { + t.Errorf("identity calls resolved %v, want the new key only", st.identity) + } + cred, ok := remote.LookupStoredCredential("ap-southeast-2") + if !ok { + t.Fatal("the rotated credential is missing") + } + if cred.AccessKeyID != "AKIANEWNEWNEWNEW01" || cred.SecretAccessKey != "new-secret" || cred.Account != "1" { + t.Errorf("rotated entry = %+v", cred) + } + if len(st.deleted) != 1 || st.deleted[0] != "AKIAOLDOLDOLDOLD01" { + t.Errorf("the superseded key was not deleted: %v", st.deleted) + } + if !strings.Contains(out, "AKIAOLDOLDOLDOLD01") { + t.Errorf("the confirmation does not name the superseded key:\n%s", out) + } +} + +func TestRemoteAuthClear(t *testing.T) { + authStoreEnv(t) + noAmbientCreds(t) + seedStoredCred(t, "ap-southeast-2", "AKIACLEARMEKEYKEY01", "secret", "1") + st := stubAuthSeams(t) + + out := captureStderr(t, func() { + if err := runRemoteAuth(false, true, "ap-southeast-2"); err != nil { + t.Fatalf("runRemoteAuth --clear: %v", err) + } + }) + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); ok { + t.Error("the entry was not removed") + } + if len(st.deleted) != 1 || st.deleted[0] != "AKIACLEARMEKEYKEY01" { + t.Errorf("AWS-side deletion = %v, want the stored key id", st.deleted) + } + if !strings.Contains(out, "deleted its access key") { + t.Errorf("confirmation missing the AWS-side deletion:\n%s", out) + } +} + +func TestRemoteAuthClearAWSDeploymentFails(t *testing.T) { + authStoreEnv(t) + noAmbientCreds(t) + seedStoredCred(t, "ap-southeast-2", "AKIACLEARMEKEYKEY01", "secret", "1") + st := stubAuthSeams(t) + st.deleteErr = errors.New("boom") + + out := captureStderr(t, func() { + if err := runRemoteAuth(false, true, "ap-southeast-2"); err != nil { + t.Fatalf("a failed AWS-side deletion must not fail the clear: %v", err) + } + }) + if _, ok := remote.LookupStoredCredential("ap-southeast-2"); ok { + t.Error("the entry was not removed after a failed AWS-side deletion") + } + if !strings.Contains(out, "boom") || !strings.Contains(out, "may still exist") { + t.Errorf("the report does not name the failure and the lingering key:\n%s", out) + } +} + +func TestRemoteAuthClearNone(t *testing.T) { + authStoreEnv(t) + noAmbientCreds(t) + + out := captureStdout(t, func() { + if err := runRemoteAuth(false, true, "ap-southeast-2"); err != nil { + t.Fatalf("clearing nothing is not an error: %v", err) + } + }) + if !strings.Contains(out, "No stored credential for ap-southeast-2") { + t.Errorf("unexpected output:\n%s", out) + } +} + +func TestRemoteAuthReportEmpty(t *testing.T) { + authStoreEnv(t) + + out := captureStdout(t, func() { + if err := runRemoteAuth(false, false, "ap-southeast-2"); err != nil { + t.Fatalf("runRemoteAuth: %v", err) + } + }) + if !strings.Contains(out, "No stored credential") || !strings.Contains(out, "--store") { + t.Errorf("the empty report does not name --store:\n%s", out) + } +} + +func TestRemoteAuthReport(t *testing.T) { + authStoreEnv(t) + noAmbientCreds(t) + seedStoredCred(t, "ap-southeast-2", "AKIASECONDKEYKEY01", "second-secret", "1") + seedStoredCred(t, "eu-west-1", "AKIAFIRSTKEYKEYKEY01", "first-secret", "2") + + st := stubAuthSeams(t) + out := captureStdout(t, func() { + if err := runRemoteAuth(false, false, ""); err != nil { + t.Fatalf("runRemoteAuth: %v", err) + } + }) + if len(st.identity) != 0 || len(st.deleted) != 0 { + t.Errorf("the report made AWS calls: identity %v, deleted %v", st.identity, st.deleted) + } + lines := strings.Split(strings.TrimSpace(out), "\n") + if len(lines) != 3 { + t.Fatalf("want a header and two entries:\n%s", out) + } + if !strings.HasPrefix(lines[0], "region\taccount\tuser\tkey id\tstored at\tstore") { + t.Errorf("unexpected header:\n%s", lines[0]) + } + for i, want := range []string{ + "ap-southeast-2\t1\t" + remote.ControlPlaneUserName + "\tAKIASECONDKEYKEY01", + "eu-west-1\t2\t" + remote.ControlPlaneUserName + "\tAKIAFIRSTKEYKEYKEY01", + } { + if !strings.HasPrefix(lines[i+1], want) { + t.Errorf("entry %d = %q, want it starting %q", i+1, lines[i+1], want) + } + if !strings.Contains(lines[i+1], "\tfile") { + t.Errorf("entry %d does not say which store: %q", i+1, lines[i+1]) + } + } + if strings.Contains(out, "first-secret") || strings.Contains(out, "second-secret") { + t.Errorf("the report printed a secret:\n%s", out) + } +} diff --git a/cmd/spinloop/remote_bootstrap.go b/cmd/spinloop/remote_bootstrap.go index f83d8fcb..93d271c7 100644 --- a/cmd/spinloop/remote_bootstrap.go +++ b/cmd/spinloop/remote_bootstrap.go @@ -302,9 +302,13 @@ func resolveRegion(flagVal string) string { return "us-east-1" } -// loadCreds resolves an AWS config and confirms credentials are retrievable. +// loadCreds resolves an AWS config from the ambient credential chain only and +// confirms credentials are retrievable. Bootstrap and bake provision the +// control plane itself, so they need the administrator's own credentials; the +// stored control-plane key is deliberately not consulted here — a day-to-day +// key must not stand in for the admin while the account is being (re)built. func loadCreds(ctx context.Context, region string) (aws.Config, error) { - cfg, err := remote.LoadAWSConfig(ctx, region) + cfg, err := remote.LoadAmbientAWSConfig(ctx, region) if err != nil { return aws.Config{}, err } diff --git a/cmd/spinloop/remote_bootstrap_test.go b/cmd/spinloop/remote_bootstrap_test.go index dca36274..65569350 100644 --- a/cmd/spinloop/remote_bootstrap_test.go +++ b/cmd/spinloop/remote_bootstrap_test.go @@ -168,6 +168,25 @@ func TestBootstrap_SignpostsTheBake(t *testing.T) { } } +// Bootstrap and bake provision the control plane, so their credential +// preflight resolves the ambient chain only: with no ambient credentials +// there is nothing to fall back to — the stored control-plane key must not +// stand in for the administrator. The loader itself is pinned to that +// behaviour by TestLoadAmbientAWSConfigIgnoresStoredKey in internal/remote. +func TestLoadCredsRequiresAmbientCredentials(t *testing.T) { + isolateConfig(t) + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", filepath.Join(t.TempDir(), "no-such-file")) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join(t.TempDir(), "no-such-file")) + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") + if _, err := loadCreds(context.Background(), "us-east-1"); err == nil { + t.Fatal("loadCreds without ambient credentials should fail, got a config") + } +} + func TestBootstrap_Preflight(t *testing.T) { t.Run("missing tooling fails naming both managers", func(t *testing.T) { t.Setenv("PATH", t.TempDir()) // no node/pnpm/npm diff --git a/docs/commands/remote.md b/docs/commands/remote.md index 86d4b9b2..ff9428c1 100644 --- a/docs/commands/remote.md +++ b/docs/commands/remote.md @@ -6,6 +6,7 @@ while you're using it. ```sh spinloop remote bootstrap # once per account: deploy the control plane +spinloop remote auth # store or report the credential this machine signs with spinloop remote bake # bake the runner AMI(s) an environment runs from spinloop remote deploy # create an endpoint (environment) and tell it what to serve spinloop remote start # boot it; prints the exports your agent needs (progress on stderr) @@ -144,11 +145,68 @@ spinloop remote ls lists each registered environment with its base URL and region, marking any whose `remote.json` is missing or unreadable. It contacts no endpoint. -Requests are signed with **your** AWS credentials (the usual profile, SSO -session, or environment variables), and the endpoint's URLs require it. Spinloop -stores no credentials of its own. Beyond invoking those URLs, the only extra -permission it wants is for [reading logs](#reading-the-logs), which talks to -CloudWatch rather than to an endpoint. +Requests are signed with an AWS credential resolved per region — explicit +environment credentials or a named profile first, then the stored +control-plane credential from `spinloop remote auth --store`, then the usual +chain of config files, SSO sessions, and instance metadata; see +[credentials](#credentials). The endpoint's URLs require it. Beyond invoking +those URLs, the only extra permission it wants is for +[reading logs](#reading-the-logs), which talks to CloudWatch rather than to an +endpoint. + +## Credentials + +Every `spinloop remote` command signs its requests with an AWS credential, +resolved for the region the command targets, in this order: + +1. Explicit credentials in the process environment (`AWS_ACCESS_KEY_ID` and + friends) or an explicit `AWS_PROFILE` — a deliberate per-process choice, so + they always win. +2. The stored control-plane credential, when one is stored for the region — + it outlives SSO log-ins, which is the point of it. +3. The standard chain: shared config files, SSO sessions, instance metadata. + +`spinloop remote auth` manages the stored credential on this machine: + +```sh +spinloop remote auth # what is stored (no AWS call) +spinloop remote auth --store # store one for the region; rotates it when stored +spinloop remote auth --store --region ap-southeast-2 +spinloop remote auth --clear # remove it and delete the access key +``` + +`--store` creates an access key for the control-plane user the stack makes +(`cloud-vm-llm-remote-cli`) and keeps it in this machine's OS keystore — +Keychain on macOS, Credential Manager on Windows, the Secret Service on Linux. +Where no keystore is reachable it keeps it in an owner-only file under the +spinloop config directory instead, and every report says which store it used; +`SPINLOOP_REMOTE_KEYSTORE=file` selects the file store even where a keystore +is, for a machine whose keystore is locked or unreachable. The secret is never +printed. The key is scoped to day-to-day control only: invoke the control +URLs, read the instance logs, discover the stack, price an instance, and manage +this user's own access keys — nothing that provisions. + +A first store runs on the administrator's ambient credentials and verifies the +new key resolves to the caller's account before storing it; a key that +resolves elsewhere is deleted, not kept. When a credential is already stored, +`--store` rotates instead: it creates the replacement with the stored key +alone — no other AWS credential needed — swaps the entry, and deletes the +superseded key on the AWS side. Rotate roughly every 90 days, like any +long-lived key. + +`--clear` removes the local entry and deletes the access key on the AWS side +with the stored credential, so a cleared key does not linger in the account. +If the AWS-side deletion cannot be made, the local entry is still removed and +the failure reported. + +A control plane deployed before this capability has no control-plane user, so +`--store` against it fails naming `spinloop remote bootstrap` — re-run it to +add the user, then store. + +`bootstrap` and `bake` never consult the stored credential: they provision the +control plane itself and run on the administrator's ambient credentials. The +fleet's operations on remote environments sign through the same resolution, so +a stored key covers them too. ## Checking on an endpoint @@ -371,8 +429,9 @@ included) for every `kind: remote` node a fleet file names — or a chosen few ## Notes -- `bootstrap` and `bake` are account-level and take no Spinloop: the control - plane and the AMIs are shared by every environment. +- `bootstrap`, `bake`, and `auth` take no Spinloop: the control plane and the + AMIs are shared by every environment, and the stored credential belongs to + the machine, not a project. - `deploy` always needs a Spinloop — it's the thing being deployed. The others take an optional Spinloop path, a [registered alias](alias.md), or a URL. Given none, they use the alias `SPINLOOP_ALIAS` names, and failing that diff --git a/docs/env-vars.md b/docs/env-vars.md index 50d98041..477998f9 100644 --- a/docs/env-vars.md +++ b/docs/env-vars.md @@ -8,12 +8,13 @@ from the environment or a `.env` beside the Spinloop — never written into an | Variable | Used by | Meaning | | --- | --- | --- | -| `SPINLOOP_CONFIG_DIR` | everything | spinloop's config directory, used **verbatim** (no `spinloop` segment appended). Overrides `XDG_CONFIG_HOME` and `~/.config`. Everything spinloop owns lives here: `config.json` (default-harness preference + alias registry), `remote.json`, the `remotes//` environment registry, the daemon state dir, and the CDK source cache. Set it when there is no usable `$HOME` — e.g. a systemd service. See [config resolution](#config-directory-resolution). | +| `SPINLOOP_CONFIG_DIR` | everything | spinloop's config directory, used **verbatim** (no `spinloop` segment appended). Overrides `XDG_CONFIG_HOME` and `~/.config`. Everything spinloop owns lives here: `config.json` (default-harness preference + alias registry), `remote.json`, the `remotes//` environment registry, the `keystore/` file credential store, the daemon state dir, and the CDK source cache. Set it when there is no usable `$HOME` — e.g. a systemd service. See [config resolution](#config-directory-resolution). | | `SPINLOOP_HARNESS` | all harness commands | Which harness to configure/launch (`opencode`, `pi` or `lucinate`). Precedence: `--harness`/`-H` flag > `SPINLOOP_HARNESS` > stored preference > `opencode`. | | `SPINLOOP_ALIAS` | every command that takes a Spinloop path | A name registered with [`spinloop alias`](commands/alias.md), used when the command is given no path. Precedence: the path or alias argument > `SPINLOOP_ALIAS` > `./Spinloop`. It holds a registry name, never a path, and a same-named file in the working directory does not shadow it. It decides *which* Spinloop is the default, not *whether* one is applied — a bare `spinloop harness` still applies nothing, and `spinloop alias` ignores it. | | `SPINLOOP_PROVIDERS` | `list`, `add`, `apply`, … | Path to a `providers.yaml` that overrides the built-in catalogue. Precedence: `--providers` flag > `SPINLOOP_PROVIDERS` > embedded. | | `SPINLOOP_BASE_URL` | `add`, `apply` | Base-URL override for the provider being configured. Precedence: `--base-url`/`-u` > `SPINLOOP_BASE_URL` > the provider's own option var > the catalogue default. | | `SPINLOOP_API_TOKEN` | `spinloop daemon`, `spinloop serve --api` | Bearer token for the daemon control API. One of three peer sources, alongside `--api-token-file` and `--api-token`; two at once is an error. From a service manager prefer the file form — see [serve](commands/serve.md). A non-loopback API listen without any of them refuses to start. | +| `SPINLOOP_REMOTE_KEYSTORE` | `spinloop remote auth` | Set to `file` to keep the stored control-plane credential in the owner-only file under the config directory, even where an OS keystore is reachable — the opt-out for a machine whose keystore is locked or unreachable. Unset, the OS keystore is used where available. See [credentials](commands/remote.md#credentials). | | `SPINLOOP_LOG_LEVEL` | `spinloop daemon`, `spinloop serve` | How much spinloop records about the control API and the supervised engine: `debug`, `info` (default), `warn` or `error`. Precedence: `--log-level` flag > `SPINLOOP_LOG_LEVEL` > `info`. An unrecognised value refuses to start rather than falling back to the default. Under `spinloop serve` the `.env` beside the Spinloop can set it; the daemon reads no Spinloop, so there it comes from the environment its service manager gives it. Records go to stderr; see [what gets logged](commands/serve.md#what-gets-logged). | | *(per-node, named by `tokenEnv`)* | `spinloop fleet` | A fleet node's bearer token. `fleet.yaml` names the variable rather than holding the value; it resolves from the environment, then the `.env` beside the fleet file. See [fleet](commands/fleet.md). | | *(per-node, named by `engineTokenEnv`)* | `spinloop fleet`, `spinloop harness` | The key a fleet node's **engine** is gated with. Resolved the same way, and supplied by the client when it starts that engine — so the node holds no key of its own and the two ends cannot disagree. See [fleet](commands/fleet.md). | diff --git a/docs/internals.md b/docs/internals.md index 22fecc1c..382c9a91 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -1,4 +1,3 @@ -- The colours and the spinner live in `palette.go`, in two groups that must not be swapped for one another. The brand colours (`brandAccent`, `brandInk`, `brandInkDim`) carry the same values as the web repo's `--accent`, `--ink` and `--ink-2` tokens, written as hex and downsampled by lipgloss, and the accent is used only where nothing about a node is being reported — the title bar's product name and the selected panel's border. The state colours (`ansiGreen` and friends) are raw ANSI and report what an engine is doing: the resource bars and the health glyph draw from them. `spinnerFrames` is one cycle for the whole tool, shared by `fleet deploy`'s progress lines and the dashboard's in-flight tiles. # Implementation notes This is maintainer reference, not a user guide (see [`docs/README.md`](README.md) @@ -11,6 +10,8 @@ fit either of those. These are mistakes already made here; each was silent rather than loud, which is what makes them worth writing down. +**The palette's two colour groups are not swappable.** The brand colours (`brandAccent`, `brandInk`, `brandInkDim`) carry the same values as the web repo's `--accent`, `--ink` and `--ink-2` tokens, written as hex and downsampled by lipgloss, and the accent is used only where nothing about a node is being reported — the title bar's product name and the selected panel's border. The state colours (`ansiGreen` and friends) are raw ANSI and report what an engine is doing: the resource bars and the health glyph draw from them. `spinnerFrames` is one cycle for the whole tool, shared by `fleet deploy`'s progress lines and the dashboard's in-flight tiles. + **The catalogue is embedded at build time.** `providers.yaml` is `//go:embed`-ed, so a previously-built `spinloop` binary keeps applying the *old* catalogue no matter what the file says. Rebuild before testing any catalogue change, or you will "verify" a fix that is not in the binary you ran. (`--providers`/`SPINLOOP_PROVIDERS` reads a file at run time and sidesteps this.) **A preset section is not the whole preset.** `Preset.Select` returns only the named `[section]`; the `[*]` defaults live separately in `Preset.Global`. Anything that consumes a section's `Params` directly — rather than going through `Args`/`Command`, which layer both — silently drops whatever the user put in `[*]`. That is usually the settings they consider obvious enough to write once, like `ngl` and `jinja`, so the failure surfaces later as a model running on CPU or refusing tool calls. @@ -29,6 +30,12 @@ These are mistakes already made here; each was silent rather than loud, which is **`up` dispatches by directory and reuses both branches.** `cmd/spinloop/up.go` routes a working-directory `fleet.yaml` to the fleet start path — `runFleetDrive` over the named nodes, or over every node when none are given, since a bare `fleet start` lists and does nothing — and everything else to `runServe`'s own body, so `up` and `serve` resolve and word things identically by construction. The completion slot is the only CWD-dependent one: `upSlot` offers the fleet's node names where `./fleet.yaml` parses, the Spinloop slot elsewhere, and nothing where a fleet file is present but unreadable — `__complete` never errors, whatever the directory holds. +**A freshly issued access key is not instantly resolvable.** STS lags the `iam:CreateAccessKey` response by seconds — on one account the store's verification needed six attempts before the key resolved, at roughly 8–10 s propagation. The verify probe in `cmd/spinloop/remote_auth.go` (`verifyNewKey`) retries only when the error message contains `InvalidClientTokenId`, on an exponential backoff from 1 s to a 16 s cap (a ~31 s window over six attempts), and the stderr line says which retry is running. Two things to preserve: the match is a string match because the pinned `service/sts` SDK version has no typed error for that code — if the SDK is upgraded, switch to the typed check — and the retry is exclusive to that code. Any other verification failure, a key that resolves to a different account or lacks a permission, fails at once and deletes the key it created; waiting it out would only delay the deletion. `verifyProbeBackoff` is a seam the tests zero. + +**An IAM user's inline policies are capped at 2,048 characters in aggregate.** The control plane's seven functions each take a `grantInvokeUrl` pair — two actions, the auth-type conditions, the function's ARN — and with the log-reading, stack-discovery, pricing and self-service statements the document far exceeds that; the first deploy of the `RemoteCliUser` inline policy failed with `ServiceLimitExceeded`, which CDK does not warn about ahead of time. It is now a stack-owned `AWS::IAM::ManagedPolicy` (`RemoteCliPolicy`, 6,144 cap; the deployed document measures ~2 KB, so the grant list has room to grow). Keep it managed rather than re-inlining it, and keep the iam self-service ARN built from the `AWS::Partition`/`AWS::AccountId` pseudo parameters instead of the user's `Arn`: the policy attaches to that user, so referencing the user from inside it is a dependency cycle. + +**The file credential store's index is non-secret by design.** OS keystores offer no way to list entries, so the file store — used where no keystore is reachable or `SPINLOOP_REMOTE_KEYSTORE=file` — keeps a plain-text index of the stored regions beside the `0600` per-region files under `/keystore/`. The report (`spinloop remote auth`) reads the index, so a file added, removed or renamed by hand is reported wrong until the index matches; and a corrupt index is reported, not silently reset, because a report that misleads about what is stored misleads about which access keys exist on the AWS side. + ## Dashboard (`fleet_dashboard.go` and friends) A few Bubble Tea/lipgloss specifics that are easy to break by "simplifying": diff --git a/go.mod b/go.mod index 21b57e79..1049ff92 100644 --- a/go.mod +++ b/go.mod @@ -7,9 +7,11 @@ require github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd require ( github.com/aws/aws-sdk-go-v2 v1.45.1 github.com/aws/aws-sdk-go-v2/config v1.33.1 + github.com/aws/aws-sdk-go-v2/credentials v1.20.1 github.com/aws/aws-sdk-go-v2/service/cloudformation v1.78.1 github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs v1.84.1 github.com/aws/aws-sdk-go-v2/service/ec2 v1.325.1 + github.com/aws/aws-sdk-go-v2/service/iam v1.52.1 github.com/aws/aws-sdk-go-v2/service/pricing v1.46.1 github.com/aws/aws-sdk-go-v2/service/sts v1.47.1 github.com/aws/smithy-go v1.28.1 @@ -17,17 +19,18 @@ require ( github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.11.8 github.com/charmbracelet/x/exp/teatest v0.0.0-20260816001655-68d539dca504 + github.com/godbus/dbus/v5 v5.2.2 github.com/muesli/termenv v0.16.0 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 + github.com/zalando/go-keyring v0.2.8 golang.org/x/term v0.45.0 gopkg.in/yaml.v3 v3.0.1 ) require ( github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.20 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.20.1 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.19.1 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.5.1 // indirect github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.8.1 // indirect @@ -45,6 +48,7 @@ require ( github.com/charmbracelet/x/term v0.2.2 // indirect github.com/clipperhouse/displaywidth v0.11.0 // indirect github.com/clipperhouse/uax29/v2 v2.7.0 // indirect + github.com/danieljoos/wincred v1.2.3 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect @@ -55,6 +59,7 @@ require ( github.com/mattn/go-runewidth v0.0.24 // indirect github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/cancelreader v0.2.2 // indirect + github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/sagikazarmark/locafero v0.11.0 // indirect @@ -66,4 +71,5 @@ require ( go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/text v0.28.0 // indirect + gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect ) diff --git a/go.sum b/go.sum index f757347a..18a65c7d 100644 --- a/go.sum +++ b/go.sum @@ -20,6 +20,8 @@ github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs v1.84.1 h1:GTN8kHGLbUlnaXPyT github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs v1.84.1/go.mod h1:hp3qwCtX+QPXVJMyKsFO/sLXkirzH0R3UHRVM4QBGV4= github.com/aws/aws-sdk-go-v2/service/ec2 v1.325.1 h1:rL19vNlxMMJHDSbX5JAwnpR/94kRgcebkdFRvmG6MWM= github.com/aws/aws-sdk-go-v2/service/ec2 v1.325.1/go.mod h1:M8AJ/M7737nKBNqZvXUNrADGBy+d3PNggJGmAStCWxI= +github.com/aws/aws-sdk-go-v2/service/iam v1.52.1 h1:OYigTuTHayk1j11osOuJqIEuXSGAVndZYKH4aZYn8qk= +github.com/aws/aws-sdk-go-v2/service/iam v1.52.1/go.mod h1:PuHz5kGh1jtsNpjezdYhRp7xgn6DzCNJJfQt7O7U9Aw= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.19 h1:bAdDl/HkGCcGPoe25ToSHEw23VIxt6CT5fLcg111BKg= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.19/go.mod h1:KaUzbLxv4CeSxh6ZCl9B4m7CuFenS8kUEaDs+f/DQr4= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.14.1 h1:RmmWQPREQdk9U+PfqeHW3MqZaBaNK7TpV9W3RY+b+7g= @@ -61,6 +63,8 @@ github.com/clipperhouse/displaywidth v0.11.0/go.mod h1:bkrFNkf81G8HyVqmKGxsPufD3 github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk= github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= +github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= @@ -71,12 +75,16 @@ github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= +github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/lucasb-eyer/go-colorful v1.4.0 h1:UtrWVfLdarDgc44HcS7pYloGHJUjHV/4FwW4TvVgFr4= @@ -93,6 +101,8 @@ github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELU github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= +github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= +github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -117,6 +127,8 @@ github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= @@ -125,6 +137,8 @@ github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd h1:Rf9uhF1+VJ7ZHq github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd/go.mod h1:EbW0wDK/qEUYI0A5bqq0C2kF8JTQwWONmGDBbzsxxHo= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= +github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs= +github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= @@ -138,7 +152,7 @@ golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= +gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/fleet/remote_node_test.go b/internal/fleet/remote_node_test.go index a617891f..fae18901 100644 --- a/internal/fleet/remote_node_test.go +++ b/internal/fleet/remote_node_test.go @@ -49,6 +49,56 @@ func remoteControlServer(t *testing.T, body string, statusCode int) *httptest.Se return srv } +// Fleet operations on remote environments go through the same signed control +// calls as `spinloop remote`, so they inherit the stored control-plane +// credential: here the only credential the process can resolve is the one +// stored for the region (the ambient chain is empty), and the status call +// still signs and gets its answer. The file store behind SPINLOOP_REMOTE_KEYSTORE +// keeps the entry in a temp directory, never in the machine's keystore. +func TestRemoteNodeSignsWithStoredCredential(t *testing.T) { + region := "ap-southeast-2" + srv := remoteControlServer(t, `{"state":"stopped"}`, http.StatusOK) + + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("SPINLOOP_CONFIG_DIR", "") + t.Setenv("SPINLOOP_REMOTE_KEYSTORE", "file") + // No ambient credential at all: env, profile, config files, IMDS. + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", filepath.Join(home, "no-such-file")) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join(home, "no-such-file")) + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") + + cred := remote.StoredCredential{ + AccessKeyID: "AKIATESTTESTTESTTEST", + SecretAccessKey: "test-secret", + Account: "0", + User: "cloud-vm-llm-remote-cli", + Region: region, + StoredAt: time.Now().UTC(), + } + if err := remote.StoreCredential(cred); err != nil { + t.Fatalf("StoreCredential: %v", err) + } + t.Cleanup(func() { remote.DeleteStoredCredential(region) }) + + node, err := NewRemoteNode("env", remote.Config{StartURL: srv.URL, StopURL: srv.URL, Region: region}) + if err != nil { + t.Fatal(err) + } + status, err := node.Status(context.Background()) + if err != nil { + t.Fatalf("status with only a stored credential available: %v", err) + } + if status.State != "stopped" { + t.Fatalf("status = %+v, want stopped", status) + } +} + func TestNewRemoteNodeRequiresACompleteConfig(t *testing.T) { if _, err := NewRemoteNode("env", remote.Config{StopURL: "http://x", Region: "r"}); err == nil { t.Error("missing start_url should be a configuration error") diff --git a/internal/remote/aws.go b/internal/remote/aws.go index f6e1abae..94103629 100644 --- a/internal/remote/aws.go +++ b/internal/remote/aws.go @@ -3,27 +3,79 @@ package remote import ( "context" "encoding/json" + "errors" "fmt" "math" + "os" "strconv" "strings" "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/service/cloudformation" + "github.com/aws/aws-sdk-go-v2/service/iam" + iamb "github.com/aws/aws-sdk-go-v2/service/iam/types" "github.com/aws/aws-sdk-go-v2/service/pricing" "github.com/aws/aws-sdk-go-v2/service/pricing/types" "github.com/aws/aws-sdk-go-v2/service/sts" ) -// LoadAWSConfig resolves the default AWS config for a region. Credentials are -// not retrieved here — callers that need them (signing, the preflight check) -// call Retrieve on the returned config, keeping the failure guidance close to -// where it is reported. +// LoadAWSConfig resolves the AWS config for a region, applying the credential +// precedence the remote commands sign with: explicit AWS environment +// credentials or an explicit profile selection win (the default chain, as +// before); then a stored control-plane credential for the region, if one is +// in the keystore; then the rest of the standard chain (shared config, SSO +// sessions, instance metadata). Credentials are not retrieved here — callers +// that need them (signing, the preflight check) call Retrieve on the returned +// config, keeping the failure guidance close to where it is reported. func LoadAWSConfig(ctx context.Context, region string) (aws.Config, error) { + opts := []func(*awsconfig.LoadOptions) error{awsconfig.WithRegion(region)} + if opt, ok := storedCredsOption(region); ok { + opts = append(opts, opt) + } + return awsconfig.LoadDefaultConfig(ctx, opts...) +} + +// LoadAmbientAWSConfig resolves the default credential chain only, never +// consulting the stored control-plane credential. Bootstrap and bake use it: +// they provision the control plane itself, so a stored day-to-day key must +// not stand in for the administrator credentials they need. +func LoadAmbientAWSConfig(ctx context.Context, region string) (aws.Config, error) { return awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(region)) } +// storedCredsOption is the config option carrying the stored control-plane +// credential for the region, when it applies — that is, when the process +// environment carries no explicit AWS credentials and no explicit profile +// selection. Those are a deliberate per-process choice (a Spinloop's .env or +// ENV may inject them, and an operator may set them to debug with other +// credentials) and override the stored key; everything else in the standard +// chain yields to it, which is the point of storing a key that outlives SSO +// log-ins. +func storedCredsOption(region string) (func(*awsconfig.LoadOptions) error, bool) { + if explicitAmbientCreds() { + return nil, false + } + cred, ok := LookupStoredCredential(region) + if !ok { + return nil, false + } + return awsconfig.WithCredentialsProvider( + credentials.NewStaticCredentialsProvider(cred.AccessKeyID, cred.SecretAccessKey, "")), true +} + +// explicitAmbientCreds reports whether the process environment names explicit +// AWS credentials or an explicit profile selection. +func explicitAmbientCreds() bool { + for _, key := range []string{"AWS_ACCESS_KEY_ID", "AWS_PROFILE"} { + if os.Getenv(key) != "" { + return true + } + } + return false +} + // CallerIdentity returns the AWS account id for the resolved credentials, so // the bootstrap plan can name the account being deployed into. func CallerIdentity(ctx context.Context, cfg aws.Config) (string, error) { @@ -34,6 +86,74 @@ func CallerIdentity(ctx context.Context, cfg aws.Config) (string, error) { return aws.ToString(out.Account), nil } +// ControlPlaneUserName is the IAM user the control-plane stack creates for the +// CLI's long-lived credential: `spinloop remote auth --store` creates access +// keys for this user and stores one in this machine's keystore. The name is +// fixed, so the CLI addresses the user without reading a stack output; the +// stack and its tests use the same literal. +const ControlPlaneUserName = "cloud-vm-llm-remote-cli" + +// ConfigFromStored builds the AWS config that signs with a stored +// control-plane credential: a static provider for the key, no other chain. +// The auth command uses it to verify a newly created key, to rotate with the +// stored key alone, and to delete a key on the AWS side during a clear. +func ConfigFromStored(cred StoredCredential) aws.Config { + return aws.Config{ + Region: cred.Region, + Credentials: credentials.NewStaticCredentialsProvider(cred.AccessKeyID, cred.SecretAccessKey, ""), + } +} + +// IAMUserExists reports whether the named IAM user exists in the account and +// region the config resolves for. An absent user is (false, nil), not an +// error: a control plane deployed before the user existed is a normal, +// fixable case the caller names its fix for. +func IAMUserExists(ctx context.Context, cfg aws.Config, userName string) (bool, error) { + _, err := iam.NewFromConfig(cfg).GetUser(ctx, &iam.GetUserInput{UserName: aws.String(userName)}) + if err != nil { + var noSuch *iamb.NoSuchEntityException + if errors.As(err, &noSuch) { + return false, nil + } + return false, err + } + return true, nil +} + +// IAMUserAccessKeyIDs returns the access key ids the named IAM user currently +// has. +func IAMUserAccessKeyIDs(ctx context.Context, cfg aws.Config, userName string) ([]string, error) { + out, err := iam.NewFromConfig(cfg).ListAccessKeys(ctx, &iam.ListAccessKeysInput{UserName: aws.String(userName)}) + if err != nil { + return nil, err + } + ids := make([]string, 0, len(out.AccessKeyMetadata)) + for _, k := range out.AccessKeyMetadata { + ids = append(ids, aws.ToString(k.AccessKeyId)) + } + return ids, nil +} + +// IAMCreateAccessKey creates an access key for the named IAM user and returns +// it. The secret is returned only here, once: IAM never returns it again, so +// a failure after this point has nothing to recover it with. +func IAMCreateAccessKey(ctx context.Context, cfg aws.Config, userName string) (string, string, error) { + out, err := iam.NewFromConfig(cfg).CreateAccessKey(ctx, &iam.CreateAccessKeyInput{UserName: aws.String(userName)}) + if err != nil { + return "", "", err + } + return aws.ToString(out.AccessKey.AccessKeyId), aws.ToString(out.AccessKey.SecretAccessKey), nil +} + +// IAMDeleteAccessKey deletes the named access key from the named IAM user. +func IAMDeleteAccessKey(ctx context.Context, cfg aws.Config, userName, accessKeyID string) error { + _, err := iam.NewFromConfig(cfg).DeleteAccessKey(ctx, &iam.DeleteAccessKeyInput{ + UserName: aws.String(userName), + AccessKeyId: aws.String(accessKeyID), + }) + return err +} + // ControlPlaneStackDeployed reports whether the named CloudFormation stack exists in // the account and region — i.e. whether `spinloop remote bootstrap` has already // run. A stack that does not exist is reported as false, not an error. @@ -111,13 +231,26 @@ func controlPlaneFromOutputs(stackName string, outputs map[string]string) (Contr return layer, nil } +// pricingConfig resolves the AWS config for the pricing call. The pricing +// service is global, so the endpoint stays us-east-1; the credential, though, +// resolves with the environment's region precedence — the stored +// control-plane key, when nothing explicit is set, signs the call the same as +// every other day-to-day command, and the user policy's pricing:GetProducts +// grant is what authorises it. +func pricingConfig(ctx context.Context, envRegion string) (aws.Config, error) { + opts := []func(*awsconfig.LoadOptions) error{awsconfig.WithRegion("us-east-1")} + if opt, ok := storedCredsOption(envRegion); ok { + opts = append(opts, opt) + } + return awsconfig.LoadDefaultConfig(ctx, opts...) +} + // GetOnDemandPrice returns the hourly on-demand price for an instance type in // a region, from the AWS Price List API. The result is cached for 5 minutes // within a single process lifetime. Returns an error if the pricing service is // unavailable or the instance type is not found. func GetOnDemandPrice(ctx context.Context, region, instanceType string) (float64, error) { - // Pricing is a global service — use us-east-1 as the API endpoint. - cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion("us-east-1")) + cfg, err := pricingConfig(ctx, region) if err != nil { return 0, fmt.Errorf("loading AWS config for pricing: %w", err) } diff --git a/internal/remote/aws_test.go b/internal/remote/aws_test.go index f25206bf..65dc9574 100644 --- a/internal/remote/aws_test.go +++ b/internal/remote/aws_test.go @@ -1,8 +1,11 @@ package remote import ( + "context" "encoding/json" "math" + "os" + "path/filepath" "testing" ) @@ -212,3 +215,167 @@ func TestExtractPrice_Fallback(t *testing.T) { t.Errorf("extractPrice fallback = %v, want 0.3580", got) } } + +// pinChainHermetically keeps the standard credential chain inside the test: +// the shared config and credentials files point at temp files, no explicit +// env credential or profile, and IMDS is off so an unresolvable chain fails +// fast instead of reaching for the metadata service. +func pinChainHermetically(t *testing.T, sharedCredsFile, configFile string) { + t.Helper() + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", sharedCredsFile) + t.Setenv("AWS_CONFIG_FILE", configFile) + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") +} + +func writeAWSCredsFile(t *testing.T, dir, content string) string { + t.Helper() + path := filepath.Join(dir, "aws-creds") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + return path +} + +// storeCredForTest parks a stored credential in a file-backed store behind the +// openCredStoreFn seam, so the tests never touch the machine's keystore. +func storeCredForTest(t *testing.T, cred StoredCredential) { + t.Helper() + dir := t.TempDir() + t.Cleanup(func() { openCredStoreFn = openCredStore }) + openCredStoreFn = func() (credentialStore, error) { + return fileStore(t, dir), nil + } + if err := StoreCredential(cred); err != nil { + t.Fatalf("StoreCredential: %v", err) + } +} + +func resolvedAccessKeyID(t *testing.T, region string) string { + t.Helper() + cfg, err := LoadAWSConfig(context.Background(), region) + if err != nil { + t.Fatalf("LoadAWSConfig: %v", err) + } + creds, err := cfg.Credentials.Retrieve(context.Background()) + if err != nil { + t.Fatalf("Retrieve: %v", err) + } + return creds.AccessKeyID +} + +func TestLoadAWSConfigPrecedence(t *testing.T) { + const region = "ap-southeast-2" + stored := testCred(region) + storeCredForTest(t, stored) + chainFile := writeAWSCredsFile(t, t.TempDir(), `[default] +aws_access_key_id = AKIACHAINCHAINCHAIN +aws_secret_access_key = chain-secret +`) + + t.Run("env credentials override the stored key", func(t *testing.T) { + pinChainHermetically(t, chainFile, chainFile) + t.Setenv("AWS_ACCESS_KEY_ID", "AKIAENVENVENVENVENV") + t.Setenv("AWS_SECRET_ACCESS_KEY", "env-secret") + if got := resolvedAccessKeyID(t, region); got != "AKIAENVENVENVENVENV" { + t.Fatalf("resolved %q; want the env credential", got) + } + }) + + t.Run("a named profile overrides the stored key", func(t *testing.T) { + profileFile := writeAWSCredsFile(t, t.TempDir(), `[worker] +aws_access_key_id = AKIAPROFILEPROFILE +aws_secret_access_key = profile-secret +`) + pinChainHermetically(t, profileFile, profileFile) + t.Setenv("AWS_PROFILE", "worker") + if got := resolvedAccessKeyID(t, region); got != "AKIAPROFILEPROFILE" { + t.Fatalf("resolved %q; want the profile credential", got) + } + }) + + t.Run("the stored key wins over the shared chain", func(t *testing.T) { + pinChainHermetically(t, chainFile, chainFile) + if got := resolvedAccessKeyID(t, region); got != stored.AccessKeyID { + t.Fatalf("resolved %q; want the stored credential", got) + } + }) + + t.Run("no stored key falls back to the chain", func(t *testing.T) { + // A region with no stored entry: the file store holds only ap-southeast-2. + pinChainHermetically(t, chainFile, chainFile) + if got := resolvedAccessKeyID(t, "eu-west-1"); got != "AKIACHAINCHAINCHAIN" { + t.Fatalf("resolved %q; want the shared-chain credential", got) + } + }) +} + +func TestPricingConfigPrecedence(t *testing.T) { + const region = "ap-southeast-2" + stored := testCred(region) + storeCredForTest(t, stored) + chainFile := writeAWSCredsFile(t, t.TempDir(), `[default] +aws_access_key_id = AKIACHAINCHAINCHAIN +aws_secret_access_key = chain-secret +`) + + t.Run("the stored key authorises pricing, endpoint stays us-east-1", func(t *testing.T) { + pinChainHermetically(t, chainFile, chainFile) + cfg, err := pricingConfig(context.Background(), region) + if err != nil { + t.Fatalf("pricingConfig: %v", err) + } + if cfg.Region != "us-east-1" { + t.Fatalf("endpoint region = %q; want us-east-1", cfg.Region) + } + creds, err := cfg.Credentials.Retrieve(context.Background()) + if err != nil { + t.Fatalf("Retrieve: %v", err) + } + if creds.AccessKeyID != stored.AccessKeyID { + t.Fatalf("resolved %q; want the stored credential", creds.AccessKeyID) + } + }) + + t.Run("explicit env credentials override the stored key", func(t *testing.T) { + pinChainHermetically(t, chainFile, chainFile) + t.Setenv("AWS_ACCESS_KEY_ID", "AKIAENVENVENVENVENV") + t.Setenv("AWS_SECRET_ACCESS_KEY", "env-secret") + cfg, err := pricingConfig(context.Background(), region) + if err != nil { + t.Fatalf("pricingConfig: %v", err) + } + creds, err := cfg.Credentials.Retrieve(context.Background()) + if err != nil { + t.Fatalf("Retrieve: %v", err) + } + if creds.AccessKeyID != "AKIAENVENVENVENVENV" { + t.Fatalf("resolved %q; want the env credential", creds.AccessKeyID) + } + }) +} + +func TestLoadAmbientAWSConfigIgnoresStoredKey(t *testing.T) { + const region = "ap-southeast-2" + storeCredForTest(t, testCred(region)) + chainFile := writeAWSCredsFile(t, t.TempDir(), `[default] +aws_access_key_id = AKIACHAINCHAINCHAIN +aws_secret_access_key = chain-secret +`) + pinChainHermetically(t, chainFile, chainFile) + + cfg, err := LoadAmbientAWSConfig(context.Background(), region) + if err != nil { + t.Fatalf("LoadAmbientAWSConfig: %v", err) + } + creds, err := cfg.Credentials.Retrieve(context.Background()) + if err != nil { + t.Fatalf("Retrieve: %v", err) + } + if creds.AccessKeyID != "AKIACHAINCHAINCHAIN" { + t.Fatalf("resolved %q; want the ambient credential, not the stored key", creds.AccessKeyID) + } +} diff --git a/internal/remote/keystore.go b/internal/remote/keystore.go new file mode 100644 index 00000000..936de62a --- /dev/null +++ b/internal/remote/keystore.go @@ -0,0 +1,314 @@ +package remote + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + "sort" + "strings" + "time" + + dbus "github.com/godbus/dbus/v5" + "github.com/zalando/go-keyring" +) + +// StoredCredential is one long-lived control-plane credential the operator +// stored with `spinloop remote auth --store`: an access key for the +// control-plane user, held in the OS keystore (or an owner-only file where no +// keystore exists) rather than in a shared AWS config. +type StoredCredential struct { + AccessKeyID string `json:"access_key_id"` + SecretAccessKey string `json:"secret_access_key"` + Account string `json:"account"` + User string `json:"user"` + Region string `json:"region"` + StoredAt time.Time `json:"stored_at"` + Store string `json:"store"` +} + +// credentialStore is one machine's store of stored control-plane credentials: +// the OS keystore where one is available, an owner-only directory under the +// user's spinloop config directory where it is not. +type credentialStore struct { + kind string // "keyring" or "file" + kr keyringBackend // set when kind is "keyring" + dir string // set when kind is "file" +} + +// keyringService is the service name this tool's entries sit under in the +// shared OS keystore; the region is the entry's user name. +const keyringService = "spinloop-remote" + +// keyringIndexUser is the entry holding the regions this tool has stored, one +// per line: a non-secret index, since the OS keystores offer no way to list +// a service's entries. +const keyringIndexUser = "index" + +// keyStoreEnvVar selects the file store over the OS keystore when set to +// "file": the opt-out for a headless macOS session whose keychain is locked +// or unreachable, and the way the test suite keeps stored credentials inside +// a temp config directory. +const keyStoreEnvVar = "SPINLOOP_REMOTE_KEYSTORE" + +// keyringBackend is the slice of the OS keystore the store drives, so tests +// substitute an in-memory fake without touching the machine's real keystore. +type keyringBackend interface { + set(user, data string) error + get(user string) (string, error) + delete(user string) error +} + +// osKeyring is keyringBackend over the OS keystore: the security CLI on +// macOS, Credential Manager on Windows, the Secret Service over D-Bus on +// Linux. No cgo, so it works in the static release binaries. +type osKeyring struct{} + +func (osKeyring) set(user, data string) error { return keyring.Set(keyringService, user, data) } +func (osKeyring) get(user string) (string, error) { + return keyring.Get(keyringService, user) +} +func (osKeyring) delete(user string) error { return keyring.Delete(keyringService, user) } + +// keyringBackendFn is the seam tests drive. +var keyringBackendFn = func() keyringBackend { return osKeyring{} } + +// keyringAvailable reports whether an OS keystore is reachable on this +// machine. macOS ships the security CLI and Windows ships Credential Manager; +// Linux needs a D-Bus session bus with the Secret Service registered, which a +// headless machine lacks. +func keyringAvailable() bool { + if runtime.GOOS != "linux" { + return true + } + conn, err := dbus.SessionBus() + if err != nil { + return false + } + obj := conn.Object("org.freedesktop.DBus", dbus.ObjectPath("/org/freedesktop/DBus")) + var hasOwner bool + err = obj.Call("org.freedesktop.DBus.NameHasOwner", 0, "org.freedesktop.secrets").Store(&hasOwner) + return err == nil && hasOwner +} + +// openCredStore opens the machine's credential store: the OS keystore where +// one is available, otherwise the owner-only file store. A machine with no +// keystore at all is the fallback case; SPINLOOP_REMOTE_KEYSTORE=file chooses +// the file store even where a keystore is reachable; a keystore that opened +// and then fails is reported, not papered over by silently switching stores. +func openCredStore() (credentialStore, error) { + if os.Getenv(keyStoreEnvVar) != "file" && keyringAvailable() { + return credentialStore{kind: "keyring", kr: keyringBackendFn()}, nil + } + home, err := ConfigHome() + if err != nil { + return credentialStore{}, err + } + dir := filepath.Join(home, "keystore") + if err := os.MkdirAll(dir, 0o700); err != nil { + return credentialStore{}, err + } + return credentialStore{kind: "file", dir: dir}, nil +} + +// openCredStoreFn is the seam tests drive so they never touch the real +// keystore or the operator's config directory. +var openCredStoreFn = openCredStore + +func (s credentialStore) fileFor(region string) string { + return filepath.Join(s.dir, "remote-"+region+".json") +} + +func (s credentialStore) put(region string, cred StoredCredential) error { + data, err := json.Marshal(cred) + if err != nil { + return err + } + if s.kind == "keyring" { + if err := s.kr.set(region, string(data)); err != nil { + return err + } + return s.putIndex(append(s.indexRegions(), region)) + } + return os.WriteFile(s.fileFor(region), append(data, '\n'), 0o600) +} + +// indexRegions reads the stored-region index. A missing index is no +// regions; a broken one is reported, since an index that misleads the +// report misleads the operator about what is stored. +func (s credentialStore) indexRegions() []string { + if s.kind != "keyring" { + return nil + } + data, err := s.kr.get(keyringIndexUser) + if err != nil { + return nil + } + var regions []string + for _, region := range strings.Split(data, "\n") { + if region != "" { + regions = append(regions, region) + } + } + return regions +} + +func (s credentialStore) putIndex(regions []string) error { + if s.kind != "keyring" { + return nil + } + seen := map[string]bool{} + var out []string + for _, region := range regions { + if region != "" && !seen[region] { + seen[region] = true + out = append(out, region) + } + } + sort.Strings(out) + return s.kr.set(keyringIndexUser, strings.Join(out, "\n")) +} + +// get returns the stored credential for the region. A missing entry is +// (zero, nil), not an error: an absent credential is the normal case, and a +// broken one must not take down a command the ambient chain could still sign. +func (s credentialStore) get(region string) (StoredCredential, error) { + if s.kind == "keyring" { + data, err := s.kr.get(region) + if err != nil { + if errors.Is(err, keyring.ErrNotFound) { + return StoredCredential{}, nil + } + return StoredCredential{}, err + } + var cred StoredCredential + if err := json.Unmarshal([]byte(data), &cred); err != nil { + return StoredCredential{}, fmt.Errorf("parsing the stored credential for %s: %w", region, err) + } + return cred, nil + } + data, err := os.ReadFile(s.fileFor(region)) + if err != nil { + if os.IsNotExist(err) { + return StoredCredential{}, nil + } + return StoredCredential{}, err + } + var cred StoredCredential + if err := json.Unmarshal(data, &cred); err != nil { + return StoredCredential{}, fmt.Errorf("parsing the stored credential for %s: %w", region, err) + } + return cred, nil +} + +// delete removes the stored credential for the region. An absent entry is not +// an error, so a clear is idempotent. +func (s credentialStore) delete(region string) error { + if s.kind == "keyring" { + if err := s.kr.delete(region); err != nil && !errors.Is(err, keyring.ErrNotFound) { + return err + } + regions := s.indexRegions() + kept := regions[:0] + for _, r := range regions { + if r != region { + kept = append(kept, r) + } + } + return s.putIndex(kept) + } + if err := os.Remove(s.fileFor(region)); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + +// list returns every stored credential, sorted by region. An entry the index +// names but the store no longer holds (removed outside of spinloop) is +// skipped rather than reported, since the index lags a removal; an entry that +// cannot be parsed is an error naming its region, since a report that hides a +// broken entry misleads the operator about what is stored. +func (s credentialStore) list() ([]StoredCredential, error) { + var regions []string + if s.kind == "keyring" { + regions = s.indexRegions() + } else { + entries, err := os.ReadDir(s.dir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + for _, e := range entries { + name := e.Name() + if e.IsDir() || filepath.Ext(name) != ".json" || !strings.HasPrefix(name, "remote-") { + continue + } + region := name[len("remote-") : len(name)-len(".json")] + if region != "" { + regions = append(regions, region) + } + } + } + sort.Strings(regions) + creds := make([]StoredCredential, 0, len(regions)) + for _, region := range regions { + cred, err := s.get(region) + if err != nil { + return nil, err + } + if cred.AccessKeyID == "" { + continue + } + creds = append(creds, cred) + } + return creds, nil +} + +// StoreCredential stores cred in the machine's credential store, recording +// which store holds it. The secret stays in the store; nothing prints it. +func StoreCredential(cred StoredCredential) error { + s, err := openCredStoreFn() + if err != nil { + return err + } + cred.Store = s.kind + return s.put(cred.Region, cred) +} + +// LookupStoredCredential returns the stored control-plane credential for the +// region. A missing or unreadable entry is (zero, false), not an error. +func LookupStoredCredential(region string) (StoredCredential, bool) { + s, err := openCredStoreFn() + if err != nil { + return StoredCredential{}, false + } + cred, err := s.get(region) + if err != nil || cred.AccessKeyID == "" { + return StoredCredential{}, false + } + return cred, true +} + +// ListStoredCredentials returns every stored control-plane credential, sorted +// by region. +func ListStoredCredentials() ([]StoredCredential, error) { + s, err := openCredStoreFn() + if err != nil { + return nil, err + } + return s.list() +} + +// DeleteStoredCredential removes the stored control-plane credential for the +// region. An absent entry is not an error. +func DeleteStoredCredential(region string) error { + s, err := openCredStoreFn() + if err != nil { + return err + } + return s.delete(region) +} diff --git a/internal/remote/keystore_test.go b/internal/remote/keystore_test.go new file mode 100644 index 00000000..121a1aef --- /dev/null +++ b/internal/remote/keystore_test.go @@ -0,0 +1,270 @@ +package remote + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/zalando/go-keyring" +) + +// fakeKeyring is an in-memory keyringBackend for tests: it never touches the +// machine's real keystore. +type fakeKeyring struct { + items map[string]string +} + +func newFakeKeyring() *fakeKeyring { + return &fakeKeyring{items: map[string]string{}} +} + +func (f *fakeKeyring) set(user, data string) error { + f.items[user] = data + return nil +} + +func (f *fakeKeyring) get(user string) (string, error) { + data, ok := f.items[user] + if !ok { + return "", keyring.ErrNotFound + } + return data, nil +} + +func (f *fakeKeyring) delete(user string) error { + if _, ok := f.items[user]; !ok { + return keyring.ErrNotFound + } + delete(f.items, user) + return nil +} + +func testCred(region string) StoredCredential { + return StoredCredential{ + AccessKeyID: "AKIAEXAMPLE" + region[:3], + SecretAccessKey: "secret-for-" + region, + Account: "0", + User: "cloud-vm-llm-remote-cli", + Region: region, + StoredAt: time.Date(2026, 9, 8, 12, 0, 0, 0, time.UTC), + } +} + +func fileStore(t *testing.T, dir string) credentialStore { + t.Helper() + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + return credentialStore{kind: "file", dir: dir} +} + +func TestCredStoreKeyringRoundTrip(t *testing.T) { + kr := newFakeKeyring() + s := credentialStore{kind: "keyring", kr: kr} + + if cred, err := s.get("ap-southeast-2"); err != nil || cred.AccessKeyID != "" { + t.Fatalf("get of a missing entry = %q, %v; want absent, no error", cred.AccessKeyID, err) + } + if err := s.put("ap-southeast-2", testCred("ap-southeast-2")); err != nil { + t.Fatalf("put: %v", err) + } + got, err := s.get("ap-southeast-2") + if err != nil { + t.Fatalf("get: %v", err) + } + if got.SecretAccessKey != "secret-for-ap-southeast-2" || got.Region != "ap-southeast-2" { + t.Fatalf("get returned %+v", got) + } + // The entry sits under the region's user name, and the index names it. + if _, ok := kr.items["ap-southeast-2"]; !ok { + t.Fatalf("entry missing under its region user name; items: %v", kr.items) + } + if idx := kr.items[keyringIndexUser]; idx != "ap-southeast-2" { + t.Fatalf("index = %q; want the stored region", idx) + } + if err := s.delete("ap-southeast-2"); err != nil { + t.Fatalf("delete: %v", err) + } + if idx := kr.items[keyringIndexUser]; idx != "" { + t.Fatalf("index after delete = %q; want empty", idx) + } + if err := s.delete("ap-southeast-2"); err != nil { + t.Fatalf("delete of an absent entry must not fail: %v", err) + } +} + +func TestCredStoreKeyringRegionKeyingAndList(t *testing.T) { + kr := newFakeKeyring() + s := credentialStore{kind: "keyring", kr: kr} + + for _, region := range []string{"us-east-1", "ap-southeast-2"} { + if err := s.put(region, testCred(region)); err != nil { + t.Fatalf("put %s: %v", region, err) + } + } + if cred, err := s.get("eu-west-1"); err != nil || cred.AccessKeyID != "" { + t.Fatalf("get of a region with no entry = %+v, %v", cred, err) + } + creds, err := s.list() + if err != nil { + t.Fatalf("list: %v", err) + } + if len(creds) != 2 || creds[0].Region != "ap-southeast-2" || creds[1].Region != "us-east-1" { + t.Fatalf("list = %+v; want the two regions, sorted", creds) + } + // A region in the index whose entry was removed outside of spinloop is + // skipped, not reported. + if err := kr.delete("us-east-1"); err != nil { + t.Fatal(err) + } + creds, err = s.list() + if err != nil { + t.Fatalf("list: %v", err) + } + if len(creds) != 1 || creds[0].Region != "ap-southeast-2" { + t.Fatalf("list after an out-of-band removal = %+v", creds) + } +} + +func TestCredStoreFileFallback(t *testing.T) { + dir := filepath.Join(t.TempDir(), "keystore") + s := fileStore(t, dir) + + if err := s.put("us-east-1", testCred("us-east-1")); err != nil { + t.Fatalf("put: %v", err) + } + // Owner-only file in an owner-only directory: the store may hold a secret. + fileInfo, err := os.Stat(s.fileFor("us-east-1")) + if err != nil { + t.Fatalf("stat: %v", err) + } + if mode := fileInfo.Mode().Perm(); mode != 0o600 { + t.Fatalf("file mode = %o; want 0600", mode) + } + dirInfo, err := os.Stat(dir) + if err != nil { + t.Fatalf("stat dir: %v", err) + } + if mode := dirInfo.Mode().Perm(); mode != 0o700 { + t.Fatalf("dir mode = %o; want 0700", mode) + } + + got, err := s.get("us-east-1") + if err != nil || got.AccessKeyID != testCred("us-east-1").AccessKeyID { + t.Fatalf("get = %+v, %v", got, err) + } + if cred, err := s.get("ap-southeast-2"); err != nil || cred.AccessKeyID != "" { + t.Fatalf("get of a region with no entry = %+v, %v", cred, err) + } + creds, err := s.list() + if err != nil { + t.Fatalf("list: %v", err) + } + if len(creds) != 1 || creds[0].Region != "us-east-1" { + t.Fatalf("list = %+v", creds) + } + if err := s.delete("us-east-1"); err != nil { + t.Fatalf("delete: %v", err) + } + if cred, err := s.get("us-east-1"); err != nil || cred.AccessKeyID != "" { + t.Fatalf("entry still present after delete: %+v, %v", cred, err) + } +} + +func TestCredStoreFileBrokenEntry(t *testing.T) { + dir := t.TempDir() + s := fileStore(t, dir) + if err := os.WriteFile(s.fileFor("us-east-1"), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := s.get("us-east-1"); err == nil { + t.Fatal("get of a broken entry returned no error") + } + if _, err := s.list(); err == nil { + t.Fatal("list of a store with a broken entry returned no error") + } +} + +func TestStoredCredentialAPI(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", filepath.Join(t.TempDir(), ".config")) + fake := newFakeKeyring() + t.Cleanup(func() { openCredStoreFn = openCredStore }) + openCredStoreFn = func() (credentialStore, error) { + return credentialStore{kind: "keyring", kr: fake}, nil + } + + if _, ok := LookupStoredCredential("us-east-1"); ok { + t.Fatal("lookup before any store reported an entry") + } + cred := testCred("us-east-1") + if err := StoreCredential(cred); err != nil { + t.Fatalf("StoreCredential: %v", err) + } + got, ok := LookupStoredCredential("us-east-1") + if !ok || got.AccessKeyID != cred.AccessKeyID || got.Store != "keyring" { + t.Fatalf("LookupStoredCredential = %+v, %v", got, ok) + } + if _, ok := LookupStoredCredential("ap-southeast-2"); ok { + t.Fatal("lookup for a different region reported the stored entry") + } + all, err := ListStoredCredentials() + if err != nil || len(all) != 1 || all[0].Region != "us-east-1" { + t.Fatalf("ListStoredCredentials = %+v, %v", all, err) + } + if err := DeleteStoredCredential("us-east-1"); err != nil { + t.Fatalf("DeleteStoredCredential: %v", err) + } + if _, ok := LookupStoredCredential("us-east-1"); ok { + t.Fatal("entry still present after DeleteStoredCredential") + } +} + +func TestKeyStoreEnvVarForcesFileStore(t *testing.T) { + // The env var must choose the file store even on a machine where a + // keystore is reachable (here: regardless of keyringAvailable). + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("SPINLOOP_CONFIG_DIR", "") + t.Setenv(keyStoreEnvVar, "file") + t.Cleanup(func() { openCredStoreFn = openCredStore }) + + if err := StoreCredential(testCred("us-east-1")); err != nil { + t.Fatalf("StoreCredential: %v", err) + } + if _, err := os.Stat(filepath.Join(home, ".config", "spinloop", "keystore", "remote-us-east-1.json")); err != nil { + t.Fatalf("file store not used under the forced config dir: %v", err) + } + got, ok := LookupStoredCredential("us-east-1") + if !ok || got.Store != "file" { + t.Fatalf("LookupStoredCredential = %+v, %v; want the file store", got, ok) + } + if err := DeleteStoredCredential("us-east-1"); err != nil { + t.Fatalf("DeleteStoredCredential: %v", err) + } +} + +func TestStoredCredentialAPIFileFallback(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Cleanup(func() { openCredStoreFn = openCredStore }) + openCredStoreFn = func() (credentialStore, error) { + return fileStore(t, filepath.Join(home, ".config", "spinloop", "keystore")), nil + } + + if err := StoreCredential(testCred("us-east-1")); err != nil { + t.Fatalf("StoreCredential: %v", err) + } + got, ok := LookupStoredCredential("us-east-1") + if !ok || got.SecretAccessKey != "secret-for-us-east-1" { + t.Fatalf("LookupStoredCredential = %+v, %v", got, ok) + } + if err := DeleteStoredCredential("us-east-1"); err != nil { + t.Fatalf("DeleteStoredCredential: %v", err) + } + if _, ok := LookupStoredCredential("us-east-1"); ok { + t.Fatal("entry still present after DeleteStoredCredential") + } +} diff --git a/internal/remote/logs.go b/internal/remote/logs.go index f9d75505..4ede7998 100644 --- a/internal/remote/logs.go +++ b/internal/remote/logs.go @@ -146,7 +146,7 @@ func FetchLogs(ctx context.Context, cfg Config, q LogQuery) (LogResult, error) { if err != nil { return LogResult{}, err } - return fetchLogs(ctx, cloudwatchlogs.NewFromConfig(awsCfg), q) + return fetchLogs(ctx, cloudwatchlogs.NewFromConfig(awsCfg), q, cfg.Region) } // fetchLogs queries every group the source selects and merges the results. A @@ -154,7 +154,7 @@ func FetchLogs(ctx context.Context, cfg Config, q LogQuery) (LogResult, error) { // environment only ever ships to the group for the engine it runs — but a read // where every group is absent is the control plane predating log shipping, which // is reported. -func fetchLogs(ctx context.Context, api logsAPI, q LogQuery) (LogResult, error) { +func fetchLogs(ctx context.Context, api logsAPI, q LogQuery, region string) (LogResult, error) { groups, err := logGroupsFor(q.Source) if err != nil { return LogResult{}, err @@ -172,7 +172,7 @@ func fetchLogs(ctx context.Context, api logsAPI, q LogQuery) (LogResult, error) missing++ continue } - return LogResult{}, logsError(err) + return LogResult{}, logsError(region, err) } events = append(events, found...) omitted += dropped @@ -287,11 +287,12 @@ func groupNames(groups []logGroup) []string { } // logsError turns the two AWS failures an operator can act on into guidance: -// credentials that have expired, and credentials that resolve but may not read -// the logs. Anything else is passed through as it came. -func logsError(err error) error { +// credentials that have expired (naming the source that signed the read), and +// credentials that resolve but may not read the logs. Anything else is passed +// through as it came. +func logsError(region string, err error) error { if credentialError(err) { - return fmt.Errorf("reading logs failed: AWS credentials are expired or invalid — %s", refreshCredsHint) + return fmt.Errorf("reading logs failed: AWS credentials are expired or invalid — %s", credsRefreshHint(region)) } if accessDenied(err) { return fmt.Errorf( diff --git a/internal/remote/logs_test.go b/internal/remote/logs_test.go index b20fbe19..301c49cb 100644 --- a/internal/remote/logs_test.go +++ b/internal/remote/logs_test.go @@ -126,7 +126,7 @@ func TestFetchLogsMergesGroupsInTimeOrder(t *testing.T) { BootLogGroup(): {page("", event("c", 1500, "prod/i-1", "boot line"))}, }} - got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceAll}) + got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceAll}, "us-east-1") if err != nil { t.Fatal(err) } @@ -149,7 +149,7 @@ func TestFetchLogsOrdersEventsSharingAMillisecondByEventID(t *testing.T) { EngineLogGroup("vllm"): {page("", event("a", 1000, "prod/i-2", "first"))}, }} - got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceEngine}) + got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceEngine}, "us-east-1") if err != nil { t.Fatal(err) } @@ -168,7 +168,7 @@ func TestFetchLogsAsksForTheEnvironmentsStreamsAndWindow(t *testing.T) { end := start.Add(time.Hour) if _, err := fetchLogs(context.Background(), api, LogQuery{ Environment: "prod", Source: LogSourceBoot, Start: start, End: end, - }); err != nil { + }, "us-east-1"); err != nil { t.Fatal(err) } if len(api.calls) != 1 { @@ -198,7 +198,7 @@ func TestFetchLogsPagesAndKeepsTheMostRecentWithinTheLimit(t *testing.T) { got, err := fetchLogs(context.Background(), api, LogQuery{ Environment: "prod", Source: LogSourceBoot, Limit: 3, - }) + }, "us-east-1") if err != nil { t.Fatal(err) } @@ -224,7 +224,7 @@ func TestFetchLogsFiltersToOneInstance(t *testing.T) { got, err := fetchLogs(context.Background(), api, LogQuery{ Environment: "prod", Source: LogSourceBoot, Instance: "i-2", - }) + }, "us-east-1") if err != nil { t.Fatal(err) } @@ -243,7 +243,7 @@ func TestFetchLogsToleratesAGroupThatDoesNotExist(t *testing.T) { }, } - got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceEngine}) + got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceEngine}, "us-east-1") if err != nil { t.Fatalf("a missing group for the other engine should not fail the read: %v", err) } @@ -258,7 +258,7 @@ func TestFetchLogsReportsWhenNoGroupExistsAtAll(t *testing.T) { EngineLogGroup("vllm"): &cwltypes.ResourceNotFoundException{}, }} - _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceEngine}) + _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceEngine}, "us-east-1") if err == nil { t.Fatal("every group missing should be an error, not an empty result") } @@ -278,7 +278,7 @@ func (deniedErr) ErrorFault() smithy.ErrorFault { return smithy.FaultClient } func TestFetchLogsExplainsAccessDenied(t *testing.T) { api := &fakeLogs{errs: map[string]error{BootLogGroup(): deniedErr{}}} - _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}) + _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}, "us-east-1") if err == nil { t.Fatal("expected the denial to be reported") } @@ -290,7 +290,7 @@ func TestFetchLogsExplainsAccessDenied(t *testing.T) { func TestFetchLogsExplainsExpiredCredentials(t *testing.T) { api := &fakeLogs{errs: map[string]error{BootLogGroup(): errors.New("ExpiredToken: the token has expired")}} - _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}) + _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}, "us-east-1") if err == nil { t.Fatal("expected the expiry to be reported") } @@ -304,7 +304,7 @@ func TestFetchLogsReturnsNoEventsWithoutError(t *testing.T) { BootLogGroup(): {page("")}, }} - got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}) + got, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}, "us-east-1") if err != nil { t.Fatalf("an empty window is not an error: %v", err) } @@ -321,7 +321,7 @@ func TestFetchLogsStopsPagingAnUnboundedWindow(t *testing.T) { } api := &fakeLogs{pages: map[string][]*cloudwatchlogs.FilterLogEventsOutput{BootLogGroup(): endless}} - _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}) + _, err := fetchLogs(context.Background(), api, LogQuery{Environment: "prod", Source: LogSourceBoot}, "us-east-1") if err == nil { t.Fatal("an endlessly paging window should be reported, not truncated silently") } diff --git a/internal/remote/remote.go b/internal/remote/remote.go index 189150a4..13315f1f 100644 --- a/internal/remote/remote.go +++ b/internal/remote/remote.go @@ -305,7 +305,7 @@ func Deploy(ctx context.Context, cfg Config, dc DeployConfig, allowedCidr string } hint := "" if resp.StatusCode == http.StatusForbidden { - hint = forbiddenHint(detail) + hint = forbiddenHint(cfg.Region, detail) } return nil, fmt.Errorf("deploy failed (HTTP %d)%s: %s", resp.StatusCode, hint, detail) } @@ -396,7 +396,7 @@ func Start(ctx context.Context, cfg Config, progress func(string), onState func( default: hint := "" if resp.StatusCode == http.StatusForbidden { - hint = forbiddenHint(resp.Message) + hint = forbiddenHint(cfg.Region, resp.Message) } return nil, fmt.Errorf("start failed (HTTP %d, state %q)%s: %s", resp.StatusCode, resp.State, hint, resp.Message) @@ -411,7 +411,7 @@ func Status(ctx context.Context, cfg Config) (*Response, error) { return nil, err } if resp.StatusCode != http.StatusOK { - return nil, controlReplyError("status", resp) + return nil, controlReplyError(cfg.Region, "status", resp) } return resp, nil } @@ -425,7 +425,7 @@ func Stop(ctx context.Context, cfg Config) (*Response, error) { return nil, err } if resp.StatusCode != http.StatusOK { - return nil, controlReplyError("stop", resp) + return nil, controlReplyError(cfg.Region, "stop", resp) } return resp, nil } @@ -443,7 +443,7 @@ func Pause(ctx context.Context, cfg Config, force bool) (*Response, error) { return nil, err } if resp.StatusCode != http.StatusOK { - return nil, controlReplyError("pause", resp) + return nil, controlReplyError(cfg.Region, "pause", resp) } return resp, nil } @@ -509,7 +509,7 @@ func Keep(ctx context.Context, cfg Config, retainUntil time.Time) (*Response, er return nil, err } if resp.StatusCode != http.StatusOK { - return nil, controlReplyError("keep", resp) + return nil, controlReplyError(cfg.Region, "keep", resp) } return resp, nil } @@ -557,7 +557,7 @@ func call(ctx context.Context, cfg Config, method, rawURL string, body []byte) ( if err := json.Unmarshal(respBody, out); err != nil { hint := "" if status == http.StatusForbidden { - hint = forbiddenHint(string(respBody)) + hint = forbiddenHint(cfg.Region, string(respBody)) } return nil, fmt.Errorf("%s returned HTTP %d%s: %s", method, status, hint, truncate(string(respBody), 200)) @@ -614,7 +614,7 @@ func sign(ctx context.Context, req *http.Request, region string, body []byte) er if err != nil { if credentialError(err) { return fmt.Errorf( - "AWS credentials are expired or invalid: %w (%s)", err, refreshCredsHint) + "AWS credentials are expired or invalid: %w (%s)", err, credsRefreshHint(region)) } return fmt.Errorf( "resolving AWS credentials: %w (configure env credentials, a profile or an SSO session)", err) @@ -635,10 +635,26 @@ func truncate(s string, n int) string { } // refreshCredsHint is the fix appended when a request is rejected because the -// caller's AWS credentials are expired or invalid, rather than lacking -// permission — spinloop stores no credentials of its own to refresh. +// caller's ambient AWS credentials are expired or invalid, rather than lacking +// permission. const refreshCredsHint = "refresh your env credentials, profile, or SSO session" +// storedCredsHint is the refresh guidance when the stored control-plane key +// was the credential in use: refreshing the ambient credentials would not +// change what signs the request, so the fix is to store a new key. +const storedCredsHint = "run `spinloop remote auth --store` to create a new stored key" + +// credsRefreshHint picks the refresh guidance for a rejected request by the +// source of the credential that signed it: explicit ambient credentials win +// over the stored key in resolution, so they name themselves; a stored key in +// use names `spinloop remote auth --store`; anything else is ambient. +func credsRefreshHint(region string) string { + if _, ok := LookupStoredCredential(region); ok && !explicitAmbientCreds() { + return storedCredsHint + } + return refreshCredsHint +} + // credentialErrorCodes are the SDK/smithy error codes that mean the caller's // credentials are expired or otherwise invalid. The same tokens appear in the // body of an authorizer 403 on a Function URL, where the rejection arrives as @@ -684,12 +700,13 @@ func expiredCredsMarker(s string) bool { // forbiddenHint builds the guidance appended to an HTTP 403 from a control // endpoint: a rejection carrying an expired/invalid-credential marker tells the -// user to refresh their credentials; anything else keeps the IAM-permission -// hint, since a resolvable credential that lacks lambda:InvokeFunctionUrl fails -// the same way. -func forbiddenHint(detail string) string { +// user to refresh the credential that signed the request (source-aware — the +// stored key, if that is what signed it); anything else keeps the +// IAM-permission hint, since a resolvable credential that lacks +// lambda:InvokeFunctionUrl fails the same way. +func forbiddenHint(region, detail string) string { if expiredCredsMarker(detail) { - return fmt.Sprintf(" (AWS credentials are expired or invalid — %s)", refreshCredsHint) + return fmt.Sprintf(" (AWS credentials are expired or invalid — %s)", credsRefreshHint(region)) } return " (do your AWS credentials grant lambda:InvokeFunctionUrl?)" } @@ -699,14 +716,14 @@ func forbiddenHint(detail string) string { // credentials are expired/invalid or merely lack permission. Callers that treat // some non-200 statuses as expected (Start's 503 "still starting") must handle // those before falling through to this. -func controlReplyError(method string, resp *Response) error { +func controlReplyError(region, method string, resp *Response) error { detail := resp.Error if detail == "" { detail = resp.Message } hint := "" if resp.StatusCode == http.StatusForbidden { - hint = forbiddenHint(detail) + hint = forbiddenHint(region, detail) } return fmt.Errorf("%s returned HTTP %d%s: %s", method, resp.StatusCode, hint, truncate(detail, 200)) } @@ -785,7 +802,7 @@ func Stats(ctx context.Context, cfg Config) (*StatsResponse, error) { } hint := "" if out.StatusCode == http.StatusForbidden { - hint = forbiddenHint(detail) + hint = forbiddenHint(cfg.Region, detail) } return nil, fmt.Errorf("stats failed (HTTP %d)%s: %s", out.StatusCode, hint, detail) } @@ -827,7 +844,7 @@ func callStats(ctx context.Context, cfg Config) (*StatsResponse, error) { if err := json.Unmarshal(respBody, out); err != nil { hint := "" if resp.StatusCode == http.StatusForbidden { - hint = forbiddenHint(string(respBody)) + hint = forbiddenHint(cfg.Region, string(respBody)) } return nil, fmt.Errorf("stats returned HTTP %d%s: %s", resp.StatusCode, hint, truncate(string(respBody), 200)) diff --git a/internal/remote/remote_test.go b/internal/remote/remote_test.go index c896f6b3..c01d6205 100644 --- a/internal/remote/remote_test.go +++ b/internal/remote/remote_test.go @@ -684,6 +684,72 @@ func TestStop_ExpiredCredentials(t *testing.T) { } } +// noExplicitAmbient keeps explicitAmbientCreds false regardless of the +// machine the tests run on: no env credential, no profile, IMDS off. +func noExplicitAmbient(t *testing.T) { + t.Helper() + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") +} + +func TestCredsRefreshHintVariants(t *testing.T) { + const region = "ap-southeast-2" + + t.Run("no stored key names the ambient credentials", func(t *testing.T) { + noExplicitAmbient(t) + storeCredForTest(t, testCred("eu-west-1")) + if got := credsRefreshHint(region); got != refreshCredsHint { + t.Fatalf("hint = %q; want the ambient hint", got) + } + }) + + t.Run("a stored key in use names the store command", func(t *testing.T) { + noExplicitAmbient(t) + storeCredForTest(t, testCred(region)) + if got := credsRefreshHint(region); got != storedCredsHint { + t.Fatalf("hint = %q; want the stored-key hint", got) + } + }) + + t.Run("explicit ambient credentials win over the stored key", func(t *testing.T) { + noExplicitAmbient(t) + t.Setenv("AWS_ACCESS_KEY_ID", "AKIAENVENVENVENVENV") + storeCredForTest(t, testCred(region)) + if got := credsRefreshHint(region); got != refreshCredsHint { + t.Fatalf("hint = %q; want the ambient hint", got) + } + }) +} + +// A rejected request signed with the stored key must point at the store +// command, not at ambient credentials that play no part. +func TestStatus_ExpiredStoredCredentials(t *testing.T) { + const region = "eu-west-1" + noExplicitAmbient(t) + storeCredForTest(t, testCred(region)) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(expiredTokenBody)) + })) + defer server.Close() + + cfg := Config{StartURL: server.URL, StopURL: server.URL, Region: region} + _, err := Status(context.Background(), cfg) + if err == nil { + t.Fatal("expected an error for expired credentials") + } + if !strings.Contains(err.Error(), "expired or invalid") || + !strings.Contains(err.Error(), storedCredsHint) { + t.Errorf("expected the stored-key refresh hint, got %v", err) + } + if strings.Contains(err.Error(), refreshCredsHint) { + t.Errorf("the ambient hint must not appear for a stored-key rejection: %v", err) + } +} + // A JSON-parseable 403 that is not a credential problem keeps the IAM hint — // the parse-success path must classify the same way as the non-JSON one. func TestStatus_ForbiddenKeepsPermissionHint(t *testing.T) { diff --git a/internal/remote/seed.go b/internal/remote/seed.go index 8535359b..0d5ab10c 100644 --- a/internal/remote/seed.go +++ b/internal/remote/seed.go @@ -125,7 +125,7 @@ func seedCall(ctx context.Context, cfg Config, method, id string, body []byte, o if err := json.Unmarshal(respBody, out); err != nil { hint := "" if status == http.StatusForbidden { - hint = forbiddenHint(string(respBody)) + hint = forbiddenHint(cfg.Region, string(respBody)) } return status, fmt.Errorf("seed %s returned HTTP %d%s: %s", method, status, hint, truncate(string(respBody), 200)) diff --git a/openspec/changes/persistent-aws-creds/.openspec.yaml b/openspec/changes/archive/2026-09-08-persistent-aws-creds/.openspec.yaml similarity index 100% rename from openspec/changes/persistent-aws-creds/.openspec.yaml rename to openspec/changes/archive/2026-09-08-persistent-aws-creds/.openspec.yaml diff --git a/openspec/changes/persistent-aws-creds/design.md b/openspec/changes/archive/2026-09-08-persistent-aws-creds/design.md similarity index 56% rename from openspec/changes/persistent-aws-creds/design.md rename to openspec/changes/archive/2026-09-08-persistent-aws-creds/design.md index 1b91b215..46a4e45a 100644 --- a/openspec/changes/persistent-aws-creds/design.md +++ b/openspec/changes/archive/2026-09-08-persistent-aws-creds/design.md @@ -7,7 +7,7 @@ Every `spinloop remote` call signs with the caller's ambient AWS credentials: La **Goals:** - A long-lived AWS credential that survives between SSO log-ins, stored only in the OS keystore (or an owner-only file where no keystore exists), created and removed by `spinloop remote auth`. -- The stored key covers the day-to-day `remote` commands only; `bootstrap` and `bake` keep requiring ambient administrator credentials. +- The stored key covers the day-to-day control calls — the `remote` subcommands and the fleet's operations on remote environments; `bootstrap` and `bake` keep requiring ambient administrator credentials. - `--store` doubles as rotation and can run with the stored key alone, so no admin login is needed after the first store. - Zero behaviour change for anyone who never stores a key: the standard chain stays the fallback. @@ -20,37 +20,40 @@ Every `spinloop remote` call signs with the caller's ambient AWS credentials: La ## Decisions -### 1. Principal: an IAM user with an inline policy, not a role +### 1. Principal: an IAM user with a stack-owned managed policy, not a role -AWS access keys attach to IAM users only; a role yields at most 12-hour assumed sessions, which would not meet the ~90-day goal. The CDK stack therefore creates an IAM user, `cloud-vm-llm-remote-cli` (fixed name, not deployment-specific, so the public-repo identifier check is unaffected), with an inline policy attached at synth: +AWS access keys attach to IAM users only; a role yields at most 12-hour assumed sessions, which would not meet the ~90-day goal. The CDK stack therefore creates an IAM user, `cloud-vm-llm-remote-cli` (fixed name, not deployment-specific, so the public-repo identifier check is unaffected), with a managed policy attached at synth. The policy is a stack-owned `AWS::IAM::ManagedPolicy` resource rather than an inline one because IAM caps the aggregate size of a user's inline policies at 2,048 bytes and the seven control functions plus their log grants do not fit (a managed policy allows 6,144); being a stack resource, it still deletes with the stack, so a re-deploy of an older template version removes it: -- `lambda:InvokeFunctionUrl` on the seven function-URL ARNs (`startUrl.functionUrlArn` et al.). -- `logs:DescribeLogStreams`, `logs:FilterLogEvents`, `logs:GetLogEvents` on the runner and boot log-group ARNs (`remote logs`). -- `cloudformation:DescribeStacks` on `this.stackArn` (control-plane discovery by `deploy` and `remote auth`). +- `lambda:InvokeFunctionUrl`, with the `FunctionUrlAuthType=AWS_IAM` condition, on the seven control functions' ARNs (referenced through each URL resource's `FunctionArn` attribute — the URL's own ARN carries a generated id CDK does not expose) — one statement, the same grant `grantInvokeUrl` renders per function. +- The companion `lambda:InvokeFunction` grant with the `InvokedViaFunctionUrl` condition, on the same seven ARNs, one statement — also part of `grantInvokeUrl`'s canonical form. +- `logs:DescribeLogStreams`, `logs:FilterLogEvents`, `logs:GetLogEvents` on the runner and boot log-group ARNs and their streams (`remote logs`). +- `cloudformation:DescribeStacks` on the stack ARN (`Ref: AWS::StackId` — control-plane discovery by `deploy` and `remote auth`). - `pricing:GetProducts` (resource `*` — the Price List API has no resource-level scoping) so `status --cost` works with the stored key. -- `iam:GetUser`, `iam:ListAccessKeys`, `iam:CreateAccessKey`, `iam:DeleteAccessKey` on the user's own ARN — self-service rotation. +- `iam:GetUser`, `iam:ListAccessKeys`, `iam:CreateAccessKey`, `iam:DeleteAccessKey` on the user's own ARN, built from the `AWS::Partition`/`AWS::AccountId` pseudo parameters — self-service rotation. The ARN is not the user's own attribute: the policy attaches to the user, so referencing the user would be a dependency cycle. -Alternatives considered: a customer-managed policy (a named, reusable policy is nicer in the IAM console, but it outlives stack deletion unless specially handled — an inline policy deletes with the stack, which is what a re-deploy of an older template version should do); having the user assume a role per call (defeats the ~90-day goal). +Alternatives considered: an inline policy (the 2,048-byte cap — the first attempt with CDK's per-function grant pairs rendered 3.5 KB and failed the deploy with `ServiceLimitExceeded`); having the user assume a role per call (defeats the ~90-day goal). The user name is a constant on the Go side, not a stack output: `remote auth --store` confirms the user exists with `iam:GetUser` before doing anything, which is also the failure path for control planes deployed before this change. -### 2. Keystore backend: `99designs/keyring`, with a file fallback +### 2. Keystore backend: `zalando/go-keyring`, with a file fallback -The OS keystore is accessed through the `99designs/keyring` library (Keychain on macOS, Credential Manager on Windows, Secret Service on Linux). Where the platform keystore is unavailable (headless Linux without a D-Bus Secret Service), the entry is stored in an owner-only file `/keystore/remote-.json` (0600 in a 0700 directory, the same treatment the repo already gives files that may hold secrets). The entry records: access key id, secret, account, user name, region, stored-at timestamp, and which store holds it. `--store` and the no-flag report say which store was used. +The OS keystore is accessed through the `zalando/go-keyring` library: it shells out to the `security` CLI on macOS, uses Credential Manager on Windows, and the Secret Service over D-Bus on Linux (pure Go, via godbus). The more widely used `99designs/keyring` was rejected because its macOS backend requires cgo while the release build pins CGO_ENABLED=0 — the shipped binary would silently lack the Keychain backend and every machine would fall back to the file store. `zalando` compiles statically; a `CGO_ENABLED=0` darwin cross-build is part of the verification. -The entry is keyed by region (`spinloop-remote-` in the keyring). Keying by (account, region) would be more precise but is a chicken-and-egg: resolving the account requires credentials, which is exactly what the lookup is for. One control plane per account per region is the norm; if two accounts are bootstrapped in the same region, the last stored entry wins and the account recorded in the entry makes the mismatch visible in the report. +Where the platform keystore is unavailable (headless Linux without a D-Bus Secret Service), or where `SPINLOOP_REMOTE_KEYSTORE=file` selects the file store — a headless macOS session whose keychain is locked or unreachable, and the test suite — the entry is stored in an owner-only file `/keystore/remote-.json` (0600 in a 0700 directory, the same treatment the repo already gives files that may hold secrets). The entry records: access key id, secret, account, user name, region, stored-at timestamp, and which store holds it. `--store` and the no-flag report say which store was used. -Alternatives: `zalando/go-keyring` (less actively maintained); hand-rolled Security.framework/wincred wrappers (cross-platform burden for nothing). +The entry is keyed by region: the keyring service is `spinloop-remote` and the region is the entry's user name. The OS keystores offer no way to list a service's entries, so the store keeps a non-secret index entry (the stored regions, one per line) to back the no-flag report; a region the index names but the store no longer holds (removed outside of spinloop) is skipped rather than reported. Keying by (account, region) would be more precise but is a chicken-and-egg: resolving the account requires credentials, which is exactly what the lookup is for. One control plane per account per region is the norm; if two accounts are bootstrapped in the same region, the last stored entry wins and the account recorded in the entry makes the mismatch visible in the report. + +Alternatives: `99designs/keyring` (cgo macOS backend conflicts with the static release build); hand-rolled Security.framework/wincred wrappers (cross-platform burden for nothing). ### 3. Precedence at the two choke points `LoadAWSConfig` becomes the single decision point: -1. If the process environment carries explicit AWS credentials (`AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` set) or an explicit profile selection (`AWS_PROFILE`, `AWS_SHARED_CREDENTIALS_FILE`, `AWS_CONFIG_FILE`), load the default chain exactly as today — these win over a stored key. This preserves the existing behaviour where a Spinloop's `.env`/`ENV` injects credentials into the process environment before AWS work (`applySpinloopEnv`), and it lets an operator override the stored key for debugging. +1. If the process environment carries explicit AWS credentials (`AWS_ACCESS_KEY_ID` set) or an explicit profile selection (`AWS_PROFILE` set), load the default chain exactly as today — these win over a stored key. This preserves the existing behaviour where a Spinloop's `.env`/`ENV` injects credentials into the process environment before AWS work (`applySpinloopEnv`), and it lets an operator override the stored key for debugging. The chain-plumbing variables (`AWS_SHARED_CREDENTIALS_FILE`, `AWS_CONFIG_FILE`) do not count as explicit: they name where a source lives, not which credential is selected, and counting them would disable the stored key for anyone who has merely relocated their config files. 2. Otherwise, if a stored entry exists for the region, load the default config with an explicit static credentials provider for it (`awsconfig.WithCredentialsProvider`). This skips the rest of the chain, so the stored key beats shared config, SSO sessions, and IMDS. 3. Otherwise, today's behaviour: plain default chain. -`sign()` already calls `LoadAWSConfig`, so Function URL signing, log reading, discovery, and bake polling all inherit this with no per-caller changes. `bootstrap` and `bake` are the exception: they call an ambient-only variant (the current `LoadAWSConfig` body), never consulting the keystore. +`sign()` already calls `LoadAWSConfig`, so Function URL signing, log reading, discovery, and bake polling all inherit this with no per-caller changes. The fleet's remote-node operations — the dashboard and `fleet start`/`stop`/`status`/`keep` on remote environments — sign through the same `sign()` and `LoadAWSConfig`, so they inherit the stored key too; a fleet unit test pins that with a status call where the stored entry is the only resolvable credential. The pricing call (`status --cost`) resolves its credential the same way but keeps the us-east-1 endpoint, since the Price List API is global. `bootstrap` and `bake` are the exception: they call an ambient-only variant (the current `LoadAWSConfig` body), never consulting the keystore. The expired-credentials hint is source-aware: when the stored key was the credential in use, the "refresh" guidance says `spinloop remote auth --store` instead of "refresh your SSO session". @@ -84,10 +87,10 @@ A control plane deployed before this change has no such user. Re-running `spinlo ## Risks / Trade-offs - [A long-lived unattended key in a keystore is a standing credential] → the policy is scoped to day-to-day control only (no stack create/delete, no bake, no instance creation); `--clear` deletes the AWS-side key; rotation is one command. -- [Keyring access can prompt for the OS password on some platforms (e.g. a locked macOS Keychain item, Linux Secret Service unlock)] → the prompt comes from the OS, not spinloop; the file fallback avoids keystore access entirely where no keystore exists, and the report says which store is in use. +- [Keyring access can prompt for the OS password on some platforms (e.g. a locked macOS Keychain item, Linux Secret Service unlock)] → the prompt comes from the OS, not spinloop; the file fallback avoids keystore access entirely where no keystore exists, `SPINLOOP_REMOTE_KEYSTORE=file` chooses it where a keystore is reachable but unusable, and the report says which store is in use. - [Two keys per user cap with multiple machines] → the cap is checked before creating; the error names the fix (`--clear` on the other machine, or delete via the console). Rotation on one machine deletes only the key that machine's entry held. - [Same region, two accounts] → last stored entry wins; the recorded account in the no-flag report makes this visible. -- [`99designs/keyring` is a new dependency] → it is the only new Go module, used in one place (`internal/remote`); its Linux path needs D-Bus only when actually used. +- [`zalando/go-keyring` is a new dependency] → it is the only new Go module, used in one place (`internal/remote`); its Linux path needs a D-Bus session bus only when actually used. - [CDK rollback: an older template re-deployed deletes the user and its keys] → expected behaviour; stored entries then 403 and the hint says to re-run bootstrap and `--store`. ## Migration Plan diff --git a/openspec/changes/persistent-aws-creds/proposal.md b/openspec/changes/archive/2026-09-08-persistent-aws-creds/proposal.md similarity index 71% rename from openspec/changes/persistent-aws-creds/proposal.md rename to openspec/changes/archive/2026-09-08-persistent-aws-creds/proposal.md index cafa9016..c2c1ebe7 100644 --- a/openspec/changes/persistent-aws-creds/proposal.md +++ b/openspec/changes/archive/2026-09-08-persistent-aws-creds/proposal.md @@ -11,7 +11,7 @@ Every `spinloop remote` call is signed with the caller's ambient AWS credentials - `--clear` removes the stored entry and deletes the access key on the AWS side (best effort, so a cleared key does not linger). - With no flag it reports what is stored — account, region, user, access key id, when stored — never the secret. - The CDK control-plane stack (`remote/lib/llm-stack.ts`) gains an IAM user (`cloud-vm-llm-remote-cli`) carrying a narrow policy: invoke the seven control-plane function URLs, read the `/cloud-vm-llm/*` CloudWatch log groups, `cloudformation:DescribeStacks` on the stack, and manage its own access keys. Access keys attach to IAM users, not roles, so this is the principal a long-lived keypair can belong to. -- Credential resolution for `spinloop remote` (and fleet's remote nodes, which share the same client) consults the OS keystore for the target region. Explicit AWS environment credentials or a named profile override the stored key; otherwise the stored key takes precedence over the rest of the standard chain (shared config, SSO sessions, IMDS), which remains the fallback. +- Credential resolution for every signed control call — each `spinloop remote` subcommand, and the fleet's operations on remote environments (dashboard, start/stop/status/keep), which sign through the same client — consults the OS keystore for the target region. Explicit AWS environment credentials or a named profile override the stored key; otherwise the stored key takes precedence over the rest of the standard chain (shared config, SSO sessions, IMDS), which remains the fallback. `SPINLOOP_REMOTE_KEYSTORE=file` selects the owner-only file store even where a keystore is reachable, for sessions without keychain access. - `bootstrap` and `bake` keep using ambient administrator credentials and explicitly ignore any stored key: they provision the control plane itself, and the stored key's policy does not cover them. - A control plane deployed before this change has no such user; `remote auth --store` then says to re-run `spinloop remote bootstrap` first. - Access keys do not expire. The ~90-day lifetime is a rotation expectation: `--store` is the rotation, and it can be run with the stored key alone. @@ -29,7 +29,7 @@ Every `spinloop remote` call is signed with the caller's ambient AWS credentials ## Impact -- Go: `internal/remote` gains a keystore-backed credential source consulted at the two choke points (`LoadAWSConfig`, `sign`); new `cmd/spinloop/remote_auth.go`; `bootstrap` and `bake` explicitly bypass the stored key. New Go dependency: an OS keyring library (e.g. `99designs/keyring`), with an owner-only file under spinloop's config directory as the fallback where no keystore exists (headless Linux). -- CDK: `remote/lib/llm-stack.ts` adds the IAM user, its policy, and a stack output naming the user; existing deployments need a re-bootstrap before `remote auth --store` works. +- Go: `internal/remote` gains a keystore-backed credential source consulted at the choke points (`LoadAWSConfig`, `sign`, the pricing call); new `cmd/spinloop/remote_auth.go`; `bootstrap` and `bake` explicitly bypass the stored key. New Go dependency: `zalando/go-keyring` (no cgo — the release build pins CGO_ENABLED=0), with an owner-only file under spinloop's config directory as the fallback where no keystore exists (headless Linux) or is selected by `SPINLOOP_REMOTE_KEYSTORE=file`. +- CDK: `remote/lib/llm-stack.ts` adds the IAM user and its inline policy (the name is a constant on the Go side, not a stack output); existing deployments need a re-bootstrap before `remote auth --store` works. - Docs: `docs/commands/remote.md`, `README.md`, `remote/README.md` — the credential story moves from "ambient only" to "stored key, ambient as fallback". - Public-repo constraint unchanged: the user name is fixed, not deployment-specific, so `scripts/check-no-cloud-identifiers.sh` has nothing new to catch. diff --git a/openspec/changes/persistent-aws-creds/specs/endpoint-provisioning/spec.md b/openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/endpoint-provisioning/spec.md similarity index 100% rename from openspec/changes/persistent-aws-creds/specs/endpoint-provisioning/spec.md rename to openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/endpoint-provisioning/spec.md diff --git a/openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md b/openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/remote-auth/spec.md similarity index 76% rename from openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md rename to openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/remote-auth/spec.md index 5c7d9df4..83b19edb 100644 --- a/openspec/changes/persistent-aws-creds/specs/remote-auth/spec.md +++ b/openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/remote-auth/spec.md @@ -12,7 +12,7 @@ When a credential is already stored for the region, `--store` SHALL rotate rathe When the control-plane IAM user does not exist — a control plane deployed before this capability — the command SHALL fail, naming `spinloop remote bootstrap` as the step to re-run first. -The secret SHALL never be printed in any output. Where no OS keystore is available on the machine, the entry MAY instead be stored in an owner-only file under the user's spinloop config directory, and the command SHALL say which store it used. +The secret SHALL never be printed in any output. Where no OS keystore is available on the machine, the entry MAY instead be stored in an owner-only file under the user's spinloop config directory. Setting `SPINLOOP_REMOTE_KEYSTORE` to `file` SHALL select that file store even where a keystore is reachable — a machine whose keystore is locked or unreachable, such as a headless session with no keychain access — and the command SHALL say which store it used in every case. #### Scenario: First store @@ -39,9 +39,14 @@ The secret SHALL never be printed in any output. Where no OS keystore is availab - **WHEN** the user runs `spinloop remote auth --store` on a machine with no OS keystore available - **THEN** the entry is stored in an owner-only file under the user's spinloop config directory instead, and the report says where +#### Scenario: The file store is forced + +- **WHEN** `SPINLOOP_REMOTE_KEYSTORE` is set to `file` and the user runs `spinloop remote auth --store` on a machine with a reachable keystore +- **THEN** the entry is stored in the owner-only file under the user's spinloop config directory, not the keystore, and the report says which store was used + ### Requirement: Stored credentials resolve for control calls -For any `spinloop remote` subcommand that resolves AWS credentials for a target region, a stored credential for that region SHALL be used when no explicit AWS environment credentials and no explicit profile selection are present, and SHALL take precedence over the remaining standard credential sources — shared config files, SSO sessions, and instance metadata. Explicit AWS environment credentials (access key id, secret, and session token set in the process environment) or an explicit profile selection SHALL override the stored credential. When no credential is stored for the region, resolution SHALL fall back to the standard credential chain as before this capability. +For every signed control-plane request made for a target region — each `spinloop remote` subcommand, and the fleet's operations on remote environments (the fleet dashboard and `fleet start`, `stop`, `status`, and `keep`), which sign through the same client — a stored credential for that region SHALL be used when no explicit AWS environment credentials and no explicit profile selection are present, and SHALL take precedence over the remaining standard credential sources — shared config files, SSO sessions, and instance metadata. Explicit AWS environment credentials (access key id, secret, and session token set in the process environment) or an explicit profile selection SHALL override the stored credential. When no credential is stored for the region, resolution SHALL fall back to the standard credential chain as before this capability. `spinloop remote bootstrap` and `spinloop remote bake` SHALL NOT consult a stored credential: they provision the control plane itself and SHALL resolve from ambient sources only. @@ -50,6 +55,11 @@ For any `spinloop remote` subcommand that resolves AWS credentials for a target - **WHEN** a credential is stored for the region, no AWS environment credential or profile is set, and the ambient SSO session is absent or expired - **THEN** `spinloop remote status` signs with the stored credential and succeeds +#### Scenario: A fleet operation signs with the stored key + +- **WHEN** a credential is stored for the region, no other AWS credential is resolvable, and a fleet operation on that region's environment (a dashboard refresh, `fleet status`, `start`, or `stop`) issues its signed control call +- **THEN** the call is signed with the stored credential and the operation succeeds, exactly as the equivalent `spinloop remote` subcommand does + #### Scenario: Explicit environment credentials win - **WHEN** an AWS access key id and secret are set in the process environment and a credential is also stored for the region diff --git a/openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md b/openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/remote-endpoint/spec.md similarity index 97% rename from openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md rename to openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/remote-endpoint/spec.md index d7903dac..52975d15 100644 --- a/openspec/changes/persistent-aws-creds/specs/remote-endpoint/spec.md +++ b/openspec/changes/archive/2026-09-08-persistent-aws-creds/specs/remote-endpoint/spec.md @@ -136,6 +136,9 @@ credentials, resolved in this order: explicit AWS environment credentials or an explicit profile selection, then a stored control-plane credential for the target region (see the Remote Auth specification), then the remaining standard credential sources — shared config files, SSO sessions, and instance metadata. +The order applies to every signed control request, including the requests the +fleet issues on a remote environment's behalf, which sign through the same +client as the `remote` subcommands. Requests SHALL carry the hash of the request body so that a request with a payload is signed over that payload. The only credentials Spinloop stores of its own are the stored control-plane credentials, held in the OS keystore (or, diff --git a/openspec/changes/archive/2026-09-08-persistent-aws-creds/tasks.md b/openspec/changes/archive/2026-09-08-persistent-aws-creds/tasks.md new file mode 100644 index 00000000..30247ee1 --- /dev/null +++ b/openspec/changes/archive/2026-09-08-persistent-aws-creds/tasks.md @@ -0,0 +1,37 @@ +## 1. Keystore entry and storage backend + +- [x] 1.1 Add the stored-credential entry type (access key id, secret, account, user name, region, stored-at, store used) and the keyring operations (put, get, delete, list) in `internal/remote`, keyed by region under the keyring service `spinloop-remote`, with an owner-only file fallback (`/keystore/remote-.json`, 0600 in a 0700 dir) where no OS keystore exists; verify with unit tests covering put/get/delete/list on a fake keyring, region keying, and the file fallback's modes (go test ./internal/remote) +- [x] 1.2 Add the `zalando/go-keyring` dependency to go.mod (chosen over `99designs/keyring`: its macOS backend needs cgo, and the release build pins CGO_ENABLED=0 — the zalando backend shells out to the `security` CLI and builds statically); verify `go mod tidy` and `go build ./...` succeed, including a `CGO_ENABLED=0` darwin cross-build +- [x] 1.3 Add the `SPINLOOP_REMOTE_KEYSTORE=file` opt-out so the file store is chosen even where a keystore is reachable (headless macOS sessions without keychain access; the test suite's hermetic entries); verify with a unit test that the forced file store lands under the config directory (go test ./internal/remote) + +## 2. Credential resolution at the choke points + +- [x] 2.1 Split `LoadAWSConfig` (`internal/remote/aws.go`) into an ambient-only loader (today's body) and a loader that applies the precedence: explicit AWS env credentials or explicit profile selection → default chain as today; else a stored entry for the region → default config with a static credentials provider for it; else default chain; verify with unit tests for the full precedence matrix, including env-over-stored, profile-over-stored, stored-over-SSO/shared-config, and no-stored-falls-back +- [x] 2.2 Make the expired-credential hint in `sign()` (`internal/remote/remote.go`) source-aware: when the stored key was the credential in use, the refresh guidance says `spinloop remote auth --store`; verify with unit tests on both hint variants +- [x] 2.3 Point `bootstrap` and `bake` credential preflights (`cmd/spinloop/remote_bootstrap.go`) at the ambient-only loader so they never consult the keystore; verify with a unit test that a stored entry is not used by the bootstrap preflight and that the existing preflight tests still pass +- [x] 2.4 Route `GetOnDemandPrice` (`internal/remote/aws.go`) through the same stored-key precedence (endpoint stays us-east-1 — the pricing service is global — but the credential resolves for the environment's region, so the policy's `pricing:GetProducts` grant is what authorises the call); verify with a unit test that a stored key authorises the pricing config and an explicit env credential overrides it +- [x] 2.5 Fleet operations on remote environments (dashboard, start/stop/status/keep) sign through the same `sign()`/`LoadAWSConfig` choke points, so they inherit the stored key with no per-caller changes; verify with a unit test in `internal/fleet` that a remote node's status call succeeds when the only resolvable credential is a stored entry (file store forced, ambient chain empty) + +## 3. CDK: control-plane IAM user and policy + +- [x] 3.1 Add the `cloud-vm-llm-remote-cli` IAM user to `remote/lib/llm-stack.ts` with a stack-owned managed policy (IAM caps a user's inline policies at 2,048 bytes; the seven control functions plus log grants do not fit) granting: `lambda:InvokeFunctionUrl` (with the `FunctionUrlAuthType` condition) and the companion `lambda:InvokeFunction` (with the `InvokedViaFunctionUrl` condition) on the seven control functions' ARNs, `logs:DescribeLogStreams`/`FilterLogEvents`/`GetLogEvents` on the runner and boot log-group ARNs and their streams, `cloudformation:DescribeStacks` on the stack ARN, `pricing:GetProducts`, and `iam:GetUser`/`ListAccessKeys`/`CreateAccessKey`/`DeleteAccessKey` on the user's own ARN (built from pseudo parameters — referencing the user's own attribute would be a dependency cycle); verify by extending `remote/test/stack.test.ts` to assert the user, each grant, and the absence of any provisioning permission (no cloudformation:CreateStack, no imagebuilder, no ec2:RunInstances) and running `pnpm test` in `remote/` +- [x] 3.2 Confirm the fixed user name introduces no deployment identifier; verify `scripts/check-no-cloud-identifiers.sh` passes + +## 4. The `remote auth` command + +- [x] 4.1 Register `auth` under the `remote` command group in `cmd/spinloop/remote_auth.go` with mutually exclusive `--store`/`--clear` flags and a `--region` flag resolved like bootstrap's; verify `spinloop remote auth --help` renders per the cli-ux conventions and that `spinloop remote --help` lists `auth` +- [x] 4.2 Implement `--store` first-store: load ambient credentials only, confirm the control-plane user with `iam:GetUser` (absent → fail naming `spinloop remote bootstrap`), create the access key, verify it with STS `GetCallerIdentity` resolves to the caller's account (mismatch → not stored, fail), write the entry, confirm with account/region/user/key-id/store and never the secret; verify with unit tests through seams for the happy path, the missing-user path, and the account-mismatch path +- [x] 4.3 Implement `--store` rotation: when an entry exists for the region, create the replacement with the stored credential (separate IAM client, no ambient credential required), swap the entry, and delete the superseded key; check IAM's two-keys-per-user cap when not rotating and fail naming the fix; verify with unit tests including rotation with no ambient credentials configured +- [x] 4.4 Implement `--clear`: look up the entry (none → say so, exit 0), delete the access key on the AWS side with the stored credential best-effort (on failure still remove locally and report the key may linger), remove the local entry; verify with unit tests for the happy path and the AWS-deletion-failure path +- [x] 4.5 Implement the no-flag report: list every stored entry (account, region, user, access key id, stored-at, store) from the local store only, no AWS call, no secret; nothing stored → say so and name `--store`; verify with unit tests for both cases +- [x] 4.6 Add `auth` to tab completion where the `remote` subcommands are completed (`cmd/spinloop/complete.go`); verify the existing completion tests pass and `__complete` never errors + +## 5. Docs + +- [x] 5.1 Update `docs/commands/remote.md`: an `auth` section (store, report, clear, rotate) and the credentials section — stored key with ambient as fallback, explicit env/profile override, bootstrap/bake stay admin-only; verify the claims match the implemented behaviour by reading the final code +- [x] 5.2 Update `README.md` (the credentials paragraph) and `remote/README.md` (prerequisites: bootstrap still admin; day-to-day commands can use the stored key; existing control planes need a re-bootstrap before `--store`; rotate roughly every 90 days); verify by reading that no claim contradicts the implementation or the identifier check + +## 6. Verification + +- [x] 6.1 Run the full suite with coverage and the linters; verify `go test ./... -cover` keeps total coverage >= 80%, `go vet ./...` and `gofmt -l .` are clean, and `pnpm test` passes in `remote/` +- [x] 6.2 End-to-end against a real account: re-run `spinloop remote bootstrap`, run `spinloop remote auth --store`, log out of SSO, and verify `spinloop remote status` succeeds with the stored key, that `spinloop remote auth` reports the entry, that `--store` again rotates without admin credentials, and that `--clear` removes both the entry and the AWS-side key diff --git a/openspec/changes/persistent-aws-creds/tasks.md b/openspec/changes/persistent-aws-creds/tasks.md deleted file mode 100644 index 8aae2549..00000000 --- a/openspec/changes/persistent-aws-creds/tasks.md +++ /dev/null @@ -1,34 +0,0 @@ -## 1. Keystore entry and storage backend - -- [ ] 1.1 Add the stored-credential entry type (access key id, secret, account, user name, region, stored-at, store used) and the keyring operations (put, get, delete, list) in `internal/remote`, keyed by region with a `spinloop-remote-` name, backed by `99designs/keyring` with an owner-only file fallback (`/keystore/remote-.json`, 0600 in a 0700 dir) where no OS keystore exists; verify with unit tests covering put/get/delete/list on a fake keyring, region keying, and the file fallback's modes (go test ./internal/remote) -- [ ] 1.2 Add the `99designs/keyring` dependency to go.mod; verify `go mod tidy` and `go build ./...` succeed - -## 2. Credential resolution at the choke points - -- [ ] 2.1 Split `LoadAWSConfig` (`internal/remote/aws.go`) into an ambient-only loader (today's body) and a loader that applies the precedence: explicit AWS env credentials or explicit profile selection → default chain as today; else a stored entry for the region → default config with a static credentials provider for it; else default chain; verify with unit tests for the full precedence matrix, including env-over-stored, profile-over-stored, stored-over-SSO/shared-config, and no-stored-falls-back -- [ ] 2.2 Make the expired-credential hint in `sign()` (`internal/remote/remote.go`) source-aware: when the stored key was the credential in use, the refresh guidance says `spinloop remote auth --store`; verify with unit tests on both hint variants -- [ ] 2.3 Point `bootstrap` and `bake` credential preflights (`cmd/spinloop/remote_bootstrap.go`) at the ambient-only loader so they never consult the keystore; verify with a unit test that a stored entry is not used by the bootstrap preflight and that the existing preflight tests still pass - -## 3. CDK: control-plane IAM user and policy - -- [ ] 3.1 Add the `cloud-vm-llm-remote-cli` IAM user to `remote/lib/llm-stack.ts` with an inline policy granting: `lambda:InvokeFunctionUrl` on the seven function-URL ARNs, `logs:DescribeLogStreams`/`FilterLogEvents`/`GetLogEvents` on the runner and boot log-group ARNs, `cloudformation:DescribeStacks` on the stack ARN, `pricing:GetProducts`, and `iam:GetUser`/`ListAccessKeys`/`CreateAccessKey`/`DeleteAccessKey` on the user's own ARN; verify by extending `remote/test/stack.test.ts` to assert the user, each grant, and the absence of any provisioning permission (no cloudformation:CreateStack, no imagebuilder, no ec2:RunInstances) and running `pnpm test` in `remote/` -- [ ] 3.2 Confirm the fixed user name introduces no deployment identifier; verify `scripts/check-no-cloud-identifiers.sh` passes - -## 4. The `remote auth` command - -- [ ] 4.1 Register `auth` under the `remote` command group in `cmd/spinloop/remote_auth.go` with mutually exclusive `--store`/`--clear` flags and a `--region` flag resolved like bootstrap's; verify `spinloop remote auth --help` renders per the cli-ux conventions and that `spinloop remote --help` lists `auth` -- [ ] 4.2 Implement `--store` first-store: load ambient credentials only, confirm the control-plane user with `iam:GetUser` (absent → fail naming `spinloop remote bootstrap`), create the access key, verify it with STS `GetCallerIdentity` resolves to the caller's account (mismatch → not stored, fail), write the entry, confirm with account/region/user/key-id/store and never the secret; verify with unit tests through seams for the happy path, the missing-user path, and the account-mismatch path -- [ ] 4.3 Implement `--store` rotation: when an entry exists for the region, create the replacement with the stored credential (separate IAM client, no ambient credential required), swap the entry, and delete the superseded key; check IAM's two-keys-per-user cap when not rotating and fail naming the fix; verify with unit tests including rotation with no ambient credentials configured -- [ ] 4.4 Implement `--clear`: look up the entry (none → say so, exit 0), delete the access key on the AWS side with the stored credential best-effort (on failure still remove locally and report the key may linger), remove the local entry; verify with unit tests for the happy path and the AWS-deletion-failure path -- [ ] 4.5 Implement the no-flag report: list every stored entry (account, region, user, access key id, stored-at, store) from the local store only, no AWS call, no secret; nothing stored → say so and name `--store`; verify with unit tests for both cases -- [ ] 4.6 Add `auth` to tab completion where the `remote` subcommands are completed (`cmd/spinloop/complete.go`); verify the existing completion tests pass and `__complete` never errors - -## 5. Docs - -- [ ] 5.1 Update `docs/commands/remote.md`: an `auth` section (store, report, clear, rotate) and the credentials section — stored key with ambient as fallback, explicit env/profile override, bootstrap/bake stay admin-only; verify the claims match the implemented behaviour by reading the final code -- [ ] 5.2 Update `README.md` (the credentials paragraph) and `remote/README.md` (prerequisites: bootstrap still admin; day-to-day commands can use the stored key; existing control planes need a re-bootstrap before `--store`; rotate roughly every 90 days); verify by reading that no claim contradicts the implementation or the identifier check - -## 6. Verification - -- [ ] 6.1 Run the full suite with coverage and the linters; verify `go test ./... -cover` keeps total coverage >= 80%, `go vet ./...` and `gofmt -l .` are clean, and `pnpm test` passes in `remote/` -- [ ] 6.2 End-to-end against a real account: re-run `spinloop remote bootstrap`, run `spinloop remote auth --store`, log out of SSO, and verify `spinloop remote status` succeeds with the stored key, that `spinloop remote auth` reports the entry, that `--store` again rotates without admin credentials, and that `--clear` removes both the entry and the AWS-side key diff --git a/openspec/specs/endpoint-provisioning/spec.md b/openspec/specs/endpoint-provisioning/spec.md index cccac352..67782158 100644 --- a/openspec/specs/endpoint-provisioning/spec.md +++ b/openspec/specs/endpoint-provisioning/spec.md @@ -12,9 +12,11 @@ endpoints is provisioned through `spinloop remote bootstrap`. The system SHALL provide `spinloop remote bootstrap`, which deploys the account-level control plane that every remote environment reuses — the EC2 Image Builder pipelines, the environment-aware lifecycle Lambdas and their IAM, -and the shared S3 weights bucket, IAM roles and VPC — by obtaining the CDK -project shipped in `remote/` and driving its deploy of the control-plane stack. -Bootstrap SHALL NOT start any AMI bake; the bake is a separate +and the shared S3 weights bucket, IAM roles and VPC, and the IAM user that +holds the long-lived control-plane credential (see the Remote Auth +specification) together with its policy — by obtaining the CDK project shipped +in `remote/` and driving its deploy of the control-plane stack. Bootstrap SHALL +NOT start any AMI bake; the bake is a separate `spinloop remote bake` step. Bootstrap SHALL NOT create any Elastic IP or EC2 instance, and SHALL NOT register an environment; those belong to `spinloop remote deploy`. Bootstrap SHALL NOT reimplement the infrastructure; @@ -29,6 +31,15 @@ signpost `spinloop remote bake` as the next step, ahead of lifecycle Lambdas, and the shared bucket/roles/VPC — with no Elastic IP or instance created and no AMI bake started +#### Scenario: The control-plane credential user is deployed + +- **WHEN** `spinloop remote bootstrap` completes +- **THEN** the control-plane IAM user exists with a policy covering the + day-to-day remote commands only — invoking the control URLs, reading the + control-plane log groups, describing the control-plane stack, and managing + its own access keys — and no permission to deploy, bake, or otherwise + provision AWS resources + #### Scenario: Bootstrap signposts the bake - **WHEN** `spinloop remote bootstrap` completes diff --git a/openspec/specs/remote-auth/spec.md b/openspec/specs/remote-auth/spec.md new file mode 100644 index 00000000..245b6db4 --- /dev/null +++ b/openspec/specs/remote-auth/spec.md @@ -0,0 +1,109 @@ +# Remote Auth Specification + +## Purpose + +Define how `spinloop remote auth` stores a long-lived control-plane AWS credential in the OS keystore, how that credential resolves in preference to other sources, and how it is reported, rotated, and cleared. + +## Requirements + +### Requirement: Storing a control-plane credential + +`spinloop remote auth --store` SHALL create an AWS access key for the control-plane IAM user created by the control-plane stack and store it in the OS keystore (Keychain, Credential Manager, or Secret Service), keyed by the target region. The stored entry SHALL record the access key id, the secret, the AWS account, the user name, the region, and when it was stored. Before storing, the command SHALL verify that the new key resolves to the same AWS account as the caller's credentials, and SHALL NOT store a key that resolves to a different account. + +When a credential is already stored for the region, `--store` SHALL rotate rather than create a second entry: it SHALL create the replacement key with the stored credential itself, so that no administrator or other ambient credential is required, verify it, replace the stored entry, and delete the superseded access key on the AWS side. + +When the control-plane IAM user does not exist — a control plane deployed before this capability — the command SHALL fail, naming `spinloop remote bootstrap` as the step to re-run first. + +The secret SHALL never be printed in any output. Where no OS keystore is available on the machine, the entry MAY instead be stored in an owner-only file under the user's spinloop config directory. Setting `SPINLOOP_REMOTE_KEYSTORE` to `file` SHALL select that file store even where a keystore is reachable — a machine whose keystore is locked or unreachable, such as a headless session with no keychain access — and the command SHALL say which store it used in every case. + +#### Scenario: First store + +- **WHEN** the user runs `spinloop remote auth --store` in an account with a bootstrapped control plane and no stored credential for the region +- **THEN** an access key is created for the control-plane user and stored in the OS keystore for that region, and the confirmation names the account, region, and access key id without printing the secret + +#### Scenario: A key that resolves elsewhere is not stored + +- **WHEN** the newly created key resolves to a different AWS account than the caller's credentials +- **THEN** the key is not stored and the command fails saying so + +#### Scenario: Rotation needs no administrator credential + +- **WHEN** a credential is already stored for the region, no other AWS credential is configured, and the user runs `spinloop remote auth --store` +- **THEN** the stored credential is used to create the replacement key, the stored entry is swapped to the new key, and the superseded access key is deleted on the AWS side + +#### Scenario: A control plane without the user + +- **WHEN** the user runs `spinloop remote auth --store` against a control plane deployed before the control-plane user existed +- **THEN** the command fails, naming `spinloop remote bootstrap` as the step to re-run first + +#### Scenario: No keystore on the machine + +- **WHEN** the user runs `spinloop remote auth --store` on a machine with no OS keystore available +- **THEN** the entry is stored in an owner-only file under the user's spinloop config directory instead, and the report says where + +#### Scenario: The file store is forced + +- **WHEN** `SPINLOOP_REMOTE_KEYSTORE` is set to `file` and the user runs `spinloop remote auth --store` on a machine with a reachable keystore +- **THEN** the entry is stored in the owner-only file under the user's spinloop config directory, not the keystore, and the report says which store was used + +### Requirement: Stored credentials resolve for control calls + +For every signed control-plane request made for a target region — each `spinloop remote` subcommand, and the fleet's operations on remote environments (the fleet dashboard and `fleet start`, `stop`, `status`, and `keep`), which sign through the same client — a stored credential for that region SHALL be used when no explicit AWS environment credentials and no explicit profile selection are present, and SHALL take precedence over the remaining standard credential sources — shared config files, SSO sessions, and instance metadata. Explicit AWS environment credentials (access key id, secret, and session token set in the process environment) or an explicit profile selection SHALL override the stored credential. When no credential is stored for the region, resolution SHALL fall back to the standard credential chain as before this capability. + +`spinloop remote bootstrap` and `spinloop remote bake` SHALL NOT consult a stored credential: they provision the control plane itself and SHALL resolve from ambient sources only. + +#### Scenario: The stored key signs between log-ins + +- **WHEN** a credential is stored for the region, no AWS environment credential or profile is set, and the ambient SSO session is absent or expired +- **THEN** `spinloop remote status` signs with the stored credential and succeeds + +#### Scenario: A fleet operation signs with the stored key + +- **WHEN** a credential is stored for the region, no other AWS credential is resolvable, and a fleet operation on that region's environment (a dashboard refresh, `fleet status`, `start`, or `stop`) issues its signed control call +- **THEN** the call is signed with the stored credential and the operation succeeds, exactly as the equivalent `spinloop remote` subcommand does + +#### Scenario: Explicit environment credentials win + +- **WHEN** an AWS access key id and secret are set in the process environment and a credential is also stored for the region +- **THEN** the command signs with the environment credentials, not the stored one + +#### Scenario: A named profile wins + +- **WHEN** an explicit profile is selected and a credential is stored for the region +- **THEN** the command signs with the profile's credentials, not the stored one + +#### Scenario: No stored key falls back as before + +- **WHEN** no credential is stored for the region +- **THEN** credential resolution behaves exactly as it did before this capability + +#### Scenario: Bootstrap ignores the stored key + +- **WHEN** a credential is stored for the region and the user runs `spinloop remote bootstrap` +- **THEN** bootstrap resolves its credentials from ambient sources only, not from the stored credential + +### Requirement: Reporting and clearing stored credentials + +`spinloop remote auth` with no flag SHALL report every stored credential — the account, region, user name, access key id, and when it was stored — without contacting AWS and without printing the secret. When nothing is stored, it SHALL say so and name `--store` as the way to store one. + +`spinloop remote auth --clear` SHALL remove the stored credential for the target region and SHALL additionally delete the access key on the AWS side, using the stored credential, so that a cleared key does not linger in the account. If the AWS-side deletion cannot be made, the local entry SHALL still be removed and the failure reported, with a note that the key may still exist on the AWS side. + +#### Scenario: Status reports what is stored + +- **WHEN** a credential is stored and the user runs `spinloop remote auth` +- **THEN** the output lists the account, region, user name, access key id, and when it was stored, no AWS call is made, and the secret is not printed + +#### Scenario: Status with nothing stored + +- **WHEN** no credential is stored and the user runs `spinloop remote auth` +- **THEN** the output says none is stored and names `--store` as the way to store one + +#### Scenario: Clear removes both sides + +- **WHEN** the user runs `spinloop remote auth --clear` and a credential is stored +- **THEN** the stored entry is removed and the access key is deleted on the AWS side + +#### Scenario: Clear still removes locally when the AWS deletion fails + +- **WHEN** the user runs `spinloop remote auth --clear` and the AWS-side deletion fails +- **THEN** the local entry is still removed, and the command reports that the key may still exist on the AWS side diff --git a/openspec/specs/remote-endpoint/spec.md b/openspec/specs/remote-endpoint/spec.md index 4da9a3fb..9a3c523d 100644 --- a/openspec/specs/remote-endpoint/spec.md +++ b/openspec/specs/remote-endpoint/spec.md @@ -8,8 +8,8 @@ what to serve from a Spinloop: the `spinloop remote` command group. ### Requirement: Remote command group The system SHALL provide a `remote` command group with the subcommands -`bootstrap`, `bake`, `start`, `stop`, `restart`, `status`, `deploy`, `ls`, -`metrics`, and `keep`. `start`, `stop`, `restart`, `status`, `metrics` and +`bootstrap`, `bake`, `auth`, `start`, `stop`, `restart`, `status`, `deploy`, +`ls`, `metrics`, and `keep`. `start`, `stop`, `restart`, `status`, `metrics` and `deploy` each take an optional Spinloop path: `start` SHALL boot the endpoint and block until it is serving, then perform a quick TCP probe of the inference endpoint — if the probe fails, a warning is @@ -39,8 +39,10 @@ account-level AWS control plane (once per account) by obtaining and driving the CDK project, and takes its own flags rather than a Spinloop path (see the Endpoint Provisioning specification). `bake` SHALL start an AMI bake for each runner named, and takes runner names rather than a Spinloop path (see the -Endpoint Provisioning specification). An unrecognised subcommand SHALL fail -naming the accepted ones. +Endpoint Provisioning specification). `auth` SHALL store, report, and clear the +long-lived control-plane credential, and takes its own flags rather than a +Spinloop path (see the Remote Auth specification). An unrecognised subcommand +SHALL fail naming the accepted ones. #### Scenario: Starting the endpoint @@ -117,8 +119,14 @@ naming the accepted ones. #### Scenario: Bake is a recognised subcommand - **WHEN** the user runs `spinloop remote bake llamacpp` -- **THEN** the command is dispatched to the bake flow rather than reported as - unknown +- **THEN** the command is dispatched to the bake flow rather than + reported as unknown + +#### Scenario: Auth is a recognised subcommand + +- **WHEN** the user runs `spinloop remote auth` +- **THEN** the command is dispatched to the credential store, report, and clear + flow rather than reported as unknown #### Scenario: Unknown subcommand @@ -281,9 +289,19 @@ it. ### Requirement: Authenticated control requests Requests to the control URLs SHALL be signed with the caller's own AWS -credentials, resolved from the standard credential chain, and SHALL carry the -hash of the request body so that a request with a payload is signed over that -payload. Spinloop SHALL NOT store AWS credentials of its own. +credentials, resolved in this order: explicit AWS environment credentials or an +explicit profile selection, then a stored control-plane credential for the +target region (see the Remote Auth specification), then the remaining standard +credential sources — shared config files, SSO sessions, and instance metadata. +The order applies to every signed control request, including the requests the +fleet issues on a remote environment's behalf, which sign through the same +client as the `remote` subcommands. +Requests SHALL carry the hash of the request body so that a request with a +payload is signed over that payload. The only credentials Spinloop stores of its +own are the stored control-plane credentials, held in the OS keystore (or, +where no keystore exists, an owner-only file under the user's config directory) +and created or removed by `spinloop remote auth` (see the Remote Auth +specification). Every control subcommand — `start`, `stop`, `status`, `deploy`, and `metrics` — SHALL treat a non-success reply from the control endpoint as a failure: it SHALL @@ -293,8 +311,9 @@ result as though the call succeeded. A rejected request SHALL be reported with an actionable cause. When the request is rejected because the caller's AWS credentials are expired or invalid, the command SHALL say to refresh them (env credentials, a profile, or an SSO -session), distinct from the case where the credentials are resolvable but may -lack permission to invoke the endpoint. +session; `spinloop remote auth --store` where a stored credential was in use), +distinct from the case where the credentials are resolvable but may lack +permission to invoke the endpoint. #### Scenario: A request carrying a body is signed over it @@ -321,6 +340,13 @@ lack permission to invoke the endpoint. - **THEN** the command reports the failure with its status and cause, and does not present the empty reply as a successful result +#### Scenario: The stored credential signs when the ambient chain has none + +- **WHEN** a control-plane credential is stored for the region, no AWS + environment credential or profile is set, and no other standard-chain source + is available +- **THEN** the control request is signed with the stored credential + ### Requirement: Deploying what the endpoint serves `spinloop remote deploy` SHALL derive the deployment from the Spinloop and its diff --git a/remote/README.md b/remote/README.md index 2e71fc8d..ab49b758 100644 --- a/remote/README.md +++ b/remote/README.md @@ -85,7 +85,10 @@ and the wake/idle lifecycle — see [docs/architecture.md](docs/architecture.md) ## Prerequisites -- An AWS account with admin (or equivalent) credentials configured locally +- An AWS account with admin (or equivalent) credentials configured locally — + for `bootstrap`, `bake`, and `deploy`. Day-to-day commands (`start`, + `status`, …) can instead sign with the stored control-plane credential from + `spinloop remote auth --store`, so they keep working between SSO log-ins - Node.js 22+ and [pnpm](https://pnpm.io) - The [`spinloop`](https://github.com/spinloop-ai/spinloop) CLI, which drives the endpoint @@ -128,11 +131,22 @@ it; endpoints come after that, one `spinloop remote deploy` per environment: ```sh spinloop remote bootstrap # once per account: control-plane stack + pipelines +spinloop remote auth --store # optional: store a day-to-day credential in the OS keystore spinloop remote bake # bakes the runner AMI(s); waits until they are available spinloop remote deploy # creates the Spinloop's REMOTE environment and says # what it serves; seeds the weights if missing ``` +`auth --store` creates an access key for the control plane's own IAM user and +keeps it in the machine's OS keystore, so the day-to-day commands sign without +a fresh SSO log-in. It needs a control plane that has that user: a control +plane deployed before this capability must be re-bootstrapped first (re-running +`spinloop remote bootstrap` is safe and updates the stack). Run `--store` +again to rotate — it swaps the key using the stored one alone, so no +administrator credential is needed, and deletes the superseded key; roughly +every 90 days is a sane cadence. `bootstrap` and `bake` themselves always run +on the administrator's credentials, never the stored key. + Under the hood, bootstrap and bake run this directory's own commands — usable by hand too: diff --git a/remote/lib/llm-stack.ts b/remote/lib/llm-stack.ts index 9ed40d7f..5ed91a0a 100644 --- a/remote/lib/llm-stack.ts +++ b/remote/lib/llm-stack.ts @@ -663,6 +663,74 @@ export class LlmStack extends cdk.Stack { const updateUrl = updateFn.addFunctionUrl({ authType: lambda.FunctionUrlAuthType.AWS_IAM }); + // The human-facing principal behind the CLI's long-lived credential: + // `spinloop remote auth --store` creates an access key for this user and + // keeps it in the operator's OS keystore, so day-to-day control calls + // work between SSO log-ins. The policy is day-to-day control only — + // invoke the control URLs, read the instance logs, discover the stack, + // price an instance, and manage this user's own access keys; bootstrap, + // bake, and provisioning stay with the administrator's credentials. + const remoteCliUserName = 'cloud-vm-llm-remote-cli'; + const remoteCliUser = new iam.User(this, 'RemoteCliUser', { userName: remoteCliUserName }); + // The grant is a customer managed policy, not an inline one: IAM caps the + // aggregate size of a user's inline policies at 2,048 characters, and the + // seven control functions plus their log groups do not fit in it (the + // managed-policy limit is 6,144). The invoke-url grant takes the form + // grantInvokeUrl renders — the two actions, the auth-type conditions, the + // backing functions' ARNs — merged into one statement per action. + const controlFunctionArns = [startUrl, stopUrl, deployUrl, statsUrl, seedUrl, envUrl, updateUrl].map( + (url) => url.functionArn, + ); + const controlLogGroupArns = [ + ...RUNNERS.map((runner) => engineLogGroups[runner].logGroupArn), + bootLogGroup.logGroupArn, + ]; + const remoteCliPolicy = new iam.ManagedPolicy(this, 'RemoteCliPolicy', { + statements: [ + new iam.PolicyStatement({ + actions: ['lambda:InvokeFunctionUrl'], + resources: controlFunctionArns, + conditions: { StringEquals: { 'lambda:FunctionUrlAuthType': 'AWS_IAM' } }, + }), + new iam.PolicyStatement({ + actions: ['lambda:InvokeFunction'], + resources: controlFunctionArns, + conditions: { Bool: { 'lambda:InvokedViaFunctionUrl': true } }, + }), + // GetLogEvents and DescribeLogStreams address the streams, so the + // grant covers the groups and their streams. + new iam.PolicyStatement({ + actions: ['logs:DescribeLogStreams', 'logs:FilterLogEvents', 'logs:GetLogEvents'], + resources: [...controlLogGroupArns, ...controlLogGroupArns.map((arn) => `${arn}:*`)], + }), + // AWS::StackId resolves to this stack's ARN. + new iam.PolicyStatement({ + actions: ['cloudformation:DescribeStacks'], + resources: [cdk.Fn.ref('AWS::StackId')], + }), + // The Price List API has no resource-level scoping. + new iam.PolicyStatement({ actions: ['pricing:GetProducts'], resources: ['*'] }), + // Self-service rotation: the stored key manages this user's own access + // keys, so `--store` can run with the stored key alone. The ARN is + // built from pseudo parameters rather than the user's attribute: the + // policy attaches to the user, so referencing the user would be a + // dependency cycle. + new iam.PolicyStatement({ + actions: ['iam:GetUser', 'iam:ListAccessKeys', 'iam:CreateAccessKey', 'iam:DeleteAccessKey'], + resources: [ + cdk.Fn.join('', [ + 'arn:', + cdk.Aws.PARTITION, + ':iam::', + cdk.Aws.ACCOUNT_ID, + `:user/${remoteCliUserName}`, + ]), + ], + }), + ], + }); + remoteCliUser.addManagedPolicy(remoteCliPolicy); + new events.Rule(this, 'IdleCheckRule', { description: 'Periodic idle sweep across every environment instance', schedule: events.Schedule.rate(cdk.Duration.minutes(5)), diff --git a/remote/test/stack.test.ts b/remote/test/stack.test.ts index 81e39a9c..bc1c3e2d 100644 --- a/remote/test/stack.test.ts +++ b/remote/test/stack.test.ts @@ -97,9 +97,17 @@ describe('environments (pure helpers)', () => { describe('LlmStack (control plane)', () => { let template: Template; + let cliStatements: Statement[]; beforeAll(() => { template = sharedTemplate(); }); + // The control-plane CLI user's inline policy statements (see the + // control-plane-user tests below). + beforeAll(() => { + cliStatements = statementsForResource(template, 'AWS::IAM::User', (_, u) => + (u as { Properties: { UserName: string } }).Properties.UserName === 'cloud-vm-llm-remote-cli', + ); + }); it('holds no EC2 instance and no persistent EBS volume', () => { template.resourceCountIs('AWS::EC2::Instance', 0); @@ -136,6 +144,122 @@ describe('LlmStack (control plane)', () => { } }); + it('creates the CLI user with one managed policy and no inline one', () => { + const users = template.findResources('AWS::IAM::User') as Record; + const user = Object.values(users).find((u) => u.Properties.UserName === 'cloud-vm-llm-remote-cli'); + expect(user).toBeDefined(); + expect(user.Properties.ManagedPolicyArns).toHaveLength(1); + expect(user.Properties.Policies).toBeUndefined(); + expect(cliStatements.length).toBeGreaterThan(0); + }); + + it('grants the CLI user invoke-url permission on all seven control functions, and nothing else in lambda', () => { + const actionsOf = (s: Statement): string[] => [s.Action].flat(); + // The invoke-url grant takes the form grantInvokeUrl renders — + // lambda:InvokeFunctionUrl (AWS_IAM auth condition) plus + // lambda:InvokeFunction (InvokedViaFunctionUrl condition), on the backing + // functions' ARNs — merged into one statement per action. + const urls = template.findResources('AWS::Lambda::Url') as Record; + const urlFunctionIds = new Set( + Object.values(urls).map((u) => u.Properties.TargetFunctionArn['Fn::GetAtt'][0]), + ); + expect(urlFunctionIds).toHaveLength(7); + // The grant names the functions' ARNs through each URL resource's + // FunctionArn attribute; resolve that back to the backing function so the + // assertion is on the functions, not the reference path. + const grantedFunctions = (s: Statement): Set => + new Set( + ([s.Resource].flat() as { 'Fn::GetAtt': [string, string] }[]).map((r) => { + const [id, attr] = r['Fn::GetAtt']; + return attr === 'FunctionArn' && urls[id] ? urls[id].Properties.TargetFunctionArn['Fn::GetAtt'][0] : id; + }), + ); + const invokeUrl = cliStatements.filter((s) => actionsOf(s).includes('lambda:InvokeFunctionUrl')); + expect(invokeUrl).toHaveLength(1); + expect(invokeUrl[0].Condition).toEqual({ StringEquals: { 'lambda:FunctionUrlAuthType': 'AWS_IAM' } }); + expect(grantedFunctions(invokeUrl[0])).toEqual(urlFunctionIds); + const invokeViaUrl = cliStatements.filter((s) => actionsOf(s).includes('lambda:InvokeFunction')); + expect(invokeViaUrl).toHaveLength(1); + expect(invokeViaUrl[0].Condition).toEqual({ Bool: { 'lambda:InvokedViaFunctionUrl': true } }); + expect(grantedFunctions(invokeViaUrl[0])).toEqual(urlFunctionIds); + const lambdaActions = cliStatements.flatMap(actionsOf).filter((a) => a.startsWith('lambda:')); + expect(lambdaActions.sort()).toEqual(['lambda:InvokeFunction', 'lambda:InvokeFunctionUrl']); + }); + + it('grants the CLI user log reading on the runner and boot groups, and their streams only', () => { + const actionsOf = (s: Statement): string[] => [s.Action].flat(); + const logsStatements = cliStatements.filter((s) => actionsOf(s).some((a) => a.startsWith('logs:'))); + expect(logsStatements).toHaveLength(1); + expect(actionsOf(logsStatements[0])).toEqual( + expect.arrayContaining(['logs:DescribeLogStreams', 'logs:FilterLogEvents', 'logs:GetLogEvents']), + ); + const controlGroupIds = Object.entries(template.findResources('AWS::Logs::LogGroup')).filter( + ([, g]) => { + const name = String((g as { Properties: { LogGroupName: string } }).Properties.LogGroupName); + return name.startsWith('/cloud-vm-llm/') && !name.includes('/lambda/') && name !== '/cloud-vm-llm/seed'; + }, + ); + expect( + controlGroupIds.map(([, g]) => (g as { Properties: { LogGroupName: string } }).Properties.LogGroupName).sort(), + ).toEqual(['/cloud-vm-llm/boot', '/cloud-vm-llm/llamacpp', '/cloud-vm-llm/vllm']); + // Each grant is a group's ARN (Fn::GetAtt) or a stream under it + // (Fn::Join of that GetAtt with ":*"). + const granted = ([logsStatements[0].Resource].flat() as Record[]).map((r) => { + const parts = r['Fn::Join'] ? (r['Fn::Join'] as [string, unknown[]])[1] : [r]; + return (parts[0] as { 'Fn::GetAtt': string[] })['Fn::GetAtt'][0]; + }); + expect(granted).toHaveLength(controlGroupIds.length * 2); + for (const [id] of controlGroupIds) { + expect(granted.filter((g) => g === id)).toHaveLength(2); + } + }); + + it('grants the CLI user DescribeStacks on this stack and pricing lookups only, in their services', () => { + const actionsOf = (s: Statement): string[] => [s.Action].flat(); + const cfn = cliStatements.find((s) => actionsOf(s).includes('cloudformation:DescribeStacks')); + expect(cfn).toBeDefined(); + expect(cfn!.Resource).toEqual({ Ref: 'AWS::StackId' }); + expect(actionsOf(cfn!)).toEqual(['cloudformation:DescribeStacks']); + const pricing = cliStatements.find((s) => actionsOf(s).includes('pricing:GetProducts')); + expect(pricing).toBeDefined(); + expect(actionsOf(pricing!)).toEqual(['pricing:GetProducts']); + }); + + it('lets the CLI user manage its own access keys, scoped to its own ARN', () => { + const actionsOf = (s: Statement): string[] => [s.Action].flat(); + const iam = cliStatements.filter((s) => actionsOf(s).some((a) => a.startsWith('iam:'))); + expect(iam).toHaveLength(1); + expect(actionsOf(iam[0])).toEqual( + expect.arrayContaining(['iam:GetUser', 'iam:ListAccessKeys', 'iam:CreateAccessKey', 'iam:DeleteAccessKey']), + ); + // Built from pseudo parameters (not the user's attribute, which would be + // a dependency cycle), so assert on its shape: the user's own ARN. + const resource = [iam[0].Resource].flat()[0] as { 'Fn::Join': [string, unknown[]] }; + expect(resource['Fn::Join'][0]).toBe(''); + expect(resource['Fn::Join'][1]).toEqual([ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':user/cloud-vm-llm-remote-cli', + ]); + }); + + it('grants the CLI user no provisioning permission: no stack creation, no image builder, no instance launch', () => { + const actions = cliStatements.flatMap((s) => [s.Action].flat()); + expect(actions).not.toContain('cloudformation:CreateStack'); + expect(actions).not.toContain('cloudformation:DeleteStack'); + expect(actions.filter((a) => a.startsWith('imagebuilder:'))).toHaveLength(0); + expect(actions).not.toContain('ec2:RunInstances'); + const selfService = new Set([ + 'iam:GetUser', + 'iam:ListAccessKeys', + 'iam:CreateAccessKey', + 'iam:DeleteAccessKey', + ]); + expect(actions.filter((a) => a.startsWith('iam:') && !selfService.has(a))).toHaveLength(0); + }); + it('lets the deploy Lambda create environments (EIP, SG, key) and seed weights', () => { const fns = template.findResources('AWS::Lambda::Function'); const deploy = Object.values(fns).find((f) => @@ -549,6 +673,26 @@ describe('ImageStack', () => { }); }); +type Statement = { Action: string | string[]; Resource?: unknown; Condition?: unknown }; + +function statementsForResource( + template: Template, + type: string, + match: (logicalId: string, resource: any) => boolean, +): Statement[] { + const resources = template.findResources(type) as Record; + const [logicalId, resource] = Object.entries(resources).find(([id, r]) => match(id, r)) ?? []; + if (!logicalId) { + throw new Error(`no ${type} matched ${match}`); + } + const [arn] = (resource.Properties.ManagedPolicyArns ?? []) as { Ref?: string }[]; + const policy = arn?.Ref ? (template.toJSON().Resources as Record)[arn.Ref] : undefined; + if (!policy || policy.Type !== 'AWS::IAM::ManagedPolicy') { + throw new Error(`no managed policy attached to ${type} ${logicalId}`); + } + return policy.Properties.PolicyDocument.Statement; +} + function allPolicyStatements( template: Template, ): { Action: string | string[]; Resource?: unknown; Condition?: unknown }[] { diff --git a/remote/vitest.config.ts b/remote/vitest.config.ts index ad957b7c..86812116 100644 --- a/remote/vitest.config.ts +++ b/remote/vitest.config.ts @@ -6,7 +6,10 @@ export default defineConfig({ // trees run in one `pnpm test` so there is a single lane to keep green. include: ['test/**/*.test.ts', 'seeder/test/**/*.test.ts'], environment: 'node', - // Stack synth (with esbuild bundling of the Lambdas) is slow on first run. + // Stack synth (with esbuild bundling of the Lambdas) is slow on first run, + // and it happens in beforeAll — so the hooks get the same budget as the + // tests that follow them. testTimeout: 120_000, + hookTimeout: 120_000, }, });