diff --git a/cli/shared/deployment/deploy-project.test.ts b/cli/shared/deployment/deploy-project.test.ts index b6386b3e03..a4e486e0cf 100644 --- a/cli/shared/deployment/deploy-project.test.ts +++ b/cli/shared/deployment/deploy-project.test.ts @@ -5,6 +5,7 @@ import { assertMatch, assertRejects, assertStrictEquals, + assertStringIncludes, } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { FakeTime } from "#std/testing/time"; @@ -1462,6 +1463,82 @@ describe("release asset manifest", () => { assertEquals(reads, 3); }); + it("names the refused dispatch when no manifest row is ever created", async () => { + // A manifest row appears the moment the project runtime begins the build. + // If none ever appears, the control plane's signed `task:release-asset-build` + // dispatch never reached the builder, which is a different failure from a + // build that is merely slow. Saying only `last state: missing` sends the + // operator to inspect a build that never started. + using time = new FakeTime(); + let reads = 0; + const controlPlane = helperControlPlane({ + getReleaseAssetManifest: () => { + reads++; + return Promise.resolve(null); + }, + }); + + const rejection = assertRejects( + () => + waitForReleaseAssetManifest(controlPlane, PROJECT_SLUG, "release-1", { + ...polling, + timeoutMs: 250, + }), + Error, + "never reached the builder", + ); + await time.tickAsync(0); + await time.tickAsync(100); + await time.tickAsync(100); + await time.tickAsync(50); + + const error = await rejection as Error; + assertStringIncludes(error.message, "last state: missing"); + assertStringIncludes(error.message, "/api/control-plane/runs/"); + assertStringIncludes(error.message, "middleware.ts"); + assertEquals(reads, 3); + }); + + it("does not name the refused dispatch once any manifest read has failed", async () => { + // A read that failed leaves the build state unknown for that window: the + // manifest may well have existed and simply not been readable. Later reads + // returning no row cannot restore the stronger claim, so the evidence that + // rules it out has to be monotonic rather than only the last failure. + using time = new FakeTime(); + let reads = 0; + const controlPlane = helperControlPlane({ + getReleaseAssetManifest: () => { + reads++; + if (reads === 1) { + return Promise.reject(Object.assign(new Error("service unavailable"), { status: 503 })); + } + return Promise.resolve(null); + }, + }); + + const rejection = assertRejects( + () => + waitForReleaseAssetManifest(controlPlane, PROJECT_SLUG, "release-1", { + ...polling, + timeoutMs: 250, + }), + Error, + "Check the release asset build and run deploy again.", + ); + await time.tickAsync(0); + await time.tickAsync(100); + await time.tickAsync(100); + await time.tickAsync(50); + + const error = await rejection as Error; + assertStringIncludes(error.message, "last state: missing"); + assertEquals(error.message.includes("never reached the builder"), false); + // The last read succeeded, so the last-failure text is correctly absent + // even though the run as a whole saw a failure. + assertEquals(error.message.includes("last control-plane failure"), false); + assertEquals(reads, 3); + }); + it("reports missing when a manifest disappears after building", async () => { using time = new FakeTime(); const responses = [{ state: "building" }, null]; diff --git a/cli/shared/deployment/deploy-project.ts b/cli/shared/deployment/deploy-project.ts index 3604e8cc87..b9bcfe2a83 100644 --- a/cli/shared/deployment/deploy-project.ts +++ b/cli/shared/deployment/deploy-project.ts @@ -618,16 +618,46 @@ function assertReadyManifestCoversPageRoutes( /** Upper bound for a plausible manifest state value from the control plane. */ const MAX_MANIFEST_STATE_LENGTH = 64; -function releaseAssetPollingTimeoutError( - timeoutMs: number, - lastState: string, - lastTransientFailure: string | null, -): Error { - const timeoutSeconds = Math.ceil(timeoutMs / 1000); +/** + * The manifest row appears the moment the project runtime begins the build, so + * never seeing one is a different failure from a build that is merely slow: the + * control plane's signed `task:release-asset-build` dispatch never reached the + * builder. Something in front of the runtime's control-plane handler answered + * it, and the operator needs to be told that rather than sent to inspect a + * build that never started. + */ +const RELEASE_ASSET_BUILD_NEVER_STARTED_HINT = + "No manifest was ever created for this release, so the release asset build " + + 'dispatch (POST /api/control-plane/runs/{runId}/execute, target "task:release-asset-build") ' + + "never reached the builder on the deployed runtime. Check the runtime logs for this " + + "release, and any request gate in front of it such as the project's middleware.ts or a " + + "security policy in veryfront.config."; + +function releaseAssetPollingTimeoutError(options: { + timeoutMs: number; + lastState: string; + /** The most recent retryable read failure, or `null` if the last read succeeded. */ + lastTransientFailure: string | null; + /** Whether any read ever returned a manifest row. */ + observedManifest: boolean; + /** Whether any read ever failed retryably, including reads a later success followed. */ + observedTransientFailure: boolean; +}): Error { + const { lastState, lastTransientFailure, observedManifest, observedTransientFailure } = options; + const timeoutSeconds = Math.ceil(options.timeoutMs / 1000); + // Only claim the dispatch never landed when nothing else can explain the + // silence: a manifest read that failed leaves the build state unknown for + // that window, so it rules out the stronger claim even if later reads + // succeeded and returned no row. + const neverStarted = !observedManifest && !observedTransientFailure; return new Error( `Release assets were not ready within ${timeoutSeconds}s (last state: ${lastState}${ lastTransientFailure === null ? "" : `; last control-plane failure: ${lastTransientFailure}` - }). Check the release asset build and run deploy again.`, + }). ${ + neverStarted + ? RELEASE_ASSET_BUILD_NEVER_STARTED_HINT + : "Check the release asset build and run deploy again." + }`, ); } @@ -675,14 +705,18 @@ export async function waitForReleaseAssetManifest( const deadline = Date.now() + timeoutMs; let lastState = "missing"; let lastTransientFailure: string | null = null; + let observedManifest = false; + let observedTransientFailure = false; for (;;) { const remainingMs = deadline - Date.now(); - const timeoutError = releaseAssetPollingTimeoutError( + const timeoutError = releaseAssetPollingTimeoutError({ timeoutMs, lastState, lastTransientFailure, - ); + observedManifest, + observedTransientFailure, + }); if (remainingMs <= 0) throw timeoutError; let raw: Awaited> = null; @@ -702,8 +736,10 @@ export async function waitForReleaseAssetManifest( lastTransientFailure = status === undefined ? "a transient connection failure" : `HTTP ${status}`; + observedTransientFailure = true; } if (raw !== null) { + observedManifest = true; const state = readUntrustedOwnDataProperty(raw, "state"); if (!isSafeBoundedText(state, MAX_MANIFEST_STATE_LENGTH)) { throw new Error(`Release assets for ${releaseId} returned an invalid state response`); diff --git a/docs/guides/middleware.md b/docs/guides/middleware.md index 62240ccee8..1e8f1dcab8 100644 --- a/docs/guides/middleware.md +++ b/docs/guides/middleware.md @@ -206,6 +206,14 @@ Root middleware has the same ordering and short-circuit contract in local develo Production middleware is cached by project, environment, and immutable release or preview branch. Preview cache invalidation reloads the file after source changes, and the cache has a fixed entry limit. A missing file passes through normally. +Root middleware runs in front of your project's routes, not in front of the platform's. A control-plane dispatch is the signed request the platform sends to your runtime to build a release asset manifest for a deploy, or to start, resume, or cancel a run. It bypasses root middleware and goes straight to the handler that verifies its signature. + +Middleware could not authorize one of these requests in any case. Infrastructure headers, including the dispatch signature, are withheld from project code, so middleware that gates on a credential rejects the platform's request to build your own deploy. + +The signature-keyed bypass is narrow. It applies only to a request that both addresses one of those platform routes and carries the signature header the receiving handler verifies. An unsigned request to `POST /api/control-plane/runs/{runId}/execute`, an unsigned request to the agents list route, and any other path under `/api/control-plane/`, including your own routes in that namespace, still run your middleware. + +Three run-lifecycle routes are a longstanding exception and bypass middleware whether or not they are signed: `POST /api/control-plane/runs/{runId}/stream`, `POST /api/control-plane/runs/{runId}/resume`, and `DELETE /api/control-plane/runs/{runId}`. Do not rely on middleware to gate those three paths. + Production loading is fail-closed. If a declared middleware file cannot be read, compiled, or validated as a middleware export, a dedicated server does not start and a shared server returns an error only for the affected project request. Failed shared loads are not cached, so a corrected deployment can recover without restarting unrelated projects. Development loading remains nonfatal and reports the loading error in the server log. ## Verify it worked diff --git a/src/release-assets/build-dispatch-security.test.ts b/src/release-assets/build-dispatch-security.test.ts index a968fd182e..19ee5f8326 100644 --- a/src/release-assets/build-dispatch-security.test.ts +++ b/src/release-assets/build-dispatch-security.test.ts @@ -1,6 +1,7 @@ /** - * Regression: a project that configures `security.csrf` must still be able to - * publish a release asset manifest. + * Regression: a project's own request gates must not block its release asset + * manifest build. Three of them have: `security.auth`, `security.csrf`, and a + * root `middleware.ts`. * * The release asset manifest is built by the project runtime, not by the CLI: * the control plane POSTs a signed operation envelope to @@ -18,6 +19,23 @@ * Nothing downstream can see it: the run fails before the manifest row is * created, and the deploy timeout names neither CSRF nor config. * + * `security.auth` is the same failure with a different gate. `AuthHandler` runs + * at priority 0 — ahead of `CsrfHandler` — and also matches every path, and the + * credential it demands is one the platform structurally cannot hold: the + * control plane sends a per-run service `Bearer` JWT the Basic branch can never + * match and the Bearer branch compares against a project-authored secret. + * + * `projectMiddlewareRuntime.execute` wraps the entire handler chain, so a root + * `middleware.ts` sits in front of the same dispatch and fails it the same way. + * It has no way to pass: `createApplicationRequest` withholds every + * `x-veryfront-*` header from project code, so the signature the platform + * authenticates with is not visible to the middleware that would have to trust + * it. + * + * All three gates are assembled here in the order the runtime assembles them — + * project middleware outermost, then the security handlers, then the run + * executor — so that a fix for one gate is exercised with the others standing. + * * @module release-assets/build-dispatch-security.test */ @@ -25,9 +43,11 @@ import "#veryfront/schemas/_test-setup.ts"; import { assertEquals } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { RouteRegistry } from "#veryfront/routing/registry/index.ts"; +import { AuthHandler } from "#veryfront/security/http/auth.ts"; import { CsrfHandler } from "#veryfront/security/http/csrf/csrf-handler.ts"; import { deriveSecurityContext } from "#veryfront/security/http/config.ts"; import type { VeryfrontConfig } from "#veryfront/config"; +import type { SecurityConfig } from "#veryfront/types"; import { ProjectRunExecuteHandler, type ProjectRunExecuteHandlerDeps, @@ -36,6 +56,8 @@ import { createControlPlaneSignature, createCtx, } from "#veryfront/server/handlers/request/internal-agent-run.test-helpers.ts"; +import type { MiddlewareFunction } from "#veryfront/server/dev-server/middleware.ts"; +import { ProjectMiddlewareRuntime } from "#veryfront/server/runtime-handler/project-middleware.ts"; const RUN_ID = "run_release_assets_1"; const EXECUTE_PATH = `/api/control-plane/runs/${RUN_ID}/execute`; @@ -52,15 +74,37 @@ interface DispatchOutcome { } /** - * Drive the real handler chain the runtime uses for a control-plane run - * dispatch: the security handlers first, then the run executor. + * Everything about a dispatch other than the CSRF setting. + * + * One options bag, not one positional parameter per gate. Every fix that + * exempts this dispatch from another gate widens this harness, and positional + * parameters grown on separate branches merge into a signature that still + * compiles while existing calls bind their argument to the wrong slot: a + * `{ projectMiddleware }` argument lands in an `auth` parameter, no middleware + * is installed, and the test that proves the middleware bypass works keeps + * passing without ever exercising it. A named field cannot merge that way. + */ +interface DispatchOptions { + /** A `security.auth` policy the project put in front of its own site. */ + readonly auth?: SecurityConfig["auth"]; + /** A root `middleware.ts`, which wraps the whole handler chain. */ + readonly projectMiddleware?: MiddlewareFunction[]; + /** Send the request without the control-plane signature header. */ + readonly unsigned?: boolean; +} + +/** + * Drive the real chain the runtime uses for a control-plane run dispatch: the + * project's root middleware, then the security handlers, then the run executor. */ async function dispatchReleaseAssetBuild( csrf: CsrfSetting | undefined, + options: DispatchOptions = {}, ): Promise { - const config = { - security: csrf === undefined ? {} : { csrf }, - } as VeryfrontConfig; + const security: Record = {}; + if (csrf !== undefined) security.csrf = csrf; + if (options.auth !== undefined) security.auth = options.auth; + const config = { security } as VeryfrontConfig; const { securityConfig } = deriveSecurityContext(config, { // The task rail dispatches to the project's main-branch runtime, which // resolves as a preview environment. Production defaults are therefore off @@ -84,7 +128,7 @@ async function dispatchReleaseAssetBuild( }); const request = new Request(`https://example-project.example.test${EXECUTE_PATH}`, { method: "POST", - headers: { + headers: options.unsigned ? { "content-type": "application/json" } : { "content-type": "application/json", "x-veryfront-control-plane-jws": jws, }, @@ -112,13 +156,28 @@ async function dispatchReleaseAssetBuild( ctx.securityConfig = securityConfig; const registry = new RouteRegistry(); - registry.registerAll([new CsrfHandler(), new ProjectRunExecuteHandler(deps)]); + registry.registerAll([ + new AuthHandler(), + new CsrfHandler(), + new ProjectRunExecuteHandler(deps), + ]); - const response = await registry.execute(request, ctx); + // The runtime wraps the whole handler chain in the project's own root + // middleware, so a dispatch meets `middleware.ts` before it meets any + // handler. + const middlewareRuntime = new ProjectMiddlewareRuntime({ + loadMiddleware: () => Promise.resolve(options.projectMiddleware ?? []), + }); + const response = await middlewareRuntime.execute({ + request, + handlerContext: ctx, + isSharedProxy: false, + next: async () => (await registry.execute(request, ctx)) ?? undefined, + }); return { begun, status: response?.status ?? 0, - body: response ? await response.text() : "", + body: response === undefined ? "" : await response.text(), }; } @@ -145,6 +204,55 @@ describe("release assets: control-plane build dispatch", () => { assertEquals(outcome.status, 200); }); + it("builds a manifest when the project's own middleware gates every request", async () => { + // A root `middleware.ts` that authorizes traffic is ordinary project code, + // and `projectMiddlewareRuntime.execute` wraps the entire handler chain, so + // it stands in front of the build dispatch as well. It cannot authorize + // that dispatch even in principle: `createApplicationRequest` strips every + // `x-veryfront-*` header before project code sees the request, so the + // signature the platform authenticates with is invisible to it. + let middlewareCalls = 0; + const outcome = await dispatchReleaseAssetBuild(undefined, { + projectMiddleware: [ + (c, next) => { + middlewareCalls++; + if (!c.req.headers.get("authorization")) { + return Promise.resolve(new Response("Unauthorized", { status: 401 })); + } + return next(); + }, + ], + }); + + assertEquals( + outcome.begun, + true, + `release asset build never started; runtime answered ${outcome.status}: ${outcome.body}`, + ); + assertEquals(outcome.status, 200); + assertEquals(middlewareCalls, 0); + }); + + it("keeps project middleware in front of an unsigned request to the same path", async () => { + // The bypass is keyed on a dispatch, not on a path. Without the signature + // header the request is project traffic, and the project's middleware + // answers it exactly as before. + let middlewareCalls = 0; + const outcome = await dispatchReleaseAssetBuild(undefined, { + unsigned: true, + projectMiddleware: [ + () => { + middlewareCalls++; + return new Response("Unauthorized", { status: 401 }); + }, + ], + }); + + assertEquals(outcome.status, 401); + assertEquals(outcome.begun, false); + assertEquals(middlewareCalls, 1); + }); + it("builds a manifest when the project excludes a path from csrf", async () => { // The shape that first surfaced this: keep CSRF enforced everywhere except // the agent endpoint the chat client posts to. diff --git a/src/server/runtime-handler/project-middleware.test.ts b/src/server/runtime-handler/project-middleware.test.ts index 645e680191..fd216ad7dd 100644 --- a/src/server/runtime-handler/project-middleware.test.ts +++ b/src/server/runtime-handler/project-middleware.test.ts @@ -694,7 +694,116 @@ describe("ProjectMiddlewareRuntime", () => { assertEquals(routeCalls, 1); }); + it("bypasses project middleware for signed control-plane dispatches", async () => { + // The control plane builds a release asset manifest by POSTing a signed + // operation envelope to the project's own runtime. That dispatch is not the + // project's traffic: it addresses a platform handler, and + // `createApplicationRequest` strips every `x-veryfront-*` header before + // project code sees the request, so middleware cannot even observe the + // credential it would have to trust. A project whose middleware gates + // requests would answer its own deploy with a rejection. + const adapter = createAdapter(); + let loads = 0; + const runtime = new ProjectMiddlewareRuntime({ + loadMiddleware: () => { + loads++; + return Promise.resolve([ + () => new Response("project-middleware", { status: 403 }), + ]); + }, + }); + let routeCalls = 0; + const next = () => { + routeCalls++; + return Promise.resolve(new Response("route")); + }; + const dispatches: Array<[string, string]> = [ + ["POST", "/api/control-plane/runs/run_1/execute"], + ["POST", "/api/control-plane/runs/run_1/stream"], + ["POST", "/api/control-plane/runs/run_1/resume"], + ["POST", "/api/control-plane/agents/list"], + ["DELETE", "/api/control-plane/runs/run_1"], + ]; + + for (const [method, path] of dispatches) { + const response = await execute( + runtime, + createContext(adapter), + new Request(`https://example.com${path}`, { + method, + headers: { "x-veryfront-control-plane-jws": "header.payload.signature" }, + }), + next, + ); + assertEquals( + response?.status, + 200, + `${method} ${path} was answered by project middleware`, + ); + assertEquals(await response?.text(), "route"); + } + + assertEquals(loads, 0); + assertEquals(routeCalls, dispatches.length); + }); + + it("keeps project middleware in front of look-alike control-plane paths", async () => { + // The `/api/control-plane/` namespace is reserved but not exclusively + // routed: paths the platform does not own fall through to project code, so + // a signature header alone must never lift project middleware off a route + // the project itself serves. + const adapter = createAdapter(); + let loads = 0; + const runtime = new ProjectMiddlewareRuntime({ + loadMiddleware: () => { + loads++; + return Promise.resolve([ + () => new Response("project-middleware", { status: 403 }), + ]); + }, + }); + let routeCalls = 0; + const next = () => { + routeCalls++; + return Promise.resolve(new Response("route")); + }; + const impostors: Array<[string, string]> = [ + // A project API route that merely sits inside the reserved namespace. + ["POST", "/api/control-plane/checkout"], + // A path that only starts alike. + ["POST", "/api/control-plane-mirror/runs/run_1/execute"], + // A registered surface addressed with a method no handler owns. + ["PUT", "/api/control-plane/runs/run_1/execute"], + // A deeper path under a registered surface. + ["POST", "/api/control-plane/runs/run_1/execute/../../../checkout"], + ]; + + for (const [method, path] of impostors) { + const response = await execute( + runtime, + createContext(adapter), + new Request(`https://example.com${path}`, { + method, + headers: { "x-veryfront-control-plane-jws": "header.payload.signature" }, + }), + next, + ); + assertEquals( + response?.status, + 403, + `${method} ${path} skipped project middleware`, + ); + assertEquals(await response?.text(), "project-middleware"); + } + + assertEquals(routeCalls, 0); + assertEquals(loads > 0, true); + }); + it("keeps project middleware enabled for control-plane run execution", async () => { + // Unsigned: a request that only looks like a dispatch carries no envelope + // for a control-plane handler to verify, so it stays project traffic and + // project middleware keeps answering it. const adapter = createAdapter(); let loads = 0; const runtime = new ProjectMiddlewareRuntime({ diff --git a/src/server/runtime-handler/project-middleware.ts b/src/server/runtime-handler/project-middleware.ts index 7b709deff7..944ac438a2 100644 --- a/src/server/runtime-handler/project-middleware.ts +++ b/src/server/runtime-handler/project-middleware.ts @@ -1,7 +1,10 @@ import type { RuntimeAdapter } from "#veryfront/platform/adapters/base.ts"; import { isExtendedFSAdapter } from "#veryfront/platform/adapters/fs/wrapper.ts"; import { registerLRUCache } from "#veryfront/cache"; -import { isConfigOptionalControlPlaneRunRequest } from "#veryfront/channels/control-plane.ts"; +import { + isConfigOptionalControlPlaneRunRequest, + isSignedControlPlaneDispatch, +} from "#veryfront/channels/control-plane.ts"; import { MiddlewareContext } from "#veryfront/middleware/core/context.ts"; import { MiddlewarePipeline } from "#veryfront/middleware/core/pipeline/index.ts"; import { getProjectEnvSnapshot } from "#veryfront/server/project-env"; @@ -104,6 +107,31 @@ export class ProjectMiddlewareRuntime { return next(); } + // A control-plane dispatch is not the project's traffic. It addresses a + // platform handler in the reserved control-plane namespace, carries a + // signed operation envelope rather than a user session, and asks the + // runtime to perform internal work such as building the release asset + // manifest for the project's own deploy. + // + // Project middleware cannot authorize such a request even in principle: + // `createApplicationRequest` withholds every `x-veryfront-*` header from + // project code, so the signature the receiving handler authenticates with + // is invisible to middleware. A root `middleware.ts` that gates requests + // therefore has no choice but to reject its own deploy, which surfaces + // only as `deploy` timing out with `last state: missing`. + // + // The bypass is keyed on the request being a real dispatch, not on it + // being path-shaped like one: `isSignedControlPlaneDispatch` requires both + // a method/path pair a control-plane handler owns and the signature header + // that handler verifies. It concedes nothing to an unauthenticated caller, + // because the only routes it can reach answer 401 without a valid envelope + // and never fall through to project code. Every other request, including + // an unsigned one to the same path and a project route that merely sits + // inside the reserved namespace, still traverses project middleware. + if (isSignedControlPlaneDispatch(request)) { + return next(); + } + if ( isConfigOptionalControlPlaneRunRequest( request.method,