diff --git a/docs/api-reference/veryfront/security.md b/docs/api-reference/veryfront/security.md index de50182800..6d34cc121d 100644 --- a/docs/api-reference/veryfront/security.md +++ b/docs/api-reference/veryfront/security.md @@ -96,7 +96,7 @@ applySecurityHeaders(response.headers, false, generateNonce(), null); | ---------------------- | ----------- | ---------------------------------------------------------------------------------------------------------- | | `AuthHandler` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/auth.ts#L156) | | `BaseHandler` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/base-handler.ts#L45) | -| `CsrfHandler` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/csrf/csrf-handler.ts#L56) | +| `CsrfHandler` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/csrf/csrf-handler.ts#L57) | | `ResponseBuilder` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/response/builder.ts#L9) | | `SecureFs` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/secure-fs.ts#L645) | | `SecurityConfigLoader` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/config.ts#L292) | diff --git a/src/channels/control-plane.ts b/src/channels/control-plane.ts index e760dc50bd..dfd793bae3 100644 --- a/src/channels/control-plane.ts +++ b/src/channels/control-plane.ts @@ -27,6 +27,77 @@ export const CONTROL_PLANE_RUN_STREAM_PATH = "/api/control-plane/runs/:runId/str const CONTROL_PLANE_RUN_ID_PATH_SEGMENT = "[^/]+"; const CONTROL_PLANE_RUNS_REGEX_PREFIX = CONTROL_PLANE_RUNS_PATH_PREFIX.replaceAll("/", "\\/"); +/** Request header the control plane carries its signed operation envelope in. */ +export const CONTROL_PLANE_JWS_HEADER = "x-veryfront-control-plane-jws"; + +const CONTROL_PLANE_RUN_OPERATION_PATH = + /^\/api\/control-plane\/runs\/[^/]+\/(?:execute|stream|resume)$/u; +const CONTROL_PLANE_RUN_PATH = /^\/api\/control-plane\/runs\/[^/]+$/u; + +/** + * True when a method and path pair addresses a registered control-plane handler. + * + * The reserved namespace is wider than the set of routes the runtime actually + * serves. Only these shapes reach a handler that authenticates a signed + * operation envelope through `verifyControlPlaneRequest`: + * + * - `POST /api/control-plane/agents/list` + * - `POST /api/control-plane/runs/{runId}/execute` + * - `POST /api/control-plane/runs/{runId}/stream` + * - `POST /api/control-plane/runs/{runId}/resume` + * - `DELETE /api/control-plane/runs/{runId}` + * + * Any other path under the prefix falls through to project code, so treating + * the prefix as proof of a control-plane request would hand a project's own + * routes whatever exemption the caller grants. + * + * Match this against `URL.pathname`, which resolves dot segments, so a path + * cannot be smuggled past the anchored patterns. + */ +export function isControlPlaneSurfaceRoute( + method: string, + pathname: string | undefined, +): boolean { + const normalizedMethod = method.toUpperCase(); + const requestPath = pathname ?? ""; + + if (normalizedMethod === "POST") { + return requestPath === CONTROL_PLANE_AGENTS_LIST_PATH || + CONTROL_PLANE_RUN_OPERATION_PATH.test(requestPath); + } + if (normalizedMethod === "DELETE") { + return CONTROL_PLANE_RUN_PATH.test(requestPath); + } + return false; +} + +/** + * True for a request that is a control-plane dispatch rather than a browser one. + * + * Both conditions must hold. The method and path must address a registered + * control-plane handler (see {@link isControlPlaneSurfaceRoute}), and the + * request must carry a control-plane signature header. The receiving handler + * verifies that envelope against the dispatch signing key, and the signature + * covers the request method and path, so an envelope minted for one surface + * cannot be replayed against another. + * + * Callers use this to keep gates that assume a browser client, such as CSRF + * double-submit validation, from standing in front of platform dispatch. A + * browser cannot attach the signature header to a cross-origin request without + * a preflight the runtime does not grant, so a forged cross-site request never + * satisfies this predicate. Neither does a project route that merely sits at a + * look-alike path. + * + * This is not authentication. It only reports that authority for the request + * comes from a signature the handler checks, never from ambient credentials. + */ +export function isSignedControlPlaneDispatch(req: Request): boolean { + const signature = req.headers.get(CONTROL_PLANE_JWS_HEADER); + if (signature === null || signature.length === 0) return false; + + return isControlPlaneSurfaceRoute(req.method, new SafeURL(req.url).pathname); +} + /** * True for control-plane run surfaces that can dispatch without project config. * diff --git a/src/proxy/control-plane-signature.ts b/src/proxy/control-plane-signature.ts index 690b744f3e..48ad171f98 100644 --- a/src/proxy/control-plane-signature.ts +++ b/src/proxy/control-plane-signature.ts @@ -28,6 +28,7 @@ import { getHostEnv } from "#veryfront/platform/compat/process.ts"; import { + isControlPlaneSurfaceRoute, verifyControlPlaneJwsRequestSignature, verifyControlPlaneJwsSignature, verifyDispatchJwsSignature, @@ -60,13 +61,14 @@ const MAX_BRANCH_NAME_CODE_UNITS = 255; export type InternalControlPlaneRouteKind = "dispatch" | "control-plane" | "reserved" | "public"; -const CONTROL_PLANE_RUN_OPERATION_PATH = - /^\/api\/control-plane\/runs\/[^/]+\/(?:execute|stream|resume)$/u; -const CONTROL_PLANE_RUN_PATH = /^\/api\/control-plane\/runs\/[^/]+$/u; - /** * Classify the internal namespace against routes whose handlers always perform * authoritative downstream JWS verification. + * + * `control-plane` and `reserved` differ in exactly the way that matters to a + * caller deciding what to trust: `control-plane` names a route the runtime + * serves through a verifying handler, `reserved` names the rest of the + * namespace, which a project can occupy with its own routes. */ export function classifyInternalControlPlaneRequest( method: string, @@ -76,14 +78,7 @@ export function classifyInternalControlPlaneRequest( if (pathname === "/channels/invoke" && normalizedMethod === "POST") { return "dispatch"; } - if ( - normalizedMethod === "POST" && - (pathname === "/api/control-plane/agents/list" || - CONTROL_PLANE_RUN_OPERATION_PATH.test(pathname)) - ) { - return "control-plane"; - } - if (normalizedMethod === "DELETE" && CONTROL_PLANE_RUN_PATH.test(pathname)) { + if (isControlPlaneSurfaceRoute(normalizedMethod, pathname)) { return "control-plane"; } diff --git a/src/release-assets/build-dispatch-security.test.ts b/src/release-assets/build-dispatch-security.test.ts new file mode 100644 index 0000000000..a968fd182e --- /dev/null +++ b/src/release-assets/build-dispatch-security.test.ts @@ -0,0 +1,159 @@ +/** + * Regression: a project that configures `security.csrf` must still be able to + * publish a release asset manifest. + * + * The release asset manifest is built by the project runtime, not by the CLI: + * the control plane POSTs a signed operation envelope to + * `/api/control-plane/runs/{runId}/execute` with `target: + * "task:release-asset-build"`, and only that dispatch calls + * `beginReleaseAssetManifestBuild`. Until it lands, the manifest does not + * exist, and `veryfront deploy` reports + * `Release assets were not ready within 120s (last state: missing)`. + * + * That dispatch is a POST, and it carries a JWS envelope rather than a CSRF + * double-submit token. The control plane is not a browser and has no + * `__Host-vf_csrf` cookie to echo. `CsrfHandler` runs at priority 5, ahead of + * `ProjectRunExecuteHandler`, and matches every path, so a project whose + * config enables CSRF at all turns its own release asset builds into 403s. + * Nothing downstream can see it: the run fails before the manifest row is + * created, and the deploy timeout names neither CSRF nor config. + * + * @module release-assets/build-dispatch-security.test + */ + +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 { CsrfHandler } from "#veryfront/security/http/csrf/csrf-handler.ts"; +import { deriveSecurityContext } from "#veryfront/security/http/config.ts"; +import type { VeryfrontConfig } from "#veryfront/config"; +import { + ProjectRunExecuteHandler, + type ProjectRunExecuteHandlerDeps, +} from "#veryfront/server/handlers/request/project-run-execute.handler.ts"; +import { + createControlPlaneSignature, + createCtx, +} from "#veryfront/server/handlers/request/internal-agent-run.test-helpers.ts"; + +const RUN_ID = "run_release_assets_1"; +const EXECUTE_PATH = `/api/control-plane/runs/${RUN_ID}/execute`; + +type CsrfSetting = VeryfrontConfig["security"] extends infer S + ? S extends { csrf?: infer C } ? C : never + : never; + +interface DispatchOutcome { + /** Whether the release asset build executor was reached at all. */ + readonly begun: boolean; + readonly status: number; + readonly body: string; +} + +/** + * Drive the real handler chain the runtime uses for a control-plane run + * dispatch: the security handlers first, then the run executor. + */ +async function dispatchReleaseAssetBuild( + csrf: CsrfSetting | undefined, +): Promise { + const config = { + security: csrf === undefined ? {} : { csrf }, + } 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 + // and only an explicit `security.csrf` can enable the gate here. + productionDefaults: false, + }); + + const body = { + runId: RUN_ID, + kind: "task", + target: "task:release-asset-build", + projectId: "proj-1", + config: { release_id: "rel-1", release_version: 1 }, + }; + const rawBody = JSON.stringify(body); + const { jws, publicKeyPem } = await createControlPlaneSignature(rawBody, { + requestId: RUN_ID, + projectId: "proj-1", + requestMethod: "POST", + requestPath: EXECUTE_PATH, + }); + const request = new Request(`https://example-project.example.test${EXECUTE_PATH}`, { + method: "POST", + headers: { + "content-type": "application/json", + "x-veryfront-control-plane-jws": jws, + }, + body: rawBody, + }); + + let begun = false; + const deps = { + executeReleaseAssetBuild: () => { + // Stands in for `runReleaseAssetBuild`, whose first act is + // `beginReleaseAssetManifestBuild`. Reaching it is what moves the + // manifest off `missing`. + begun = true; + return Promise.resolve({ + success: true, + result: { state: "ready", moduleCount: 1, cssCount: 1, routeCount: 1 }, + logs: null, + duration_ms: 10, + }); + }, + now: () => 0, + } as unknown as ProjectRunExecuteHandlerDeps; + + const ctx = createCtx(publicKeyPem); + ctx.securityConfig = securityConfig; + + const registry = new RouteRegistry(); + registry.registerAll([new CsrfHandler(), new ProjectRunExecuteHandler(deps)]); + + const response = await registry.execute(request, ctx); + return { + begun, + status: response?.status ?? 0, + body: response ? await response.text() : "", + }; +} + +describe("release assets: control-plane build dispatch", () => { + it("builds a manifest when the project leaves csrf unset", async () => { + const outcome = await dispatchReleaseAssetBuild(undefined); + assertEquals(outcome.status, 200); + assertEquals(outcome.begun, true); + }); + + it("builds a manifest when the project disables csrf", async () => { + const outcome = await dispatchReleaseAssetBuild(false); + assertEquals(outcome.status, 200); + assertEquals(outcome.begun, true); + }); + + it("builds a manifest when the project enables csrf with a boolean", async () => { + const outcome = await dispatchReleaseAssetBuild(true); + assertEquals( + outcome.begun, + true, + `release asset build never started; runtime answered ${outcome.status}: ${outcome.body}`, + ); + assertEquals(outcome.status, 200); + }); + + 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. + const outcome = await dispatchReleaseAssetBuild({ excludePaths: ["/api/ag-ui"] }); + assertEquals( + outcome.begun, + true, + `release asset build never started; runtime answered ${outcome.status}: ${outcome.body}`, + ); + assertEquals(outcome.status, 200); + }); +}); diff --git a/src/security/http/csrf/csrf-handler.test.ts b/src/security/http/csrf/csrf-handler.test.ts index 840d0c22b2..52b3f21fb6 100644 --- a/src/security/http/csrf/csrf-handler.test.ts +++ b/src/security/http/csrf/csrf-handler.test.ts @@ -62,6 +62,118 @@ describe("security/http/csrf/csrf-handler", () => { }); }); + describe("signed control-plane surfaces", () => { + const SIGNED = { "x-veryfront-control-plane-jws": "header.payload.signature" }; + + it("passes every registered surface through for every enabled csrf shape", async () => { + // The control plane holds no `__Host-vf_csrf` cookie and authorizes from + // a signed envelope the receiving handler verifies. Gating it here left a + // project that configured CSRF unable to build its own release assets. + const surfaces = [ + { method: "POST", path: "/api/control-plane/agents/list" }, + { method: "POST", path: "/api/control-plane/runs/run_1/execute" }, + { method: "POST", path: "/api/control-plane/runs/run_1/stream" }, + { method: "POST", path: "/api/control-plane/runs/run_1/resume" }, + { method: "DELETE", path: "/api/control-plane/runs/run_1" }, + ]; + + for (const csrf of [true, { excludePaths: ["/api/ag-ui"] }]) { + for (const surface of surfaces) { + const result = await handler.handle( + new Request(`https://acme.example.test${surface.path}`, { + method: surface.method, + headers: SIGNED, + body: "{}", + }), + createCtx(csrf), + ); + + assertEquals( + result.response, + undefined, + `${surface.method} ${surface.path} was gated`, + ); + } + } + }); + + it("still rejects a token-less POST to a path that merely starts alike", async () => { + const result = await handler.handle( + new Request("https://acme.example.test/api/control-plane-mirror/runs", { + method: "POST", + body: "{}", + }), + createCtx(true), + ); + + assertEquals(result.response?.status, 403); + }); + + it("still enforces CSRF on a project route inside the control-plane namespace", async () => { + // The reserved namespace is not exclusively routed. In a custom runtime + // served with `createHandler`, an App or Pages API route under + // `/api/control-plane/*` that no control-plane handler claims falls + // through to `ApiHandlerWrapper` and runs project code authenticated by + // cookies. Exempting the prefix would let any project disable CSRF on its + // own state-changing routes by choosing a path. + const projectRoutes = [ + { method: "POST", path: "/api/control-plane/checkout" }, + { method: "POST", path: "/api/control-plane/runs" }, + { method: "POST", path: "/api/control-plane/runs/run_1" }, + { method: "POST", path: "/api/control-plane/runs/run_1/execute/extra" }, + { method: "POST", path: "/api/control-plane/agents/list/all" }, + { method: "PUT", path: "/api/control-plane/runs/run_1/execute" }, + { method: "DELETE", path: "/api/control-plane/runs/run_1/execute" }, + ]; + + for (const route of projectRoutes) { + const result = await handler.handle( + new Request(`https://acme.example.test${route.path}`, { + method: route.method, + body: "{}", + }), + createCtx(true), + ); + + assertEquals( + result.response?.status, + 403, + `${route.method} ${route.path} skipped the CSRF gate`, + ); + } + }); + + it("does not let a signature header alone exempt a project route", async () => { + // A same-origin caller can set any header it likes. The signature only + // earns an exemption on a path a verifying handler owns, so an + // unrecognized path stays gated even when the header is present. + const result = await handler.handle( + new Request("https://acme.example.test/api/control-plane/checkout", { + method: "POST", + headers: SIGNED, + body: "{}", + }), + createCtx(true), + ); + + assertEquals(result.response?.status, 403); + }); + + it("still enforces CSRF on a registered surface with no signature header", async () => { + // A cross-site form POST cannot attach the signature header. Without it + // the request is browser shaped and must present a CSRF token. + const result = await handler.handle( + new Request("https://acme.example.test/api/control-plane/runs/run_1/execute", { + method: "POST", + body: "{}", + }), + createCtx(true), + ); + + assertEquals(result.response?.status, 403); + }); + }); + describe("when CSRF is not configured", () => { it("should pass through all requests when securityConfig is null", async () => { const ctx = createCtx(); diff --git a/src/security/http/csrf/csrf-handler.ts b/src/security/http/csrf/csrf-handler.ts index bbff9f42a3..8dfba2134c 100644 --- a/src/security/http/csrf/csrf-handler.ts +++ b/src/security/http/csrf/csrf-handler.ts @@ -42,6 +42,7 @@ */ import { isCspReportRequest } from "#veryfront/security/http/csp-report-endpoint.ts"; +import { isSignedControlPlaneDispatch } from "#veryfront/channels/control-plane.ts"; import { BaseHandler } from "../base-handler.ts"; import { validateCsrf } from "../../csrf/helpers.ts"; import type { @@ -79,6 +80,23 @@ export class CsrfHandler extends BaseHandler { // would make reporting another thing a project has to configure first. if (isCspReportRequest(method, pathname)) return this.continue(); + // A control-plane dispatch is not a browser request. Release asset builds, + // run execute/resume/cancel and agent listing arrive carrying a signed + // operation envelope, verified before the handler acts on them; they hold + // no `__Host-vf_csrf` cookie to echo and derive no authority from one. + // Rejecting them here protects nothing and instead stops the platform from + // building the project's own release asset manifest, which surfaces only as + // `deploy` timing out with `last state: missing`. + // + // The exemption is keyed on the request being a real dispatch, not on it + // being path-shaped like one: `isSignedControlPlaneDispatch` requires both a + // method/path pair that a control-plane handler owns and the signature + // header that handler verifies. The `/api/control-plane/` namespace is + // reserved but not exclusively routed, so a project App or Pages API route + // can sit under it in a custom runtime; such a route is cookie + // authenticated, is not a registered surface, and keeps CSRF enforced. + if (isSignedControlPlaneDispatch(req)) return this.continue(); + // Check exclude paths if (typeof csrfConfig === "object" && csrfConfig.excludePaths?.length) { for (const excludePath of csrfConfig.excludePaths) {