diff --git a/packages/api/src/routes/agent-request.server.ts b/packages/api/src/routes/agent-request.server.ts index cd8a659..809ccd6 100644 --- a/packages/api/src/routes/agent-request.server.ts +++ b/packages/api/src/routes/agent-request.server.ts @@ -4,10 +4,14 @@ import type { Bindings } from "../server"; import { detectRequestLocation } from "../server-helper"; import { generateAgentInvocationToken } from "./agents/me/me.server"; +export type AgentRequestRouting = + | { mode: "webhook"; subpath?: string } + | { mode: "subdomain" }; + export default async function handleAgentRequest( c: Context<{ Bindings: Bindings }>, id: string, - legacy?: boolean + routing: AgentRequestRouting ) { const db = await c.env.database(); const query = await db.selectAgentDeploymentByRequestID(id); @@ -37,8 +41,8 @@ export default async function handleAgentRequest( const incomingUrl = new URL(c.req.raw.url); let url: URL; - if (legacy) { - url = new URL("/webhook" + incomingUrl.search, directAccessURL); + if (routing.mode === "webhook") { + url = new URL(routing.subpath || "/", directAccessURL); } else { url = new URL(incomingUrl.pathname, directAccessURL); } @@ -49,7 +53,7 @@ export default async function handleAgentRequest( const contentLengthRaw = c.req.raw.headers.get("content-length"); if (contentLengthRaw) { contentLength = Number(contentLengthRaw); - if (isNaN(contentLength)) { + if (Number.isNaN(contentLength)) { contentLength = undefined; } } @@ -73,7 +77,7 @@ export default async function handleAgentRequest( const pathWithQuery = incomingUrl.pathname + incomingUrl.search; const truncatedPath = pathWithQuery.length > 80 - ? pathWithQuery.slice(0, 80) + "..." + ? `${pathWithQuery.slice(0, 80)}...` : pathWithQuery; // Extract useful headers for logging (not sensitive ones) @@ -125,18 +129,15 @@ export default async function handleAgentRequest( }) ); - let requestBodyPromise: Promise | undefined; - let upstreamBody: ReadableStream | undefined; - if (c.req.raw.body) { - let downstreamBody: ReadableStream; - [upstreamBody, downstreamBody] = c.req.raw.body.tee(); - requestBodyPromise = readBody(c.req.raw.headers, downstreamBody, 64 * 1024); - } - const headers = new Headers(); c.req.raw.headers.forEach((value, key) => { headers.set(key, value); }); + // Strip cookies from webhook requests to prevent session leakage + // Subdomain requests are on a different origin, so cookies won't be sent anyway + if (routing.mode === "webhook") { + headers.delete("cookie"); + } headers.set( BlinkInvocationTokenHeader, await generateAgentInvocationToken(c.env.AUTH_SECRET, { @@ -150,7 +151,7 @@ export default async function handleAgentRequest( let error: string | undefined; try { response = await fetch(url, { - body: upstreamBody, + body: c.req.raw.body, method: c.req.raw.method, signal, headers, @@ -162,15 +163,60 @@ export default async function handleAgentRequest( const agentID = query.agent_deployment.agent_id; const deploymentID = query.agent_deployment.id; - let responseBodyPromise: Promise | undefined; - if (response && response.body) { - const [toClient, toLog] = response.body.tee(); - responseBodyPromise = readBody(response.headers, toLog, 64 * 1024); - response = new Response(toClient, { - status: response.status, - statusText: response.statusText, - headers: response.headers, - }); + if (response) { + // Strip sensitive headers from webhook responses to prevent: + // - Session hijacking via set-cookie + // - Permissive CORS policies that could expose user data + // - XSS attacks via HTML responses + // - Open redirects via Location header + // Subdomain requests are on a different origin, so these don't apply + if (routing.mode === "webhook") { + const responseHeaders = new Headers(response.headers); + responseHeaders.delete("set-cookie"); + responseHeaders.delete("access-control-allow-origin"); + responseHeaders.delete("access-control-allow-credentials"); + responseHeaders.delete("access-control-allow-methods"); + responseHeaders.delete("access-control-allow-headers"); + + // Prevent open redirects - strip Location header + responseHeaders.delete("location"); + + // Security headers to prevent XSS and other attacks + // nosniff prevents browsers from MIME-sniffing responses + responseHeaders.set("x-content-type-options", "nosniff"); + // Restrictive CSP blocks all active content (scripts, styles, etc.) + responseHeaders.set( + "content-security-policy", + "default-src 'none'; frame-ancestors 'none'" + ); + // Prevent clickjacking + responseHeaders.set("x-frame-options", "DENY"); + + // Filter CORS-related values from Vary header + const vary = responseHeaders.get("vary"); + if (vary) { + const corsVaryValues = [ + "origin", + "access-control-request-method", + "access-control-request-headers", + ]; + const filtered = vary + .split(",") + .map((v) => v.trim()) + .filter((v) => !corsVaryValues.includes(v.toLowerCase())); + if (filtered.length > 0) { + responseHeaders.set("vary", filtered.join(", ")); + } else { + responseHeaders.delete("vary"); + } + } + + response = new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers: responseHeaders, + }); + } } const durationMs = Math.round(performance.now() - startTime); @@ -249,104 +295,3 @@ export default async function handleAgentRequest( ); } } - -interface RedactHeadersResult { - headers: Record; - redacted: boolean; -} - -// redactHeaders replaces sensitive headers with "REDACTED" and -// limits the number of headers to 100. -function redactHeaders(incoming: Headers): RedactHeadersResult { - const headers: Record = {}; - let headerCount = 0; - let redacted = false; - const sensitiveHeaders = ["authorization", "cookie", "set-cookie"]; - incoming.forEach((value, key) => { - if (headerCount >= 60) { - redacted = true; - return; - } - if (key.length > 128) { - redacted = true; - key = key.slice(0, 128); - } - if (value.length > 2048) { - redacted = true; - value = value.slice(0, 2048) + " ... [truncated]"; - } - headerCount++; - if (sensitiveHeaders.includes(key.toLowerCase())) { - headers[key] = "REDACTED"; - } else { - headers[key] = value; - } - }); - return { - headers: headers, - redacted, - }; -} - -interface ReadBodyResult { - body: string; - truncated: boolean; -} - -async function readBody( - headers: Headers, - body: ReadableStream, - maxLength: number -): Promise { - if (!isTextual(headers.get("content-type"))) { - // For non-textual content, cancel the stream immediately. - // We don't need to read it, just ensure it's canceled to signal - // to Cloudflare that we're not using this teed stream. - await body.cancel(); - return undefined; - } - const reader = body.getReader(); - try { - const decoder = new TextDecoder(); - let result = ""; - let totalRead = 0; - while (true) { - const { done, value } = await reader.read(); - if (done) { - break; - } - const chunk = decoder.decode(value, { stream: true }); - result += chunk; - totalRead += chunk.length; - if (totalRead > maxLength) { - // Cancel the reader - we've read enough - await reader.cancel(); - return { - body: result, - truncated: true, - }; - } - } - return { - body: result, - truncated: false, - }; - } finally { - reader.releaseLock(); - } -} - -const isTextual = (contentType: string | null) => { - if (!contentType) { - return false; - } - const v = contentType.toLowerCase(); - return ( - v.startsWith("text/") || - v.includes("json") || - v.includes("xml") || - v.includes("x-www-form-urlencoded") || - v.includes("graphql") || - v.includes("cloudevents+json") - ); -}; diff --git a/packages/api/src/routes/agent-request.test.ts b/packages/api/src/routes/agent-request.test.ts index c41860c..c6352ff 100644 --- a/packages/api/src/routes/agent-request.test.ts +++ b/packages/api/src/routes/agent-request.test.ts @@ -1,17 +1,37 @@ -import { expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; +import type { Server } from "bun"; import { serve } from "../test"; -test("webhook", async () => { +interface SetupAgentOptions { + name: string; + handler: (req: Request) => Response; +} + +interface SetupAgentResult extends Disposable { + /** Subpath webhook URL (/api/webhook/:id) - cookies are stripped */ + webhookUrl: string; + getWebhookUrl: (subpath?: string) => string; + /** Fetch via subdomain (uses Host header) - cookies pass through */ + fetchSubdomain: (path?: string, init?: RequestInit) => Promise; +} + +async function setupAgent( + options: SetupAgentOptions +): Promise { let deployedPromise: Promise | undefined; + let mockServer: Server | undefined; + let requestId: string | undefined; - const { bindings, helpers, url } = await serve({ + const { + bindings, + helpers, + url: apiUrl, + } = await serve({ bindings: { async deployAgent(deployment): Promise { deployedPromise = (async () => { - const srv = await Bun.serve({ - fetch: () => { - return new Response("Hello, world!"); - }, + mockServer = Bun.serve({ + fetch: options.handler, port: 0, }); @@ -20,7 +40,7 @@ test("webhook", async () => { id: deployment.id, agent_id: deployment.agent_id, status: "success", - direct_access_url: srv.url.toString(), + direct_access_url: mockServer.url.toString(), }); await db.updateAgent({ id: deployment.agent_id, @@ -28,34 +48,429 @@ test("webhook", async () => { }); })(); }, + matchRequestHost: (host: string) => { + // Match subdomain pattern: {request_id}.localhost:port + const match = host.match(/^([^.]+)\./); + if (match && requestId && match[1] === requestId) { + return requestId; + } + return undefined; + }, }, }); const { client } = await helpers.createUser(); const org = await client.organizations.create({ - name: "test-org", + name: `${options.name}-org`, }); const agent = await client.agents.create({ organization_id: org.id, - name: "test-agent", + name: `${options.name}-agent`, visibility: "public", + output_files: [{ path: "test.js", data: "console.log('test');" }], + }); + + if (deployedPromise) await deployedPromise; + if (!agent.request_url) throw new Error("No webhook route"); + + const db = await bindings.database(); + const target = await db.selectAgentDeploymentTargetByName( + agent.id, + "production" + ); + if (!target) throw new Error("No deployment target"); + requestId = target.request_id; + + const parsedApiUrl = new URL(apiUrl); + const subdomainHost = `${requestId}.${parsedApiUrl.host}`; + + return { + webhookUrl: `${apiUrl}/api/webhook/${target.request_id}`, + getWebhookUrl: (subpath?: string) => + `${apiUrl}/api/webhook/${target.request_id}${subpath || ""}`, + fetchSubdomain: (path?: string, init?: RequestInit) => { + // Make request to API server with Host header set to subdomain + // This simulates subdomain routing without needing DNS resolution + const headers = new Headers(init?.headers); + headers.set("host", subdomainHost); + return fetch(`${apiUrl}${path || "/"}`, { ...init, headers }); + }, + [Symbol.dispose]: () => mockServer?.stop(), + }; +} + +describe("webhook requests (/api/webhook/:id)", () => { + test("basic request", async () => { + using agent = await setupAgent({ + name: "basic", + handler: () => new Response("Hello, world!"), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello, world!"); + }); + + test("proxies subpaths", async () => { + let receivedPath: string | undefined; + + using agent = await setupAgent({ + name: "subpath", + handler: (req) => { + receivedPath = new URL(req.url).pathname; + return new Response(`Path: ${receivedPath}`); + }, + }); + + const response = await fetch(agent.getWebhookUrl("/github/events")); + expect(response.status).toBe(200); + expect(receivedPath).toBe("/github/events"); + }); + + test("strips cookies from requests", async () => { + let receivedCookieHeader: string | null | undefined; + + using agent = await setupAgent({ + name: "cookies", + handler: (req) => { + receivedCookieHeader = req.headers.get("cookie"); + return new Response("OK"); + }, + }); + + const response = await fetch(agent.webhookUrl, { + headers: { cookie: "session=secret-token; other=value" }, + }); + expect(response.status).toBe(200); + expect(receivedCookieHeader).toBeNull(); + }); + + test("strips set-cookie from responses", async () => { + using agent = await setupAgent({ + name: "setcookie", + handler: () => + new Response("OK", { + headers: { + "set-cookie": "malicious=hijack; Path=/; HttpOnly", + "x-custom-header": "should-pass-through", + }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(response.headers.get("set-cookie")).toBeNull(); + expect(response.headers.get("x-custom-header")).toBe("should-pass-through"); + }); + + test("strips CORS headers from responses", async () => { + using agent = await setupAgent({ + name: "cors", + handler: () => + new Response("OK", { + headers: { + "access-control-allow-origin": "*", + "access-control-allow-credentials": "true", + "access-control-allow-methods": "GET, POST, PUT, DELETE", + "access-control-allow-headers": "Content-Type, Authorization", + "x-custom-header": "should-pass-through", + }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(response.headers.get("access-control-allow-origin")).toBeNull(); + expect(response.headers.get("access-control-allow-credentials")).toBeNull(); + expect(response.headers.get("access-control-allow-methods")).toBeNull(); + expect(response.headers.get("access-control-allow-headers")).toBeNull(); + expect(response.headers.get("x-custom-header")).toBe("should-pass-through"); + }); + + test("filters CORS values from Vary header", async () => { + using agent = await setupAgent({ + name: "vary", + handler: () => + new Response("OK", { + headers: { + vary: "Origin, Accept-Encoding, Access-Control-Request-Method, Access-Control-Request-Headers", + }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + // Note: API's CORS middleware may add "Origin" back to Vary + // The important thing is that Access-Control-Request-* values are filtered + const vary = response.headers.get("vary"); + expect(vary).toContain("Accept-Encoding"); + expect(vary?.toLowerCase()).not.toContain("access-control-request-method"); + expect(vary?.toLowerCase()).not.toContain("access-control-request-headers"); + }); + + test("removes agent CORS values from Vary header", async () => { + using agent = await setupAgent({ + name: "vary-only-cors", + handler: () => + new Response("OK", { + headers: { + vary: "Access-Control-Request-Method, Access-Control-Request-Headers", + }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + // Agent's CORS Vary values should be removed + // API's CORS middleware may add "Origin" which is fine + const vary = response.headers.get("vary"); + if (vary) { + expect(vary.toLowerCase()).not.toContain("access-control-request-method"); + expect(vary.toLowerCase()).not.toContain( + "access-control-request-headers" + ); + } + }); + + test("strips uppercase Set-Cookie header", async () => { + using agent = await setupAgent({ + name: "uppercase-setcookie", + handler: () => + new Response("OK", { + headers: [["Set-Cookie", "malicious=hijack; Path=/; HttpOnly"]], + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(response.headers.get("set-cookie")).toBeNull(); + }); + + test("strips uppercase CORS headers", async () => { + using agent = await setupAgent({ + name: "uppercase-cors", + handler: () => + new Response("OK", { + headers: [ + ["ACCESS-CONTROL-ALLOW-ORIGIN", "*"], + ["ACCESS-CONTROL-ALLOW-CREDENTIALS", "true"], + ["ACCESS-CONTROL-ALLOW-METHODS", "GET, POST"], + ["ACCESS-CONTROL-ALLOW-HEADERS", "Content-Type"], + ], + }), + }); - output_files: [ - { - path: "test.js", - data: "console.log('Hello, world!');", + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(response.headers.get("ACCESS-CONTROL-ALLOW-ORIGIN")).toBeNull(); + expect(response.headers.get("access-control-allow-origin")).toBeNull(); + expect(response.headers.get("access-control-allow-credentials")).toBeNull(); + expect(response.headers.get("access-control-allow-methods")).toBeNull(); + expect(response.headers.get("access-control-allow-headers")).toBeNull(); + }); + + test("filters uppercase CORS values from Vary header", async () => { + using agent = await setupAgent({ + name: "uppercase-vary", + handler: () => + new Response("OK", { + headers: [ + [ + "VARY", + "ORIGIN, Accept-Encoding, ACCESS-CONTROL-REQUEST-METHOD, ACCESS-CONTROL-REQUEST-HEADERS", + ], + ], + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + const vary = response.headers.get("vary"); + expect(vary).toContain("Accept-Encoding"); + expect(vary?.toLowerCase()).not.toContain("access-control-request-method"); + expect(vary?.toLowerCase()).not.toContain("ACCESS-CONTROL-REQUEST-METHOD"); + expect(vary?.toLowerCase()).not.toContain("access-control-request-headers"); + }); + + test("adds X-Content-Type-Options: nosniff to prevent MIME sniffing", async () => { + using agent = await setupAgent({ + name: "nosniff", + handler: () => + new Response("", { + headers: { "content-type": "text/html" }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(response.headers.get("x-content-type-options")).toBe("nosniff"); + }); + + test("adds restrictive Content-Security-Policy header", async () => { + using agent = await setupAgent({ + name: "csp", + handler: () => + new Response("", { + headers: { "content-type": "text/html" }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + const csp = response.headers.get("content-security-policy"); + expect(csp).toContain("default-src 'none'"); + expect(csp).toContain("frame-ancestors 'none'"); + }); + + test("adds X-Frame-Options: DENY to prevent clickjacking", async () => { + using agent = await setupAgent({ + name: "xfo", + handler: () => new Response("OK"), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + expect(response.headers.get("x-frame-options")).toBe("DENY"); + }); + + test("strips Location header to prevent open redirects", async () => { + using agent = await setupAgent({ + name: "redirect", + handler: () => + new Response("Redirecting...", { + status: 200, + headers: { + location: "https://evil.com/phishing", + "x-custom": "preserved", + }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + // Location header should be stripped to prevent open redirects + expect(response.headers.get("location")).toBeNull(); + // Other headers should be preserved + expect(response.headers.get("x-custom")).toBe("preserved"); + }); + + test("overrides malicious security headers from agent", async () => { + using agent = await setupAgent({ + name: "malicious-headers", + handler: () => + new Response("OK", { + headers: { + "x-content-type-options": "unsafe", + "content-security-policy": "default-src *", + "x-frame-options": "ALLOWALL", + }, + }), + }); + + const response = await fetch(agent.webhookUrl); + expect(response.status).toBe(200); + // Our security headers should override malicious ones + expect(response.headers.get("x-content-type-options")).toBe("nosniff"); + expect(response.headers.get("content-security-policy")).toContain( + "default-src 'none'" + ); + expect(response.headers.get("x-frame-options")).toBe("DENY"); + }); +}); + +describe("subdomain requests", () => { + test("basic request", async () => { + using agent = await setupAgent({ + name: "subdomain-basic", + handler: () => new Response("Hello from subdomain!"), + }); + + const response = await agent.fetchSubdomain(); + expect(response.status).toBe(200); + expect(await response.text()).toBe("Hello from subdomain!"); + }); + + test("preserves path", async () => { + let receivedPath: string | undefined; + + using agent = await setupAgent({ + name: "subdomain-path", + handler: (req) => { + receivedPath = new URL(req.url).pathname; + return new Response(`Path: ${receivedPath}`); }, - ], - }); - await deployedPromise!; - if (!agent.request_url) { - throw new Error("No webhook route"); - } - - // Test the wildcard hostname routing (current implementation). - // We need to make the request go through the test server with the correct Host header. - const agentRequestURL = new URL(agent.request_url!); - const response1 = await fetch(agentRequestURL); - expect(response1.status).toBe(200); - expect(await response1.text()).toBe("Hello, world!"); + }); + + const response = await agent.fetchSubdomain("/api/data"); + expect(response.status).toBe(200); + expect(receivedPath).toBe("/api/data"); + }); + + test("passes cookies through", async () => { + let receivedCookieHeader: string | null | undefined; + + using agent = await setupAgent({ + name: "subdomain-cookies", + handler: (req) => { + receivedCookieHeader = req.headers.get("cookie"); + return new Response("OK"); + }, + }); + + const response = await agent.fetchSubdomain("/", { + headers: { cookie: "session=secret-token; other=value" }, + }); + expect(response.status).toBe(200); + expect(receivedCookieHeader).toBe("session=secret-token; other=value"); + }); + + test("passes set-cookie through", async () => { + using agent = await setupAgent({ + name: "subdomain-setcookie", + handler: () => + new Response("OK", { + headers: { + "set-cookie": "agent-cookie=value; Path=/; HttpOnly", + "x-custom-header": "should-pass-through", + }, + }), + }); + + const response = await agent.fetchSubdomain(); + expect(response.status).toBe(200); + expect(response.headers.get("set-cookie")).toBe( + "agent-cookie=value; Path=/; HttpOnly" + ); + expect(response.headers.get("x-custom-header")).toBe("should-pass-through"); + }); + + test("passes CORS headers through", async () => { + using agent = await setupAgent({ + name: "subdomain-cors", + handler: () => + new Response("OK", { + headers: { + "access-control-allow-origin": "*", + "access-control-allow-credentials": "true", + "access-control-allow-methods": "GET, POST", + "access-control-allow-headers": "Content-Type", + vary: "Origin, Accept-Encoding", + }, + }), + }); + + const response = await agent.fetchSubdomain(); + expect(response.status).toBe(200); + expect(response.headers.get("access-control-allow-origin")).toBe("*"); + expect(response.headers.get("access-control-allow-credentials")).toBe( + "true" + ); + expect(response.headers.get("access-control-allow-methods")).toBe( + "GET, POST" + ); + expect(response.headers.get("access-control-allow-headers")).toBe( + "Content-Type" + ); + expect(response.headers.get("vary")).toBe("Origin, Accept-Encoding"); + }); }); diff --git a/packages/api/src/server.ts b/packages/api/src/server.ts index 7546137..6d2e9eb 100644 --- a/packages/api/src/server.ts +++ b/packages/api/src/server.ts @@ -241,12 +241,17 @@ const api = new Hono<{ Bindings: Bindings; }>() .basePath("/api") - .use( - cors({ + .use(async (c, next) => { + // Skip CORS middleware for webhook routes - they handle their own CORS filtering + // to preserve non-CORS Vary header values from agents + if (c.req.path.startsWith("/api/webhook/")) { + return next(); + } + return cors({ // We're going to test the embedding of chats on coder.com. origin: ["https://blink.so"], - }) - ); + })(c, next); + }); api.use(async (c, next) => { await next(); @@ -307,13 +312,19 @@ mountTools(api.basePath("/tools")); mountOtlp(api.basePath("/otlp")); mountDevhook(api.basePath("/devhook")); -// Legacy webhook route for backwards compatibility. -api.all("/webhook/:id", async (c) => { - const id = c.req.param("id"); +// Webhook route for proxying requests to agents +// The wildcard route handles subpaths like /api/webhook/:id/github/events +api.all("/webhook/:id{.*}", async (c) => { + // Extract id and subpath from the matched param + const fullParam = c.req.param("id"); + const slashIndex = fullParam.indexOf("/"); + const id = slashIndex === -1 ? fullParam : fullParam.slice(0, slashIndex); + const subpath = slashIndex === -1 ? "" : fullParam.slice(slashIndex); + if (!validate(id)) { return c.json({ message: "Invalid ID" }, 400); } - return handleAgentRequest(c, id, true); + return handleAgentRequest(c, id, { mode: "webhook", subpath }); }); export type AgentServer = Hono<{ @@ -332,7 +343,7 @@ app.all("*", (c) => { // We handle all agent requests here if they match. // This could be for dev requests or production requests. if (match) { - return handleAgentRequest(c, match, false); + return handleAgentRequest(c, match, { mode: "subdomain" }); } // Just pass through to the API. // We could make it 404 here, but it's really unnecessary