diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d39633f..8df7d57ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,26 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A Bot's computer is rebuilt when it holds a token the deployment has stopped using + +A computer checks every caller against the `COMPUTER_TOKEN` it was created with, and holds that one +for the life of the container. The shell mints the generated secrets once per deployment and does +not rotate them, precisely because a computer outlives a restart, so ordinarily there is nothing +here to go wrong. Setting a machine up again from nothing is the occasion where the token really +does change: the credential store is emptied, a new one is minted, compose rebuilds everything it +owns with it, and the computers, which the supervisor makes rather than compose, survive holding the +old one. + +Everything then refuses, and nothing says why. The gateway allows the action and the trail records it +as carried out, the computer answers 401, and the screen says "Not authorised" while naming no token +and no container. Measured on a first run of v0.0.9 against a computer made by the install before it, +five days earlier: every page the Bot tried to open, and the live screen beside it, failed that way. + +The supervisor now replaces a computer whose token is not the one it is handing out, the same way it +already replaces one built from an older image, keeping the profile and workspace volumes so the Bot +comes back with its logins and its files. A deployment that sets no token is left alone, because a +computer with no door on it is a choice the environment made rather than a mismatch to act on. + ## 0.0.9 ### The People screen keeps a person's last sign-in when their sessions go away diff --git a/supervisor/src/docker.ts b/supervisor/src/docker.ts index 19bafbecd..d339054b3 100644 --- a/supervisor/src/docker.ts +++ b/supervisor/src/docker.ts @@ -217,6 +217,7 @@ async function inspectOwned(names: ComputerNames): Promise<{ port?: number; image?: string; startedAt?: string; + token?: string; } | null> { try { const info = await docker.getContainer(names.container).inspect(); @@ -230,6 +231,9 @@ async function inspectOwned(names: ComputerNames): Promise<{ // The resolved image, not the tag it was started from. A tag moves when the image is // rebuilt; this is what the container is actually running. ...(info.Image ? { image: info.Image } : {}), + // The token this container was born holding, which is the one it will check callers against + // for the rest of its life. See `holdsCurrentToken`. + token: tokenIn(info.Config?.Env), /* * When this run of the container began, which is what tells two runs apart. * @@ -281,6 +285,46 @@ async function runsCurrentImage( } } +/** `COMPUTER_TOKEN=...` out of a list of `KEY=value`, which is how both sides carry an environment. */ +function tokenIn(environment: string[] | undefined): string | undefined { + const entry = environment?.find((line) => line.startsWith(`${TOKEN_NAME}=`)); + return entry?.slice(TOKEN_NAME.length + 1); +} + +const TOKEN_NAME = "COMPUTER_TOKEN"; + +/** + * Whether the computer that exists will accept the token this deployment now hands out. + * + * A computer is checked against the `COMPUTER_TOKEN` it was created with, and it holds that one for + * as long as the container lives. Normally that is nothing to worry about, because the shell mints + * the generated secrets once per deployment and deliberately does not rotate them: a computer + * outliving a restart is the reason it does not. + * + * The token does change, though, on exactly the occasion nobody tests: a machine set up again from + * nothing. Emptying the credential store, or installing over a deployment whose secrets are gone, + * mints a new one. Compose then rebuilds everything it owns with it, the supervisor included, and + * the computers are the one thing compose does not own. They survive, holding the old token, and + * every call to them comes back 401. + * + * What that looks like to a person is the reason this is a defect rather than an inconvenience: the + * gateway allows the action and the trail records it as carried out, the computer refuses it, and + * the screen says "Not authorised" while naming nothing. Found on a first run of v0.0.9 against a + * computer container created by the install before it, five days earlier. + * + * A deployment that sets no token is not a mismatch. That is a computer with no door on it, which is + * a choice the environment makes, and replacing a working browser over it would be this function + * inventing a policy of its own. + */ +function holdsCurrentToken( + existingToken: string | undefined, + environment: string[], +): boolean { + const wanted = tokenIn(environment); + if (wanted === undefined) return true; + return existingToken === wanted; +} + /** Long enough for a cold start with a large image, short enough that a caller is not left hanging. */ const DEFAULT_READY_TIMEOUT_MS = 60_000; @@ -440,7 +484,11 @@ export async function ensure( * ended, and a Bot carrying an hour-old handover prompt into a new conversation is the symptom * that found this. */ - if (existing && !(await runsCurrentImage(existing.image, options.image))) { + if ( + existing && + (!(await runsCurrentImage(existing.image, options.image)) || + !holdsCurrentToken(existing.token, options.environment)) + ) { try { await docker .getContainer(names.container) diff --git a/supervisor/tests/docker.integration.test.ts b/supervisor/tests/docker.integration.test.ts index 6f6cfd73f..bdc54555a 100644 --- a/supervisor/tests/docker.integration.test.ts +++ b/supervisor/tests/docker.integration.test.ts @@ -16,7 +16,7 @@ async function available() { } test.skipIf(!(await available()))( - "supervisor Docker lifecycle in an isolated namespace (five cases)", + "supervisor Docker lifecycle in an isolated namespace (seven cases)", async () => { const namespace = `supervisor-test-${crypto.randomUUID()}`; const child = Bun.spawn( @@ -49,7 +49,7 @@ test.skipIf(!(await available()))( if (!summaryLine) throw new Error(`Missing fixture result: ${stdout} ${stderr}`); const summary = JSON.parse(summaryLine.slice(prefix.length)); - expect(summary.completedCases).toBe(5); + expect(summary.completedCases).toBe(7); expect(summary.cleanup).toBe("complete"); // Do not echo nested Bun summaries: scripts/test-ci.ts counts the outer suite's summary. console.log(summaryLine); diff --git a/supervisor/tests/fixtures/docker-lifecycle.ts b/supervisor/tests/fixtures/docker-lifecycle.ts index c1e9ad660..686214955 100644 --- a/supervisor/tests/fixtures/docker-lifecycle.ts +++ b/supervisor/tests/fixtures/docker-lifecycle.ts @@ -297,6 +297,66 @@ describe("a computer built from an older image", () => { expect(kept.map(createdAt)).toEqual(volumes.map(createdAt)); }, 180_000); + /* + * The install that reached the containers but not the computers. + * + * A computer checks callers against the `COMPUTER_TOKEN` it was created with and keeps that one + * for the life of the container. Setting a machine up again from nothing mints a new token: + * compose rebuilds everything it owns with it, and the computers, which the supervisor makes + * rather than compose, survive holding the old one. Every call to them is then a 401 that no + * screen can account for, because the gateway allowed the action and the trail says it was + * carried out. + * + * Found on a first run of v0.0.9 against a computer container the install before it had made. + */ + test("is replaced when it holds a token this deployment no longer uses", async () => { + await withDocker().supervisor.ensure(names, { + image: IMAGE, + environment: [`COMPUTER_TOKEN=${"old-token"}`], + }); + const before = await withDocker() + .docker.getContainer(names.container) + .inspect(); + + const state = await withDocker().supervisor.ensure(names, { + image: IMAGE, + environment: [`COMPUTER_TOKEN=${"new-token"}`], + }); + const after = await withDocker() + .docker.getContainer(names.container) + .inspect(); + + expect(state).not.toBeNull(); + expect(after.Id).not.toBe(before.Id); + expect(after.Config.Env).toContain("COMPUTER_TOKEN=new-token"); + expect(after.State.Health?.Status).toBe("healthy"); + }, 180_000); + + test("is left alone when it holds the token asked for", async () => { + /* + * The half that keeps this from replacing a working browser on every request, which is the same + * risk the image comparison beside it carries. A deployment that sets no token at all is also + * not a mismatch: that is a choice the environment made, not something to act on. + */ + const environment = ["COMPUTER_TOKEN=steady"]; + await withDocker().supervisor.ensure(names, { image: IMAGE, environment }); + const before = await withDocker() + .docker.getContainer(names.container) + .inspect(); + + await withDocker().supervisor.ensure(names, { image: IMAGE, environment }); + const withNone = await withDocker().supervisor.ensure(names, { + image: IMAGE, + environment: [], + }); + const after = await withDocker() + .docker.getContainer(names.container) + .inspect(); + + expect(withNone).not.toBeNull(); + expect(after.Id).toBe(before.Id); + }, 180_000); + test("is left alone when it is already the image asked for", async () => { /* * The other half, and the one that keeps this from being a fix that restarts every computer on