diff --git a/docs/api-reference/veryfront/security.md b/docs/api-reference/veryfront/security.md index 6d34cc121d..2b1a16a648 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#L57) | +| `CsrfHandler` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/security/http/csrf/csrf-handler.ts#L60) | | `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 dfd793bae3..bc0362bad4 100644 --- a/src/channels/control-plane.ts +++ b/src/channels/control-plane.ts @@ -30,6 +30,12 @@ const CONTROL_PLANE_RUNS_REGEX_PREFIX = CONTROL_PLANE_RUNS_PATH_PREFIX.replaceAl /** Request header the control plane carries its signed operation envelope in. */ export const CONTROL_PLANE_JWS_HEADER = "x-veryfront-control-plane-jws"; +/** Request header a platform channel dispatch carries its signed envelope in. */ +export const DISPATCH_JWS_HEADER = "x-veryfront-dispatch-jws"; + +/** The one route that accepts a signed channel dispatch envelope. */ +export const CHANNEL_INVOKE_PATH = "/channels/invoke"; + const CONTROL_PLANE_RUN_OPERATION_PATH = /^\/api\/control-plane\/runs\/[^/]+\/(?:execute|stream|resume)$/u; const CONTROL_PLANE_RUN_PATH = /^\/api\/control-plane\/runs\/[^/]+$/u; @@ -98,6 +104,56 @@ export function isSignedControlPlaneDispatch(req: Request): boolean { return isControlPlaneSurfaceRoute(req.method, new SafeURL(req.url).pathname); } +/** + * True when a method and path pair addresses the channel dispatch handler. + * + * `POST /channels/invoke` is the one route `ChannelInvokeHandler` registers, + * and the only route that verifies a channel dispatch envelope. It is + * deliberately not part of {@link isControlPlaneSurfaceRoute}: the control + * plane's `channels` surface names a product surface inside a control-plane + * envelope, not this HTTP route, and this route carries a different envelope. + * + * The `/channels/` namespace is reserved but not exclusively routed, so any + * sibling or child path is matched exactly rather than by prefix. Match this + * against `URL.pathname`, which resolves dot segments. + */ +export function isChannelDispatchRoute( + method: string, + pathname: string | undefined, +): boolean { + return method.toUpperCase() === "POST" && pathname === CHANNEL_INVOKE_PATH; +} + +/** + * True for a request that is a platform channel dispatch rather than a browser one. + * + * Both conditions must hold. The method and path must be the one route the + * channel invoke handler owns (see {@link isChannelDispatchRoute}), and the + * request must carry a dispatch signature header. The handler then verifies + * that envelope with `verifyDispatchJws`, which binds the Ed25519 signature to + * the issuer, the project audience, the project id, the dispatch id, the + * platform and a SHA-256 hash of the body, with expiry and skew bounds; the + * handler additionally rejects an envelope whose claims do not match the + * dispatch id, platform and project id in the payload it acts on. + * + * Callers use this to keep gates that assume a browser client, such as CSRF + * double-submit validation, from standing in front of platform dispatch. The + * channel dispatcher and the runtime-owner re-dispatch in + * `resolveRuntimeOwnerInvokeUrl` hold no `__Host-vf_csrf` cookie to echo and + * derive no authority from one. 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. + * + * 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 isSignedChannelDispatch(req: Request): boolean { + const signature = req.headers.get(DISPATCH_JWS_HEADER); + if (signature === null || signature.length === 0) return false; + + return isChannelDispatchRoute(req.method, new SafeURL(req.url).pathname); +} + /** * True for control-plane run surfaces that can dispatch without project config. * diff --git a/src/channels/invoke-dispatch-security.test.ts b/src/channels/invoke-dispatch-security.test.ts new file mode 100644 index 0000000000..e1e17c3f2f --- /dev/null +++ b/src/channels/invoke-dispatch-security.test.ts @@ -0,0 +1,319 @@ +/** + * Regression: a project that configures `security.csrf` must still receive its + * own channel dispatches. + * + * A Slack/Discord/etc. message reaches an agent because the platform channel + * dispatcher POSTs `/channels/invoke` with a signed dispatch envelope in + * `x-veryfront-dispatch-jws`; `resolveRuntimeOwnerInvokeUrl` re-dispatches to + * the same route when the owning runtime instance is a different pod. Neither + * caller is a browser, so neither holds a `__Host-vf_csrf` cookie to echo. + * + * `CsrfHandler` runs at priority 5 with an empty pattern list, ahead of + * `ChannelInvokeHandler` at 700, so a project whose config enables CSRF at all + * answers its own channel dispatch with + * `403 Forbidden - invalid or missing CSRF token`. The agent never runs and the + * channel simply goes quiet: the failure names neither CSRF nor config. + * + * PR #3641 exempted the control-plane surfaces, but a channel dispatch carries + * a different envelope (`verifyDispatchJws`, bound to dispatch id, platform, + * project id and body hash) under a different header, so + * `isSignedControlPlaneDispatch` does not and must not match it. + * + * @module channels/invoke-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 { createEmptyDiscoveryResult } from "#veryfront/discovery"; +import type { Agent, AgentMessage, AgentResponse } from "#veryfront/agent"; +import type { VeryfrontConfig } from "#veryfront/config"; +import type { HandlerContext } from "#veryfront/types"; +import { base64urlEncode, base64urlEncodeBytes } from "#veryfront/utils/base64url.ts"; +import { ChannelInvokeHandler } from "#veryfront/server/handlers/request/channel-invoke.handler.ts"; + +const INVOKE_PATH = "/channels/invoke"; +const encoder = new TextEncoder(); + +type CsrfSetting = VeryfrontConfig["security"] extends infer S + ? S extends { csrf?: infer C } ? C : never + : never; + +function encodePem(label: string, der: ArrayBuffer): string { + const base64 = btoa(String.fromCharCode(...new Uint8Array(der))); + const lines = base64.match(/.{1,64}/g) ?? [base64]; + return `-----BEGIN ${label}-----\n${lines.join("\n")}\n-----END ${label}-----`; +} + +async function sha256Base64url(body: string): Promise { + const digest = await crypto.subtle.digest("SHA-256", encoder.encode(body)); + return base64urlEncodeBytes(new Uint8Array(digest)); +} + +async function createDispatchSignature( + body: string, +): Promise<{ jws: string; publicKeyPem: string }> { + const keyPair = await crypto.subtle.generateKey( + "Ed25519", + true, + ["sign", "verify"], + ) as CryptoKeyPair; + const publicKeyDer = await crypto.subtle.exportKey("spki", keyPair.publicKey); + const publicKeyPem = encodePem("PUBLIC KEY", publicKeyDer); + const now = Math.floor(Date.now() / 1000); + + const header = base64urlEncode(JSON.stringify({ alg: "EdDSA", typ: "JWT" })); + const payload = base64urlEncode(JSON.stringify({ + iss: "veryfront-api", + aud: "demo-project", + sub: "dispatch-1", + project_id: "proj-1", + platform: "slack", + body_sha256: await sha256Base64url(body), + iat: now, + exp: now + 60, + })); + + const signingInput = encoder.encode(`${header}.${payload}`); + const signature = await crypto.subtle.sign("Ed25519", keyPair.privateKey, signingInput); + + return { + publicKeyPem, + jws: `${header}.${payload}.${base64urlEncodeBytes(new Uint8Array(signature))}`, + }; +} + +function createInvokeBody(): string { + return JSON.stringify({ + dispatchId: "dispatch-1", + conversationId: "conversation-1", + projectId: "proj-1", + assistantId: "agent-1", + platform: "slack", + inboundMessage: { + text: "Hello from Slack", + userId: "U123", + userName: "Alice", + isDirectMessage: false, + }, + conversationHistory: [ + { id: "user-1", role: "user", parts: [{ type: "text", text: "Hello from Slack" }] }, + ], + }); +} + +function createAgentResponse(): AgentResponse { + const assistantMessage: AgentMessage = { + id: "assistant-1", + role: "assistant", + parts: [{ type: "text", text: "Hello from the agent" }], + }; + return { + text: "Hello from the agent", + messages: [assistantMessage], + toolCalls: [], + status: "completed", + usage: { promptTokens: 5, completionTokens: 7, totalTokens: 12 }, + }; +} + +function createCtx(publicKeyPem?: string): HandlerContext { + return { + projectDir: "/project", + adapter: { + env: { + get: (key: string) => + key === "CHANNEL_DISPATCH_SIGNING_PUBLIC_KEY" ? publicKeyPem : undefined, + }, + fs: {}, + }, + securityConfig: null, + projectSlug: "demo-project", + projectId: "proj-1", + isLocalProject: false, + } as unknown as HandlerContext; +} + +interface DispatchOutcome { + /** Whether the agent behind the channel dispatch was reached at all. */ + readonly answered: boolean; + readonly status: number; + readonly body: string; +} + +interface DispatchOverrides { + readonly method?: string; + readonly path?: string; + /** Replaces the dispatch signature header name, or drops it when null. */ + readonly signatureHeader?: string | null; + /** + * Replaces the signed envelope with an arbitrary value, keeping the real + * verification key in context so the handler genuinely rejects it. + */ + readonly signatureValue?: string; +} + +/** + * Drive the real handler chain the runtime uses for a channel dispatch: the + * security handlers first, then the channel invoke handler. + */ +async function dispatchChannelInvoke( + csrf: CsrfSetting | undefined, + overrides: DispatchOverrides = {}, +): Promise { + const config = { + security: csrf === undefined ? {} : { csrf }, + } as VeryfrontConfig; + const { securityConfig } = deriveSecurityContext(config, { productionDefaults: false }); + + const body = createInvokeBody(); + const { jws, publicKeyPem } = await createDispatchSignature(body); + + const headers: Record = { "content-type": "application/json" }; + if (overrides.signatureHeader !== null) { + headers[overrides.signatureHeader ?? "x-veryfront-dispatch-jws"] = overrides.signatureValue ?? + jws; + } + + const request = new Request( + `https://demo-project.example.test${overrides.path ?? INVOKE_PATH}`, + { method: overrides.method ?? "POST", headers, body }, + ); + + let answered = false; + const handler = new ChannelInvokeHandler({ + ensureProjectDiscovery: async () => createEmptyDiscoveryResult(), + getAgent: () => + createAgent(() => { + answered = true; + return Promise.resolve(createAgentResponse()); + }), + getAllAgentIds: () => ["agent-1"], + }); + + const ctx = createCtx(publicKeyPem); + ctx.securityConfig = securityConfig; + + const registry = new RouteRegistry(); + registry.registerAll([new CsrfHandler(), handler]); + + const response = await registry.execute(request, ctx); + return { + answered, + status: response?.status ?? 0, + body: response ? await response.text() : "", + }; +} + +function createAgent(generate: () => Promise): Agent { + return { + id: "agent-1", + config: {} as Agent["config"], + generate: generate as unknown as Agent["generate"], + stream: async () => ({ toDataStreamResponse: () => new Response() } as never), + respond: async () => new Response(), + getMemory: () => ({} as never), + getMemoryStats: async () => ({ totalMessages: 0, estimatedTokens: 0, type: "conversation" }), + clearMemory: async () => {}, + }; +} + +describe("channels: signed channel dispatch vs a project CSRF policy", () => { + it("answers a dispatch when the project leaves csrf unset", async () => { + const outcome = await dispatchChannelInvoke(undefined); + assertEquals(outcome.status, 200); + assertEquals(outcome.answered, true); + }); + + it("answers a dispatch when the project enables csrf with a boolean", async () => { + const outcome = await dispatchChannelInvoke(true); + assertEquals( + outcome.answered, + true, + `channel dispatch never reached the agent; runtime answered ${outcome.status}: ${outcome.body}`, + ); + assertEquals(outcome.status, 200); + }); + + it("answers a dispatch when the project excludes an unrelated path from csrf", async () => { + const outcome = await dispatchChannelInvoke({ excludePaths: ["/api/ag-ui"] }); + assertEquals( + outcome.answered, + true, + `channel dispatch never reached the agent; runtime answered ${outcome.status}: ${outcome.body}`, + ); + assertEquals(outcome.status, 200); + }); + + it("still rejects an invoke POST that carries no dispatch signature", 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 outcome = await dispatchChannelInvoke(true, { signatureHeader: null }); + assertEquals(outcome.status, 403); + assertEquals(outcome.answered, false); + }); + + it("still rejects an invoke POST that carries only a control-plane signature", async () => { + // The two envelopes are not interchangeable. A control-plane JWS binds a + // method/path pair under `/api/control-plane/`, and the invoke handler + // verifies a dispatch JWS instead, so presenting the wrong header must not + // buy the exemption. + const outcome = await dispatchChannelInvoke(true, { + signatureHeader: "x-veryfront-control-plane-jws", + }); + assertEquals(outcome.status, 403); + assertEquals(outcome.answered, false); + }); + + it("still rejects an invoke POST whose dispatch signature does not verify", async () => { + // The exemption is granted on the header being present, so the whole of its + // safety rests on the handler behind it verifying the envelope. Drive the + // full chain to prove the request that takes the exemption is still + // rejected: `CsrfHandler` steps aside (no 403), `ChannelInvokeHandler` + // fails `verifyDispatchJws` and answers 401, and the agent never runs. + const foreign = await createDispatchSignature(createInvokeBody()); + + for ( + const signatureValue of [ + // Not a JWS at all. + "not-a-dispatch-envelope", + // Well formed and correctly bound to this body, but signed by a key the + // runtime does not trust. + foreign.jws, + ] + ) { + const outcome = await dispatchChannelInvoke(true, { signatureValue }); + assertEquals( + outcome.status, + 401, + `an unverifiable envelope was answered ${outcome.status}: ${outcome.body}`, + ); + assertEquals(outcome.answered, false); + } + }); + + it("still rejects a genuinely signed dispatch aimed at a look-alike route", async () => { + // `/channels/` is reserved but not exclusively routed, so a project route + // can sit beside or beneath the one dispatch route. A real envelope, minted + // for a real dispatch, must not exempt a neighbouring path or a method the + // invoke handler does not serve. + for ( + const route of [ + { method: "POST", path: "/channels/invoke/application-route" }, + { method: "POST", path: "/channels/invoker" }, + { method: "PUT", path: INVOKE_PATH }, + ] + ) { + const outcome = await dispatchChannelInvoke(true, route); + assertEquals( + outcome.status, + 403, + `${route.method} ${route.path} skipped the CSRF gate`, + ); + assertEquals(outcome.answered, false); + } + }); +}); diff --git a/src/proxy/control-plane-signature.ts b/src/proxy/control-plane-signature.ts index 48ad171f98..5ff1bec8f2 100644 --- a/src/proxy/control-plane-signature.ts +++ b/src/proxy/control-plane-signature.ts @@ -28,6 +28,8 @@ import { getHostEnv } from "#veryfront/platform/compat/process.ts"; import { + CHANNEL_INVOKE_PATH, + isChannelDispatchRoute, isControlPlaneSurfaceRoute, verifyControlPlaneJwsRequestSignature, verifyControlPlaneJwsSignature, @@ -75,7 +77,7 @@ export function classifyInternalControlPlaneRequest( pathname: string, ): InternalControlPlaneRouteKind { const normalizedMethod = method.toUpperCase(); - if (pathname === "/channels/invoke" && normalizedMethod === "POST") { + if (isChannelDispatchRoute(normalizedMethod, pathname)) { return "dispatch"; } if (isControlPlaneSurfaceRoute(normalizedMethod, pathname)) { @@ -89,8 +91,8 @@ export function classifyInternalControlPlaneRequest( pathname.startsWith("/internal/tasks/") || pathname === "/internal/workflows" || pathname.startsWith("/internal/workflows/") || - pathname === "/channels/invoke" || - pathname.startsWith("/channels/invoke/") + pathname === CHANNEL_INVOKE_PATH || + pathname.startsWith(`${CHANNEL_INVOKE_PATH}/`) ) { return "reserved"; } diff --git a/src/security/http/csrf/csrf-handler.test.ts b/src/security/http/csrf/csrf-handler.test.ts index 52b3f21fb6..0cb6c93cd4 100644 --- a/src/security/http/csrf/csrf-handler.test.ts +++ b/src/security/http/csrf/csrf-handler.test.ts @@ -174,6 +174,119 @@ describe("security/http/csrf/csrf-handler", () => { }); }); + describe("signed channel dispatch", () => { + const SIGNED_DISPATCH = { "x-veryfront-dispatch-jws": "header.payload.signature" }; + const SIGNED_CONTROL_PLANE = { "x-veryfront-control-plane-jws": "header.payload.signature" }; + + it("passes the channel invoke route through for every enabled csrf shape", async () => { + // The platform channel dispatcher, and the runtime-owner re-dispatch that + // forwards to the instance owning the run, both POST this route with a + // signed dispatch envelope and no browser cookie. Gating them here stops + // a project's own Slack and Discord channels from answering. + for (const csrf of [true, { excludePaths: ["/api/ag-ui"] }]) { + const result = await handler.handle( + new Request("https://acme.example.test/channels/invoke", { + method: "POST", + headers: SIGNED_DISPATCH, + body: "{}", + }), + createCtx(csrf), + ); + + assertEquals(result.response, undefined, "POST /channels/invoke was gated"); + } + }); + + it("still enforces CSRF on the invoke route with no signature header", async () => { + const result = await handler.handle( + new Request("https://acme.example.test/channels/invoke", { + method: "POST", + body: "{}", + }), + createCtx(true), + ); + + assertEquals(result.response?.status, 403); + }); + + it("does not accept a control-plane envelope on the invoke route", async () => { + // The two envelopes are verified by different code against different + // claims. `ChannelInvokeHandler` reads only the dispatch header, so a + // control-plane header here would buy an exemption no handler redeems. + const result = await handler.handle( + new Request("https://acme.example.test/channels/invoke", { + method: "POST", + headers: SIGNED_CONTROL_PLANE, + body: "{}", + }), + createCtx(true), + ); + + assertEquals(result.response?.status, 403); + }); + + it("does not accept a dispatch envelope on a control-plane surface", async () => { + // Symmetric to the above: the run execute handler verifies a + // control-plane envelope and never reads the dispatch header. + const result = await handler.handle( + new Request("https://acme.example.test/api/control-plane/runs/run_1/execute", { + method: "POST", + headers: SIGNED_DISPATCH, + body: "{}", + }), + createCtx(true), + ); + + assertEquals(result.response?.status, 403); + }); + + it("still enforces CSRF on look-alike and sibling channel routes", async () => { + // `/channels/` is reserved but not exclusively routed. A project App or + // Pages API route can sit beside or beneath the one dispatch route, is + // cookie authenticated, and must keep CSRF enforced even when the caller + // sets the signature header itself. + const projectRoutes = [ + { method: "POST", path: "/channels/invoke/application-route" }, + { method: "POST", path: "/channels/invoker" }, + { method: "POST", path: "/channels/invoke-mirror/run" }, + { method: "POST", path: "/channels" }, + { method: "POST", path: "/api/channels/invoke" }, + { method: "PUT", path: "/channels/invoke" }, + { method: "DELETE", path: "/channels/invoke" }, + ]; + + for (const route of projectRoutes) { + const result = await handler.handle( + new Request(`https://acme.example.test${route.path}`, { + method: route.method, + headers: SIGNED_DISPATCH, + body: "{}", + }), + createCtx(true), + ); + + assertEquals( + result.response?.status, + 403, + `${route.method} ${route.path} skipped the CSRF gate`, + ); + } + }); + + it("still enforces CSRF when the signature header is empty", async () => { + const result = await handler.handle( + new Request("https://acme.example.test/channels/invoke", { + method: "POST", + headers: { "x-veryfront-dispatch-jws": "" }, + 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 8dfba2134c..5610c95059 100644 --- a/src/security/http/csrf/csrf-handler.ts +++ b/src/security/http/csrf/csrf-handler.ts @@ -42,7 +42,10 @@ */ import { isCspReportRequest } from "#veryfront/security/http/csp-report-endpoint.ts"; -import { isSignedControlPlaneDispatch } from "#veryfront/channels/control-plane.ts"; +import { + isSignedChannelDispatch, + isSignedControlPlaneDispatch, +} from "#veryfront/channels/control-plane.ts"; import { BaseHandler } from "../base-handler.ts"; import { validateCsrf } from "../../csrf/helpers.ts"; import type { @@ -97,6 +100,22 @@ export class CsrfHandler extends BaseHandler { // authenticated, is not a registered surface, and keeps CSRF enforced. if (isSignedControlPlaneDispatch(req)) return this.continue(); + // A platform channel dispatch is not a browser request either. A Slack or + // Discord message reaches an agent because the channel dispatcher POSTs + // `/channels/invoke` carrying a signed dispatch envelope, and the runtime + // re-dispatches to the same route when another instance owns the run. + // Neither caller holds a `__Host-vf_csrf` cookie, so gating them here does + // not stop a cross-site request; it silently stops the project's own + // channels from answering at all. + // + // This is a separate predicate rather than another entry in the + // control-plane route table on purpose. A channel dispatch carries a + // different envelope under a different header, verified by + // `verifyDispatchJws` against the dispatch id, platform, project id and + // body hash, so the two are not interchangeable and neither header may + // stand in for the other. + if (isSignedChannelDispatch(req)) return this.continue(); + // Check exclude paths if (typeof csrfConfig === "object" && csrfConfig.excludePaths?.length) { for (const excludePath of csrfConfig.excludePaths) {