Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/control-plane/src/router.auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import { routes } from "./routes/catalog";
import {
handleRequest,
matchRoute,
signedServiceRequest,
TEST_BACKGROUND_TASK_CONTEXT,
TEST_SERVICE_SECRETS,
} from "./router.test-support";

function routeFor(method: string, path: string) {
return routes.find((route) => route.method === method && route.pattern.test(path));
return matchRoute(routes, method, path)?.route;
}

function createEnv(verifyStatus: number) {
Expand Down
50 changes: 21 additions & 29 deletions packages/control-plane/src/router.policy.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { describe, expect, it, vi } from "vitest";
import { enforceRoutePrincipal } from "./routing/route-admission";
import { routes } from "./routes/catalog";
import { handleRequest, TEST_BACKGROUND_TASK_CONTEXT } from "./router.test-support";
import { parsePattern } from "./routes/shared";
import { handleRequest, matchRoute, TEST_BACKGROUND_TASK_CONTEXT } from "./router.test-support";
import { serviceAllowsPermission } from "./authorization/service-permissions";
import { SCOPED_PERMISSION_PAIRS } from "@open-inspect/shared/rbac";

function routeFor(method: string, path: string) {
return routes.find((route) => route.method === method && route.pattern.test(path));
return matchRoute(routes, method, path)?.route;
}

describe("route policy table", () => {
Expand All @@ -17,16 +16,12 @@ describe("route policy table", () => {
const paths = routes.map((route) => route.path);
expect(new Set(paths).size).toBe(130);
expect(new Set(routes.map((route) => `${route.method}:${route.path}`)).size).toBe(171);

for (const route of routes) {
expect(route.pattern.source).toBe(parsePattern(route.path).source);
}
});

it("declares every path in the literal-or-parameter grammar shared by Hono and parsePattern", () => {
// Hono gives `*`, `?`, `{...}` and `.` routing meaning that parsePattern
// compiles as literals, so a path outside this grammar would be selected
// by Hono and then rejected by the raw-path regex.
it("declares every path in the literal-or-parameter grammar", () => {
// Hono gives `*`, `?`, `{...}` and `.` routing meaning, and raw parameters
// are read back from the pathname by position, so a path outside this
// grammar could be selected by Hono and yield the wrong parameters.
for (const route of routes) {
expect(route.path, `${route.method} ${route.path}`).toMatch(/^(\/([A-Za-z0-9_-]+|:\w+))+$/);
}
Expand Down Expand Up @@ -61,7 +56,7 @@ describe("route policy table", () => {
});

it("has no duplicate method and pattern declarations", () => {
const identities = routes.map((route) => `${route.method}:${route.pattern}`);
const identities = routes.map((route) => `${route.method}:${route.path}`);
expect(new Set(identities).size).toBe(identities.length);
});

Expand All @@ -87,13 +82,13 @@ describe("route policy table", () => {
expect(authorization.allOf.length).toBeGreaterThan(0);
for (const requirement of authorization.allOf) {
if (requirement.kind === "automation") {
expect(route.pattern.source).toContain(`?<${requirement.automationIdParam}>`);
expect(route.path.split("/")).toContain(`:${requirement.automationIdParam}`);
}
}
if (authorization.service.kind === "actor") {
for (const grant of authorization.service.actorlessGrants ?? []) {
for (const pathParam of Object.keys(grant.pathParams ?? {})) {
expect(route.pattern.source).toContain(`?<${pathParam}>`);
expect(route.path.split("/")).toContain(`:${pathParam}`);
}
}
}
Expand Down Expand Up @@ -165,7 +160,7 @@ describe("route policy table", () => {
if (requirement.kind === "permission") {
expect(
serviceAllowsPermission(grant.service, requirement.permission),
`${grant.service} must allow ${requirement.permission} for ${route.method} ${route.pattern}`
`${grant.service} must allow ${requirement.permission} for ${route.method} ${route.path}`
).toBe(true);
} else if (requirement.kind === "scoped-permission") {
expect(
Expand Down Expand Up @@ -246,11 +241,10 @@ 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 = routeFor("GET", path);
const match = path.match(route!.pattern)!;
const { route, match } = matchRoute(routes, "GET", path)!;
const prepare = vi.fn();

const response = await route!.handler(
const response = await route.handler(
new Request(`https://test.local${path}`),
{} as never,
match,
Expand Down Expand Up @@ -317,13 +311,12 @@ describe("route policy table", () => {
["PUT", "/sessions/session-1/diff"],
["POST", "/sessions/session-1/diff/failure"],
])("allows user/service auth with sandbox fallback for %s %s", (method, path) => {
const route = routeFor(method, path);
const match = path.match(route!.pattern)!;
expect(route?.authentication.kind).toBe("user-or-service-with-sandbox-fallback");
if (route?.authentication.kind === "user-or-service-with-sandbox-fallback") {
expect(route.authentication.getSessionId(match)).toBe("session-1");
const { route, params } = matchRoute(routes, method, path)!;
expect(route.authentication.kind).toBe("user-or-service-with-sandbox-fallback");
if (route.authentication.kind === "user-or-service-with-sandbox-fallback") {
expect(route.authentication.getSessionId(params)).toBe("session-1");
}
expect(route?.authorization.kind).toBe("active-user");
expect(route.authorization.kind).toBe("active-user");
});

it.each([
Expand All @@ -336,11 +329,10 @@ describe("route policy table", () => {
["GET", "/sessions/session-1/sandbox-skills"],
["POST", "/sessions/session-1/provider-auth/openai/access-token"],
])("requires the bound sandbox for %s %s", (method, path) => {
const route = routeFor(method, path);
const match = path.match(route!.pattern)!;
expect(route?.authentication.kind).toBe("sandbox");
if (route?.authentication.kind === "sandbox") {
expect(route.authentication.getSessionId(match)).toBe(
const { route, params } = matchRoute(routes, method, path)!;
expect(route.authentication.kind).toBe("sandbox");
if (route.authentication.kind === "sandbox") {
expect(route.authentication.getSessionId(params)).toBe(
path.includes("/children/") ? "parent-1" : "session-1"
);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/control-plane/src/router.scm-credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { describe, expect, it, vi } from "vitest";
import { routes } from "./routes/catalog";
import {
handleRequest,
matchRoute,
signedServiceRequest,
TEST_BACKGROUND_TASK_CONTEXT,
TEST_SERVICE_SECRETS,
} from "./router.test-support";

function routeFor(method: string, path: string) {
return routes.find((route) => route.method === method && route.pattern.test(path));
return matchRoute(routes, method, path)?.route;
}

function createEnv(options?: { actorAuthorized?: boolean }) {
Expand Down
63 changes: 39 additions & 24 deletions packages/control-plane/src/router.test-support.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Test-only builder for service-authenticated router requests.
* Test-only builders for router requests.
*
* sig1 binds method, URL, and body, so every request is signed individually
* — there is no reusable Authorization header. Env fixtures must bind the
Expand All @@ -9,18 +9,23 @@
import { buildServiceAuthHeaders, type ServiceName } from "@open-inspect/shared/service-auth";
import type { BackgroundTasks } from "./platform-ports";
import { createTestBackgroundTasks } from "./background-tasks.test-support";
import {
createControlPlaneHttpHandler,
handleControlPlaneHttp,
type ControlPlaneHttpHandler,
} from "./routing/hono-app";
import type { Route } from "./routes/shared";
import { cloudflareHost, createControlPlaneApp } from "./routing/hono-app";
import { routes } from "./routes/catalog";
import type { Route, RouteParams } from "./routes/shared";
import type { Env } from "./types";

// The single contract-faithful double lives in background-tasks.test-support;
// this shared instance's recordings are unused by the router suites.
export const TEST_BACKGROUND_TASK_CONTEXT: BackgroundTasks = createTestBackgroundTasks();

/** Request handler signature used by unit fixtures that provide the platform-neutral port. */
export type TestRequestHandler = (
request: Request,
env: Env,
backgroundTasks: BackgroundTasks
) => Promise<Response>;

/** Present the fixture's port as the execution context the Cloudflare host expects. */
function executionContextFromBackgroundTasks(tasks: BackgroundTasks): ExecutionContext {
return {
waitUntil(promise): void {
Expand All @@ -30,28 +35,38 @@ function executionContextFromBackgroundTasks(tasks: BackgroundTasks): ExecutionC
} as ExecutionContext;
}

/** Request handler signature used by unit fixtures that provide the platform-neutral port. */
export type TestRequestHandler = (
request: Request,
env: Env,
backgroundTasks: BackgroundTasks
) => Promise<Response>;

function adaptForTests(handler: ControlPlaneHttpHandler): TestRequestHandler {
/**
* Test-only adapter over an explicit catalog, through the production host.
* Hono registers routes when the app is built, so fixtures that need
* synthetic routes construct their own handler instead of mutating the
* production catalog.
*/
export function createTestRequestHandler(catalog: readonly Route[]): TestRequestHandler {
const app = createControlPlaneApp(catalog, cloudflareHost);
return (request, env, backgroundTasks) =>
handler(request, env, executionContextFromBackgroundTasks(backgroundTasks));
Promise.resolve(app.fetch(request, env, executionContextFromBackgroundTasks(backgroundTasks)));
}

/** Test-only adapter over the production catalog. */
export const handleRequest: TestRequestHandler = adaptForTests(handleControlPlaneHttp);
export const handleRequest: TestRequestHandler = createTestRequestHandler(routes);

/**
* Test-only adapter over an explicit catalog. Hono registers routes when the
* app is built, so fixtures that need synthetic routes construct their own
* handler instead of mutating the production catalog.
*/
export function createTestRequestHandler(catalog: readonly Route[]): TestRequestHandler {
return adaptForTests(createControlPlaneHttpHandler(catalog));
/** Compile a catalog path into the legacy raw-path matcher, for handler-level fixtures. */
export function routePathPattern(path: string): RegExp {
return new RegExp(`^${path.replace(/:(\w+)/g, "(?<$1>[^/]+)")}$`);
}

/** Select the catalog route for a concrete path and rebuild what the adapter hands its handler. */
export function matchRoute(
catalog: readonly Route[],
method: string,
path: string
): { route: Route; match: RegExpMatchArray; params: RouteParams } | undefined {
for (const route of catalog) {
if (route.method !== method) continue;
const match = path.match(routePathPattern(route.path));
if (match) return { route, match, params: { ...match.groups } };
}
return undefined;
}

/** Per-service secrets for unit-test env fixtures, mirrored by signedServiceRequest. */
Expand Down
6 changes: 3 additions & 3 deletions packages/control-plane/src/routes/analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { HUMAN_SPAWN_SOURCES } from "../db/analytics-store";
import type { RequestContext } from "./shared";
import type { SqlDatabase } from "../db/sql-database";
import type { Env } from "../types";
import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";

const FIXED_NOW = 1_700_000_000_000;

Expand Down Expand Up @@ -37,8 +37,8 @@ vi.mock("../db/analytics-dashboard-store", () => ({
function getHandler(method: string, path: string) {
const pathname = new URL(`https://test.local${path}`).pathname;
for (const route of analyticsRoutes) {
if (route.method === method && route.pattern.test(pathname)) {
const match = pathname.match(route.pattern)!;
if (route.method === method && routePathPattern(route.path).test(pathname)) {
const match = pathname.match(routePathPattern(route.path))!;
return { handler: route.handler, match };
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/control-plane/src/routes/automations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { HttpError, resolveRepoOrError, type RequestContext } from "./shared";
import type { Principal } from "../auth/principal";
import type { SqlDatabase } from "../db/sql-database";
import type { Env } from "../types";
import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import {
AutomationExecutionUnauthorizedError,
AutomationTriggerBlockedError,
Expand Down Expand Up @@ -239,10 +239,10 @@ async function callRoute(
}
): Promise<Response> {
const route = automationRoutes.find(
(candidate) => candidate.method === method && candidate.pattern.test(path)
(candidate) => candidate.method === method && routePathPattern(candidate.path).test(path)
);
if (!route) throw new Error(`No route found for ${method} ${path}`);
const match = path.match(route.pattern)!;
const match = path.match(routePathPattern(route.path))!;
const url = new URL(`https://test.local${path}`);
if (options?.query) {
for (const [k, v] of Object.entries(options.query)) {
Expand Down
6 changes: 3 additions & 3 deletions packages/control-plane/src/routes/environment-secrets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import { describe, expect, it, vi } from "vitest";
import { generateEncryptionKey } from "../auth/crypto";
import { environmentSecretsRoutes } from "./environment-secrets";
import type { RequestContext, Route } from "./shared";
import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { routePathPattern, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";

function findRoute(method: string, path: string): { route: Route; match: RegExpMatchArray } {
const route = environmentSecretsRoutes.find(
(candidate) => candidate.method === method && path.match(candidate.pattern)
(candidate) => candidate.method === method && path.match(routePathPattern(candidate.path))
);
if (!route) throw new Error(`Missing ${method} ${path} route`);
return { route, match: path.match(route.pattern)! };
return { route, match: path.match(routePathPattern(route.path))! };
}

function createContext() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type * as VercelClientModule from "../sandbox/providers/vercel/client";
import type * as OpenComputerProviderModule from "../sandbox/providers/opencomputer-provider";
import type * as OpenComputerClientModule from "../sandbox/opencomputer-rest-client";
import type * as IntegrationSettingsResolutionModule from "../session/integration-settings-resolution";
import { routePathPattern } from "../router.test-support";

// The repo trigger resolves the repo's actual default branch (never assumes
// "main") and threads it into the build's repository set + fingerprint + the
Expand Down Expand Up @@ -113,14 +114,14 @@ function findRoute(method: string, path: string): Route {
// Match on method as well as pattern so a same-pattern route of another
// method (or a reordering) can never resolve to the wrong handler.
const route = imageBuildRoutes.find(
(candidate) => candidate.method === method && candidate.pattern.test(path)
(candidate) => candidate.method === method && routePathPattern(candidate.path).test(path)
);
if (!route) throw new Error(`route not found: ${method} ${path}`);
return route;
}

function matchFor(route: Route, path: string): RegExpMatchArray {
const match = path.match(route.pattern);
const match = path.match(routePathPattern(route.path));
if (!match) throw new Error("path did not match route pattern");
return match;
}
Expand Down
20 changes: 7 additions & 13 deletions packages/control-plane/src/routes/repos.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Env } from "../types";
import { reposRoutes } from "./repos";
import type * as SharedRoutes from "./shared";
import type { RequestContext } from "./shared";
import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { matchRoute, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";

const {
mockCacheDelete,
Expand Down Expand Up @@ -75,21 +75,15 @@ vi.mock("./shared", async () => {
});

function getListHandler() {
const route = reposRoutes.find(
(candidate) => candidate.method === "GET" && candidate.pattern.test("/repos")
);
if (!route) throw new Error("No repository list route found");
const match = "/repos".match(route.pattern);
if (!match) throw new Error("List route did not match /repos");
return { handler: route.handler, match };
const matched = matchRoute(reposRoutes, "GET", "/repos");
if (!matched) throw new Error("No repository list route found");
return { handler: matched.route.handler, match: matched.match };
}

function getUpdateHandler(path: string) {
const route = reposRoutes.find((candidate) => candidate.method === "PUT");
if (!route) throw new Error("No repository metadata update route found");
const match = path.match(route.pattern);
if (!match) throw new Error(`Update route did not match ${path}`);
return { handler: route.handler, match };
const matched = matchRoute(reposRoutes, "PUT", path);
if (!matched) throw new Error(`Update route did not match ${path}`);
return { handler: matched.route.handler, match: matched.match };
}

describe("repository list route", () => {
Expand Down
10 changes: 4 additions & 6 deletions packages/control-plane/src/routes/scm-settings.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { describe, expect, it, vi } from "vitest";
import { scmSettingsRoutes } from "./scm-settings";
import type { RequestContext, Route } from "./shared";
import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";
import { matchRoute, TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support";

function findRoute(method: string, path: string): { route: Route; match: RegExpMatchArray } {
const route = scmSettingsRoutes.find(
(candidate) => candidate.method === method && path.match(candidate.pattern)
);
if (!route) throw new Error(`Missing ${method} ${path} route`);
return { route, match: path.match(route.pattern)! };
const matched = matchRoute(scmSettingsRoutes, method, path);
if (!matched) throw new Error(`Missing ${method} ${path} route`);
return { route: matched.route, match: matched.match };
}

function failingContext(): RequestContext {
Expand Down
Loading
Loading