diff --git a/packages/control-plane/src/router.policy.test.ts b/packages/control-plane/src/router.policy.test.ts index 49ef25aad..7c9e15043 100644 --- a/packages/control-plane/src/router.policy.test.ts +++ b/packages/control-plane/src/router.policy.test.ts @@ -2,10 +2,10 @@ import { describe, expect, it, vi } from "vitest"; import { enforceRoutePrincipal } from "./routing/route-admission"; import { handleRequest, - legacyRoutes, matchRoute, routeContracts as routes, TEST_BACKGROUND_TASK_CONTEXT, + TEST_SERVICE_SECRETS, } from "./router.test-support"; import { serviceAllowsPermission } from "./authorization/service-permissions"; import { SCOPED_PERMISSION_PAIRS } from "@open-inspect/shared/rbac"; @@ -244,23 +244,20 @@ describe("route policy table", () => { }); }); - it("returns 400 for a malformed percent-encoded role ID before querying D1", async () => { - const path = "/roles/%E0%A4%A"; - const { route, match } = matchRoute(legacyRoutes(), "GET", path)!; + it("refuses a malformed percent-encoded role ID before authentication or D1", async () => { + // Hono leaves the segment undecoded; admission refuses it before the + // principal is resolved, so neither authentication nor the role lookup + // touches D1. const prepare = vi.fn(); - const response = await route.handler( - new Request(`https://test.local${path}`), - {} as never, - match, - { - principal: { kind: "user", userId: "user-1" }, - db: { prepare }, - } as never + const response = await handleRequest( + new Request("https://test.local/roles/%E0%A4%A"), + { ...TEST_SERVICE_SECRETS, DB: { prepare } } as never, + TEST_BACKGROUND_TASK_CONTEXT ); expect(response.status).toBe(400); - await expect(response.json()).resolves.toEqual({ error: "Invalid role ID" }); + await expect(response.json()).resolves.toEqual({ error: "Invalid path encoding" }); expect(prepare).not.toHaveBeenCalled(); }); diff --git a/packages/control-plane/src/routes/catalog.ts b/packages/control-plane/src/routes/catalog.ts index 72bc1a728..f65f63bc2 100644 --- a/packages/control-plane/src/routes/catalog.ts +++ b/packages/control-plane/src/routes/catalog.ts @@ -62,22 +62,22 @@ export const catalog: RouteCatalogEntry[] = [ modelPreferencesRoutes, // Subscription provider account management and sandbox access broker - ...modelProviderAccountRoutes, + modelProviderAccountRoutes, // Integration settings - ...integrationSettingsRoutes, + integrationSettingsRoutes, // Deployment-wide commit signing identity - ...commitSigningRoutes, + commitSigningRoutes, // SCM (source-control) settings - ...scmSettingsRoutes, + scmSettingsRoutes, // Automations ...automationRoutes, // MCP servers - ...mcpServerRoutes, + mcpServerRoutes, // Analytics analyticsRoutes, @@ -89,13 +89,13 @@ export const catalog: RouteCatalogEntry[] = [ ...autofixRoutes, // Installation-wide managed skills and personal profiles - ...skillRoutes, + skillRoutes, // Personal keyboard shortcuts keyboardShortcutRoutes, // Workspace roles, members, and current-user authorization - ...rbacRoutes, + rbacRoutes, // Webhooks (public routes — auth handled per-route) ...webhookRoutes, diff --git a/packages/control-plane/src/routes/commit-signing.ts b/packages/control-plane/src/routes/commit-signing.ts index 467d244f8..daa11cee8 100644 --- a/packages/control-plane/src/routes/commit-signing.ts +++ b/packages/control-plane/src/routes/commit-signing.ts @@ -1,4 +1,5 @@ import { commitSigningWriteRequestSchema } from "@open-inspect/shared/types/commit-signing"; +import { Hono } from "hono"; import { OpenSshKeyValidationError, @@ -7,6 +8,8 @@ import { } from "../auth/openssh-ed25519"; import { CommitSigningStore } from "../db/commit-signing"; import type { SqlDatabase } from "../db/sql-database"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { resolveScmProviderFromEnv } from "../source-control"; import type { Env } from "../types"; import { @@ -14,8 +17,6 @@ import { json, parseJsonBody, type RequestContext, - type Route, - defineRoute, GITHUB_USER_OR_SERVICE_ROUTE, SCM_AGNOSTIC_SANDBOX_ROUTE, NO_AUTHORIZATION, @@ -70,7 +71,7 @@ async function readSigningPayload(request: Request): Promise async function handleGetCommitSigning( _request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const store = createStore(env, ctx.db); @@ -87,7 +88,7 @@ async function handleGetCommitSigning( async function handlePutCommitSigning( request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const store = createStore(env, ctx.db); @@ -120,7 +121,7 @@ async function handlePutCommitSigning( async function handleDeleteCommitSigning( _request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const store = createStore(env, ctx.db); @@ -137,12 +138,9 @@ async function handleDeleteCommitSigning( async function handleGetSandboxCommitSigning( _request: Request, env: Env, - match: RegExpMatchArray, + _params: { id: string }, ctx: RequestContext ): Promise { - const sessionId = match.groups?.id; - if (!sessionId) return noStore(error("Session ID required", 400)); - // The bridge runs on every supported SCM deployment. Signing is GitHub-only, // so other providers receive the explicit disabled state required for safe // unsigned execution instead of failing the session at the provider gate. @@ -164,11 +162,9 @@ async function handleGetSandboxCommitSigning( async function handlePostSandboxCommitSigning( request: Request, env: Env, - match: RegExpMatchArray, + _params: { id: string }, ctx: RequestContext ): Promise { - const sessionId = match.groups?.id; - if (!sessionId) return noStore(error("Session ID required", 400)); if (resolveScmProviderFromEnv(env.SCM_PROVIDER) !== "github") { return noStore(error("Commit signing is disabled", 409)); } @@ -212,35 +208,28 @@ async function handlePostSandboxCommitSigning( } } -export const commitSigningRoutes: Route[] = [ - defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { - method: "GET", - path: "/commit-signing", - authorization: requirePermission("integrations.read"), - handler: handleGetCommitSigning, - }), - defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { - method: "PUT", - path: "/commit-signing", - authorization: requirePermission("commit_signing.manage"), - handler: handlePutCommitSigning, - }), - defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { - method: "DELETE", - path: "/commit-signing", - authorization: requirePermission("commit_signing.manage"), - handler: handleDeleteCommitSigning, - }), - defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { - method: "GET", - path: "/sessions/:id/commit-signing", - authorization: NO_AUTHORIZATION, - handler: handleGetSandboxCommitSigning, - }), - defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { - method: "POST", - path: "/sessions/:id/commit-signing", - authorization: NO_AUTHORIZATION, - handler: handlePostSandboxCommitSigning, - }), -]; +const COMMIT_SIGNING_MANAGE = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("commit_signing.manage"), +}); +const SANDBOX = admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, authorization: NO_AUTHORIZATION }); + +export const commitSigningRoutes = new Hono(); + +commitSigningRoutes.get( + "/commit-signing", + admit({ ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("integrations.read") }), + (c) => dispatch(c, handleGetCommitSigning) +); +commitSigningRoutes.put("/commit-signing", COMMIT_SIGNING_MANAGE, (c) => + dispatch(c, handlePutCommitSigning) +); +commitSigningRoutes.delete("/commit-signing", COMMIT_SIGNING_MANAGE, (c) => + dispatch(c, handleDeleteCommitSigning) +); +commitSigningRoutes.get("/sessions/:id/commit-signing", SANDBOX, (c) => + dispatch(c, handleGetSandboxCommitSigning) +); +commitSigningRoutes.post("/sessions/:id/commit-signing", SANDBOX, (c) => + dispatch(c, handlePostSandboxCommitSigning) +); diff --git a/packages/control-plane/src/routes/integration-settings.ts b/packages/control-plane/src/routes/integration-settings.ts index cb84dd56d..f2d948670 100644 --- a/packages/control-plane/src/routes/integration-settings.ts +++ b/packages/control-plane/src/routes/integration-settings.ts @@ -20,28 +20,27 @@ import { isValidIntegrationId, supportsEnvironmentSettings, } from "../db/integration-settings"; +import { Hono } from "hono"; import { EnvironmentStore } from "../db/environments"; import type { Env } from "../types"; import type { SqlDatabase } from "../db/sql-database"; import { createLogger } from "../logger"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; +import { repositoryParams } from "./repository-params"; import { - type Route, type RequestContext, GITHUB_USER_OR_SERVICE_ROUTE, - defineRoutes, json, error, parseJsonBody, - extractRepoParams, requirePermission, } from "./shared"; const logger = createLogger("router:integration-settings"); -function extractIntegrationId(match: RegExpMatchArray): IntegrationId | null { - const id = match.groups?.id; - if (!id || !isValidIntegrationId(id)) return null; - return id; +function integrationId(id: string): IntegrationId | null { + return isValidIntegrationId(id) ? id : null; } /** @@ -50,9 +49,9 @@ function extractIntegrationId(match: RegExpMatchArray): IntegrationId | null { * environment id, and — because the settings table is an owned child of * `environments` — an environment that actually exists. */ -async function extractEnvironmentSettingsParams( +async function environmentSettingsParams( db: SqlDatabase, - match: RegExpMatchArray + params: { id: string; environmentId: string } ): Promise< | { integrationId: EnvironmentSettingsIntegrationId; @@ -61,14 +60,13 @@ async function extractEnvironmentSettingsParams( } | Response > { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); if (!supportsEnvironmentSettings(id)) { return error(`Integration ${id} does not support environment-level settings`, 400); } - const environmentId = match.groups?.environmentId; - if (!environmentId) return error("Environment ID required", 400); + const { environmentId } = params; const environmentStore = new EnvironmentStore(db); if (!(await environmentStore.getById(environmentId))) { @@ -81,11 +79,11 @@ async function extractEnvironmentSettingsParams( async function handleGetIntegrationSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); const store = new IntegrationSettingsStore(ctx.db); const settings = await store.getGlobal(id); @@ -95,11 +93,11 @@ async function handleGetIntegrationSettings( async function handleSetIntegrationSettings( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); const body = await parseJsonBody<{ settings?: Record }>(request); if (body instanceof Response) return body; @@ -137,11 +135,11 @@ async function handleSetIntegrationSettings( async function handleDeleteIntegrationSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); const store = new IntegrationSettingsStore(ctx.db); @@ -169,11 +167,11 @@ async function handleDeleteIntegrationSettings( async function handleListRepoSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); const store = new IntegrationSettingsStore(ctx.db); const repos = await store.listRepoSettings(id); @@ -183,15 +181,15 @@ async function handleListRepoSettings( async function handleGetRepoSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; owner: string; name: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); - const params = extractRepoParams(match); - if (params instanceof Response) return params; - const { owner, name } = params; + const repository = repositoryParams(params); + if (repository instanceof Response) return repository; + const { owner, name } = repository; const repo = `${owner}/${name}`; @@ -203,15 +201,15 @@ async function handleGetRepoSettings( async function handleSetRepoSettings( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; owner: string; name: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); - const params = extractRepoParams(match); - if (params instanceof Response) return params; - const { owner, name } = params; + const repository = repositoryParams(params); + if (repository instanceof Response) return repository; + const { owner, name } = repository; const body = await parseJsonBody<{ settings?: Record }>(request); if (body instanceof Response) return body; @@ -251,15 +249,15 @@ async function handleSetRepoSettings( async function handleDeleteRepoSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; owner: string; name: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); - const params = extractRepoParams(match); - if (params instanceof Response) return params; - const { owner, name } = params; + const repository = repositoryParams(params); + if (repository instanceof Response) return repository; + const { owner, name } = repository; const store = new IntegrationSettingsStore(ctx.db); const repo = `${owner}/${name}`; @@ -289,12 +287,12 @@ async function handleDeleteRepoSettings( async function handleGetEnvironmentSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; environmentId: string }, ctx: RequestContext ): Promise { - const params = await extractEnvironmentSettingsParams(ctx.db, match); - if (params instanceof Response) return params; - const { integrationId, environmentId, store } = params; + const settingsParams = await environmentSettingsParams(ctx.db, params); + if (settingsParams instanceof Response) return settingsParams; + const { integrationId, environmentId, store } = settingsParams; const settings = await store.getEnvironmentSettings(integrationId, environmentId); return json({ integrationId, environmentId, settings }); @@ -303,12 +301,12 @@ async function handleGetEnvironmentSettings( async function handleSetEnvironmentSettings( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; environmentId: string }, ctx: RequestContext ): Promise { - const params = await extractEnvironmentSettingsParams(ctx.db, match); - if (params instanceof Response) return params; - const { integrationId, environmentId, store } = params; + const settingsParams = await environmentSettingsParams(ctx.db, params); + if (settingsParams instanceof Response) return settingsParams; + const { integrationId, environmentId, store } = settingsParams; const body = await parseJsonBody<{ settings?: Record }>(request); if (body instanceof Response) return body; @@ -345,12 +343,12 @@ async function handleSetEnvironmentSettings( async function handleDeleteEnvironmentSettings( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; environmentId: string }, ctx: RequestContext ): Promise { - const params = await extractEnvironmentSettingsParams(ctx.db, match); - if (params instanceof Response) return params; - const { integrationId, environmentId, store } = params; + const settingsParams = await environmentSettingsParams(ctx.db, params); + if (settingsParams instanceof Response) return settingsParams; + const { integrationId, environmentId, store } = settingsParams; try { await store.deleteEnvironmentSettings(integrationId, environmentId); @@ -377,15 +375,15 @@ async function handleDeleteEnvironmentSettings( async function handleGetResolvedConfig( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; owner: string; name: string }, ctx: RequestContext ): Promise { - const id = extractIntegrationId(match); - if (!id) return error(`Unknown integration: ${match.groups?.id}`, 404); + const id = integrationId(params.id); + if (!id) return error(`Unknown integration: ${params.id}`, 404); - const params = extractRepoParams(match); - if (params instanceof Response) return params; - const { owner, name } = params; + const repository = repositoryParams(params); + if (repository instanceof Response) return repository; + const { owner, name } = repository; const store = new IntegrationSettingsStore(ctx.db); const repo = `${owner}/${name}`; @@ -487,83 +485,89 @@ async function handleGetResolvedConfig( return error(`Unsupported integration: ${id}`, 400); } -export const integrationSettingsRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - // Integration settings — global - { - method: "GET", - path: "/integration-settings/:id", +const INTEGRATIONS_READ = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("integrations.read"), +}); +const INTEGRATIONS_MANAGE = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("integrations.manage"), +}); +const REPO_SETTINGS_MANAGE = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("repositories.settings.manage"), +}); +const ENVIRONMENT_SETTINGS_MANAGE = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("environments.settings.manage"), +}); + +export const integrationSettingsRoutes = new Hono(); + +// Integration settings — global +integrationSettingsRoutes.get( + "/integration-settings/:id", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("integrations.read", { actorlessGrants: [{ service: "slack-bot", pathParams: { id: "slack" } }], }), - handler: handleGetIntegrationSettings, - }, - { - method: "PUT", - path: "/integration-settings/:id", - authorization: requirePermission("integrations.manage"), - handler: handleSetIntegrationSettings, - }, - { - method: "DELETE", - path: "/integration-settings/:id", - authorization: requirePermission("integrations.manage"), - handler: handleDeleteIntegrationSettings, - }, - // Integration settings — per-repo - { - method: "GET", - path: "/integration-settings/:id/repos", - authorization: requirePermission("integrations.read"), - handler: handleListRepoSettings, - }, - { - method: "GET", - path: "/integration-settings/:id/repos/:owner/:name", - authorization: requirePermission("integrations.read"), - handler: handleGetRepoSettings, - }, - { - method: "PUT", - path: "/integration-settings/:id/repos/:owner/:name", - authorization: requirePermission("repositories.settings.manage"), - handler: handleSetRepoSettings, - }, - { - method: "DELETE", - path: "/integration-settings/:id/repos/:owner/:name", - authorization: requirePermission("repositories.settings.manage"), - handler: handleDeleteRepoSettings, - }, - // Integration settings — per-environment (design §13.5; sandbox and - // code-server, and VNC only) - { - method: "GET", - path: "/integration-settings/:id/environments/:environmentId", - authorization: requirePermission("integrations.read"), - handler: handleGetEnvironmentSettings, - }, - { - method: "PUT", - path: "/integration-settings/:id/environments/:environmentId", - authorization: requirePermission("environments.settings.manage"), - handler: handleSetEnvironmentSettings, - }, - { - method: "DELETE", - path: "/integration-settings/:id/environments/:environmentId", - authorization: requirePermission("environments.settings.manage"), - handler: handleDeleteEnvironmentSettings, - }, - // Resolved config — used by bots at runtime - { - method: "GET", - path: "/integration-settings/:id/resolved/:owner/:name", + }), + (c) => dispatch(c, handleGetIntegrationSettings) +); +integrationSettingsRoutes.put("/integration-settings/:id", INTEGRATIONS_MANAGE, (c) => + dispatch(c, handleSetIntegrationSettings) +); +integrationSettingsRoutes.delete("/integration-settings/:id", INTEGRATIONS_MANAGE, (c) => + dispatch(c, handleDeleteIntegrationSettings) +); +// Integration settings — per-repo +integrationSettingsRoutes.get("/integration-settings/:id/repos", INTEGRATIONS_READ, (c) => + dispatch(c, handleListRepoSettings) +); +integrationSettingsRoutes.get( + "/integration-settings/:id/repos/:owner/:name", + INTEGRATIONS_READ, + (c) => dispatch(c, handleGetRepoSettings) +); +integrationSettingsRoutes.put( + "/integration-settings/:id/repos/:owner/:name", + REPO_SETTINGS_MANAGE, + (c) => dispatch(c, handleSetRepoSettings) +); +integrationSettingsRoutes.delete( + "/integration-settings/:id/repos/:owner/:name", + REPO_SETTINGS_MANAGE, + (c) => dispatch(c, handleDeleteRepoSettings) +); +// Integration settings — per-environment (design §13.5; sandbox and +// code-server, and VNC only) +integrationSettingsRoutes.get( + "/integration-settings/:id/environments/:environmentId", + INTEGRATIONS_READ, + (c) => dispatch(c, handleGetEnvironmentSettings) +); +integrationSettingsRoutes.put( + "/integration-settings/:id/environments/:environmentId", + ENVIRONMENT_SETTINGS_MANAGE, + (c) => dispatch(c, handleSetEnvironmentSettings) +); +integrationSettingsRoutes.delete( + "/integration-settings/:id/environments/:environmentId", + ENVIRONMENT_SETTINGS_MANAGE, + (c) => dispatch(c, handleDeleteEnvironmentSettings) +); +// Resolved config — used by bots at runtime +integrationSettingsRoutes.get( + "/integration-settings/:id/resolved/:owner/:name", + admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, authorization: requirePermission("integrations.read", { actorlessGrants: [ { service: "github-bot", pathParams: { id: "github" } }, { service: "linear-bot", pathParams: { id: "linear" } }, ], }), - handler: handleGetResolvedConfig, - }, -]); + }), + (c) => dispatch(c, handleGetResolvedConfig) +); diff --git a/packages/control-plane/src/routes/mcp-servers.ts b/packages/control-plane/src/routes/mcp-servers.ts index 6101e1751..6ad0b4307 100644 --- a/packages/control-plane/src/routes/mcp-servers.ts +++ b/packages/control-plane/src/routes/mcp-servers.ts @@ -7,13 +7,14 @@ import { McpServerStore, McpServerValidationError, } from "../db/mcp-servers"; +import { Hono } from "hono"; import type { Env } from "../types"; import { createLogger } from "../logger"; import { requireRepoSecretsEncryptionKey } from "../env-validation"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { - type Route, GITHUB_USER_OR_SERVICE_ROUTE, - defineRoutes, type RequestContext, json, error, @@ -26,7 +27,7 @@ const logger = createLogger("router:mcp-servers"); async function handleListMcpServers( request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { if (!ctx.db) return error("Database not configured", 503); @@ -48,11 +49,10 @@ async function handleListMcpServers( async function handleGetMcpServer( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = match.groups?.id; - if (!id) return error("Missing server ID", 400); + const { id } = params; if (!ctx.db) return error("Database not configured", 503); const store = new McpServerStore(ctx.db, requireRepoSecretsEncryptionKey(env)); @@ -70,7 +70,7 @@ async function handleGetMcpServer( async function handleCreateMcpServer( request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { if (!ctx.db) return error("Database not configured", 503); @@ -103,11 +103,10 @@ async function handleCreateMcpServer( async function handleUpdateMcpServer( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = match.groups?.id; - if (!id) return error("Missing server ID", 400); + const { id } = params; if (!ctx.db) return error("Database not configured", 503); const body = await parseJsonBody(request); @@ -143,11 +142,10 @@ async function handleUpdateMcpServer( async function handleDeleteMcpServer( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = match.groups?.id; - if (!id) return error("Missing server ID", 400); + const { id } = params; if (!ctx.db) return error("Database not configured", 503); const store = new McpServerStore(ctx.db, requireRepoSecretsEncryptionKey(env)); @@ -163,35 +161,19 @@ async function handleDeleteMcpServer( return json({ ok: true }); } -export const mcpServerRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - { - method: "GET", - path: "/mcp-servers", - authorization: requirePermission("mcp_servers.read"), - handler: handleListMcpServers, - }, - { - method: "POST", - path: "/mcp-servers", - authorization: requirePermission("mcp_servers.manage"), - handler: handleCreateMcpServer, - }, - { - method: "GET", - path: "/mcp-servers/:id", - authorization: requirePermission("mcp_servers.read"), - handler: handleGetMcpServer, - }, - { - method: "PUT", - path: "/mcp-servers/:id", - authorization: requirePermission("mcp_servers.manage"), - handler: handleUpdateMcpServer, - }, - { - method: "DELETE", - path: "/mcp-servers/:id", - authorization: requirePermission("mcp_servers.manage"), - handler: handleDeleteMcpServer, - }, -]); +const MCP_READ = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("mcp_servers.read"), +}); +const MCP_MANAGE = admit({ + ...GITHUB_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("mcp_servers.manage"), +}); + +export const mcpServerRoutes = new Hono(); + +mcpServerRoutes.get("/mcp-servers", MCP_READ, (c) => dispatch(c, handleListMcpServers)); +mcpServerRoutes.post("/mcp-servers", MCP_MANAGE, (c) => dispatch(c, handleCreateMcpServer)); +mcpServerRoutes.get("/mcp-servers/:id", MCP_READ, (c) => dispatch(c, handleGetMcpServer)); +mcpServerRoutes.put("/mcp-servers/:id", MCP_MANAGE, (c) => dispatch(c, handleUpdateMcpServer)); +mcpServerRoutes.delete("/mcp-servers/:id", MCP_MANAGE, (c) => dispatch(c, handleDeleteMcpServer)); diff --git a/packages/control-plane/src/routes/model-provider-accounts.ts b/packages/control-plane/src/routes/model-provider-accounts.ts index bc4975861..246d4f2c7 100644 --- a/packages/control-plane/src/routes/model-provider-accounts.ts +++ b/packages/control-plane/src/routes/model-provider-accounts.ts @@ -10,6 +10,7 @@ import { subscriptionProviderIdSchema, type SubscriptionProviderId, } from "@open-inspect/shared/types/provider-accounts"; +import { Hono } from "hono"; import { z } from "zod"; import { createLogger } from "../logger"; import { generateId } from "../auth/crypto"; @@ -41,20 +42,19 @@ import { ProviderAccountSelectionPolicy, ProviderAccountSelectionPolicyError, } from "../model-provider-accounts/selection-policy"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import type { Env } from "../types"; import { SessionInternalPaths } from "../session/contracts"; import { createSessionRuntimeClient } from "../session/runtime-client"; import { - defineRoute, error, json, parseJsonBody, SCM_AGNOSTIC_HUMAN_USER_ROUTE, SCM_AGNOSTIC_SANDBOX_ROUTE, type RequestContext, - type Route, type SandboxRouteContext, - type UserRouteContext, NO_AUTHORIZATION, requirePermission, } from "./shared"; @@ -104,15 +104,13 @@ function authorizationService(env: Env, ctx: RequestContext): ProviderDeviceAuth ); } -function provider(value: string | undefined): SubscriptionProviderId | Response { - if (!value) return error("Provider required", 400); +function provider(value: string): SubscriptionProviderId | Response { const parsed = subscriptionProviderIdSchema.safeParse(value); return parsed.success ? parsed.data : error("Unsupported model provider", 400); } -function accountId(match: RegExpMatchArray): string | Response { - const id = match.groups?.id; - return id && MODEL_PROVIDER_ACCOUNT_ID_PATTERN.test(id) +function accountId(id: string): string | Response { + return MODEL_PROVIDER_ACCOUNT_ID_PATTERN.test(id) ? id : error("Invalid provider account ID", 400); } @@ -162,42 +160,32 @@ async function authorizationOperation( } } -function authorizationId(match: RegExpMatchArray): string | Response { - const id = match.groups?.id; - return id && PROVIDER_DEVICE_AUTHORIZATION_ID_PATTERN.test(id) +function authorizationId(id: string): string | Response { + return PROVIDER_DEVICE_AUTHORIZATION_ID_PATTERN.test(id) ? id : error("Authorization transaction not found", 404); } -function managementRoute( - method: string, - path: string, - handler: ( - request: Request, - env: Env, - match: RegExpMatchArray, - ctx: UserRouteContext - ) => Promise -): Route { - return defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { - method, - path: path, - cacheControl: PRIVATE_NO_STORE, - authorization: requirePermission( - method === "GET" ? "provider_accounts.read" : "provider_accounts.manage" - ), - handler, - }); -} +const ACCOUNTS_READ = admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + cacheControl: PRIVATE_NO_STORE, + authorization: requirePermission("provider_accounts.read"), +}); +const ACCOUNTS_MANAGE = admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + cacheControl: PRIVATE_NO_STORE, + authorization: requirePermission("provider_accounts.manage"), +}); -const managementRoutes: Route[] = [ - managementRoute( - "GET", - "/model-provider-accounts/legacy-credentials", - async (_request, _env, _match, ctx) => - json({ legacyKeys: await listLegacyProviderCredentials(ctx.db) }) - ), - managementRoute("GET", "/model-provider-accounts", async (request, env, _match, ctx) => { +export const modelProviderAccountRoutes = new Hono(); + +modelProviderAccountRoutes.get("/model-provider-accounts/legacy-credentials", ACCOUNTS_READ, (c) => + dispatch(c, async (_request, _env, _params, ctx) => + json({ legacyKeys: await listLegacyProviderCredentials(ctx.db) }) + ) +); +modelProviderAccountRoutes.get("/model-provider-accounts", ACCOUNTS_READ, (c) => + dispatch(c, async (request, env, _params, ctx) => { const accounts = service(env, ctx); const url = new URL(request.url); const providerFilter = url.searchParams.get("provider"); @@ -216,8 +204,10 @@ const managementRoutes: Route[] = [ return json({ accounts: status ? listed.filter((account) => account.status === status) : listed, }); - }), - managementRoute("POST", "/model-provider-accounts", async (request, env, _match, ctx) => { + }) +); +modelProviderAccountRoutes.post("/model-provider-accounts", ACCOUNTS_MANAGE, (c) => + dispatch(c, async (request, env, _params, ctx) => { const body = await parseJsonBody(request); if (body instanceof Response) return body; const parsed = connectModelProviderAccountRequestSchema.safeParse(body); @@ -227,12 +217,14 @@ const managementRoutes: Route[] = [ const result = await accounts.create(parsed.data, ctx.principal.userId); return json(result, result.reconnectedExisting ? 200 : 201); }); - }), - managementRoute( - "POST", - "/model-provider-accounts/:provider/device-authorizations", - async (request, env, match, ctx) => { - const parsedProvider = provider(match.groups?.provider); + }) +); +modelProviderAccountRoutes.post( + "/model-provider-accounts/:provider/device-authorizations", + ACCOUNTS_MANAGE, + (c) => + dispatch(c, async (request, env, params, ctx) => { + const parsedProvider = provider(params.provider); if (parsedProvider instanceof Response) return parsedProvider; const body = await parseJsonBody(request); if (body instanceof Response) return body; @@ -248,43 +240,48 @@ const managementRoutes: Route[] = [ 201 ) ); - } - ), - managementRoute( - "POST", - "/model-provider-accounts/:provider/device-authorizations/:id/poll", - async (_request, env, match, ctx) => { - const parsedProvider = provider(match.groups?.provider); + }) +); +modelProviderAccountRoutes.post( + "/model-provider-accounts/:provider/device-authorizations/:id/poll", + ACCOUNTS_MANAGE, + (c) => + dispatch(c, async (_request, env, params, ctx) => { + const parsedProvider = provider(params.provider); if (parsedProvider instanceof Response) return parsedProvider; - const id = authorizationId(match); + const id = authorizationId(params.id); if (id instanceof Response) return id; return authorizationOperation(ctx, async () => json(await authorizationService(env, ctx).poll(ctx.principal.userId, parsedProvider, id)) ); - } - ), - managementRoute( - "DELETE", - "/model-provider-accounts/:provider/device-authorizations/:id", - async (_request, env, match, ctx) => { - const parsedProvider = provider(match.groups?.provider); + }) +); +modelProviderAccountRoutes.delete( + "/model-provider-accounts/:provider/device-authorizations/:id", + ACCOUNTS_MANAGE, + (c) => + dispatch(c, async (_request, env, params, ctx) => { + const parsedProvider = provider(params.provider); if (parsedProvider instanceof Response) return parsedProvider; - const id = authorizationId(match); + const id = authorizationId(params.id); if (id instanceof Response) return id; return authorizationOperation(ctx, async () => { await authorizationService(env, ctx).cancel(ctx.principal.userId, parsedProvider, id); return new Response(null, { status: 204 }); }); - } - ), - managementRoute("GET", "/model-provider-accounts/:id", async (_request, env, match, ctx) => { - const id = accountId(match); + }) +); +modelProviderAccountRoutes.get("/model-provider-accounts/:id", ACCOUNTS_READ, (c) => + dispatch(c, async (_request, env, params, ctx) => { + const id = accountId(params.id); if (id instanceof Response) return id; const accounts = service(env, ctx); return accountOperation(ctx, async () => json({ account: await accounts.get(id) })); - }), - managementRoute("PATCH", "/model-provider-accounts/:id", async (request, env, match, ctx) => { - const id = accountId(match); + }) +); +modelProviderAccountRoutes.patch("/model-provider-accounts/:id", ACCOUNTS_MANAGE, (c) => + dispatch(c, async (request, env, params, ctx) => { + const id = accountId(params.id); if (id instanceof Response) return id; const body = await parseJsonBody(request); if (body instanceof Response) return body; @@ -294,108 +291,107 @@ const managementRoutes: Route[] = [ return accountOperation(ctx, async () => json({ account: await accounts.rename(id, parsed.data.displayName, ctx.principal.userId) }) ); - }), - ...(["verify", "disable", "enable"] as const).map((action) => - managementRoute( - "POST", - `/model-provider-accounts/:id/${action}`, - async (_request, env, match, ctx) => { - const id = accountId(match); - if (id instanceof Response) return id; - const accounts = service(env, ctx); - return accountOperation(ctx, async () => { - const account = - action === "verify" - ? await accounts.verify(id, ctx.principal.userId) - : await accounts.setStatus( - id, - action === "enable" ? "active" : "disabled", - ctx.principal.userId - ); - return json({ account }); - }); - } - ) - ), - managementRoute( - "POST", - "/model-provider-accounts/:id/reconnect", - async (request, env, match, ctx) => { - const id = accountId(match); + }) +); +for (const action of ["verify", "disable", "enable"] as const) { + modelProviderAccountRoutes.post(`/model-provider-accounts/:id/${action}`, ACCOUNTS_MANAGE, (c) => + dispatch(c, async (_request, env, params, ctx) => { + const id = accountId(params.id); if (id instanceof Response) return id; - const body = await parseJsonBody(request); - if (body instanceof Response) return body; - const parsed = reconnectModelProviderAccountRequestSchema.safeParse(body); - if (!parsed.success) return error("Invalid provider account reconnect request", 400); const accounts = service(env, ctx); - return accountOperation(ctx, async () => - json({ account: await accounts.reconnect(id, parsed.data, ctx.principal.userId) }) - ); - } - ), - managementRoute("DELETE", "/model-provider-accounts/:id", async (_request, env, match, ctx) => { - const id = accountId(match); + return accountOperation(ctx, async () => { + const account = + action === "verify" + ? await accounts.verify(id, ctx.principal.userId) + : await accounts.setStatus( + id, + action === "enable" ? "active" : "disabled", + ctx.principal.userId + ); + return json({ account }); + }); + }) + ); +} +modelProviderAccountRoutes.post("/model-provider-accounts/:id/reconnect", ACCOUNTS_MANAGE, (c) => + dispatch(c, async (request, env, params, ctx) => { + const id = accountId(params.id); + if (id instanceof Response) return id; + const body = await parseJsonBody(request); + if (body instanceof Response) return body; + const parsed = reconnectModelProviderAccountRequestSchema.safeParse(body); + if (!parsed.success) return error("Invalid provider account reconnect request", 400); + const accounts = service(env, ctx); + return accountOperation(ctx, async () => + json({ account: await accounts.reconnect(id, parsed.data, ctx.principal.userId) }) + ); + }) +); +modelProviderAccountRoutes.delete("/model-provider-accounts/:id", ACCOUNTS_MANAGE, (c) => + dispatch(c, async (_request, env, params, ctx) => { + const id = accountId(params.id); if (id instanceof Response) return id; const accounts = service(env, ctx); return accountOperation(ctx, async () => { await accounts.archive(id, ctx.principal.userId); return new Response(null, { status: 204 }); }); - }), - managementRoute("GET", "/model-provider-account-defaults", async (_request, _env, _match, ctx) => + }) +); +modelProviderAccountRoutes.get("/model-provider-account-defaults", ACCOUNTS_READ, (c) => + dispatch(c, async (_request, _env, _params, ctx) => json({ defaults: await new ProviderDefaultStore(ctx.db).list() }) - ), - managementRoute( - "PUT", - "/model-provider-account-defaults/:provider", - async (request, _env, match, ctx) => { - const parsedProvider = provider(match.groups?.provider); - if (parsedProvider instanceof Response) return parsedProvider; - const body = await parseJsonBody(request); - if (body instanceof Response) return body; - const parsed = modelProviderAccountDefaultRequestSchema.safeParse(body); - if (!parsed.success) return error("Invalid provider default", 400); - const defaults = new ProviderDefaultStore(ctx.db); - try { - await new ProviderAccountSelectionPolicy( - new ModelProviderAccountStore(ctx.db), - modelProviderAccountAdapterRegistry - ).validateDefault(parsedProvider, parsed.data.providerAccountId); - await defaults.set( - parsedProvider, - parsed.data.providerAccountId, - parsed.data.unattendedMode, - ctx.principal.userId - ); - return json({ default: await defaults.get(parsedProvider) }); - } catch (cause) { - if (cause instanceof ProviderAccountSelectionPolicyError) { - return error(cause.message, cause.status); - } - if (cause instanceof ProviderDefaultConstraintError) { - return error(cause.message, 409); - } - logger.error("provider_account.default_update_failed", { - event: "provider_account.default_update_failed", - request_id: ctx.request_id, - trace_id: ctx.trace_id, - error: cause instanceof Error ? cause : String(cause), - }); - return error("Provider default could not be updated", 502); + ) +); +modelProviderAccountRoutes.put("/model-provider-account-defaults/:provider", ACCOUNTS_MANAGE, (c) => + dispatch(c, async (request, _env, params, ctx) => { + const parsedProvider = provider(params.provider); + if (parsedProvider instanceof Response) return parsedProvider; + const body = await parseJsonBody(request); + if (body instanceof Response) return body; + const parsed = modelProviderAccountDefaultRequestSchema.safeParse(body); + if (!parsed.success) return error("Invalid provider default", 400); + const defaults = new ProviderDefaultStore(ctx.db); + try { + await new ProviderAccountSelectionPolicy( + new ModelProviderAccountStore(ctx.db), + modelProviderAccountAdapterRegistry + ).validateDefault(parsedProvider, parsed.data.providerAccountId); + await defaults.set( + parsedProvider, + parsed.data.providerAccountId, + parsed.data.unattendedMode, + ctx.principal.userId + ); + return json({ default: await defaults.get(parsedProvider) }); + } catch (cause) { + if (cause instanceof ProviderAccountSelectionPolicyError) { + return error(cause.message, cause.status); } + if (cause instanceof ProviderDefaultConstraintError) { + return error(cause.message, 409); + } + logger.error("provider_account.default_update_failed", { + event: "provider_account.default_update_failed", + request_id: ctx.request_id, + trace_id: ctx.trace_id, + error: cause instanceof Error ? cause : String(cause), + }); + return error("Provider default could not be updated", 502); } - ), - managementRoute( - "DELETE", - "/model-provider-account-defaults/:provider", - async (_request, _env, match, ctx) => { - const parsedProvider = provider(match.groups?.provider); + }) +); +modelProviderAccountRoutes.delete( + "/model-provider-account-defaults/:provider", + ACCOUNTS_MANAGE, + (c) => + dispatch(c, async (_request, _env, params, ctx) => { + const parsedProvider = provider(params.provider); if (parsedProvider instanceof Response) return parsedProvider; await new ProviderDefaultStore(ctx.db).remove(parsedProvider); return new Response(null, { status: 204 }); - } - ), -]; + }) +); async function handleLegacyProviderAccess( env: Env, @@ -424,12 +420,11 @@ async function handleLegacyProviderAccess( async function handleProviderAccess( _request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string; provider: string }, ctx: SandboxRouteContext ): Promise { - const sessionId = match.groups?.id; - const parsedProvider = provider(match.groups?.provider); - if (!sessionId) return error("Session ID required", 400); + const sessionId = params.id; + const parsedProvider = provider(params.provider); if (parsedProvider instanceof Response) return parsedProvider; let binding; try { @@ -479,13 +474,8 @@ async function handleProviderAccess( } } -export const modelProviderAccountRoutes: Route[] = [ - ...managementRoutes, - defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { - method: "POST", - path: "/sessions/:id/provider-auth/:provider/access-token", - cacheControl: NO_STORE, - authorization: NO_AUTHORIZATION, - handler: handleProviderAccess, - }), -]; +modelProviderAccountRoutes.post( + "/sessions/:id/provider-auth/:provider/access-token", + admit({ ...SCM_AGNOSTIC_SANDBOX_ROUTE, cacheControl: NO_STORE, authorization: NO_AUTHORIZATION }), + (c) => dispatch(c, handleProviderAccess) +); diff --git a/packages/control-plane/src/routes/rbac.ts b/packages/control-plane/src/routes/rbac.ts index 4e95dd0c6..90178321b 100644 --- a/packages/control-plane/src/routes/rbac.ts +++ b/packages/control-plane/src/routes/rbac.ts @@ -3,18 +3,19 @@ import { replaceMemberRoleInputSchema, replaceMemberStatusInputSchema, } from "@open-inspect/shared/rbac"; +import { Hono } from "hono"; import { ZodError } from "zod"; import { AuthorizationError, AuthorizationService, RbacConflictError, } from "../authorization/service"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import type { Env } from "../types"; -import type { Route } from "./shared"; import { AUTHENTICATED_USER, SCM_AGNOSTIC_HUMAN_USER_ROUTE, - defineRoutes, error, json, parseJsonBody, @@ -40,18 +41,10 @@ function rbacErrorResponse(cause: unknown): Response { return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); } -function decodePathSegment(value: string): string | null { - try { - return decodeURIComponent(value); - } catch { - return null; - } -} - async function handleGetCurrentAuthorization( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: UserRouteContext ): Promise { const service = new AuthorizationService(ctx.db); @@ -65,7 +58,7 @@ async function handleGetCurrentAuthorization( async function handleListRoles( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: UserRouteContext ): Promise { const service = new AuthorizationService(ctx.db); @@ -79,14 +72,12 @@ async function handleListRoles( async function handleGetRole( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: UserRouteContext ): Promise { const service = new AuthorizationService(ctx.db); try { - const roleId = decodePathSegment(match.groups!.id); - if (roleId === null) return error("Invalid role ID", 400); - const role = await service.getRole(roleId); + const role = await service.getRole(params.id); return role ? json(role) : error("Role not found", 404); } catch (cause) { return rbacErrorResponse(cause); @@ -96,7 +87,7 @@ async function handleGetRole( async function handleListMembers( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: UserRouteContext ): Promise { const service = new AuthorizationService(ctx.db); @@ -110,13 +101,11 @@ async function handleListMembers( async function handleReplaceMemberRole( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: UserRouteContext ): Promise { - const targetUserId = decodePathSegment(match.groups!.id); - if (targetUserId === null || !isCanonicalUserId(targetUserId)) { - return error("Invalid user ID", 400); - } + const targetUserId = params.id; + if (!isCanonicalUserId(targetUserId)) return error("Invalid user ID", 400); const body = await parseJsonBody(request); if (body instanceof Response) return body; const service = new AuthorizationService(ctx.db); @@ -137,13 +126,11 @@ async function handleReplaceMemberRole( async function handleReplaceMemberStatus( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: UserRouteContext ): Promise { - const targetUserId = decodePathSegment(match.groups!.id); - if (targetUserId === null || !isCanonicalUserId(targetUserId)) { - return error("Invalid user ID", 400); - } + const targetUserId = params.id; + if (!isCanonicalUserId(targetUserId)) return error("Invalid user ID", 400); const body = await parseJsonBody(request); if (body instanceof Response) return body; const service = new AuthorizationService(ctx.db); @@ -161,47 +148,52 @@ async function handleReplaceMemberStatus( } } -export const rbacRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_HUMAN_USER_ROUTE, [ - { - method: "GET", - path: "/me/authorization", +const PRIVATE_NO_STORE = { cacheControl: "private, no-store" } as const; + +export const rbacRoutes = new Hono(); + +rbacRoutes.get( + "/me/authorization", + admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + ...PRIVATE_NO_STORE, authorization: AUTHENTICATED_USER, - cacheControl: "private, no-store", - handler: handleGetCurrentAuthorization, - }, - { - method: "GET", - path: "/roles", + }), + (c) => dispatch(c, handleGetCurrentAuthorization) +); +rbacRoutes.get( + "/roles", + admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + ...PRIVATE_NO_STORE, authorization: requirePermission("workspace.roles.read"), - cacheControl: "private, no-store", - handler: handleListRoles, - }, - { - method: "GET", - path: "/roles/:id", + }), + (c) => dispatch(c, handleListRoles) +); +rbacRoutes.get( + "/roles/:id", + admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + ...PRIVATE_NO_STORE, authorization: requirePermission("workspace.roles.read"), - cacheControl: "private, no-store", - handler: handleGetRole, - }, - { - method: "GET", - path: "/members", + }), + (c) => dispatch(c, handleGetRole) +); +rbacRoutes.get( + "/members", + admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + ...PRIVATE_NO_STORE, authorization: requirePermission("workspace.members.read"), - cacheControl: "private, no-store", - handler: handleListMembers, - }, - { - method: "PUT", - path: "/members/:id/role", - authorization: requirePermission("workspace.members.manage"), - cacheControl: "private, no-store", - handler: handleReplaceMemberRole, - }, - { - method: "PUT", - path: "/members/:id/status", - authorization: requirePermission("workspace.members.manage"), - cacheControl: "private, no-store", - handler: handleReplaceMemberStatus, - }, -]); + }), + (c) => dispatch(c, handleListMembers) +); +const MEMBERS_MANAGE = admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + ...PRIVATE_NO_STORE, + authorization: requirePermission("workspace.members.manage"), +}); +rbacRoutes.put("/members/:id/role", MEMBERS_MANAGE, (c) => dispatch(c, handleReplaceMemberRole)); +rbacRoutes.put("/members/:id/status", MEMBERS_MANAGE, (c) => + dispatch(c, handleReplaceMemberStatus) +); diff --git a/packages/control-plane/src/routes/repository-params.ts b/packages/control-plane/src/routes/repository-params.ts new file mode 100644 index 000000000..cf3b50c2b --- /dev/null +++ b/packages/control-plane/src/routes/repository-params.ts @@ -0,0 +1,18 @@ +import { validateRepositoryPathSegments } from "@open-inspect/shared/types/repositories"; +import { error } from "../http/responses"; + +/** + * The repository a route's `:owner/:name` parameters name, or the 400 the + * route answers when they are not a canonical pair. Hono decoded the + * segments once before admission; the shared identity module owns the rule. + */ +export function repositoryParams(params: { + owner: string; + name: string; +}): { owner: string; name: string } | Response { + const repository = validateRepositoryPathSegments(params.owner, params.name); + if (!repository) { + return error("Owner and name must be valid repository path segments", 400); + } + return { owner: repository.repoOwner, name: repository.repoName }; +} diff --git a/packages/control-plane/src/routes/scm-settings.test.ts b/packages/control-plane/src/routes/scm-settings.test.ts index c12ad8b65..09d7acf51 100644 --- a/packages/control-plane/src/routes/scm-settings.test.ts +++ b/packages/control-plane/src/routes/scm-settings.test.ts @@ -1,39 +1,127 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { BUILT_IN_ROLE_REGISTRY } from "@open-inspect/shared/rbac"; +import type * as AuthenticateModule from "../auth/authenticate"; +import type * as ScmSettingsModule from "../db/scm-settings"; +import type { SqlDatabase, SqlStatement } from "../db/sql-database"; +import { + createTestRequestHandler, + TEST_BACKGROUND_TASK_CONTEXT, + TEST_SERVICE_SECRETS, +} from "../router.test-support"; +import type { Env } from "../types"; import { scmSettingsRoutes } from "./scm-settings"; -import type { RequestContext, Route } from "./shared"; -import { matchRoute, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; -function findRoute(method: string, path: string): { route: Route; match: RegExpMatchArray } { - const matched = matchRoute(scmSettingsRoutes, method, path); - if (!matched) throw new Error(`Missing ${method} ${path} route`); - return { route: matched.route, match: matched.match }; -} +const mocks = vi.hoisted(() => ({ + authenticate: vi.fn(), + store: { + getGlobal: vi.fn(), + setGlobal: vi.fn(), + deleteGlobal: vi.fn(), + listRepoSettings: vi.fn(), + setRepoSettings: vi.fn(), + deleteRepoSettings: vi.fn(), + }, +})); + +vi.mock("../auth/authenticate", async (importOriginal) => ({ + ...(await importOriginal()), + authenticate: mocks.authenticate, +})); + +vi.mock("../db/scm-settings", async (importOriginal) => ({ + ...(await importOriginal()), + ScmSettingsStore: vi.fn().mockImplementation(function () { + return mocks.store; + }), +})); + +const handleRequest = createTestRequestHandler([scmSettingsRoutes]); +const JSON_HEADERS = { "Content-Type": "application/json" }; -function failingContext(): RequestContext { +/** Answers admission's owner lookup; the settings store is mocked, so nothing else reads D1. */ +function ownerDatabase(): SqlDatabase { return { - request_id: "request-1", - trace_id: "trace-1", - executionCtx: TEST_BACKGROUND_TASK_CONTEXT, - db: { - prepare: vi.fn(() => { - throw new Error("D1 unavailable"); - }), + prepare(sql: string) { + const statement: SqlStatement = { + bind: () => statement, + first: async () => + (sql.includes("FROM users u") + ? { + user_id: "user-1", + suspended_at: null, + role_id: BUILT_IN_ROLE_REGISTRY.owner.id, + role_key: "owner", + role_name: "Owner", + } + : null) as T | null, + all: async () => ({ results: [] as T[], meta: { changes: 0 } }), + run: async () => ({ results: [] as T[], meta: { changes: 0 } }), + }; + return statement; }, - } as unknown as RequestContext; + batch: async () => [], + }; +} + +const env = { + ...TEST_SERVICE_SECRETS, + SCM_PROVIDER: "github", + DB: ownerDatabase(), +} as unknown as Env; + +function callRoute(method: string, path: string, body?: unknown): Promise { + return handleRequest( + new Request(`https://test.local${path}`, { + method, + ...(body === undefined ? {} : { headers: JSON_HEADERS, body: JSON.stringify(body) }), + }), + env, + TEST_BACKGROUND_TASK_CONTEXT + ); } describe("SCM settings routes", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.authenticate.mockImplementation(async (request: Request) => ({ + principal: { kind: "user", userId: "user-1" }, + request, + })); + for (const method of Object.values(mocks.store)) { + method.mockRejectedValue(new Error("D1 unavailable")); + } + }); + + it.each([ + ["GET", "/scm-settings", undefined, "getGlobal", []], + ["PUT", "/scm-settings", { settings: {} }, "setGlobal", [{}]], + ["DELETE", "/scm-settings", undefined, "deleteGlobal", []], + ["GET", "/scm-settings/repos", undefined, "listRepoSettings", []], + [ + "PUT", + "/scm-settings/repos/acme/web", + { settings: { alwaysUseDraftMode: true } }, + "setRepoSettings", + ["acme/web", { alwaysUseDraftMode: true }], + ], + ["DELETE", "/scm-settings/repos/acme/web", undefined, "deleteRepoSettings", ["acme/web"]], + ] as const)("routes %s %s to the store", async (method, path, body, storeMethod, args) => { + const target = mocks.store[storeMethod]; + target.mockResolvedValue(storeMethod === "listRepoSettings" ? [] : null); + + const response = await callRoute(method, path, body); + + expect(response.status).toBe(200); + expect(target).toHaveBeenCalledWith(...args); + for (const [name, other] of Object.entries(mocks.store)) { + if (name !== storeMethod) expect(other).not.toHaveBeenCalled(); + } + }); + it.each(["/scm-settings", "/scm-settings/repos"])( "maps storage read failures for GET %s to 503", async (path) => { - const { route, match } = findRoute("GET", path); - - const response = await route.handler( - new Request(`https://test.local${path}`), - {} as never, - match, - failingContext() - ); + const response = await callRoute("GET", path); expect(response.status).toBe(503); await expect(response.json()).resolves.toEqual({ @@ -51,22 +139,30 @@ describe("SCM settings routes", () => { "alwaysUseDraftMode must be a boolean", ], ])("rejects malformed settings for %s %s before storage", async (method, path, body, message) => { - const { route, match } = findRoute(method, path); - - const response = await route.handler( - new Request(`https://test.local${path}`, { - method, - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(body), - }), - {} as never, - match, - failingContext() - ); + const response = await callRoute(method, path, body); expect(response.status).toBe(400); await expect(response.json()).resolves.toMatchObject({ error: expect.stringContaining(message), }); + expect(mocks.store.setGlobal).not.toHaveBeenCalled(); + expect(mocks.store.setRepoSettings).not.toHaveBeenCalled(); + }); + + it("reads the repository from the segments Hono decoded, without decoding again", async () => { + mocks.store.deleteRepoSettings.mockResolvedValue(undefined); + + // One decode turns `%2F` into a slash the name may not hold. + const slashInName = await callRoute("DELETE", "/scm-settings/repos/acme/web%2Fapp"); + expect(slashInName.status).toBe(400); + await expect(slashInName.json()).resolves.toEqual({ + error: "Owner and name must be valid repository path segments", + }); + expect(mocks.store.deleteRepoSettings).not.toHaveBeenCalled(); + + // A doubly-encoded slash survives the single decode as a literal `%2F`. + const doubleEncoded = await callRoute("DELETE", "/scm-settings/repos/acme/web%252Fapp"); + expect(doubleEncoded.status).toBe(200); + expect(mocks.store.deleteRepoSettings).toHaveBeenCalledWith("acme/web%2Fapp"); }); }); diff --git a/packages/control-plane/src/routes/scm-settings.ts b/packages/control-plane/src/routes/scm-settings.ts index fe0ff9881..fce83edaa 100644 --- a/packages/control-plane/src/routes/scm-settings.ts +++ b/packages/control-plane/src/routes/scm-settings.ts @@ -12,18 +12,19 @@ import { type ScmGlobalConfig, type ScmRepoSettings, } from "@open-inspect/shared/types/integrations"; +import { Hono } from "hono"; import { ScmSettingsStore, ScmSettingsValidationError } from "../db/scm-settings"; import type { Env } from "../types"; import { createLogger } from "../logger"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; +import { repositoryParams } from "./repository-params"; import { - type Route, type RequestContext, SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - defineRoutes, json, error, parseJsonBody, - extractRepoParams, requirePermission, } from "./shared"; @@ -52,7 +53,7 @@ function parseScmRepoSettingsBody(body: unknown): ScmRepoSettings | Response { async function handleGetGlobal( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const store = new ScmSettingsStore(ctx.db); @@ -72,7 +73,7 @@ async function handleGetGlobal( async function handleSetGlobal( request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const body = await parseJsonBody(request); @@ -106,7 +107,7 @@ async function handleSetGlobal( async function handleDeleteGlobal( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const store = new ScmSettingsStore(ctx.db); @@ -132,7 +133,7 @@ async function handleDeleteGlobal( async function handleListRepoSettings( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const store = new ScmSettingsStore(ctx.db); @@ -152,12 +153,12 @@ async function handleListRepoSettings( async function handleSetRepoSettings( request: Request, _env: Env, - match: RegExpMatchArray, + params: { owner: string; name: string }, ctx: RequestContext ): Promise { - const params = extractRepoParams(match); - if (params instanceof Response) return params; - const { owner, name } = params; + const repository = repositoryParams(params); + if (repository instanceof Response) return repository; + const { owner, name } = repository; const repo = `${owner}/${name}`; const body = await parseJsonBody(request); @@ -192,12 +193,12 @@ async function handleSetRepoSettings( async function handleDeleteRepoSettings( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { owner: string; name: string }, ctx: RequestContext ): Promise { - const params = extractRepoParams(match); - if (params instanceof Response) return params; - const { owner, name } = params; + const repository = repositoryParams(params); + if (repository instanceof Response) return repository; + const { owner, name } = repository; const repo = `${owner}/${name}`; const store = new ScmSettingsStore(ctx.db); @@ -221,41 +222,28 @@ async function handleDeleteRepoSettings( } } -export const scmSettingsRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, [ - { - method: "GET", - path: "/scm-settings", - authorization: requirePermission("integrations.read"), - handler: handleGetGlobal, - }, - { - method: "PUT", - path: "/scm-settings", - authorization: requirePermission("scm_settings.manage"), - handler: handleSetGlobal, - }, - { - method: "DELETE", - path: "/scm-settings", - authorization: requirePermission("scm_settings.manage"), - handler: handleDeleteGlobal, - }, - { - method: "GET", - path: "/scm-settings/repos", - authorization: requirePermission("integrations.read"), - handler: handleListRepoSettings, - }, - { - method: "PUT", - path: "/scm-settings/repos/:owner/:name", - authorization: requirePermission("scm_settings.manage"), - handler: handleSetRepoSettings, - }, - { - method: "DELETE", - path: "/scm-settings/repos/:owner/:name", - authorization: requirePermission("scm_settings.manage"), - handler: handleDeleteRepoSettings, - }, -]); +const SCM_SETTINGS_READ = admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("integrations.read"), +}); +const SCM_SETTINGS_MANAGE = admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("scm_settings.manage"), +}); + +export const scmSettingsRoutes = new Hono(); + +scmSettingsRoutes.get("/scm-settings", SCM_SETTINGS_READ, (c) => dispatch(c, handleGetGlobal)); +scmSettingsRoutes.put("/scm-settings", SCM_SETTINGS_MANAGE, (c) => dispatch(c, handleSetGlobal)); +scmSettingsRoutes.delete("/scm-settings", SCM_SETTINGS_MANAGE, (c) => + dispatch(c, handleDeleteGlobal) +); +scmSettingsRoutes.get("/scm-settings/repos", SCM_SETTINGS_READ, (c) => + dispatch(c, handleListRepoSettings) +); +scmSettingsRoutes.put("/scm-settings/repos/:owner/:name", SCM_SETTINGS_MANAGE, (c) => + dispatch(c, handleSetRepoSettings) +); +scmSettingsRoutes.delete("/scm-settings/repos/:owner/:name", SCM_SETTINGS_MANAGE, (c) => + dispatch(c, handleDeleteRepoSettings) +); diff --git a/packages/control-plane/src/routes/skills.ts b/packages/control-plane/src/routes/skills.ts index 3959fb4c5..e6cc35986 100644 --- a/packages/control-plane/src/routes/skills.ts +++ b/packages/control-plane/src/routes/skills.ts @@ -1,3 +1,4 @@ +import { Hono } from "hono"; import { createSkillInputSchema, createSkillProfileInputSchema, @@ -30,15 +31,15 @@ import { SkillRevisionValidationError, } from "../skills/content-addressing"; import { fetchSkillImport, SkillImportError, type SkillImportResult } from "../skills/git-import"; +import { admit, dispatch } from "../routing/admit"; +import type { ControlPlaneHonoEnv } from "../routing/hono-env"; import { createRouteSourceControlProvider, error, json, type RequestContext, - type Route, SCM_AGNOSTIC_HUMAN_USER_ROUTE, SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - defineRoutes, requirePermission, } from "./shared"; @@ -92,14 +93,10 @@ async function parsedBody(request: Request): Promise { } } -function resourceId(match: RegExpMatchArray): string | Response { - return match.groups?.id ?? error("Resource ID required", 400); -} - async function handleListSkills( request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const url = new URL(request.url); @@ -125,11 +122,10 @@ async function handleListSkills( async function handleGetSkill( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const skill = await new SkillStore(ctx.db).get(id); return skill ? json({ skill }) : error("Skill not found", 404); } @@ -137,7 +133,7 @@ async function handleGetSkill( async function handleCreateSkill( request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const userId = canonicalUserId(ctx); @@ -162,7 +158,7 @@ async function handleCreateSkill( async function handlePreviewSkill( request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, _ctx: RequestContext ): Promise { const body = await parsedBody(request); @@ -241,7 +237,7 @@ function confirmedImport( async function handlePreviewSkillImport( request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const body = await parsedBody(request); @@ -263,7 +259,7 @@ async function handlePreviewSkillImport( async function handleImportSkill( request: Request, env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const userId = canonicalUserId(ctx); @@ -329,11 +325,10 @@ function recordedImportSource( async function handlePreviewSkillReimport( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const body = await parsedBody(request); if (body instanceof Response) return body; const parsed = reimportSkillPreviewInputSchema.safeParse(body); @@ -354,11 +349,10 @@ async function handlePreviewSkillReimport( async function handleReimportSkill( request: Request, env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const userId = canonicalUserId(ctx); if (!userId) return error("Canonical user required", 403); const ifMatch = request.headers.get("If-Match")?.replace(/^"|"$/g, ""); @@ -415,11 +409,10 @@ function sourceAuditFields(source: SkillImportResult["source"]) { async function handleSetSkillEnabled( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const userId = canonicalUserId(ctx); if (!userId) return error("Canonical user required", 403); const body = await parsedBody(request); @@ -438,11 +431,10 @@ async function handleSetSkillEnabled( async function handleReplaceSkillContentAndAssignments( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const userId = canonicalUserId(ctx); if (!userId) return error("Canonical user required", 403); const ifMatch = request.headers.get("If-Match")?.replace(/^"|"$/g, ""); @@ -473,11 +465,10 @@ async function handleReplaceSkillContentAndAssignments( async function handleDeleteSkill( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const userId = canonicalUserId(ctx); if (!userId) return error("Canonical user required", 403); const deleted = await new SkillStore(ctx.db).delete(id, userId); @@ -488,7 +479,7 @@ async function handleDeleteSkill( async function handleListProfiles( _request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const userId = canonicalUserId(ctx); @@ -499,7 +490,7 @@ async function handleListProfiles( async function handleCreateProfile( request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const userId = canonicalUserId(ctx); @@ -525,11 +516,10 @@ async function handleCreateProfile( async function handleUpdateProfile( request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const userId = canonicalUserId(ctx); if (!userId) return error("Canonical user required", 403); const body = await parsedBody(request); @@ -548,11 +538,10 @@ async function handleUpdateProfile( async function handleDeleteProfile( _request: Request, _env: Env, - match: RegExpMatchArray, + params: { id: string }, ctx: RequestContext ): Promise { - const id = resourceId(match); - if (id instanceof Response) return id; + const { id } = params; const userId = canonicalUserId(ctx); if (!userId) return error("Canonical user required", 403); const deleted = await new SkillProfileStore(ctx.db).delete(id, userId); @@ -563,7 +552,7 @@ async function handleDeleteProfile( async function handleResolvePreview( request: Request, _env: Env, - _match: RegExpMatchArray, + _params: object, ctx: RequestContext ): Promise { const body = await parsedBody(request); @@ -624,106 +613,47 @@ function profileWriteError(value: unknown): Response { throw value; } -const skillReadRoutes = defineRoutes(SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, [ - { - method: "GET", - path: "/skills", - authorization: requirePermission("skills.read"), - handler: handleListSkills, - }, - { - method: "POST", - path: "/skills/preview", - authorization: requirePermission("skills.read"), - handler: handlePreviewSkill, - }, - { - method: "POST", - path: "/skills/resolve-preview", - authorization: requirePermission("skills.read"), - handler: handleResolvePreview, - }, - { - method: "GET", - path: "/skills/:id", - authorization: requirePermission("skills.read"), - handler: handleGetSkill, - }, -]); - -const skillAdministrationRoutes = defineRoutes(SCM_AGNOSTIC_HUMAN_USER_ROUTE, [ - { - method: "POST", - path: "/skills", - authorization: requirePermission("skills.manage"), - handler: handleCreateSkill, - }, - { - method: "POST", - path: "/skills/import/preview", - authorization: requirePermission("skills.manage"), - handler: handlePreviewSkillImport, - }, - { - method: "POST", - path: "/skills/import", - authorization: requirePermission("skills.manage"), - handler: handleImportSkill, - }, - { - method: "POST", - path: "/skills/:id/reimport/preview", - authorization: requirePermission("skills.manage"), - handler: handlePreviewSkillReimport, - }, - { - method: "POST", - path: "/skills/:id/reimport", - authorization: requirePermission("skills.manage"), - handler: handleReimportSkill, - }, - { - method: "PATCH", - path: "/skills/:id", - authorization: requirePermission("skills.manage"), - handler: handleSetSkillEnabled, - }, - { - method: "PUT", - path: "/skills/:id", - authorization: requirePermission("skills.manage"), - handler: handleReplaceSkillContentAndAssignments, - }, - { - method: "DELETE", - path: "/skills/:id", - authorization: requirePermission("skills.manage"), - handler: handleDeleteSkill, - }, - { - method: "GET", - path: "/skill-profiles", - authorization: requirePermission("skill_profiles.manage_own"), - handler: handleListProfiles, - }, - { - method: "POST", - path: "/skill-profiles", - authorization: requirePermission("skill_profiles.manage_own"), - handler: handleCreateProfile, - }, - { - method: "PATCH", - path: "/skill-profiles/:id", - authorization: requirePermission("skill_profiles.manage_own"), - handler: handleUpdateProfile, - }, - { - method: "DELETE", - path: "/skill-profiles/:id", - authorization: requirePermission("skill_profiles.manage_own"), - handler: handleDeleteProfile, - }, -]); - -export const skillRoutes: Route[] = [...skillReadRoutes, ...skillAdministrationRoutes]; +const SKILLS_READ = admit({ + ...SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + authorization: requirePermission("skills.read"), +}); +const SKILLS_MANAGE = admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + authorization: requirePermission("skills.manage"), +}); +const PROFILES_MANAGE_OWN = admit({ + ...SCM_AGNOSTIC_HUMAN_USER_ROUTE, + authorization: requirePermission("skill_profiles.manage_own"), +}); + +export const skillRoutes = new Hono(); + +// Read routes register ahead of administration so `/skills/preview` and +// `/skills/resolve-preview` take precedence over the parameterized paths. +skillRoutes.get("/skills", SKILLS_READ, (c) => dispatch(c, handleListSkills)); +skillRoutes.post("/skills/preview", SKILLS_READ, (c) => dispatch(c, handlePreviewSkill)); +skillRoutes.post("/skills/resolve-preview", SKILLS_READ, (c) => dispatch(c, handleResolvePreview)); +skillRoutes.get("/skills/:id", SKILLS_READ, (c) => dispatch(c, handleGetSkill)); + +skillRoutes.post("/skills", SKILLS_MANAGE, (c) => dispatch(c, handleCreateSkill)); +skillRoutes.post("/skills/import/preview", SKILLS_MANAGE, (c) => + dispatch(c, handlePreviewSkillImport) +); +skillRoutes.post("/skills/import", SKILLS_MANAGE, (c) => dispatch(c, handleImportSkill)); +skillRoutes.post("/skills/:id/reimport/preview", SKILLS_MANAGE, (c) => + dispatch(c, handlePreviewSkillReimport) +); +skillRoutes.post("/skills/:id/reimport", SKILLS_MANAGE, (c) => dispatch(c, handleReimportSkill)); +skillRoutes.patch("/skills/:id", SKILLS_MANAGE, (c) => dispatch(c, handleSetSkillEnabled)); +skillRoutes.put("/skills/:id", SKILLS_MANAGE, (c) => + dispatch(c, handleReplaceSkillContentAndAssignments) +); +skillRoutes.delete("/skills/:id", SKILLS_MANAGE, (c) => dispatch(c, handleDeleteSkill)); +skillRoutes.get("/skill-profiles", PROFILES_MANAGE_OWN, (c) => dispatch(c, handleListProfiles)); +skillRoutes.post("/skill-profiles", PROFILES_MANAGE_OWN, (c) => dispatch(c, handleCreateProfile)); +skillRoutes.patch("/skill-profiles/:id", PROFILES_MANAGE_OWN, (c) => + dispatch(c, handleUpdateProfile) +); +skillRoutes.delete("/skill-profiles/:id", PROFILES_MANAGE_OWN, (c) => + dispatch(c, handleDeleteProfile) +); diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index d513985c6..ea94c2ad1 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -54,6 +54,7 @@ export { encodeRepositoryPathSegments, formatRepositoryFullName, parseRepositoryFullName, + validateRepositoryPathSegments, normalizeOptionalRepositoryPair, } from "./repositories"; export type { diff --git a/packages/shared/src/types/repositories.ts b/packages/shared/src/types/repositories.ts index e45a351b5..ece7da665 100644 --- a/packages/shared/src/types/repositories.ts +++ b/packages/shared/src/types/repositories.ts @@ -186,19 +186,31 @@ export function parseRepositoryFullName(fullName: string): RepositoryPair | null return { repoOwner, repoName }; } +/** + * The repository named by two already-decoded path segments, or null when + * they are not a canonical pair: an owner may be a nested namespace, but a + * name may not contain a slash, and neither may be empty. + */ +export function validateRepositoryPathSegments( + repoOwner: string, + repoName: string +): RepositoryPair | null { + const repository = parseRepositoryFullName(formatRepositoryFullName({ repoOwner, repoName })); + return repository?.repoOwner === repoOwner && repository.repoName === repoName + ? repository + : null; +} + /** Decode and validate the two path segments used by repository APIs. */ export function decodeRepositoryPathSegments( encodedOwner: string, encodedName: string ): RepositoryPair | null { try { - const repoOwner = decodeURIComponent(encodedOwner); - const repoName = decodeURIComponent(encodedName); - const repository = parseRepositoryFullName(formatRepositoryFullName({ repoOwner, repoName })); - - return repository?.repoOwner === repoOwner && repository.repoName === repoName - ? repository - : null; + return validateRepositoryPathSegments( + decodeURIComponent(encodedOwner), + decodeURIComponent(encodedName) + ); } catch { return null; } diff --git a/packages/shared/src/types/repository-contracts.test.ts b/packages/shared/src/types/repository-contracts.test.ts index 41067e534..3163835a4 100644 --- a/packages/shared/src/types/repository-contracts.test.ts +++ b/packages/shared/src/types/repository-contracts.test.ts @@ -10,6 +10,7 @@ import { serverMessageSchema, sessionRepositoriesInputSchema, toRepositoryRef, + validateRepositoryPathSegments, } from "./index"; import { sandboxEventSchema } from "./sandbox-events"; import { createSessionRequestSchema } from "./session-api"; @@ -41,6 +42,28 @@ describe("repository full names", () => { ])("rejects a non-canonical repository API path (%s/%s)", (owner, name) => { expect(decodeRepositoryPathSegments(owner, name)).toBeNull(); }); + + it("validates already-decoded segments with the same rules the decoder applies", () => { + expect(validateRepositoryPathSegments("group/subgroup", "web app")).toEqual({ + repoOwner: "group/subgroup", + repoName: "web app", + }); + // A once-decoded escape is data, not a separator. + expect(validateRepositoryPathSegments("acme", "web%2Fapp")).toEqual({ + repoOwner: "acme", + repoName: "web%2Fapp", + }); + }); + + it.each([ + ["group", "web/api"], + ["group//subgroup", "web"], + ["", "web"], + ["group", ""], + ["/group", "web"], + ])("rejects a non-canonical decoded pair (%j/%j)", (owner, name) => { + expect(validateRepositoryPathSegments(owner, name)).toBeNull(); + }); }); describe("sessionRepositoriesInputSchema", () => {