From 300c6f812fa453769ce51c8681fc4e7df5e743cd Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Tue, 25 Aug 2026 03:36:43 +0100 Subject: [PATCH 1/2] feat(remote): create or rotate an environment's engine key outfit remote deploy --api-key-env VAR resolves the variable and sends its value to the deploy, which creates the environment's key secret or rotates it; the reply carries the action (created/rotated), never the value. A deploy with no flag leaves the stored key alone. fleet.yaml gains apiKeyEnv, the default variable holding a kind: remote node's engine key (a node's own engineTokenEnv still wins), so a launch resolves the key or fails before the agent starts. --- cmd/spinloop/remote.go | 35 +++- cmd/spinloop/remote_deploy_test.go | 95 ++++++++++ docs/commands/fleet.md | 14 ++ docs/commands/remote.md | 8 + docs/env-vars.md | 1 + internal/fleet/config.go | 27 +++ internal/fleet/config_test.go | 124 ++++++++++++ internal/fleet/select.go | 7 + internal/fleet/select_test.go | 42 +++++ internal/fleet/wake_test.go | 14 ++ internal/remote/remote.go | 24 ++- internal/remote/remote_test.go | 44 ++++- .../remote-external-api-key/.openspec.yaml | 2 + .../changes/remote-external-api-key/design.md | 177 ++++++++++++++++++ .../remote-external-api-key/proposal.md | 84 +++++++++ .../specs/environment-deployment/spec.md | 58 ++++++ .../specs/fleet-config/spec.md | 51 +++++ .../specs/fleet-routing/spec.md | 85 +++++++++ .../specs/remote-endpoint/spec.md | 73 ++++++++ .../changes/remote-external-api-key/tasks.md | 42 +++++ remote/docs/architecture.md | 11 ++ remote/lambda/deploy/index.ts | 20 +- remote/lambda/shared/environments.ts | 37 +++- remote/test/deploy-api-key.test.ts | 112 +++++++++++ remote/test/env-api-key.test.ts | 117 ++++++++++++ 25 files changed, 1285 insertions(+), 19 deletions(-) create mode 100644 openspec/changes/remote-external-api-key/.openspec.yaml create mode 100644 openspec/changes/remote-external-api-key/design.md create mode 100644 openspec/changes/remote-external-api-key/proposal.md create mode 100644 openspec/changes/remote-external-api-key/specs/environment-deployment/spec.md create mode 100644 openspec/changes/remote-external-api-key/specs/fleet-config/spec.md create mode 100644 openspec/changes/remote-external-api-key/specs/fleet-routing/spec.md create mode 100644 openspec/changes/remote-external-api-key/specs/remote-endpoint/spec.md create mode 100644 openspec/changes/remote-external-api-key/tasks.md create mode 100644 remote/test/deploy-api-key.test.ts create mode 100644 remote/test/env-api-key.test.ts diff --git a/cmd/spinloop/remote.go b/cmd/spinloop/remote.go index c0bad9da..32f1cb13 100644 --- a/cmd/spinloop/remote.go +++ b/cmd/spinloop/remote.go @@ -1346,6 +1346,7 @@ func remoteDeployCmd() *cobra.Command { allowedCidr string region string spinloopVersion string + apiKeyEnv string ) c := &cobra.Command{ Use: "deploy", @@ -1361,7 +1362,7 @@ installs the latest published release.`, ValidArgsFunction: aliasSlot, RunE: func(c *cobra.Command, args []string) error { resolve(c) - return runRemoteDeploy(args, dryRun, overwrite, reseed, allowedCidr, region, spinloopVersion) + return runRemoteDeploy(args, dryRun, overwrite, reseed, allowedCidr, region, spinloopVersion, apiKeyEnv) }, } fs := c.Flags() @@ -1371,11 +1372,12 @@ installs the latest published release.`, fs.StringVar(&allowedCidr, "allowed-cidr", "", "who may reach this environment's instance (default: your public IP as a /32, on first deploy)") fs.StringVar(®ion, "region", "", "AWS region of the control plane (default: AWS_REGION or us-east-1)") fs.StringVar(&spinloopVersion, "spinloop-version", "", "spinloop release the environment's instances install at boot (default: latest)") + fs.StringVar(&apiKeyEnv, "api-key-env", "", "the environment variable holding the API key to store for this environment (a variable name, never the key itself)") return c } // runRemoteDeploy is the body of `spinloop remote deploy`. -func runRemoteDeploy(args []string, dryRun, overwrite, reseed bool, allowedCidr, region, spinloopVersion string) error { +func runRemoteDeploy(args []string, dryRun, overwrite, reseed bool, allowedCidr, region, spinloopVersion, apiKeyEnv string) error { // deploy reads the Spinloop for what to serve, so unlike the other // subcommands it always needs one — the per-user remote config alone is not // enough. @@ -1390,6 +1392,20 @@ func runRemoteDeploy(args []string, dryRun, overwrite, reseed bool, allowedCidr, if err := applySpinloopEnv(sel, spinloopPath); err != nil { return err } + // A supplied key arrives the way every other secret does: as a reference + // to an environment variable, never a literal on the command line. The + // Spinloop's local environment has just been applied, so the variable is + // resolvable from the process environment; one set nowhere fails before + // anything is sent. + var apiKey string + if apiKeyEnv != "" { + apiKey = os.Getenv(apiKeyEnv) + if apiKey == "" { + return fmt.Errorf( + "--api-key-env: %s is not set: export it, or put it in the .env beside the Spinloop", + apiKeyEnv) + } + } dc, err := deployConfigFor(sel, spinloopPath) if err != nil { return err @@ -1451,6 +1467,11 @@ func runRemoteDeploy(args []string, dryRun, overwrite, reseed bool, allowedCidr, spinloopVer = "latest" } fmt.Printf(" spinloop: %s\n", spinloopVer) + // A key is worth stating too — it rotates — but the value is never + // printed, in the dry run or the report. + if apiKeyEnv != "" { + fmt.Printf(" api key: stored from %s\n", apiKeyEnv) + } if dryRun { return nil } @@ -1503,7 +1524,7 @@ func runRemoteDeploy(args []string, dryRun, overwrite, reseed bool, allowedCidr, fmt.Printf(" ingress: %s (your public IP; override with --allowed-cidr)\n", allowedCidr) } - resp, err := remoteDeployFn(ctx, cfg, dc, allowedCidr, reseed) + resp, err := remoteDeployFn(ctx, cfg, dc, allowedCidr, reseed, apiKey) if err != nil { return err } @@ -1518,6 +1539,14 @@ func runRemoteDeploy(args []string, dryRun, overwrite, reseed bool, allowedCidr, fmt.Println() fmt.Printf("deployed: environment %s at %s\n", env, resp.BaseURL) fmt.Printf("registered: %s\n", envConfigPath) + // A rotation is worth stating out loud: it invalidates the key a live + // agent may still be holding, and the action — never the value — is all + // the reply carries. + if resp.APIKeyAction == "rotated" { + fmt.Println("api key: rotated — the previous key no longer works") + } else if resp.APIKeyAction != "" { + fmt.Println("api key: created") + } if resp.Seeding { fmt.Printf("seeding the weights — follow it with `spinloop remote seed status %s`.\n", resp.SeedID) fmt.Println("Wait for it to finish before `spinloop remote start`, or the instance will") diff --git a/cmd/spinloop/remote_deploy_test.go b/cmd/spinloop/remote_deploy_test.go index 048bd819..dbc6b1b7 100644 --- a/cmd/spinloop/remote_deploy_test.go +++ b/cmd/spinloop/remote_deploy_test.go @@ -617,6 +617,101 @@ func TestRemoteDeploy_SpinloopVersion(t *testing.T) { }) } +// A supplied key is resolved from the environment the Spinloop's local +// environment populated, reaches the signed body as a request-scoped field, +// and the report says what happened to it — the action, never the value. +func TestRemoteDeploy_APIKeyEnvReachesTheRequest(t *testing.T) { + isolateConfig(t) + stubAWSEnv(t) + + var gotAPIKey string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + var got struct { + APIKey string `json:"apiKey"` + } + _ = json.Unmarshal(body, &got) + gotAPIKey = got.APIKey + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"deployed":true,"environment":"testenv","base_url":"http://198.51.100.9:8000/v1","apiKeyAction":"rotated"}`)) + })) + defer server.Close() + stubDeploySeams(t, server.URL, "undeployed") + writeDeployEnvSpinloop(t, "testenv") + t.Setenv("SHARED_KEY", "sk-supplied") + + out := captureStdout(t, func() { + if err := cmdRemoteDeploy([]string{"--api-key-env", "SHARED_KEY"}); err != nil { + t.Fatalf("cmdRemoteDeploy: %v", err) + } + }) + + if gotAPIKey != "sk-supplied" { + t.Errorf("the request carried apiKey %q, want the resolved value", gotAPIKey) + } + // The report names the action and never the value. + if !strings.Contains(out, "api key: rotated") { + t.Errorf("the report should say the key was rotated, got:\n%s", out) + } + if strings.Contains(out, "sk-supplied") { + t.Errorf("the report printed the key value:\n%s", out) + } +} + +// A named variable that is set nowhere fails before anything is sent. +func TestRemoteDeploy_APIKeyEnvUnsetFailsBeforeSending(t *testing.T) { + isolateConfig(t) + stubAWSEnv(t) + + sent := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sent = true + w.Write([]byte(`{"deployed":true}`)) + })) + defer server.Close() + stubDeploySeams(t, server.URL, "undeployed") + writeDeployEnvSpinloop(t, "testenv") + os.Unsetenv("NOWHERE_DEPLOY_KEY") + + err := cmdRemoteDeploy([]string{"--api-key-env", "NOWHERE_DEPLOY_KEY"}) + if err == nil || !strings.Contains(err.Error(), "NOWHERE_DEPLOY_KEY") { + t.Errorf("want an error naming the variable, got %v", err) + } + if sent { + t.Error("the deploy was sent despite the key failing to resolve") + } +} + +// A dry run with a key names the variable it will store from — never the +// value — and still touches nothing. +func TestRemoteDeploy_APIKeyEnvDryRunNamesTheVariable(t *testing.T) { + isolateConfig(t) + called := false + origDiscover := deployDiscoverFn + t.Cleanup(func() { deployDiscoverFn = origDiscover }) + deployDiscoverFn = func(context.Context, aws.Config, string) (remote.ControlPlane, error) { + called = true + return remote.ControlPlane{}, fmt.Errorf("must not be called") + } + writeDeployEnvSpinloop(t, "testenv") + t.Setenv("SHARED_KEY", "sk-supplied") + + out := captureStdout(t, func() { + if err := cmdRemoteDeploy([]string{"--dry-run", "--api-key-env", "SHARED_KEY"}); err != nil { + t.Fatalf("cmdRemoteDeploy --dry-run: %v", err) + } + }) + if called { + t.Error("--dry-run must touch nothing — not even discovery") + } + if !strings.Contains(out, "api key: stored from SHARED_KEY") { + t.Errorf("--dry-run should name the key's variable, got:\n%s", out) + } + if strings.Contains(out, "sk-supplied") { + t.Errorf("--dry-run printed the key value:\n%s", out) + } +} + // Guard the assumption deployConfigFor relies on: PROVIDER names the engine. func TestRunnerFor(t *testing.T) { for _, provider := range []string{"llamacpp", "vllm"} { diff --git a/docs/commands/fleet.md b/docs/commands/fleet.md index 0ca5dc02..fbea6cdc 100644 --- a/docs/commands/fleet.md +++ b/docs/commands/fleet.md @@ -155,6 +155,20 @@ its engine's key out: it says only that one is required. engineTokenEnv: GATED_ENGINE_KEY # to talk to its engine ``` +A `kind: remote` environment is always keyed, so it needs an engine key too — +its `engineTokenEnv` works as above, and a fleet-wide `apiKeyEnv` is the +default for every remote node that does not name one of its own. Either way the +launch fails before it starts the agent rather than pointing it at a gate it +cannot pass: + +```yaml +apiKeyEnv: REMOTE_ENGINE_KEY # the default for every kind: remote node +nodes: + - name: qwen + kind: remote + engineTokenEnv: OTHER_KEY # overrides it for this node +``` + ## A node that is down never blanks the view Fan-out is for observing, so a node that cannot be reached is a **row**, not a diff --git a/docs/commands/remote.md b/docs/commands/remote.md index c1cc4cdb..2a97d5ee 100644 --- a/docs/commands/remote.md +++ b/docs/commands/remote.md @@ -329,6 +329,13 @@ or a seed you want to run again. It starts the same ~20-minute seed instance a first deploy does, and re-downloads the weights, so it is opt-in rather than something to reach for by habit. +By default deploy stores the engine key the environment was first deployed +with — one generated per environment, held in its secret. To choose or change +it, `--api-key-env VAR` resolves `VAR` from your environment (or the `.env` +beside the Spinloop) and sends the value: the deploy creates or **rotates** the +environment's key, so the old value stops working — and the reply says which +happened, never the value itself. + ## Flags | Flag | Meaning | @@ -339,6 +346,7 @@ something to reach for by habit. | `-n`, `--dry-run` | `deploy` only: print what would be sent, without sending it | | `--reseed` | `deploy` only: re-fetch the weights even if they are already in S3 | | `--spinloop-version` | `deploy` only: the spinloop release fresh boots install (default: the latest published release) | +| `--api-key-env` | `deploy` only: name the environment variable holding the engine key to create or rotate; with no flag the stored key is kept | `bootstrap` and `bake` have their own too (`--ref`, `--dir`, `--region`, `--package-manager`, and `--no-wait` on bake) — see their sections above. diff --git a/docs/env-vars.md b/docs/env-vars.md index ea44c196..8d054298 100644 --- a/docs/env-vars.md +++ b/docs/env-vars.md @@ -17,6 +17,7 @@ from the environment or a `.env` beside the Spinloop — never written into an | `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). | +| *(fleet-wide, named by `apiKeyEnv`)* | `spinloop fleet`, `spinloop harness` | The default key for a `kind: remote` environment's engine, for every remote node that does not name its own `engineTokenEnv`. Resolved the same way. See [fleet](commands/fleet.md). | ## Remote (`spinloop remote`) diff --git a/internal/fleet/config.go b/internal/fleet/config.go index 88a20465..1433bc48 100644 --- a/internal/fleet/config.go +++ b/internal/fleet/config.go @@ -71,6 +71,15 @@ type Config struct { // should be used — spread the work, or consolidate it. Empty means // PreferIdle. Prefer Prefer `yaml:"prefer"` + // APIKeyEnv names the environment variable holding the key this fleet's + // remote nodes require, shared by every one of them: a remote's engine is + // always gated by its key, so a fleet of remotes can name the variable + // once rather than on each node — a node's own EngineTokenEnv overrides + // it. It is a remote-only default: a daemon gates on its own + // EngineTokenEnv, and a fleet-wide key must not start gating an engine + // that was never set up to accept one. As with every other secret in + // this file, the value is never written here. + APIKeyEnv string `yaml:"apiKeyEnv"` // Path is the file this was read from, and Dir its directory — the .env // beside it fills token references. @@ -267,6 +276,24 @@ func (c *Config) EngineToken(n NodeConfig) (string, error) { return c.resolveTokenEnv(n, n.EngineTokenEnv) } +// RemoteEngineToken resolves the key a remote node's engine requires: the +// variable the node names when it names one, else the fleet-wide APIKeyEnv. +// A remote's engine is always gated by its key, so a node that names no +// resolvable key fails here, before a launch depends on it — the way every +// other missing secret in this file is named, the node and the fix. +func (c *Config) RemoteEngineToken(n NodeConfig) (string, error) { + name := n.EngineTokenEnv + if name == "" { + name = c.APIKeyEnv + } + if name == "" { + return "", fmt.Errorf( + "node %q is a remote environment, so its engine key must be set: name the variable holding it, in this node's `engineTokenEnv` or the file's fleet-wide `apiKeyEnv` (%s)", + n.Name, c.Path) + } + return c.resolveTokenEnv(n, name) +} + // resolveTokenEnv reads one of a node's token references: the process // environment first, then the .env beside the fleet file — the precedence // spinloop uses everywhere, so an exported value wins and the .env only fills a diff --git a/internal/fleet/config_test.go b/internal/fleet/config_test.go index 235b9c58..b86f91fa 100644 --- a/internal/fleet/config_test.go +++ b/internal/fleet/config_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" + "gopkg.in/yaml.v3" + "github.com/spinloop-ai/spinloop/internal/daemon" ) @@ -344,6 +346,128 @@ nodes: } } +// The fleet-wide key is a remote-only default: resolved like every other +// secret in the file, a node's own reference overrides it, and a remote that +// names no resolvable key is named for it. +func TestRemoteEngineTokenResolution(t *testing.T) { + path := writeFleet(t, ` +apiKeyEnv: FLEET_KEY +nodes: + - name: shared + kind: remote + - name: own + kind: remote + engineTokenEnv: OWN_KEY + - name: box + host: box.local +`, "FLEET_KEY=from-dotenv\nOWN_KEY=own-dotenv\n") + cfg, err := Load(path) + if err != nil { + t.Fatal(err) + } + + shared, _ := cfg.Node("shared") + // The fleet's variable, from the .env beside the file. + if got, err := cfg.RemoteEngineToken(shared); err != nil || got != "from-dotenv" { + t.Errorf("key = %q, %v; want the fleet .env value", got, err) + } + // An exported value wins, as everywhere else in spinloop. + t.Setenv("FLEET_KEY", "exported") + if got, err := cfg.RemoteEngineToken(shared); err != nil || got != "exported" { + t.Errorf("key = %q, %v; want the exported value", got, err) + } + + // A node's own reference overrides the fleet-wide one. + own, _ := cfg.Node("own") + if got, err := cfg.RemoteEngineToken(own); err != nil || got != "own-dotenv" { + t.Errorf("key = %q, %v; want the node's own value", got, err) + } + + // A daemon is gated only by its own reference: the fleet-wide key does + // not reach it. + box, _ := cfg.Node("box") + if got, err := cfg.EngineToken(box); err != nil || got != "" { + t.Errorf("daemon engine token = %q, %v; want empty and no error", got, err) + } +} + +func TestRemoteEngineTokenUnsetNamesTheVariable(t *testing.T) { + path := writeFleet(t, ` +apiKeyEnv: NOWHERE_FLEET_KEY +nodes: + - name: shared + kind: remote +`, "") + cfg, err := Load(path) + if err != nil { + t.Fatal(err) + } + node, _ := cfg.Node("shared") + _, err = cfg.RemoteEngineToken(node) + if err == nil { + t.Fatal("an unset fleet key variable should be a config error") + } + for _, want := range []string{"NOWHERE_FLEET_KEY", "shared", "fleet.yaml"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should name %s, got %q", want, err) + } + } +} + +func TestRemoteEngineTokenMissingNamesBothPlaces(t *testing.T) { + path := writeFleet(t, ` +nodes: + - name: shared + kind: remote +`, "") + cfg, err := Load(path) + if err != nil { + t.Fatal(err) + } + node, _ := cfg.Node("shared") + _, err = cfg.RemoteEngineToken(node) + if err == nil { + t.Fatal("a remote naming no key anywhere should be a config error") + } + for _, want := range []string{"shared", "engineTokenEnv", "apiKeyEnv"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should mention %s, got %q", want, err) + } + } +} + +// A fleet file written for a newer spinloop — carrying fields this one does not +// know — must still parse: yaml.Unmarshal ignores unknown fields, so an older +// binary reading a newer file is a no-op, not an error. +func TestLoadIgnoresUnknownFleetFields(t *testing.T) { + path := writeFleet(t, ` +apiKeyEnv: SHARED_KEY +nodes: + - name: shared + kind: remote +`, "") + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + // The same file into a struct without the field, the way an older binary + // would read it. + var legacy struct { + Nodes []NodeConfig `yaml:"nodes"` + } + if err := yaml.Unmarshal(data, &legacy); err != nil { + t.Fatalf("a fleet file with an unknown field must parse: %v", err) + } + // And the current one keeps the field. + cfg, err := Load(path) + if err != nil { + t.Fatal(err) + } + if cfg.APIKeyEnv != "SHARED_KEY" { + t.Errorf("apiKeyEnv = %q, want SHARED_KEY", cfg.APIKeyEnv) + } +} + func TestPreferSetting(t *testing.T) { cases := map[string]Prefer{ "prefer: active\n": PreferActive, diff --git a/internal/fleet/select.go b/internal/fleet/select.go index 9421f7fe..d93574cc 100644 --- a/internal/fleet/select.go +++ b/internal/fleet/select.go @@ -369,7 +369,14 @@ func hostIsLoopback(host string) bool { // known. A gated engine whose // node names no variable fails here, before anything is launched: an agent that // cannot authenticate is worse than a message saying so. +// +// A remote is the constant case: its engine is always gated by its key, and +// the control plane reports the instance, never the gate, so its key is looked +// up whatever the status says — from the node's own reference or the fleet's. func (c *Config) engineKeyFor(n NodeConfig, status daemon.StatusResponse) (string, error) { + if n.Kind == KindRemote { + return c.RemoteEngineToken(n) + } if status.Engine == nil || !status.Engine.RequiresKey { return "", nil } diff --git a/internal/fleet/select_test.go b/internal/fleet/select_test.go index b722c487..3190a733 100644 --- a/internal/fleet/select_test.go +++ b/internal/fleet/select_test.go @@ -390,6 +390,48 @@ func TestEngineKeyResolution(t *testing.T) { } } +// A remote's engine is always gated by its key — the control plane reports the +// instance, never the gate — so its key is looked up whatever the status says, +// from the node's own reference or the fleet's, and a remote with no resolvable +// key fails before a launch depends on it. +func TestRemoteEngineKeyResolution(t *testing.T) { + cfg := &Config{Path: "fleet.yaml", Dir: t.TempDir(), APIKeyEnv: "FLEET_KEY"} + t.Setenv("FLEET_KEY", "sk-fleet") + t.Setenv("NODE_ENGINE_KEY", "sk-node") + // The control plane reports no engine gate at all. + empty := daemon.StatusResponse{} + + // The fleet's key, by default. + remote := NodeConfig{Name: "cloud", Kind: KindRemote} + if key, err := cfg.engineKeyFor(remote, empty); err != nil || key != "sk-fleet" { + t.Errorf("key = %q, %v; want sk-fleet", key, err) + } + + // The node's own reference overrides it. + own := NodeConfig{Name: "cloud", Kind: KindRemote, EngineTokenEnv: "NODE_ENGINE_KEY"} + if key, err := cfg.engineKeyFor(own, empty); err != nil || key != "sk-node" { + t.Errorf("key = %q, %v; want sk-node", key, err) + } + + // No key named anywhere fails, naming the node and both places to fix it. + cfg.APIKeyEnv = "" + _, err := cfg.engineKeyFor(NodeConfig{Name: "cloud", Kind: KindRemote}, empty) + if err == nil { + t.Fatal("a remote with no key named should fail") + } + for _, want := range []string{"cloud", "engineTokenEnv", "apiKeyEnv"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("message should mention %q, got: %v", want, err) + } + } + + // A daemon is untouched by the fleet's key: an ungated engine needs none. + daemonNode := NodeConfig{Name: "box", Kind: KindDaemon, Host: "box.local"} + if key, err := cfg.engineKeyFor(daemonNode, daemon.StatusResponse{Engine: &daemon.EngineEndpoint{Port: 8080}}); err != nil || key != "" { + t.Errorf("daemon key = %q, %v; want empty and no error", key, err) + } +} + // A node woken from a Spinloop reports the deploy config's model id, which is // not the ALIAS the client asked for — a Spinloop may take its model from a // preset and state no MODEL at all. Unless that id counts as a match, a second diff --git a/internal/fleet/wake_test.go b/internal/fleet/wake_test.go index c09320c5..25ecc3df 100644 --- a/internal/fleet/wake_test.go +++ b/internal/fleet/wake_test.go @@ -379,6 +379,20 @@ func TestWakeWithoutAKeyIsUngated(t *testing.T) { } } +// The wake path stays daemon-only: a remote is never woken — what it serves is +// set by `spinloop remote deploy`, a heavier flow a node start must not conflate. +// The refusal is the contract Wake relies on to move to its next candidate. +func TestRemoteRefusesToBeWoken(t *testing.T) { + n, err := NewRemoteNode("cloud", remote.Config{StartURL: "https://s", StopURL: "https://x", Region: "us-east-1"}) + if err != nil { + t.Fatal(err) + } + _, err = n.StartWith(context.Background(), &remote.DeployConfig{Runner: "llamacpp", ModelID: "m"}, "sk-key") + if err == nil || !strings.Contains(err.Error(), "not a node to be woken") { + t.Errorf("want the remote refusal, got %v", err) + } +} + // A variable that resolves to nothing fails before any engine is started. func TestWakeFailsOnAnUnresolvableKey(t *testing.T) { shortWake(t) diff --git a/internal/remote/remote.go b/internal/remote/remote.go index 3b642852..217e1994 100644 --- a/internal/remote/remote.go +++ b/internal/remote/remote.go @@ -222,6 +222,12 @@ type Response struct { ModelID string `json:"modelId"` ContextSize int `json:"contextSize"` WeightsPrefix string `json:"weightsPrefix"` + // APIKeyAction is what the deploy did to the environment's key secret, + // present only when the deploy supplied a key: "created" (a new secret) + // or "rotated" (an existing secret set to a new value, invalidating the + // old key). It carries the action, never the value — a deploy never + // returns the key itself. + APIKeyAction string `json:"apiKeyAction"` // RetainUntil is the instance's retention deadline, returned when the // Retain-Until tag is present (set-keep or start --keep). camelCase to // match the Lambda's JSON. @@ -264,10 +270,17 @@ type DeployConfig struct { // allowedCidr scopes who may reach this environment's instance; it is required // the first time and optional afterwards (empty leaves ingress alone). // reseed asks the control plane to fetch the weights even when they are -// already in S3. It is a property of this request, not of what the environment -// serves, so it rides beside allowedCidr rather than on DeployConfig — which is -// persisted verbatim, and would re-seed on every wake that read it back. -func Deploy(ctx context.Context, cfg Config, dc DeployConfig, allowedCidr string, reseed bool) (*Response, error) { +// already in S3. Both are properties of this request, not of what the +// environment serves, so they ride beside each other rather than on +// DeployConfig — which is persisted verbatim, and would re-seed on every wake +// that read it back. +// +// apiKey is an externally provided key to store as the environment's key, +// riding the same way: a property of this request, never persisted, and +// omitted entirely when empty, so a control plane that predates it sees the +// body it always saw. The reply's APIKeyAction says what happened to the +// secret — the action, never the value. +func Deploy(ctx context.Context, cfg Config, dc DeployConfig, allowedCidr string, reseed bool, apiKey string) (*Response, error) { if cfg.DeployURL == "" { return nil, fmt.Errorf( "no deploy_url configured: add the remote/ deployment's DeployUrl output to the remote config (or set SPINLOOP_REMOTE_DEPLOY_URL)") @@ -276,7 +289,8 @@ func Deploy(ctx context.Context, cfg Config, dc DeployConfig, allowedCidr string DeployConfig AllowedCidr string `json:"allowedCidr,omitempty"` Reseed bool `json:"reseed,omitempty"` - }{dc, allowedCidr, reseed}) + APIKey string `json:"apiKey,omitempty"` + }{dc, allowedCidr, reseed, apiKey}) if err != nil { return nil, err } diff --git a/internal/remote/remote_test.go b/internal/remote/remote_test.go index 6d0af1f9..964675ed 100644 --- a/internal/remote/remote_test.go +++ b/internal/remote/remote_test.go @@ -751,7 +751,7 @@ func TestDeploy_Success(t *testing.T) { cfg := Config{DeployURL: server.URL, Region: "eu-west-1"} dc := DeployConfig{Runner: "vllm", ModelID: "org/model"} - resp, err := Deploy(context.Background(), cfg, dc, "203.0.113.0/24", false) + resp, err := Deploy(context.Background(), cfg, dc, "203.0.113.0/24", false, "") if err != nil { t.Fatal(err) } @@ -765,8 +765,10 @@ func TestDeploy_Success(t *testing.T) { } // Omitted entirely when not asked for, so an older control plane sees the // body it always saw. - if strings.Contains(string(gotBody), "reseed") { - t.Errorf("reseed should be omitted when false: %s", gotBody) + for _, field := range []string{"reseed", "apiKey"} { + if strings.Contains(string(gotBody), field) { + t.Errorf("%s should be omitted when unset: %s", field, gotBody) + } } } @@ -781,7 +783,7 @@ func TestDeploy_SpinloopVersionReachesTheRequest(t *testing.T) { cfg := Config{DeployURL: server.URL, Region: "eu-west-1"} dc := DeployConfig{Runner: "vllm", ModelID: "org/model", SpinloopVersion: "1.26.1"} - if _, err := Deploy(context.Background(), cfg, dc, "", false); err != nil { + if _, err := Deploy(context.Background(), cfg, dc, "", false, ""); err != nil { t.Fatal(err) } if !strings.Contains(string(gotBody), `"spinloopVersion":"1.26.1"`) { @@ -802,7 +804,7 @@ func TestDeploy_SpinloopVersionOmittedWhenUnpinned(t *testing.T) { cfg := Config{DeployURL: server.URL, Region: "eu-west-1"} dc := DeployConfig{Runner: "vllm", ModelID: "org/model"} - if _, err := Deploy(context.Background(), cfg, dc, "", false); err != nil { + if _, err := Deploy(context.Background(), cfg, dc, "", false, ""); err != nil { t.Fatal(err) } if strings.Contains(string(gotBody), "spinloopVersion") { @@ -821,7 +823,7 @@ func TestDeploy_ReseedReachesTheRequest(t *testing.T) { cfg := Config{DeployURL: server.URL, Region: "eu-west-1"} dc := DeployConfig{Runner: "vllm", ModelID: "org/model"} - if _, err := Deploy(context.Background(), cfg, dc, "", true); err != nil { + if _, err := Deploy(context.Background(), cfg, dc, "", true, ""); err != nil { t.Fatal(err) } if !strings.Contains(string(gotBody), `"reseed":true`) { @@ -829,6 +831,32 @@ func TestDeploy_ReseedReachesTheRequest(t *testing.T) { } } +// A supplied key rides the signed body beside the other request-scoped fields +// — never on DeployConfig, so it is never persisted or read back. +func TestDeploy_APIKeyReachesTheRequest(t *testing.T) { + stubAWSEnv(t) + var gotBody []byte + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotBody, _ = io.ReadAll(r.Body) + w.Write([]byte(`{"deployed":true,"apiKeyAction":"rotated"}`)) + })) + defer server.Close() + + cfg := Config{DeployURL: server.URL, Region: "eu-west-1"} + dc := DeployConfig{Runner: "vllm", ModelID: "org/model"} + resp, err := Deploy(context.Background(), cfg, dc, "", false, "sk-supplied") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(gotBody), `"apiKey":"sk-supplied"`) { + t.Errorf("the key did not reach the request body: %s", gotBody) + } + // The action comes back without the value. + if resp.APIKeyAction != "rotated" { + t.Errorf("APIKeyAction = %q, want rotated", resp.APIKeyAction) + } +} + func TestDeploy_ExpiredCredentials(t *testing.T) { stubAWSEnv(t) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -838,14 +866,14 @@ func TestDeploy_ExpiredCredentials(t *testing.T) { defer server.Close() cfg := Config{DeployURL: server.URL, Region: "eu-west-1"} - if _, err := Deploy(context.Background(), cfg, DeployConfig{Runner: "vllm"}, "", false); err == nil || + if _, err := Deploy(context.Background(), cfg, DeployConfig{Runner: "vllm"}, "", false, ""); err == nil || !strings.Contains(err.Error(), "expired or invalid") { t.Errorf("expected deploy to fail with an expired-credentials error, got %v", err) } } func TestDeploy_MissingURL(t *testing.T) { - if _, err := Deploy(context.Background(), Config{}, DeployConfig{}, "", false); err == nil || + if _, err := Deploy(context.Background(), Config{}, DeployConfig{}, "", false, ""); err == nil || !strings.Contains(err.Error(), "no deploy_url") { t.Errorf("expected a missing-URL error, got %v", err) } diff --git a/openspec/changes/remote-external-api-key/.openspec.yaml b/openspec/changes/remote-external-api-key/.openspec.yaml new file mode 100644 index 00000000..e685d45e --- /dev/null +++ b/openspec/changes/remote-external-api-key/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-25 diff --git a/openspec/changes/remote-external-api-key/design.md b/openspec/changes/remote-external-api-key/design.md new file mode 100644 index 00000000..e50b963c --- /dev/null +++ b/openspec/changes/remote-external-api-key/design.md @@ -0,0 +1,177 @@ +# Design + +## Context + +Two independent seams already carry the pieces this change joins: + +- `fleet.yaml` names secrets by **reference only** — a node's `tokenEnv` and + `engineTokenEnv` are variable *names*, resolved from the process environment + first, then the `.env` beside the file (`internal/fleet/config.go`, + `resolveTokenEnv`). A reference that resolves to nothing is a configuration + error naming the variable. +- The deploy flow already carries **request-scoped** fields beside the + persisted config: `allowedCidr` and `reseed` ride the SigV4-signed body, are + read from the raw body in the Lambda, and are deliberately **not** part of the + `DeployConfig` that is stored verbatim to SSM and read back on every wake + (`internal/remote/remote.go` `Deploy`, `remote/lambda/deploy/index.ts`). +- The control plane mints each environment's key with `ensureEnvApiKey`, which + generates a random value on first creation and otherwise leaves the secret + alone (`remote/lambda/shared/environments.ts`). +- A fleet-routed launch already injects the chosen node's key into the agent + from `Choice.APIKey` (`cmd/spinloop/commands.go`, `main.go`), so once routing + resolves a remote's key the agent picks it up with no new injection code. + +See proposal.md for the motivation. + +## Goals / Non-Goals + +**Goals** + +- A fleet can name one key shared by its remote nodes, resolved with the same + reference discipline as every other secret in `fleet.yaml`. +- `spinloop remote deploy` can store a caller-chosen key in an environment's + secret, with explicit, visible rotation. +- The key travels only in the signed deploy body and in Secrets Manager — + never in `fleet.yaml`, `remote.json`, the persisted deploy-config, a reply, or + a log line. + +**Non-Goals** + +- No fleet-driven deploy that reads `fleet.yaml`'s `apiKeyEnv` automatically + (deploy does not read the fleet file today; that is a later, separate change + that can adopt the reference directly). +- No change to how a `REMOTE` Spinloop fetches its key at launch (the env Lambda + path is untouched — it now simply returns the shared key). +- No key material in any persistent spinloop-owned file. + +## Decisions + +### 1. Fleet field is `apiKeyEnv`, a remote-only default + +`Config` gains `APIKeyEnv string` (yaml `apiKeyEnv`), resolved by a small +helper that reuses the existing env-then-`.env` lookup but attributes an error +to the fleet file rather than a node. It is the **default key for remote +nodes only**: a remote's effective key is its own `engineTokenEnv` when named, +else the fleet's `apiKeyEnv`. Daemon nodes are unchanged — they still gate only +on their own `engineTokenEnv`. + +*Why remote-only:* the field's purpose is to share a key across remotes. Making +it a default for *all* nodes would let a fleet that adds `apiKeyEnv` (to share +a remote key) suddenly gate its daemon nodes with a key their engines were never +set up to accept, breaking them. Scoping it to remotes keeps the change additive +and opt-in. + +*Alternative considered:* a fleet-wide default for every node. Rejected for the +reason above. + +### 2. Deploy takes a variable reference, not a literal + +`spinloop remote deploy` gains `--api-key-env `. The CLI resolves `$VAR` +from the **process environment** — which `applySpinloopEnv` has already populated +with the Spinloop's `.env` and `ENV` lines — and sends the value. A named variable +set nowhere fails the deploy before anything is sent. + +*Why a reference:* a literal on the command line is visible in `ps` and shell +history. The codebase already refuses literal engine keys (the daemon's +`--api-token-file`, the engine's `--api-key-file`); a variable reference is the +consistent choice. + +*Alternatives considered:* `--api-key ` (rejected — `ps` exposure); +`--api-key-file ` (works, but the value crosses the signed network +anyway, so a file is more machinery than the reference needs, and the reference +reuses the `applySpinloopEnv` population for free). + +### 3. The key rides request-scoped, beside `allowedCidr` and `reseed` + +`Deploy`'s body struct gains `APIKey string \`json:"apiKey,omitempty"\`` +alongside the existing request-scoped fields; `Deploy` takes the resolved value +as a parameter. It is **not** added to `DeployConfig`, so it is never written to +the SSM deploy-config and never read back on a wake. `omitempty` keeps the +request byte-identical when no key is supplied, so an older control plane that +predates the field simply ignores it. + +*Why:* `DeployConfig` is persisted verbatim and re-sent on every wake. A key in +it would be stored in SSM and echoed by any read of the config. `allowedCidr` +and `reseed` solve the identical problem the identical way; the key joins them. + +### 4. The Lambda create-or-sets the secret, with explicit rotation + +`ensureEnvApiKey(env)` becomes `ensureEnvApiKey(env, providedKey)` and reports +what it did: + +- `providedKey` set, secret absent → `CreateSecretCommand` with that value + ("created"). +- `providedKey` set, secret present → `PutSecretValueCommand` with that value + ("rotated"). The old key is invalid immediately. +- `providedKey` empty → existing behaviour: create-and-generate if absent, + otherwise leave the secret **alone** ("unchanged"). It is never regenerated. + +The Lambda adds the action word (`apiKey: "created" | "rotated"`) to its reply — +the action, never the value — and the deploy report prints it +(` api key: rotated`), so a rotation out from under a live agent is visible +rather than surfacing later as 401s. + +*Why leave-alone-when-absent:* regenerating on a no-key redeploy would silently +invalidate a key the operator still holds. Leaving it alone is the safe default; +rotation is only ever explicit. + +### 5. Fleet key and deploy flag stay independent + +Both reference environment variables; the operator keeps them in sync by naming +the same variable in `fleet.yaml` (`apiKeyEnv: SHARED_KEY`) and on the deploy +flag (`--api-key-env SHARED_KEY`). Deploy does not read the fleet file, so there +is no automatic wiring. + +*Why:* the issue scopes these as "complementary rather than dependent". Adding +a `--fleet` flag to deploy (or auto-detecting `./fleet.yaml` and checking +membership) is new machinery for a benefit that only lands once fleet-driven +deploys exist. Keeping them independent now means that later change can adopt +the reference directly without reworking this one. + +### 6. A selected remote is always given a key, or the launch fails + +In `select.go`, `engineKeyFor` special-cases `n.Kind == KindRemote`: the key is +the node's own `engineTokenEnv` if named, else the fleet's `apiKeyEnv`; if +neither names a variable — or the variable it names is set nowhere — routing +fails before the agent launches, naming the node and what to set. A remote is +never reached ungated. The value flows into the existing `Choice.APIKey` → +`OPENAI_API_KEY` injection, so no new injection code is needed. Daemon handling +in `engineKeyFor` is unchanged. + +*Why fail early:* a remote whose key cannot be resolved would hand the agent an +endpoint that 401s. Failing before launch, naming what to set, matches the +existing "gated node with no key fails early" behaviour for daemons. + +## Risks / Trade-offs + +- **Rotation invalidates a live agent's key.** → The reply and report carry the + explicit action word ("rotated"), and `remote/docs/architecture.md` documents + that a key-supplied deploy replaces the environment's key. +- **The key leaks into a log or reply.** → Neither the Lambda's `console.log` + nor the Go deploy summary prints the value; the reply carries only the action + word. Tests assert the supplied key is absent from the reply body and from the + persisted deploy-config parameter. +- **Existing remotes hold minted keys the operator cannot read.** → Migration: + `spinloop remote env ` prints the current key, or a redeploy with + `--api-key-env` sets a known one. Documented in the migration plan. +- **New CLI against an old control plane.** → `omitempty` means a supplied key + is silently ignored (the key is still minted); the deploy succeeds. The gap is + one-directional and benign; deploying the Lambda closes it. +- **A fleet adds `apiKeyEnv` and a remote it lists now requires a key.** → That + is the opt-in. Such a remote previously 401'd at request time (the agent got + no key); it now either works or fails earlier with a clear message. Strictly + an improvement. An old binary reading a new fleet file is unaffected: + `yaml.Unmarshal` ignores the unknown `apiKeyEnv` field. + +## Migration Plan + +1. Ship the control-plane change (Lambda + `ensureEnvApiKey`) and the Go client + together or the Lambda first — the request field is `omitempty`, so an old + Lambda ignores it and a new Lambda with no key behaves exactly as today. +2. For a fleet that already lists remotes: to route to them, name the key. + Either add `apiKeyEnv` (or a per-node `engineTokenEnv`) in `fleet.yaml` + pointing at a variable the operator holds — obtainable with + `spinloop remote env ` — or redeploy the remotes with + `--api-key-env ` to set a known shared key. +3. Rollback: revert the code. The `apiKey` body field and the `apiKeyEnv` fleet + field are both ignored by the other side, so a mixed version state is safe. diff --git a/openspec/changes/remote-external-api-key/proposal.md b/openspec/changes/remote-external-api-key/proposal.md new file mode 100644 index 00000000..e3de451c --- /dev/null +++ b/openspec/changes/remote-external-api-key/proposal.md @@ -0,0 +1,84 @@ +## Why + +A remote endpoint's API key is always minted by the control plane — at deploy, +`ensureEnvApiKey` creates `cloud-vm-llm//api-key` in Secrets Manager with a +random value, and there is no way to say what the key should be. Every remote +environment therefore gets its own key, so a user cannot share one key across +several remotes or reuse a key they already manage elsewhere. Once remote +environments are first-class fleet nodes, a `fleet.yaml` that lists several +remotes is the natural place to say "these endpoints all take this key" — and +today that is not expressible: an agent pointed at several endpoints needs a +different key per endpoint, and there is no single shared key to give it. + +## What Changes + +- A fleet-wide API key reference in `fleet.yaml`: a top-level `apiKeyEnv` field + naming the environment variable that holds the key shared by the fleet's + remote nodes. Same discipline as every other secret reference in the file — + the variable's *name*, never the value; resolved from the process environment + first, then the `.env` beside the file; named-but-unset is a configuration + error naming the variable. It is the default key for a remote node; a node's + own `engineTokenEnv` still overrides it. Daemon nodes are unaffected — they + keep requiring their own `engineTokenEnv` to be gated. +- `spinloop remote deploy` accepts a caller-supplied key as a variable reference + (`--api-key-env `) — never a literal, so the key does not appear on the + command line. The CLI resolves the variable and sends the value to the deploy + Lambda in the SigV4-signed request body as a **request-scoped** field: it is + not part of the persisted deploy-config and never appears in any reply. +- The deploy Lambda stores a supplied key in the environment's existing + Secrets Manager secret — created if absent, set when supplied — instead of + always generating one. Everything downstream is unchanged: the instance reads + the same secret at boot, the env/start Lambdas still report it, and + `spinloop harness` injects it at launch exactly as today. +- Rotation semantics: deploying *with* a key replaces the environment's secret, + instantly invalidating the old one; deploying *without* a key leaves the + existing secret alone (never regenerated). A deploy that replaces the key says + so in its report (without printing the value), because an agent holding the + retired key otherwise gets silent 401s. +- **Non-goal:** wiring a fleet-driven deploy that reads the fleet file's + `apiKeyEnv` automatically. The two sides stay independent and both name + environment variables, so an operator keeps them in sync by naming the same + variable in `fleet.yaml` and on the `--api-key-env` flag. A fleet-driven + deploy can adopt the fleet reference directly later. + +## Capabilities + +### New Capabilities + +(none — this change modifies existing capabilities only) + +### Modified Capabilities + +- `fleet-config`: a fleet file MAY declare a fleet-wide `apiKeyEnv` naming the + variable holding the key shared by its remote nodes; it is resolved the way + every other reference in the file is, and a remote node's own `engineTokenEnv` + overrides it. +- `fleet-routing`: the "engine key the client sets" requirement — when routing + selects a *remote* node, the key the launched agent is given is the node's own + variable when named, otherwise the fleet-wide key; a remote's engine is always + gated, so a remote that names no key of its own and whose fleet names none + fails before the agent launches. +- `environment-deployment`: deploy SHALL accept an externally provided API key + (a variable reference), carry it in the signed request as a request-scoped + field, and the control plane SHALL store it in the environment's secret — + create if absent, set when supplied — with the rotation semantics above. +- `remote-endpoint`: the `deploy` subcommand SHALL accept a `--api-key-env` + flag naming the variable that holds the key to deploy, and the deploy report + SHALL say a supplied key was applied without printing the value. + +## Impact + +- Go client: `internal/fleet` (new `Config.APIKeyEnv` field + resolution, and + the remote-node key path in `select.go`), `internal/remote` (deploy request + carries a request-scoped key field), `cmd/spinloop` (`deploy`'s `--api-key-env` + flag and its report line, and the fleet-routed launch's key for a remote node). +- Control plane (`remote/`, TypeScript): the deploy Lambda accepts the + request-scoped key and `remote/lambda/shared/environments.ts` gains a + create-or-set for the environment's API-key secret (a `PutSecretValue` for the + update case). +- Security: the key travels only in the SigV4-signed deploy body and in Secrets + Manager — never in `remote.json`, never in `fleet.yaml`, never in a reply, + never persisted in the deploy-config parameter. +- Tests: Go suite (fleet config resolution, remote-node key selection, deploy + request body, the deploy flag) and `remote/` vitest (the Lambda's + create-or-set and rotation behaviour). Coverage stays ≥ 80%. diff --git a/openspec/changes/remote-external-api-key/specs/environment-deployment/spec.md b/openspec/changes/remote-external-api-key/specs/environment-deployment/spec.md new file mode 100644 index 00000000..1e63dc3e --- /dev/null +++ b/openspec/changes/remote-external-api-key/specs/environment-deployment/spec.md @@ -0,0 +1,58 @@ +## ADDED Requirements + +### Requirement: Externally provided API key + +`spinloop remote deploy` SHALL accept an externally provided API key as a +reference to an environment variable, and pass it to the control plane to store +as the environment's API key. The value SHALL NOT be written on the command line +or in any file the CLI owns: the flag names a variable, and the CLI resolves it +from the process environment — which the Spinloop's local environment has already +populated — before sending it. A named variable that is set nowhere SHALL fail +the deploy, naming the variable, before anything is sent. + +The key SHALL travel in the signed deploy request as a **request-scoped** field +— riding beside the allowed CIDR and the reseed choice — and SHALL NOT be part +of the persisted deploy-config and SHALL NOT appear in any reply. The control +plane SHALL store a supplied key in the environment's existing API-key secret: +creating the secret when it is absent, and setting its value when the secret +already exists. + +Deploying with a key is a rotation: it SHALL replace the environment's secret, +instantly invalidating the previous key. Deploying without a key SHALL leave any +existing secret untouched and SHALL NOT regenerate one. A deploy that supplies a +key SHALL report that the key was applied — or, when it replaced an existing +one, that it was rotated — without printing the value, so an operator who +rotates a key out from under a live agent sees it happen rather than discovering +it as 401s. + +#### Scenario: A deploy stores a supplied key + +- **WHEN** `spinloop remote deploy` is given a key variable that is set, for an + environment whose API-key secret does not yet exist +- **THEN** the environment's API-key secret is created holding that value, and + the report says the key was applied + +#### Scenario: A deploy rotates an existing key + +- **WHEN** `spinloop remote deploy` is given a key for an environment that already + has an API-key secret +- **THEN** the secret is set to the new value, the old key is no longer valid, + and the report says the key was rotated + +#### Scenario: A deploy without a key keeps the existing one + +- **WHEN** `spinloop remote deploy` runs with no key for an environment that + already has an API-key secret +- **THEN** the secret is left unchanged and no new key is generated + +#### Scenario: The key is not persisted or echoed + +- **WHEN** a deploy supplies a key +- **THEN** the value is not written to the environment's deploy-config, is not + in the registered remote configuration, and is not printed in any reply or in + the deploy report + +#### Scenario: A named variable that is unset fails early + +- **WHEN** `spinloop remote deploy` names a key variable that is set nowhere +- **THEN** the deploy fails naming the variable, before anything is sent diff --git a/openspec/changes/remote-external-api-key/specs/fleet-config/spec.md b/openspec/changes/remote-external-api-key/specs/fleet-config/spec.md new file mode 100644 index 00000000..fce6f3ae --- /dev/null +++ b/openspec/changes/remote-external-api-key/specs/fleet-config/spec.md @@ -0,0 +1,51 @@ +## ADDED Requirements + +### Requirement: Fleet-wide API key reference + +A `fleet.yaml` MAY declare a top-level `apiKeyEnv` naming the environment +variable that holds the API key shared by the fleet's remote nodes. The file +SHALL hold the variable's *name*, never the value — the same discipline as the +daemon and engine-token references — and the reference SHALL be resolved exactly +the way those are: the process environment first, then the `.env` beside the +fleet file. + +The reference is the default key for a remote node. A remote node whose own +entry names no `engineTokenEnv` takes the fleet-wide key. A node's own +`engineTokenEnv` SHALL override the fleet-wide reference, so one remote may +carry a distinct key while the rest of the fleet shares one. A daemon node SHALL +NOT take the fleet-wide reference: it is gated only by its own `engineTokenEnv`, +exactly as it is today. + +A reference that is named but resolves to nothing SHALL be a configuration error +naming the variable, in the same way a missing engine-token variable is. + +#### Scenario: A fleet shares one key across its remotes + +- **WHEN** a `fleet.yaml` declares `apiKeyEnv: SHARED_KEY`, that variable is + set, and it lists two `kind: remote` nodes that name no `engineTokenEnv` +- **THEN** the value of `SHARED_KEY` is the key both remotes are reached with + +#### Scenario: A per-node reference overrides the fleet-wide one + +- **WHEN** a `fleet.yaml` declares `apiKeyEnv: SHARED_KEY` and one remote node + names `engineTokenEnv: SPECIAL_KEY` +- **THEN** that node is reached with the value of `SPECIAL_KEY` and the other + remotes with the value of `SHARED_KEY` + +#### Scenario: The fleet file holds no key value + +- **WHEN** a `fleet.yaml` declaring `apiKeyEnv` is parsed +- **THEN** it carries only the reference, never a literal key value + +#### Scenario: An unset fleet-wide variable names itself + +- **WHEN** a `fleet.yaml` declares an `apiKeyEnv` that is set nowhere, and a + remote node naming no key of its own is reached for its key +- **THEN** the failure names that variable, and no agent is launched without a + key + +#### Scenario: A daemon node is not gated by the fleet-wide key + +- **WHEN** a `fleet.yaml` declares `apiKeyEnv` and a daemon node names no + `engineTokenEnv` +- **THEN** the daemon node is started ungated, as it is today diff --git a/openspec/changes/remote-external-api-key/specs/fleet-routing/spec.md b/openspec/changes/remote-external-api-key/specs/fleet-routing/spec.md new file mode 100644 index 00000000..3f5f673e --- /dev/null +++ b/openspec/changes/remote-external-api-key/specs/fleet-routing/spec.md @@ -0,0 +1,85 @@ +## MODIFIED Requirements + +### Requirement: The engine key the client sets + +Routing SHALL resolve the engine key from the variable the node's fleet entry +names, supply it to the node when it wakes one, and place it in the launched +agent's environment as `OPENAI_API_KEY` — and, for a harness that reads the key +under its own name, under that name too, as the remote path already does. A key +already set in spinloop's environment SHALL win. + +The client is therefore the one party that holds the key: it decides what the +engine it starts is gated with, and it knows what to give the agent because it +set it. A daemon node whose fleet entry names no key SHALL wake an ungated +engine, which is correct for a node reached over loopback. + +For a remote node the resolution is the same with a fleet-wide default: the key +is the node's own `engineTokenEnv` when it names one, otherwise the fleet file's +`apiKeyEnv`. A remote's engine is always gated by its API key, so a remote that +is selected — running or woken — SHALL be reached with a key. When neither the +node nor the fleet names a variable, or the variable it names is set nowhere, +routing SHALL fail before the agent launches, naming the node and what to set: a +remote is never reached ungated. + +Routing SHALL NOT ask the daemon for a key, and the daemon SHALL NOT return one: +saying a key is required is a fact a router needs, and handing the key out is +not. + +Where routing selects a daemon node that is *already running* — one it did not +wake — the engine was gated by whoever started it. When such a node reports that +it requires a key and the fleet entry names none, routing SHALL fail before the +agent launches, naming the node and the variable to set: an agent that cannot +authenticate is worse than a message that says so. + +#### Scenario: A woken node is gated with the client's key + +- **WHEN** routing wakes a node and its fleet entry names a variable holding a + key +- **THEN** the node is started gated with that key, and the launched agent's + environment carries the same value as `OPENAI_API_KEY` + +#### Scenario: No key wakes an ungated engine + +- **WHEN** routing wakes a daemon node whose fleet entry names no engine key +- **THEN** the engine starts ungated and the agent launches with no key injected + for it + +#### Scenario: A remote node takes the fleet-wide key + +- **WHEN** routing selects a remote node whose entry names no `engineTokenEnv`, + and the fleet file declares an `apiKeyEnv` that is set +- **THEN** the launched agent's environment carries that value as + `OPENAI_API_KEY` + +#### Scenario: A remote node's own key overrides the fleet-wide one + +- **WHEN** routing selects a remote node that names its own `engineTokenEnv` + and the fleet file also declares an `apiKeyEnv` +- **THEN** the node's own variable is the key the agent is given, not the + fleet-wide one + +#### Scenario: A remote node with no key fails early + +- **WHEN** routing selects a remote node whose entry names no `engineTokenEnv` + and the fleet file declares no `apiKeyEnv` +- **THEN** the command fails naming the node and what to set, and no agent is + launched + +#### Scenario: An already-running gated node with no key fails early + +- **WHEN** routing selects a daemon node that is already running a gated engine + and the node's fleet entry names no key +- **THEN** the command fails naming the node and what to set, and no agent is + launched + +#### Scenario: An unset variable fails before anything starts + +- **WHEN** a node's fleet entry names an engine key variable that is set nowhere +- **THEN** the command fails naming the node and the variable, and no engine is + started + +#### Scenario: An exported key wins + +- **WHEN** `OPENAI_API_KEY` is already set in the user's environment and a + fleet-routed launch runs +- **THEN** the existing value reaches the agent unchanged diff --git a/openspec/changes/remote-external-api-key/specs/remote-endpoint/spec.md b/openspec/changes/remote-external-api-key/specs/remote-endpoint/spec.md new file mode 100644 index 00000000..6a415a52 --- /dev/null +++ b/openspec/changes/remote-external-api-key/specs/remote-endpoint/spec.md @@ -0,0 +1,73 @@ +## MODIFIED Requirements + +### Requirement: Deploying what the endpoint serves + +`spinloop remote deploy` SHALL derive the deployment from the Spinloop and its +preset: `PROVIDER` SHALL select the inference engine, `MODEL` or the preset's +Hugging Face reference SHALL name the weights as a repository and optional +quantisation, `CONTEXT` or the preset's context size SHALL set the window, +`ALIAS` SHALL set the name the endpoint serves under (defaulting to the +repository), and the preset's remaining settings SHALL become the engine's +arguments. Settings the endpoint owns — host, port, model location, API key, +context size, alias, and metrics — SHALL be dropped, so one preset can both +serve locally and deploy unchanged. The request SHALL describe only what to +serve, never where the weights are stored. A `--dry-run` SHALL print the +derived deployment without sending it. + +`deploy` SHALL additionally accept a `--api-key-env ` flag naming the +environment variable that holds the API key to store for the environment. The +flag names a variable, never a literal, and the CLI resolves it from the +process environment before sending it to the control plane to be stored as the +environment's key (see the Environment Deployment specification for how it is +stored and rotated). When a key is supplied, the deploy report SHALL say so +without printing the value. + +Deploy SHALL target a named environment: in addition to deriving what to serve, +it SHALL create and register that environment on the control plane (its Elastic +IP, instance configuration, per-environment API key and ingress, and SSM state), +as defined by the Environment Deployment specification. Deploying SHALL NOT start +the instance. + +#### Scenario: A preset drives both serving and deploying + +- **WHEN** a Spinloop with a preset is deployed +- **THEN** the engine's arguments are the preset's, minus the settings the + endpoint sets itself + +#### Scenario: The Spinloop overrides its preset + +- **WHEN** the Spinloop states a `MODEL` and `CONTEXT` that differ from the + preset's +- **THEN** the Spinloop's values are deployed + +#### Scenario: A provider that is not a self-hosted engine + +- **WHEN** a Spinloop naming a hosted provider is deployed +- **THEN** the command fails saying that only a self-hosted engine can be + deployed + +#### Scenario: A local model file + +- **WHEN** a Spinloop naming a local model file is deployed +- **THEN** the command fails saying to name a repository instead, because the + endpoint fetches its own weights + +#### Scenario: Deploying creates and registers the environment + +- **WHEN** a deployment succeeds against a bootstrapped account +- **THEN** the named environment is created and registered in the registry, and + the report says whether the weights still have to be fetched before it can + serve + +#### Scenario: Deploying is not starting + +- **WHEN** a deployment succeeds +- **THEN** the environment is configured but not started, and the report says + whether the weights still have to be fetched before it can serve + +#### Scenario: Deploying with a supplied key + +- **WHEN** `spinloop remote deploy` is given `--api-key-env SHARED_KEY` and that + variable is set +- **THEN** the key is sent to the control plane to be stored for the + environment, and the report says a key was applied without printing the value diff --git a/openspec/changes/remote-external-api-key/tasks.md b/openspec/changes/remote-external-api-key/tasks.md new file mode 100644 index 00000000..4895ce7b --- /dev/null +++ b/openspec/changes/remote-external-api-key/tasks.md @@ -0,0 +1,42 @@ +# Tasks + +## 1. Fleet: fleet-wide key reference (internal/fleet) + +- [x] 1.1 Add `APIKeyEnv` (yaml `apiKeyEnv`) to `Config` in `internal/fleet/config.go`, beside `NodeConfig.EngineTokenEnv` +- [x] 1.2 Add the fleet-level resolver (process environment first, then the `.env` beside the fleet file, reusing the `resolveTokenEnv` lookup); a named-but-unset variable is an error naming the variable and the fleet file +- [x] 1.3 Unit tests: unset fleet variable is fine until resolved, set variable resolves, named-but-unset fails naming the variable and file, `.env`-beside-the-file fills a gap +- [x] 1.4 `yaml.Unmarshal` ignores an unknown `apiKeyEnv` on an old binary — confirm with a parse test that a fleet file with `apiKeyEnv` loads without error + +## 2. Fleet: remote key resolution (internal/fleet/select.go) + +- [x] 2.1 In `engineKeyFor`, special-case `n.Kind == KindRemote`: the key is the node's own `engineTokenEnv` when named, else the fleet's `apiKeyEnv`; daemon logic unchanged +- [x] 2.2 A remote with no resolvable key (neither named, or the variable set nowhere) fails before launch, naming the node and what to set +- [x] 2.3 Unit tests: fleet key used for a remote, node `engineTokenEnv` overrides the fleet key, no key fails early with the node named, unset variable fails naming the variable, a daemon node is unaffected by the fleet key +- [x] 2.4 Confirm the wake path is unchanged: a remote wake candidate is still refused by `StartWith`, and a daemon wake still resolves only the node's own `engineTokenEnv` + +## 3. Remote deploy: externally provided key (Go) + +- [x] 3.1 `internal/remote/remote.go`: add `APIKey string \`json:"apiKey,omitempty"\`` to `Deploy`'s request-scoped body struct beside `allowedCidr`/`reseed`, and a `apiKey` parameter to `Deploy` +- [x] 3.2 Test: the request body carries `apiKey` only when a key is supplied, and is byte-identical to today's body when it is not; the key never enters `DeployConfig` marshalling +- [x] 3.3 `cmd/spinloop/remote.go`: add the `--api-key-env ` flag to `deploy`, registered on its own flag set; resolve it from the process environment (after `applySpinloopEnv`), failing naming the variable before anything is sent +- [x] 3.4 Deploy report: print the key action from the control-plane reply (`api key: created` / `api key: rotated`), never the value +- [x] 3.5 Tests through the existing `httptest` seam: a supplied key reaches the request body, an unset variable fails before the request is made, the report names the action without the value + +## 4. Control plane: store or rotate the key (remote/lambda) + +- [x] 4.1 `remote/lambda/shared/environments.ts`: `ensureEnvApiKey(env, providedKey?)` — supplied key creates the secret if absent and sets its value (`PutSecretValueCommand`, adding the import) if present; no key keeps today's generate-only-if-absent and never regenerates; return the action taken (`created` / `rotated` / none) +- [x] 4.2 `remote/lambda/deploy/index.ts`: read the request-scoped `apiKey` from the raw body the way `allowedCidr`/`reseed` are read (not from `parseDeployConfig`), pass it to `ensureEnvApiKey`, and include the action word in the reply — never the value; the `console.log` summary does not gain the key +- [x] 4.3 vitest: key + no secret creates it with the supplied value, key + existing secret calls `PutSecretValue` and reports rotated, no key leaves an existing secret untouched (no `PutSecretValue` call) and reports no action, the reply body never contains the value +- [x] 4.4 Confirm the persisted deploy-config (SSM parameter) is written from `parseDeployConfig` only, so a supplied key is not persisted — add an assertion if one is not already present + +## 5. Documentation + +- [x] 5.1 `remote/docs/architecture.md`: document that a deploy with a key replaces the environment's API key (rotation; the old key stops working) and that omitting it leaves the existing key in place +- [x] 5.2 Update the user-facing docs (`docs/`) and `AGENTS.md` where the fleet file fields and the `remote deploy` flags are described, adding `apiKeyEnv` (remote-only default) and `--api-key-env ` + +## 6. Verification + +- [x] 6.1 `gofmt -l .` clean, `go vet ./...` clean +- [x] 6.2 `go test ./... -cover` green with total coverage still >= 80% +- [x] 6.3 `pnpm test` in `remote/` green, including the deploy tests +- [x] 6.4 `scripts/check-no-cloud-identifiers.sh` clean diff --git a/remote/docs/architecture.md b/remote/docs/architecture.md index 8327f86f..58414cc8 100644 --- a/remote/docs/architecture.md +++ b/remote/docs/architecture.md @@ -115,6 +115,17 @@ weights are not in the bucket, the Lambda launches a seed itself and replies prefix, so wait for it. The id is stable (derived from the weights), unlike the instance it replaced, so it is what `spinloop remote seed status` takes. +The environment's engine API key is the same shape: a property of the *request*, +not of what the environment serves. `spinloop remote deploy --api-key-env VAR` +resolves the variable and sends the value beside the deploy body; the Lambda +stores it in the environment's Secrets Manager secret (the one the start Lambda +fetches into the daemon) and never writes it into the SSM parameter. A supplied +key is a rotation — `create` when the secret is absent, `PutSecretValue` (which +invalidates the old value) when it exists — and the reply carries only the +action (`apiKeyAction: "created" | "rotated"`), never the value. A deploy that +sends no key leaves the secret alone: first creation generates one, and an +existing value is never regenerated. + ## Seeding A seed is a supervised job, not a fire-and-forget script. diff --git a/remote/lambda/deploy/index.ts b/remote/lambda/deploy/index.ts index b83947ec..0f25d6cd 100644 --- a/remote/lambda/deploy/index.ts +++ b/remote/lambda/deploy/index.ts @@ -89,6 +89,17 @@ export async function handler(event: LambdaFunctionURLEvent): Promise { +export async function ensureEnvApiKey(env: string, providedKey?: string): Promise { const name = apiKeySecretName(env); + let exists = false; try { await secretsManager.send(new DescribeSecretCommand({ SecretId: name })); - return; + exists = true; } catch (err) { if (errorName(err) !== 'ResourceNotFoundException') { throw err; } } + if (providedKey) { + if (exists) { + await secretsManager.send(new PutSecretValueCommand({ SecretId: name, SecretString: providedKey })); + return 'rotated'; + } + await secretsManager.send( + new CreateSecretCommand({ + Name: name, + Description: `API key for the cloud-vm-llm environment ${env}`, + SecretString: providedKey, + }), + ); + return 'created'; + } + if (exists) { + return 'unchanged'; + } await secretsManager.send( new CreateSecretCommand({ Name: name, @@ -228,6 +260,7 @@ export async function ensureEnvApiKey(env: string): Promise { SecretString: randomBytes(36).toString('base64url'), }), ); + return 'created'; } /** Read an environment's API key. */ diff --git a/remote/test/deploy-api-key.test.ts b/remote/test/deploy-api-key.test.ts new file mode 100644 index 00000000..76d8b962 --- /dev/null +++ b/remote/test/deploy-api-key.test.ts @@ -0,0 +1,112 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { LambdaFunctionURLEvent } from 'aws-lambda'; + +// Controlled per test: what the secret's state makes the action, and what the +// Lambda handed it. +const keyCalls: (string | undefined)[] = []; +let keyAction = 'rotated'; +const persisted: unknown[] = []; + +vi.mock('../lambda/shared/aws', () => ({ + requireEnv: (name: string) => (name === 'ENGINE_PORT' ? '8000' : `stub-${name}`), + readDeployConfig: vi.fn(), + writeDeployConfig: vi.fn(async (_param: string, cfg: unknown) => { + persisted.push(cfg); + }), + errorName: (err: unknown) => (err instanceof Error ? err.name : String(err)), +})); + +vi.mock('../lambda/shared/environments', () => ({ + environmentFrom: (_q: unknown, body: unknown) => (body as string) ?? 'default', + deployConfigParam: (env: string) => `/cloud-vm-llm/${env}/deploy-config`, + baseUrlFor: (ip: string, port: number) => `http://${ip}:${port}/v1`, + ensureEnvEip: async () => ({ publicIp: '198.51.100.7' }), + ensureEnvSecurityGroup: async () => undefined, + findEnvSecurityGroup: async () => 'sg-1', + ensureEnvApiKey: async (_env: string, key?: string) => { + keyCalls.push(key); + return keyAction; + }, +})); + +vi.mock('../lambda/shared/seed', () => ({ + weightsPresent: async () => true, +})); + +vi.mock('../lambda/shared/seed/launch', () => ({ + seedInfraFromEnv: () => ({ bucket: 'weights' }), + buildSeedJob: (cfg: unknown) => ({ seedId: 'llamacpp--m', cfg }), + launchSeedInstance: async (job: { seedId: string }, _infra: unknown, _opts?: { force?: boolean }) => { + return { seedId: job.seedId, instanceId: 'i-seed', started: true }; + }, +})); + +let handler: (event: LambdaFunctionURLEvent) => Promise<{ statusCode: number; body: string }>; + +beforeAll(async () => { + ({ handler } = (await import('../lambda/deploy/index')) as never); +}); + +const CONFIG = { + environment: 'glimmer', + runner: 'llamacpp', + modelId: 'meta-models/Muse-Glimmer-30B-GGUF', + quant: 'kquant-dynamic', + contextSize: 524288, + servedModelName: 'muse-glimmer-30b', + serveArgs: [], + allowedCidr: '203.0.113.7/32', +}; + +function post(body: Record): LambdaFunctionURLEvent { + return { + requestContext: { http: { method: 'POST' } }, + body: JSON.stringify(body), + isBase64Encoded: false, + } as unknown as LambdaFunctionURLEvent; +} + +beforeEach(() => { + keyCalls.length = 0; + persisted.length = 0; + keyAction = 'rotated'; +}); + +describe('deploy with a supplied key', () => { + it('stores the key in the environment\'s secret and reports the action', async () => { + const res = await handler(post({ ...CONFIG, apiKey: 'sk-supplied' })); + expect(res.statusCode).toBe(200); + expect(keyCalls).toEqual(['sk-supplied']); + const reply = JSON.parse(res.body); + expect(reply.apiKeyAction).toBe('rotated'); + // The action, never the value. + expect(res.body).not.toContain('sk-supplied'); + }); + + it('reports a created key the same way', async () => { + keyAction = 'created'; + const res = await handler(post({ ...CONFIG, apiKey: 'sk-supplied' })); + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).apiKeyAction).toBe('created'); + }); + + it('never persists the key into the stored deploy-config', async () => { + // Stored, it would be re-sent on every wake that read the config back. + await handler(post({ ...CONFIG, apiKey: 'sk-supplied' })); + expect(persisted).toHaveLength(1); + expect(persisted[0]).not.toHaveProperty('apiKey'); + }); + + it('leaves the secret alone when no key is sent', async () => { + const res = await handler(post(CONFIG)); + expect(res.statusCode).toBe(200); + expect(keyCalls).toEqual([undefined]); + expect(JSON.parse(res.body)).not.toHaveProperty('apiKeyAction'); + }); + + it('rejects a non-string apiKey', async () => { + const res = await handler(post({ ...CONFIG, apiKey: 42 })); + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body).error).toMatch(/apiKey must be a string/); + }); +}); diff --git a/remote/test/env-api-key.test.ts b/remote/test/env-api-key.test.ts new file mode 100644 index 00000000..5b56d8e7 --- /dev/null +++ b/remote/test/env-api-key.test.ts @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + CreateSecretCommand, + DescribeSecretCommand, + PutSecretValueCommand, +} from '@aws-sdk/client-secrets-manager'; + +// Every command the Lambda constructs, for the assertions. +const sent: unknown[] = []; +// The secret's state as DescribeSecretCommand finds it. +let outcome: 'exists' | 'missing' = 'missing'; + +vi.mock('@aws-sdk/client-ec2', () => ({ + EC2Client: vi.fn().mockImplementation(() => ({})), + AllocateAddressCommand: class {}, + AuthorizeSecurityGroupIngressCommand: class {}, + CreateSecurityGroupCommand: class {}, + DescribeAddressesCommand: class {}, + DescribeSecurityGroupsCommand: class {}, + RevokeSecurityGroupIngressCommand: class {}, +})); + +vi.mock('@aws-sdk/client-secrets-manager', () => { + class CreateSecretCommand { + input: unknown; + constructor(input: unknown) { + this.input = input; + } + } + class DescribeSecretCommand { + input: unknown; + constructor(input: unknown) { + this.input = input; + } + } + class GetSecretValueCommand { + input: unknown; + constructor(input: unknown) { + this.input = input; + } + } + class PutSecretValueCommand { + input: unknown; + constructor(input: unknown) { + this.input = input; + } + } + const send = async (cmd: unknown) => { + sent.push(cmd); + if (cmd instanceof DescribeSecretCommand) { + if (outcome === 'missing') { + const err = new Error("Secrets Manager can't find the specified secret"); + err.name = 'ResourceNotFoundException'; + throw err; + } + return {}; + } + return {}; + }; + return { + CreateSecretCommand, + DescribeSecretCommand, + GetSecretValueCommand, + PutSecretValueCommand, + SecretsManagerClient: vi.fn().mockImplementation(() => ({ send })), + }; +}); + +import { apiKeySecretName, ensureEnvApiKey } from '../lambda/shared/environments'; + +describe('ensureEnvApiKey', () => { + beforeEach(() => { + sent.length = 0; + outcome = 'missing'; + }); + + it('creates the secret with a supplied key when it is absent', async () => { + const action = await ensureEnvApiKey('glimmer', 'sk-supplied'); + expect(action).toBe('created'); + const create = sent.find((c) => c instanceof CreateSecretCommand) as CreateSecretCommand; + expect(create.input).toMatchObject({ + Name: apiKeySecretName('glimmer'), + SecretString: 'sk-supplied', + }); + expect(sent.some((c) => c instanceof PutSecretValueCommand)).toBe(false); + }); + + it('rotates an existing secret to the supplied key', async () => { + outcome = 'exists'; + const action = await ensureEnvApiKey('glimmer', 'sk-new'); + expect(action).toBe('rotated'); + const put = sent.find((c) => c instanceof PutSecretValueCommand) as PutSecretValueCommand; + expect(put.input).toMatchObject({ + SecretId: apiKeySecretName('glimmer'), + SecretString: 'sk-new', + }); + // The old value is replaced in place, not a new secret minted. + expect(sent.some((c) => c instanceof CreateSecretCommand)).toBe(false); + }); + + it('leaves an existing secret alone without a key', async () => { + outcome = 'exists'; + const action = await ensureEnvApiKey('glimmer'); + expect(action).toBe('unchanged'); + // Only the describe — no create, no put. + expect(sent).toHaveLength(1); + expect(sent[0] instanceof DescribeSecretCommand).toBe(true); + }); + + it('generates a value on first creation when no key is supplied', async () => { + const action = await ensureEnvApiKey('glimmer'); + expect(action).toBe('created'); + const create = sent.find((c) => c instanceof CreateSecretCommand) as CreateSecretCommand; + const input = create.input as { SecretString: string }; + expect(input.SecretString.length).toBeGreaterThan(0); + }); +}); From 43ad8af5e025c92763007a5c4b1fd7f5195e7598 Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Thu, 3 Sep 2026 16:35:29 +0100 Subject: [PATCH 2/2] docs(openspec): archive remote-external-api-key and sync its spec --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/environment-deployment/spec.md | 0 .../specs/fleet-config/spec.md | 0 .../specs/fleet-routing/spec.md | 0 .../specs/remote-endpoint/spec.md | 0 .../tasks.md | 0 openspec/specs/environment-deployment/spec.md | 57 +++++++++++++++++++ openspec/specs/fleet-config/spec.md | 50 ++++++++++++++++ openspec/specs/fleet-routing/spec.md | 45 ++++++++++++--- openspec/specs/remote-endpoint/spec.md | 15 +++++ 12 files changed, 159 insertions(+), 8 deletions(-) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/.openspec.yaml (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/design.md (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/proposal.md (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/specs/environment-deployment/spec.md (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/specs/fleet-config/spec.md (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/specs/fleet-routing/spec.md (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/specs/remote-endpoint/spec.md (100%) rename openspec/changes/{remote-external-api-key => archive/2026-09-03-remote-external-api-key}/tasks.md (100%) diff --git a/openspec/changes/remote-external-api-key/.openspec.yaml b/openspec/changes/archive/2026-09-03-remote-external-api-key/.openspec.yaml similarity index 100% rename from openspec/changes/remote-external-api-key/.openspec.yaml rename to openspec/changes/archive/2026-09-03-remote-external-api-key/.openspec.yaml diff --git a/openspec/changes/remote-external-api-key/design.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/design.md similarity index 100% rename from openspec/changes/remote-external-api-key/design.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/design.md diff --git a/openspec/changes/remote-external-api-key/proposal.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/proposal.md similarity index 100% rename from openspec/changes/remote-external-api-key/proposal.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/proposal.md diff --git a/openspec/changes/remote-external-api-key/specs/environment-deployment/spec.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/specs/environment-deployment/spec.md similarity index 100% rename from openspec/changes/remote-external-api-key/specs/environment-deployment/spec.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/specs/environment-deployment/spec.md diff --git a/openspec/changes/remote-external-api-key/specs/fleet-config/spec.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/specs/fleet-config/spec.md similarity index 100% rename from openspec/changes/remote-external-api-key/specs/fleet-config/spec.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/specs/fleet-config/spec.md diff --git a/openspec/changes/remote-external-api-key/specs/fleet-routing/spec.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/specs/fleet-routing/spec.md similarity index 100% rename from openspec/changes/remote-external-api-key/specs/fleet-routing/spec.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/specs/fleet-routing/spec.md diff --git a/openspec/changes/remote-external-api-key/specs/remote-endpoint/spec.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/specs/remote-endpoint/spec.md similarity index 100% rename from openspec/changes/remote-external-api-key/specs/remote-endpoint/spec.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/specs/remote-endpoint/spec.md diff --git a/openspec/changes/remote-external-api-key/tasks.md b/openspec/changes/archive/2026-09-03-remote-external-api-key/tasks.md similarity index 100% rename from openspec/changes/remote-external-api-key/tasks.md rename to openspec/changes/archive/2026-09-03-remote-external-api-key/tasks.md diff --git a/openspec/specs/environment-deployment/spec.md b/openspec/specs/environment-deployment/spec.md index 31041416..86e068f2 100644 --- a/openspec/specs/environment-deployment/spec.md +++ b/openspec/specs/environment-deployment/spec.md @@ -162,3 +162,60 @@ SHALL appear alongside the runner and model the plan already prints. - **WHEN** `spinloop remote deploy --dry-run` runs without an spinloop version pin - **THEN** the printed plan names `latest` as the spinloop the environment will run + +### Requirement: Externally provided API key + +`spinloop remote deploy` SHALL accept an externally provided API key as a +reference to an environment variable, and pass it to the control plane to store +as the environment's API key. The value SHALL NOT be written on the command line +or in any file the CLI owns: the flag names a variable, and the CLI resolves it +from the process environment — which the Spinloop's local environment has already +populated — before sending it. A named variable that is set nowhere SHALL fail +the deploy, naming the variable, before anything is sent. + +The key SHALL travel in the signed deploy request as a **request-scoped** field +— riding beside the allowed CIDR and the reseed choice — and SHALL NOT be part +of the persisted deploy-config and SHALL NOT appear in any reply. The control +plane SHALL store a supplied key in the environment's existing API-key secret: +creating the secret when it is absent, and setting its value when the secret +already exists. + +Deploying with a key is a rotation: it SHALL replace the environment's secret, +instantly invalidating the previous key. Deploying without a key SHALL leave any +existing secret untouched and SHALL NOT regenerate one. A deploy that supplies a +key SHALL report that the key was applied — or, when it replaced an existing +one, that it was rotated — without printing the value, so an operator who +rotates a key out from under a live agent sees it happen rather than discovering +it as 401s. + +#### Scenario: A deploy stores a supplied key + +- **WHEN** `spinloop remote deploy` is given a key variable that is set, for an + environment whose API-key secret does not yet exist +- **THEN** the environment's API-key secret is created holding that value, and + the report says the key was applied + +#### Scenario: A deploy rotates an existing key + +- **WHEN** `spinloop remote deploy` is given a key for an environment that already + has an API-key secret +- **THEN** the secret is set to the new value, the old key is no longer valid, + and the report says the key was rotated + +#### Scenario: A deploy without a key keeps the existing one + +- **WHEN** `spinloop remote deploy` runs with no key for an environment that + already has an API-key secret +- **THEN** the secret is left unchanged and no new key is generated + +#### Scenario: The key is not persisted or echoed + +- **WHEN** a deploy supplies a key +- **THEN** the value is not written to the environment's deploy-config, is not + in the registered remote configuration, and is not printed in any reply or in + the deploy report + +#### Scenario: A named variable that is unset fails early + +- **WHEN** `spinloop remote deploy` names a key variable that is set nowhere +- **THEN** the deploy fails naming the variable, before anything is sent diff --git a/openspec/specs/fleet-config/spec.md b/openspec/specs/fleet-config/spec.md index a83c4d06..ea445178 100644 --- a/openspec/specs/fleet-config/spec.md +++ b/openspec/specs/fleet-config/spec.md @@ -163,3 +163,53 @@ the variable, in the same way a missing daemon token is. an engine there - **THEN** the engine is started ungated +### Requirement: Fleet-wide API key reference + +A `fleet.yaml` MAY declare a top-level `apiKeyEnv` naming the environment +variable that holds the API key shared by the fleet's remote nodes. The file +SHALL hold the variable's *name*, never the value — the same discipline as the +daemon and engine-token references — and the reference SHALL be resolved exactly +the way those are: the process environment first, then the `.env` beside the +fleet file. + +The reference is the default key for a remote node. A remote node whose own +entry names no `engineTokenEnv` takes the fleet-wide key. A node's own +`engineTokenEnv` SHALL override the fleet-wide reference, so one remote may +carry a distinct key while the rest of the fleet shares one. A daemon node SHALL +NOT take the fleet-wide reference: it is gated only by its own `engineTokenEnv`, +exactly as it is today. + +A reference that is named but resolves to nothing SHALL be a configuration error +naming the variable, in the same way a missing engine-token variable is. + +#### Scenario: A fleet shares one key across its remotes + +- **WHEN** a `fleet.yaml` declares `apiKeyEnv: SHARED_KEY`, that variable is + set, and it lists two `kind: remote` nodes that name no `engineTokenEnv` +- **THEN** the value of `SHARED_KEY` is the key both remotes are reached with + +#### Scenario: A per-node reference overrides the fleet-wide one + +- **WHEN** a `fleet.yaml` declares `apiKeyEnv: SHARED_KEY` and one remote node + names `engineTokenEnv: SPECIAL_KEY` +- **THEN** that node is reached with the value of `SPECIAL_KEY` and the other + remotes with the value of `SHARED_KEY` + +#### Scenario: The fleet file holds no key value + +- **WHEN** a `fleet.yaml` declaring `apiKeyEnv` is parsed +- **THEN** it carries only the reference, never a literal key value + +#### Scenario: An unset fleet-wide variable names itself + +- **WHEN** a `fleet.yaml` declares an `apiKeyEnv` that is set nowhere, and a + remote node naming no key of its own is reached for its key +- **THEN** the failure names that variable, and no agent is launched without a + key + +#### Scenario: A daemon node is not gated by the fleet-wide key + +- **WHEN** a `fleet.yaml` declares `apiKeyEnv` and a daemon node names no + `engineTokenEnv` +- **THEN** the daemon node is started ungated, as it is today + diff --git a/openspec/specs/fleet-routing/spec.md b/openspec/specs/fleet-routing/spec.md index 1ec18cd3..9a41f0ec 100644 --- a/openspec/specs/fleet-routing/spec.md +++ b/openspec/specs/fleet-routing/spec.md @@ -335,16 +335,24 @@ already set in spinloop's environment SHALL win. The client is therefore the one party that holds the key: it decides what the engine it starts is gated with, and it knows what to give the agent because it -set it. A node whose fleet entry names no key SHALL wake an ungated engine, -which is correct for a node reached over loopback. +set it. A daemon node whose fleet entry names no key SHALL wake an ungated +engine, which is correct for a node reached over loopback. + +For a remote node the resolution is the same with a fleet-wide default: the key +is the node's own `engineTokenEnv` when it names one, otherwise the fleet file's +`apiKeyEnv`. A remote's engine is always gated by its API key, so a remote that +is selected — running or woken — SHALL be reached with a key. When neither the +node nor the fleet names a variable, or the variable it names is set nowhere, +routing SHALL fail before the agent launches, naming the node and what to set: a +remote is never reached ungated. Routing SHALL NOT ask the daemon for a key, and the daemon SHALL NOT return one: saying a key is required is a fact a router needs, and handing the key out is not. -Where routing selects a node that is *already running* — one it did not wake — -the engine was gated by whoever started it. When such a node reports that it -requires a key and the fleet entry names none, routing SHALL fail before the +Where routing selects a daemon node that is *already running* — one it did not +wake — the engine was gated by whoever started it. When such a node reports that +it requires a key and the fleet entry names none, routing SHALL fail before the agent launches, naming the node and the variable to set: an agent that cannot authenticate is worse than a message that says so. @@ -357,14 +365,35 @@ authenticate is worse than a message that says so. #### Scenario: No key wakes an ungated engine -- **WHEN** routing wakes a node whose fleet entry names no engine key +- **WHEN** routing wakes a daemon node whose fleet entry names no engine key - **THEN** the engine starts ungated and the agent launches with no key injected for it +#### Scenario: A remote node takes the fleet-wide key + +- **WHEN** routing selects a remote node whose entry names no `engineTokenEnv`, + and the fleet file declares an `apiKeyEnv` that is set +- **THEN** the launched agent's environment carries that value as + `OPENAI_API_KEY` + +#### Scenario: A remote node's own key overrides the fleet-wide one + +- **WHEN** routing selects a remote node that names its own `engineTokenEnv` + and the fleet file also declares an `apiKeyEnv` +- **THEN** the node's own variable is the key the agent is given, not the + fleet-wide one + +#### Scenario: A remote node with no key fails early + +- **WHEN** routing selects a remote node whose entry names no `engineTokenEnv` + and the fleet file declares no `apiKeyEnv` +- **THEN** the command fails naming the node and what to set, and no agent is + launched + #### Scenario: An already-running gated node with no key fails early -- **WHEN** routing selects a node that is already running a gated engine and the - node's fleet entry names no key +- **WHEN** routing selects a daemon node that is already running a gated engine + and the node's fleet entry names no key - **THEN** the command fails naming the node and what to set, and no agent is launched diff --git a/openspec/specs/remote-endpoint/spec.md b/openspec/specs/remote-endpoint/spec.md index 944c5950..4da9a3fb 100644 --- a/openspec/specs/remote-endpoint/spec.md +++ b/openspec/specs/remote-endpoint/spec.md @@ -335,6 +335,14 @@ serve locally and deploy unchanged. The request SHALL describe only what to serve, never where the weights are stored. A `--dry-run` SHALL print the derived deployment without sending it. +`deploy` SHALL additionally accept a `--api-key-env ` flag naming the +environment variable that holds the API key to store for the environment. The +flag names a variable, never a literal, and the CLI resolves it from the +process environment before sending it to the control plane to be stored as the +environment's key (see the Environment Deployment specification for how it is +stored and rotated). When a key is supplied, the deploy report SHALL say so +without printing the value. + Deploy SHALL target a named environment: in addition to deriving what to serve, it SHALL create and register that environment on the control plane (its Elastic IP, instance configuration, per-environment API key and ingress, and SSM state), @@ -378,6 +386,13 @@ the instance. - **THEN** the environment is configured but not started, and the report says whether the weights still have to be fetched before it can serve +#### Scenario: Deploying with a supplied key + +- **WHEN** `spinloop remote deploy` is given `--api-key-env SHARED_KEY` and that + variable is set +- **THEN** the key is sent to the control plane to be stored for the + environment, and the report says a key was applied without printing the value + ### Requirement: Status reports when the endpoint last did work `spinloop remote status` SHALL report how long it has been since the endpoint's