diff --git a/docs/superpowers/plans/2026-09-24-per-thread-sandbox.md b/docs/superpowers/plans/2026-09-24-per-thread-sandbox.md index 1b40154f7..6e98329aa 100644 --- a/docs/superpowers/plans/2026-09-24-per-thread-sandbox.md +++ b/docs/superpowers/plans/2026-09-24-per-thread-sandbox.md @@ -4859,7 +4859,7 @@ Expected: the job well under its 30-minute `timeout-minutes` (the budget in Task ## Follow-ups recorded, not in this plan - **The drafter as the same app.** With `sandbox.thread`, the drafter's reason to be a separate process (rung 3 §6: a builder serves one target, intake runs before a target is chosen) is gone. Folding it in is optional (spec §1) and changes the drafter's image and inspection root handling; its own PR. -- **A controller-side identity check.** The Docker lane proves each builder thread's intent records its target's `localId`. The controller could also refuse, after a turn, a builder thread whose recorded identity differs from `task.target.image.localId` (a moved tag between prepare and dispatch). Cheap with `openWorkspaceInstallationReader`; worth it once item 4's image registry owns tags. +- **A controller-side identity check.** The Docker lane proves each builder thread's intent records its target's `localId`. The controller could also refuse, after a turn, a builder thread whose recorded identity differs from `task.target.image.localId` (a moved tag between prepare and dispatch). Cheap with `openWorkspaceInstallationReader`; worth it once item 4's image registry owns tags. Closed by `2026-09-25-images-built-on-demand.md`: the verifier runs the bound image id (PR 1, Task 13a); the builder's handoff names it and its labels are checked (PR 2). - **Per-thread `images` re-check on reconnect.** A thread admitted under an `images` predicate the operator later narrows keeps running its recorded image (never re-resolved, by design). If that ever needs revoking, it is a deletion of the thread, not a re-resolution. - **Parent-and-subagent grant sharing.** Under `permissionsMode: "boot"` a subagent's file-store grant is visible to its parent at once; a thread-scoped grant is visible to the parent at its next preparation (each preparation builds its own thread store over the same record). If a turn needs the immediate form, cache the thread store per sandbox key in the manager. @@ -4893,3 +4893,5 @@ An independent review found no critical issues. Each item it raised, and where t Minor: Task 5 Step 2 names the real failure (the constructor throws, since a type-only import is erased); Task 8 Step 3 points at Task 7; Task 20 drops `prepareMs` and the imports only the moved code used; D4's refusal is tested with a real `kubernetesSandbox` over a stub client, at check and at boot (Task 7); the PR 3 README trust paragraph and the PR 3 preamble say the manifest writer controls the whole allow-list, `tool` and `subagent` keys included; `b4 check` prints "sandbox: workspace, image and policy are resolved per thread"; the `withManagedWorkspaceReader` doc comments name `sandbox.thread` (Task 9 Step 3). **PR 3 review follow-up (recorded, not implemented).** The builder's intent records the image ID the manifest's tag resolved to at the thread's first admission, and the verifier resolves the same tag again at verify time; nothing compares either with the target's recorded `target.image.localId`. A tag moved between prepare, admission and verification would build in one image and verify in another with no refusal. The fix is the controller-side identity check already listed under "Follow-ups recorded" (compare the thread's recorded `environment.identity` and the verifier's resolved ID with `task.target.image.localId`, refuse on a mismatch), best done once item 4's image registry owns tags. The PR 3 review also tightened the manifest: its image tag's target and pin segments must equal its own `targetId` and `pin[:12]`. + +Closed by `2026-09-25-images-built-on-demand.md`: the verifier runs the bound image id (PR 1, Task 13a); the builder's handoff names it and its labels are checked (PR 2). diff --git a/docs/superpowers/plans/2026-09-25-images-built-on-demand.md b/docs/superpowers/plans/2026-09-25-images-built-on-demand.md index 4798ba7bc..76180c03f 100644 --- a/docs/superpowers/plans/2026-09-25-images-built-on-demand.md +++ b/docs/superpowers/plans/2026-09-25-images-built-on-demand.md @@ -5314,6 +5314,8 @@ git commit -m "feat(software-factory): the builder handoff names the bound image Co-Authored-By: Claude Opus 5.5 " ``` +> **As landed:** `openImageRegistryReader` also refuses a registry that records no schema version (a file no factory finished creating), leaving it unmigrated, besides an absent path and a newer factory's registry. It creates and writes nothing, but SQLite may leave `-wal`/`-shm` files beside a quiescent WAL registry; its doc comment and test title say so. `fakeBuilderHandoff` takes the bound image's id but keeps its fixed fake tag (`b4-factory-fake-target:…`), because a handoff's tag must name its own target and pin and the fake's are the fake target's. + ### Task 19: The builder checks the image's build labels against its handoff Added after review (item 7). An image ID alone binds no target: the provider's predicate admits any `sha256:` the daemon holds. PR 1's builds carry `b4.factory.target`, `b4.factory.pin` and `b4.factory.key` labels (Task 6); the builder's thread resolver reads them by ID and refuses an image whose labels are not the handoff's own target, pin and the tag's key. Labels are part of the image config the ID content-addresses, so a check by ID has no time-of-check gap. No framework hook is needed: the resolver is the builder app's own code, and the check runs there before the framework resolves the image. Trust impact, stated: the labels bind an ID to a target, pin and recipe as the controller built it; whoever can build or load images on the daemon can forge labels, which is the bound the handoff had before (anyone who can tag an image can already run anything as root there). Without this task, PR 2's ID-only predicate would be a narrower bound than the tag shape it replaces for the target and pin segments; with it, it is at least as narrow. A framework-level `images: (reference, inspected) => …` predicate would let the provider make the same check and is recorded as a follow-up. @@ -5471,6 +5473,8 @@ git commit -m "feat(software-factory): the builder admits only an image built fo Co-Authored-By: Claude Opus 5.5 " ``` +> **As landed:** `builderThreadSandbox` parses the handoff and verifies the staged workspace before the label check, so neither refusal needs a daemon. Parsing is stricter than the plan's: an answer that is not a string-valued object is refused, and `null` labels read as none. Label values are JSON-quoted and capped (80 characters) in refusals, and the daemon's stderr is capped (500). The argv puts `--` before the id, and the thread's abort signal is forwarded to `execFile` beside the 30 s timeout. A behavioural test drives the real `b4.config.ts` thread resolver with `node:child_process` mocked: the argv, the signal, and a refusal for every malformed or mismatched answer (a follow-up commit). + ### Task 20: The Docker proof: a moved tag moves nothing, and an image not built for the handoff is refused **Files:** @@ -5540,6 +5544,8 @@ git commit -m "test(software-factory): a moved tag moves no builder, and an unla Co-Authored-By: Claude Opus 5.5 " ``` +> **As landed:** each refusal is asserted by its reason, not only that admission failed. A mutation removing the label check still made the plan's boolean assertion pass: the unlabelled base image was admitted and failed later, at workspace preparation. The recipe tag is put back on the bound image in a `finally`. + ### Task 21: Docs: the follow-up is closed **Files:** @@ -5562,6 +5568,10 @@ Co-Authored-By: Claude Opus 5.5 " PR 2 verification: the PR 1 table, plus `pnpm --filter @b4-example/software-factory-server test` and Task 20's lane. Push `blove/images-by-id` and open the PR only when Brian asks. +> **As landed:** the README's second statement of the builder's image bound (the builder quick start) and its `builder-handoff` reference paragraph were corrected too (`--image-id`, or the registry read-only, refusing to guess), and the images plan gained As-landed notes for Tasks 18-20. +> +> **Final-review fixes:** dispatch's kept-binding test asserts the builder handoff names the bound id and tag, not the registry's newer build (mutation-checked); the CLI usage and comment say `builder-handoff` reads `images.sqlite` read-only unless `--image-id` is given, and a missing or unreadable registry carries the `--image-id` hint; the README says the handoff names the registry's current image (use `--image-id` from `image_bound` to reproduce a work order), states the full-pin/key-prefix label rule, and that controller and builder upgrade together; `FACTORY_LABELS` is exported from `controller/src/lib/targets/image-builder.ts` and a test pins the builder's copy to it. Follow-up: carry the full recipe key in the handoff so the builder's label check compares all 64 hex, not the tag's 12. + --- ## Proof map diff --git a/docs/superpowers/specs/2026-09-23-software-factory-framework-gaps-design.md b/docs/superpowers/specs/2026-09-23-software-factory-framework-gaps-design.md index ab88d5c9d..d0beac0d3 100644 --- a/docs/superpowers/specs/2026-09-23-software-factory-framework-gaps-design.md +++ b/docs/superpowers/specs/2026-09-23-software-factory-framework-gaps-design.md @@ -243,7 +243,17 @@ target whose files exist at the pin. `FACTORY_MAX_IMAGE_BUILDS` (default 1) boun `target:prepare` only warms the registry; `FACTORY_TARGETS_DIR` is retired. The CLI's `dispatch` follows a build past its request timeout, bounded by the journalled `deadlineMs`. CI's explicit prepares are gone: each lane run builds through a registry of its own. PR 2 -moves the builder to the bound ID, checked against the image's build labels. Deferred: the +moved the builder to the bound ID: the builder handoff is version 4 (`target.image` is the +bound ID, `sha256:<64 hex>`, with its recipe tag beside it in `target.tag`; a version-3 +handoff is refused at first admission, and a thread admitted before the upgrade keeps its +recorded image), the builder's `dockerSandbox({ images })` accepts only IDs, and its thread +resolver reads the ID's build labels (`docker image inspect -- `, 30 s, the thread's abort +signal) and refuses, failing closed, unless `b4.factory.target`, the full `b4.factory.pin` and +a 64-hex `b4.factory.key` whose prefix is the tag's key segment are the handoff's own. Whoever +can build or load images on the daemon can forge labels, the bound the tag shape had before. +`factory builder-handoff` takes `--image-id` or reads `images.sqlite` read-only, refusing to +guess. The Docker lane proves a moved recipe tag moves no builder, and that a version-3 +handoff and an unlabelled image are each refused by their own reason. Deferred: the factory's own git object store (§9 finding 3) and budgets from measured verifier time (§9 finding 5). diff --git a/examples/software-factory/README.md b/examples/software-factory/README.md index a92bf908c..f41d8437f 100644 --- a/examples/software-factory/README.md +++ b/examples/software-factory/README.md @@ -169,15 +169,25 @@ image, policy and permissions, by uploading a source and creating a thread with within the builder's own bounds, which no handoff can move: the network is denied (a thread may not open what the app denies), and the permissions mode is `non-interactive`. The image bound is narrower than "an image the factory prepared": a -handoff may name only a tag in the factory's shape (`b4-factory-:-`, -the recipe key's first twelve hex digits; `dockerSandbox({ images })`) whose target and pin -segments are the handoff's own `targetId` and `pin` (the handoff schema refuses any other at -admission), and only an image present on the daemon under that tag runs. The verifier runs the -work order's bound image by id (its verdict and receipt digest that image); the builder runs -the recipe tag until PR 2 of the images plan moves it to the id. Until then, whoever can tag an -image on the builder's Docker daemon can put anything behind the tag the builder runs, but -that access is already root on the host, so it adds no power a token holder lacks, and the -verdict is still earned in the bound image. That includes the WHOLE allow-list: `permissions` is a record keyed by any +handoff names the image its work order bound by id (`target.image`, `sha256:<64 hex>`), with +that image's recipe tag beside it (`target.tag`, `b4-factory-:-`, +the recipe key's first twelve hex digits) whose target and pin segments must be the handoff's +own `targetId` and `pin` (the handoff schema refuses any other at admission, and refuses a +version-3 handoff that named only a tag). The controller and the builder must be upgraded +together: a mismatch refuses every new work order at its builder's first run (spending one +candidate attempt each), while threads already admitted keep running. The builder's provider accepts only ids +(`dockerSandbox({ images })`), and before the framework resolves the image, the builder's +thread resolver reads the id's build labels (`docker image inspect -- `, bounded at 30 +seconds and by the thread's abort) and refuses it, failing closed, unless `b4.factory.target`, +`b4.factory.pin` (the full pin) and `b4.factory.key` (a 64-hex recipe key whose prefix is the +tag's key segment) are the handoff's own. The labels live in the image config the id +content-addresses, so nothing can change between the check and the run. The framework records +that id as the thread's environment identity at its first admission, and the verifier runs the +same bound id (its verdict and receipt digest that image). A tag is a name for people and for +pruning, never the identity: moving the recipe tag onto another image moves no builder and no +verdict. A thread admitted before this upgrade keeps the image it recorded. The bound that +remains: whoever can build or load images on the builder's Docker daemon can forge the labels, +but that access is already root on the host, so it adds no power a token holder lacks. That includes the WHOLE allow-list: `permissions` is a record keyed by any tool name, so a handoff's author also decides the `tool` and `subagent` keys (which tools run without approval and which subagents may be dispatched), not only `bash` and the path keys; the builder's `non-interactive` mode means anything off that list is refused, never asked @@ -365,10 +375,13 @@ terminal (the same value in all three): pnpm --filter @b4-example/software-factory-server dev --port 4100 One builder serves every target and pin. Each work order's handoff names the image its task -is verified in, the sandbox policy and the permission allow-list; the builder records them at -the thread's first admission and runs that thread in them, and only an image the factory -built (`b4-factory-…`) can be named. The handoff names an image, it does not build one: the -controller builds it before the handoff is written. An upgraded controller's allow-list reaches the next +is verified in (by id, with its recipe tag), the sandbox policy and the permission +allow-list; the builder records them at the thread's first admission and runs that thread in +them, and only an image id whose build labels name the handoff's own target, full pin and a recipe +key matching the tag's key prefix can run. The handoff names an image, it does not build one: the controller builds it +before the handoff is written. To write a handoff by hand, `factory builder-handoff` takes +`--image-id sha256:<64 hex>`, or reads the image `/images.sqlite` records +for the task's target at its pin (read-only), and refuses rather than guess. An upgraded controller's allow-list reaches the next work order at once, because it travels in that work order's handoff; a thread already admitted keeps the list it was admitted with. @@ -679,15 +692,20 @@ policy too, so set the token for `check` after `build` as well (or delete the gi `FACTORY_BUILDER_LANE` (`1` runs them, anything else skips them with a notice). The drafter app reads `FACTORY_DRAFTER_IMAGE` (default: the pinned digest in `drafter/src/drafter-image.ts`) and `FACTORY_DRAFTER_MODEL` (default `gpt-5-mini`). The CLI reads `FACTORY_CONTROLLER_URL` for -writes and `FACTORY_STATE_DIR` for reads; `builder-handoff` needs -neither. `builder-handoff --task --out [--work-order ]` writes one work -order's captured source and its handoff (`.source.json` and +writes and `FACTORY_STATE_DIR` for reads; `builder-handoff` needs no controller. +`builder-handoff --task --out [--work-order ] [--image-id sha256:<64 hex>]` +writes one work order's captured source and its handoff (`.source.json` and `.handoff.json`, named by the task id by default) for driving a builder without a controller: `PUT` the source to `/workspace/sources/`, then `POST /threads` with the handoff as `metadata.factoryBuilder` and its `workspace` as the body's `workspace`, both with the token. It stages its capture under `FACTORY_STATE_DIR` when that is set (as the controller does) and otherwise under a temporary directory it removes, never under the -controller package. `target:prepare` builds from a temporary archive into +controller package. The handoff names `--image-id`, or else the image +`/images.sqlite` records for the task's target at its pin, read without +creating, migrating or writing the registry; with neither it refuses rather than guess (run +`target:prepare`, or pass the id). It names the image the registry records now, not any work +order's binding; to reproduce a work order's builder, pass `--image-id` from its `image_bound` +event (`factory events `). `target:prepare` builds from a temporary archive into `/images.sqlite` and writes nothing under the target. A work order whose worker has left the map — `FACTORY_DRAFTER_URL` unset while a draft is in diff --git a/examples/software-factory/controller/src/cli.ts b/examples/software-factory/controller/src/cli.ts index 5ffc69377..f70b59b0a 100644 --- a/examples/software-factory/controller/src/cli.ts +++ b/examples/software-factory/controller/src/cli.ts @@ -4,7 +4,7 @@ import { join, resolve } from "node:path" import { createInterface } from "node:readline" import { setTimeout as sleep } from "node:timers/promises" import { parseArgs } from "node:util" -import { captureBuilderHandoff } from "./lib/builder-handoff.js" +import { captureBuilderHandoff, isFactoryImageId } from "./lib/builder-handoff.js" import { type ControllerClient, ControllerHttpError, createControllerClient } from "./lib/client.js" import { generatedTasksDirFor } from "./lib/config.js" import { dispatchPreparing, imageWaitBoundMs } from "./lib/controller/images.js" @@ -31,7 +31,9 @@ import { ensurePin, loadTaskRecipe, repositoryRoot, + type TaskRecipe, } from "./lib/targets/catalog.js" +import { openImageRegistryReader, recipeTag } from "./lib/targets/images.js" const USAGE = `factory [options] @@ -53,18 +55,21 @@ const USAGE = `factory [options] events evidence list - builder-handoff --task --out [--work-order ] + builder-handoff --task --out [--work-order ] [--image-id sha256:<64 hex>] The commands that change something are requests to a running controller: FACTORY_CONTROLLER_URL is its base URL. The commands that read do not go through the controller at all: they open /registry.sqlite read-only. The cancel command uses both: it asks the controller to stop the run and then reads the row back. -builder-handoff needs neither. +builder-handoff needs no controller; it reads /images.sqlite (read-only) +unless --image-id is given. builder-handoff writes /.source.json and /.handoff.json (the work order defaults to the task id): the captured workspace's files, and the handoff naming them with the target's image, pin, sandbox policy and permissions, which one builder serving -every target and pin runs that work order's thread in. The controller stages both over the +every target and pin runs that work order's thread in. The image is --image-id, or the one +/images.sqlite records for the task's target at its pin (read-only); with +neither, the command refuses rather than guess. The controller stages both over the builder's Agent Protocol port at dispatch; to drive a builder without a controller, PUT the source to /workspace/sources/, then POST /threads with {"metadata":{"factoryWorkOrderId":,"factoryBuilder":},"workspace":}, @@ -129,6 +134,47 @@ function print(value: unknown) { process.stdout.write(`${JSON.stringify(value, null, 2)}\n`) } +/** + * The image `builder-handoff` names: `--image-id` (with this host's recipe tag for the task's + * target), else the one `/images.sqlite` records for that target at its pin, read + * without creating, migrating or writing the registry. With neither, a refusal: a handoff + * naming a guessed image would run the builder in something no one bound. + */ +function builderHandoffImage( + task: TaskRecipe, + imageId: string | undefined, + stateDir: string | undefined, +): { localId: string; tag: string } { + if (imageId !== undefined) { + if (!isFactoryImageId(imageId)) + throw new Error(`--image-id must be sha256:<64 hex>, got ${JSON.stringify(imageId)}`) + return { localId: imageId, tag: recipeTag(task.target) } + } + if (!stateDir) + throw new Error( + "builder-handoff needs --image-id, or FACTORY_STATE_DIR whose images.sqlite records the task's image (target:prepare writes it)", + ) + let reader: ReturnType + try { + reader = openImageRegistryReader(join(stateDir, "images.sqlite")) + } catch (error) { + // Missing, schema-less or newer: the same refusal, so the hint (--image-id) appears. + throw new Error( + `builder-handoff needs --image-id, or FACTORY_STATE_DIR whose images.sqlite records the task's image: ${error instanceof Error ? error.message : String(error)}`, + ) + } + try { + const recorded = reader.recorded(task.target) + if (recorded === undefined) + throw new Error( + `builder-handoff needs --image-id, or FACTORY_STATE_DIR whose images.sqlite records the task's image: none is recorded for target ${task.target.id} at ${task.target.pin}`, + ) + return { localId: recorded.image.localId, tag: recorded.tag } + } finally { + reader.close() + } +} + const registryPath = (): string => { const stateDir = process.env.FACTORY_STATE_DIR if (!stateDir) throw new Error("FACTORY_STATE_DIR is required to read the registry") @@ -796,6 +842,7 @@ async function main(argv: string[]): Promise { out: { type: "string" }, pin: { type: "string" }, "work-order": { type: "string" }, + "image-id": { type: "string" }, approve: { type: "boolean", default: false }, reject: { type: "boolean", default: false }, "allow-missing-evidence": { type: "boolean", default: false }, @@ -816,7 +863,8 @@ async function main(argv: string[]): Promise { return id } // Answered before anything is opened: writing a builder handoff reads the catalog and - // captures an archive, and needs neither a controller nor a registry. + // captures an archive, and needs no controller; it opens the image registry read-only only + // when --image-id is absent. if (command === "builder-handoff") { if (!values.task) throw new Error("builder-handoff requires --task") if (!values.out) throw new Error("builder-handoff requires --out") @@ -824,6 +872,8 @@ async function main(argv: string[]): Promise { // builder root, only the state directory's generated tasks, and only when there is one. const stateDir = process.env.FACTORY_STATE_DIR if (stateDir) configureCatalog({ generatedTasksDir: generatedTasksDirFor(stateDir) }) + const task = loadTaskRecipe(values.task) + const image = builderHandoffImage(task, values["image-id"], stateDir) const workOrder = values["work-order"] // The capture is staged under the state directory when there is one (where the controller // stages its own), and otherwise under a temporary directory this command removes: never @@ -832,8 +882,9 @@ async function main(argv: string[]): Promise { ? resolve(stateDir) : mkdtempSync(join(tmpdir(), "factory-captures-")) try { - const { handoff, workspace } = await captureBuilderHandoff(loadTaskRecipe(values.task), { + const { handoff, workspace } = await captureBuilderHandoff(task, { captureRoot, + image, ...(workOrder !== undefined ? { workOrderId: workOrder } : {}), }) // The work order id is a catalog id (the capture refused anything else): a plain name. diff --git a/examples/software-factory/controller/src/lib/builder-handoff.ts b/examples/software-factory/controller/src/lib/builder-handoff.ts index d46b657f8..564e8c617 100644 --- a/examples/software-factory/controller/src/lib/builder-handoff.ts +++ b/examples/software-factory/controller/src/lib/builder-handoff.ts @@ -6,7 +6,6 @@ import { captureWorkspaceDefinition } from "@b4run/workspace/node" import { z } from "zod" import { captureDirectory } from "./targets/archive.js" import { isCatalogId, type TaskRecipe } from "./targets/catalog.js" -import { recipeTag } from "./targets/images.js" import { builderPermissions } from "./targets/permissions.js" import { targetSandboxPolicy, targetWorkspace } from "./targets/workspace.js" @@ -19,15 +18,20 @@ import { targetSandboxPolicy, targetWorkspace } from "./targets/workspace.js" const CATALOG_ID = /^[A-Za-z0-9][A-Za-z0-9._-]*$/ /** - * A tag in the factory's shape: `b4-factory-:-`, the tag the - * builder's sandbox runs (the verifier never runs a tag: it runs the work order's bound image - * by its id), with the target and the pin prefix captured. The builder's provider allows no - * other shape, and the handoff schema requires the captured target and pin prefix to be the - * handoff's own `targetId` and `pin`. That bounds a handoff to an image present on the daemon - * under a tag naming its own target and pin; it does not prove the image is the one the work - * order bound (PR 2 moves the builder to the id). + * The recipe tag's shape: `b4-factory-:-`, with the target, the pin + * prefix and the key prefix captured. A handoff carries its bound image's tag beside the id, as + * its name; the handoff schema requires the captured target and pin prefix to be the handoff's + * own `targetId` and `pin`. Nothing runs the tag: a tag can move, so the builder runs the id. */ -const FACTORY_IMAGE = /^b4-factory-([A-Za-z0-9][A-Za-z0-9._-]*):([0-9a-f]{12})-[0-9a-f]{12}$/ +const FACTORY_IMAGE = /^b4-factory-([A-Za-z0-9][A-Za-z0-9._-]*):([0-9a-f]{12})-([0-9a-f]{12})$/ + +/** + * An image by its id, `sha256:<64 hex>`: what a handoff names and the builder's provider runs. + * The framework records `docker image inspect `'s `.Id` as the thread's environment + * identity, which for an id is the id itself, so the builder runs exactly the image the + * controller bound, whatever any tag names by then. + */ +const IMAGE_ID = /^sha256:[0-9a-f]{64}$/ /** * Everything the builder app's `b4.config.ts` needs for one thread besides its files, as data: @@ -51,18 +55,22 @@ const FACTORY_IMAGE = /^b4-factory-([A-Za-z0-9][A-Za-z0-9._-]*):([0-9a-f]{12})-[ * refusal at admission, never a silent drop to a broader default (a misspelled `netwrok` would * otherwise leave the thread under the app's network rather than the one the controller * wrote). The network is `deny` only (the builder app denies it too, and a thread may not open - * what its app denies), with no `allowlist` or `denylist`; the image must be one the factory - * prepared; a pattern that is empty or only whitespace, which names nothing, is refused. + * what its app denies), with no `allowlist` or `denylist`; the image is named by its id, never + * a tag, and the tag beside it must name the handoff's own target and pin; a pattern that is + * empty or only whitespace, which names nothing, is refused. */ export const BuilderHandoffSchema = z .object({ - version: z.literal(3), + version: z.literal(4), workOrderId: z.string().regex(CATALOG_ID), taskId: z.string().regex(CATALOG_ID), targetId: z.string().regex(CATALOG_ID), target: z .object({ - image: z.string().regex(FACTORY_IMAGE), + /** The bound image, by id: the one the controller prepared and verifies in. */ + image: z.string().regex(IMAGE_ID), + /** Its recipe tag; target and pin segments must be this handoff's own. */ + tag: z.string().regex(FACTORY_IMAGE), /** The commit that image was prepared at. */ pin: z.string().regex(/^[a-f0-9]{40}$/), policy: z @@ -100,19 +108,19 @@ export const BuilderHandoffSchema = z .strict() .superRefine((handoff, ctx) => { // The tag's target and pin segments must be this handoff's own: a work order may not - // run in another target's image, or in its own target's image at another pin. - const [, target, pin] = FACTORY_IMAGE.exec(handoff.target.image) ?? [] + // name another target's image, or its own target's image at another pin. + const [, target, pin] = FACTORY_IMAGE.exec(handoff.target.tag) ?? [] if (target !== handoff.targetId || pin !== handoff.target.pin.slice(0, 12)) ctx.addIssue({ code: "custom", - path: ["target", "image"], - message: `image ${handoff.target.image} is not target ${handoff.targetId} at pin ${handoff.target.pin}: a factory tag names b4-factory-${handoff.targetId}:${handoff.target.pin.slice(0, 12)}-`, + path: ["target", "tag"], + message: `tag ${handoff.target.tag} is not target ${handoff.targetId} at pin ${handoff.target.pin}: a factory tag names b4-factory-${handoff.targetId}:${handoff.target.pin.slice(0, 12)}-`, }) }) export type BuilderHandoff = z.infer -/** Whether `reference` is an image the factory prepared: the builder's `dockerSandbox({ images })`. */ -export const isFactoryImage = (reference: string): boolean => FACTORY_IMAGE.test(reference) +/** Whether `reference` is an image id: the builder's `dockerSandbox({ images })`. */ +export const isFactoryImageId = (reference: string): boolean => IMAGE_ID.test(reference) /** What `POST /threads` names: the links and baseline, which the source digest does not cover. */ export function stagedReferenceOf( @@ -138,11 +146,10 @@ export interface CaptureBuilderHandoffOptions { readonly captureRoot: string readonly signal?: AbortSignal /** - * The image tag the handoff names: the work order's bound tag (`image_bound`). This host's - * recipe tag for the task when absent (`recipeTag`), which is what `factory builder-handoff` - * writes for a lane with no controller. + * The bound image (`image_bound`): its id is what the builder runs, its tag what it is named. + * `factory builder-handoff` supplies one from `--image-id` or the host's image registry. */ - readonly tag?: string + readonly image: { readonly localId: string; readonly tag: string } } export interface CapturedBuilderHandoff { @@ -184,13 +191,14 @@ export async function captureBuilderHandoff( // Parsed, not merely typed: the controller checks what it sends against the SAME schema the // builder will apply to it, so a handoff the builder would refuse cannot be produced here. const handoff = BuilderHandoffSchema.parse({ - version: 3, + version: 4, workOrderId, taskId: task.id, targetId: task.target.id, workspace: stagedReferenceOf(workspace), target: { - image: options.tag ?? recipeTag(task.target), + image: options.image.localId, + tag: options.image.tag, pin: task.target.pin, policy: targetSandboxPolicy(task.target), permissions: builderPermissions(task.target), diff --git a/examples/software-factory/controller/src/lib/controller/factory.ts b/examples/software-factory/controller/src/lib/controller/factory.ts index e8a2337a7..3da6bb836 100644 --- a/examples/software-factory/controller/src/lib/controller/factory.ts +++ b/examples/software-factory/controller/src/lib/controller/factory.ts @@ -119,8 +119,12 @@ export interface FactoryOptions { readonly taskId: string readonly workOrderId: string readonly signal: AbortSignal - /** The work order's bound tag (`image_bound`); this host's recipe tag when absent. */ - readonly tag?: string + /** + * The work order's bound image (`image_bound`): its id is what the builder runs, its tag + * what it is named. Absent only under the `tasks` test seam, which binds none; the + * catalog capture refuses without one. + */ + readonly image?: { readonly localId: string; readonly tag: string } }) => Promise /** The controller's own baseline for a task. Injected so tests need no container. */ captureBaseline( @@ -285,15 +289,17 @@ export async function createFactory(options: FactoryOptions): Promise { * (`promptCatalog` when a test scopes one, the process-wide search path otherwise), * captured. */ - const captureBuilderHandoffFromCatalog: NonNullable = ( - input, - ) => - captureBuilderHandoffOfTask(loadTaskRecipe(input.taskId, options.promptCatalog ?? {}), { + const captureBuilderHandoffFromCatalog: NonNullable< + FactoryOptions["captureBuilderHandoff"] + > = async (input) => { + if (input.image === undefined) throw new Error("dispatch bound no image") + return captureBuilderHandoffOfTask(loadTaskRecipe(input.taskId, options.promptCatalog ?? {}), { workOrderId: input.workOrderId, captureRoot: options.captureRoot, signal: input.signal, - ...(input.tag !== undefined ? { tag: input.tag } : {}), + image: input.image, }) + } const now = options.now ?? Date.now const iso = () => new Date(now()).toISOString() const abort = new AbortController() @@ -1005,7 +1011,7 @@ export async function createFactory(options: FactoryOptions): Promise { taskId: row.taskId, workOrderId: id, signal: abort.signal, - ...(bound !== undefined ? { tag: bound.tag } : {}), + ...(bound !== undefined ? { image: { localId: bound.image.localId, tag: bound.tag } } : {}), }) const status = await worker.client.uploadSource(captured.workspace.source, abort.signal) recordEvent(id, "builder_source_staged", { diff --git a/examples/software-factory/controller/src/lib/targets/image-builder.ts b/examples/software-factory/controller/src/lib/targets/image-builder.ts index effd8344e..8051513e2 100644 --- a/examples/software-factory/controller/src/lib/targets/image-builder.ts +++ b/examples/software-factory/controller/src/lib/targets/image-builder.ts @@ -7,6 +7,17 @@ import { ImageSchema, idTagFor } from "./catalog.js" import { baseDigestOf, type ImageBuilder } from "./images.js" import { recipeProblem } from "./prepare.js" +/** + * The labels every factory image build writes into the image config (which its id + * content-addresses), and the names the builder reads back to admit an image for its handoff. + * The builder's copy (`server/src/builder-handoff.ts`) is pinned to this one by a test. + */ +export const FACTORY_LABELS = { + target: "b4.factory.target", + pin: "b4.factory.pin", + key: "b4.factory.key", +} as const + /** Run a command to completion: its stdout, or an Error naming the exit and stderr's tail. */ export type Run = ( command: string, @@ -135,11 +146,11 @@ export function dockerImageBuilder(options: DockerImageBuilderOptions = {}): Ima `PNPM_VERSION=${pnpmVersion}`, // Part of the image config the id content-addresses: PR 2's builder checks them. "--label", - `b4.factory.target=${recipe.id}`, + `${FACTORY_LABELS.target}=${recipe.id}`, "--label", - `b4.factory.pin=${recipe.pin}`, + `${FACTORY_LABELS.pin}=${recipe.pin}`, "--label", - `b4.factory.key=${key}`, + `${FACTORY_LABELS.key}=${key}`, // This build's own id, whatever any tag names by the time it is read (D10). "--iidfile", iidFile, diff --git a/examples/software-factory/controller/src/lib/targets/images.ts b/examples/software-factory/controller/src/lib/targets/images.ts index 67f88e753..d8f2a868e 100644 --- a/examples/software-factory/controller/src/lib/targets/images.ts +++ b/examples/software-factory/controller/src/lib/targets/images.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto" -import { mkdirSync, readFileSync } from "node:fs" +import { existsSync, mkdirSync, readFileSync } from "node:fs" import { dirname, join } from "node:path" import { DatabaseSync } from "node:sqlite" import { imageRecipeDigest } from "../domain/digest.js" @@ -320,13 +320,29 @@ interface Flight { settled: boolean } -export function openImageRegistry(options: ImageRegistryOptions): ImageRegistry { - mkdirSync(dirname(options.path), { recursive: true }) - const db = new DatabaseSync(options.path) - db.exec("PRAGMA journal_mode = WAL") - db.exec("PRAGMA busy_timeout = 5000") - // The version first: a registry a newer factory wrote is refused before this one creates - // its own tables or indexes in it. +/** The recorded image under `key`, or undefined: the one read both the registry and its reader do. */ +function readRecorded(db: DatabaseSync, key: string): Image | undefined { + const row = db + .prepare( + "SELECT tag, local_id, platform, base_manifest_digest, dockerfile_sha256, lockfile_sha256, pnpm_version FROM images WHERE key = ?", + ) + .get(key) as unknown as ImageRow | undefined + if (row === undefined) return undefined + return ImageSchema.parse({ + localId: row.local_id, + platform: row.platform, + baseManifestDigest: row.base_manifest_digest, + dockerfileSha256: row.dockerfile_sha256, + lockfileSha256: row.lockfile_sha256, + pnpmVersion: row.pnpm_version, + }) +} + +/** + * The schema version `db` records (0 when it has no `schema_version` table), refusing, and + * closing `db`, when a newer factory wrote it. + */ +function checkedSchemaVersion(db: DatabaseSync): number { const versioned = db .prepare("SELECT 1 AS present FROM sqlite_master WHERE type = 'table' AND name = ?") @@ -343,6 +359,62 @@ export function openImageRegistry(options: ImageRegistryOptions): ImageRegistry `The image registry schema version ${found} is newer than this factory supports (${IMAGE_REGISTRY_VERSION}): upgrade the factory, or give it another FACTORY_STATE_DIR`, ) } + return found +} + +/** The read-only registry `openImageRegistryReader` opens: `recorded`, and nothing that writes. */ +export interface ImageRegistryReader { + /** What the registry records for `recipe` at its own pin on the reader's platform, if anything. */ + recorded(recipe: TargetRecipe): RecordedImage | undefined + close(): void +} + +/** + * The registry, read-only: what a command that must not create, migrate or write a host's + * registry opens (`factory builder-handoff`). It creates no registry and writes nothing to it; + * SQLite may leave `-wal`/`-shm` files beside a quiescent WAL registry, which is SQLite's own + * bookkeeping, not a write to the registry. Refuses a path with no registry, one written by a + * newer factory, and one no factory has finished creating (no schema version), which it leaves + * as it found it. + */ +export function openImageRegistryReader( + path: string, + platform: string = hostPlatform(), +): ImageRegistryReader { + if (!existsSync(path)) throw new Error(`no image registry at ${path}`) + const db = new DatabaseSync(path, { readOnly: true }) + let found: number + try { + db.exec("PRAGMA busy_timeout = 5000") + found = checkedSchemaVersion(db) + } catch (error) { + if (db.isOpen) db.close() + throw error + } + if (found < IMAGE_REGISTRY_VERSION) { + db.close() + throw new Error(`no image registry at ${path}: it records no schema version`) + } + return { + recorded(recipe) { + const key = recipeKey(recipe, platform) + const image = readRecorded(db, key) + return image === undefined + ? undefined + : { key, tag: tagFor(recipe.id, recipe.pin, key), image } + }, + close: () => db.close(), + } +} + +export function openImageRegistry(options: ImageRegistryOptions): ImageRegistry { + mkdirSync(dirname(options.path), { recursive: true }) + const db = new DatabaseSync(options.path) + db.exec("PRAGMA journal_mode = WAL") + db.exec("PRAGMA busy_timeout = 5000") + // The version first: a registry a newer factory wrote is refused before this one creates + // its own tables or indexes in it. + const found = checkedSchemaVersion(db) db.exec(SCHEMA) if (found < IMAGE_REGISTRY_VERSION) db.prepare("INSERT OR IGNORE INTO schema_version(version) VALUES (?)").run( @@ -362,22 +434,7 @@ export function openImageRegistry(options: ImageRegistryOptions): ImageRegistry const key = recipeKey(recipe, platform, dockerfileSha256) return { key, tag: tagFor(recipe.id, recipe.pin, key), dockerfileSha256 } } - const read = (key: string): Image | undefined => { - const row = db - .prepare( - "SELECT tag, local_id, platform, base_manifest_digest, dockerfile_sha256, lockfile_sha256, pnpm_version FROM images WHERE key = ?", - ) - .get(key) as unknown as ImageRow | undefined - if (row === undefined) return undefined - return ImageSchema.parse({ - localId: row.local_id, - platform: row.platform, - baseManifestDigest: row.base_manifest_digest, - dockerfileSha256: row.dockerfile_sha256, - lockfileSha256: row.lockfile_sha256, - pnpmVersion: row.pnpm_version, - }) - } + const read = (key: string): Image | undefined => readRecorded(db, key) const write = (described: Described, recipe: TargetRecipe, image: Image, ms: number) => { db.prepare( `INSERT OR REPLACE INTO images (key, target_id, pin, tag, local_id, platform, base_manifest_digest, dockerfile_sha256, lockfile_sha256, pnpm_version, built_at, build_ms) diff --git a/examples/software-factory/controller/test/builder-handoff.test.ts b/examples/software-factory/controller/test/builder-handoff.test.ts index bf2a80df5..934851f27 100644 --- a/examples/software-factory/controller/test/builder-handoff.test.ts +++ b/examples/software-factory/controller/test/builder-handoff.test.ts @@ -5,8 +5,10 @@ import { createSourceBundle, verifyCapturedWorkspaceDefinition } from "@b4run/wo import { afterEach, describe, expect, it } from "vitest" import { builderHandoffOf, + isFactoryImageId, refuseRetiredVariables, stagedBuilderWorkspace, + FACTORY_LABELS as THE_BUILDERS_LABELS, BuilderHandoffSchema as TheBuildersHandoffSchema, } from "../../server/src/builder-handoff.ts" import { @@ -14,7 +16,8 @@ import { captureBuilderHandoff, stagedReferenceOf, } from "../src/lib/builder-handoff.ts" -import { loadTask, loadTaskRecipe } from "../src/lib/targets/catalog.ts" +import { loadTask, loadTaskRecipe, type TaskRecipe } from "../src/lib/targets/catalog.ts" +import { FACTORY_LABELS } from "../src/lib/targets/image-builder.ts" import { recipeTag } from "../src/lib/targets/images.ts" import { builderPermissions } from "../src/lib/targets/permissions.ts" import { targetSandboxPolicy } from "../src/lib/targets/workspace.ts" @@ -29,13 +32,23 @@ const tempDir = (prefix: string) => { return dir } +/** An image bound for `task`: an id, and this host's recipe tag for its target. */ +const boundTo = (task: TaskRecipe) => ({ + localId: `sha256:${"3".repeat(64)}`, + tag: recipeTag(task.target), +}) + describe("captureBuilderHandoff", () => { - it("names the tag it is given: the one the work order bound", async () => { + it("names the image it is given by id, and its tag: the one the work order bound", async () => { const { handoff } = await captureBuilderHandoff(loadTaskRecipe("cli-flags"), { captureRoot: tempDir("factory-handoff-app-"), - tag: `b4-factory-cli-flags:${loadTaskRecipe("cli-flags").target.pin.slice(0, 12)}-0123456789ab`, + image: { + localId: `sha256:${"2".repeat(64)}`, + tag: `b4-factory-cli-flags:${loadTaskRecipe("cli-flags").target.pin.slice(0, 12)}-0123456789ab`, + }, }) - expect(handoff.target.image).toMatch(/-0123456789ab$/) + expect(handoff.target.image).toBe(`sha256:${"2".repeat(64)}`) + expect(handoff.target.tag).toMatch(/-0123456789ab$/) }) it("carries the work order, the task, the target and the reference its workspace is staged under, no prompt", async () => { @@ -44,6 +57,7 @@ describe("captureBuilderHandoff", () => { const { handoff, workspace } = await captureBuilderHandoff(task, { workOrderId: "wo-0123456789abcdef", captureRoot: app, + image: { localId: `sha256:${"1".repeat(64)}`, tag: recipeTag(task.target) }, }) // The prompt is the run's user message; the handoff does not carry a second copy, and the // workspace's files travel as the upload, not in the handoff. @@ -55,9 +69,10 @@ describe("captureBuilderHandoff", () => { "workOrderId", "workspace", ]) - expect(handoff.version).toBe(3) + expect(handoff.version).toBe(4) expect(handoff.target).toEqual({ - image: recipeTag(task.target), + image: `sha256:${"1".repeat(64)}`, + tag: recipeTag(task.target), pin: task.target.pin, policy: targetSandboxPolicy(task.target), permissions: builderPermissions(task.target), @@ -91,9 +106,9 @@ describe("captureBuilderHandoff", () => { const app = tempDir("factory-handoff-app-") const task = loadTask("cli-flags") const [first, second, third] = await Promise.all([ - captureBuilderHandoff(task, { captureRoot: app }), - captureBuilderHandoff(task, { workOrderId: "wo-a", captureRoot: app }), - captureBuilderHandoff(task, { workOrderId: "wo-b", captureRoot: app }), + captureBuilderHandoff(task, { captureRoot: app, image: boundTo(task) }), + captureBuilderHandoff(task, { workOrderId: "wo-a", captureRoot: app, image: boundTo(task) }), + captureBuilderHandoff(task, { workOrderId: "wo-b", captureRoot: app, image: boundTo(task) }), ]) expect(first?.handoff.workOrderId).toBe("cli-flags") // Concurrent captures of one task each staged in their own directory, so each read the @@ -108,6 +123,7 @@ describe("captureBuilderHandoff", () => { captureBuilderHandoff(loadTask("cli-flags"), { workOrderId: "../escape", captureRoot: tempDir("factory-handoff-app-"), + image: boundTo(loadTaskRecipe("cli-flags")), }), ).rejects.toThrow(/catalog id/) }) @@ -117,13 +133,14 @@ const DIGEST_SOURCE = createSourceBundle([ { path: "a.ts", bytes: new TextEncoder().encode("a"), executable: false }, ]) const handoff = { - version: 3, + version: 4, workOrderId: "wo-1", taskId: "cli-flags", targetId: "cli-flags", workspace: { sourceDigest: DIGEST_SOURCE.digest, environmentLinks: [], baseline: "git" }, target: { - image: `b4-factory-cli-flags:${"a".repeat(12)}-${"b".repeat(12)}`, + image: `sha256:${"0".repeat(64)}`, + tag: `b4-factory-cli-flags:${"a".repeat(12)}-${"b".repeat(12)}`, pin: "a".repeat(40), policy: { network: { mode: "deny" }, @@ -212,6 +229,26 @@ describe("the builder's handoff", () => { ).toThrow(/is not the one work order wo-1 names/) }) + it("names an image only by id, and its tag only as its own target's at its own pin", () => { + const id = `sha256:${"a".repeat(64)}` + const parse = (target: Record) => + TheBuildersHandoffSchema.safeParse({ ...handoff, target: { ...handoff.target, ...target } }) + .success + expect(parse({ image: id })).toBe(true) + for (const image of [ + handoff.target.tag, + "alpine:latest", + `sha256:${"a".repeat(63)}`, + `b4-factory-t@sha256:${"a".repeat(64)}`, + ]) + expect(parse({ image }), image).toBe(false) + expect(parse({ tag: `b4-factory-${handoff.targetId}:${"f".repeat(12)}-0123456789ab` })).toBe( + false, + ) + expect(isFactoryImageId(id)).toBe(true) + expect(isFactoryImageId(handoff.target.tag)).toBe(false) + }) + it("refuses the retired variables by name", () => { expect(() => refuseRetiredVariables({ FACTORY_BUILDER_MANIFEST_DIR: "/m" })).toThrow( /FACTORY_BUILDER_MANIFEST_DIR is retired/, @@ -225,13 +262,14 @@ describe("the builder's handoff", () => { describe("the handoff's target block", () => { const good = () => ({ - version: 3, + version: 4, workOrderId: "wo-a", taskId: "cli-flags", targetId: "cli-flags", workspace: { sourceDigest: "c".repeat(64), environmentLinks: [], baseline: "git" }, target: { - image: "b4-factory-cli-flags:6a59e00aed46-0123456789ab", + image: `sha256:${"e".repeat(64)}`, + tag: "b4-factory-cli-flags:6a59e00aed46-0123456789ab", pin: `6a59e00aed46${"0".repeat(28)}`, policy: { network: { mode: "deny" }, @@ -264,12 +302,17 @@ describe("the handoff's target block", () => { (m: Handoff) => withPolicy(m, { network: { mode: "deny", allowlist: ["10.0.0.0/8"] } }), ], [ - "an image that is not the factory's", + "an image named by a reference, not an id", (m: Handoff) => withTarget(m, { image: "alpine:latest" }), ], [ - "a factory-named image under a floating tag", - (m: Handoff) => withTarget(m, { image: "b4-factory-cli-flags:latest" }), + "an image named by its factory tag, not its id", + (m: Handoff) => withTarget(m, { image: m.target.tag }), + ], + ["no tag", (m: Handoff) => withTarget(m, { tag: undefined })], + [ + "a factory-named tag that floats", + (m: Handoff) => withTarget(m, { tag: "b4-factory-cli-flags:latest" }), ], ["a security key", (m: Handoff) => withPolicy(m, { security: {} })], [ @@ -280,13 +323,14 @@ describe("the handoff's target block", () => { ["a whitespace-only pattern", (m: Handoff) => withTarget(m, { permissions: { bash: [" "] } })], ["an unknown target key", (m: Handoff) => withTarget(m, { scope: "elsewhere" })], ["version 2, the manifest's", (m: Handoff) => ({ ...m, version: 2 })], + ["version 3, which named the image by tag", (m: Handoff) => ({ ...m, version: 3 })], [ - "another target's image", - (m: Handoff) => withTarget(m, { image: "b4-factory-devkit:6a59e00aed46-0123456789ab" }), + "another target's tag", + (m: Handoff) => withTarget(m, { tag: "b4-factory-devkit:6a59e00aed46-0123456789ab" }), ], [ - "its own target's image at another pin", - (m: Handoff) => withTarget(m, { image: "b4-factory-cli-flags:bfaf0c2b3030-0123456789ab" }), + "its own target's tag at another pin", + (m: Handoff) => withTarget(m, { tag: "b4-factory-cli-flags:bfaf0c2b3030-0123456789ab" }), ], ["no workspace", (m: Handoff) => ({ ...m, workspace: undefined })], ["a workspace digest that is not one", (m: Handoff) => withWorkspace(m, { sourceDigest: "x" })], @@ -322,9 +366,16 @@ describe("the builder's copy of the schemas", () => { expect(schemas(there)).toBe(block) const rule = (text: string, name: string) => text.match(new RegExp(`^const ${name} = (.*)$`, "m"))?.[1] - for (const name of ["CATALOG_ID", "FACTORY_IMAGE"]) { + for (const name of ["CATALOG_ID", "FACTORY_IMAGE", "IMAGE_ID"]) { expect(rule(here, name)).toBeDefined() expect(rule(there, name)).toBe(rule(here, name)) } }) + + it("checks the three labels the controller's image build writes, by the same names", () => { + // The builder refuses an image whose labels it cannot find: a rename on one side only + // would refuse every image the factory builds. + expect(Object.keys(FACTORY_LABELS).sort()).toEqual(["key", "pin", "target"]) + expect(THE_BUILDERS_LABELS).toEqual(FACTORY_LABELS) + }) }) diff --git a/examples/software-factory/controller/test/builder.integration.test.ts b/examples/software-factory/controller/test/builder.integration.test.ts index 58deb8562..1c016efc8 100644 --- a/examples/software-factory/controller/test/builder.integration.test.ts +++ b/examples/software-factory/controller/test/builder.integration.test.ts @@ -14,7 +14,7 @@ import { } from "../src/lib/builder-handoff.ts" import { taskPrompt } from "../src/lib/prompts.ts" import { captureTarget } from "../src/lib/targets/archive.ts" -import { loadTarget, loadTask } from "../src/lib/targets/catalog.ts" +import { loadTarget, loadTargetRecipe, loadTask } from "../src/lib/targets/catalog.ts" import { imageTag } from "../src/lib/targets/images.ts" import { SECOND_PIN } from "./devkit-second-pin.ts" import { ensureLaneImage } from "./lane-images.ts" @@ -58,6 +58,7 @@ beforeAll(async () => { const alpha = await captureBuilderHandoff(task, { workOrderId: "wo-alpha", captureRoot: alphaRoot, + image: { localId: task.target.image.localId, tag: imageTag(task.target) }, }).finally(() => rm(alphaRoot, { recursive: true, force: true })) handoffs["wo-alpha"] = alpha digests["wo-alpha"] = alpha.handoff.workspace.sourceDigest @@ -296,6 +297,7 @@ it("serves a cli-flags thread and devkit threads at two pins from one process", const devkit = await captureBuilderHandoff(devkitTask, { workOrderId: "wo-devkit", captureRoot: root, + image: { localId: devkitTask.target.image.localId, tag: imageTag(devkitTask.target) }, }) // The same capture at the second pin: only the target block changes, which is all the // image and the policy are drawn from. Captured as dispatch would, then re-pinned. @@ -307,7 +309,12 @@ it("serves a cli-flags thread and devkit threads at two pins from one process", handoff: BuilderHandoffSchema.parse({ ...devkit.handoff, workOrderId: "wo-devkit-2", - target: { ...devkit.handoff.target, image: imageTag(atSecond), pin: SECOND_PIN }, + target: { + ...devkit.handoff.target, + image: atSecond.image.localId, + tag: imageTag(atSecond), + pin: SECOND_PIN, + }, }), }, } @@ -368,6 +375,71 @@ it("serves a cli-flags thread and devkit threads at two pins from one process", want.memoryMb * 1024 * 1024, ]) } + + // A handoff admitted or refused: upload its source, create its thread, run one turn. + // `true` when the turn ran; otherwise the refusal's text, so each refusal is pinned to + // its own reason rather than to any failure at all. + const admitted = async ( + workOrderId: string, + factoryBuilder: unknown, + ): Promise => { + await builder.client.uploadSource(devkit.workspace.source) + try { + const threadId = await builder.client.createThread( + { factoryWorkOrderId: workOrderId, factoryBuilder }, + stagedReferenceOf(devkit.workspace), + ) + threads.push(threadId) + builder.aimock.addFixtures( + script().user(LIST).callsTool("listDir", { path: "." }).replies("Listed.").build(), + ) + const turn = await builder.runTurn(threadId, LIST) + return turn.status === 200 ? true : turn.text + } catch (error) { + return error instanceof Error ? error.message : String(error) + } + } + const tag = imageTag(devkitTask.target) + const base = loadTargetRecipe("devkit").baseImage + const baseId = execFileSync("docker", ["image", "inspect", "--format", "{{.Id}}", base], { + encoding: "utf8", + }).trim() + // Moved on a shared daemon, so put back in the `finally` below, whatever fails. + execFileSync("docker", ["tag", base, tag]) + try { + // The recipe tag now names the base image; a thread handed the bound id runs the bound id. + const moved = { ...devkit.handoff, workOrderId: "wo-devkit-3" } + expect(await admitted("wo-devkit-3", moved)).toBe(true) + const record = openWorkspaceInstallationReader(builder.appRoot) + let operation: string + try { + operation = record.associations.get(threads.at(-1) as string)?.intent.operationId as string + } finally { + record.close() + } + expect(sessionOf(operation).Image).toBe(devkitTask.target.image.localId) + // The pre-version-4 shape (a tag in `image`) is refused by the schema at admission. + expect( + await admitted("wo-devkit-4", { + ...moved, + workOrderId: "wo-devkit-4", + version: 3, + target: { ...moved.target, image: tag }, + }), + ).toContain("thread metadata factoryBuilder is invalid") + // An id the daemon holds but the factory did not build for this handoff is refused by + // its labels. + expect( + await admitted("wo-devkit-5", { + ...moved, + workOrderId: "wo-devkit-5", + target: { ...moved.target, image: baseId }, + }), + ).toContain(`image ${baseId} carries no factory labels`) + } finally { + // Put the recipe tag back on the bound image, as the registry's next `ensure` would. + execFileSync("docker", ["tag", devkitTask.target.image.localId, tag]) + } } finally { await rm(root, { recursive: true, force: true }) } diff --git a/examples/software-factory/controller/test/cli.test.ts b/examples/software-factory/controller/test/cli.test.ts index ed4ad0bdf..a744f7b60 100644 --- a/examples/software-factory/controller/test/cli.test.ts +++ b/examples/software-factory/controller/test/cli.test.ts @@ -3,6 +3,8 @@ import { appendFileSync, chmodSync, cpSync, + existsSync, + mkdirSync, mkdtempSync, readFileSync, rmSync, @@ -16,7 +18,7 @@ import { verifySourceBundle } from "@b4run/workspace/node" import { afterEach, describe, expect, it } from "vitest" import { BuilderHandoffSchema } from "../src/lib/builder-handoff.ts" import { openRegistryReader } from "../src/lib/registry/reader.ts" -import { loadTask, tasksDir } from "../src/lib/targets/catalog.ts" +import { loadTask, loadTaskRecipe, tasksDir } from "../src/lib/targets/catalog.ts" import { openImageRegistry } from "../src/lib/targets/images.ts" import { fakeImageBuilder } from "./fake-image-builder.ts" import { createFakeVerifier } from "./fake-verifier.ts" @@ -36,6 +38,8 @@ const run = promisify(execFile) const tsxBin = join(import.meta.dirname, "../node_modules/tsx/dist/cli.mjs") const cliEntry = join(import.meta.dirname, "../src/cli.ts") const packageRoot = join(import.meta.dirname, "..") +/** A lane with no controller names the image the handoff runs by id. */ +const IMAGE_ID_ARGS = ["--image-id", `sha256:${"1".repeat(64)}`] let dir: string // Undefined for the tests that serve nothing, and cleared after every test so a later one @@ -1197,7 +1201,7 @@ esac // The work order defaults to the task: a lane with no controller names the files itself. const { stdout } = await run( process.execPath, - [tsxBin, cliEntry, "builder-handoff", "--task", "cli-flags", "--out", out], + [tsxBin, cliEntry, "builder-handoff", "--task", "cli-flags", "--out", out, ...IMAGE_ID_ARGS], { env: rest, cwd: packageRoot }, ) const written = JSON.parse(stdout) @@ -1205,9 +1209,10 @@ esac expect(written.source).toBe(join(out, "cli-flags.source.json")) const handoff = BuilderHandoffSchema.parse(JSON.parse(readFileSync(written.handoff, "utf8"))) expect(handoff).toMatchObject({ taskId: "cli-flags", workOrderId: "cli-flags", targetId }) - // The target block: the image prepared at the task's pin, and the pin beside it. + // The target block: the image it was given, by id, its tag at the task's pin, and the pin. expect(handoff.target.pin).toBe(task.target.pin) - expect(handoff.target.image).toContain(`:${task.target.pin.slice(0, 12)}-`) + expect(handoff.target.image).toBe(`sha256:${"1".repeat(64)}`) + expect(handoff.target.tag).toContain(`:${task.target.pin.slice(0, 12)}-`) expect(handoff.target.policy.network.mode).toBe("deny") // The source is the body `PUT /workspace/sources/` takes, under the handoff's digest. const source = verifySourceBundle(JSON.parse(readFileSync(written.source, "utf8"))) @@ -1225,6 +1230,7 @@ esac "wo-named", "--out", out, + ...IMAGE_ID_ARGS, ], { env: rest, cwd: packageRoot }, ) @@ -1251,7 +1257,16 @@ esac const out = join(dir, "handoffs") const { stdout } = await run( process.execPath, - [tsxBin, cliEntry, "builder-handoff", "--task", "wo-0123456789abcdef", "--out", out], + [ + tsxBin, + cliEntry, + "builder-handoff", + "--task", + "wo-0123456789abcdef", + "--out", + out, + ...IMAGE_ID_ARGS, + ], { env: { ...rest, FACTORY_STATE_DIR: join(dir, "state") }, cwd: packageRoot }, ) const { handoff } = JSON.parse(stdout) @@ -1260,4 +1275,57 @@ esac "wo-0123456789abcdef", ) }, 60_000) + + it("names the image the state directory's registry recorded, and refuses to guess one", async () => { + dir = mkdtempSync(join(tmpdir(), "factory-cli-")) + const { FACTORY_CONTROLLER_URL, FACTORY_WORKER_URL, FACTORY_STATE_DIR, ...rest } = process.env + const out = join(dir, "handoffs") + const args = [tsxBin, cliEntry, "builder-handoff", "--task", "cli-flags", "--out", out] + // No --image-id and no state directory: nothing to read the image from. + const unnamed = await failing(run(process.execPath, args, { env: rest, cwd: packageRoot })) + expect(unnamed.stderr).toContain( + "builder-handoff needs --image-id, or FACTORY_STATE_DIR whose images.sqlite records", + ) + const state = join(dir, "state") + mkdirSync(state, { recursive: true }) + // A state directory with no registry: refused, and none is created. + const absent = await failing( + run(process.execPath, args, { env: { ...rest, FACTORY_STATE_DIR: state }, cwd: packageRoot }), + ) + expect(absent.stderr).toContain( + `builder-handoff needs --image-id, or FACTORY_STATE_DIR whose images.sqlite records the task's image: no image registry at ${join(state, "images.sqlite")}`, + ) + expect(existsSync(join(state, "images.sqlite"))).toBe(false) + // A registry that records nothing for the task's target at its pin. + openImageRegistry({ path: join(state, "images.sqlite"), builder: fakeImageBuilder() }).close() + const unrecorded = await failing( + run(process.execPath, args, { env: { ...rest, FACTORY_STATE_DIR: state }, cwd: packageRoot }), + ) + expect(unrecorded.stderr).toContain("none is recorded for target") + // A malformed --image-id is refused, not passed through. + const malformed = await failing( + run(process.execPath, [...args, "--image-id", "alpine:latest"], { + env: rest, + cwd: packageRoot, + }), + ) + expect(malformed.stderr).toContain("--image-id must be sha256:<64 hex>") + + const registry = openImageRegistry({ + path: join(state, "images.sqlite"), + builder: fakeImageBuilder(), + }) + const ensured = await registry.ensure(loadTaskRecipe("cli-flags").target, { + signal: AbortSignal.timeout(5_000), + }) + registry.close() + const { stdout } = await run(process.execPath, args, { + env: { ...rest, FACTORY_STATE_DIR: state }, + cwd: packageRoot, + }) + const handoff = BuilderHandoffSchema.parse( + JSON.parse(readFileSync(JSON.parse(stdout).handoff, "utf8")), + ) + expect(handoff.target).toMatchObject({ image: ensured.image.localId, tag: ensured.tag }) + }, 60_000) }) diff --git a/examples/software-factory/controller/test/factory-dispatch.test.ts b/examples/software-factory/controller/test/factory-dispatch.test.ts index 9ccf36a64..63517484b 100644 --- a/examples/software-factory/controller/test/factory-dispatch.test.ts +++ b/examples/software-factory/controller/test/factory-dispatch.test.ts @@ -38,6 +38,8 @@ let factory: Factory let reader: FakeWorkspaceReader /** The tag each builder handoff capture was asked to name (`(none)` when none was bound). */ let handoffTags: string[] = [] +/** The image id each builder handoff capture was asked to name (`(none)` when none was bound). */ +let handoffImages: string[] = [] const REPAIRED = "export const fixed = true\n" @@ -78,7 +80,8 @@ async function boot( }, }), captureBuilderHandoff: async (input) => { - handoffTags.push(input.tag ?? "(none)") + handoffTags.push(input.image?.tag ?? "(none)") + handoffImages.push(input.image?.localId ?? "(none)") return fakeBuilderHandoff(input) }, exportDir: join(dir, "out"), @@ -129,6 +132,7 @@ async function until(condition: () => boolean, ms = 10_000): Promise { afterEach(async () => { resetCatalogForTests() handoffTags = [] + handoffImages = [] await factory?.close() await fake?.close() if (dir) rmSync(dir, { recursive: true, force: true }) @@ -429,7 +433,7 @@ describe("generated tasks", () => { }) describe("the task's image at dispatch", () => { - it("builds it while the row waits in received, binds it, and hands the builder its tag", async () => { + it("builds it while the row waits in received, binds it, and hands the builder its id and tag", async () => { await boot() const images = fakeImages() images.builder.hold() @@ -446,6 +450,9 @@ describe("the task's image at dispatch", () => { const types = factory.events(id).map((e) => e.type) expect(types.indexOf("image_bound")).toBeLessThan(types.indexOf("builder_source_staged")) expect(handoffTags).toEqual([bound?.payload.tag]) + const boundImage = (bound?.payload as { image?: { localId?: string } } | undefined)?.image + expect(boundImage?.localId).toMatch(/^sha256:[0-9a-f]{64}$/) + expect(handoffImages).toEqual([boundImage?.localId]) } finally { images.builder.release() images.restore() @@ -576,6 +583,9 @@ describe("the task's image at dispatch", () => { expect(await factory.dispatch(id)).toMatchObject({ ok: true, state: "dispatched" }) expect(images.builder.requests).toHaveLength(1) expect(eventsOf(id, "image_bound")).toHaveLength(1) + // The builder runs the image this work order bound, not the newer one the registry records. + expect(handoffImages).toEqual([earlier.localId]) + expect(handoffTags).toEqual([built.tag]) const { id: other } = await factory.create({ taskId: "cli-flags", operationKey: "other" }) const gone = { ...built.image, localId: `sha256:${"6".repeat(64)}` } @@ -707,6 +717,7 @@ describe("the task's image at dispatch", () => { }) expect(fake.requests.filter((r) => r.path === "/threads")).toHaveLength(0) expect(handoffTags).toEqual([]) + expect(handoffImages).toEqual([]) // Nothing spent: the key holds no outcome, so it is free for the dispatch after a fix. expect(factory.show(id)?.candidateAttempts).toBe(0) } finally { diff --git a/examples/software-factory/controller/test/fake-worker-map.ts b/examples/software-factory/controller/test/fake-worker-map.ts index 55e517349..43e056fbd 100644 --- a/examples/software-factory/controller/test/fake-worker-map.ts +++ b/examples/software-factory/controller/test/fake-worker-map.ts @@ -63,6 +63,7 @@ export function fakeWorkerMap(options: FakeWorkerMapOptions): WorkerMap { export const fakeBuilderHandoff: NonNullable = async ({ taskId, workOrderId, + image, }) => { const workspace = { version: 1 as const, @@ -73,13 +74,16 @@ export const fakeBuilderHandoff: NonNullable { .map((row) => (row as { name: string }).name) after.close() expect(tables).toEqual(["schema_version"]) + expect(() => openImageRegistryReader(join(dir, "images.sqlite"))).toThrow( + /image registry schema version 99 is newer than this factory supports \(1\)/, + ) + }) + + it("refuses a file no factory finished creating, and leaves it as it found it", () => { + const path = join(dir, "images.sqlite") + new DatabaseSync(path).close() + expect(() => openImageRegistryReader(path)).toThrow(/records no schema version/) + // Never migrated: the reader created no table or index in it. + const after = new DatabaseSync(path) + const objects = after.prepare("SELECT name FROM sqlite_master").all() + after.close() + expect(objects).toEqual([]) + }) + + it("creates no registry and writes nothing to it; SQLite may leave -wal/-shm beside a quiescent WAL registry", async () => { + const builder = fakeImageBuilder() + const recipe = recipeFixture() + expect(() => openImageRegistryReader(join(dir, "absent.sqlite"))).toThrow( + /no image registry at/, + ) + expect(existsSync(join(dir, "absent.sqlite"))).toBe(false) + const ensured = await ensure(open(builder), recipe) + const reader = openImageRegistryReader(join(dir, "images.sqlite"), "linux/arm64") + try { + expect(reader.recorded(recipe)).toEqual({ + key: ensured.key, + tag: ensured.tag, + image: ensured.image, + }) + expect(reader.recorded({ ...recipe, pin: "f".repeat(40) })).toBeUndefined() + } finally { + reader.close() + } + // The registry's contents are what the writer left: the reader wrote nothing to it. + expect(open(builder).recorded(recipe)).toEqual({ + key: ensured.key, + tag: ensured.tag, + image: ensured.image, + }) }) }) diff --git a/examples/software-factory/controller/test/targets-workspace.test.ts b/examples/software-factory/controller/test/targets-workspace.test.ts index 0ac0b8112..a09098f4d 100644 --- a/examples/software-factory/controller/test/targets-workspace.test.ts +++ b/examples/software-factory/controller/test/targets-workspace.test.ts @@ -3,7 +3,7 @@ import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node: import { tmpdir } from "node:os" import { join } from "node:path" import { afterEach, describe, expect, it } from "vitest" -import { isFactoryImage } from "../src/lib/builder-handoff.ts" +import { isFactoryImageId } from "../src/lib/builder-handoff.ts" import { type Task, tagFor } from "../src/lib/targets/catalog.ts" import { drafterInspectionOptions, @@ -169,10 +169,11 @@ describe("targetSandboxPolicy", () => { }) describe("the factory's images", () => { - it("are every one an image the builder's provider allows", () => { + it("are every one named, to the builder's provider, by an id it allows", () => { const { target } = task("0".repeat(40)) - expect(isFactoryImage(tagFor(target.id, target.pin, "c".repeat(64)))).toBe(true) - expect(isFactoryImage("alpine:latest")).toBe(false) + expect(isFactoryImageId(target.image.localId)).toBe(true) + expect(isFactoryImageId(tagFor(target.id, target.pin, "c".repeat(64)))).toBe(false) + expect(isFactoryImageId("alpine:latest")).toBe(false) }) }) diff --git a/examples/software-factory/server/b4.config.ts b/examples/software-factory/server/b4.config.ts index 70da7e6ec..1a1a74961 100644 --- a/examples/software-factory/server/b4.config.ts +++ b/examples/software-factory/server/b4.config.ts @@ -1,10 +1,9 @@ import { config } from "@b4run/cli" import { dockerSandbox } from "@b4run/sandbox" import { - builderHandoffOf, - isFactoryImage, + builderThreadSandbox, + isFactoryImageId, refuseRetiredVariables, - stagedBuilderWorkspace, } from "./src/builder-handoff.js" refuseRetiredVariables() @@ -13,9 +12,10 @@ export default config({ appDir: "src/app", build: { targets: ["node"] }, sandbox: { - // No default image: every thread runs the image its handoff names, and only an image the - // factory prepared may be named. A managed workspace's image is read from its own record. - provider: dockerSandbox({ scope: "software-factory-builder", images: isFactoryImage }), + // No default image: every thread runs the image its handoff names, by id, never a tag + // that could have moved since the controller bound it. A managed workspace's image is read + // from its own record. + provider: dockerSandbox({ scope: "software-factory-builder", images: isFactoryImageId }), // The controller reads a thread's workspace through this app's own port // (`POST /threads/:id/workspace/inspect`), authorized by src/thread-access.ts, and // never opens this app's installation store or its volumes itself. @@ -31,15 +31,9 @@ export default config({ // Per thread, once, at its first admission: the thread runs the staged workspace, image, // policy and permissions, recorded, and no other. The handoff and the staged workspace must // name the same digest, links and baseline; nothing is read from disk. - thread: async (thread) => { - const handoff = builderHandoffOf(thread.metadata) - return { - workspace: stagedBuilderWorkspace(thread.staged, handoff), - environment: { image: handoff.target.image }, - policy: handoff.target.policy, - permissions: { allow: handoff.target.permissions }, - } - }, + // After the image's build labels are checked against the handoff (by id, so the labels + // are the image's own): an image not built for this target, pin and recipe is refused. + thread: (thread) => builderThreadSandbox(thread), }, toolOutput: { // The controller never reads a tool result, so nothing here is load-bearing diff --git a/examples/software-factory/server/src/builder-handoff.ts b/examples/software-factory/server/src/builder-handoff.ts index 7a1e15bb0..de4084601 100644 --- a/examples/software-factory/server/src/builder-handoff.ts +++ b/examples/software-factory/server/src/builder-handoff.ts @@ -1,3 +1,4 @@ +import { execFile } from "node:child_process" import type { CapturedWorkspaceDefinition } from "@b4run/workspace" import { verifyCapturedWorkspaceDefinition } from "@b4run/workspace/node" import { z } from "zod" @@ -10,15 +11,20 @@ import { z } from "zod" const CATALOG_ID = /^[A-Za-z0-9][A-Za-z0-9._-]*$/ /** - * A tag in the factory's shape: `b4-factory-:-`, the tag the - * builder's sandbox runs (the verifier never runs a tag: it runs the work order's bound image - * by its id), with the target and the pin prefix captured. The builder's provider allows no - * other shape, and the handoff schema requires the captured target and pin prefix to be the - * handoff's own `targetId` and `pin`. That bounds a handoff to an image present on the daemon - * under a tag naming its own target and pin; it does not prove the image is the one the work - * order bound (PR 2 moves the builder to the id). + * The recipe tag's shape: `b4-factory-:-`, with the target, the pin + * prefix and the key prefix captured. A handoff carries its bound image's tag beside the id, as + * its name; the handoff schema requires the captured target and pin prefix to be the handoff's + * own `targetId` and `pin`. Nothing runs the tag: a tag can move, so the builder runs the id. */ -const FACTORY_IMAGE = /^b4-factory-([A-Za-z0-9][A-Za-z0-9._-]*):([0-9a-f]{12})-[0-9a-f]{12}$/ +const FACTORY_IMAGE = /^b4-factory-([A-Za-z0-9][A-Za-z0-9._-]*):([0-9a-f]{12})-([0-9a-f]{12})$/ + +/** + * An image by its id, `sha256:<64 hex>`: what a handoff names and the builder's provider runs. + * The framework records `docker image inspect `'s `.Id` as the thread's environment + * identity, which for an id is the id itself, so the builder runs exactly the image the + * controller bound, whatever any tag names by then. + */ +const IMAGE_ID = /^sha256:[0-9a-f]{64}$/ /** * The builder's one input per thread besides its staged workspace, carried in thread metadata @@ -43,18 +49,22 @@ const FACTORY_IMAGE = /^b4-factory-([A-Za-z0-9][A-Za-z0-9._-]*):([0-9a-f]{12})-[ * refusal at admission, never a silent drop to a broader default (a misspelled `netwrok` would * otherwise leave the thread under the app's network rather than the one the controller * wrote). The network is `deny` only (the builder app denies it too, and a thread may not open - * what its app denies), with no `allowlist` or `denylist`; the image must be one the factory - * prepared; a pattern that is empty or only whitespace, which names nothing, is refused. + * what its app denies), with no `allowlist` or `denylist`; the image is named by its id, never + * a tag, and the tag beside it must name the handoff's own target and pin; a pattern that is + * empty or only whitespace, which names nothing, is refused. */ export const BuilderHandoffSchema = z .object({ - version: z.literal(3), + version: z.literal(4), workOrderId: z.string().regex(CATALOG_ID), taskId: z.string().regex(CATALOG_ID), targetId: z.string().regex(CATALOG_ID), target: z .object({ - image: z.string().regex(FACTORY_IMAGE), + /** The bound image, by id: the one the controller prepared and verifies in. */ + image: z.string().regex(IMAGE_ID), + /** Its recipe tag; target and pin segments must be this handoff's own. */ + tag: z.string().regex(FACTORY_IMAGE), /** The commit that image was prepared at. */ pin: z.string().regex(/^[a-f0-9]{40}$/), policy: z @@ -92,19 +102,19 @@ export const BuilderHandoffSchema = z .strict() .superRefine((handoff, ctx) => { // The tag's target and pin segments must be this handoff's own: a work order may not - // run in another target's image, or in its own target's image at another pin. - const [, target, pin] = FACTORY_IMAGE.exec(handoff.target.image) ?? [] + // name another target's image, or its own target's image at another pin. + const [, target, pin] = FACTORY_IMAGE.exec(handoff.target.tag) ?? [] if (target !== handoff.targetId || pin !== handoff.target.pin.slice(0, 12)) ctx.addIssue({ code: "custom", - path: ["target", "image"], - message: `image ${handoff.target.image} is not target ${handoff.targetId} at pin ${handoff.target.pin}: a factory tag names b4-factory-${handoff.targetId}:${handoff.target.pin.slice(0, 12)}-`, + path: ["target", "tag"], + message: `tag ${handoff.target.tag} is not target ${handoff.targetId} at pin ${handoff.target.pin}: a factory tag names b4-factory-${handoff.targetId}:${handoff.target.pin.slice(0, 12)}-`, }) }) export type BuilderHandoff = z.infer -/** Whether `reference` is an image the factory prepared: the builder's `dockerSandbox({ images })`. */ -export const isFactoryImage = (reference: string): boolean => FACTORY_IMAGE.test(reference) +/** Whether `reference` is an image id: the builder's `dockerSandbox({ images })`. */ +export const isFactoryImageId = (reference: string): boolean => IMAGE_ID.test(reference) const describe = (error: unknown) => error instanceof z.ZodError @@ -211,3 +221,128 @@ export function stagedBuilderWorkspace( ) return verifyCapturedWorkspaceDefinition(staged) } + +/** + * The labels every factory image build stamps (the controller's `dockerImageBuilder`): the + * target and full pin the image was prepared for, and its full recipe key. They live in the + * image config, which the image id content-addresses. + */ +export const FACTORY_LABELS = { + target: "b4.factory.target", + pin: "b4.factory.pin", + key: "b4.factory.key", +} as const + +/** + * An image id's labels (`{}` when it has none), or null when the daemon does not hold it. + * `signal` is the thread's: an abandoned admission stops asking. + */ +export type InspectLabels = ( + id: string, + signal?: AbortSignal, +) => Promise> | null> + +/** Text from outside (a label value, the daemon's stderr) quoted into a refusal, capped. */ +const quoted = (value: unknown, max: number): string => { + const text = value === undefined ? "(none)" : JSON.stringify(value) + return text.length > max ? `${text.slice(0, max)}…` : text +} + +/** + * The daemon's answer for `id`, read by id. Fails closed: any error other than the daemon + * saying it holds no such image, and any answer that is not a string-valued object, rejects. + */ +export const dockerLabels: InspectLabels = (id, signal) => + new Promise((resolve, reject) => { + execFile( + "docker", + ["image", "inspect", "--format", "{{json .Config.Labels}}", "--", id], + { timeout: 30_000, ...(signal !== undefined ? { signal } : {}) }, + (error, stdout, stderr) => { + if (error) { + if (/No such image/i.test(String(stderr))) resolve(null) + else + reject( + new Error( + `docker image inspect ${id} failed: ${quoted(String(stderr).trim() || error.message, 500)}`, + { cause: error }, + ), + ) + return + } + try { + const parsed: unknown = JSON.parse(stdout) + if (parsed === null) return resolve({}) + if ( + typeof parsed !== "object" || + Array.isArray(parsed) || + !Object.values(parsed).every((value) => typeof value === "string") + ) + throw new Error("labels are not a string map") + resolve(parsed as Record) + } catch (cause) { + reject(new Error(`docker image inspect ${id} answered unreadable labels`, { cause })) + } + }, + ) + }) + +const RECIPE_KEY = /^[0-9a-f]{64}$/ + +/** + * Refuse, by name, the handoff's image when its build labels are not the handoff's own target, + * full pin and a recipe key whose prefix is the tag's key segment. An id alone binds no target: + * the provider admits any id the daemon holds. Read by id, so the labels are the image's own + * (the id content-addresses the config they live in) and nothing can change between this check + * and the run. Whoever can build or load images on the daemon can forge labels; that is the + * bound the tag shape had before. + */ +export async function assertFactoryImage( + handoff: BuilderHandoff, + inspect: InspectLabels = dockerLabels, + signal?: AbortSignal, +): Promise { + const id = handoff.target.image + const labels = await inspect(id, signal) + if (labels === null) throw new Error(`image ${id} is not on this daemon`) + if (!Object.hasOwn(labels, FACTORY_LABELS.target)) + throw new Error(`image ${id} carries no factory labels: it was not built by the factory`) + const label = (name: string) => (Object.hasOwn(labels, name) ? labels[name] : undefined) + const [, , , key] = FACTORY_IMAGE.exec(handoff.target.tag) ?? [] + const problems: string[] = [] + const target = label(FACTORY_LABELS.target) + if (target !== handoff.targetId) + problems.push(`target ${quoted(target, 80)}, not ${handoff.targetId}`) + const pin = label(FACTORY_LABELS.pin) + if (pin !== handoff.target.pin) problems.push(`pin ${quoted(pin, 80)}, not ${handoff.target.pin}`) + const built = label(FACTORY_LABELS.key) + if (key === undefined || built === undefined || !RECIPE_KEY.test(built) || !built.startsWith(key)) + problems.push(`recipe key ${quoted(built, 80)}, not ${key ?? "(no key in the tag)"}…`) + if (problems.length > 0) throw new Error(`image ${id} was built for ${problems.join("; ")}`) +} + +/** + * The builder's whole per-thread sandbox, from the thread's handoff: what `b4.config.ts`'s + * `sandbox.thread` returns. The handoff is parsed and the staged workspace verified first (no + * daemon needed to refuse either), then the image's build labels are checked against the + * handoff, all before the framework resolves the image or starts anything. `inspect` is a test + * seam; the config passes none, so the daemon is asked. + */ +export async function builderThreadSandbox( + thread: { + readonly metadata: Readonly> + readonly staged?: CapturedWorkspaceDefinition | undefined + readonly signal?: AbortSignal | undefined + }, + options: { readonly inspect?: InspectLabels } = {}, +) { + const handoff = builderHandoffOf(thread.metadata) + const workspace = stagedBuilderWorkspace(thread.staged, handoff) + await assertFactoryImage(handoff, options.inspect, thread.signal) + return { + workspace, + environment: { image: handoff.target.image }, + policy: handoff.target.policy, + permissions: { allow: handoff.target.permissions }, + } +} diff --git a/examples/software-factory/server/test/builder-config.test.ts b/examples/software-factory/server/test/builder-config.test.ts index 1ffe4e4d9..dff2a323b 100644 --- a/examples/software-factory/server/test/builder-config.test.ts +++ b/examples/software-factory/server/test/builder-config.test.ts @@ -1,11 +1,12 @@ import { spawnSync } from "node:child_process" +import { createHash } from "node:crypto" import { existsSync, readdirSync, readFileSync } from "node:fs" import { join } from "node:path" import { fileURLToPath } from "node:url" import type { CapturedWorkspaceDefinition } from "@b4run/workspace" import { createSourceBundle } from "@b4run/workspace/node" import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" -import type { BuilderHandoff } from "../src/builder-handoff.ts" +import type { BuilderHandoff, InspectLabels } from "../src/builder-handoff.ts" /** * The builder's config reads nothing per work order from disk: its thread resolver is a @@ -22,6 +23,10 @@ const bundle = (text: string) => { path: "src/cli.ts", bytes: Buffer.from(text), executable: false }, ]) +/** A stand-in image id, distinct per target and pin. */ +const imageIdOf = (targetId: string, pin: string) => + `sha256:${createHash("sha256").update(`${targetId}@${pin}`).digest("hex")}` + interface WorkOrder { readonly handoff: BuilderHandoff readonly staged: CapturedWorkspaceDefinition @@ -42,7 +47,7 @@ const workOrder = ( return { staged, handoff: { - version: 3, + version: 4, workOrderId, taskId: "fixture-task", targetId, @@ -52,8 +57,10 @@ const workOrder = ( baseline: "git", }, target: { - // The factory's tag shape, naming this handoff's own target and pin. - image: `b4-factory-${targetId}:${pin.slice(0, 12)}-0123456789ab`, + // An image id, one per target and pin, and the factory's tag shape naming this + // handoff's own target and pin beside it. + image: imageIdOf(targetId, pin), + tag: `b4-factory-${targetId}:${pin.slice(0, 12)}-0123456789ab`, pin, policy: { network: { mode: "deny" }, @@ -95,6 +102,24 @@ const metadataOf = (order: WorkOrder, handoff: unknown = order.handoff) => ({ factoryWorkOrderId: order.handoff.workOrderId, factoryBuilder: handoff, }) +/** The labels a factory build stamps, for `handoff`: what a fake daemon answers for its id. */ +const labelsOf = (handoff: BuilderHandoff): Record => ({ + "b4.factory.target": handoff.targetId, + "b4.factory.pin": handoff.target.pin, + "b4.factory.key": `${handoff.target.tag.slice(-12)}${"0".repeat(52)}`, +}) +/** A fake daemon: each id's labels, or null (not held) for an id it was not given. */ +const inspectFor = + (answers: Record | null>): InspectLabels => + async (id) => + Object.hasOwn(answers, id) ? (answers[id] ?? null) : null +type ResolverThread = ReturnType +/** + * The builder's thread resolver against an honest daemon: one holding, under each thread's + * handoff's image id, the labels the factory stamped for that handoff. The config's own + * resolver is `builderThreadSandbox` with the real daemon (asserted by text below), which a + * unit test does not reach. + */ const resolver = async () => { const sandbox = (await loadConfig()).sandbox // A thread resolver, not a workspace resolver: the bytes, the image, the policy and the @@ -102,7 +127,12 @@ const resolver = async () => { if (typeof sandbox?.thread !== "function") throw new Error("builder config must resolve each thread's whole sandbox") expect(sandbox.workspace).toBeUndefined() - return sandbox.thread + const { builderThreadSandbox } = await import("../src/builder-handoff.ts") + return (t: ResolverThread) => { + const raw = (t.metadata as { factoryBuilder?: BuilderHandoff }).factoryBuilder + const honest = raw?.target?.image !== undefined ? { [raw.target.image]: labelsOf(raw) } : {} + return builderThreadSandbox(t, { inspect: inspectFor(honest) }) + } } const digestOf = (resolved: unknown) => (resolved as { workspace: { source: { digest: string } } }).workspace.source.digest @@ -137,7 +167,7 @@ describe("builder configuration", () => { const text = readFileSync(new URL("../b4.config.ts", import.meta.url), "utf8") // Scope and allowed images, and no default image. expect(text).toContain( - 'dockerSandbox({ scope: "software-factory-builder", images: isFactoryImage })', + 'dockerSandbox({ scope: "software-factory-builder", images: isFactoryImageId })', ) }) }) @@ -175,41 +205,42 @@ describe("the builder's thread resolver", () => { expect((first.workspace as CapturedWorkspaceDefinition).baseline).toBe("git") // One builder, two targets: each thread runs its own handoff's image under its own // policy and allow-list, which the framework records at the thread's first admission. + // By id: never a tag, which could have moved since the controller bound the image. expect(first.environment).toEqual({ image: alpha.handoff.target.image }) - expect(second.environment).toEqual({ - image: `b4-factory-other-target:${"e".repeat(12)}-0123456789ab`, - }) + expect(first.environment?.image).toMatch(/^sha256:[0-9a-f]{64}$/) + expect(second.environment).toEqual({ image: imageIdOf("other-target", "e".repeat(40)) }) expect(first.policy).toEqual(alpha.handoff.target.policy) expect(second.policy?.resources).toEqual({ memoryMb: 8192, cpus: 4, timeoutMs: 600_000 }) expect(first.permissions).toEqual({ allow: alpha.handoff.target.permissions }) expect(second.permissions).toEqual({ allow: { bash: ["make"] } }) }) - it("refuses a handoff whose image names another target or another pin", async () => { + it("refuses a handoff whose tag names another target or another pin", async () => { const order = workOrder("wo-alpha", "x\n") const resolve = await resolver() - // Both are factory-shaped tags the provider's `images` predicate would admit; only the + // Both are factory-shaped tags naming a real target and pin; only the // handoff's own target and pin make them wrong, and the thread is refused before any // image is resolved. - for (const image of [ + for (const tag of [ "b4-factory-devkit:dddddddddddd-0123456789ab", "b4-factory-fixture-target:eeeeeeeeeeee-0123456789ab", ]) await expect( resolve( thread( - metadataOf(order, { ...order.handoff, target: { ...order.handoff.target, image } }), + metadataOf(order, { ...order.handoff, target: { ...order.handoff.target, tag } }), order.staged, ), ), ).rejects.toThrow(/is not target fixture-target at pin d{40}/) }) - it("allows only the factory's own images", async () => { - const { isFactoryImage } = await import("../src/builder-handoff.ts") - expect(isFactoryImage("b4-factory-devkit:6a59e00aed46-0123456789ab")).toBe(true) - expect(isFactoryImage("alpine:latest")).toBe(false) - expect(isFactoryImage("b4-factory-devkit:latest")).toBe(false) + it("allows an image only by id, never by a tag, the factory's included", async () => { + const { isFactoryImageId } = await import("../src/builder-handoff.ts") + expect(isFactoryImageId(`sha256:${"0".repeat(64)}`)).toBe(true) + expect(isFactoryImageId("b4-factory-devkit:6a59e00aed46-0123456789ab")).toBe(false) + expect(isFactoryImageId("alpine:latest")).toBe(false) + expect(isFactoryImageId(`sha256:${"0".repeat(63)}`)).toBe(false) }) it("refuses, by name, a thread created with no handoff or with no staged workspace", async () => { @@ -294,6 +325,201 @@ describe("the builder's thread resolver", () => { }) }) +describe("the builder's image label check", () => { + it("refuses an image whose build labels are not the handoff's own target, pin and recipe", async () => { + const { builderThreadSandbox } = await import("../src/builder-handoff.ts") + const order = workOrder("wo-alpha", "x\n") + const good = labelsOf(order.handoff) + const id = order.handoff.target.image + const cases: [string, Record | null, RegExp][] = [ + ["absent", null, /is not on this daemon/], + ["unlabelled (a base image)", {}, /carries no factory labels/], + [ + "another target", + { ...good, "b4.factory.target": "devkit" }, + /target "devkit", not fixture-target/, + ], + ["another pin", { ...good, "b4.factory.pin": "e".repeat(40) }, /pin "e{40}", not d{40}/], + [ + "another recipe", + { ...good, "b4.factory.key": "f".repeat(64) }, + /recipe key "f{64}", not 0123456789ab…/, + ], + [ + "a key label that is only the tag's prefix", + { ...good, "b4.factory.key": "0123456789ab" }, + /recipe key "0123456789ab", not 0123456789ab…/, + ], + [ + "a target label missing, the others present", + { + "b4.factory.pin": good["b4.factory.pin"] ?? "", + "b4.factory.key": good["b4.factory.key"] ?? "", + }, + /carries no factory labels/, + ], + [ + "a pin label missing", + { "b4.factory.target": "fixture-target", "b4.factory.key": good["b4.factory.key"] ?? "" }, + /pin \(none\), not d{40}/, + ], + ] + for (const [name, labels, message] of cases) + await expect( + builderThreadSandbox(thread(metadataOf(order), order.staged), { + inspect: inspectFor({ [id]: labels }), + }), + name, + ).rejects.toThrow(message) + await expect( + builderThreadSandbox(thread(metadataOf(order), order.staged), { + inspect: inspectFor({ [id]: good }), + }), + ).resolves.toMatchObject({ environment: { image: id } }) + }) + + it("fails closed when the daemon cannot be asked", async () => { + const { builderThreadSandbox } = await import("../src/builder-handoff.ts") + const order = workOrder("wo-alpha", "x\n") + await expect( + builderThreadSandbox(thread(metadataOf(order), order.staged), { + inspect: async () => { + throw new Error("Cannot connect to the Docker daemon") + }, + }), + ).rejects.toThrow(/Cannot connect to the Docker daemon/) + }) + + it("asks the daemon about the handoff's image id, and only after the handoff parses", async () => { + const { builderThreadSandbox } = await import("../src/builder-handoff.ts") + const order = workOrder("wo-alpha", "x\n") + const asked: string[] = [] + const inspect: InspectLabels = async (id) => { + asked.push(id) + return labelsOf(order.handoff) + } + await builderThreadSandbox(thread(metadataOf(order), order.staged), { inspect }) + expect(asked).toEqual([order.handoff.target.image]) + await expect( + builderThreadSandbox(thread({ factoryWorkOrderId: "wo-alpha" }, order.staged), { inspect }), + ).rejects.toThrow(/factoryBuilder is required/) + expect(asked).toHaveLength(1) + }) + + it("caps what the image and the daemon say when quoting them into a refusal", async () => { + const { builderThreadSandbox } = await import("../src/builder-handoff.ts") + const order = workOrder("wo-alpha", "x\n") + const long = "x".repeat(10_000) + const refusal = await builderThreadSandbox(thread(metadataOf(order), order.staged), { + inspect: inspectFor({ + [order.handoff.target.image]: { ...labelsOf(order.handoff), "b4.factory.target": long }, + }), + }).catch((error: Error) => error.message) + expect(refusal).toMatch(/target "x{79}…, not fixture-target/) + expect(String(refusal).length).toBeLessThan(400) + }) +}) + +describe("the config's thread resolver, against the docker CLI", () => { + type Callback = (error: Error | null, stdout: string, stderr: string) => void + /** A `docker` that answers every call with `answer`, recording what it was asked. */ + const fakeDocker = (answer: { stdout?: string; stderr?: string; exit?: number }) => { + const calls: { file: string; args: readonly string[]; options: Record }[] = [] + vi.doMock("node:child_process", async (importOriginal) => ({ + ...(await importOriginal()), + execFile: ( + file: string, + args: readonly string[], + options: Record, + callback: Callback, + ) => { + calls.push({ file, args, options }) + const error = + answer.exit !== undefined && answer.exit !== 0 + ? Object.assign(new Error(`Command failed: docker ${args.join(" ")}`), { + code: answer.exit, + }) + : null + queueMicrotask(() => callback(error, answer.stdout ?? "", answer.stderr ?? "")) + }, + })) + return calls + } + afterEach(() => { + vi.doUnmock("node:child_process") + }) + + const resolveWith = async (answer: Parameters[0]) => { + const calls = fakeDocker(answer) + const sandbox = (await loadConfig()).sandbox + if (typeof sandbox?.thread !== "function") throw new Error("no thread resolver") + const order = workOrder("wo-alpha", "x\n") + const admission = thread(metadataOf(order), order.staged) + const outcome = await Promise.resolve(sandbox.thread(admission)).then( + (resolved) => ({ resolved }), + (error: Error) => ({ error }), + ) + return { calls, order, admission, outcome } + } + + it("asks docker for the handoff's image labels, by id, under the thread's signal", async () => { + const order = workOrder("wo-alpha", "x\n") + const { calls, admission, outcome } = await resolveWith({ + stdout: `${JSON.stringify(labelsOf(order.handoff))}\n`, + }) + expect(outcome).toMatchObject({ + resolved: { environment: { image: order.handoff.target.image } }, + }) + expect(calls).toHaveLength(1) + expect(calls[0]?.file).toBe("docker") + expect(calls[0]?.args).toEqual([ + "image", + "inspect", + "--format", + "{{json .Config.Labels}}", + "--", + order.handoff.target.image, + ]) + expect(calls[0]?.options).toMatchObject({ timeout: 30_000, signal: admission.signal }) + }) + + it("refuses every answer that is not this handoff's own labels", async () => { + const order = workOrder("wo-alpha", "x\n") + const cases: [string, Parameters[0], RegExp][] = [ + [ + "another target's labels", + { stdout: JSON.stringify({ ...labelsOf(order.handoff), "b4.factory.target": "devkit" }) }, + /target "devkit", not fixture-target/, + ], + ["no labels (null)", { stdout: "null\n" }, /carries no factory labels/], + ["an array", { stdout: "[]\n" }, /answered unreadable labels/], + ["a non-string value", { stdout: '{"b4.factory.target":1}\n' }, /answered unreadable labels/], + ["not JSON", { stdout: "\n" }, /answered unreadable labels/], + [ + "no such image", + { + exit: 1, + stderr: `Error response from daemon: No such image: ${order.handoff.target.image}\n`, + }, + /is not on this daemon/, + ], + [ + "another daemon error", + { exit: 1, stderr: `Cannot connect to the Docker daemon ${"z".repeat(5_000)}` }, + /docker image inspect sha256:[0-9a-f]{64} failed: "Cannot connect to the Docker daemon z+…$/, + ], + ] + for (const [name, answer, message] of cases) { + vi.resetModules() + const { outcome } = await resolveWith(answer) + expect("error" in outcome, name).toBe(true) + const error = (outcome as { error: Error }).error + expect(error.message, name).toMatch(message) + expect(error.message.length, name).toBeLessThan(700) + } + }) +}) + describe("the builder route", () => { it("is a bounded agent with no approval gate and no custom tool", async () => { const builder = (await import("../src/app/build/index.ts")).default