From 4ad11f3b12fc06a2f7ef6333d6bdbd7ce2923b07 Mon Sep 17 00:00:00 2001 From: Cole Murray Date: Wed, 2 Sep 2026 18:11:19 -0700 Subject: [PATCH 1/2] refactor: convert the sessions cluster to Hono sub-apps Every session module, the Slack notification route, and the runtime proxy register natively behind admit(policy), nested under one sessions sub-app in the old precedence order. Handlers take a typed params object in place of the match array and are exported for their tests; the session runtime client is attached with withSessionRuntime() instead of the sessionRoute() wrapper. Admission now reads Hono's decoded parameters, so path segments are decoded exactly once, and its own decodeURIComponent calls are gone. An encoded session id therefore resolves the same session as its plain form; the guardrail tests from #1720 record that change. Claude-Session: https://claude.ai/code/session_01KdDpTgGEjXpBA9SaGQVUH1 --- .../src/router.create-session.test.ts | 5 +- packages/control-plane/src/routes/catalog.ts | 15 +- .../src/routes/session-attachments.test.ts | 39 +- .../src/routes/session-attachments.ts | 69 +-- .../src/routes/session-child-spawn.ts | 33 +- .../src/routes/session-children.test.ts | 6 +- .../src/routes/session-children.ts | 109 ++-- .../src/routes/session-create.ts | 24 +- .../control-plane/src/routes/session-diffs.ts | 149 +++--- .../src/routes/session-index.test.ts | 31 +- .../control-plane/src/routes/session-index.ts | 70 ++- .../src/routes/session-media-stream.ts | 41 +- .../src/routes/session-media-upload.ts | 40 +- .../control-plane/src/routes/session-media.ts | 10 +- .../src/routes/session-prompt.ts | 39 +- .../src/routes/session-pull-requests.ts | 39 +- .../control-plane/src/routes/session-route.ts | 28 +- .../src/routes/session-runtime-proxy.test.ts | 85 +-- .../src/routes/session-runtime-proxy.ts | 487 ++++++++++-------- .../src/routes/session-skills.ts | 50 +- .../src/routes/session-ws-token.test.ts | 57 +- .../src/routes/session-ws-token.ts | 40 +- packages/control-plane/src/routes/sessions.ts | 35 +- .../src/routes/slack-notify.test.ts | 4 +- .../control-plane/src/routes/slack-notify.ts | 27 +- packages/control-plane/src/routing/admit.ts | 4 +- .../control-plane/src/routing/hono-app.ts | 3 +- .../src/routing/route-admission.ts | 22 +- .../route-admission-matrix.test.ts | 11 +- 29 files changed, 852 insertions(+), 720 deletions(-) diff --git a/packages/control-plane/src/router.create-session.test.ts b/packages/control-plane/src/router.create-session.test.ts index 17294dbd0a..4f8f4501b6 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 92d74cb171..72bc1a7282 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 7fa1e01a28..aeacf45618 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 f0b15ae48f..84f617a38d 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 { withSessionRuntime, type SessionRouteContext } 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,31 @@ 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) => + handleAttachmentPost( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); + +sessionAttachmentRoutes.get( + "/sessions/:id/attachments/:attachmentId", + admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => + handleAttachmentGet( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-child-spawn.ts b/packages/control-plane/src/routes/session-child-spawn.ts index 5e25679140..b7bfe7046e 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 { withSessionRuntime, type SessionRouteContext } 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,22 @@ 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) => + handleSpawnChild( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-children.test.ts b/packages/control-plane/src/routes/session-children.test.ts index dae26b5059..8a703a35d8 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 14fe3c065f..7f8a123031 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 } 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 { withSessionRuntime, type SessionRouteContext } 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,46 @@ 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) => handleListChildren(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) +); +sessionChildRoutes.get( + "/sessions/:id/children/:childId", + admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => + handleGetChild( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + handleCancelChild( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionChildRoutes.post( + "/sessions/:id/children/:childId/prompt", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => + handlePromptChild( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-create.ts b/packages/control-plane/src/routes/session-create.ts index b746dbf240..82b47cbff7 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 c66b1adf5f..f0c84f9977 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 { withSessionRuntime, type SessionRouteContext } 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,60 @@ 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) => + handleDiffState( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionDiffRoutes.put("/sessions/:id/diff", DIFF_WRITE, (c) => + handleDiffUpload( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionDiffRoutes.post("/sessions/:id/diff/failure", DIFF_WRITE, (c) => + handleDiffFailure( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionDiffRoutes.get("/sessions/:id/diff/:revisionId/files/:fileId", DIFF_READ, (c) => + handleDiffFile( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionDiffRoutes.post( + "/sessions/:id/diff/retry", + admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("sessions.lifecycle"), + }), + (c) => + handleDiffRetry( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-index.test.ts b/packages/control-plane/src/routes/session-index.test.ts index 0dbb53cefa..85812c4a12 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 d69c0359d0..e697a793b6 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 } 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) => handlePatchReadState(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) +); +sessionIndexRoutes.delete( + "/sessions/:id", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.delete") }), + (c) => handleDeleteSession(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) +); diff --git a/packages/control-plane/src/routes/session-media-stream.ts b/packages/control-plane/src/routes/session-media-stream.ts index f3b5e964f0..d815206498 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 { withSessionRuntime, type SessionRouteContext } 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,21 @@ 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) => + handleMediaGet( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-media-upload.ts b/packages/control-plane/src/routes/session-media-upload.ts index b4317c7d08..174dacd0f7 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 { withSessionRuntime, type SessionRouteContext } 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,19 @@ 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) => + handleMediaUpload( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-media.ts b/packages/control-plane/src/routes/session-media.ts index fa735ac017..06df7b1e95 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 41072f2279..8a47e919e1 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 { withSessionRuntime, type SessionRouteContext } 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,19 @@ 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) => + handleSessionPrompt( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-pull-requests.ts b/packages/control-plane/src/routes/session-pull-requests.ts index f42b66ccff..550d102a58 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 { withSessionRuntime, type SessionRouteContext } 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,19 @@ 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) => + handleRefreshPullRequests( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-route.ts b/packages/control-plane/src/routes/session-route.ts index 88fabc47af..50e580804b 100644 --- a/packages/control-plane/src/routes/session-route.ts +++ b/packages/control-plane/src/routes/session-route.ts @@ -1,29 +1,15 @@ -import type { SessionRuntimeClient } from "../session/runtime-client"; -import { createSessionRuntimeClient } from "../session/runtime-client"; +import type { RequestContext } from "../http/request-context"; +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), - }); -} - -export function sessionRoute( - route: Omit & { handler: SessionRouteHandler } -): RouteDefinition { - return { ...route, handler: withSessionRuntime(route.handler) }; + ctx: Context +): Context & { sessionRuntime: SessionRuntimeClient } { + return { ...ctx, sessionRuntime: createSessionRuntimeClient(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 ee3f3e10f9..bb261ca24d 100644 --- a/packages/control-plane/src/routes/session-runtime-proxy.test.ts +++ b/packages/control-plane/src/routes/session-runtime-proxy.test.ts @@ -3,9 +3,14 @@ import { SessionInternalPaths } from "../session/contracts"; 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 { sessionRuntimeProxyHandlers } from "./session-runtime-proxy"; import type { Env } from "../types"; -import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; +import { + contractFor, + routePathPattern, + TEST_BACKGROUND_TASK_CONTEXT, +} from "../router.test-support"; +import { withSessionRuntime } from "./session-route"; function createCtx( db: SqlDatabase = {} as SqlDatabase, @@ -44,13 +49,35 @@ function createEnv(fetch: (request: Request) => Promise): Env { } as unknown as Env; } +const PROXY_HANDLERS: Record = { + "GET /sessions/:id/sandbox-access": "sandboxAccess", + "GET /sessions/:id": "snapshot", + "POST /sessions/:id/stop": "stop", + "POST /sessions/:id/sandbox-error": "sandboxError", + "GET /sessions/:id/events": "events", + "GET /sessions/:id/artifacts": "artifacts", + "GET /sessions/:id/participants": "participants", + "GET /sessions/:id/participant-profiles": "participantProfiles", + "GET /sessions/:id/messages": "messages", + "POST /sessions/:id/pr": "createPr", + "POST /sessions/:id/openai-token-refresh": "openaiTokenRefresh", + "POST /sessions/:id/xai-token-refresh": "xaiTokenRefresh", + "POST /sessions/:id/scm-credentials": "scmCredentials", + "GET /sessions/:id/tunnel-urls": "tunnelUrls", + "PATCH /sessions/:id/title": "updateTitle", + "POST /sessions/:id/archive": "archive", + "POST /sessions/:id/unarchive": "unarchive", +}; + +/** The production contract for a concrete path, the handler behind it, and its parameters. */ 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}`); + const route = contractFor(method, path); + if (!route) throw new Error(`No route found for ${method} ${path}`); + const key = PROXY_HANDLERS[`${method} ${route.path}`]; + if (!key) throw new Error(`No proxy handler registered for ${method} ${route.path}`); + const id = path.match(routePathPattern(route.path))?.groups?.id; + if (!id) throw new Error(`No session id in ${path}`); + return { handler: sessionRuntimeProxyHandlers[key], match: { id }, route }; } describe("session runtime proxy routes", () => { @@ -67,7 +94,7 @@ describe("session runtime proxy routes", () => { new Request(`https://test.local${path}`), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(200); @@ -113,7 +140,7 @@ describe("session runtime proxy routes", () => { new Request(`https://test.local${path}`), createEnv(fetch), match, - createCtx({} as SqlDatabase, input.permissions) + withSessionRuntime(createEnv(fetch), createCtx({} as SqlDatabase, input.permissions)) ); const snapshot = (await response.json()) as { session: Record }; @@ -141,7 +168,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1/events?limit=10"), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); await expect(response.json()).resolves.toEqual({ events: [] }); @@ -171,7 +198,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(200); @@ -201,7 +228,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(413); @@ -217,7 +244,7 @@ describe("session runtime proxy routes", () => { new Request(`https://test.local${path}`, { method: "POST", body: "not json" }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(401); @@ -239,7 +266,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(400); @@ -301,7 +328,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1/participant-profiles"), createEnv(fetch), match, - createCtx(db) + withSessionRuntime(createEnv(fetch), createCtx(db)) ); expect(response.status).toBe(200); @@ -339,7 +366,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1/participant-profiles"), createEnv(fetch), match, - createCtx(db) + withSessionRuntime(createEnv(fetch), createCtx(db)) ); expect(response.status).toBe(502); @@ -356,7 +383,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1/participant-profiles"), createEnv(fetch), match, - createCtx(db) + withSessionRuntime(createEnv(fetch), createCtx(db)) ); expect(response.status).toBe(502); @@ -373,7 +400,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1/participant-profiles"), createEnv(fetch), match, - createCtx(db) + withSessionRuntime(createEnv(fetch), createCtx(db)) ); expect(response.status).toBe(404); @@ -396,7 +423,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); await expect(response.json()).resolves.toEqual({ status: "updated" }); @@ -435,7 +462,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - ctx + withSessionRuntime(createEnv(fetch), ctx) ); expect(response.status).toBe(200); @@ -457,7 +484,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(400); @@ -475,7 +502,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1"), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(500); @@ -490,7 +517,7 @@ describe("session runtime proxy routes", () => { new Request("https://test.local/sessions/session-1"), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(404); @@ -513,7 +540,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(200); @@ -534,7 +561,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(400); @@ -554,7 +581,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(400); @@ -585,7 +612,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); expect(response.status).toBe(200); @@ -612,7 +639,7 @@ describe("session runtime proxy routes", () => { }), createEnv(fetch), match, - createCtx() + withSessionRuntime(createEnv(fetch), createCtx()) ); 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 39278cb60e..5fa455cb8e 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 { withSessionRuntime, type SessionRouteContext } 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,257 @@ 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. */ +export const sessionRuntimeProxyHandlers = { + sandboxAccess: simpleProxy({ internalPath: SessionInternalPaths.sandboxAccess }), + snapshot: handleSessionSnapshot, + stop: simpleProxy({ internalPath: SessionInternalPaths.stop, runtimeMethod: "POST" }), + sandboxError: handleSandboxError, + events: simpleProxy({ internalPath: SessionInternalPaths.events, forwardSearch: true }), + artifacts: simpleProxy({ internalPath: SessionInternalPaths.artifacts }), + participants: simpleProxy({ internalPath: SessionInternalPaths.participants }), + participantProfiles: handleParticipantProfiles, + messages: simpleProxy({ internalPath: SessionInternalPaths.messages, forwardSearch: true }), + createPr: handleCreatePR, + openaiTokenRefresh: legacyTokenRefresh("openai", SessionInternalPaths.openaiTokenRefresh), + xaiTokenRefresh: legacyTokenRefresh("xai", SessionInternalPaths.xaiTokenRefresh), + scmCredentials: simpleProxy({ + internalPath: SessionInternalPaths.scmCredentials, + runtimeMethod: "POST", + }), + tunnelUrls: simpleProxy({ internalPath: SessionInternalPaths.tunnelUrls, runtimeMethod: "GET" }), + updateTitle: lifecycleProxy(SessionInternalPaths.updateTitle), + archive: lifecycleProxy(SessionInternalPaths.archive), + unarchive: lifecycleProxy(SessionInternalPaths.unarchive), +} satisfies Record; + +const proxy = sessionRuntimeProxyHandlers; +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) => + proxy.sandboxAccess( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id", + admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => + proxy.snapshot( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + proxy.stop( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/sandbox-error", + admit({ ...SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => + proxy.sandboxError( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + proxy.events( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + proxy.artifacts( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/participants", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => + proxy.participants( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + proxy.participantProfiles( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.get( + "/sessions/:id/messages", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), + (c) => + proxy.messages( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + proxy.createPr( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/openai-token-refresh", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => + proxy.openaiTokenRefresh( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/xai-token-refresh", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => + proxy.xaiTokenRefresh( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.post( + "/sessions/:id/scm-credentials", + admit({ ...SCM_CREDENTIALS_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => + proxy.scmCredentials( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +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) => + proxy.tunnelUrls( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.patch("/sessions/:id/title", LIFECYCLE, (c) => + proxy.updateTitle( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.post("/sessions/:id/archive", LIFECYCLE, (c) => + proxy.archive( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); +sessionRuntimeProxyRoutes.post("/sessions/:id/unarchive", LIFECYCLE, (c) => + proxy.unarchive( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/session-skills.ts b/packages/control-plane/src/routes/session-skills.ts index ceb48d812b..f07b4ef52b 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 } 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) => handleSessionSkillsView(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) +); + +sessionSkillRoutes.get( + "/sessions/:id/sandbox-skills", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), + (c) => handleSandboxInstallation(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) +); 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 9720d30cfa..2cff57d459 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 58ce8d1ded..062db65039 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 { withSessionRuntime, 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,19 @@ 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) => + handleSessionWsToken( + c.var.admitted.request, + c.env, + c.req.param(), + withSessionRuntime(c.env, c.var.admitted.ctx) + ) +); diff --git a/packages/control-plane/src/routes/sessions.ts b/packages/control-plane/src/routes/sessions.ts index a26806db96..7403e87f05 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 b58bcb7af7..d4d87e8632 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 024559d167..655c54955d 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 } 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) => handleSlackNotify(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) +); diff --git a/packages/control-plane/src/routing/admit.ts b/packages/control-plane/src/routing/admit.ts index a3efd7ccce..83983141fd 100644 --- a/packages/control-plane/src/routing/admit.ts +++ b/packages/control-plane/src/routing/admit.ts @@ -12,7 +12,6 @@ import type { import type { Env } from "../types"; import type { ControlPlaneHonoEnv } from "./hono-env"; import { admitRoute, type RouteAdmissionResult } from "./route-admission"; -import { rawRouteParams } from "./route-params"; /** Everything admission and response policy need to know about a route. */ export type AdmissionPolicy = RouteAdmissionPolicy & Pick; @@ -65,7 +64,8 @@ export function admit( // 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); + // 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.ts b/packages/control-plane/src/routing/hono-app.ts index d88f58f650..7ee64e0751 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 24f61c2a64..4fb8395c3d 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/route-admission-matrix.test.ts b/packages/control-plane/test/integration/route-admission-matrix.test.ts index 903cd2b99f..b474357bf1 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,13 @@ 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("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 +323,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 From 71224c4cecd8fe8d052062b94a32623540e3f4c6 Mon Sep 17 00:00:00 2001 From: Cole Murray Date: Wed, 2 Sep 2026 18:37:13 -0700 Subject: [PATCH 2/2] refactor: dispatch session handlers through one adapter and refuse malformed path segments Review follow-ups for the sessions conversion. Admission now checks every raw `:param` segment once: a segment Hono could not decode answers 400 `Invalid path encoding` on every route, before the lookup that used to turn it into a 404. The automation-id, role-id, and member-id paths that each validated encoding in their own words now share that one rule; the two RBAC integration expectations and the routing-compatibility probe record the uniform envelope. `dispatch(c, handler)` is the one place a Hono context is unpacked for a route handler, and `dispatchSession(c, handler)` adds the runtime client. The context type is checked against the route's policy, so the curried `handle(handler)` form was rejected: Hono infers the environment from the returned function and the check disappears. The runtime proxy registers its handlers directly; the test-only handler map is gone and the proxy suite dispatches every request through the production sub-app, with a wiring table that fails if a route is bound to the wrong proxy. Claude-Session: https://claude.ai/code/session_01KdDpTgGEjXpBA9SaGQVUH1 --- .../src/routes/session-attachments.ts | 18 +- .../src/routes/session-child-spawn.ts | 10 +- .../src/routes/session-children.ts | 30 +- .../control-plane/src/routes/session-diffs.ts | 40 +- .../control-plane/src/routes/session-index.ts | 6 +- .../src/routes/session-media-stream.ts | 10 +- .../src/routes/session-media-upload.ts | 10 +- .../src/routes/session-prompt.ts | 10 +- .../src/routes/session-pull-requests.ts | 10 +- .../control-plane/src/routes/session-route.ts | 17 + .../src/routes/session-runtime-proxy.test.ts | 440 ++++++++++-------- .../src/routes/session-runtime-proxy.ts | 159 ++----- .../src/routes/session-skills.ts | 6 +- .../src/routes/session-ws-token.ts | 10 +- .../control-plane/src/routes/slack-notify.ts | 4 +- packages/control-plane/src/routing/admit.ts | 68 ++- .../src/routing/hono-app.test.ts | 45 +- .../test/integration/rbac-routes.test.ts | 3 +- .../route-admission-matrix.test.ts | 17 + .../integration/routing-compatibility.test.ts | 5 +- 20 files changed, 443 insertions(+), 475 deletions(-) diff --git a/packages/control-plane/src/routes/session-attachments.ts b/packages/control-plane/src/routes/session-attachments.ts index 84f617a38d..371f63acd0 100644 --- a/packages/control-plane/src/routes/session-attachments.ts +++ b/packages/control-plane/src/routes/session-attachments.ts @@ -54,7 +54,7 @@ import { json, requirePermission, } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-attachments"); @@ -246,23 +246,11 @@ sessionAttachmentRoutes.post( ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.collaborate"), }), - (c) => - handleAttachmentPost( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleAttachmentPost) ); sessionAttachmentRoutes.get( "/sessions/:id/attachments/:attachmentId", admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => - handleAttachmentGet( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (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 b7bfe7046e..266e0a4562 100644 --- a/packages/control-plane/src/routes/session-child-spawn.ts +++ b/packages/control-plane/src/routes/session-child-spawn.ts @@ -37,7 +37,7 @@ import { permissionRequirement, requireAll, } from "./shared"; -import { withSessionRuntime, 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"; @@ -362,11 +362,5 @@ sessionChildSpawnRoutes.post( permissionRequirement("sessions.collaborate") ), }), - (c) => - handleSpawnChild( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleSpawnChild) ); diff --git a/packages/control-plane/src/routes/session-children.ts b/packages/control-plane/src/routes/session-children.ts index 7f8a123031..54354248ca 100644 --- a/packages/control-plane/src/routes/session-children.ts +++ b/packages/control-plane/src/routes/session-children.ts @@ -1,5 +1,5 @@ import { Hono } from "hono"; -import { admit } from "../routing/admit"; +import { admit, dispatch } from "../routing/admit"; import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { cancelChildSessionRequestSchema, @@ -23,7 +23,7 @@ import { SCM_AGNOSTIC_SANDBOX_ROUTE, type RequestContext, } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-children"); @@ -266,18 +266,12 @@ export const sessionChildRoutes = new Hono(); sessionChildRoutes.get( "/sessions/:id/children", admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => handleListChildren(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) + (c) => dispatch(c, handleListChildren) ); sessionChildRoutes.get( "/sessions/:id/children/:childId", admit({ ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => - handleGetChild( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleGetChild) ); sessionChildRoutes.post( "/sessions/:id/children/:childId/cancel", @@ -285,22 +279,10 @@ sessionChildRoutes.post( ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.lifecycle"), }), - (c) => - handleCancelChild( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleCancelChild) ); sessionChildRoutes.post( "/sessions/:id/children/:childId/prompt", admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), - (c) => - handlePromptChild( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handlePromptChild) ); diff --git a/packages/control-plane/src/routes/session-diffs.ts b/packages/control-plane/src/routes/session-diffs.ts index f0c84f9977..609303cd46 100644 --- a/packages/control-plane/src/routes/session-diffs.ts +++ b/packages/control-plane/src/routes/session-diffs.ts @@ -15,7 +15,7 @@ import { SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, requirePermission, } from "./shared"; -import { withSessionRuntime, 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; @@ -199,37 +199,15 @@ const DIFF_WRITE = admit({ authorization: requirePermission("sessions.collaborate"), }); -sessionDiffRoutes.get("/sessions/:id/diff", DIFF_READ, (c) => - handleDiffState( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) -); +sessionDiffRoutes.get("/sessions/:id/diff", DIFF_READ, (c) => dispatchSession(c, handleDiffState)); sessionDiffRoutes.put("/sessions/:id/diff", DIFF_WRITE, (c) => - handleDiffUpload( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + dispatchSession(c, handleDiffUpload) ); sessionDiffRoutes.post("/sessions/:id/diff/failure", DIFF_WRITE, (c) => - handleDiffFailure( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + dispatchSession(c, handleDiffFailure) ); sessionDiffRoutes.get("/sessions/:id/diff/:revisionId/files/:fileId", DIFF_READ, (c) => - handleDiffFile( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + dispatchSession(c, handleDiffFile) ); sessionDiffRoutes.post( "/sessions/:id/diff/retry", @@ -237,11 +215,5 @@ sessionDiffRoutes.post( ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.lifecycle"), }), - (c) => - handleDiffRetry( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleDiffRetry) ); diff --git a/packages/control-plane/src/routes/session-index.ts b/packages/control-plane/src/routes/session-index.ts index e697a793b6..2718c24ba7 100644 --- a/packages/control-plane/src/routes/session-index.ts +++ b/packages/control-plane/src/routes/session-index.ts @@ -1,5 +1,5 @@ import { Hono } from "hono"; -import { admit } from "../routing/admit"; +import { admit, dispatch } from "../routing/admit"; import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { parseSessionListQuery, @@ -252,10 +252,10 @@ sessionIndexRoutes.get( sessionIndexRoutes.patch( "/sessions/:id/read-state", admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => handlePatchReadState(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) + (c) => dispatch(c, handlePatchReadState) ); sessionIndexRoutes.delete( "/sessions/:id", admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.delete") }), - (c) => handleDeleteSession(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) + (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 d815206498..cda2952e31 100644 --- a/packages/control-plane/src/routes/session-media-stream.ts +++ b/packages/control-plane/src/routes/session-media-stream.ts @@ -14,7 +14,7 @@ import { } from "./responses/stored-object-response"; import { getSessionArtifactFromRuntime } from "./session-media-artifacts"; import { error, GITHUB_USER_OR_SERVICE_ROUTE, requirePermission } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-media"); function getMediaMimeType( @@ -148,11 +148,5 @@ sessionMediaStreamRoutes.get( actorlessGrants: [{ service: "slack-bot" }], }), }), - (c) => - handleMediaGet( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (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 174dacd0f7..a697649391 100644 --- a/packages/control-plane/src/routes/session-media-upload.ts +++ b/packages/control-plane/src/routes/session-media-upload.ts @@ -25,7 +25,7 @@ import { createMediaObjectStorage, type ObjectStorage } from "../storage/object- import type { Env } from "../types"; import { listSessionArtifactsFromRuntime, persistMediaArtifact } from "./session-media-artifacts"; import { error, GITHUB_SANDBOX_FALLBACK_ROUTE, json, requirePermission } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; function getRequiredFormString(value: MultipartFieldValue | null, name: string): string | Response { if (typeof value !== "string" || value.trim().length === 0) { @@ -250,11 +250,5 @@ sessionMediaUploadRoutes.post( ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.collaborate"), }), - (c) => - handleMediaUpload( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleMediaUpload) ); diff --git a/packages/control-plane/src/routes/session-prompt.ts b/packages/control-plane/src/routes/session-prompt.ts index 8a47e919e1..1eb66bfebe 100644 --- a/packages/control-plane/src/routes/session-prompt.ts +++ b/packages/control-plane/src/routes/session-prompt.ts @@ -28,7 +28,7 @@ import { } from "../session/identity"; import type { Env } from "../types"; import { error, GITHUB_USER_OR_SERVICE_ROUTE, requirePermission } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const logger = createLogger("router:session-prompt"); @@ -185,11 +185,5 @@ sessionPromptRoutes.post( ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.collaborate"), }), - (c) => - handleSessionPrompt( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (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 550d102a58..4b858cef83 100644 --- a/packages/control-plane/src/routes/session-pull-requests.ts +++ b/packages/control-plane/src/routes/session-pull-requests.ts @@ -4,7 +4,7 @@ import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { SessionInternalPaths } from "../session/contracts"; import type { Env } from "../types"; import { error, GITHUB_USER_OR_SERVICE_ROUTE, requirePermission } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; /** * Manual PR sync (design §5.3): forwards to the session DO's internal @@ -34,11 +34,5 @@ sessionPullRequestRoutes.post( ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.lifecycle"), }), - (c) => - handleRefreshPullRequests( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (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 50e580804b..1b21c063ef 100644 --- a/packages/control-plane/src/routes/session-route.ts +++ b/packages/control-plane/src/routes/session-route.ts @@ -1,4 +1,6 @@ +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"; @@ -13,3 +15,18 @@ export function withSessionRuntime( ): Context & { sessionRuntime: SessionRuntimeClient } { return { ...ctx, sessionRuntime: createSessionRuntimeClient(env, ctx) }; } + +/** 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 bb261ca24d..e5f2472d72 100644 --- a/packages/control-plane/src/routes/session-runtime-proxy.test.ts +++ b/packages/control-plane/src/routes/session-runtime-proxy.test.ts @@ -1,47 +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 { sessionRuntimeProxyHandlers } from "./session-runtime-proxy"; -import type { Env } from "../types"; +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 { - contractFor, - routePathPattern, + createTestRequestHandler, TEST_BACKGROUND_TASK_CONTEXT, + TEST_SERVICE_SECRETS, } from "../router.test-support"; -import { withSessionRuntime } from "./session-route"; +import { SessionInternalPaths } from "../session/contracts"; +import type { Env } from "../types"; +import { sessionRuntimeProxyRoutes } from "./session-runtime-proxy"; + +const mocks = vi.hoisted(() => ({ authenticate: vi.fn() })); + +vi.mock("../auth/authenticate", async (importOriginal) => ({ + ...(await importOriginal()), + authenticate: mocks.authenticate, +})); -function createCtx( - db: SqlDatabase = {} as SqlDatabase, - permissions: PermissionId[] = ["sessions.read"] -): RequestContext { +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 })), @@ -49,52 +92,109 @@ function createEnv(fetch: (request: Request) => Promise): Env { } as unknown as Env; } -const PROXY_HANDLERS: Record = { - "GET /sessions/:id/sandbox-access": "sandboxAccess", - "GET /sessions/:id": "snapshot", - "POST /sessions/:id/stop": "stop", - "POST /sessions/:id/sandbox-error": "sandboxError", - "GET /sessions/:id/events": "events", - "GET /sessions/:id/artifacts": "artifacts", - "GET /sessions/:id/participants": "participants", - "GET /sessions/:id/participant-profiles": "participantProfiles", - "GET /sessions/:id/messages": "messages", - "POST /sessions/:id/pr": "createPr", - "POST /sessions/:id/openai-token-refresh": "openaiTokenRefresh", - "POST /sessions/:id/xai-token-refresh": "xaiTokenRefresh", - "POST /sessions/:id/scm-credentials": "scmCredentials", - "GET /sessions/:id/tunnel-urls": "tunnelUrls", - "PATCH /sessions/:id/title": "updateTitle", - "POST /sessions/:id/archive": "archive", - "POST /sessions/:id/unarchive": "unarchive", -}; +function dispatch(request: Request, env: Env): Promise { + return handleRequest(request, env, TEST_BACKGROUND_TASK_CONTEXT); +} -/** The production contract for a concrete path, the handler behind it, and its parameters. */ -function getHandler(method: string, path: string) { - const route = contractFor(method, path); - if (!route) throw new Error(`No route found for ${method} ${path}`); - const key = PROXY_HANDLERS[`${method} ${route.path}`]; - if (!key) throw new Error(`No proxy handler registered for ${method} ${route.path}`); - const id = path.match(routePathPattern(route.path))?.groups?.id; - if (!id) throw new Error(`No session id in ${path}`); - return { handler: sessionRuntimeProxyHandlers[key], match: { id }, route }; +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, - withSessionRuntime(createEnv(fetch), createCtx()) + + const response = await dispatch( + new Request("https://test.local/sessions/session-1/sandbox-access"), + createEnv(fetch) ); expect(response.status).toBe(200); @@ -133,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, - withSessionRuntime(createEnv(fetch), 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 }; @@ -162,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); await expect(response.json()).resolves.toEqual({ events: [] }); @@ -183,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, - withSessionRuntime(createEnv(fetch), 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"); @@ -214,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(413); @@ -237,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, - withSessionRuntime(createEnv(fetch), 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); @@ -253,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(400); @@ -322,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, - withSessionRuntime(createEnv(fetch), createCtx(db)) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(200); @@ -360,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, - withSessionRuntime(createEnv(fetch), createCtx(db)) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(502); @@ -377,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, - withSessionRuntime(createEnv(fetch), createCtx(db)) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(502); @@ -394,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, - withSessionRuntime(createEnv(fetch), createCtx(db)) + createEnv(fetch, { delegate: db }) ); expect(response.status).toBe(404); @@ -413,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); await expect(response.json()).resolves.toEqual({ status: "updated" }); @@ -441,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: { @@ -452,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, - withSessionRuntime(createEnv(fetch), ctx) + createEnv(fetch) ); expect(response.status).toBe(200); @@ -474,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(400); @@ -496,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(500); @@ -511,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(404); @@ -530,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(200); @@ -551,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(400); @@ -571,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, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(400); @@ -595,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", @@ -610,9 +643,7 @@ describe("session runtime proxy routes", () => { repoName: "backend", }), }), - createEnv(fetch), - match, - withSessionRuntime(createEnv(fetch), createCtx()) + createEnv(fetch) ); expect(response.status).toBe(200); @@ -629,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, - withSessionRuntime(createEnv(fetch), 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 5fa455cb8e..d4e4c0b3ab 100644 --- a/packages/control-plane/src/routes/session-runtime-proxy.ts +++ b/packages/control-plane/src/routes/session-runtime-proxy.ts @@ -30,7 +30,7 @@ import { SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, SCM_CREDENTIALS_ROUTE, } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { type SessionRouteContext, dispatchSession } from "./session-route"; const participantsResponseSchema = z.object({ participants: z.array( @@ -274,30 +274,6 @@ function lifecycleProxy(internalPath: SessionInternalPath): ProxyHandler { } /** Every proxied session operation, by the name its route is known by. */ -export const sessionRuntimeProxyHandlers = { - sandboxAccess: simpleProxy({ internalPath: SessionInternalPaths.sandboxAccess }), - snapshot: handleSessionSnapshot, - stop: simpleProxy({ internalPath: SessionInternalPaths.stop, runtimeMethod: "POST" }), - sandboxError: handleSandboxError, - events: simpleProxy({ internalPath: SessionInternalPaths.events, forwardSearch: true }), - artifacts: simpleProxy({ internalPath: SessionInternalPaths.artifacts }), - participants: simpleProxy({ internalPath: SessionInternalPaths.participants }), - participantProfiles: handleParticipantProfiles, - messages: simpleProxy({ internalPath: SessionInternalPaths.messages, forwardSearch: true }), - createPr: handleCreatePR, - openaiTokenRefresh: legacyTokenRefresh("openai", SessionInternalPaths.openaiTokenRefresh), - xaiTokenRefresh: legacyTokenRefresh("xai", SessionInternalPaths.xaiTokenRefresh), - scmCredentials: simpleProxy({ - internalPath: SessionInternalPaths.scmCredentials, - runtimeMethod: "POST", - }), - tunnelUrls: simpleProxy({ internalPath: SessionInternalPaths.tunnelUrls, runtimeMethod: "GET" }), - updateTitle: lifecycleProxy(SessionInternalPaths.updateTitle), - archive: lifecycleProxy(SessionInternalPaths.archive), - unarchive: lifecycleProxy(SessionInternalPaths.unarchive), -} satisfies Record; - -const proxy = sessionRuntimeProxyHandlers; const LIFECYCLE = admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.lifecycle"), @@ -311,24 +287,12 @@ sessionRuntimeProxyRoutes.get( ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.sandbox_access"), }), - (c) => - proxy.sandboxAccess( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, simpleProxy({ internalPath: SessionInternalPaths.sandboxAccess })) ); sessionRuntimeProxyRoutes.get( "/sessions/:id", admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => - proxy.snapshot( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleSessionSnapshot) ); sessionRuntimeProxyRoutes.post( "/sessions/:id/stop", @@ -339,23 +303,15 @@ sessionRuntimeProxyRoutes.post( }), }), (c) => - proxy.stop( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.stop, runtimeMethod: "POST" }) ) ); sessionRuntimeProxyRoutes.post( "/sessions/:id/sandbox-error", admit({ ...SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, authorization: NO_AUTHORIZATION }), - (c) => - proxy.sandboxError( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleSandboxError) ); sessionRuntimeProxyRoutes.get( "/sessions/:id/events", @@ -366,11 +322,9 @@ sessionRuntimeProxyRoutes.get( }), }), (c) => - proxy.events( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.events, forwardSearch: true }) ) ); sessionRuntimeProxyRoutes.get( @@ -381,24 +335,12 @@ sessionRuntimeProxyRoutes.get( actorlessGrants: [{ service: "slack-bot" }, { service: "linear-bot" }], }), }), - (c) => - proxy.artifacts( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, simpleProxy({ internalPath: SessionInternalPaths.artifacts })) ); sessionRuntimeProxyRoutes.get( "/sessions/:id/participants", admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => - proxy.participants( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, simpleProxy({ internalPath: SessionInternalPaths.participants })) ); sessionRuntimeProxyRoutes.get( "/sessions/:id/participant-profiles", @@ -406,23 +348,15 @@ sessionRuntimeProxyRoutes.get( ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read"), }), - (c) => - proxy.participantProfiles( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleParticipantProfiles) ); sessionRuntimeProxyRoutes.get( "/sessions/:id/messages", admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("sessions.read") }), (c) => - proxy.messages( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.messages, forwardSearch: true }) ) ); sessionRuntimeProxyRoutes.post( @@ -431,45 +365,25 @@ sessionRuntimeProxyRoutes.post( ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.collaborate"), }), - (c) => - proxy.createPr( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleCreatePR) ); sessionRuntimeProxyRoutes.post( "/sessions/:id/openai-token-refresh", admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), - (c) => - proxy.openaiTokenRefresh( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, legacyTokenRefresh("openai", SessionInternalPaths.openaiTokenRefresh)) ); sessionRuntimeProxyRoutes.post( "/sessions/:id/xai-token-refresh", admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), - (c) => - proxy.xaiTokenRefresh( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, legacyTokenRefresh("xai", SessionInternalPaths.xaiTokenRefresh)) ); sessionRuntimeProxyRoutes.post( "/sessions/:id/scm-credentials", admit({ ...SCM_CREDENTIALS_ROUTE, authorization: NO_AUTHORIZATION }), (c) => - proxy.scmCredentials( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.scmCredentials, runtimeMethod: "POST" }) ) ); sessionRuntimeProxyRoutes.get( @@ -479,34 +393,17 @@ sessionRuntimeProxyRoutes.get( authorization: requirePermission("sessions.sandbox_access"), }), (c) => - proxy.tunnelUrls( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) + dispatchSession( + c, + simpleProxy({ internalPath: SessionInternalPaths.tunnelUrls, runtimeMethod: "GET" }) ) ); sessionRuntimeProxyRoutes.patch("/sessions/:id/title", LIFECYCLE, (c) => - proxy.updateTitle( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + dispatchSession(c, lifecycleProxy(SessionInternalPaths.updateTitle)) ); sessionRuntimeProxyRoutes.post("/sessions/:id/archive", LIFECYCLE, (c) => - proxy.archive( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + dispatchSession(c, lifecycleProxy(SessionInternalPaths.archive)) ); sessionRuntimeProxyRoutes.post("/sessions/:id/unarchive", LIFECYCLE, (c) => - proxy.unarchive( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + 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 f07b4ef52b..0a16d7bda2 100644 --- a/packages/control-plane/src/routes/session-skills.ts +++ b/packages/control-plane/src/routes/session-skills.ts @@ -1,5 +1,5 @@ import { Hono } from "hono"; -import { admit } from "../routing/admit"; +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"; @@ -76,11 +76,11 @@ export const sessionSkillRoutes = new Hono(); sessionSkillRoutes.get( "/sessions/:id/skills", admit({ ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, authorization: requirePermission("sessions.read") }), - (c) => handleSessionSkillsView(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) + (c) => dispatch(c, handleSessionSkillsView) ); sessionSkillRoutes.get( "/sessions/:id/sandbox-skills", admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }), - (c) => handleSandboxInstallation(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) + (c) => dispatch(c, handleSandboxInstallation) ); diff --git a/packages/control-plane/src/routes/session-ws-token.ts b/packages/control-plane/src/routes/session-ws-token.ts index 062db65039..5b01b84b83 100644 --- a/packages/control-plane/src/routes/session-ws-token.ts +++ b/packages/control-plane/src/routes/session-ws-token.ts @@ -6,7 +6,7 @@ import { SESSION_WEBSOCKET_CONNECT_PERMISSION } from "@open-inspect/shared/rbac" import { SessionInternalPaths, sessionScmDisplayFieldsSchema } from "../session/contracts"; import type { Env } from "../types"; import { error, GITHUB_USER_OR_SERVICE_ROUTE, parseJsonBody, requirePermission } from "./shared"; -import { withSessionRuntime, type SessionRouteContext } from "./session-route"; +import { dispatchSession, type SessionRouteContext } from "./session-route"; export async function handleSessionWsToken( request: Request, @@ -58,11 +58,5 @@ sessionWsTokenRoutes.post( ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission(SESSION_WEBSOCKET_CONNECT_PERMISSION), }), - (c) => - handleSessionWsToken( - c.var.admitted.request, - c.env, - c.req.param(), - withSessionRuntime(c.env, c.var.admitted.ctx) - ) + (c) => dispatchSession(c, handleSessionWsToken) ); diff --git a/packages/control-plane/src/routes/slack-notify.ts b/packages/control-plane/src/routes/slack-notify.ts index 655c54955d..0dda049a92 100644 --- a/packages/control-plane/src/routes/slack-notify.ts +++ b/packages/control-plane/src/routes/slack-notify.ts @@ -1,5 +1,5 @@ import { Hono } from "hono"; -import { admit } from "../routing/admit"; +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 @@ -321,5 +321,5 @@ slackNotifyRoutes.post( ...GITHUB_SANDBOX_FALLBACK_ROUTE, authorization: requirePermission("sessions.collaborate"), }), - (c) => handleSlackNotify(c.var.admitted.request, c.env, c.req.param(), c.var.admitted.ctx) + (c) => dispatch(c, handleSlackNotify) ); diff --git a/packages/control-plane/src/routing/admit.ts b/packages/control-plane/src/routing/admit.ts index 83983141fd..b9378e8d2a 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, @@ -12,6 +16,7 @@ import type { import type { Env } from "../types"; import type { ControlPlaneHonoEnv } from "./hono-env"; import { admitRoute, type RouteAdmissionResult } from "./route-admission"; +import { rawRouteParams } from "./route-params"; /** Everything admission and response policy need to know about a route. */ export type AdmissionPolicy = RouteAdmissionPolicy & Pick; @@ -30,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. * @@ -58,12 +107,21 @@ 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); + 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({ diff --git a/packages/control-plane/src/routing/hono-app.test.ts b/packages/control-plane/src/routing/hono-app.test.ts index ad25e1d194..fb0c04a9ad 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/test/integration/rbac-routes.test.ts b/packages/control-plane/test/integration/rbac-routes.test.ts index da92fd9a98..1021ef04f5 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 b474357bf1..50a6b6df20 100644 --- a/packages/control-plane/test/integration/route-admission-matrix.test.ts +++ b/packages/control-plane/test/integration/route-admission-matrix.test.ts @@ -305,6 +305,23 @@ describe("route admission matrix", { timeout: MATRIX_TIMEOUT_MS }, () => { expect(observed).toMatchSnapshot(); }); + 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. diff --git a/packages/control-plane/test/integration/routing-compatibility.test.ts b/packages/control-plane/test/integration/routing-compatibility.test.ts index ebd7fe9996..70439285c5 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 () => {