diff --git a/packages/control-plane/src/router.create-session.test.ts b/packages/control-plane/src/router.create-session.test.ts index 17294dbd0..4f8f4501b 100644 --- a/packages/control-plane/src/router.create-session.test.ts +++ b/packages/control-plane/src/router.create-session.test.ts @@ -8,7 +8,7 @@ import { TEST_BACKGROUND_TASK_CONTEXT, TEST_SERVICE_SECRETS, } from "./router.test-support"; -import { sessionCreateRoutes } from "./routes/session-create"; +import { handleCreateSession } from "./routes/session-create"; import { HttpError, resolveRepoOrError } from "./routes/shared"; import { SessionInternalPaths } from "./session/contracts"; import { resolveManagedSkills } from "./session/skill-resolution"; @@ -619,7 +619,7 @@ describe("handleCreateSession D1 ordering", () => { const testEnv: Record = createEnv(initFetch); testEnv.SCM_PROVIDER = "gitlab"; - const response = await sessionCreateRoutes[0].handler( + const response = await handleCreateSession( new Request("https://test.local/sessions", { method: "POST", headers: { "Content-Type": "application/json" }, @@ -634,7 +634,6 @@ describe("handleCreateSession D1 ordering", () => { }), }), testEnv as never, - [] as unknown as RegExpMatchArray, { request_id: "test-request", trace_id: "test-trace", diff --git a/packages/control-plane/src/routes/catalog.ts b/packages/control-plane/src/routes/catalog.ts index 92d74cb17..72bc1a728 100644 --- a/packages/control-plane/src/routes/catalog.ts +++ b/packages/control-plane/src/routes/catalog.ts @@ -27,10 +27,9 @@ import { reposRoutes } from "./repos"; import { scmSettingsRoutes } from "./scm-settings"; import { secretsRoutes } from "./secrets"; import { sessionRoutes } from "./sessions"; -import { handleSlackNotify } from "./slack-notify"; +import { slackNotifyRoutes } from "./slack-notify"; import { signInProviderRoutes } from "./sign-in-providers"; import { skillRoutes } from "./skills"; -import { defineRoute, GITHUB_SANDBOX_FALLBACK_ROUTE, requirePermission } from "./shared"; /** * Registration order is the precedence order. A Hono sub-app is mounted where @@ -42,15 +41,9 @@ export const catalog: RouteCatalogEntry[] = [ ...browserAuthRoutes, signInProviderRoutes, - // Session management - ...sessionRoutes, - // Agent-initiated Slack notification (sandbox-authenticated) - defineRoute(GITHUB_SANDBOX_FALLBACK_ROUTE, { - method: "POST", - path: "/sessions/:id/slack-notify", - authorization: requirePermission("sessions.collaborate"), - handler: handleSlackNotify, - }), + // Session management, then the agent-initiated Slack notification + sessionRoutes, + slackNotifyRoutes, // Repository management ...reposRoutes, diff --git a/packages/control-plane/src/routes/session-attachments.test.ts b/packages/control-plane/src/routes/session-attachments.test.ts index 7fa1e01a2..aeacf4561 100644 --- a/packages/control-plane/src/routes/session-attachments.test.ts +++ b/packages/control-plane/src/routes/session-attachments.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it, vi } from "vitest"; import { SESSION_ATTACHMENT_MAX_REQUEST_BYTES } from "../media"; import type { Env } from "../types"; -import { sessionAttachmentRoutes } from "./session-attachments"; +import { handleAttachmentPost } from "./session-attachments"; import type { RequestContext } from "./shared"; import type { SqlDatabase } from "../db/sql-database"; -import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; +import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; +import { withSessionRuntime } from "./session-route"; const PNG_BYTES = Uint8Array.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); @@ -65,28 +66,16 @@ function oversizedStreamingUploadRequest(): Request { } as RequestInit & { duplex: "half" }); } -function getUploadRoute() { - const path = "/sessions/session-1/attachments"; - const route = sessionAttachmentRoutes.find( - (candidate) => candidate.method === "POST" && path.match(routePathPattern(candidate.path)) - ); - if (!route) throw new Error("Attachment upload route not found"); - const match = path.match(routePathPattern(route.path)); - if (!match) throw new Error("Attachment upload route did not match"); - return { route, match }; -} - describe("session attachment routes", () => { it("bounds streamed requests when Content-Length is unavailable", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); const { env, put } = createEnv(fetch); - const { route, match } = getUploadRoute(); - const response = await route.handler( + const response = await handleAttachmentPost( oversizedStreamingUploadRequest(), env, - match, - createContext() + { id: "session-1" }, + withSessionRuntime(env, createContext()) ); expect(response.status).toBe(413); @@ -105,9 +94,13 @@ describe("session attachment routes", () => { Response.json({ error: message }, { status: registryStatus }) ); const { env, put } = createEnv(fetch); - const { route, match } = getUploadRoute(); - const response = await route.handler(attachmentUploadRequest(), env, match, createContext()); + const response = await handleAttachmentPost( + attachmentUploadRequest(), + env, + { id: "session-1" }, + withSessionRuntime(env, createContext()) + ); expect(response.status).toBe(routeStatus); await expect(response.json()).resolves.toEqual({ error: message }); @@ -133,9 +126,13 @@ describe("session attachment routes", () => { }); const { env, put, remove } = createEnv(fetch); remove.mockRejectedValue(new Error("R2 unavailable")); - const { route, match } = getUploadRoute(); - const response = await route.handler(attachmentUploadRequest(), env, match, createContext()); + const response = await handleAttachmentPost( + attachmentUploadRequest(), + env, + { id: "session-1" }, + withSessionRuntime(env, createContext()) + ); expect(response.status).toBe(503); await expect(response.json()).resolves.toEqual({ diff --git a/packages/control-plane/src/routes/session-attachments.ts b/packages/control-plane/src/routes/session-attachments.ts index f0b15ae48..371f63acd 100644 --- a/packages/control-plane/src/routes/session-attachments.ts +++ b/packages/control-plane/src/routes/session-attachments.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; /** * Session image attachments added through the chat composer. * @@ -45,15 +48,13 @@ import { createStoredObjectResponse, } from "./responses/stored-object-response"; import { - defineRoute, error, GITHUB_SANDBOX_FALLBACK_ROUTE, GITHUB_USER_OR_SERVICE_ROUTE, json, requirePermission, - type Route, } from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-attachments"); @@ -76,13 +77,13 @@ function attachmentStorageErrorResponse(cause: SessionAttachmentStorageError): R } } -async function handleAttachmentPost( +export async function handleAttachmentPost( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); if (sessionAttachmentRequestExceedsLimit(request)) { return error("Attachment request is too large", 413); @@ -179,14 +180,14 @@ async function handleAttachmentPost( ); } -async function handleAttachmentGet( +export async function handleAttachmentGet( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; attachmentId: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; - const attachmentId = match.groups?.attachmentId; + const sessionId = params.id; + const attachmentId = params.attachmentId; if (!sessionId || !attachmentId) { return error("Session ID and attachment ID are required", 400); } @@ -237,23 +238,19 @@ async function handleAttachmentGet( : createStoredObjectResponse(body, metadata, contentType); } -export const sessionAttachmentRoutes: Route[] = [ - defineRoute( - GITHUB_USER_OR_SERVICE_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/attachments", - authorization: requirePermission("sessions.collaborate"), - handler: handleAttachmentPost, - }) - ), - defineRoute( - GITHUB_SANDBOX_FALLBACK_ROUTE, - sessionRoute({ - method: "GET", - path: "/sessions/:id/attachments/:attachmentId", - authorization: requirePermission("sessions.read"), - handler: handleAttachmentGet, - }) - ), -]; +export const sessionAttachmentRoutes = new Hono(); + +sessionAttachmentRoutes.post( + "/sessions/:id/attachments", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("sessions.collaborate"), + }), + (c) => dispatchSession(c, handleAttachmentPost) +); + +sessionAttachmentRoutes.get( + "/sessions/:id/attachments/:attachmentId", + admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatchSession(c, handleAttachmentGet) +); diff --git a/packages/control-plane/src/routes/session-child-spawn.ts b/packages/control-plane/src/routes/session-child-spawn.ts index 5e2567914..266e0a456 100644 --- a/packages/control-plane/src/routes/session-child-spawn.ts +++ b/packages/control-plane/src/routes/session-child-spawn.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { spawnChildSessionRequestSchema } from "@open-inspect/shared/types/session-api"; import { DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS, @@ -28,15 +31,13 @@ import { import { spawnContextSchema } from "../session/spawn-context"; import type { Env } from "../types"; import { - defineRoutes, error, GITHUB_SANDBOX_FALLBACK_ROUTE, json, permissionRequirement, requireAll, - type Route, } from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; import { DEFAULT_BASE_BRANCH } from "../repos/default-branch"; import { authorizeSessionTarget } from "./session-target-authorization"; @@ -47,13 +48,13 @@ function isJsonRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -async function handleSpawnChild( +export async function handleSpawnChild( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const parentId = match.groups?.id; + const parentId = params.id; if (!parentId) return error("Parent session ID required"); const parsedBody = spawnChildSessionRequestSchema.safeParse(await request.json()); @@ -350,14 +351,16 @@ async function handleSpawnChild( return json({ sessionId: childId, status: "created" }, 201); } -export const sessionChildSpawnRoutes: Route[] = defineRoutes(GITHUB_SANDBOX_FALLBACK_ROUTE, [ - sessionRoute({ - method: "POST", - path: "/sessions/:id/children", +export const sessionChildSpawnRoutes = new Hono(); + +sessionChildSpawnRoutes.post( + "/sessions/:id/children", + admit({ + ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requireAll( permissionRequirement("sessions.create"), permissionRequirement("sessions.collaborate") ), - handler: handleSpawnChild, }), -]); + (c) => dispatchSession(c, handleSpawnChild) +); diff --git a/packages/control-plane/src/routes/session-children.test.ts b/packages/control-plane/src/routes/session-children.test.ts index dae26b505..8a703a35d 100644 --- a/packages/control-plane/src/routes/session-children.test.ts +++ b/packages/control-plane/src/routes/session-children.test.ts @@ -13,10 +13,10 @@ vi.mock("../session/integration-settings-resolution", () => ({ resolveSandboxSettings: vi.fn(), })); -function routeMatch(path: string, pattern: string): RegExpMatchArray { +function routeMatch(path: string, pattern: string): { id: string; childId: string } { const match = path.match(routePathPattern(pattern)); - if (!match) throw new Error("Expected route match"); - return match; + if (!match?.groups?.id || !match.groups.childId) throw new Error("Expected route match"); + return { id: match.groups.id, childId: match.groups.childId }; } const defaultPromptAuthor: ActivePromptAuthor = { diff --git a/packages/control-plane/src/routes/session-children.ts b/packages/control-plane/src/routes/session-children.ts index 14fe3c065..54354248c 100644 --- a/packages/control-plane/src/routes/session-children.ts +++ b/packages/control-plane/src/routes/session-children.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { cancelChildSessionRequestSchema, childFollowUpPromptRequestSchema, @@ -12,7 +15,6 @@ import { resolveSandboxSettings } from "../session/integration-settings-resoluti import { activePromptAuthorSchema } from "../session/active-prompt-author"; import type { Env } from "../types"; import { - defineRoute, error, GITHUB_SANDBOX_FALLBACK_ROUTE, json, @@ -20,19 +22,18 @@ import { requirePermission, SCM_AGNOSTIC_SANDBOX_ROUTE, type RequestContext, - type Route, } from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-children"); -async function handleListChildren( +export async function handleListChildren( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const parentId = match.groups?.id; + const parentId = params.id; if (!parentId) return error("Parent session ID required"); const sessionStore = new SessionIndexStore(ctx.db); @@ -41,14 +42,14 @@ async function handleListChildren( return json({ children }); } -async function handleGetChild( +export async function handleGetChild( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; childId: string }, ctx: SessionRouteContext ): Promise { - const parentId = match.groups?.id; - const childId = match.groups?.childId; + const parentId = params.id; + const childId = params.childId; if (!parentId || !childId) return error("Parent and child session IDs required"); const sessionStore = new SessionIndexStore(ctx.db); @@ -69,11 +70,11 @@ async function handleGetChild( export async function handlePromptChild( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string; childId: string }, ctx: SessionRouteContext ): Promise { - const parentId = match.groups?.id; - const childId = match.groups?.childId; + const parentId = params.id; + const childId = params.childId; if (!parentId || !childId) return error("Parent and child session IDs required"); let rawBody: unknown; @@ -188,11 +189,11 @@ export async function handlePromptChild( export async function handleCancelChild( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; childId: string }, ctx: SessionRouteContext ): Promise { - const parentId = match.groups?.id; - const childId = match.groups?.childId; + const parentId = params.id; + const childId = params.childId; if (!parentId || !childId) return error("Parent and child session IDs required"); const sessionStore = new SessionIndexStore(ctx.db); @@ -260,38 +261,28 @@ export async function handleCancelChild( return response; } -export const sessionChildRoutes: Route[] = [ - defineRoute(GITHUB_SANDBOX_FALLBACK_ROUTE, { - method: "GET", - path: "/sessions/:id/children", - authorization: requirePermission("sessions.read"), - handler: handleListChildren, +export const sessionChildRoutes = new Hono(); + +sessionChildRoutes.get( + "/sessions/:id/children", + admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatch(c, handleListChildren) +); +sessionChildRoutes.get( + "/sessions/:id/children/:childId", + admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatchSession(c, handleGetChild) +); +sessionChildRoutes.post( + "/sessions/:id/children/:childId/cancel", + admit({ + ...GITHUB_SANDBOX_FALLBACK_ROUTE, + authorization: requirePermission("sessions.lifecycle"), }), - defineRoute( - GITHUB_SANDBOX_FALLBACK_ROUTE, - sessionRoute({ - method: "GET", - path: "/sessions/:id/children/:childId", - authorization: requirePermission("sessions.read"), - handler: handleGetChild, - }) - ), - defineRoute( - GITHUB_SANDBOX_FALLBACK_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/children/:childId/cancel", - authorization: requirePermission("sessions.lifecycle"), - handler: handleCancelChild, - }) - ), - defineRoute( - SCM_AGNOSTIC_SANDBOX_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/children/:childId/prompt", - authorization: NO_AUTHORIZATION, - handler: handlePromptChild, - }) - ), -]; + (c) => dispatchSession(c, handleCancelChild) +); +sessionChildRoutes.post( + "/sessions/:id/children/:childId/prompt", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => dispatchSession(c, handlePromptChild) +); diff --git a/packages/control-plane/src/routes/session-create.ts b/packages/control-plane/src/routes/session-create.ts index b746dbf24..82b47cbff 100644 --- a/packages/control-plane/src/routes/session-create.ts +++ b/packages/control-plane/src/routes/session-create.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import type { RepositoryRef, RepositoryPair } from "@open-inspect/shared/types/repositories"; import { getValidModelOrDefault, isValidReasoningEffort } from "@open-inspect/shared/models"; import type { CreateSessionResponse } from "@open-inspect/shared/types/session-api"; @@ -30,9 +33,7 @@ import { json, resolveRepoOrError, type RequestContext, - type Route, GITHUB_USER_OR_SERVICE_ROUTE, - defineRoutes, requirePermission, type ServiceActorClaimsResult, } from "./shared"; @@ -65,10 +66,9 @@ async function extractSessionActorProfileClaims( }; } -async function handleCreateSession( +export async function handleCreateSession( request: Request, env: Env, - _match: RegExpMatchArray, ctx: RequestContext ): Promise { const parsed = await parseCreateSessionInput(request); @@ -292,12 +292,14 @@ async function handleCreateSession( return json(result, 201); } -export const sessionCreateRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - { - method: "POST", - path: "/sessions", +export const sessionCreateRoutes = new Hono(); + +sessionCreateRoutes.post( + "/sessions", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.create"), serviceActorClaims: extractSessionActorProfileClaims, - handler: handleCreateSession, - }, -]); + }), + (c) => handleCreateSession(c.var.admitted.request, c.env, c.var.admitted.ctx) +); diff --git a/packages/control-plane/src/routes/session-diffs.ts b/packages/control-plane/src/routes/session-diffs.ts index c66b1adf5..609303cd4 100644 --- a/packages/control-plane/src/routes/session-diffs.ts +++ b/packages/control-plane/src/routes/session-diffs.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { SESSION_DIFF_FAILURE_BODY_MAX_BYTES, SESSION_DIFF_ID_PATTERN, @@ -7,20 +10,18 @@ import { } from "@open-inspect/shared/types/session-diffs"; import { SessionInternalPaths } from "../session/contracts"; import { - defineRoute, error, SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, requirePermission, - type Route, } from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; import type { Env } from "../types"; export const SESSION_DIFF_UPLOAD_BODY_MAX_BYTES = SESSION_DIFF_MAX_BUNDLE_BYTES; -function routeId(match: RegExpMatchArray, name: string): string | null { - const value = match.groups?.[name]; +function routeId(params: Record, name: string): string | null { + const value = params[name]; return value && SESSION_DIFF_ID_PATTERN.test(value) ? value : null; } @@ -77,13 +78,13 @@ async function runtimeJson( }); } -async function handleDiffState( +export async function handleDiffState( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required", 400); const response = await ctx.sessionRuntime.fetch(sessionId, SessionInternalPaths.diffState); if (!response.ok) { @@ -97,13 +98,13 @@ async function handleDiffState( }); } -async function handleDiffUpload( +export async function handleDiffUpload( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required", 400); const body = await readBoundedJson( request, @@ -117,13 +118,13 @@ async function handleDiffUpload( return new Response(response.body, { status: response.status, headers: response.headers }); } -async function handleDiffFailure( +export async function handleDiffFailure( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required", 400); const body = await readBoundedJson( request, @@ -137,15 +138,15 @@ async function handleDiffFailure( return new Response(response.body, { status: response.status, headers: response.headers }); } -async function handleDiffFile( +export async function handleDiffFile( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string; revisionId: string; fileId: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; - const revisionId = routeId(match, "revisionId"); - const fileId = routeId(match, "fileId"); + const sessionId = params.id; + const revisionId = routeId(params, "revisionId"); + const fileId = routeId(params, "fileId"); if (!sessionId || !revisionId || !fileId) return error("Invalid diff file identity", 400); const response = await ctx.sessionRuntime.fetch( sessionId, @@ -163,13 +164,13 @@ async function handleDiffFile( }); } -async function handleDiffRetry( +export async function handleDiffRetry( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required", 400); const response = await ctx.sessionRuntime.fetch(sessionId, SessionInternalPaths.diffRetry, { method: "POST", @@ -187,50 +188,32 @@ async function handleDiffRetry( * Only bundle upload and failure reporting additionally accept the per-session * sandbox token; the Session DO validates that token before these handlers run. */ -export const sessionDiffRoutes: Route[] = [ - defineRoute( - SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - sessionRoute({ - method: "GET", - path: "/sessions/:id/diff", - authorization: requirePermission("sessions.read"), - handler: handleDiffState, - }) - ), - defineRoute( - SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, - sessionRoute({ - method: "PUT", - path: "/sessions/:id/diff", - authorization: requirePermission("sessions.collaborate"), - handler: handleDiffUpload, - }) - ), - defineRoute( - SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/diff/failure", - authorization: requirePermission("sessions.collaborate"), - handler: handleDiffFailure, - }) - ), - defineRoute( - SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - sessionRoute({ - method: "GET", - path: "/sessions/:id/diff/:revisionId/files/:fileId", - authorization: requirePermission("sessions.read"), - handler: handleDiffFile, - }) - ), - defineRoute( - SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/diff/retry", - authorization: requirePermission("sessions.lifecycle"), - handler: handleDiffRetry, - }) - ), -]; +export const sessionDiffRoutes = new Hono(); + +const DIFF_READ = admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("sessions.read"), +}); +const DIFF_WRITE = admit({ + ...SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, + authorization: requirePermission("sessions.collaborate"), +}); + +sessionDiffRoutes.get("/sessions/:id/diff", DIFF_READ, (c) => dispatchSession(c, handleDiffState)); +sessionDiffRoutes.put("/sessions/:id/diff", DIFF_WRITE, (c) => + dispatchSession(c, handleDiffUpload) +); +sessionDiffRoutes.post("/sessions/:id/diff/failure", DIFF_WRITE, (c) => + dispatchSession(c, handleDiffFailure) +); +sessionDiffRoutes.get("/sessions/:id/diff/:revisionId/files/:fileId", DIFF_READ, (c) => + dispatchSession(c, handleDiffFile) +); +sessionDiffRoutes.post( + "/sessions/:id/diff/retry", + admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("sessions.lifecycle"), + }), + (c) => dispatchSession(c, handleDiffRetry) +); diff --git a/packages/control-plane/src/routes/session-index.test.ts b/packages/control-plane/src/routes/session-index.test.ts index 0dbb53cef..85812c4a1 100644 --- a/packages/control-plane/src/routes/session-index.test.ts +++ b/packages/control-plane/src/routes/session-index.test.ts @@ -1,10 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { sessionIndexRoutes } from "./session-index"; -import type { RequestContext } from "./shared"; +import { handleListSessions, handlePatchReadState } from "./session-index"; +import type { RequestContext, UserRouteContext } from "./shared"; import type { SqlDatabase } from "../db/sql-database"; import type { Env } from "../types"; import type { Principal } from "../auth/principal"; -import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; +import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; const mockSessionIndexStore = { list: vi.fn(), @@ -61,21 +61,10 @@ function createEnv(): Env { } as Env; } -function getHandler(method: string, path: string) { - for (const route of sessionIndexRoutes) { - if (route.method !== method) continue; - const match = path.match(routePathPattern(route.path)); - if (match) return { handler: route.handler, match }; - } - throw new Error(`No route found for ${method} ${path}`); -} - async function listSessions(query = "", principal?: Principal): Promise { - const { handler, match } = getHandler("GET", "/sessions"); - return handler( + return handleListSessions( new Request(`https://test.local/sessions${query}`), createEnv(), - match, createCtx(principal) ); } @@ -83,17 +72,16 @@ async function listSessions(query = "", principal?: Principal): Promise { - const { handler, match } = getHandler("PATCH", "/sessions/session-1/read-state"); - return handler( + return handlePatchReadState( new Request("https://test.local/sessions/session-1/read-state", { method: "PATCH", body, }), createEnv(), - matchOverride ?? match, - createCtx(principal) + paramsOverride ?? { id: "session-1" }, + createCtx(principal) as UserRouteContext ); } @@ -289,11 +277,10 @@ describe("session index routes", () => { }); it("requires a session ID for read-state mutations", async () => { - const { match } = getHandler("PATCH", "/sessions/session-1/read-state"); const response = await patchReadState( JSON.stringify({ action: "mark_latest_message_read" }), { kind: "user", userId: "user-1" }, - Object.assign(match, { groups: {} }) + { id: "" } ); expect(response.status).toBe(400); diff --git a/packages/control-plane/src/routes/session-index.ts b/packages/control-plane/src/routes/session-index.ts index d69c0359d..2718c24ba 100644 --- a/packages/control-plane/src/routes/session-index.ts +++ b/packages/control-plane/src/routes/session-index.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { parseSessionListQuery, SESSION_LIST_CURRENT_USER, @@ -13,14 +16,12 @@ import { isCanonicalUserId } from "@open-inspect/shared/user-id"; import { SessionIndexStore } from "../db/session-index"; import { error, - defineRoute, GITHUB_USER_OR_SERVICE_ROUTE, json, parseJsonBody, SCM_AGNOSTIC_HUMAN_USER_ROUTE, requirePermission, type RequestContext, - type Route, type UserRouteContext, } from "./shared"; import type { Env } from "../types"; @@ -53,10 +54,9 @@ function parseCreatedByFilters( return userIds; } -async function handleListSessions( +export async function handleListSessions( request: Request, env: Env, - _match: RegExpMatchArray, ctx: RequestContext ): Promise { const url = new URL(request.url); @@ -108,10 +108,9 @@ async function handleListSessions( return response; } -async function handleListSessionInbox( +export async function handleListSessionInbox( request: Request, _env: Env, - _match: RegExpMatchArray, ctx: UserRouteContext ): Promise { const searchParams = new URL(request.url).searchParams; @@ -187,13 +186,13 @@ function encodeInboxPage( }; } -async function handlePatchReadState( +export async function handlePatchReadState( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: UserRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); const unparsedBody = await parseJsonBody(request); @@ -220,13 +219,13 @@ async function handlePatchReadState( return response; } -async function handleDeleteSession( +export async function handleDeleteSession( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); const sessionStore = new SessionIndexStore(ctx.db); @@ -235,29 +234,28 @@ async function handleDeleteSession( return json({ status: "deleted", sessionId }); } -export const sessionIndexRoutes: Route[] = [ - defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { - method: "GET", - path: "/sessions", - authorization: requirePermission("sessions.read"), - handler: handleListSessions, - }), - defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { - method: "GET", - path: "/sessions/inbox", +export const sessionIndexRoutes = new Hono(); + +sessionIndexRoutes.get( + "/sessions", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => handleListSessions(c.var.admitted.request, c.env, c.var.admitted.ctx) +); +sessionIndexRoutes.get( + "/sessions/inbox", + admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read", { service: "deny" }), - handler: handleListSessionInbox, - }), - defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { - method: "PATCH", - path: "/sessions/:id/read-state", - authorization: requirePermission("sessions.read"), - handler: handlePatchReadState, - }), - defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { - method: "DELETE", - path: "/sessions/:id", - authorization: requirePermission("sessions.delete"), - handler: handleDeleteSession, }), -]; + (c) => handleListSessionInbox(c.var.admitted.request, c.env, c.var.admitted.ctx) +); +sessionIndexRoutes.patch( + "/sessions/:id/read-state", + admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatch(c, handlePatchReadState) +); +sessionIndexRoutes.delete( + "/sessions/:id", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.delete") }), + (c) => dispatch(c, handleDeleteSession) +); diff --git a/packages/control-plane/src/routes/session-media-stream.ts b/packages/control-plane/src/routes/session-media-stream.ts index f3b5e964f..cda2952e3 100644 --- a/packages/control-plane/src/routes/session-media-stream.ts +++ b/packages/control-plane/src/routes/session-media-stream.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { createLogger } from "../logger"; import { isSupportedScreenshotMimeType, isSupportedVideoMimeType } from "../media"; import type { NormalizedArtifactResponse } from "../session/artifacts"; @@ -10,14 +13,8 @@ import { createStoredObjectResponse, } from "./responses/stored-object-response"; import { getSessionArtifactFromRuntime } from "./session-media-artifacts"; -import { - defineRoutes, - error, - GITHUB_USER_OR_SERVICE_ROUTE, - requirePermission, - type Route, -} from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { error, GITHUB_USER_OR_SERVICE_ROUTE, requirePermission } from "./shared"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-media"); function getMediaMimeType( @@ -52,14 +49,14 @@ function resolveMediaContentType( return getMediaMimeType(artifact); } -async function handleMediaGet( +export async function handleMediaGet( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; artifactId: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; - const artifactId = match.groups?.artifactId; + const sessionId = params.id; + const artifactId = params.artifactId; if (!sessionId || !artifactId) { return error("Session ID and artifact ID are required", 400); } @@ -141,13 +138,15 @@ async function handleMediaGet( : createStoredObjectResponse(body, metadata, contentType); } -export const sessionMediaStreamRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - sessionRoute({ - method: "GET", - path: "/sessions/:id/media/:artifactId", +export const sessionMediaStreamRoutes = new Hono(); + +sessionMediaStreamRoutes.get( + "/sessions/:id/media/:artifactId", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read", { actorlessGrants: [{ service: "slack-bot" }], }), - handler: handleMediaGet, }), -]); + (c) => dispatchSession(c, handleMediaGet) +); diff --git a/packages/control-plane/src/routes/session-media-upload.ts b/packages/control-plane/src/routes/session-media-upload.ts index b4317c7d0..a69764939 100644 --- a/packages/control-plane/src/routes/session-media-upload.ts +++ b/packages/control-plane/src/routes/session-media-upload.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import type { ScreenshotArtifactMetadata, VideoArtifactMetadata, @@ -21,15 +24,8 @@ import { import { createMediaObjectStorage, type ObjectStorage } from "../storage/object-storage"; import type { Env } from "../types"; import { listSessionArtifactsFromRuntime, persistMediaArtifact } from "./session-media-artifacts"; -import { - defineRoutes, - error, - GITHUB_SANDBOX_FALLBACK_ROUTE, - json, - requirePermission, - type Route, -} from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { error, GITHUB_SANDBOX_FALLBACK_ROUTE, json, requirePermission } from "./shared"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; function getRequiredFormString(value: MultipartFieldValue | null, name: string): string | Response { if (typeof value !== "string" || value.trim().length === 0) { @@ -45,13 +41,13 @@ function getOptionalFormString(value: MultipartFieldValue | null): string | unde return trimmed.length > 0 ? trimmed : undefined; } -async function handleMediaUpload( +export async function handleMediaUpload( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); const storage = createMediaObjectStorage(env); @@ -246,11 +242,13 @@ async function handleVideoUpload(input: { return json({ artifactId, objectKey }, 201); } -export const sessionMediaUploadRoutes: Route[] = defineRoutes(GITHUB_SANDBOX_FALLBACK_ROUTE, [ - sessionRoute({ - method: "POST", - path: "/sessions/:id/media", +export const sessionMediaUploadRoutes = new Hono(); + +sessionMediaUploadRoutes.post( + "/sessions/:id/media", + admit({ + ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.collaborate"), - handler: handleMediaUpload, }), -]); + (c) => dispatchSession(c, handleMediaUpload) +); diff --git a/packages/control-plane/src/routes/session-media.ts b/packages/control-plane/src/routes/session-media.ts index fa735ac01..06df7b1e9 100644 --- a/packages/control-plane/src/routes/session-media.ts +++ b/packages/control-plane/src/routes/session-media.ts @@ -1,8 +1,8 @@ -import type { Route } from "./shared"; +import { Hono } from "hono"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { sessionMediaStreamRoutes } from "./session-media-stream"; import { sessionMediaUploadRoutes } from "./session-media-upload"; -export const sessionMediaRoutes: Route[] = [ - ...sessionMediaUploadRoutes, - ...sessionMediaStreamRoutes, -]; +export const sessionMediaRoutes = new Hono(); +sessionMediaRoutes.route("/", sessionMediaUploadRoutes); +sessionMediaRoutes.route("/", sessionMediaStreamRoutes); diff --git a/packages/control-plane/src/routes/session-prompt.ts b/packages/control-plane/src/routes/session-prompt.ts index 41072f227..1eb66bfeb 100644 --- a/packages/control-plane/src/routes/session-prompt.ts +++ b/packages/control-plane/src/routes/session-prompt.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { callbackContextSchema, sendPromptRequestSchema, @@ -24,14 +27,8 @@ import { type GitHubEnrichment, } from "../session/identity"; import type { Env } from "../types"; -import { - defineRoutes, - error, - GITHUB_USER_OR_SERVICE_ROUTE, - requirePermission, - type Route, -} from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { error, GITHUB_USER_OR_SERVICE_ROUTE, requirePermission } from "./shared"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-prompt"); @@ -50,13 +47,13 @@ function validateAttachments(raw: unknown): SessionAttachmentReference[] | Respo return result.data; } -async function handleSessionPrompt( +export async function handleSessionPrompt( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); let rawBody: unknown; @@ -180,11 +177,13 @@ async function handleSessionPrompt( return response; } -export const sessionPromptRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - sessionRoute({ - method: "POST", - path: "/sessions/:id/prompt", +export const sessionPromptRoutes = new Hono(); + +sessionPromptRoutes.post( + "/sessions/:id/prompt", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.collaborate"), - handler: handleSessionPrompt, }), -]); + (c) => dispatchSession(c, handleSessionPrompt) +); diff --git a/packages/control-plane/src/routes/session-pull-requests.ts b/packages/control-plane/src/routes/session-pull-requests.ts index f42b66ccf..4b858cef8 100644 --- a/packages/control-plane/src/routes/session-pull-requests.ts +++ b/packages/control-plane/src/routes/session-pull-requests.ts @@ -1,13 +1,10 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { SessionInternalPaths } from "../session/contracts"; import type { Env } from "../types"; -import { - defineRoutes, - error, - GITHUB_USER_OR_SERVICE_ROUTE, - requirePermission, - type Route, -} from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { error, GITHUB_USER_OR_SERVICE_ROUTE, requirePermission } from "./shared"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; /** * Manual PR sync (design §5.3): forwards to the session DO's internal @@ -15,13 +12,13 @@ import { sessionRoute, type SessionRouteContext } from "./session-route"; * immediately. Deliberately no session-index touch — PR changes must never * reorder the session list. */ -async function handleRefreshPullRequests( +export async function handleRefreshPullRequests( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); return ctx.sessionRuntime.fetch(sessionId, SessionInternalPaths.pullRequestsRefresh, { @@ -29,11 +26,13 @@ async function handleRefreshPullRequests( }); } -export const sessionPullRequestRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - sessionRoute({ - method: "POST", - path: "/sessions/:id/pull-requests/refresh", +export const sessionPullRequestRoutes = new Hono(); + +sessionPullRequestRoutes.post( + "/sessions/:id/pull-requests/refresh", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.lifecycle"), - handler: handleRefreshPullRequests, }), -]); + (c) => dispatchSession(c, handleRefreshPullRequests) +); diff --git a/packages/control-plane/src/routes/session-route.ts b/packages/control-plane/src/routes/session-route.ts index 88fabc47a..1b21c063e 100644 --- a/packages/control-plane/src/routes/session-route.ts +++ b/packages/control-plane/src/routes/session-route.ts @@ -1,29 +1,32 @@ -import type { SessionRuntimeClient } from "../session/runtime-client"; -import { createSessionRuntimeClient } from "../session/runtime-client"; +import type { Context as HonoContext } from "hono"; +import type { RequestContext } from "../http/request-context"; +import { dispatch, type HandlerEnv, type PathParams } from "../routing/admit"; +import { createSessionRuntimeClient, type SessionRuntimeClient } from "../session/runtime-client"; import type { Env } from "../types"; -import type { RequestContext, RouteDefinition } from "./shared"; export type SessionRouteContext = RequestContext & { sessionRuntime: SessionRuntimeClient; }; -export type SessionRouteHandler = ( - request: Request, +/** Give a session route's handler a runtime client bound to this request. */ +export function withSessionRuntime( env: Env, - match: RegExpMatchArray, - ctx: SessionRouteContext -) => Promise; - -function withSessionRuntime(handler: SessionRouteHandler): RouteDefinition["handler"] { - return (request, env, match, ctx) => - handler(request, env, match, { - ...ctx, - sessionRuntime: createSessionRuntimeClient(env, ctx), - }); + ctx: Context +): Context & { sessionRuntime: SessionRuntimeClient } { + return { ...ctx, sessionRuntime: createSessionRuntimeClient(env, ctx) }; } -export function sessionRoute( - route: Omit & { handler: SessionRouteHandler } -): RouteDefinition { - return { ...route, handler: withSessionRuntime(route.handler) }; +/** Run a session handler for an admitted request, with the runtime client bound to it. */ +export function dispatchSession( + c: HonoContext, Path>, + handler: ( + request: Request, + env: Env, + params: PathParams, + ctx: Context & { sessionRuntime: SessionRuntimeClient } + ) => Promise +): Promise { + return dispatch(c, (request, env, params, ctx) => + handler(request, env, params, withSessionRuntime(env, ctx)) + ); } diff --git a/packages/control-plane/src/routes/session-runtime-proxy.test.ts b/packages/control-plane/src/routes/session-runtime-proxy.test.ts index ee3f3e10f..e5f2472d7 100644 --- a/packages/control-plane/src/routes/session-runtime-proxy.test.ts +++ b/packages/control-plane/src/routes/session-runtime-proxy.test.ts @@ -1,42 +1,90 @@ -import { describe, expect, it, vi } from "vitest"; -import { SessionInternalPaths } from "../session/contracts"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { PermissionId } from "@open-inspect/shared/rbac"; -import type { RequestContext } from "./shared"; -import type { SqlDatabase } from "../db/sql-database"; -import { sessionRuntimeProxyRoutes } from "./session-runtime-proxy"; +import { BUILT_IN_ROLE_REGISTRY } from "@open-inspect/shared/rbac"; +import type * as AuthenticateModule from "../auth/authenticate"; +import type { Principal } from "../auth/principal"; +import type { SqlDatabase, SqlStatement } from "../db/sql-database"; +import { + createTestRequestHandler, + TEST_BACKGROUND_TASK_CONTEXT, + TEST_SERVICE_SECRETS, +} from "../router.test-support"; +import { SessionInternalPaths } from "../session/contracts"; import type { Env } from "../types"; -import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; +import { sessionRuntimeProxyRoutes } from "./session-runtime-proxy"; -function createCtx( - db: SqlDatabase = {} as SqlDatabase, - permissions: PermissionId[] = ["sessions.read"] -): RequestContext { +const mocks = vi.hoisted(() => ({ authenticate: vi.fn() })); + +vi.mock("../auth/authenticate", async (importOriginal) => ({ + ...(await importOriginal()), + authenticate: mocks.authenticate, +})); + +const handleRequest = createTestRequestHandler([sessionRuntimeProxyRoutes]); + +const USER: Principal = { kind: "user", userId: "user-1" }; +const JSON_HEADERS = { "Content-Type": "application/json" }; +const SANDBOX_HEADERS = { Authorization: "Bearer sandbox-token", "X-Sandbox-ID": "sandbox-1" }; +/** Sandbox-authenticated routes verify the bearer token against the runtime before proxying. */ +const SANDBOX_TOKEN_HEADERS = { Authorization: "Bearer sandbox-token" }; + +type DatabaseOptions = { + /** Custom-role grants for user-1; omitted means the owner role with every permission. */ + permissions?: PermissionId[]; + /** Answers every statement admission and the proxy's own reads do not own. */ + delegate?: SqlDatabase; +}; + +/** + * A database that answers admission's role lookup and the token-refresh + * binding read, handing anything else to the test's delegate. + */ +function createDatabase(options: DatabaseOptions = {}): SqlDatabase { + const role = options.permissions + ? { role_id: "role-1", role_key: null, role_name: "Viewer" } + : { role_id: BUILT_IN_ROLE_REGISTRY.owner.id, role_key: "owner", role_name: "Owner" }; + const rows = (sql: string): unknown[] | null => { + if (sql.includes("FROM role_permissions")) { + return (options.permissions ?? []).map((permission_id) => ({ permission_id })); + } + return null; + }; + const row = (sql: string): unknown => { + if (sql.includes("FROM users u")) return { user_id: "user-1", suspended_at: null, ...role }; + if (sql.includes("FROM session_model_provider_auth")) { + return { + provider: "openai", + auth_mode: "legacy_scoped_oauth", + provider_account_id: null, + selection_source: "explicit", + }; + } + return null; + }; return { - trace_id: "trace-1", - request_id: "req-1", - db, - executionCtx: TEST_BACKGROUND_TASK_CONTEXT, - principal: { - kind: "user", - userId: "user-1", - }, - authorization: { - userId: "user-1", - suspendedAt: null, - role: { id: "role-1", key: "viewer", name: "Viewer" }, - permissions, - }, - metrics: { - d1Queries: [], - spans: {}, - time: async (_name: string, fn: () => Promise) => fn(), - summarize: () => ({}), + prepare(sql: string) { + const owned = rows(sql) !== null || row(sql) !== null; + if (!owned && options.delegate) return options.delegate.prepare(sql); + const statement: SqlStatement = { + bind: () => statement, + first: async () => row(sql) as T | null, + all: async () => ({ results: (rows(sql) ?? []) as T[], meta: { changes: 0 } }), + run: async () => ({ results: [] as T[], meta: { changes: 0 } }), + }; + return statement; }, + batch: async (statements) => (options.delegate ? options.delegate.batch(statements) : []), }; } -function createEnv(fetch: (request: Request) => Promise): Env { +function createEnv( + fetch: (request: Request) => Promise, + database: DatabaseOptions = {} +): Env { return { + ...TEST_SERVICE_SECRETS, + SCM_PROVIDER: "github", + DB: createDatabase(database), SESSION: { idFromName: vi.fn((name: string) => `do-${name}`), get: vi.fn(() => ({ fetch })), @@ -44,30 +92,109 @@ function createEnv(fetch: (request: Request) => Promise): Env { } as unknown as Env; } -function getHandler(method: string, path: string) { - for (const route of sessionRuntimeProxyRoutes) { - if (route.method !== method) continue; - const match = path.match(routePathPattern(route.path)); - if (match) return { handler: route.handler, match, route }; - } - throw new Error(`No route found for ${method} ${path}`); +function dispatch(request: Request, env: Env): Promise { + return handleRequest(request, env, TEST_BACKGROUND_TASK_CONTEXT); +} + +function authenticateAs(principal: Principal): void { + mocks.authenticate.mockImplementation(async (request: Request) => ({ principal, request })); } describe("session runtime proxy routes", () => { + beforeEach(() => { + vi.clearAllMocks(); + authenticateAs(USER); + }); + + it.each([ + { method: "GET", path: "/sessions/session-1/sandbox-access", internal: "sandboxAccess" }, + { method: "GET", path: "/sessions/session-1", internal: "snapshot", status: 502 }, + { method: "POST", path: "/sessions/session-1/stop", internal: "stop" }, + { + method: "POST", + path: "/sessions/session-1/sandbox-error", + internal: "sandboxError", + init: { headers: SANDBOX_HEADERS, body: JSON.stringify({ error: "crash", fatal: true }) }, + }, + { method: "GET", path: "/sessions/session-1/events", internal: "events" }, + { method: "GET", path: "/sessions/session-1/artifacts", internal: "artifacts" }, + { method: "GET", path: "/sessions/session-1/participants", internal: "participants" }, + { + method: "GET", + path: "/sessions/session-1/participant-profiles", + internal: "participants", + status: 502, + }, + { method: "GET", path: "/sessions/session-1/messages", internal: "messages" }, + { + method: "POST", + path: "/sessions/session-1/pr", + internal: "createPr", + init: { headers: JSON_HEADERS, body: JSON.stringify({ title: "T", body: "B" }) }, + }, + { + method: "POST", + path: "/sessions/session-1/openai-token-refresh", + internal: "openaiTokenRefresh", + init: { headers: SANDBOX_TOKEN_HEADERS }, + sandbox: true, + }, + { + method: "POST", + path: "/sessions/session-1/xai-token-refresh", + internal: "xaiTokenRefresh", + init: { headers: SANDBOX_TOKEN_HEADERS }, + sandbox: true, + }, + { + method: "POST", + path: "/sessions/session-1/scm-credentials", + internal: "scmCredentials", + init: { headers: SANDBOX_TOKEN_HEADERS }, + sandbox: true, + }, + { method: "GET", path: "/sessions/session-1/tunnel-urls", internal: "tunnelUrls" }, + { + method: "PATCH", + path: "/sessions/session-1/title", + internal: "updateTitle", + init: { headers: JSON_HEADERS, body: JSON.stringify({ title: "New title" }) }, + }, + { method: "POST", path: "/sessions/session-1/archive", internal: "archive" }, + { method: "POST", path: "/sessions/session-1/unarchive", internal: "unarchive" }, + ] as const)( + "routes $method $path to the runtime's $internal path", + async ({ method, path, internal, ...options }) => { + const paths: string[] = []; + const fetch = vi.fn(async (request: Request) => { + paths.push(new URL(request.url).pathname); + return Response.json({ ok: true }); + }); + const init = "init" in options ? options.init : {}; + + const response = await dispatch( + new Request(`https://test.local${path}`, { method, ...init }), + createEnv(fetch) + ); + + // A handler that parses the runtime's answer rejects this stub body; + // the route is still proven to reach the expected internal path. + expect(response.status).toBe("status" in options ? options.status : 200); + const verified = "sandbox" in options ? [SessionInternalPaths.verifySandboxToken] : []; + expect(paths).toEqual([...verified, SessionInternalPaths[internal]]); + } + ); + it("forwards sandbox access for users", async () => { const requests: Request[] = []; const fetch = vi.fn(async (request: Request) => { requests.push(request); return Response.json({ sessionId: "session-1" }); }); - const path = "/sessions/session-1/sandbox-access"; - const { handler, match } = getHandler("GET", path); - - const response = await handler( - new Request(`https://test.local${path}`), - createEnv(fetch), - match, - createCtx() + + const response = await dispatch( + new Request("https://test.local/sessions/session-1/sandbox-access"), + createEnv(fetch) ); expect(response.status).toBe(200); @@ -106,14 +233,10 @@ describe("session runtime proxy routes", () => { timeline: { events: [], hasMore: false, cursor: null }, }) ); - const path = "/sessions/session-1"; - const { handler, match } = getHandler("GET", path); - - const response = await handler( - new Request(`https://test.local${path}`), - createEnv(fetch), - match, - createCtx({} as SqlDatabase, input.permissions) + + const response = await dispatch( + new Request("https://test.local/sessions/session-1"), + createEnv(fetch, { permissions: input.permissions }) ); const snapshot = (await response.json()) as { session: Record }; @@ -135,13 +258,10 @@ describe("session runtime proxy routes", () => { requests.push(request); return Response.json({ events: [] }); }); - const { handler, match } = getHandler("GET", "/sessions/session-1/events"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/events?limit=10"), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); await expect(response.json()).resolves.toEqual({ events: [] }); @@ -156,26 +276,19 @@ describe("session runtime proxy routes", () => { requests.push(request); return Response.json({ status: "ok" }); }); - const path = "/sessions/session-1/sandbox-error"; - const { handler, match, route } = getHandler("POST", path); - const response = await handler( - new Request(`https://test.local${path}`, { + const response = await dispatch( + new Request("https://test.local/sessions/session-1/sandbox-error", { method: "POST", - headers: { - "content-type": "application/json", - Authorization: "Bearer sandbox-token", - "X-Sandbox-ID": "sandbox-1", - }, + headers: { "content-type": "application/json", ...SANDBOX_HEADERS }, body: JSON.stringify({ error: "Bridge repeatedly crashed", fatal: true }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(200); - expect(route.authentication.kind).toBe("handler-authenticated"); + // The route authenticates the sandbox itself; admission never asks. + expect(mocks.authenticate).not.toHaveBeenCalled(); expect(new URL(requests[0].url).pathname).toBe(SessionInternalPaths.sandboxError); expect(requests[0].headers.get("Authorization")).toBe("Bearer sandbox-token"); expect(requests[0].headers.get("X-Sandbox-ID")).toBe("sandbox-1"); @@ -187,21 +300,14 @@ describe("session runtime proxy routes", () => { it("rejects oversized sandbox errors before forwarding them", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); - const path = "/sessions/session-1/sandbox-error"; - const { handler, match } = getHandler("POST", path); - const response = await handler( - new Request(`https://test.local${path}`, { + const response = await dispatch( + new Request("https://test.local/sessions/session-1/sandbox-error", { method: "POST", - headers: { - Authorization: "Bearer sandbox-token", - "X-Sandbox-ID": "sandbox-1", - }, + headers: SANDBOX_HEADERS, body: "x".repeat(2049), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(413); @@ -210,14 +316,13 @@ describe("session runtime proxy routes", () => { it("rejects missing sandbox credentials before reading or forwarding the body", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); - const path = "/sessions/session-1/sandbox-error"; - const { handler, match } = getHandler("POST", path); - - const response = await handler( - new Request(`https://test.local${path}`, { method: "POST", body: "not json" }), - createEnv(fetch), - match, - createCtx() + + const response = await dispatch( + new Request("https://test.local/sessions/session-1/sandbox-error", { + method: "POST", + body: "not json", + }), + createEnv(fetch) ); expect(response.status).toBe(401); @@ -226,20 +331,13 @@ describe("session runtime proxy routes", () => { it("rejects an empty sandbox error before forwarding it", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); - const path = "/sessions/session-1/sandbox-error"; - const { handler, match } = getHandler("POST", path); - const response = await handler( - new Request(`https://test.local${path}`, { + const response = await dispatch( + new Request("https://test.local/sessions/session-1/sandbox-error", { method: "POST", - headers: { - Authorization: "Bearer sandbox-token", - "X-Sandbox-ID": "sandbox-1", - }, + headers: SANDBOX_HEADERS, }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(400); @@ -295,13 +393,10 @@ describe("session runtime proxy routes", () => { }, ]), } as unknown as SqlDatabase; - const { handler, match } = getHandler("GET", "/sessions/session-1/participant-profiles"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/participant-profiles"), - createEnv(fetch), - match, - createCtx(db) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(200); @@ -333,13 +428,10 @@ describe("session runtime proxy routes", () => { Response.json({ participants: [{ canonicalUserId: "user-1" }] }) ); const db = { prepare: vi.fn(), batch: vi.fn() } as unknown as SqlDatabase; - const { handler, match } = getHandler("GET", "/sessions/session-1/participant-profiles"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/participant-profiles"), - createEnv(fetch), - match, - createCtx(db) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(502); @@ -350,13 +442,10 @@ describe("session runtime proxy routes", () => { it("returns a bad-gateway error when the participant response is not JSON", async () => { const fetch = vi.fn(async () => new Response("not json", { status: 200 })); const db = { prepare: vi.fn(), batch: vi.fn() } as unknown as SqlDatabase; - const { handler, match } = getHandler("GET", "/sessions/session-1/participant-profiles"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/participant-profiles"), - createEnv(fetch), - match, - createCtx(db) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(502); @@ -367,13 +456,10 @@ describe("session runtime proxy routes", () => { it("preserves participant runtime errors without querying profiles", async () => { const fetch = vi.fn(async () => Response.json({ error: "missing" }, { status: 404 })); const db = { prepare: vi.fn(), batch: vi.fn() } as unknown as SqlDatabase; - const { handler, match } = getHandler("GET", "/sessions/session-1/participant-profiles"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/participant-profiles"), - createEnv(fetch), - match, - createCtx(db) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(404); @@ -386,17 +472,14 @@ describe("session runtime proxy routes", () => { requests.push(request); return Response.json({ status: "updated" }); }); - const { handler, match } = getHandler("PATCH", "/sessions/session-1/title"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/title", { method: "PATCH", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ title: "New title" }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); await expect(response.json()).resolves.toEqual({ status: "updated" }); @@ -414,9 +497,7 @@ describe("session runtime proxy routes", () => { requests.push(request); return Response.json({ status: "updated" }); }); - const { handler, match } = getHandler("PATCH", "/sessions/session-1/title"); - const ctx = createCtx(); - ctx.principal = { + authenticateAs({ kind: "service", service: "slack-bot", actor: { @@ -425,17 +506,15 @@ describe("session runtime proxy routes", () => { canonicalUserId: "user-1", participantUserId: "slack:U0123", }, - }; + }); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/title", { method: "PATCH", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ title: "New title" }), }), - createEnv(fetch), - match, - ctx + createEnv(fetch) ); expect(response.status).toBe(200); @@ -447,17 +526,14 @@ describe("session runtime proxy routes", () => { it("rejects a caller-asserted title-update userId without forwarding to the runtime", async () => { const fetch = vi.fn(async () => Response.json({ status: "updated" })); - const { handler, match } = getHandler("PATCH", "/sessions/session-1/title"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/title", { method: "PATCH", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ userId: "someone-else", title: "New title" }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(400); @@ -469,13 +545,10 @@ describe("session runtime proxy routes", () => { it("only rewrites runtime 404 responses to the configured not-found response", async () => { const fetch = vi.fn(async () => Response.json({ error: "runtime failed" }, { status: 500 })); - const { handler, match } = getHandler("GET", "/sessions/session-1"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1"), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(500); @@ -484,13 +557,10 @@ describe("session runtime proxy routes", () => { it("maps runtime 404 responses to the configured not-found response", async () => { const fetch = vi.fn(async () => Response.json({ error: "missing" }, { status: 404 })); - const { handler, match } = getHandler("GET", "/sessions/session-1"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1"), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(404); @@ -503,17 +573,14 @@ describe("session runtime proxy routes", () => { requests.push(request); return Response.json({ prNumber: 1, prUrl: "https://example/pr/1", state: "draft" }); }); - const { handler, match } = getHandler("POST", "/sessions/session-1/pr"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/pr", { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ title: "T", body: "B", draft: true }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(200); @@ -524,17 +591,14 @@ describe("session runtime proxy routes", () => { it("rejects a non-boolean draft without forwarding to the runtime", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); - const { handler, match } = getHandler("POST", "/sessions/session-1/pr"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/pr", { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ title: "T", body: "B", draft: "yes" }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(400); @@ -544,17 +608,14 @@ describe("session runtime proxy routes", () => { it("rejects malformed create-PR JSON without forwarding to the runtime", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); - const { handler, match } = getHandler("POST", "/sessions/session-1/pr"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/pr", { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: "{", }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(400); @@ -568,12 +629,11 @@ describe("session runtime proxy routes", () => { requests.push(request); return Response.json({ prNumber: 7 }); }); - const { handler, match } = getHandler("POST", "/sessions/session-1/pr"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/pr", { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ title: "PR", body: "desc", @@ -583,9 +643,7 @@ describe("session runtime proxy routes", () => { repoName: "backend", }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(200); @@ -602,17 +660,14 @@ describe("session runtime proxy routes", () => { it("rejects a non-string create-PR repo target without forwarding", async () => { const fetch = vi.fn(async () => Response.json({ status: "ok" })); - const { handler, match } = getHandler("POST", "/sessions/session-1/pr"); - const response = await handler( + const response = await dispatch( new Request("https://test.local/sessions/session-1/pr", { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: JSON_HEADERS, body: JSON.stringify({ title: "PR", body: "desc", repoOwner: 42, repoName: "backend" }), }), - createEnv(fetch), - match, - createCtx() + createEnv(fetch) ); expect(response.status).toBe(400); diff --git a/packages/control-plane/src/routes/session-runtime-proxy.ts b/packages/control-plane/src/routes/session-runtime-proxy.ts index 39278cb60..d4e4c0b3a 100644 --- a/packages/control-plane/src/routes/session-runtime-proxy.ts +++ b/packages/control-plane/src/routes/session-runtime-proxy.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { readBodyCapped } from "@open-inspect/shared/http-body"; import type { SessionParticipantProfilesResponse, @@ -14,7 +17,6 @@ import type { SubscriptionProviderId } from "@open-inspect/shared/types/provider import { SessionInternalPaths, type SessionInternalPath } from "../session/contracts"; import type { Env } from "../types"; import { - defineRoute, error, GITHUB_SANDBOX_FALLBACK_ROUTE, GITHUB_USER_OR_SERVICE_ROUTE, @@ -27,11 +29,8 @@ import { SCM_AGNOSTIC_HUMAN_USER_ROUTE, SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, SCM_CREDENTIALS_ROUTE, - type Route, - type RouteAuthorization, - type RoutePolicy, } from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const participantsResponseSchema = z.object({ participants: z.array( @@ -44,89 +43,66 @@ const participantsResponseSchema = z.object({ const SANDBOX_ERROR_BODY_MAX_BYTES = 2 * 1024; -type SimpleProxyRouteConfig = { - policy: RoutePolicy; - method: string; - routePath: string; +type SessionParams = { id: string }; +type ProxyHandler = ( + request: Request, + env: Env, + params: SessionParams, + ctx: SessionRouteContext +) => Promise; + +type SimpleProxyConfig = { internalPath: SessionInternalPath; - authorization: RouteAuthorization; runtimeMethod?: string; forwardSearch?: boolean; notFoundMessage?: string; }; -function getSessionId(match: RegExpMatchArray): string | Response { - const sessionId = match.groups?.id; - return sessionId ? sessionId : error("Session ID required"); -} - function isObjectBody(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -function simpleProxyRoute(config: SimpleProxyRouteConfig): Route { - return defineRoute( - config.policy, - sessionRoute({ - method: config.method, - path: config.routePath, - authorization: config.authorization, - handler: async (request, _env, match, ctx) => { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; - - const response = await ctx.sessionRuntime.fetch( - sessionId, - config.internalPath, - config.runtimeMethod ? { method: config.runtimeMethod } : undefined, - config.forwardSearch ? new URL(request.url).search : undefined - ); - - if (config.notFoundMessage && response.status === 404) { - return error(config.notFoundMessage, 404); - } - - return response; - }, - }) - ); +/** Forward the request to one internal session path and relay the runtime's answer. */ +function simpleProxy(config: SimpleProxyConfig): ProxyHandler { + return async (request, _env, params, ctx) => { + const response = await ctx.sessionRuntime.fetch( + params.id, + config.internalPath, + config.runtimeMethod ? { method: config.runtimeMethod } : undefined, + config.forwardSearch ? new URL(request.url).search : undefined + ); + + if (config.notFoundMessage && response.status === 404) { + return error(config.notFoundMessage, 404); + } + + return response; + }; } -function legacyTokenRefreshRoute( +function legacyTokenRefresh( provider: SubscriptionProviderId, - routePath: string, internalPath: SessionInternalPath -): Route { - return defineRoute( - SCM_AGNOSTIC_SANDBOX_ROUTE, - sessionRoute({ - method: "POST", - path: routePath, - authorization: NO_AUTHORIZATION, - handler: async (_request, _env, match, ctx) => { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; - const binding = await new SessionIndexStore(ctx.db).getProviderAuthForProvider( - sessionId, - provider - ); - if (binding?.authMode !== "legacy_scoped_oauth") { - return error("Session does not use legacy scoped OAuth for this provider", 409); - } - return ctx.sessionRuntime.fetch(sessionId, internalPath, { method: "POST" }); - }, - }) - ); +): ProxyHandler { + return async (_request, _env, params, ctx) => { + const binding = await new SessionIndexStore(ctx.db).getProviderAuthForProvider( + params.id, + provider + ); + if (binding?.authMode !== "legacy_scoped_oauth") { + return error("Session does not use legacy scoped OAuth for this provider", 409); + } + return ctx.sessionRuntime.fetch(params.id, internalPath, { method: "POST" }); + }; } async function handleSandboxError( request: Request, _env: Env, - match: RegExpMatchArray, + params: SessionParams, ctx: SessionRouteContext ): Promise { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; + const sessionId = params.id; const authorization = request.headers.get("Authorization"); const sandboxId = request.headers.get("X-Sandbox-ID"); if (!authorization?.startsWith("Bearer ") || !sandboxId) { @@ -150,11 +126,10 @@ async function handleSandboxError( async function handleParticipantProfiles( _request: Request, _env: Env, - match: RegExpMatchArray, + params: SessionParams, ctx: SessionRouteContext ): Promise { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; + const sessionId = params.id; const participantsResponse = await ctx.sessionRuntime.fetch( sessionId, @@ -187,11 +162,10 @@ async function handleParticipantProfiles( async function handleSessionSnapshot( _request: Request, _env: Env, - match: RegExpMatchArray, + params: SessionParams, ctx: SessionRouteContext ): Promise { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; + const sessionId = params.id; const response = await ctx.sessionRuntime.fetch(sessionId, SessionInternalPaths.snapshot); if (response.status === 404) return error("Session not found", 404); @@ -210,11 +184,10 @@ async function handleSessionSnapshot( async function handleCreatePR( request: Request, _env: Env, - match: RegExpMatchArray, + params: SessionParams, ctx: SessionRouteContext ): Promise { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; + const sessionId = params.id; const body = await parseJsonBody(request); if (body instanceof Response) return body; @@ -283,153 +256,154 @@ async function readTitleBody(request: Request): Promise<{ title?: string; reject return { title: body.title }; } -function lifecycleProxyRoute( - method: string, - routePath: string, - internalPath: SessionInternalPath -): Route { - return defineRoute( - GITHUB_USER_OR_SERVICE_ROUTE, - sessionRoute({ - method, - path: routePath, - authorization: requirePermission("sessions.lifecycle"), - handler: async (request, _env, match, ctx) => { - const sessionId = getSessionId(match); - if (sessionId instanceof Response) return sessionId; - - let body = {}; - if (internalPath === SessionInternalPaths.updateTitle) { - const { title, rejection } = await readTitleBody(request); - if (rejection) return rejection; - body = { title }; - } - - return ctx.sessionRuntime.fetch(sessionId, internalPath, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(body), - }); - }, - }) - ); +function lifecycleProxy(internalPath: SessionInternalPath): ProxyHandler { + return async (request, _env, params, ctx) => { + let body = {}; + if (internalPath === SessionInternalPaths.updateTitle) { + const { title, rejection } = await readTitleBody(request); + if (rejection) return rejection; + body = { title }; + } + + return ctx.sessionRuntime.fetch(params.id, internalPath, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + }; } -export const sessionRuntimeProxyRoutes: Route[] = [ - simpleProxyRoute({ - policy: SCM_AGNOSTIC_HUMAN_USER_ROUTE, - method: "GET", - routePath: "/sessions/:id/sandbox-access", - internalPath: SessionInternalPaths.sandboxAccess, +/** Every proxied session operation, by the name its route is known by. */ +const LIFECYCLE = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("sessions.lifecycle"), +}); + +export const sessionRuntimeProxyRoutes = new Hono(); + +sessionRuntimeProxyRoutes.get( + "/sessions/:id/sandbox-access", + admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.sandbox_access"), }), - defineRoute( - SCM_AGNOSTIC_HUMAN_USER_ROUTE, - sessionRoute({ - method: "GET", - path: "/sessions/:id", - authorization: requirePermission("sessions.read"), - handler: handleSessionSnapshot, - }) - ), - simpleProxyRoute({ - policy: GITHUB_USER_OR_SERVICE_ROUTE, - method: "POST", - routePath: "/sessions/:id/stop", - internalPath: SessionInternalPaths.stop, + (c) => dispatchSession(c, simpleProxy({ internalPath: SessionInternalPaths.sandboxAccess })) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id", + admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatchSession(c, handleSessionSnapshot) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/stop", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.lifecycle", { actorlessGrants: [{ service: "linear-bot" }], }), - runtimeMethod: "POST", }), - defineRoute( - SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/sandbox-error", - authorization: NO_AUTHORIZATION, - handler: handleSandboxError, - }) - ), - simpleProxyRoute({ - policy: GITHUB_USER_OR_SERVICE_ROUTE, - method: "GET", - routePath: "/sessions/:id/events", - internalPath: SessionInternalPaths.events, + (c) => + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.stop, runtimeMethod: "POST" }) + ) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/sandbox-error", + admit({ ...SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => dispatchSession(c, handleSandboxError) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/events", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read", { actorlessGrants: [{ service: "slack-bot" }, { service: "linear-bot" }], }), - forwardSearch: true, }), - simpleProxyRoute({ - policy: GITHUB_USER_OR_SERVICE_ROUTE, - method: "GET", - routePath: "/sessions/:id/artifacts", - internalPath: SessionInternalPaths.artifacts, + (c) => + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.events, forwardSearch: true }) + ) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/artifacts", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read", { actorlessGrants: [{ service: "slack-bot" }, { service: "linear-bot" }], }), }), - simpleProxyRoute({ - policy: GITHUB_USER_OR_SERVICE_ROUTE, - method: "GET", - routePath: "/sessions/:id/participants", - internalPath: SessionInternalPaths.participants, + (c) => dispatchSession(c, simpleProxy({ internalPath: SessionInternalPaths.artifacts })) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/participants", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatchSession(c, simpleProxy({ internalPath: SessionInternalPaths.participants })) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/participant-profiles", + admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read"), }), - defineRoute( - SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - sessionRoute({ - method: "GET", - path: "/sessions/:id/participant-profiles", - authorization: requirePermission("sessions.read"), - handler: handleParticipantProfiles, - }) - ), - simpleProxyRoute({ - policy: GITHUB_USER_OR_SERVICE_ROUTE, - method: "GET", - routePath: "/sessions/:id/messages", - internalPath: SessionInternalPaths.messages, - authorization: requirePermission("sessions.read"), - forwardSearch: true, - }), - defineRoute( - GITHUB_SANDBOX_FALLBACK_ROUTE, - sessionRoute({ - method: "POST", - path: "/sessions/:id/pr", - authorization: requirePermission("sessions.collaborate"), - handler: handleCreatePR, - }) - ), - legacyTokenRefreshRoute( - "openai", - "/sessions/:id/openai-token-refresh", - SessionInternalPaths.openaiTokenRefresh - ), - legacyTokenRefreshRoute( - "xai", - "/sessions/:id/xai-token-refresh", - SessionInternalPaths.xaiTokenRefresh - ), - simpleProxyRoute({ - policy: SCM_CREDENTIALS_ROUTE, - method: "POST", - routePath: "/sessions/:id/scm-credentials", - internalPath: SessionInternalPaths.scmCredentials, - authorization: NO_AUTHORIZATION, - runtimeMethod: "POST", + (c) => dispatchSession(c, handleParticipantProfiles) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/messages", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.messages, forwardSearch: true }) + ) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/pr", + admit({ + ...GITHUB_SANDBOX_FALLBACK_ROUTE, + authorization: requirePermission("sessions.collaborate"), }), - simpleProxyRoute({ - policy: SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, - method: "GET", - routePath: "/sessions/:id/tunnel-urls", - internalPath: SessionInternalPaths.tunnelUrls, + (c) => dispatchSession(c, handleCreatePR) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/openai-token-refresh", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => dispatchSession(c, legacyTokenRefresh("openai", SessionInternalPaths.openaiTokenRefresh)) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/xai-token-refresh", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => dispatchSession(c, legacyTokenRefresh("xai", SessionInternalPaths.xaiTokenRefresh)) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/scm-credentials", + admit({ ...SCM_CREDENTIALS_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.scmCredentials, runtimeMethod: "POST" }) + ) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/tunnel-urls", + admit({ + ...SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.sandbox_access"), - runtimeMethod: "GET", }), - lifecycleProxyRoute("PATCH", "/sessions/:id/title", SessionInternalPaths.updateTitle), - lifecycleProxyRoute("POST", "/sessions/:id/archive", SessionInternalPaths.archive), - lifecycleProxyRoute("POST", "/sessions/:id/unarchive", SessionInternalPaths.unarchive), -]; + (c) => + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.tunnelUrls, runtimeMethod: "GET" }) + ) +); +sessionRuntimeProxyRoutes.patch("/sessions/:id/title", LIFECYCLE, (c) => + dispatchSession(c, lifecycleProxy(SessionInternalPaths.updateTitle)) +); +sessionRuntimeProxyRoutes.post("/sessions/:id/archive", LIFECYCLE, (c) => + dispatchSession(c, lifecycleProxy(SessionInternalPaths.archive)) +); +sessionRuntimeProxyRoutes.post("/sessions/:id/unarchive", LIFECYCLE, (c) => + dispatchSession(c, lifecycleProxy(SessionInternalPaths.unarchive)) +); diff --git a/packages/control-plane/src/routes/session-skills.ts b/packages/control-plane/src/routes/session-skills.ts index ceb48d812..0a16d7bda 100644 --- a/packages/control-plane/src/routes/session-skills.ts +++ b/packages/control-plane/src/routes/session-skills.ts @@ -1,8 +1,10 @@ +import { Hono } from "hono"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { MAX_SANDBOX_SKILL_PAGE_SIZE } from "@open-inspect/shared/types/skills"; import { SessionSkillStore } from "../db/session-skills"; import type { Env } from "../types"; import { - defineRoute, error, json, NO_AUTHORIZATION, @@ -10,22 +12,16 @@ import { SCM_AGNOSTIC_SANDBOX_ROUTE, SCM_AGNOSTIC_HUMAN_USER_ROUTE, type SandboxRouteContext, - type Route, type UserRouteContext, } from "./shared"; -function sessionId(match: RegExpMatchArray): string | Response { - return match.groups?.id ?? error("Session ID required", 400); -} - -async function handleSessionSkillsView( +export async function handleSessionSkillsView( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: UserRouteContext ): Promise { - const id = sessionId(match); - if (id instanceof Response) return id; + const id = params.id; const view = await new SessionSkillStore(ctx.db).getSessionSkillsView(id); if (!view) return error("Session skill manifest not found", 404); const response = json(view); @@ -53,14 +49,13 @@ function installationPage(request: Request): { after: number; limit: number } | return { after, limit }; } -async function handleSandboxInstallation( +export async function handleSandboxInstallation( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SandboxRouteContext ): Promise { - const id = sessionId(match); - if (id instanceof Response) return id; + const id = params.id; const page = installationPage(request); if (page instanceof Response) return page; const manifest = await new SessionSkillStore(ctx.db).getSandboxInstallation( @@ -76,17 +71,16 @@ async function handleSandboxInstallation( return response; } -export const sessionSkillRoutes: Route[] = [ - defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { - method: "GET", - path: "/sessions/:id/skills", - authorization: requirePermission("sessions.read"), - handler: handleSessionSkillsView, - }), - defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { - method: "GET", - path: "/sessions/:id/sandbox-skills", - authorization: NO_AUTHORIZATION, - handler: handleSandboxInstallation, - }), -]; +export const sessionSkillRoutes = new Hono(); + +sessionSkillRoutes.get( + "/sessions/:id/skills", + admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => dispatch(c, handleSessionSkillsView) +); + +sessionSkillRoutes.get( + "/sessions/:id/sandbox-skills", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => dispatch(c, handleSandboxInstallation) +); diff --git a/packages/control-plane/src/routes/session-ws-token.test.ts b/packages/control-plane/src/routes/session-ws-token.test.ts index 9720d30cf..2cff57d45 100644 --- a/packages/control-plane/src/routes/session-ws-token.test.ts +++ b/packages/control-plane/src/routes/session-ws-token.test.ts @@ -1,18 +1,15 @@ import { describe, expect, it, vi } from "vitest"; import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; -import { sessionWsTokenRoutes } from "./session-ws-token"; -import type { RequestContext, Route } from "./shared"; +import { handleSessionWsToken } from "./session-ws-token"; +import type { RequestContext } from "./shared"; import type { Env } from "../types"; import type { SqlDatabase } from "../db/sql-database"; +import { withSessionRuntime } from "./session-route"; -function routeFor(path: string): { route: Route; match: RegExpMatchArray } { - const route = sessionWsTokenRoutes.find((candidate) => - routePathPattern(candidate.path).test(path) - ); - if (!route) throw new Error(`route not found: ${path}`); - const match = path.match(routePathPattern(route.path)); - if (!match) throw new Error(`path did not match: ${path}`); - return { route, match }; +function routeFor(path: string): { handler: typeof handleSessionWsToken; params: { id: string } } { + const match = path.match(routePathPattern("/sessions/:id/ws-token")); + if (!match?.groups?.id) throw new Error(`path did not match: ${path}`); + return { handler: handleSessionWsToken, params: { id: match.groups.id } }; } function accessDatabase() { @@ -66,9 +63,9 @@ describe("session ws-token route", () => { forwarded.push(request); return Response.json({ token: "token-1" }); }); - const { route, match } = routeFor("/sessions/session-1/ws-token"); + const { handler, params } = routeFor("/sessions/session-1/ws-token"); - const response = await route.handler( + const response = await handler( new Request("https://test.local/sessions/session-1/ws-token", { method: "POST", body: JSON.stringify({ @@ -78,8 +75,8 @@ describe("session ws-token route", () => { }), }), createEnv(fetch), - match, - createContext() + params, + withSessionRuntime(createEnv(fetch), createContext()) ); expect(response.status).toBe(200); @@ -96,16 +93,16 @@ describe("session ws-token route", () => { it("forwards a runtime rejection without writing D1", async () => { const access = accessDatabase(); const fetch = vi.fn(async () => Response.json({ error: "rejected" }, { status: 409 })); - const { route, match } = routeFor("/sessions/session-1/ws-token"); + const { handler, params } = routeFor("/sessions/session-1/ws-token"); - const response = await route.handler( + const response = await handler( new Request("https://test.local/sessions/session-1/ws-token", { method: "POST", body: JSON.stringify({}), }), createEnv(fetch), - match, - createContext(access.db) + params, + withSessionRuntime(createEnv(fetch), createContext(access.db)) ); expect(response.status).toBe(409); @@ -118,16 +115,16 @@ describe("session ws-token route", () => { forwarded.push(request); return Response.json({ token: "token-1" }); }); - const { route, match } = routeFor("/sessions/session-1/ws-token"); + const { handler, params } = routeFor("/sessions/session-1/ws-token"); - const response = await route.handler( + const response = await handler( new Request("https://test.local/sessions/session-1/ws-token", { method: "POST", body: JSON.stringify({ scmLogin: null, scmName: null, scmEmail: null }), }), createEnv(fetch), - match, - createContext() + params, + withSessionRuntime(createEnv(fetch), createContext()) ); expect(response.status).toBe(200); @@ -141,16 +138,16 @@ describe("session ws-token route", () => { it("rejects malformed optional SCM display fields", async () => { const fetch = vi.fn(async () => Response.json({ token: "token-1" })); - const { route, match } = routeFor("/sessions/session-1/ws-token"); + const { handler, params } = routeFor("/sessions/session-1/ws-token"); - const response = await route.handler( + const response = await handler( new Request("https://test.local/sessions/session-1/ws-token", { method: "POST", body: JSON.stringify({ scmLogin: 123 }), }), createEnv(fetch), - match, - createContext() + params, + withSessionRuntime(createEnv(fetch), createContext()) ); expect(response.status).toBe(400); @@ -160,16 +157,16 @@ describe("session ws-token route", () => { it("still rejects forbidden identity fields before schema stripping", async () => { const fetch = vi.fn(async () => Response.json({ token: "token-1" })); - const { route, match } = routeFor("/sessions/session-1/ws-token"); + const { handler, params } = routeFor("/sessions/session-1/ws-token"); - const response = await route.handler( + const response = await handler( new Request("https://test.local/sessions/session-1/ws-token", { method: "POST", body: JSON.stringify({ userId: "attacker" }), }), createEnv(fetch), - match, - createContext() + params, + withSessionRuntime(createEnv(fetch), createContext()) ); expect(response.status).toBe(400); diff --git a/packages/control-plane/src/routes/session-ws-token.ts b/packages/control-plane/src/routes/session-ws-token.ts index 58ce8d1de..5b01b84b8 100644 --- a/packages/control-plane/src/routes/session-ws-token.ts +++ b/packages/control-plane/src/routes/session-ws-token.ts @@ -1,24 +1,20 @@ +import { Hono } from "hono"; +import { admit } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { applyIdentityEnforcement } from "../routing/identity-enforcement"; import { SESSION_WEBSOCKET_CONNECT_PERMISSION } from "@open-inspect/shared/rbac"; import { SessionInternalPaths, sessionScmDisplayFieldsSchema } from "../session/contracts"; import type { Env } from "../types"; -import { - defineRoutes, - error, - GITHUB_USER_OR_SERVICE_ROUTE, - parseJsonBody, - requirePermission, - type Route, -} from "./shared"; -import { sessionRoute, type SessionRouteContext } from "./session-route"; +import { error, GITHUB_USER_OR_SERVICE_ROUTE, parseJsonBody, requirePermission } from "./shared"; +import { dispatchSession, type SessionRouteContext } from "./session-route"; -async function handleSessionWsToken( +export async function handleSessionWsToken( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: SessionRouteContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required"); const rawBody = await parseJsonBody(request); @@ -54,11 +50,13 @@ async function handleSessionWsToken( ); } -export const sessionWsTokenRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - sessionRoute({ - method: "POST", - path: "/sessions/:id/ws-token", +export const sessionWsTokenRoutes = new Hono(); + +sessionWsTokenRoutes.post( + "/sessions/:id/ws-token", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission(SESSION_WEBSOCKET_CONNECT_PERMISSION), - handler: handleSessionWsToken, }), -]); + (c) => dispatchSession(c, handleSessionWsToken) +); diff --git a/packages/control-plane/src/routes/sessions.ts b/packages/control-plane/src/routes/sessions.ts index a26806db9..7403e87f0 100644 --- a/packages/control-plane/src/routes/sessions.ts +++ b/packages/control-plane/src/routes/sessions.ts @@ -1,4 +1,5 @@ -import type { Route } from "./shared"; +import { Hono } from "hono"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { sessionCreateRoutes } from "./session-create"; import { sessionChildRoutes } from "./session-children"; import { sessionChildSpawnRoutes } from "./session-child-spawn"; @@ -12,17 +13,21 @@ import { sessionWsTokenRoutes } from "./session-ws-token"; import { sessionDiffRoutes } from "./session-diffs"; import { sessionSkillRoutes } from "./session-skills"; -export const sessionRoutes: Route[] = [ - ...sessionCreateRoutes, - ...sessionIndexRoutes, - ...sessionRuntimeProxyRoutes, - ...sessionWsTokenRoutes, - ...sessionPromptRoutes, - ...sessionPullRequestRoutes, - ...sessionMediaRoutes, - ...sessionAttachmentRoutes, - ...sessionDiffRoutes, - ...sessionSkillRoutes, - ...sessionChildSpawnRoutes, - ...sessionChildRoutes, -]; +/** Mount order is precedence order: `/sessions/inbox` must register before `/sessions/:id`. */ +export const sessionRoutes = new Hono(); +for (const module of [ + sessionCreateRoutes, + sessionIndexRoutes, + sessionRuntimeProxyRoutes, + sessionWsTokenRoutes, + sessionPromptRoutes, + sessionPullRequestRoutes, + sessionMediaRoutes, + sessionAttachmentRoutes, + sessionDiffRoutes, + sessionSkillRoutes, + sessionChildSpawnRoutes, + sessionChildRoutes, +]) { + sessionRoutes.route("/", module); +} diff --git a/packages/control-plane/src/routes/slack-notify.test.ts b/packages/control-plane/src/routes/slack-notify.test.ts index b58bcb7af..d4d87e863 100644 --- a/packages/control-plane/src/routes/slack-notify.test.ts +++ b/packages/control-plane/src/routes/slack-notify.test.ts @@ -75,7 +75,7 @@ function createEnv(overrides?: Partial): Env { } async function callHandler(body: unknown, envOverrides?: Partial): Promise { - const match = PATH.match(PATTERN)!; + const params = { id: PATH.match(PATTERN)!.groups!.id }; const init: RequestInit = { method: "POST", headers: { "Content-Type": "application/json" }, @@ -84,7 +84,7 @@ async function callHandler(body: unknown, envOverrides?: Partial): Promise< return handleSlackNotify( new Request(`https://test.local${PATH}`, init), createEnv(envOverrides), - match, + params, createCtx() ); } diff --git a/packages/control-plane/src/routes/slack-notify.ts b/packages/control-plane/src/routes/slack-notify.ts index 024559d16..0dda049a9 100644 --- a/packages/control-plane/src/routes/slack-notify.ts +++ b/packages/control-plane/src/routes/slack-notify.ts @@ -1,3 +1,6 @@ +import { Hono } from "hono"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; /** * Intentionally emits no transcript events: the agent's own tool_call event * is the single source of truth. Audit detail lives in the structured logs. @@ -17,7 +20,13 @@ import { IntegrationSettingsStore, resolveSlackSettings } from "../db/integratio import { SessionIndexStore } from "../db/session-index"; import { createLogger } from "../logger"; import type { Env } from "../types"; -import { error, json, type RequestContext } from "./shared"; +import { + error, + GITHUB_SANDBOX_FALLBACK_ROUTE, + json, + requirePermission, + type RequestContext, +} from "./shared"; const logger = createLogger("slack-notify"); @@ -50,10 +59,10 @@ interface AuditFields { export async function handleSlackNotify( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const sessionId = match.groups?.id; + const sessionId = params.id; if (!sessionId) return error("Session ID required", 400); const parsed = await parseBody(request); @@ -302,3 +311,15 @@ function logDenial( ...audit, }); } + +export const slackNotifyRoutes = new Hono(); + +// Agent-initiated Slack notification (sandbox-authenticated). +slackNotifyRoutes.post( + "/sessions/:id/slack-notify", + admit({ + ...GITHUB_SANDBOX_FALLBACK_ROUTE, + authorization: requirePermission("sessions.collaborate"), + }), + (c) => dispatch(c, handleSlackNotify) +); diff --git a/packages/control-plane/src/routing/admit.ts b/packages/control-plane/src/routing/admit.ts index a3efd7ccc..b9378e8d2 100644 --- a/packages/control-plane/src/routing/admit.ts +++ b/packages/control-plane/src/routing/admit.ts @@ -1,7 +1,11 @@ /** Route admission as Hono middleware over the framework-neutral admission pipeline. */ -import type { MiddlewareHandler } from "hono"; +import type { Context as HonoContext, MiddlewareHandler } from "hono"; +import type { ParamKeys, ParamKeyToRecord } from "hono/types"; +import type { Simplify, UnionToIntersection } from "hono/utils/types"; import { auditRouteAuthorizationDecision } from "../authorization/request-audit"; +import type { RequestContext } from "../http/request-context"; +import { error } from "../http/responses"; import type { RouteAdmissionPolicy, RouteAuthentication, @@ -31,16 +35,60 @@ export interface Admitted { } /** The Hono environment a handler behind `admit(policy)` sees. */ -export type AdmittedEnv = { +export type AdmittedEnv = { Bindings: Env; - Variables: ControlPlaneHonoEnv["Variables"] & { admitted: Admitted }; + Variables: ControlPlaneHonoEnv["Variables"] & { admitted: Admitted }; }; /** The middleware `admit()` returns, carrying the policy it enforces for route enumeration. */ export type AdmitMiddleware = MiddlewareHandler< - AdmittedEnv + AdmittedEnv > & { readonly policy: Policy }; +/** The path parameters Hono decodes for a route path, typed from its `:param` segments. */ +export type PathParams = Simplify< + UnionToIntersection>> +>; + +/** The Hono environment a route handler needs: admission ran, and its context has the shape the handler asks for. */ +export type HandlerEnv = { + Bindings: Env; + Variables: ControlPlaneHonoEnv["Variables"] & { admitted: { request: Request; ctx: Context } }; +}; + +/** + * Run a route handler for an admitted request. + * + * The one place a Hono context is unpacked for a handler. The parameter + * type comes from the route's path literal and the context type is checked + * against what the route's policy produces, so a handler wired to a path + * without its parameters, or behind a policy that cannot produce the + * context it asks for, fails to compile. + */ +export function dispatch( + c: HonoContext, Path>, + handler: (request: Request, env: Env, params: PathParams, ctx: Context) => Promise +): Promise { + return handler(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx); +} + +/** + * Hono's parameter decoder leaves a segment it cannot decode as it arrived, + * so a malformed escape would otherwise reach the handler as data. The raw + * segments are checked once here; the decoded values are never re-decoded. + */ +function malformedSegment(routePath: string, pathname: string): boolean { + return Object.values(rawRouteParams(routePath, pathname)).some((segment) => { + if (!segment.includes("%")) return false; + try { + decodeURIComponent(segment); + return false; + } catch { + return true; + } + }); +} + /** * Evaluate `policy` for the selected route before its handler runs. * @@ -59,13 +107,23 @@ export function admit( throw new Error("Route without a verified principal cannot require authorization"); } - const middleware: MiddlewareHandler> = async (c, next) => { + const middleware: MiddlewareHandler> = async (c, next) => { const context = c.get("requestContext"); const pathname = c.req.path; // Recorded before anything can fail so the lifecycle finalizes an // admission error with this route's response policy. c.set("routePolicy", policy); - const params = rawRouteParams(c.req.routePath, pathname); + if (malformedSegment(c.req.routePath, pathname)) { + const result: RouteAdmissionResult = { + kind: "denied", + response: error("Invalid path encoding", 400), + requestLog: "emit", + }; + c.set("admission", { policy, params: {}, result }); + return result.response; + } + // Hono decodes each segment exactly once; admission reads those values. + const params = c.req.param() as RouteParams; const result = await admitRoute({ request: c.req.raw, env: c.env, diff --git a/packages/control-plane/src/routing/hono-app.test.ts b/packages/control-plane/src/routing/hono-app.test.ts index ad25e1d19..fb0c04a9a 100644 --- a/packages/control-plane/src/routing/hono-app.test.ts +++ b/packages/control-plane/src/routing/hono-app.test.ts @@ -3,9 +3,10 @@ import { HttpError, json } from "../http/responses"; import { TEST_SERVICE_SECRETS } from "../router.test-support"; import { createTestBackgroundTasks } from "../background-tasks.test-support"; import { defineRoute, NO_AUTHORIZATION, type Route } from "../routes/shared"; +import type { RequestContext } from "../http/request-context"; import type { Env } from "../types"; import { Hono } from "hono"; -import { admit } from "./admit"; +import { admit, dispatch } from "./admit"; import { createControlPlaneApp, type ControlPlaneHonoEnv, type ControlPlaneHost } from "./hono-app"; const PUBLIC = { authentication: { kind: "public" }, supportedScmProviders: "all" } as const; @@ -159,6 +160,48 @@ describe("control-plane Hono app lifecycle", () => { expect(response.headers.get("Access-Control-Allow-Origin")).toBe("*"); }); + it("hands a dispatched handler the admitted request, Hono's parameters, and the context", async () => { + const module = new Hono(); + const handler = vi.fn( + async (request: Request, _env: Env, params: { id: string }, ctx: RequestContext) => + json({ id: params.id, url: request.url, requestId: ctx.request_id }) + ); + module.get("/module/:id", admit({ ...PUBLIC, authorization: NO_AUTHORIZATION }), (c) => + dispatch(c, handler) + ); + const app = createControlPlaneApp([module], host); + + const response = await app.fetch(new Request("https://cp.test/module/m%2D1?x=1"), env); + expect(response.status).toBe(200); + const body = (await response.json()) as Record; + expect(body).toMatchObject({ id: "m-1", url: "https://cp.test/module/m%2D1?x=1" }); + expect(body.requestId).toBe(response.headers.get("x-request-id")); + expect(handler).toHaveBeenCalledOnce(); + }); + + it("rejects a parameter Hono could not decode before admission or the handler run", async () => { + const handler = vi.fn(async () => json({ ok: true })); + const module = new Hono(); + module.get("/module/:id", admit({ ...PUBLIC, authorization: NO_AUTHORIZATION }), (c) => + dispatch(c, handler) + ); + const app = createControlPlaneApp([module], host); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + const response = await app.fetch(new Request("https://cp.test/module/%E0%A4%A"), env); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ error: "Invalid path encoding" }); + expect(response.headers.get("Access-Control-Allow-Origin")).toBe("*"); + expect(response.headers.get("x-request-id")).toBeTruthy(); + expect(handler).not.toHaveBeenCalled(); + expect(loggedEvents(log).map((event) => event.http_status)).toEqual([400]); + + // A well-formed escape still reaches the handler decoded. + const decoded = await app.fetch(new Request("https://cp.test/module/%2541"), env); + expect(decoded.status).toBe(200); + expect(handler).toHaveBeenCalledOnce(); + }); + it("refuses a module whose route does not begin with admit(), before any request can run", () => { const sideEffect = vi.fn(); const naked = new Hono(); diff --git a/packages/control-plane/src/routing/hono-app.ts b/packages/control-plane/src/routing/hono-app.ts index d88f58f65..7ee64e075 100644 --- a/packages/control-plane/src/routing/hono-app.ts +++ b/packages/control-plane/src/routing/hono-app.ts @@ -16,6 +16,7 @@ import { catalog } from "../routes/catalog"; import type { Route, RouteParams } from "../routes/shared"; import type { Env } from "../types"; import { admit } from "./admit"; +import { rawRouteParams } from "./route-params"; import type { ControlPlaneHonoEnv, ControlPlaneHost, @@ -126,7 +127,7 @@ function legacy(route: Route): Handler { return route.handler( admission.result.handlerRequest, c.env, - legacyMatch(c.req.path, admission.params), + legacyMatch(c.req.path, rawRouteParams(c.req.routePath, c.req.path)), c.get("requestContext") ); }; diff --git a/packages/control-plane/src/routing/route-admission.ts b/packages/control-plane/src/routing/route-admission.ts index 24f61c2a6..4fb8395c3 100644 --- a/packages/control-plane/src/routing/route-admission.ts +++ b/packages/control-plane/src/routing/route-admission.ts @@ -415,15 +415,9 @@ function actorlessGrantMatches( params: RouteParams ): boolean { if (grant.service !== service) return false; - return Object.entries(grant.pathParams ?? {}).every(([name, expected]) => { - const value = params[name]; - if (value === undefined) return false; - try { - return decodeURIComponent(value) === expected; - } catch { - return false; - } - }); + return Object.entries(grant.pathParams ?? {}).every( + ([name, expected]) => params[name] === expected + ); } function enforceServiceRouteAuthorization( @@ -586,14 +580,8 @@ async function enforceAutomationRequirement( "Forbidden" ); } - const encodedAutomationId = params[requirement.automationIdParam]; - if (!encodedAutomationId) return { response: json({ error: "Invalid automation route" }, 400) }; - let automationId: string; - try { - automationId = decodeURIComponent(encodedAutomationId); - } catch { - return { response: json({ error: "Invalid automation route" }, 400) }; - } + const automationId = params[requirement.automationIdParam]; + if (!automationId) return { response: json({ error: "Invalid automation route" }, 400) }; try { const authorization = ctx.authorization; diff --git a/packages/control-plane/test/integration/rbac-routes.test.ts b/packages/control-plane/test/integration/rbac-routes.test.ts index da92fd9a9..1021ef04f 100644 --- a/packages/control-plane/test/integration/rbac-routes.test.ts +++ b/packages/control-plane/test/integration/rbac-routes.test.ts @@ -468,8 +468,9 @@ describe("RBAC routes", () => { headers: { "Content-Type": "application/json" }, }); + // Admission refuses a segment Hono cannot decode before the handler runs. expect(response.status).toBe(400); - await expect(response.json()).resolves.toEqual({ error: "Invalid user ID" }); + await expect(response.json()).resolves.toEqual({ error: "Invalid path encoding" }); } ); diff --git a/packages/control-plane/test/integration/route-admission-matrix.test.ts b/packages/control-plane/test/integration/route-admission-matrix.test.ts index 903cd2b99..50a6b6df2 100644 --- a/packages/control-plane/test/integration/route-admission-matrix.test.ts +++ b/packages/control-plane/test/integration/route-admission-matrix.test.ts @@ -305,14 +305,30 @@ describe("route admission matrix", { timeout: MATRIX_TIMEOUT_MS }, () => { expect(observed).toMatchSnapshot(); }); - it("passes percent-encoded path segments through undecoded", async () => { - // Session ids are looked up by the raw segment, so an encoded letter misses. + it("rejects a parameter Hono could not decode, on every route, before admission", async () => { + // Hono leaves an undecodable segment as it arrived; admission refuses it + // uniformly rather than letting each handler discover it as data. + const malformed = [ + `${BASE}/sessions/%E0%A4%A`, + `${BASE}/sessions/%E0%A4%A/events`, + `${BASE}/automations/%E0%A4%A`, + `${BASE}/roles/%E0%A4%A`, + `${BASE}/repos/acme/%E0%A4%A/secrets`, + ]; + for (const url of malformed) { + const response = await serviceFetch(url); + expect(response.status, url).toBe(400); + await expect(response.json()).resolves.toEqual({ error: "Invalid path encoding" }); + } + }); + + it("decodes percent-encoded path segments exactly once", async () => { + // Session ids are decoded exactly once, by Hono, before the lookup and + // before the sandbox binding, so an encoded letter still names the session. const encodedSessionId = `%74${fixtures.readonlySessionId.slice(1)}`; const session = await serviceFetch(`${BASE}/sessions/${encodedSessionId}`); - expect(session.status).toBe(404); - await expect(session.json()).resolves.toEqual({ error: "Session not found" }); + expect(session.status).toBe(200); - // The sandbox binding verifies the token against the same raw segment. const sandboxInit = { headers: { Authorization: `Bearer ${SANDBOX_TOKEN}` } }; const encodedSandboxId = `%74${fixtures.sandboxSessionId.slice(1)}`; const plain = await SELF.fetch( @@ -324,7 +340,7 @@ describe("route admission matrix", { timeout: MATRIX_TIMEOUT_MS }, () => { `${BASE}/sessions/${encodedSandboxId}/tunnel-urls`, sandboxInit ); - expect(encoded.status).toBe(401); + expect(encoded.status).toBe(200); // Repository segments decode exactly once in the handler: a nested owner // arrives as one segment, a slash in the name is refused after that one diff --git a/packages/control-plane/test/integration/routing-compatibility.test.ts b/packages/control-plane/test/integration/routing-compatibility.test.ts index ebd7fe999..70439285c 100644 --- a/packages/control-plane/test/integration/routing-compatibility.test.ts +++ b/packages/control-plane/test/integration/routing-compatibility.test.ts @@ -83,10 +83,11 @@ describe("Worker routing compatibility", () => { } ); - it("passes a malformed percent escape through raw dynamic matching", async () => { + it("rejects a malformed percent escape before the route runs", async () => { const response = await serviceFetch("https://test.local/sessions/%E0%A4%A"); - await expectJsonNotFound(response, "Session not found"); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ error: "Invalid path encoding" }); }); it("returns the universal preflight response for an unknown path", async () => {