diff --git a/packages/control-plane/src/auth/identity-enforcement.test.ts b/packages/control-plane/src/auth/identity-enforcement.test.ts index 74a7247382..466f4d19e6 100644 --- a/packages/control-plane/src/auth/identity-enforcement.test.ts +++ b/packages/control-plane/src/auth/identity-enforcement.test.ts @@ -4,7 +4,6 @@ import { applyIdentityEnforcement, deriveIdentity, mayAttachCallbackContext, - requireEventPoster, resolveCanonicalUserId, } from "./identity-enforcement"; import type { Principal, ResolvedIdentity } from "./principal"; @@ -31,12 +30,17 @@ const SLACK_BOT_PRINCIPAL: Principal = { }; function createCtx(principal?: Principal): RequestContext { + const statement = { + bind: vi.fn(() => statement), + first: vi.fn(async () => ({ active: 1 })), + }; return { trace_id: "trace-test", request_id: "req-test", principal, + db: { prepare: vi.fn(() => statement) }, executionCtx: TEST_BACKGROUND_TASK_CONTEXT, - } as RequestContext; + } as unknown as RequestContext; } function loggedEvents(spy: { mock: { calls: unknown[][] } }): Array> { @@ -93,26 +97,7 @@ describe("applyIdentityEnforcement — identityless principals", () => { }); describe("applyIdentityEnforcement — forbidden-field rejection", () => { - it("rejects forbidden keys with a 400 naming the field", async () => { - const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); - const { rejection } = applyIdentityEnforcement(createCtx(USER_PRINCIPAL), "session-lifecycle", { - userId: "someone", - title: "ok", - }); - expect(rejection).toBeDefined(); - expect(rejection!.status).toBe(400); - expect(((await rejection!.clone().json()) as { error: string }).error).toBe( - "Field 'userId' is not accepted from verified callers" - ); - const logged = loggedEvents(warn).find((e) => e.event === "identity.forbidden_field_rejected"); - expect(logged).toMatchObject({ route: "session-lifecycle", field: "userId" }); - }); - it("accepts bodies carrying only permitted fields", () => { - expect( - applyIdentityEnforcement(createCtx(USER_PRINCIPAL), "session-lifecycle", { title: "ok" }) - .rejection - ).toBeUndefined(); expect( applyIdentityEnforcement(createCtx(USER_PRINCIPAL), "session-create", { scmLogin: "ada", @@ -194,11 +179,9 @@ describe("applyIdentityEnforcement — requires-user rejection", () => { }); it("does not gate routes that accept participantless principals", () => { - for (const route of ["prompt", "session-lifecycle"] as const) { - const result = applyIdentityEnforcement(createCtx(ACTORLESS_BOT), route, {}); - expect(result.rejection).toBeUndefined(); - expect(result.enforced).toMatchObject({ participantUserId: null }); - } + const result = applyIdentityEnforcement(createCtx(ACTORLESS_BOT), "prompt", {}); + expect(result.rejection).toBeUndefined(); + expect(result.enforced).toMatchObject({ participantUserId: null }); }); }); @@ -241,6 +224,67 @@ describe("resolveCanonicalUserId", () => { ); }); + it("rejects when actor enrichment relinks to a different authorized user", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const ctx = createCtx(SLACK_BOT_PRINCIPAL); + ctx.authorization = { + userId: "canon-provisional", + suspendedAt: null, + permissions: ["sessions.create"], + role: { id: "role-member", key: "member", name: "Member" }, + }; + const result = await resolveCanonicalUserId( + { + resolveOrCreateUser: vi.fn(async () => ({ id: "canon-existing" })), + } as unknown as UserStore, + ctx, + { + participantUserId: "slack:U0123", + canonicalUserId: null, + actor: SLACK_ACTOR, + spawnSource: "slack-bot", + }, + display + ); + + expect(result).toBeInstanceOf(Response); + expect((result as Response).status).toBe(409); + await expect((result as Response).json()).resolves.toMatchObject({ + code: "actor_identity_changed", + }); + expect(loggedEvents(warn)).toContainEqual( + expect.objectContaining({ + event: "identity.mismatch_rejected", + expected: "canon-provisional", + actual: "canon-existing", + }) + ); + }); + + it("rejects a canonical identity whose workspace access is suspended", async () => { + const ctx = createCtx(USER_PRINCIPAL); + const statement = { + bind: vi.fn(() => statement), + first: vi.fn(async () => null), + }; + ctx.db = { prepare: vi.fn(() => statement) } as never; + + const result = await resolveCanonicalUserId( + { resolveOrCreateUser: vi.fn() } as unknown as UserStore, + ctx, + { + participantUserId: "canon-1", + canonicalUserId: "canon-1", + actor: null, + spawnSource: "user", + }, + display + ); + + expect(result).toBeInstanceOf(Response); + expect((result as Response).status).toBe(403); + }); + it("fails closed with a 500 if a participant ever lacks both a canonical user and an actor", async () => { vi.spyOn(console, "error").mockImplementation(() => undefined); const userStore = { resolveOrCreateUser: vi.fn() } as unknown as UserStore; @@ -296,38 +340,3 @@ describe("mayAttachCallbackContext", () => { expect(mayAttachCallbackContext(createCtx(undefined))).toBe(false); }); }); - -describe("requireEventPoster", () => { - const GITHUB_BOT: Principal = { - kind: "service", - service: "github-bot", - actor: null, - }; - - it("logs and 401s a mismatched poster", () => { - const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); - const rejection = requireEventPoster(createCtx(GITHUB_BOT), "slack"); - expect(rejection?.status).toBe(401); - const mismatch = loggedEvents(warn).find((e) => e.event === "identity.mismatch_rejected"); - expect(mismatch).toMatchObject({ - route: "internal-slack-event", - field: "service", - expected: "slack-bot", - actual: "github-bot", - }); - }); - - it("401s non-service principals — the gate never falls open", () => { - expect(requireEventPoster(createCtx(USER_PRINCIPAL), "slack")?.status).toBe(401); - expect(requireEventPoster(createCtx(undefined), "slack")?.status).toBe(401); - expect( - requireEventPoster(createCtx({ kind: "sandbox", sessionId: "s1" }), "sentry")?.status - ).toBe(401); - }); - - it("passes the matching bot and exempt sources", () => { - expect(requireEventPoster(createCtx(SLACK_BOT_PRINCIPAL), "slack")).toBeNull(); - // Sentry events are not bot-posted: explicit exemption for any service. - expect(requireEventPoster(createCtx(GITHUB_BOT), "sentry")).toBeNull(); - }); -}); diff --git a/packages/control-plane/src/auth/identity-enforcement.ts b/packages/control-plane/src/auth/identity-enforcement.ts index 7ae8620c11..ee0a945528 100644 --- a/packages/control-plane/src/auth/identity-enforcement.ts +++ b/packages/control-plane/src/auth/identity-enforcement.ts @@ -9,14 +9,13 @@ * can run the steps out of order or skip one. */ -import type { AutomationEventSource } from "@open-inspect/shared/triggers"; import type { SpawnSource } from "@open-inspect/shared/types/sessions"; import type { ServiceName } from "@open-inspect/shared/service-auth"; import { createLogger } from "./../logger"; import { CALLBACK_DESTINATIONS } from "./service/callback-signing"; import type { Principal, ResolvedIdentity } from "./principal"; import type { UserStore } from "../db/user-store"; -import { error, type RequestContext } from "../routes/shared"; +import { error, json, type RequestContext } from "../routes/shared"; const logger = createLogger("identity-enforcement"); @@ -198,7 +197,23 @@ export async function resolveCanonicalUserId( enforced: DerivedIdentity & { participantUserId: string }, display: { displayName?: string; email?: string; avatarUrl?: string } ): Promise<{ userId: string } | Response> { - if (enforced.canonicalUserId) return { userId: enforced.canonicalUserId }; + const requireActive = async (userId: string): Promise<{ userId: string } | Response> => { + try { + const active = await ctx.db + .prepare("SELECT 1 AS active FROM users WHERE id = ? AND suspended_at IS NULL") + .bind(userId) + .first<{ active: number }>(); + return active ? { userId } : error("Workspace access is disabled", 403); + } catch (cause) { + logger.error("Failed to verify workspace access", { + error: cause instanceof Error ? cause : String(cause), + request_id: ctx.request_id, + trace_id: ctx.trace_id, + }); + return error("Authorization unavailable", 503); + } + }; + if (enforced.canonicalUserId) return requireActive(enforced.canonicalUserId); const actor = enforced.actor; if (!actor) { // Unreachable while deriveIdentity holds its invariant (a participant @@ -219,7 +234,23 @@ export async function resolveCanonicalUserId( providerEmail: display.email, avatarUrl: display.avatarUrl, }); - return { userId: user.id }; + if (ctx.authorization && user.id !== ctx.authorization.userId) { + logMismatchRejected( + "actor-resolution", + "canonicalUserId", + ctx.authorization.userId, + user.id, + ctx + ); + return json( + { + error: "Actor identity changed; retry the request", + code: "actor_identity_changed", + }, + 409 + ); + } + return requireActive(user.id); } catch (e) { logger.error("Failed to resolve verified actor identity", { error: e instanceof Error ? e : String(e), @@ -261,37 +292,3 @@ function logMismatchRejected( trace_id: ctx.trace_id, }); } - -/** - * The bot service allowed to post each normalized automation event source. - * `null` marks sources that are not bot-posted (sentry/webhook arrive on the - * CP's own public webhook surface; linear posts no normalized events today) - * — an explicit exemption, not a missing row. - */ -const EVENT_SOURCE_SERVICE: Record = { - slack: "slack-bot", - github: "github-bot", - linear: null, - sentry: null, - webhook: null, -}; - -/** - * Gate for the internal normalized automation-event endpoints: the poster - * must be a service principal (401 otherwise), and per-service sources - * accept only the source's own bot. Sources with a null row arrive via the - * CP's own public webhook surface, so any service may forward them. - */ -export function requireEventPoster( - ctx: RequestContext, - source: AutomationEventSource -): Response | null { - const principal = ctx.principal; - if (principal?.kind !== "service") { - return error("Unauthorized", 401); - } - const expected = EVENT_SOURCE_SERVICE[source]; - if (expected === null || principal.service === expected) return null; - logMismatchRejected(`internal-${source}-event`, "service", expected, principal.service, ctx); - return error("Unauthorized", 401); -} diff --git a/packages/control-plane/src/authorization/service-permissions.test.ts b/packages/control-plane/src/authorization/service-permissions.test.ts new file mode 100644 index 0000000000..b1208e1cd2 --- /dev/null +++ b/packages/control-plane/src/authorization/service-permissions.test.ts @@ -0,0 +1,11 @@ +import { describe, expect, it } from "vitest"; +import { serviceAllowsPermission } from "./service-permissions"; + +describe("serviceAllowsPermission", () => { + it("allows launch capabilities but denies management capabilities", () => { + expect(serviceAllowsPermission("slack-bot", "sessions.create")).toBe(true); + expect(serviceAllowsPermission("linear-bot", "integrations.read")).toBe(true); + expect(serviceAllowsPermission("slack-bot", "global_secrets.manage")).toBe(false); + expect(serviceAllowsPermission("github-bot", "sessions.sandbox_access")).toBe(false); + }); +}); diff --git a/packages/control-plane/src/authorization/service-permissions.ts b/packages/control-plane/src/authorization/service-permissions.ts new file mode 100644 index 0000000000..1867923e7c --- /dev/null +++ b/packages/control-plane/src/authorization/service-permissions.ts @@ -0,0 +1,49 @@ +import type { PermissionId } from "@open-inspect/shared/rbac"; +import type { ServiceName } from "@open-inspect/shared/service-auth"; + +const SERVICE_PERMISSION_CEILINGS: Record = { + web: [], + "github-bot": [ + "repositories.read", + "repositories.use", + "environments.read", + "environments.use", + "integrations.read", + "sessions.create", + "sessions.read", + "sessions.collaborate", + "sessions.lifecycle", + "skills.read", + ], + "slack-bot": [ + "automations.read", + "repositories.read", + "repositories.use", + "environments.read", + "environments.use", + "integrations.read", + "sessions.create", + "sessions.read", + "sessions.collaborate", + "sessions.lifecycle", + "sessions.sandbox_access", + "skills.read", + ], + "linear-bot": [ + "repositories.read", + "repositories.use", + "environments.read", + "environments.use", + "integrations.read", + "sessions.create", + "sessions.read", + "sessions.collaborate", + "sessions.lifecycle", + "skills.read", + ], +}; + +/** Checks the hard permission ceiling for a trusted service, independent of user grants. */ +export function serviceAllowsPermission(service: ServiceName, permission: PermissionId): boolean { + return SERVICE_PERMISSION_CEILINGS[service].includes(permission); +} diff --git a/packages/control-plane/src/router.analytics.test.ts b/packages/control-plane/src/router.analytics.test.ts index 9972051573..4c8d5db66f 100644 --- a/packages/control-plane/src/router.analytics.test.ts +++ b/packages/control-plane/src/router.analytics.test.ts @@ -27,7 +27,7 @@ describe("analytics router integration", () => { vi.clearAllMocks(); }); - it("serves analytics routes even when the SCM provider is not github", async () => { + it("does not let an actorless service read analytics", async () => { mockStore.getSummary.mockResolvedValue({ totalSessions: 1, activeUsers: 1, @@ -63,21 +63,8 @@ describe("analytics router integration", () => { TEST_BACKGROUND_TASK_CONTEXT ); - expect(response.status).toBe(200); - await expect(response.json()).resolves.toEqual({ - totalSessions: 1, - activeUsers: 1, - totalCost: 0, - avgCost: 0, - totalPrs: 0, - statusBreakdown: { - created: 1, - active: 0, - completed: 0, - failed: 0, - archived: 0, - cancelled: 0, - }, - }); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_actor_required" }); + expect(mockStore.getSummary).not.toHaveBeenCalled(); }); }); diff --git a/packages/control-plane/src/router.create-session.test.ts b/packages/control-plane/src/router.create-session.test.ts index 0114453cd2..1e27d75a6a 100644 --- a/packages/control-plane/src/router.create-session.test.ts +++ b/packages/control-plane/src/router.create-session.test.ts @@ -1,6 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { generateEncryptionKey } from "./auth/crypto"; -import type { Principal } from "./auth/principal"; import { SessionIndexStore } from "./db/session-index"; import { UserStore } from "./db/user-store"; import { handleRequest } from "./router"; @@ -15,6 +14,7 @@ import { SessionInternalPaths } from "./session/contracts"; import { resolveManagedSkills } from "./session/skill-resolution"; import { resolveSessionProviderAuth } from "./session/provider-account-resolution"; import { ProviderAccountSelectionPolicyError } from "./model-provider-accounts/selection-policy"; +import { resolveEnvironmentTarget, resolveSessionRepositories } from "./repos/resolve"; vi.mock("./db/session-index", () => ({ SessionIndexStore: vi.fn(), @@ -45,10 +45,14 @@ vi.mock("./routes/shared", async (importOriginal) => { }; }); -const USER_PRINCIPAL: Principal = { - kind: "user", - userId: "user-1", -}; +vi.mock("./repos/resolve", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + resolveEnvironmentTarget: vi.fn(), + resolveSessionRepositories: vi.fn(), + }; +}); describe("handleCreateSession D1 ordering", () => { beforeEach(() => { @@ -68,6 +72,16 @@ describe("handleCreateSession D1 ordering", () => { repoId: 12345, defaultBranch: "main", } as never); + vi.mocked(resolveEnvironmentTarget).mockResolvedValue([ + { repoOwner: "acme", repoName: "environment-repo", baseBranch: "main" }, + ]); + vi.mocked(resolveSessionRepositories).mockImplementation(async (_env, repositories) => + repositories.map((repository, index) => ({ + ...repository, + repoId: 12345 + index, + baseBranch: repository.baseBranch ?? "main", + })) + ); // Default identity fixture: the slack-bot's asserted actor resolves to an // already-known canonical user with no linked GitHub identity. vi.mocked(UserStore).mockImplementation(function () { @@ -119,7 +133,10 @@ describe("handleCreateSession D1 ordering", () => { ); } - function createEnv(initFetch: ReturnType): Record { + function createEnv( + initFetch: ReturnType, + permissions = ["sessions.create", "repositories.use", "environments.use"] + ): Record { const statement = { bind: vi.fn(() => statement), first: vi.fn(async () => null), @@ -134,7 +151,42 @@ describe("handleCreateSession D1 ordering", () => { // the env must carry valid key material (the db stub answers "no rows"). TOKEN_ENCRYPTION_KEY: generateEncryptionKey(), DB: { - prepare: vi.fn(() => statement), + prepare: vi.fn((sql: string) => { + if (sql.includes("FROM users u") && sql.includes("user_role_assignments")) { + const authorizationStatement = { + bind: vi.fn(() => authorizationStatement), + first: vi.fn(async () => ({ + user_id: "user-1", + suspended_at: null, + role_id: "role-1", + role_key: null, + role_name: "Test Role", + })), + all: vi.fn(async () => ({ results: [] })), + }; + return authorizationStatement; + } + if (sql.includes("FROM role_permissions")) { + const permissionStatement = { + bind: vi.fn(() => permissionStatement), + first: vi.fn(async () => null), + all: vi.fn(async () => ({ + results: permissions.map((permission_id) => ({ permission_id })), + })), + }; + return permissionStatement; + } + if (sql.includes("suspended_at IS NULL")) { + const activeStatement = { + bind: vi.fn(() => activeStatement), + first: vi.fn(async () => ({ active: 1 })), + all: vi.fn(async () => ({ results: [] })), + run: vi.fn(async () => ({ meta: { changes: 0 } })), + }; + return activeStatement; + } + return statement; + }), batch: vi.fn(), exec: vi.fn(), dump: vi.fn(), @@ -146,6 +198,76 @@ describe("handleCreateSession D1 ordering", () => { }; } + it.each([ + { + target: "environment", + body: { environmentId: "env_1" }, + permissions: ["sessions.create", "environments.use"], + status: 201, + deniedPermission: null, + }, + { + target: "environment", + body: { environmentId: "env_1" }, + permissions: ["sessions.create", "repositories.use"], + status: 403, + deniedPermission: "environments.use", + }, + { + target: "scalar repository", + body: { repoOwner: "acme", repoName: "widgets" }, + permissions: ["sessions.create", "repositories.use"], + status: 201, + deniedPermission: null, + }, + { + target: "scalar repository", + body: { repoOwner: "acme", repoName: "widgets" }, + permissions: ["sessions.create", "environments.use"], + status: 403, + deniedPermission: "repositories.use", + }, + { + target: "repository list", + body: { repositories: [{ repoOwner: "acme", repoName: "widgets" }] }, + permissions: ["sessions.create", "repositories.use"], + status: 201, + deniedPermission: null, + }, + { + target: "repository list", + body: { repositories: [{ repoOwner: "acme", repoName: "widgets" }] }, + permissions: ["sessions.create", "environments.use"], + status: 403, + deniedPermission: "repositories.use", + }, + ])( + "enforces the permission matrix for $target targets", + async ({ body, permissions, status, deniedPermission }) => { + const create = vi.fn().mockResolvedValue(undefined); + vi.mocked(SessionIndexStore).mockImplementation(function () { + return { create } as never; + }); + const initFetch = vi.fn(async () => Response.json({ status: "created" })); + + const response = await createSessionRequestWithBody(createEnv(initFetch, permissions), { + ...body, + title: "Permission matrix", + }); + + expect(response.status).toBe(status); + if (deniedPermission) { + await expect(response.json()).resolves.toMatchObject({ + code: "permission_required", + permission: deniedPermission, + }); + expect(create).not.toHaveBeenCalled(); + } else { + expect(create).toHaveBeenCalledOnce(); + } + } + ); + it("does not initialize the SessionDO when D1 session index creation fails", async () => { const create = vi.fn().mockRejectedValue(new Error("D1 unavailable")); vi.mocked(SessionIndexStore).mockImplementation(function () { @@ -402,7 +524,7 @@ describe("handleCreateSession D1 ordering", () => { vi.mocked(SessionIndexStore).mockImplementation(function () { return { create } as never; }); - const resolveOrCreateUser = vi.fn(async () => ({ id: "user-9" })); + const resolveOrCreateUser = vi.fn(async () => ({ id: "user-1" })); vi.mocked(UserStore).mockImplementation(function () { return { getIdentity: async () => null, @@ -428,7 +550,7 @@ describe("handleCreateSession D1 ordering", () => { providerEmail: "ada@example.com", avatarUrl: "https://avatars.example.com/ada.png", }); - expect(create).toHaveBeenCalledWith(expect.objectContaining({ userId: "user-9" })); + expect(create).toHaveBeenCalledWith(expect.objectContaining({ userId: "user-1" })); expect(initFetch).toHaveBeenCalledOnce(); }); @@ -452,9 +574,10 @@ describe("handleCreateSession D1 ordering", () => { model: "anthropic/claude-haiku-4-5", }); - expect(response.status).toBe(500); + expect(response.status).toBe(503); await expect(response.json()).resolves.toEqual({ - error: "Failed to resolve session identity", + error: "Authorization unavailable", + code: "authorization_unavailable", }); expect(create).not.toHaveBeenCalled(); expect(initFetch).not.toHaveBeenCalled(); @@ -511,7 +634,22 @@ describe("handleCreateSession D1 ordering", () => { { request_id: "test-request", trace_id: "test-trace", - principal: USER_PRINCIPAL, + principal: { + kind: "service", + service: "linear-bot", + actor: { + provider: "linear", + providerUserId: "linear-user-1", + canonicalUserId: "user-1", + participantUserId: "linear:linear-user-1", + }, + }, + authorization: { + userId: "user-1", + suspendedAt: null, + role: { id: "role-1", key: "member", name: "Member" }, + permissions: ["sessions.create", "repositories.use", "environments.use"], + }, db: testEnv["DB"] as never, executionCtx: TEST_BACKGROUND_TASK_CONTEXT, metrics: { diff --git a/packages/control-plane/src/router.policy.test.ts b/packages/control-plane/src/router.policy.test.ts index 2765af0856..d4b4a4fa8e 100644 --- a/packages/control-plane/src/router.policy.test.ts +++ b/packages/control-plane/src/router.policy.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it, vi } from "vitest"; import { enforceRoutePrincipal, handleRequest, routes } from "./router"; import { 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)); @@ -13,11 +15,213 @@ describe("route policy table", () => { routes.every( (route) => route.authentication && + route.authorization && (route.supportedScmProviders === "all" || route.supportedScmProviders.length > 0) ) ).toBe(true); }); + it("has no duplicate method and pattern declarations", () => { + const identities = routes.map((route) => `${route.method}:${route.pattern}`); + expect(new Set(identities).size).toBe(identities.length); + }); + + it("declares authorization compatible with authentication", () => { + for (const route of routes) { + const authentication = route.authentication.kind; + const authorization = route.authorization; + if (authorization.kind === "none") { + expect(["public", "handler-authenticated", "web-service", "sandbox"]).toContain( + authentication + ); + } else if (authorization.kind === "authenticated" || authorization.kind === "active-self") { + expect(authentication).toBe("user"); + } else if (authorization.kind === "service") { + expect(authentication).toBe("service"); + expect(authorization.services.length).toBeGreaterThan(0); + } else if (authorization.kind === "active-global") { + expect(["user", "user-or-service"]).toContain(authentication); + } else { + expect(["user", "user-or-service", "user-or-service-with-sandbox-fallback"]).toContain( + authentication + ); + expect(authorization.allOf.length).toBeGreaterThan(0); + for (const requirement of authorization.allOf) { + if (requirement.kind === "automation") { + expect(route.pattern.source).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}>`); + } + } + } + } + } + }); + + it.each([ + ["GET", "/repos", [{ service: "slack-bot" }, { service: "linear-bot" }]], + ["GET", "/repos/acme/widgets/metadata", [{ service: "github-bot" }]], + ["GET", "/environments", [{ service: "slack-bot" }, { service: "linear-bot" }]], + ["GET", "/environments/env-1", [{ service: "github-bot" }]], + ["GET", "/integration-settings/slack", [{ service: "slack-bot", pathParams: { id: "slack" } }]], + [ + "GET", + "/integration-settings/github/resolved/acme/widgets", + [ + { service: "github-bot", pathParams: { id: "github" } }, + { service: "linear-bot", pathParams: { id: "linear" } }, + ], + ], + ["GET", "/integration-settings/slack/watched-channels", [{ service: "slack-bot" }]], + ["GET", "/model-preferences", [{ service: "slack-bot" }]], + ])("declares the exact actorless grants for %s %s", (method, path, expected) => { + const authorization = routeFor(method, path)?.authorization; + expect(["active-user", "active-global"]).toContain(authorization?.kind); + if (authorization?.kind === "active-user" || authorization?.kind === "active-global") { + expect(authorization.service.kind).toBe("actor"); + if (authorization.service.kind === "actor") { + expect(authorization.service.actorlessGrants).toEqual(expected); + } + } + }); + + it("does not declare actorless grants on other routes", () => { + const expected = new Set([ + routeFor("GET", "/repos"), + routeFor("GET", "/repos/acme/widgets/metadata"), + routeFor("GET", "/environments"), + routeFor("GET", "/environments/env-1"), + routeFor("GET", "/integration-settings/slack"), + routeFor("GET", "/integration-settings/github/resolved/acme/widgets"), + routeFor("GET", "/integration-settings/slack/watched-channels"), + routeFor("GET", "/model-preferences"), + routeFor("POST", "/sessions/session-1/stop"), + routeFor("GET", "/sessions/session-1/media/artifact-1"), + ]); + const granted = routes.filter( + (route) => + (route.authorization.kind === "active-user" || + route.authorization.kind === "active-global") && + route.authorization.service.kind === "actor" && + (route.authorization.service.actorlessGrants?.length ?? 0) > 0 + ); + + expect(new Set(granted)).toEqual(expected); + }); + + it("keeps every actorless route grant within its service permission ceiling", () => { + for (const route of routes) { + if (route.authorization.kind !== "active-user") continue; + if (route.authorization.service.kind !== "actor") continue; + for (const grant of route.authorization.service.actorlessGrants ?? []) { + for (const requirement of route.authorization.allOf) { + if (requirement.kind === "permission") { + expect( + serviceAllowsPermission(grant.service, requirement.permission), + `${grant.service} must allow ${requirement.permission} for ${route.method} ${route.pattern}` + ).toBe(true); + } else if (requirement.kind === "scoped-permission") { + expect( + serviceAllowsPermission(grant.service, SCOPED_PERMISSION_PAIRS[requirement.stem].own) + ).toBe(true); + } + } + } + } + }); + + it("keeps contextual route requirements explicit", () => { + expect(routeFor("GET", "/keyboard-shortcuts")?.authorization).toEqual({ + kind: "active-self", + }); + expect(routeFor("GET", "/model-preferences")?.authorization).toMatchObject({ + kind: "active-global", + service: { kind: "actor", actorlessGrants: [{ service: "slack-bot" }] }, + }); + expect(routeFor("GET", "/sessions")?.authorization).toMatchObject({ + kind: "active-user", + allOf: [{ kind: "permission", permission: "sessions.read" }], + service: { kind: "actor" }, + }); + expect(routeFor("GET", "/sessions/inbox")?.authorization).toMatchObject({ + kind: "active-user", + allOf: [{ kind: "permission", permission: "sessions.read" }], + service: { kind: "deny" }, + }); + expect(routeFor("POST", "/sessions/session-1/stop")?.authorization).toMatchObject({ + service: { kind: "actor", actorlessGrants: [{ service: "linear-bot" }] }, + }); + expect(routeFor("GET", "/sessions/session-1/media/artifact-1")?.authorization).toMatchObject({ + service: { kind: "actor", actorlessGrants: [{ service: "slack-bot" }] }, + }); + expect(routeFor("POST", "/sessions/session-1/participants")?.authorization).toEqual({ + kind: "active-user", + allOf: [{ kind: "permission", permission: "sessions.collaborate" }], + service: { kind: "actor" }, + }); + expect(routeFor("POST", "/sessions/parent/children")?.authorization).toMatchObject({ + kind: "active-user", + allOf: [ + { kind: "permission", permission: "sessions.create" }, + { kind: "permission", permission: "sessions.collaborate" }, + ], + }); + expect(routeFor("GET", "/sessions/parent/children/child")?.authorization).toMatchObject({ + kind: "active-user", + allOf: [{ kind: "permission", permission: "sessions.read" }], + }); + expect(routeFor("POST", "/internal/github-event")?.authorization).toMatchObject({ + kind: "service", + services: ["github-bot"], + }); + expect(routeFor("POST", "/internal/github-event")?.authentication).toEqual({ + kind: "service", + }); + expect(routeFor("POST", "/internal/slack-event")?.authentication).toEqual({ + kind: "service", + }); + }); + + 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 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 + ); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ error: "Invalid role ID" }); + expect(prepare).not.toHaveBeenCalled(); + }); + + it.each([ + ["PUT", "/automations/automation-1", "manage"], + ["DELETE", "/automations/automation-1", "manage"], + ["POST", "/automations/automation-1/pause", "manage"], + ["POST", "/automations/automation-1/resume", "manage"], + ["POST", "/automations/automation-1/trigger", "trigger"], + ["POST", "/automations/automation-1/regenerate-key", "manage"], + ])("declares automation ownership authorization for %s %s", (method, path, operation) => { + expect(routeFor(method, path)?.authorization).toMatchObject({ + kind: "active-user", + allOf: [{ kind: "automation", operation, automationIdParam: "id" }], + service: { kind: "deny" }, + }); + }); + it.each([ ["GET", "/health", "public"], ["POST", "/webhooks/sentry/automation-1", "handler-authenticated"], @@ -61,6 +265,7 @@ describe("route policy table", () => { if (route?.authentication.kind === "user-or-service-with-sandbox-fallback") { expect(route.authentication.getSessionId(match)).toBe("session-1"); } + expect(route?.authorization.kind).toBe("active-user"); }); it.each([ @@ -199,6 +404,25 @@ describe("route policy dispatch ordering", () => { }); }); + it("keeps health dependency-free when D1 is unavailable", async () => { + const testEnv = env("github"); + testEnv.DB.prepare = vi.fn(() => { + throw new Error("D1 unavailable"); + }); + + const response = await handleRequest( + new Request("https://test.local/health"), + testEnv as never, + TEST_BACKGROUND_TASK_CONTEXT + ); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toEqual({ + status: "healthy", + service: "open-inspect-control-plane", + }); + }); + it("applies broker cache policy when sandbox authentication is unavailable", async () => { const testEnv = env("github") as ReturnType & { SESSION: { @@ -229,6 +453,10 @@ describe("route principal policy", () => { it.each([ [{ kind: "web-service" } as const, { kind: "service", service: "web", actor: null } as const], [{ kind: "user" } as const, { kind: "user", userId: "user-1" } as const], + [ + { kind: "service" } as const, + { kind: "service", service: "github-bot", actor: null } as const, + ], [ { kind: "user-or-service" } as const, { kind: "service", service: "linear-bot", actor: null } as const, @@ -249,6 +477,7 @@ describe("route principal policy", () => { { kind: "service", service: "linear-bot", actor: null } as const, 403, ], + [{ kind: "service" } as const, { kind: "user", userId: "user-1" } as const, 403], ])("rejects mismatched principals for %o", (authentication, principal, status) => { expect(enforceRoutePrincipal(authentication, principal)?.status).toBe(status); }); diff --git a/packages/control-plane/src/router.scm-credentials.test.ts b/packages/control-plane/src/router.scm-credentials.test.ts index 32c3e48fe9..85e8ddde33 100644 --- a/packages/control-plane/src/router.scm-credentials.test.ts +++ b/packages/control-plane/src/router.scm-credentials.test.ts @@ -10,7 +10,7 @@ function routeFor(method: string, path: string) { return routes.find((route) => route.method === method && route.pattern.test(path)); } -function createEnv() { +function createEnv(options?: { actorAuthorized?: boolean }) { const fetch = vi.fn(async (request: Request) => { if (new URL(request.url).pathname === "/internal/verify-sandbox-token") { const body = (await request.json()) as { token?: string }; @@ -35,7 +35,42 @@ function createEnv() { SCM_PROVIDER: "gitlab", GITLAB_ACCESS_TOKEN: "glpat-test", DB: { - prepare: vi.fn(() => statement), + prepare: vi.fn((sql: string) => { + if (options?.actorAuthorized && sql.includes("FROM user_identities")) { + const identityStatement = { + bind: vi.fn(() => identityStatement), + first: vi.fn(async () => ({ + id: "identity-linear-u1", + user_id: "user-1", + provider: "linear", + provider_user_id: "U1", + provider_login: null, + provider_email: null, + provider_issuer: null, + created_at: 1, + })), + }; + return identityStatement; + } + if ( + options?.actorAuthorized && + sql.includes("FROM users u") && + sql.includes("user_role_assignments") + ) { + const authorizationStatement = { + bind: vi.fn(() => authorizationStatement), + first: vi.fn(async () => ({ + user_id: "user-1", + suspended_at: null, + role_id: "role_builtin_member", + role_key: "member", + role_name: "Member", + })), + }; + return authorizationStatement; + } + return statement; + }), batch: vi.fn(), exec: vi.fn(), dump: vi.fn(), @@ -128,7 +163,7 @@ describe("SCM credentials router provider gate", () => { expect(new URL(fetch.mock.calls[1][0].url).pathname).toBe("/internal/scm-credentials"); }); - it("allows GitLab deployments to reach the tunnel URLs endpoint", async () => { + it("requires an actor for service access to tunnel URLs", async () => { const { env, fetch } = createEnv(); const response = await handleRequest( @@ -139,10 +174,9 @@ describe("SCM credentials router provider gate", () => { TEST_BACKGROUND_TASK_CONTEXT ); - expect(response.status).toBe(202); - expect(fetch).toHaveBeenCalledOnce(); - const request = fetch.mock.calls[0][0]; - expect(new URL(request.url).pathname).toBe("/internal/tunnel-urls"); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_actor_required" }); + expect(fetch).not.toHaveBeenCalled(); }); it("treats provider-neutral SCM settings routes as SCM-agnostic", () => { @@ -218,7 +252,7 @@ describe("SCM credentials router provider gate", () => { expect(new URL(fetch.mock.calls[0][0].url).pathname).toBe("/internal/verify-sandbox-token"); }); - it("continues blocking unrelated GitLab session routes", async () => { + it("rejects actorless services before unrelated GitLab session routes", async () => { const { env, fetch } = createEnv(); const response = await handleRequest( @@ -230,6 +264,24 @@ describe("SCM credentials router provider gate", () => { TEST_BACKGROUND_TASK_CONTEXT ); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_actor_required" }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it("returns the provider gate after authorizing an actor on a GitHub-only route", async () => { + const { env, fetch } = createEnv({ actorAuthorized: true }); + + const response = await handleRequest( + await signedServiceRequest("https://test.local/sessions/session-1/pr", { + method: "POST", + service: "linear-bot", + actor: "linear:U1", + }), + env as never, + TEST_BACKGROUND_TASK_CONTEXT + ); + expect(response.status).toBe(501); await expect(response.json()).resolves.toEqual({ error: "SCM provider 'gitlab' is not implemented in this deployment.", diff --git a/packages/control-plane/src/router.session-prompt.test.ts b/packages/control-plane/src/router.session-prompt.test.ts index 38074d47a6..88fffb4996 100644 --- a/packages/control-plane/src/router.session-prompt.test.ts +++ b/packages/control-plane/src/router.session-prompt.test.ts @@ -58,8 +58,17 @@ function userPromptRequest(body: Record): Promise { function createEnv(sessionFetch: ReturnType): Record { const statement = { bind: vi.fn(() => statement), - first: vi.fn(async () => null), - all: vi.fn(async () => ({ results: [] })), + first: vi.fn(async () => ({ + user_id: "user-1", + suspended_at: null, + assigned: 1, + role_id: "role_builtin_administrator", + role_key: "administrator", + role_name: "Administrator", + })), + all: vi.fn(async () => ({ + results: [{ permission_id: "sessions.collaborate" }], + })), run: vi.fn(async () => ({ meta: { changes: 0 } })), }; return { diff --git a/packages/control-plane/src/router.spawn-child.test.ts b/packages/control-plane/src/router.spawn-child.test.ts index 40d2ccfd5b..103ec2d117 100644 --- a/packages/control-plane/src/router.spawn-child.test.ts +++ b/packages/control-plane/src/router.spawn-child.test.ts @@ -23,6 +23,12 @@ vi.mock("./db/model-preferences", () => ({ getEffectiveEnabledModels: vi.fn(), })); +vi.mock("./db/user-store", () => ({ + UserStore: vi.fn().mockImplementation(function () { + return { getIdentity: async () => ({ userId: "canonical-user-123" }) }; + }), +})); + vi.mock("./session/integration-settings-resolution", () => integrationSettingsMocks); describe("handleSpawnChild prompt enqueue handling", () => { @@ -86,13 +92,14 @@ describe("handleSpawnChild prompt enqueue handling", () => { const makeStore = ( parentUserId: string | null = null, - context: typeof spawnContext = spawnContext + context: typeof spawnContext = spawnContext, + environmentId: string | null = "env_parent" ) => ({ get: vi.fn().mockResolvedValue({ userId: parentUserId, repoOwner: context.repoOwner, repoName: context.repoName, - environmentId: "env_parent", + environmentId, }), getSpawnDepth: vi.fn().mockResolvedValue(0), getCompleteProviderAuth: vi.fn().mockResolvedValue(parentProviderAuth), @@ -173,13 +180,14 @@ describe("handleSpawnChild prompt enqueue handling", () => { method: "POST", body: JSON.stringify(body), service: "linear-bot", + actor: "linear:U1", }), env as never, TEST_BACKGROUND_TASK_CONTEXT ); } - function makeSuccessfulEnv(context: TestSpawnContext) { + function makeSuccessfulEnv(context: TestSpawnContext, permissions?: string[]) { const parentStub: DurableObjectStub = { fetch: vi.fn(async () => Response.json(context)), } as never; @@ -198,7 +206,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { env: { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(permissions), SESSION: { idFromName: (name: string) => name, get: (id: string) => (id === parentId ? parentStub : childStub), @@ -207,6 +215,44 @@ describe("handleSpawnChild prompt enqueue handling", () => { }; } + it("rejects a repository-backed child when the actor cannot use repositories", async () => { + const store = makeStore(null, spawnContext, null); + vi.mocked(SessionIndexStore).mockImplementation(function () { + return store as never; + }); + const { env } = makeSuccessfulEnv(spawnContext, ["sessions.create", "sessions.collaborate"]); + + const response = await makeRequest(env); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "repositories.use", + }); + expect(store.create).not.toHaveBeenCalled(); + }); + + it("rejects an environment-backed child when the actor cannot use environments", async () => { + const store = makeStore(); + vi.mocked(SessionIndexStore).mockImplementation(function () { + return store as never; + }); + const { env } = makeSuccessfulEnv(spawnContext, [ + "sessions.create", + "sessions.collaborate", + "repositories.use", + ]); + + const response = await makeRequest(env); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "environments.use", + }); + expect(store.create).not.toHaveBeenCalled(); + }); + async function getInitBody(childStub: DurableObjectStub) { const initRequest = vi.mocked(childStub.fetch).mock.calls.find((call) => { const request = call[0] as Request; @@ -340,7 +386,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: (id: string) => (id === parentId ? parentStub : childStub), @@ -434,7 +480,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: (id: string) => (id === parentId ? parentStub : childStub), @@ -482,7 +528,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: (id: string) => (id === parentId ? parentStub : childStub), @@ -514,7 +560,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -525,6 +571,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { await signedServiceRequest(`https://test.local/sessions/${parentId}/children`, { method: "POST", service: "linear-bot", + actor: "linear:U1", body: JSON.stringify({ title: "Child task", prompt: "Do the thing", @@ -554,7 +601,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -577,7 +624,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: vi.fn(), @@ -588,6 +635,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { await signedServiceRequest(`https://test.local/sessions/${parentId}/children`, { method: "POST", service: "linear-bot", + actor: "linear:U1", body: JSON.stringify({ title: "Child task" }), }), env as never, @@ -612,7 +660,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -640,7 +688,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -666,7 +714,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -699,7 +747,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -740,7 +788,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -768,7 +816,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -779,6 +827,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { await signedServiceRequest(`https://test.local/sessions/${parentId}/children`, { method: "POST", service: "linear-bot", + actor: "linear:U1", body: JSON.stringify({ title: "Child task", prompt: "Do the thing", @@ -809,7 +858,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: () => parentStub, @@ -849,7 +898,7 @@ describe("handleSpawnChild prompt enqueue handling", () => { const env = { ...TEST_SERVICE_SECRETS, SCM_PROVIDER: "github", - DB: {}, + DB: authorizedDb(), SESSION: { idFromName: (name: string) => name, get: (id: string) => (id === parentId ? parentStub : childStub), @@ -866,3 +915,31 @@ describe("handleSpawnChild prompt enqueue handling", () => { expect(store.updateStatus).toHaveBeenCalledWith(createdChildId, "failed"); }); }); +function authorizedDb( + permissions = ["sessions.create", "repositories.use", "environments.use", "sessions.collaborate"] +) { + return { + prepare: vi.fn((sql: string) => { + const statement = { + bind: vi.fn(() => statement), + first: vi.fn(async () => + sql.includes("FROM users u") + ? { + user_id: "canonical-user-123", + suspended_at: null, + role_id: "role_custom_spawn_test", + role_key: null, + role_name: "Spawn Test", + } + : null + ), + all: vi.fn(async () => ({ + results: sql.includes("FROM role_permissions") + ? permissions.map((permission_id) => ({ permission_id })) + : [], + })), + }; + return statement; + }), + }; +} diff --git a/packages/control-plane/src/router.ts b/packages/control-plane/src/router.ts index ed694c9db3..5b406dca05 100644 --- a/packages/control-plane/src/router.ts +++ b/packages/control-plane/src/router.ts @@ -15,15 +15,24 @@ import { SessionInternalPaths } from "./session/contracts"; import { createSessionRuntimeClient } from "./session/runtime-client"; import { createRequestMetrics, instrumentD1 } from "./db/instrumented-d1"; +import { UserStore } from "./db/user-store"; +import { AutomationStore } from "./db/automation-store"; +import { AuthorizationError, AuthorizationService } from "./authorization/service"; +import { serviceAllowsPermission } from "./authorization/service-permissions"; +import { SCOPED_PERMISSION_PAIRS, resolveScopedPermission } from "@open-inspect/shared/rbac"; import { createLogger } from "./logger"; import type { BackgroundTasks } from "./platform-ports"; import { + type ActorlessServiceGrant, type Route, type RouteAuthentication, + type RouteAuthorizationRequirement, type RequestContext, defineRoute, GITHUB_SANDBOX_FALLBACK_ROUTE, + NO_AUTHORIZATION, parsePattern, + requirePermission, json, error, HttpError, @@ -45,6 +54,7 @@ import { analyticsRoutes } from "./routes/analytics"; import { autofixRoutes } from "./routes/autofix"; import { skillRoutes } from "./routes/skills"; import { keyboardShortcutRoutes } from "./routes/keyboard-shortcuts"; +import { rbacRoutes } from "./routes/rbac"; import { sessionRoutes } from "./routes/sessions"; import { modelProviderAccountRoutes } from "./routes/model-provider-accounts"; import { handleSlackNotify } from "./routes/slack-notify"; @@ -292,6 +302,219 @@ export function enforceRoutePrincipal( if (authentication.kind === "user" && principal.kind !== "user") { return error("Human user authentication required", 403); } + if (authentication.kind === "service" && principal.kind !== "service") { + return error("Service authentication required", 403); + } + return null; +} + +async function enforceActiveUser(route: Route, ctx: RequestContext): Promise { + if ( + route.authorization.kind !== "active-user" && + route.authorization.kind !== "active-self" && + route.authorization.kind !== "active-global" + ) { + return null; + } + let resolvedServiceUserId: string | null = null; + if ( + ctx.principal?.kind === "service" && + ctx.principal.actor && + !ctx.principal.actor.canonicalUserId + ) { + try { + const user = await new UserStore(ctx.db).resolveOrCreateUser({ + provider: ctx.principal.actor.provider, + providerUserId: ctx.principal.actor.providerUserId, + }); + resolvedServiceUserId = user.id; + } catch { + return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); + } + } + const userId = + ctx.principal?.kind === "user" + ? ctx.principal.userId + : ctx.principal?.kind === "service" + ? (ctx.principal.actor?.canonicalUserId ?? resolvedServiceUserId) + : null; + if (!userId) return null; + try { + const authorization = await new AuthorizationService(ctx.db).getEffectiveAuthorization(userId); + ctx.authorization = authorization; + if (authorization.suspendedAt !== null) { + return json({ error: "Forbidden", code: "active_user_required" }, 403); + } + return null; + } catch (cause) { + if (cause instanceof AuthorizationError) { + return json({ error: "Forbidden", code: cause.code }, cause.status); + } + return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); + } +} + +function authorizationUserId(ctx: RequestContext): string | null { + if (ctx.principal?.kind === "user") return ctx.principal.userId; + if (ctx.principal?.kind === "service") { + return ctx.principal.actor?.canonicalUserId ?? ctx.authorization?.userId ?? null; + } + return null; +} + +function actorlessGrantMatches( + grant: ActorlessServiceGrant, + service: string, + match: RegExpMatchArray +): boolean { + if (grant.service !== service) return false; + return Object.entries(grant.pathParams ?? {}).every(([name, expected]) => { + const value = match.groups?.[name]; + if (value === undefined) return false; + try { + return decodeURIComponent(value) === expected; + } catch { + return false; + } + }); +} + +function enforceServiceRouteAuthorization( + route: Route, + match: RegExpMatchArray, + ctx: RequestContext +): Response | null { + const principal = ctx.principal; + const authorization = route.authorization; + if (authorization.kind === "service") { + if (principal?.kind !== "service") { + return json({ error: "Forbidden", code: "service_capability_required" }, 403); + } + if (!authorization.services.some((service) => service === principal.service)) { + return json({ error: "Forbidden", code: "service_capability_required" }, 403); + } + if (authorization.actor === "required" && !principal.actor) { + return json({ error: "Forbidden", code: "service_actor_required" }, 403); + } + return null; + } + if (principal?.kind !== "service") return null; + if (route.authentication.kind === "web-service" && principal.service === "web") return null; + if ( + (authorization.kind !== "active-user" && authorization.kind !== "active-global") || + authorization.service.kind === "deny" + ) { + return json({ error: "Forbidden", code: "service_capability_required" }, 403); + } + if (principal.actor) return null; + const granted = authorization.service.actorlessGrants?.some((grant) => + actorlessGrantMatches(grant, principal.service, match) + ); + return granted ? null : json({ error: "Forbidden", code: "service_actor_required" }, 403); +} + +async function enforcePermissionRequirement( + requirement: Extract, + ctx: RequestContext +): Promise { + if ( + ctx.principal?.kind === "service" && + !serviceAllowsPermission(ctx.principal.service, requirement.permission) + ) { + return json({ error: "Forbidden", code: "service_capability_required" }, 403); + } + const userId = authorizationUserId(ctx); + if (!userId) return null; + if (ctx.authorization?.permissions.includes(requirement.permission)) return null; + return json( + { error: "Forbidden", code: "permission_required", permission: requirement.permission }, + 403 + ); +} + +async function enforceScopedPermissionRequirement( + requirement: Extract, + ctx: RequestContext +): Promise { + const pair = SCOPED_PERMISSION_PAIRS[requirement.stem]; + if ( + ctx.principal?.kind === "service" && + !serviceAllowsPermission(ctx.principal.service, pair.own) + ) { + return json({ error: "Forbidden", code: "service_capability_required" }, 403); + } + const userId = authorizationUserId(ctx); + if (!userId) return null; + if ( + ctx.authorization && + resolveScopedPermission(requirement.stem, ctx.authorization.permissions) + ) { + return null; + } + return json({ error: "Forbidden", code: "permission_required", permission: pair.own }, 403); +} + +async function enforceAutomationRequirement( + requirement: Extract, + match: RegExpMatchArray, + ctx: RequestContext +): Promise { + if (ctx.principal?.kind !== "user") return null; + const encodedAutomationId = match.groups?.[requirement.automationIdParam]; + if (!encodedAutomationId) return json({ error: "Invalid automation route" }, 400); + let automationId: string; + try { + automationId = decodeURIComponent(encodedAutomationId); + } catch { + return json({ error: "Invalid automation route" }, 400); + } + + try { + const authorization = ctx.authorization; + if (!authorization) throw new Error("Missing request authorization"); + const automation = await new AutomationStore(ctx.db).getById(automationId); + if (!automation) return error("Automation not found", 404); + + const permissionStem = `automations.${requirement.operation}` as const; + const permissionScope = resolveScopedPermission(permissionStem, authorization.permissions); + const ownPermission = SCOPED_PERMISSION_PAIRS[permissionStem].own; + if ( + !permissionScope || + (permissionScope === "own" && automation.user_id !== ctx.principal.userId) + ) { + return json( + { error: "Forbidden", code: "permission_required", permission: ownPermission }, + 403 + ); + } + + return null; + } catch { + return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); + } +} + +async function enforceRouteAuthorization( + route: Route, + match: RegExpMatchArray, + ctx: RequestContext +): Promise { + if (route.authorization.kind !== "active-user") return null; + for (const requirement of route.authorization.allOf) { + let authorizationError: Response | null; + switch (requirement.kind) { + case "permission": + authorizationError = await enforcePermissionRequirement(requirement, ctx); + break; + case "scoped-permission": + authorizationError = await enforceScopedPermissionRequirement(requirement, ctx); + break; + case "automation": + authorizationError = await enforceAutomationRequirement(requirement, match, ctx); + break; + } + if (authorizationError) return authorizationError; + } return null; } @@ -305,7 +528,12 @@ export const routes: Route[] = [ supportedScmProviders: "all", method: "GET", pattern: parsePattern("/health"), - handler: async () => json({ status: "healthy", service: "open-inspect-control-plane" }), + authorization: NO_AUTHORIZATION, + handler: async () => + json({ + status: "healthy", + service: "open-inspect-control-plane", + }), }, ...browserAuthRoutes, @@ -317,6 +545,7 @@ export const routes: Route[] = [ defineRoute(GITHUB_SANDBOX_FALLBACK_ROUTE, { method: "POST", pattern: parsePattern("/sessions/:id/slack-notify"), + authorization: requirePermission("sessions.collaborate"), handler: handleSlackNotify, }), @@ -366,6 +595,9 @@ export const routes: Route[] = [ // Personal keyboard shortcuts ...keyboardShortcutRoutes, + // Workspace roles, members, and current-user authorization + ...rbacRoutes, + // Webhooks (public routes — auth handled per-route) ...webhookRoutes, ]; @@ -457,7 +689,10 @@ export async function handleRequest( : error("Unauthorized: Invalid session path", 401); } else { const authResult = await authenticate(request, env, ctx, { - webService: authentication.kind === "web-service" ? "service" : "user", + webService: + authentication.kind === "web-service" || authentication.kind === "service" + ? "service" + : "user", }); if (isAuthError(authResult)) { @@ -495,6 +730,38 @@ export async function handleRequest( } } + const serviceAccessError = enforceServiceRouteAuthorization( + matchedRoute.route, + matchedRoute.match, + ctx + ); + if (serviceAccessError) { + logRequest(serviceAccessError, ctx, method, path, startTime); + return withCorsAndTraceHeaders( + withRouteCachePolicy(serviceAccessError, matchedRoute.route), + ctx + ); + } + + const userAccessError = await enforceActiveUser(matchedRoute.route, ctx); + if (userAccessError) { + logRequest(userAccessError, ctx, method, path, startTime); + return withCorsAndTraceHeaders(withRouteCachePolicy(userAccessError, matchedRoute.route), ctx); + } + + const authorizationError = await enforceRouteAuthorization( + matchedRoute.route, + matchedRoute.match, + ctx + ); + if (authorizationError) { + logRequest(authorizationError, ctx, method, path, startTime); + return withCorsAndTraceHeaders( + withRouteCachePolicy(authorizationError, matchedRoute.route), + ctx + ); + } + const providerCheck = enforceImplementedScmProvider(matchedRoute.route, path, env, ctx); if (providerCheck) { return withRouteCachePolicy(providerCheck, matchedRoute.route); diff --git a/packages/control-plane/src/routes/analytics.ts b/packages/control-plane/src/routes/analytics.ts index 5bbef8120e..53e17b20be 100644 --- a/packages/control-plane/src/routes/analytics.ts +++ b/packages/control-plane/src/routes/analytics.ts @@ -18,6 +18,7 @@ import { error, json, parsePattern, + requirePermission, } from "./shared"; function parseDaysParam(value: string | null): AnalyticsDays | null { @@ -124,21 +125,25 @@ export const analyticsRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_USER_OR_SERVIC { method: "GET", pattern: parsePattern("/analytics/summary"), + authorization: requirePermission("analytics.read"), handler: handleSummary, }, { method: "GET", pattern: parsePattern("/analytics/timeseries"), + authorization: requirePermission("analytics.read"), handler: handleTimeseries, }, { method: "GET", pattern: parsePattern("/analytics/breakdown"), + authorization: requirePermission("analytics.read"), handler: handleBreakdown, }, { method: "GET", pattern: parsePattern("/analytics/pull-requests"), + authorization: requirePermission("analytics.read"), handler: handlePullRequests, }, ]); diff --git a/packages/control-plane/src/routes/autofix.ts b/packages/control-plane/src/routes/autofix.ts index 723dbb295e..f132d243d1 100644 --- a/packages/control-plane/src/routes/autofix.ts +++ b/packages/control-plane/src/routes/autofix.ts @@ -3,6 +3,7 @@ import { defineRoutes, error, json, + NO_AUTHORIZATION, parsePattern, SCM_AGNOSTIC_WEB_SERVICE_ROUTE, type Route, @@ -35,6 +36,7 @@ export const autofixRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_WEB_SERVICE_ROUT { method: "GET", pattern: parsePattern("/autofix/activity"), + authorization: NO_AUTHORIZATION, handler: handleActivity, }, ]); diff --git a/packages/control-plane/src/routes/automations.test.ts b/packages/control-plane/src/routes/automations.test.ts index c517525777..769ca4953f 100644 --- a/packages/control-plane/src/routes/automations.test.ts +++ b/packages/control-plane/src/routes/automations.test.ts @@ -179,11 +179,20 @@ const SLACK_BOT_PRINCIPAL: Principal = { }; function createCtx(principal: Principal = USER_PRINCIPAL): RequestContext { + const statement = { + bind: vi.fn(), + first: vi.fn(async () => ({ active: 1 })), + }; + statement.bind.mockReturnValue(statement); + return { trace_id: "trace-1", request_id: "req-1", principal, - db: { batch: mockBatch } as unknown as SqlDatabase, + db: { + batch: mockBatch, + prepare: vi.fn(() => statement), + } as unknown as SqlDatabase, executionCtx: TEST_BACKGROUND_TASK_CONTEXT, metrics: { d1Queries: [], diff --git a/packages/control-plane/src/routes/automations.ts b/packages/control-plane/src/routes/automations.ts index 624232123e..fc638ed7fe 100644 --- a/packages/control-plane/src/routes/automations.ts +++ b/packages/control-plane/src/routes/automations.ts @@ -61,6 +61,8 @@ import { error, parseJsonBody, resolveRepoOrError, + requireAutomation, + requirePermission, } from "./shared"; import type { Env } from "../types"; import type { SqlDatabase, SqlStatement } from "../db/sql-database"; @@ -1354,66 +1356,81 @@ export const automationRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROU { method: "GET", pattern: parsePattern("/integration-settings/slack/watched-channels"), + authorization: requirePermission("automations.read", { + actorlessGrants: [{ service: "slack-bot" }], + }), handler: handleGetWatchedSlackChannels, }, { method: "GET", pattern: parsePattern("/integration-settings/slack/channels"), + authorization: requirePermission("automations.read"), handler: handleGetSlackChannels, }, { method: "GET", pattern: parsePattern("/automations"), + authorization: requirePermission("automations.read"), handler: handleListAutomations, }, { method: "POST", pattern: parsePattern("/automations"), + authorization: requirePermission("automations.create"), handler: handleCreateAutomation, }, { method: "GET", pattern: parsePattern("/automations/:id"), + authorization: requirePermission("automations.read"), handler: handleGetAutomation, }, { method: "PUT", pattern: parsePattern("/automations/:id"), + authorization: requireAutomation("manage"), handler: handleUpdateAutomation, }, { method: "DELETE", pattern: parsePattern("/automations/:id"), + authorization: requireAutomation("manage"), handler: handleDeleteAutomation, }, { method: "POST", pattern: parsePattern("/automations/:id/pause"), + authorization: requireAutomation("manage"), handler: handlePauseAutomation, }, { method: "POST", pattern: parsePattern("/automations/:id/resume"), + authorization: requireAutomation("manage"), handler: handleResumeAutomation, }, { method: "POST", pattern: parsePattern("/automations/:id/trigger"), + authorization: requireAutomation("trigger"), handler: handleTriggerAutomation, }, { method: "GET", pattern: parsePattern("/automations/:id/invocations"), + authorization: requirePermission("automations.read"), handler: handleListInvocations, }, { method: "GET", pattern: parsePattern("/automations/:id/runs/:runId"), + authorization: requirePermission("automations.read"), handler: handleGetRun, }, { method: "POST", pattern: parsePattern("/automations/:id/regenerate-key"), + authorization: requireAutomation("manage"), handler: handleRegenerateKey, }, ]); diff --git a/packages/control-plane/src/routes/browser-auth.ts b/packages/control-plane/src/routes/browser-auth.ts index 7bbc71546f..395fe0eb88 100644 --- a/packages/control-plane/src/routes/browser-auth.ts +++ b/packages/control-plane/src/routes/browser-auth.ts @@ -4,6 +4,7 @@ import { createLogger } from "../logger"; import { defineRoutes, error, + NO_AUTHORIZATION, parsePattern, SCM_AGNOSTIC_WEB_SERVICE_ROUTE, type Route, @@ -54,7 +55,8 @@ const handleBrowserAuth: Route["handler"] = async (request, _env, _match, ctx) = if (!ctx.getUserAuth) { throw new UserAuthConfigurationError("User authentication runtime is unavailable"); } - const response = await forwardBrowserAuthRequest(ctx.getUserAuth(), request); + const auth = ctx.getUserAuth(); + const response = await forwardBrowserAuthRequest(auth, request); const headers = copyBrowserAuthResponseHeaders(response.headers); headers.set("Cache-Control", "no-store"); headers.set("Referrer-Policy", "no-referrer"); @@ -86,6 +88,7 @@ export const browserAuthRoutes: Route[] = defineRoutes( BROWSER_AUTH_PROXY_ROUTES.map(([method, path]) => ({ method, pattern: parsePattern(path), + authorization: NO_AUTHORIZATION, handler: handleBrowserAuth, })) ); diff --git a/packages/control-plane/src/routes/commit-signing.ts b/packages/control-plane/src/routes/commit-signing.ts index a9e851176b..9286faa8ab 100644 --- a/packages/control-plane/src/routes/commit-signing.ts +++ b/packages/control-plane/src/routes/commit-signing.ts @@ -19,6 +19,8 @@ import { defineRoute, GITHUB_USER_OR_SERVICE_ROUTE, SCM_AGNOSTIC_SANDBOX_ROUTE, + NO_AUTHORIZATION, + requirePermission, } from "./shared"; const MAX_SIGNING_PAYLOAD_BYTES = 1024 * 1024; @@ -215,26 +217,31 @@ export const commitSigningRoutes: Route[] = [ defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "GET", pattern: parsePattern("/commit-signing"), + authorization: requirePermission("integrations.read"), handler: handleGetCommitSigning, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "PUT", pattern: parsePattern("/commit-signing"), + authorization: requirePermission("commit_signing.manage"), handler: handlePutCommitSigning, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "DELETE", pattern: parsePattern("/commit-signing"), + authorization: requirePermission("commit_signing.manage"), handler: handleDeleteCommitSigning, }), defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { method: "GET", pattern: parsePattern("/sessions/:id/commit-signing"), + authorization: NO_AUTHORIZATION, handler: handleGetSandboxCommitSigning, }), defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { method: "POST", pattern: parsePattern("/sessions/:id/commit-signing"), + authorization: NO_AUTHORIZATION, handler: handlePostSandboxCommitSigning, }), ]; diff --git a/packages/control-plane/src/routes/environment-secrets.ts b/packages/control-plane/src/routes/environment-secrets.ts index 790c073df7..460d56505b 100644 --- a/packages/control-plane/src/routes/environment-secrets.ts +++ b/packages/control-plane/src/routes/environment-secrets.ts @@ -23,6 +23,7 @@ import { error, parseJsonBody, resolveRepoOrError, + requirePermission, } from "./shared"; import { environmentSecretsImportBodySchema, @@ -304,21 +305,25 @@ export const environmentSecretsRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SER { method: "GET", pattern: parsePattern("/environments/:id/secrets"), + authorization: requirePermission("environments.secrets.manage"), handler: handleListEnvironmentSecrets, }, { method: "PUT", pattern: parsePattern("/environments/:id/secrets"), + authorization: requirePermission("environments.secrets.manage"), handler: handleSetEnvironmentSecrets, }, { method: "POST", pattern: parsePattern("/environments/:id/secrets/import"), + authorization: requirePermission("environments.secrets.manage"), handler: handleImportEnvironmentSecrets, }, { method: "DELETE", pattern: parsePattern("/environments/:id/secrets/:key"), + authorization: requirePermission("environments.secrets.manage"), handler: handleDeleteEnvironmentSecret, }, ]); diff --git a/packages/control-plane/src/routes/environments.ts b/packages/control-plane/src/routes/environments.ts index 80fc8942eb..17201e3e88 100644 --- a/packages/control-plane/src/routes/environments.ts +++ b/packages/control-plane/src/routes/environments.ts @@ -30,6 +30,7 @@ import { error, parseJsonBody, resolveRepoOrError, + requirePermission, } from "./shared"; import type { Env } from "../types"; @@ -263,13 +264,38 @@ async function handleDeleteEnvironment( } export const environmentRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ - { method: "GET", pattern: parsePattern("/environments"), handler: handleListEnvironments }, - { method: "POST", pattern: parsePattern("/environments"), handler: handleCreateEnvironment }, - { method: "GET", pattern: parsePattern("/environments/:id"), handler: handleGetEnvironment }, - { method: "PUT", pattern: parsePattern("/environments/:id"), handler: handleUpdateEnvironment }, + { + method: "GET", + pattern: parsePattern("/environments"), + authorization: requirePermission("environments.read", { + actorlessGrants: [{ service: "slack-bot" }, { service: "linear-bot" }], + }), + handler: handleListEnvironments, + }, + { + method: "POST", + pattern: parsePattern("/environments"), + authorization: requirePermission("environments.manage"), + handler: handleCreateEnvironment, + }, + { + method: "GET", + pattern: parsePattern("/environments/:id"), + authorization: requirePermission("environments.read", { + actorlessGrants: [{ service: "github-bot" }], + }), + handler: handleGetEnvironment, + }, + { + method: "PUT", + pattern: parsePattern("/environments/:id"), + authorization: requirePermission("environments.manage"), + handler: handleUpdateEnvironment, + }, { method: "DELETE", pattern: parsePattern("/environments/:id"), + authorization: requirePermission("environments.manage"), handler: handleDeleteEnvironment, }, ]); diff --git a/packages/control-plane/src/routes/image-builds.ts b/packages/control-plane/src/routes/image-builds.ts index 6610925c73..d199bf7a1d 100644 --- a/packages/control-plane/src/routes/image-builds.ts +++ b/packages/control-plane/src/routes/image-builds.ts @@ -49,6 +49,8 @@ import { json, parseJsonBody, parsePattern, + NO_AUTHORIZATION, + requirePermission, } from "./shared"; const logger = createLogger("router:image-builds"); @@ -494,41 +496,49 @@ export const imageBuildRoutes: Route[] = [ defineRoute(SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, { method: "POST", pattern: parsePattern("/image-builds/build-complete"), + authorization: NO_AUTHORIZATION, handler: handleBuildComplete, }), defineRoute(SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, { method: "POST", pattern: parsePattern("/image-builds/build-failed"), + authorization: NO_AUTHORIZATION, handler: handleBuildFailed, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "POST", pattern: parsePattern("/image-builds/trigger/environment/:id"), + authorization: requirePermission("environments.images.manage"), handler: handleTriggerEnvironmentBuild, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "POST", pattern: parsePattern("/image-builds/trigger/repo/:owner/:name"), + authorization: requirePermission("repositories.images.manage"), handler: handleTriggerRepoBuild, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "PUT", pattern: parsePattern("/image-builds/toggle/repo/:owner/:name"), + authorization: requirePermission("repositories.images.manage"), handler: handleToggleRepoImageBuilds, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "GET", pattern: parsePattern("/image-builds/status"), + authorization: requirePermission("image_builds.read"), handler: handleGetStatus, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "GET", pattern: parsePattern("/image-builds/enabled"), + authorization: requirePermission("image_builds.read"), handler: handleGetEnabledUnits, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "GET", pattern: parsePattern("/image-builds/enabled-repos"), + authorization: requirePermission("image_builds.read"), handler: handleGetEnabledRepos, }), ]; diff --git a/packages/control-plane/src/routes/integration-settings.ts b/packages/control-plane/src/routes/integration-settings.ts index 1d1ca75485..bc2e81e89c 100644 --- a/packages/control-plane/src/routes/integration-settings.ts +++ b/packages/control-plane/src/routes/integration-settings.ts @@ -34,6 +34,7 @@ import { error, parseJsonBody, extractRepoParams, + requirePermission, } from "./shared"; const logger = createLogger("router:integration-settings"); @@ -492,37 +493,46 @@ export const integrationSettingsRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SE { method: "GET", pattern: parsePattern("/integration-settings/:id"), + authorization: requirePermission("integrations.read", { + actorlessGrants: [{ service: "slack-bot", pathParams: { id: "slack" } }], + }), handler: handleGetIntegrationSettings, }, { method: "PUT", pattern: parsePattern("/integration-settings/:id"), + authorization: requirePermission("integrations.manage"), handler: handleSetIntegrationSettings, }, { method: "DELETE", pattern: parsePattern("/integration-settings/:id"), + authorization: requirePermission("integrations.manage"), handler: handleDeleteIntegrationSettings, }, // Integration settings — per-repo { method: "GET", pattern: parsePattern("/integration-settings/:id/repos"), + authorization: requirePermission("integrations.read"), handler: handleListRepoSettings, }, { method: "GET", pattern: parsePattern("/integration-settings/:id/repos/:owner/:name"), + authorization: requirePermission("integrations.read"), handler: handleGetRepoSettings, }, { method: "PUT", pattern: parsePattern("/integration-settings/:id/repos/:owner/:name"), + authorization: requirePermission("repositories.settings.manage"), handler: handleSetRepoSettings, }, { method: "DELETE", pattern: parsePattern("/integration-settings/:id/repos/:owner/:name"), + authorization: requirePermission("repositories.settings.manage"), handler: handleDeleteRepoSettings, }, // Integration settings — per-environment (design §13.5; sandbox and @@ -530,22 +540,31 @@ export const integrationSettingsRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SE { method: "GET", pattern: parsePattern("/integration-settings/:id/environments/:environmentId"), + authorization: requirePermission("integrations.read"), handler: handleGetEnvironmentSettings, }, { method: "PUT", pattern: parsePattern("/integration-settings/:id/environments/:environmentId"), + authorization: requirePermission("environments.settings.manage"), handler: handleSetEnvironmentSettings, }, { method: "DELETE", pattern: parsePattern("/integration-settings/:id/environments/:environmentId"), + authorization: requirePermission("environments.settings.manage"), handler: handleDeleteEnvironmentSettings, }, // Resolved config — used by bots at runtime { method: "GET", pattern: parsePattern("/integration-settings/:id/resolved/:owner/:name"), + authorization: requirePermission("integrations.read", { + actorlessGrants: [ + { service: "github-bot", pathParams: { id: "github" } }, + { service: "linear-bot", pathParams: { id: "linear" } }, + ], + }), handler: handleGetResolvedConfig, }, ]); diff --git a/packages/control-plane/src/routes/keyboard-shortcuts.ts b/packages/control-plane/src/routes/keyboard-shortcuts.ts index 14ab950531..9bf4a22b6f 100644 --- a/packages/control-plane/src/routes/keyboard-shortcuts.ts +++ b/packages/control-plane/src/routes/keyboard-shortcuts.ts @@ -2,30 +2,23 @@ import { updateKeyboardShortcutPreferencesSchema } from "@open-inspect/shared/ty import { KeyboardShortcutPreferencesStore } from "../db/keyboard-shortcut-preferences"; import type { Env } from "../types"; import { + ACTIVE_SELF, defineRoutes, error, json, parsePattern, - SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, - type RequestContext, + SCM_AGNOSTIC_HUMAN_USER_ROUTE, type Route, + type UserRouteContext, } from "./shared"; -function canonicalUserId(ctx: RequestContext): string | null { - if (ctx.principal?.kind === "user") return ctx.principal.userId; - if (ctx.principal?.kind === "service") return ctx.principal.actor?.canonicalUserId ?? null; - return null; -} - async function getPreferences( _request: Request, _env: Env, _match: RegExpMatchArray, - ctx: RequestContext + ctx: UserRouteContext ): Promise { - const userId = canonicalUserId(ctx); - if (!userId) return error("Canonical user required", 403); - const shortcuts = await new KeyboardShortcutPreferencesStore(ctx.db).get(userId); + const shortcuts = await new KeyboardShortcutPreferencesStore(ctx.db).get(ctx.principal.userId); return json({ shortcuts }); } @@ -33,10 +26,8 @@ async function updatePreferences( request: Request, _env: Env, _match: RegExpMatchArray, - ctx: RequestContext + ctx: UserRouteContext ): Promise { - const userId = canonicalUserId(ctx); - if (!userId) return error("Canonical user required", 403); let body: unknown; try { body = await request.json(); @@ -46,13 +37,23 @@ async function updatePreferences( const parsed = updateKeyboardShortcutPreferencesSchema.safeParse(body); if (!parsed.success) return error("Invalid keyboard shortcuts", 400); const shortcuts = await new KeyboardShortcutPreferencesStore(ctx.db).set( - userId, + ctx.principal.userId, parsed.data.shortcuts ); return json({ shortcuts }); } -export const keyboardShortcutRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, [ - { method: "GET", pattern: parsePattern("/keyboard-shortcuts"), handler: getPreferences }, - { method: "PUT", pattern: parsePattern("/keyboard-shortcuts"), handler: updatePreferences }, +export const keyboardShortcutRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_HUMAN_USER_ROUTE, [ + { + method: "GET", + pattern: parsePattern("/keyboard-shortcuts"), + authorization: ACTIVE_SELF, + handler: getPreferences, + }, + { + method: "PUT", + pattern: parsePattern("/keyboard-shortcuts"), + authorization: ACTIVE_SELF, + handler: updatePreferences, + }, ]); diff --git a/packages/control-plane/src/routes/mcp-servers.ts b/packages/control-plane/src/routes/mcp-servers.ts index 3b6a2ec9a4..311ac3ab53 100644 --- a/packages/control-plane/src/routes/mcp-servers.ts +++ b/packages/control-plane/src/routes/mcp-servers.ts @@ -19,6 +19,7 @@ import { json, error, parseJsonBody, + requirePermission, } from "./shared"; const logger = createLogger("router:mcp-servers"); @@ -167,26 +168,31 @@ export const mcpServerRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUT { method: "GET", pattern: parsePattern("/mcp-servers"), + authorization: requirePermission("mcp_servers.read"), handler: handleListMcpServers, }, { method: "POST", pattern: parsePattern("/mcp-servers"), + authorization: requirePermission("mcp_servers.manage"), handler: handleCreateMcpServer, }, { method: "GET", pattern: parsePattern("/mcp-servers/:id"), + authorization: requirePermission("mcp_servers.read"), handler: handleGetMcpServer, }, { method: "PUT", pattern: parsePattern("/mcp-servers/:id"), + authorization: requirePermission("mcp_servers.manage"), handler: handleUpdateMcpServer, }, { method: "DELETE", pattern: parsePattern("/mcp-servers/:id"), + authorization: requirePermission("mcp_servers.manage"), handler: handleDeleteMcpServer, }, ]); diff --git a/packages/control-plane/src/routes/model-preferences.ts b/packages/control-plane/src/routes/model-preferences.ts index 3268b8c8ec..cfcfe38fa9 100644 --- a/packages/control-plane/src/routes/model-preferences.ts +++ b/packages/control-plane/src/routes/model-preferences.ts @@ -15,6 +15,8 @@ import { json, error, parseJsonBody, + activeGlobal, + requirePermission, } from "./shared"; const logger = createLogger("router:model-preferences"); @@ -109,11 +111,15 @@ export const modelPreferencesRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVI { method: "GET", pattern: parsePattern("/model-preferences"), + authorization: activeGlobal({ + actorlessGrants: [{ service: "slack-bot" }], + }), handler: handleGetModelPreferences, }, { method: "PUT", pattern: parsePattern("/model-preferences"), + authorization: requirePermission("models.preferences.manage"), handler: handleSetModelPreferences, }, ]); diff --git a/packages/control-plane/src/routes/model-provider-accounts.ts b/packages/control-plane/src/routes/model-provider-accounts.ts index 5a1725ee84..f854c2344e 100644 --- a/packages/control-plane/src/routes/model-provider-accounts.ts +++ b/packages/control-plane/src/routes/model-provider-accounts.ts @@ -56,6 +56,8 @@ import { type Route, type SandboxRouteContext, type UserRouteContext, + NO_AUTHORIZATION, + requirePermission, } from "./shared"; const PRIVATE_NO_STORE = "private, no-store" as const; @@ -182,6 +184,9 @@ function managementRoute( method, pattern: parsePattern(path), cacheControl: PRIVATE_NO_STORE, + authorization: requirePermission( + method === "GET" ? "provider_accounts.read" : "provider_accounts.manage" + ), handler, }); } @@ -481,6 +486,7 @@ export const modelProviderAccountRoutes: Route[] = [ method: "POST", pattern: parsePattern("/sessions/:id/provider-auth/:provider/access-token"), cacheControl: NO_STORE, + authorization: NO_AUTHORIZATION, handler: handleProviderAccess, }), ]; diff --git a/packages/control-plane/src/routes/rbac.ts b/packages/control-plane/src/routes/rbac.ts new file mode 100644 index 0000000000..33a2fcb3ea --- /dev/null +++ b/packages/control-plane/src/routes/rbac.ts @@ -0,0 +1,115 @@ +import { AuthorizationError, AuthorizationService } from "../authorization/service"; +import type { Env } from "../types"; +import type { Route } from "./shared"; +import { + AUTHENTICATED_USER, + SCM_AGNOSTIC_HUMAN_USER_ROUTE, + defineRoutes, + error, + json, + requirePermission, + type UserRouteContext, +} from "./shared"; + +function rbacErrorResponse(cause: unknown): Response { + if (cause instanceof AuthorizationError) { + return json( + { + error: "Forbidden", + code: cause.code, + ...(cause.permission ? { permission: cause.permission } : {}), + }, + cause.status + ); + } + return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); +} + +async function handleGetCurrentAuthorization( + _request: Request, + _env: Env, + _match: RegExpMatchArray, + ctx: UserRouteContext +): Promise { + const service = new AuthorizationService(ctx.db); + try { + return json(await service.getEffectiveAuthorization(ctx.principal.userId)); + } catch (cause) { + return rbacErrorResponse(cause); + } +} + +async function handleListRoles( + _request: Request, + _env: Env, + _match: RegExpMatchArray, + ctx: UserRouteContext +): Promise { + const service = new AuthorizationService(ctx.db); + try { + return json(await service.listRoles()); + } catch (cause) { + return rbacErrorResponse(cause); + } +} + +async function handleGetRole( + _request: Request, + _env: Env, + match: RegExpMatchArray, + ctx: UserRouteContext +): Promise { + const service = new AuthorizationService(ctx.db); + try { + const role = await service.getRole(decodeURIComponent(match.groups!.id)); + return role ? json(role) : error("Role not found", 404); + } catch (cause) { + if (cause instanceof URIError) return error("Invalid role ID", 400); + return rbacErrorResponse(cause); + } +} + +async function handleListMembers( + _request: Request, + _env: Env, + _match: RegExpMatchArray, + ctx: UserRouteContext +): Promise { + const service = new AuthorizationService(ctx.db); + try { + return json(await service.listMembers()); + } catch (cause) { + return rbacErrorResponse(cause); + } +} + +export const rbacRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_HUMAN_USER_ROUTE, [ + { + method: "GET", + pattern: /^\/me\/authorization$/, + authorization: AUTHENTICATED_USER, + cacheControl: "private, no-store", + handler: handleGetCurrentAuthorization, + }, + { + method: "GET", + pattern: /^\/roles$/, + authorization: requirePermission("workspace.roles.read"), + cacheControl: "private, no-store", + handler: handleListRoles, + }, + { + method: "GET", + pattern: /^\/roles\/(?[^/]+)$/, + authorization: requirePermission("workspace.roles.read"), + cacheControl: "private, no-store", + handler: handleGetRole, + }, + { + method: "GET", + pattern: /^\/members$/, + authorization: requirePermission("workspace.members.read"), + cacheControl: "private, no-store", + handler: handleListMembers, + }, +]); diff --git a/packages/control-plane/src/routes/repos.ts b/packages/control-plane/src/routes/repos.ts index b5fc4ce606..b936ad41e2 100644 --- a/packages/control-plane/src/routes/repos.ts +++ b/packages/control-plane/src/routes/repos.ts @@ -24,6 +24,7 @@ import { error, extractRepoParams, createRouteSourceControlProvider, + requirePermission, } from "./shared"; const logger = createLogger("router:repos"); @@ -329,21 +330,29 @@ export const reposRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, [ { method: "GET", pattern: parsePattern("/repos"), + authorization: requirePermission("repositories.read", { + actorlessGrants: [{ service: "slack-bot" }, { service: "linear-bot" }], + }), handler: handleListRepos, }, { method: "PUT", pattern: parsePattern("/repos/:owner/:name/metadata"), + authorization: requirePermission("repositories.settings.manage"), handler: handleUpdateRepoMetadata, }, { method: "GET", pattern: parsePattern("/repos/:owner/:name/metadata"), + authorization: requirePermission("repositories.read", { + actorlessGrants: [{ service: "github-bot" }], + }), handler: handleGetRepoMetadata, }, { method: "GET", pattern: parsePattern("/repos/:owner/:name/branches"), + authorization: requirePermission("repositories.read"), handler: handleListBranches, }, ]); diff --git a/packages/control-plane/src/routes/scm-settings.ts b/packages/control-plane/src/routes/scm-settings.ts index 98e6fd2cf2..df5c3476e2 100644 --- a/packages/control-plane/src/routes/scm-settings.ts +++ b/packages/control-plane/src/routes/scm-settings.ts @@ -25,6 +25,7 @@ import { error, parseJsonBody, extractRepoParams, + requirePermission, } from "./shared"; const logger = createLogger("router:scm-settings"); @@ -222,18 +223,40 @@ async function handleDeleteRepoSettings( } export const scmSettingsRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, [ - { method: "GET", pattern: parsePattern("/scm-settings"), handler: handleGetGlobal }, - { method: "PUT", pattern: parsePattern("/scm-settings"), handler: handleSetGlobal }, - { method: "DELETE", pattern: parsePattern("/scm-settings"), handler: handleDeleteGlobal }, - { method: "GET", pattern: parsePattern("/scm-settings/repos"), handler: handleListRepoSettings }, + { + method: "GET", + pattern: parsePattern("/scm-settings"), + authorization: requirePermission("integrations.read"), + handler: handleGetGlobal, + }, + { + method: "PUT", + pattern: parsePattern("/scm-settings"), + authorization: requirePermission("scm_settings.manage"), + handler: handleSetGlobal, + }, + { + method: "DELETE", + pattern: parsePattern("/scm-settings"), + authorization: requirePermission("scm_settings.manage"), + handler: handleDeleteGlobal, + }, + { + method: "GET", + pattern: parsePattern("/scm-settings/repos"), + authorization: requirePermission("integrations.read"), + handler: handleListRepoSettings, + }, { method: "PUT", pattern: parsePattern("/scm-settings/repos/:owner/:name"), + authorization: requirePermission("scm_settings.manage"), handler: handleSetRepoSettings, }, { method: "DELETE", pattern: parsePattern("/scm-settings/repos/:owner/:name"), + authorization: requirePermission("scm_settings.manage"), handler: handleDeleteRepoSettings, }, ]); diff --git a/packages/control-plane/src/routes/secrets.ts b/packages/control-plane/src/routes/secrets.ts index 25f1fc3ff3..b727a9659e 100644 --- a/packages/control-plane/src/routes/secrets.ts +++ b/packages/control-plane/src/routes/secrets.ts @@ -18,6 +18,7 @@ import { parseJsonBody, extractRepoParams, resolveRepoOrError, + requirePermission, } from "./shared"; import { secretsRequestBodySchema } from "./secret-request-schemas"; @@ -380,31 +381,37 @@ export const secretsRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ROUTE, { method: "PUT", pattern: parsePattern("/repos/:owner/:name/secrets"), + authorization: requirePermission("repositories.secrets.manage"), handler: handleSetRepoSecrets, }, { method: "GET", pattern: parsePattern("/repos/:owner/:name/secrets"), + authorization: requirePermission("repositories.secrets.manage"), handler: handleListRepoSecrets, }, { method: "DELETE", pattern: parsePattern("/repos/:owner/:name/secrets/:key"), + authorization: requirePermission("repositories.secrets.manage"), handler: handleDeleteRepoSecret, }, { method: "PUT", pattern: parsePattern("/secrets"), + authorization: requirePermission("global_secrets.manage"), handler: handleSetGlobalSecrets, }, { method: "GET", pattern: parsePattern("/secrets"), + authorization: requirePermission("global_secrets.manage"), handler: handleListGlobalSecrets, }, { method: "DELETE", pattern: parsePattern("/secrets/:key"), + authorization: requirePermission("global_secrets.manage"), handler: handleDeleteGlobalSecret, }, ]); diff --git a/packages/control-plane/src/routes/session-attachments.ts b/packages/control-plane/src/routes/session-attachments.ts index 519cbacc36..52f1b7e3d1 100644 --- a/packages/control-plane/src/routes/session-attachments.ts +++ b/packages/control-plane/src/routes/session-attachments.ts @@ -51,6 +51,7 @@ import { GITHUB_USER_OR_SERVICE_ROUTE, json, parsePattern, + requirePermission, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -243,6 +244,7 @@ export const sessionAttachmentRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/attachments"), + authorization: requirePermission("sessions.collaborate"), handler: handleAttachmentPost, }) ), @@ -251,6 +253,7 @@ export const sessionAttachmentRoutes: Route[] = [ sessionRoute({ method: "GET", pattern: parsePattern("/sessions/:id/attachments/:attachmentId"), + authorization: requirePermission("sessions.read"), handler: handleAttachmentGet, }) ), diff --git a/packages/control-plane/src/routes/session-child-spawn.ts b/packages/control-plane/src/routes/session-child-spawn.ts index 9d077e9af8..1258edc527 100644 --- a/packages/control-plane/src/routes/session-child-spawn.ts +++ b/packages/control-plane/src/routes/session-child-spawn.ts @@ -33,10 +33,13 @@ import { GITHUB_SANDBOX_FALLBACK_ROUTE, json, parsePattern, + permissionRequirement, + requireAll, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; import { DEFAULT_BASE_BRANCH } from "../repos/default-branch"; +import { authorizeSessionTarget } from "./session-target-authorization"; const logger = createLogger("router:session-child-spawn"); const MAX_SPAWN_DEPTH = 2; @@ -143,6 +146,12 @@ async function handleSpawnChild( } } + const targetAuthorizationError = authorizeSessionTarget(ctx, { + environmentId: parentEnvironmentId, + hasRepository: Boolean(parentRepoOwner && parentRepoName), + }); + if (targetAuthorizationError) return targetAuthorizationError; + let enabledModels: ValidModel[]; try { enabledModels = await getEffectiveEnabledModels(ctx.db); @@ -346,6 +355,10 @@ export const sessionChildSpawnRoutes: Route[] = defineRoutes(GITHUB_SANDBOX_FALL sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/children"), + authorization: requireAll( + permissionRequirement("sessions.create"), + permissionRequirement("sessions.collaborate") + ), handler: handleSpawnChild, }), ]); diff --git a/packages/control-plane/src/routes/session-children.ts b/packages/control-plane/src/routes/session-children.ts index ee1aa2f228..1cfcf250b5 100644 --- a/packages/control-plane/src/routes/session-children.ts +++ b/packages/control-plane/src/routes/session-children.ts @@ -16,7 +16,9 @@ import { error, GITHUB_SANDBOX_FALLBACK_ROUTE, json, + NO_AUTHORIZATION, parsePattern, + requirePermission, SCM_AGNOSTIC_SANDBOX_ROUTE, type RequestContext, type Route, @@ -263,6 +265,7 @@ export const sessionChildRoutes: Route[] = [ defineRoute(GITHUB_SANDBOX_FALLBACK_ROUTE, { method: "GET", pattern: parsePattern("/sessions/:id/children"), + authorization: requirePermission("sessions.read"), handler: handleListChildren, }), defineRoute( @@ -270,6 +273,7 @@ export const sessionChildRoutes: Route[] = [ sessionRoute({ method: "GET", pattern: parsePattern("/sessions/:id/children/:childId"), + authorization: requirePermission("sessions.read"), handler: handleGetChild, }) ), @@ -278,6 +282,7 @@ export const sessionChildRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/children/:childId/cancel"), + authorization: requirePermission("sessions.lifecycle"), handler: handleCancelChild, }) ), @@ -286,6 +291,7 @@ export const sessionChildRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/children/:childId/prompt"), + authorization: NO_AUTHORIZATION, handler: handlePromptChild, }) ), diff --git a/packages/control-plane/src/routes/session-create.ts b/packages/control-plane/src/routes/session-create.ts index 7c76ab63dc..550f885d04 100644 --- a/packages/control-plane/src/routes/session-create.ts +++ b/packages/control-plane/src/routes/session-create.ts @@ -17,6 +17,7 @@ import { resolveManagedSkills, SkillResolutionError } from "../session/skill-res import type { Env } from "../types"; import { resolveSessionProviderAuth } from "../session/provider-account-resolution"; import { ProviderAccountSelectionPolicyError } from "../model-provider-accounts/selection-policy"; +import { authorizeSessionTarget } from "./session-target-authorization"; import { normalizeOptionalRepositoryPair, RepositoryPairValidationError, @@ -30,6 +31,7 @@ import { type Route, GITHUB_USER_OR_SERVICE_ROUTE, defineRoutes, + requirePermission, } from "./shared"; const logger = createLogger("router:session-create"); @@ -65,6 +67,12 @@ async function handleCreateSession( throw e; } + const targetAuthorizationError = authorizeSessionTarget(ctx, { + environmentId: body.environmentId, + hasRepository: Boolean(repositoryContext || body.repositories), + }); + if (targetAuthorizationError) return targetAuthorizationError; + // Validate branch names if provided (defense in depth) if (body.branch && !BRANCH_NAME_PATTERN.test(body.branch)) { return error("Invalid branch name"); @@ -266,6 +274,7 @@ export const sessionCreateRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ { method: "POST", pattern: parsePattern("/sessions"), + authorization: requirePermission("sessions.create"), handler: handleCreateSession, }, ]); diff --git a/packages/control-plane/src/routes/session-diffs.ts b/packages/control-plane/src/routes/session-diffs.ts index 9c07780f56..3a1fa4603a 100644 --- a/packages/control-plane/src/routes/session-diffs.ts +++ b/packages/control-plane/src/routes/session-diffs.ts @@ -11,6 +11,7 @@ import { error, SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, + requirePermission, parsePattern, type Route, } from "./shared"; @@ -193,6 +194,7 @@ export const sessionDiffRoutes: Route[] = [ sessionRoute({ method: "GET", pattern: parsePattern("/sessions/:id/diff"), + authorization: requirePermission("sessions.read"), handler: handleDiffState, }) ), @@ -201,6 +203,7 @@ export const sessionDiffRoutes: Route[] = [ sessionRoute({ method: "PUT", pattern: parsePattern("/sessions/:id/diff"), + authorization: requirePermission("sessions.collaborate"), handler: handleDiffUpload, }) ), @@ -209,6 +212,7 @@ export const sessionDiffRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/diff/failure"), + authorization: requirePermission("sessions.collaborate"), handler: handleDiffFailure, }) ), @@ -217,6 +221,7 @@ export const sessionDiffRoutes: Route[] = [ sessionRoute({ method: "GET", pattern: parsePattern("/sessions/:id/diff/:revisionId/files/:fileId"), + authorization: requirePermission("sessions.read"), handler: handleDiffFile, }) ), @@ -225,6 +230,7 @@ export const sessionDiffRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/diff/retry"), + authorization: requirePermission("sessions.lifecycle"), handler: handleDiffRetry, }) ), diff --git a/packages/control-plane/src/routes/session-index.test.ts b/packages/control-plane/src/routes/session-index.test.ts index c8c23da545..5922306b00 100644 --- a/packages/control-plane/src/routes/session-index.test.ts +++ b/packages/control-plane/src/routes/session-index.test.ts @@ -9,7 +9,6 @@ import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; const mockSessionIndexStore = { list: vi.fn(), delete: vi.fn(), - getVisibleForUser: vi.fn(), updateReadState: vi.fn(), }; @@ -20,10 +19,21 @@ vi.mock("../db/session-index", () => ({ })); function createCtx(principal?: Principal): RequestContext { + const statement = { + bind: vi.fn(() => statement), + first: vi.fn(async () => ({ + user_id: "user-1", + suspended_at: null, + role_id: "role_builtin_owner", + role_key: "owner", + role_name: "Owner", + })), + all: vi.fn(async () => ({ results: [] })), + }; return { trace_id: "trace-1", request_id: "req-1", - db: {} as SqlDatabase, + db: { prepare: vi.fn(() => statement) } as unknown as SqlDatabase, executionCtx: TEST_BACKGROUND_TASK_CONTEXT, metrics: { d1Queries: [], @@ -32,6 +42,16 @@ function createCtx(principal?: Principal): RequestContext { summarize: () => ({}), }, principal, + ...(principal?.kind === "user" + ? { + authorization: { + userId: principal.userId, + suspendedAt: null, + role: { id: "role_builtin_owner", key: "owner" as const, name: "Owner" }, + permissions: ["sessions.read", "sessions.delete", "sessions.lifecycle"] as const, + }, + } + : {}), }; } @@ -84,7 +104,6 @@ describe("session index routes", () => { sessions: [], hasMore: false, }); - mockSessionIndexStore.getVisibleForUser.mockResolvedValue({ id: "session-1" }); mockSessionIndexStore.updateReadState.mockResolvedValue({ sessionId: "session-1", outcome: "marked_read", @@ -293,18 +312,6 @@ describe("session index routes", () => { expect(mockSessionIndexStore.updateReadState).not.toHaveBeenCalled(); }); - it("does not expose invisible sessions through read-state mutations", async () => { - mockSessionIndexStore.getVisibleForUser.mockResolvedValue(null); - - const response = await patchReadState(JSON.stringify({ action: "mark_latest_message_read" }), { - kind: "user", - userId: "user-1", - }); - - expect(response.status).toBe(404); - expect(mockSessionIndexStore.updateReadState).not.toHaveBeenCalled(); - }); - it.each([ [ JSON.stringify({ action: "mark_latest_message_read" }), @@ -325,7 +332,6 @@ describe("session index routes", () => { expect(response.status).toBe(200); expect(response.headers.get("Cache-Control")).toBe("private, no-store"); - expect(mockSessionIndexStore.getVisibleForUser).toHaveBeenCalledWith("session-1", "user-1"); expect(mockSessionIndexStore.updateReadState).toHaveBeenCalledWith( "user-1", "session-1", diff --git a/packages/control-plane/src/routes/session-index.ts b/packages/control-plane/src/routes/session-index.ts index 651e3ed21f..e12522ce87 100644 --- a/packages/control-plane/src/routes/session-index.ts +++ b/packages/control-plane/src/routes/session-index.ts @@ -19,6 +19,7 @@ import { parseJsonBody, parsePattern, SCM_AGNOSTIC_HUMAN_USER_ROUTE, + requirePermission, type RequestContext, type Route, type UserRouteContext, @@ -32,18 +33,13 @@ const SESSION_INBOX_LIMIT = 20; function parseCreatedByFilters( values: readonly string[], - principal: RequestContext["principal"] + currentUserId: string | null ): string[] | Response { const userIds: string[] = []; const seen = new Set(); for (const value of values) { - const userId = - value === SESSION_LIST_CURRENT_USER - ? principal?.kind === "user" - ? principal.userId - : null - : value; + const userId = value === SESSION_LIST_CURRENT_USER ? currentUserId : value; if (!isCanonicalUserId(userId)) { return error("Invalid createdBy", 400); @@ -70,7 +66,13 @@ async function handleListSessions( const { createdBy, status, excludeStatus, excludeAutomationLineage, limit, offset } = parsedQuery.data; - const createdByUserIds = parseCreatedByFilters(createdBy, ctx.principal); + const viewerUserId = + ctx.principal?.kind === "user" + ? ctx.principal.userId + : ctx.principal?.kind === "service" + ? (ctx.principal.actor?.canonicalUserId ?? ctx.authorization?.userId) + : undefined; + const createdByUserIds = parseCreatedByFilters(createdBy, viewerUserId ?? null); if (createdByUserIds instanceof Response) { return createdByUserIds; @@ -78,7 +80,6 @@ async function handleListSessions( const store = new SessionIndexStore(ctx.db); const listStartedAt = Date.now(); - const viewerUserId = ctx.principal?.kind === "user" ? ctx.principal.userId : undefined; const result = await store.list({ status, excludeStatus, @@ -86,7 +87,7 @@ async function handleListSessions( createdByUserIds, limit, offset, - viewerUserId, + ...(viewerUserId ? { viewerUserId } : {}), }); if (viewerUserId) { log.info("session_read_state.decorated", { @@ -203,9 +204,6 @@ async function handlePatchReadState( const body = parsedBody.data; const store = new SessionIndexStore(ctx.db); - const visibleSession = await store.getVisibleForUser(sessionId, ctx.principal.userId); - if (!visibleSession) return error("Session not found", 404); - const result = await store.updateReadState(ctx.principal.userId, sessionId, body); if (!result) return error("Session not found", 404); @@ -242,21 +240,25 @@ export const sessionIndexRoutes: Route[] = [ defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "GET", pattern: parsePattern("/sessions"), + authorization: requirePermission("sessions.read"), handler: handleListSessions, }), defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { method: "GET", pattern: parsePattern("/sessions/inbox"), + authorization: requirePermission("sessions.read", { service: "deny" }), handler: handleListSessionInbox, }), defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { method: "PATCH", pattern: parsePattern("/sessions/:id/read-state"), + authorization: requirePermission("sessions.read"), handler: handlePatchReadState, }), defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { method: "DELETE", pattern: parsePattern("/sessions/:id"), + authorization: requirePermission("sessions.delete"), handler: handleDeleteSession, }), ]; diff --git a/packages/control-plane/src/routes/session-media-stream.ts b/packages/control-plane/src/routes/session-media-stream.ts index 0664e4140d..6204d75b58 100644 --- a/packages/control-plane/src/routes/session-media-stream.ts +++ b/packages/control-plane/src/routes/session-media-stream.ts @@ -15,6 +15,7 @@ import { error, GITHUB_USER_OR_SERVICE_ROUTE, parsePattern, + requirePermission, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -145,6 +146,9 @@ export const sessionMediaStreamRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SER sessionRoute({ method: "GET", pattern: parsePattern("/sessions/:id/media/:artifactId"), + authorization: requirePermission("sessions.read", { + actorlessGrants: [{ service: "slack-bot" }], + }), handler: handleMediaGet, }), ]); diff --git a/packages/control-plane/src/routes/session-media-upload.ts b/packages/control-plane/src/routes/session-media-upload.ts index 44b16acd64..77164f10fd 100644 --- a/packages/control-plane/src/routes/session-media-upload.ts +++ b/packages/control-plane/src/routes/session-media-upload.ts @@ -27,6 +27,7 @@ import { GITHUB_SANDBOX_FALLBACK_ROUTE, json, parsePattern, + requirePermission, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -250,6 +251,7 @@ export const sessionMediaUploadRoutes: Route[] = defineRoutes(GITHUB_SANDBOX_FAL sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/media"), + authorization: requirePermission("sessions.collaborate"), handler: handleMediaUpload, }), ]); diff --git a/packages/control-plane/src/routes/session-prompt.ts b/packages/control-plane/src/routes/session-prompt.ts index 39dd03a730..5916dfeea0 100644 --- a/packages/control-plane/src/routes/session-prompt.ts +++ b/packages/control-plane/src/routes/session-prompt.ts @@ -26,6 +26,7 @@ import { error, GITHUB_USER_OR_SERVICE_ROUTE, parsePattern, + requirePermission, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -181,6 +182,7 @@ export const sessionPromptRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE_ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/prompt"), + authorization: requirePermission("sessions.collaborate"), handler: handleSessionPrompt, }), ]); diff --git a/packages/control-plane/src/routes/session-pull-requests.ts b/packages/control-plane/src/routes/session-pull-requests.ts index df6a8e4d52..dd56af8c3b 100644 --- a/packages/control-plane/src/routes/session-pull-requests.ts +++ b/packages/control-plane/src/routes/session-pull-requests.ts @@ -5,6 +5,7 @@ import { error, GITHUB_USER_OR_SERVICE_ROUTE, parsePattern, + requirePermission, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -33,6 +34,7 @@ export const sessionPullRequestRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SER sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/pull-requests/refresh"), + authorization: requirePermission("sessions.lifecycle"), handler: handleRefreshPullRequests, }), ]); diff --git a/packages/control-plane/src/routes/session-runtime-proxy.ts b/packages/control-plane/src/routes/session-runtime-proxy.ts index 8ccfe5c869..01144fb8b8 100644 --- a/packages/control-plane/src/routes/session-runtime-proxy.ts +++ b/packages/control-plane/src/routes/session-runtime-proxy.ts @@ -15,8 +15,10 @@ import { error, GITHUB_SANDBOX_FALLBACK_ROUTE, GITHUB_USER_OR_SERVICE_ROUTE, + NO_AUTHORIZATION, parseJsonBody, parsePattern, + requirePermission, SCM_AGNOSTIC_SANDBOX_FALLBACK_ROUTE, SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, SCM_AGNOSTIC_SANDBOX_ROUTE, @@ -24,6 +26,7 @@ import { SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, SCM_CREDENTIALS_ROUTE, type Route, + type RouteAuthorization, type RoutePolicy, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -44,6 +47,7 @@ type SimpleProxyRouteConfig = { method: string; routePath: string; internalPath: SessionInternalPath; + authorization: RouteAuthorization; runtimeMethod?: string; forwardSearch?: boolean; notFoundMessage?: string; @@ -64,6 +68,7 @@ function simpleProxyRoute(config: SimpleProxyRouteConfig): Route { sessionRoute({ method: config.method, pattern: parsePattern(config.routePath), + authorization: config.authorization, handler: async (request, _env, match, ctx) => { const sessionId = getSessionId(match); if (sessionId instanceof Response) return sessionId; @@ -95,6 +100,7 @@ function legacyTokenRefreshRoute( sessionRoute({ method: "POST", pattern: parsePattern(routePath), + authorization: NO_AUTHORIZATION, handler: async (_request, _env, match, ctx) => { const sessionId = getSessionId(match); if (sessionId instanceof Response) return sessionId; @@ -252,12 +258,7 @@ async function handleCreatePR( }); } -/** - * Read a lifecycle-route body (title/archive/unarchive) under identity - * enforcement. Lifecycle routes accept bodyless requests — a parse failure - * just yields no fields. The DO participant check runs against the verified - * identity, never a caller-asserted one. - */ +/** Read a lifecycle body under verified identity enforcement. */ async function readEnforcedLifecycleBody( request: Request, ctx: SessionRouteContext @@ -286,6 +287,7 @@ function lifecycleProxyRoute( sessionRoute({ method, pattern: parsePattern(routePath), + authorization: requirePermission("sessions.lifecycle"), handler: async (request, _env, match, ctx) => { const sessionId = getSessionId(match); if (sessionId instanceof Response) return sessionId; @@ -311,12 +313,14 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "GET", routePath: "/sessions/:id/sandbox-access", internalPath: SessionInternalPaths.sandboxAccess, + authorization: requirePermission("sessions.sandbox_access"), }), simpleProxyRoute({ policy: SCM_AGNOSTIC_HUMAN_USER_ROUTE, method: "GET", routePath: "/sessions/:id", internalPath: SessionInternalPaths.snapshot, + authorization: requirePermission("sessions.read"), notFoundMessage: "Session not found", }), simpleProxyRoute({ @@ -324,6 +328,9 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "POST", routePath: "/sessions/:id/stop", internalPath: SessionInternalPaths.stop, + authorization: requirePermission("sessions.lifecycle", { + actorlessGrants: [{ service: "linear-bot" }], + }), runtimeMethod: "POST", }), defineRoute( @@ -331,6 +338,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/sandbox-error"), + authorization: NO_AUTHORIZATION, handler: handleSandboxError, }) ), @@ -339,6 +347,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "GET", routePath: "/sessions/:id/events", internalPath: SessionInternalPaths.events, + authorization: requirePermission("sessions.read"), forwardSearch: true, }), simpleProxyRoute({ @@ -346,18 +355,21 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "GET", routePath: "/sessions/:id/artifacts", internalPath: SessionInternalPaths.artifacts, + authorization: requirePermission("sessions.read"), }), simpleProxyRoute({ policy: GITHUB_USER_OR_SERVICE_ROUTE, method: "GET", routePath: "/sessions/:id/participants", internalPath: SessionInternalPaths.participants, + authorization: requirePermission("sessions.read"), }), defineRoute( SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, sessionRoute({ method: "GET", pattern: parsePattern("/sessions/:id/participant-profiles"), + authorization: requirePermission("sessions.read"), handler: handleParticipantProfiles, }) ), @@ -366,6 +378,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/participants"), + authorization: requirePermission("sessions.collaborate"), handler: handleAddParticipant, }) ), @@ -374,6 +387,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "GET", routePath: "/sessions/:id/messages", internalPath: SessionInternalPaths.messages, + authorization: requirePermission("sessions.read"), forwardSearch: true, }), defineRoute( @@ -381,6 +395,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/pr"), + authorization: requirePermission("sessions.collaborate"), handler: handleCreatePR, }) ), @@ -399,6 +414,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "POST", routePath: "/sessions/:id/scm-credentials", internalPath: SessionInternalPaths.scmCredentials, + authorization: NO_AUTHORIZATION, runtimeMethod: "POST", }), simpleProxyRoute({ @@ -406,6 +422,7 @@ export const sessionRuntimeProxyRoutes: Route[] = [ method: "GET", routePath: "/sessions/:id/tunnel-urls", internalPath: SessionInternalPaths.tunnelUrls, + authorization: requirePermission("sessions.sandbox_access"), runtimeMethod: "GET", }), lifecycleProxyRoute("PATCH", "/sessions/:id/title", SessionInternalPaths.updateTitle), diff --git a/packages/control-plane/src/routes/session-skills.ts b/packages/control-plane/src/routes/session-skills.ts index 37497ff4e7..c36cfb989e 100644 --- a/packages/control-plane/src/routes/session-skills.ts +++ b/packages/control-plane/src/routes/session-skills.ts @@ -8,6 +8,8 @@ import { error, json, parsePattern, + NO_AUTHORIZATION, + requirePermission, SCM_AGNOSTIC_SANDBOX_ROUTE, SCM_AGNOSTIC_HUMAN_USER_ROUTE, type SandboxRouteContext, @@ -27,9 +29,6 @@ async function handleSessionSkillsView( ): Promise { const id = sessionId(match); if (id instanceof Response) return id; - if (!(await new SessionIndexStore(ctx.db).getVisibleForUser(id, ctx.principal.userId))) { - return error("Session not found", 404); - } const view = await new SessionSkillStore(ctx.db).getSessionSkillsView(id); if (!view) return error("Session skill manifest not found", 404); const response = json(view); @@ -96,11 +95,13 @@ export const sessionSkillRoutes: Route[] = [ defineRoute(SCM_AGNOSTIC_HUMAN_USER_ROUTE, { method: "GET", pattern: parsePattern("/sessions/:id/skills"), + authorization: requirePermission("sessions.read"), handler: handleSessionSkillsView, }), defineRoute(SCM_AGNOSTIC_SANDBOX_ROUTE, { method: "GET", pattern: parsePattern("/sessions/:id/sandbox-skills"), + authorization: NO_AUTHORIZATION, handler: handleSandboxInstallation, }), ]; diff --git a/packages/control-plane/src/routes/session-target-authorization.ts b/packages/control-plane/src/routes/session-target-authorization.ts new file mode 100644 index 0000000000..db1923185f --- /dev/null +++ b/packages/control-plane/src/routes/session-target-authorization.ts @@ -0,0 +1,37 @@ +import type { PermissionId } from "@open-inspect/shared/rbac"; +import { serviceAllowsPermission } from "../authorization/service-permissions"; +import { json, type RequestContext } from "./shared"; + +export interface SessionTarget { + environmentId?: string | null; + hasRepository: boolean; +} + +/** Enforce use of the environment or repository inherited by a new session. */ +export function authorizeSessionTarget( + ctx: RequestContext, + target: SessionTarget +): Response | null { + if (ctx.principal?.kind !== "user" && ctx.principal?.kind !== "service") return null; + + const permission: PermissionId | null = target.environmentId + ? "environments.use" + : target.hasRepository + ? "repositories.use" + : null; + if (!permission) return null; + + if ( + ctx.principal.kind === "service" && + !serviceAllowsPermission(ctx.principal.service, permission) + ) { + return json({ error: "Forbidden", code: "service_capability_required" }, 403); + } + if (!ctx.authorization) { + return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); + } + if (!ctx.authorization.permissions.includes(permission)) { + return json({ error: "Forbidden", code: "permission_required", permission }, 403); + } + return null; +} diff --git a/packages/control-plane/src/routes/session-ws-token.test.ts b/packages/control-plane/src/routes/session-ws-token.test.ts index 4699b48ada..608d5f4718 100644 --- a/packages/control-plane/src/routes/session-ws-token.test.ts +++ b/packages/control-plane/src/routes/session-ws-token.test.ts @@ -3,6 +3,7 @@ import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; import { sessionWsTokenRoutes } from "./session-ws-token"; import type { RequestContext, Route } from "./shared"; import type { Env } from "../types"; +import type { SqlDatabase } from "../db/sql-database"; function routeFor(path: string): { route: Route; match: RegExpMatchArray } { const route = sessionWsTokenRoutes.find((candidate) => candidate.pattern.test(path)); @@ -12,13 +13,32 @@ function routeFor(path: string): { route: Route; match: RegExpMatchArray } { return { route, match }; } -function createContext(): RequestContext { +function accessDatabase() { + const run = vi.fn(async () => ({ meta: { changes: 1 } })); + const statement = { + bind: vi.fn(() => statement), + run, + }; + return { + db: { prepare: vi.fn(() => statement) } as unknown as SqlDatabase, + statement, + run, + }; +} + +function createContext(db: SqlDatabase = accessDatabase().db): RequestContext { return { request_id: "request-1", trace_id: "trace-1", - db: {} as never, + db, executionCtx: TEST_BACKGROUND_TASK_CONTEXT, principal: { kind: "user", userId: "user-1" }, + authorization: { + userId: "user-1", + suspendedAt: null, + role: { id: "role-1", key: "member", name: "Member" }, + permissions: ["sessions.collaborate"], + }, metrics: { d1Queries: [], spans: {}, @@ -71,6 +91,25 @@ describe("session ws-token route", () => { }); }); + it("forwards a runtime rejection without writing D1", async () => { + const access = accessDatabase(); + const fetch = vi.fn(async () => Response.json({ error: "rejected" }, { status: 409 })); + const { route, match } = routeFor("/sessions/session-1/ws-token"); + + const response = await route.handler( + new Request("https://test.local/sessions/session-1/ws-token", { + method: "POST", + body: JSON.stringify({}), + }), + createEnv(fetch), + match, + createContext(access.db) + ); + + expect(response.status).toBe(409); + expect(access.db.prepare).not.toHaveBeenCalled(); + }); + it("forwards null SCM display fields accepted by the session contract", async () => { const forwarded: Request[] = []; const fetch = vi.fn(async (request: Request) => { diff --git a/packages/control-plane/src/routes/session-ws-token.ts b/packages/control-plane/src/routes/session-ws-token.ts index 4d684b1b76..d1d23bf592 100644 --- a/packages/control-plane/src/routes/session-ws-token.ts +++ b/packages/control-plane/src/routes/session-ws-token.ts @@ -7,6 +7,7 @@ import { GITHUB_USER_OR_SERVICE_ROUTE, parseJsonBody, parsePattern, + requirePermission, type Route, } from "./shared"; import { sessionRoute, type SessionRouteContext } from "./session-route"; @@ -33,8 +34,10 @@ async function handleSessionWsToken( if (!parsedBody.success) return error("Invalid websocket token body", 400); const body = parsedBody.data; + const authorization = ctx.authorization; + if (!authorization) return error("Authorization unavailable", 503); const userId = enforcement.enforced.participantUserId; - const canonicalUserId = enforcement.enforced.canonicalUserId; + const canonicalUserId = authorization.userId; return ctx.metrics.time("do_fetch", () => ctx.sessionRuntime.fetch(sessionId, SessionInternalPaths.wsToken, { @@ -55,6 +58,7 @@ export const sessionWsTokenRoutes: Route[] = defineRoutes(GITHUB_USER_OR_SERVICE sessionRoute({ method: "POST", pattern: parsePattern("/sessions/:id/ws-token"), + authorization: requirePermission("sessions.collaborate"), handler: handleSessionWsToken, }), ]); diff --git a/packages/control-plane/src/routes/shared.ts b/packages/control-plane/src/routes/shared.ts index cf17736d24..1696f07b60 100644 --- a/packages/control-plane/src/routes/shared.ts +++ b/packages/control-plane/src/routes/shared.ts @@ -11,6 +11,12 @@ import type { Env } from "../types"; import type { Logger } from "../logger"; import type { BackgroundTasks } from "../platform-ports"; import type { BetterAuthRuntime, UserAuthRuntime } from "../auth/user/runtime"; +import type { + EffectiveAuthorization, + PermissionId, + ScopedPermissionStem, +} from "@open-inspect/shared/rbac"; +import type { ServiceName } from "@open-inspect/shared/service-auth"; import { createSourceControlProviderFromEnv, SourceControlProviderError, @@ -19,9 +25,7 @@ import { type SourceControlProviderName, } from "../source-control"; -/** - * Request context with correlation IDs and per-request metrics. - */ +/** Request-scoped dependencies, identity, and resolved authorization state. */ export type RequestContext = CorrelationContext & { metrics: RequestMetrics; /** @@ -44,18 +48,144 @@ export type RequestContext = CorrelationContext & { principal?: Principal; /** Authentication provenance, separate from the principal being authorized. */ authentication?: AuthenticationContext; + /** Effective human authorization loaded once by the router for this request. */ + authorization?: EffectiveAuthorization; }; -/** - * Route configuration. - */ +/** Route matching, authorization, and handler configuration. */ export interface RouteDefinition { method: string; pattern: RegExp; + /** Authorization policy enforced before the handler runs. */ + authorization: RouteAuthorization; cacheControl?: "no-store" | "private, no-store"; handler: (request: Request, env: Env, match: RegExpMatchArray, ctx: Context) => Promise; } +/** One permission or resource-admission requirement for an active user. */ +export type RouteAuthorizationRequirement = + | { kind: "permission"; permission: PermissionId } + | { kind: "scoped-permission"; stem: ScopedPermissionStem } + | { + kind: "automation"; + operation: "manage" | "trigger"; + automationIdParam: string; + }; + +type BotServiceName = Exclude; + +/** Narrow route grant for a trusted service without an acting user. */ +export interface ActorlessServiceGrant { + service: BotServiceName; + pathParams?: Readonly>; +} + +type ServiceAuthorization = + | { kind: "deny" } + | { + kind: "actor"; + actorlessGrants?: readonly ActorlessServiceGrant[]; + }; + +/** Declarative authorization policy enforced by the router. */ +export type RouteAuthorization = + | { kind: "none" } + | { kind: "authenticated" } + | { kind: "active-self" } + | { kind: "active-global"; service: ServiceAuthorization } + | { + kind: "active-user"; + allOf: readonly RouteAuthorizationRequirement[]; + service: ServiceAuthorization; + } + | { + kind: "service"; + services: readonly BotServiceName[]; + actor: "required" | "optional"; + }; + +/** + * Skips router-level permission checks after route authentication. + * + * The route may still require a service signature, a session-bound sandbox token, or credentials + * verified by its handler. Only routes whose authentication policy is `public` are publicly + * accessible. + */ +export const NO_AUTHORIZATION = { kind: "none" } as const satisfies RouteAuthorization; +/** Policy requiring any authenticated principal. */ +export const AUTHENTICATED_USER = { + kind: "authenticated", +} as const satisfies RouteAuthorization; +/** Policy requiring an active user to access their own account resource. */ +export const ACTIVE_SELF = { kind: "active-self" } as const satisfies RouteAuthorization; + +/** Build a global permission requirement for composition with other requirements. */ +export function permissionRequirement(permission: PermissionId): RouteAuthorizationRequirement { + return { kind: "permission", permission }; +} + +/** Require an active user with a global permission, optionally allowing service actors. */ +export function requirePermission( + permission: PermissionId, + options?: { service?: "actor" | "deny"; actorlessGrants?: readonly ActorlessServiceGrant[] } +): RouteAuthorization { + return { + kind: "active-user", + allOf: [permissionRequirement(permission)], + service: + options?.service === "deny" + ? { kind: "deny" } + : { kind: "actor", actorlessGrants: options?.actorlessGrants }, + }; +} + +/** Require an active user with at least one permission under a scoped stem. */ +export function requireScopedPermission( + stem: ScopedPermissionStem, + options?: { service?: "actor" } +): RouteAuthorization { + return { + kind: "active-user", + allOf: [{ kind: "scoped-permission", stem }], + service: options?.service === "actor" ? { kind: "actor" } : { kind: "deny" }, + }; +} + +/** Require admission to manage or trigger the automation identified by a path parameter. */ +export function requireAutomation( + operation: "manage" | "trigger", + automationIdParam = "id" +): RouteAuthorization { + return { + kind: "active-user", + allOf: [{ kind: "automation", operation, automationIdParam }], + service: { kind: "deny" }, + }; +} + +/** Require an active user to satisfy every supplied authorization requirement. */ +export function requireAll(...allOf: readonly RouteAuthorizationRequirement[]): RouteAuthorization { + return { kind: "active-user", allOf, service: { kind: "actor" } }; +} + +/** Require any active user, with optional actorless service grants. */ +export function activeGlobal(options?: { + actorlessGrants?: readonly ActorlessServiceGrant[]; +}): RouteAuthorization { + return { + kind: "active-global", + service: { kind: "actor", actorlessGrants: options?.actorlessGrants }, + }; +} + +/** Restrict a route to one trusted service, with optional actor identity. */ +export function serviceAuthorized( + service: BotServiceName, + actor: "required" | "optional" = "optional" +): RouteAuthorization { + return { kind: "service", services: [service], actor }; +} + type UserPrincipal = Extract; type SandboxPrincipal = Extract; type ServicePrincipal = Extract; @@ -70,6 +200,7 @@ export type RouteAuthentication = | { kind: "public" } | { kind: "handler-authenticated" } | { kind: "web-service" } + | { kind: "service" } | { kind: "user" } | { kind: "user-or-service" } | ({ kind: "sandbox" } & SandboxSessionBinding) @@ -82,11 +213,13 @@ export type RouteContext = RequestCo ? SandboxPrincipal : Authentication extends { kind: "web-service" } ? WebServicePrincipal - : Authentication extends { kind: "user-or-service" } - ? UserOrServicePrincipal - : Authentication extends { kind: "user-or-service-with-sandbox-fallback" } - ? Principal - : Principal | undefined; + : Authentication extends { kind: "service" } + ? ServicePrincipal + : Authentication extends { kind: "user-or-service" } + ? UserOrServicePrincipal + : Authentication extends { kind: "user-or-service-with-sandbox-fallback" } + ? Principal + : Principal | undefined; }; export type UserRouteContext = RouteContext<{ kind: "user" }>; @@ -108,6 +241,11 @@ export const GITHUB_USER_OR_SERVICE_ROUTE = { supportedScmProviders: ["github"], } as const satisfies RoutePolicy; +export const GITHUB_SERVICE_ROUTE = { + authentication: { kind: "service" }, + supportedScmProviders: ["github"], +} as const satisfies RoutePolicy; + export const SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE = { authentication: { kind: "user-or-service" }, supportedScmProviders: "all", diff --git a/packages/control-plane/src/routes/sign-in-providers.ts b/packages/control-plane/src/routes/sign-in-providers.ts index a28828208e..15058e7dd4 100644 --- a/packages/control-plane/src/routes/sign-in-providers.ts +++ b/packages/control-plane/src/routes/sign-in-providers.ts @@ -4,6 +4,7 @@ import { defineRoutes, error, json, + NO_AUTHORIZATION, parsePattern, SCM_AGNOSTIC_WEB_SERVICE_ROUTE, type Route, @@ -39,6 +40,7 @@ export const signInProviderRoutes: Route[] = defineRoutes(SCM_AGNOSTIC_WEB_SERVI { method: "GET", pattern: parsePattern("/internal/auth/sign-in-providers"), + authorization: NO_AUTHORIZATION, handler: handleSignInProviders, }, ]); diff --git a/packages/control-plane/src/routes/skills.ts b/packages/control-plane/src/routes/skills.ts index 942b799feb..84e3c3be3e 100644 --- a/packages/control-plane/src/routes/skills.ts +++ b/packages/control-plane/src/routes/skills.ts @@ -40,6 +40,7 @@ import { SCM_AGNOSTIC_HUMAN_USER_ROUTE, SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, defineRoutes, + requirePermission, } from "./shared"; const log = createLogger("router:skills"); @@ -625,63 +626,103 @@ function profileWriteError(value: unknown): Response { } const skillReadRoutes = defineRoutes(SCM_AGNOSTIC_USER_OR_SERVICE_ROUTE, [ - { method: "GET", pattern: parsePattern("/skills"), handler: handleListSkills }, + { + method: "GET", + pattern: parsePattern("/skills"), + authorization: requirePermission("skills.read"), + handler: handleListSkills, + }, { method: "POST", pattern: parsePattern("/skills/preview"), + authorization: requirePermission("skills.read"), handler: handlePreviewSkill, }, { method: "POST", pattern: parsePattern("/skills/resolve-preview"), + authorization: requirePermission("skills.read"), handler: handleResolvePreview, }, - { method: "GET", pattern: parsePattern("/skills/:id"), handler: handleGetSkill }, + { + method: "GET", + pattern: parsePattern("/skills/:id"), + authorization: requirePermission("skills.read"), + handler: handleGetSkill, + }, ]); const skillAdministrationRoutes = defineRoutes(SCM_AGNOSTIC_HUMAN_USER_ROUTE, [ - { method: "POST", pattern: parsePattern("/skills"), handler: handleCreateSkill }, + { + method: "POST", + pattern: parsePattern("/skills"), + authorization: requirePermission("skills.manage"), + handler: handleCreateSkill, + }, { method: "POST", pattern: parsePattern("/skills/import/preview"), + authorization: requirePermission("skills.manage"), handler: handlePreviewSkillImport, }, - { method: "POST", pattern: parsePattern("/skills/import"), handler: handleImportSkill }, + { + method: "POST", + pattern: parsePattern("/skills/import"), + authorization: requirePermission("skills.manage"), + handler: handleImportSkill, + }, { method: "POST", pattern: parsePattern("/skills/:id/reimport/preview"), + authorization: requirePermission("skills.manage"), handler: handlePreviewSkillReimport, }, { method: "POST", pattern: parsePattern("/skills/:id/reimport"), + authorization: requirePermission("skills.manage"), handler: handleReimportSkill, }, { method: "PATCH", pattern: parsePattern("/skills/:id"), + authorization: requirePermission("skills.manage"), handler: handleSetSkillEnabled, }, { method: "PUT", pattern: parsePattern("/skills/:id"), + authorization: requirePermission("skills.manage"), handler: handleReplaceSkillContentAndAssignments, }, - { method: "DELETE", pattern: parsePattern("/skills/:id"), handler: handleDeleteSkill }, - { method: "GET", pattern: parsePattern("/skill-profiles"), handler: handleListProfiles }, + { + method: "DELETE", + pattern: parsePattern("/skills/:id"), + authorization: requirePermission("skills.manage"), + handler: handleDeleteSkill, + }, + { + method: "GET", + pattern: parsePattern("/skill-profiles"), + authorization: requirePermission("skill_profiles.manage_own"), + handler: handleListProfiles, + }, { method: "POST", pattern: parsePattern("/skill-profiles"), + authorization: requirePermission("skill_profiles.manage_own"), handler: handleCreateProfile, }, { method: "PATCH", pattern: parsePattern("/skill-profiles/:id"), + authorization: requirePermission("skill_profiles.manage_own"), handler: handleUpdateProfile, }, { method: "DELETE", pattern: parsePattern("/skill-profiles/:id"), + authorization: requirePermission("skill_profiles.manage_own"), handler: handleDeleteProfile, }, ]); diff --git a/packages/control-plane/src/webhooks/automation-event.ts b/packages/control-plane/src/webhooks/automation-event.ts index 6d93e46da2..558361b579 100644 --- a/packages/control-plane/src/webhooks/automation-event.ts +++ b/packages/control-plane/src/webhooks/automation-event.ts @@ -14,15 +14,15 @@ import { type AutomationEvent, type AutomationEventSource, } from "@open-inspect/shared/triggers"; -import { requireEventPoster } from "../auth/identity-enforcement"; import { createLogger } from "../logger"; import type { Route, RequestContext } from "../routes/shared"; import { defineRoute, error, - GITHUB_USER_OR_SERVICE_ROUTE, + GITHUB_SERVICE_ROUTE, json, parsePattern, + serviceAuthorized, } from "../routes/shared"; import type { Env } from "../types"; import { Scheduler } from "../scheduler/scheduler"; @@ -124,6 +124,7 @@ export async function forwardAutomationEventToScheduler( return json({ ok: true, ...result }); } +/** Create an authenticated route for a normalized automation event source. */ export function createAutomationEventRoute(opts: { path: string; source: AutomationEventSource; @@ -134,9 +135,6 @@ export function createAutomationEventRoute(opts: { _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const authFailure = requireEventPoster(ctx, opts.source); - if (authFailure) return authFailure; - let body: unknown; try { body = await request.json(); @@ -154,9 +152,10 @@ export function createAutomationEventRoute(opts: { return forwardAutomationEventToScheduler(env, validated.event, ctx); } - return defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { + return defineRoute(GITHUB_SERVICE_ROUTE, { method: "POST", pattern: parsePattern(opts.path), + authorization: serviceAuthorized("slack-bot"), handler, }); } diff --git a/packages/control-plane/src/webhooks/automation-webhook.ts b/packages/control-plane/src/webhooks/automation-webhook.ts index b715b69a8d..ebcd1ca6f3 100644 --- a/packages/control-plane/src/webhooks/automation-webhook.ts +++ b/packages/control-plane/src/webhooks/automation-webhook.ts @@ -10,6 +10,7 @@ import { defineRoute, error, json, + NO_AUTHORIZATION, parsePattern, SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, } from "../routes/shared"; @@ -90,5 +91,6 @@ async function handleAutomationWebhook( export const automationWebhookRoute: Route = defineRoute(SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, { method: "POST", pattern: parsePattern("/webhooks/automation/:id"), + authorization: NO_AUTHORIZATION, handler: handleAutomationWebhook, }); diff --git a/packages/control-plane/src/webhooks/github.ts b/packages/control-plane/src/webhooks/github.ts index d2e2c7eef4..96987b721d 100644 --- a/packages/control-plane/src/webhooks/github.ts +++ b/packages/control-plane/src/webhooks/github.ts @@ -14,8 +14,13 @@ import { SessionInternalPaths } from "../session/contracts"; import { createSessionRuntimeClient } from "../session/runtime-client"; import type { Env } from "../types"; import type { RequestContext, Route } from "../routes/shared"; -import { defineRoute, error, GITHUB_USER_OR_SERVICE_ROUTE, parsePattern } from "../routes/shared"; -import { requireEventPoster } from "../auth/identity-enforcement"; +import { + defineRoute, + error, + GITHUB_SERVICE_ROUTE, + parsePattern, + serviceAuthorized, +} from "../routes/shared"; import { forwardAutomationEventToScheduler, logAutomationEventRejection, @@ -100,9 +105,6 @@ async function handleGitHubAutomationEvent( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const authFailure = requireEventPoster(ctx, "github"); - if (authFailure) return authFailure; - let body: unknown; try { body = await request.json(); @@ -124,8 +126,9 @@ async function handleGitHubAutomationEvent( return forwardAutomationEventToScheduler(env, validated.event, ctx); } -export const githubAutomationEventRoute: Route = defineRoute(GITHUB_USER_OR_SERVICE_ROUTE, { +export const githubAutomationEventRoute: Route = defineRoute(GITHUB_SERVICE_ROUTE, { method: "POST", pattern: parsePattern("/internal/github-event"), + authorization: serviceAuthorized("github-bot"), handler: handleGitHubAutomationEvent, }); diff --git a/packages/control-plane/src/webhooks/sentry.ts b/packages/control-plane/src/webhooks/sentry.ts index ebd410341f..484ff1a8d6 100644 --- a/packages/control-plane/src/webhooks/sentry.ts +++ b/packages/control-plane/src/webhooks/sentry.ts @@ -12,6 +12,7 @@ import { defineRoute, error, json, + NO_AUTHORIZATION, parsePattern, SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, } from "../routes/shared"; @@ -122,5 +123,6 @@ async function handleSentryWebhook( export const sentryWebhookRoute: Route = defineRoute(SCM_AGNOSTIC_HANDLER_AUTHENTICATED_ROUTE, { method: "POST", pattern: parsePattern("/webhooks/sentry/:id"), + authorization: NO_AUTHORIZATION, handler: handleSentryWebhook, }); diff --git a/packages/control-plane/test/integration/automations-slack-route.test.ts b/packages/control-plane/test/integration/automations-slack-route.test.ts index 091c4e27fb..88dd08330b 100644 --- a/packages/control-plane/test/integration/automations-slack-route.test.ts +++ b/packages/control-plane/test/integration/automations-slack-route.test.ts @@ -44,13 +44,9 @@ function createBody(overrides: Record) { } async function postAutomation(body: Record): Promise { - // automation-create requires a participant identity: sign as a bot with an - // asserted actor (the userless web service credential is rejected, 403). return serviceFetch("https://test.local/automations", { method: "POST", body: JSON.stringify(body), - service: "slack-bot", - actor: "slack:U0123", }); } diff --git a/packages/control-plane/test/integration/helpers.ts b/packages/control-plane/test/integration/helpers.ts index 32ae6965d1..510c758cbc 100644 --- a/packages/control-plane/test/integration/helpers.ts +++ b/packages/control-plane/test/integration/helpers.ts @@ -2,6 +2,7 @@ import { SELF, env } from "cloudflare:test"; import { runInSessionDO } from "./session-do-access"; import type { SandboxSettings } from "@open-inspect/shared/types/integrations"; import { buildServiceAuthHeaders, type ServiceName } from "@open-inspect/shared/service-auth"; +import { BUILT_IN_ROLE_REGISTRY, type BuiltInRoleKey } from "@open-inspect/shared/rbac"; import type { SandboxStatus } from "@open-inspect/shared/types/sessions"; import type { SessionDO } from "../../src/session/durable-object"; import { hashToken } from "../../src/auth/crypto"; @@ -28,11 +29,22 @@ export function getSetCookies(headers: Headers): string[] { return (headers as Headers & { getSetCookie(): string[] }).getSetCookie(); } +export async function seedActiveUser(userId: string): Promise { + const now = Date.now(); + await env.DB.prepare( + `INSERT INTO users (id, display_name, created_at, updated_at) VALUES (?, ?, ?, ?)` + ) + .bind(userId, "Integration User", now, now) + .run(); +} + const DEFAULT_WAIT_FOR_SANDBOX_STATUS_TIMEOUT_MS = 3000; export const INTEGRATION_WEBSOCKET_TIMEOUT_MS = 2000; const TEST_BROWSER_USER_ID = "11111111111111111111111111111111"; const TEST_BROWSER_ACCOUNT_ID = "test-browser-account"; const TEST_BROWSER_PROVIDER_SUBJECT = "583231"; +type InitialUserRole = Exclude; +const DEFAULT_INITIAL_USER_ROLE = "owner" as const; const TEST_BROWSER_SESSION_ID = "test-browser-session"; const TEST_BROWSER_SESSION_TOKEN = "test-browser-session-token"; const TEST_BROWSER_SESSION_COOKIE = "__Secure-openinspect.session_token"; @@ -69,13 +81,16 @@ async function signCookieValue(value: string, secret: string): Promise { * web request must carry the same compound credential as production. Direct * service-auth tests intentionally build their own bare sig1 requests. */ -async function testBrowserSessionCookie(): Promise { +async function testBrowserSessionCookie(initialRole: InitialUserRole): Promise { const secret = env.BROWSER_AUTH_SECRET; if (!secret) throw new Error("BROWSER_AUTH_SECRET is not configured for integration tests"); const now = new Date(); const expiresAt = new Date(now.getTime() + 7 * 24 * 60 * 60 * 1000); const applicationTimestamp = now.getTime(); + const existingUser = await env.DB.prepare("SELECT 1 FROM users WHERE id = ?") + .bind(TEST_BROWSER_USER_ID) + .first(); await env.DB.batch([ env.DB.prepare( `INSERT OR IGNORE INTO users @@ -86,7 +101,7 @@ async function testBrowserSessionCookie(): Promise { "Integration Browser User", "browser@test.local", 1, - null, + "browser@test.local", applicationTimestamp, applicationTimestamp ), @@ -121,6 +136,11 @@ async function testBrowserSessionCookie(): Promise { TEST_BROWSER_USER_ID ), ]); + if (initialRole !== "member" && !existingUser) { + await env.DB.prepare(`UPDATE user_role_assignments SET role_id = ? WHERE user_id = ?`) + .bind(BUILT_IN_ROLE_REGISTRY[initialRole].id, TEST_BROWSER_USER_ID) + .run(); + } const signedToken = await signCookieValue(TEST_BROWSER_SESSION_TOKEN, secret); return `${TEST_BROWSER_SESSION_COOKIE}=${signedToken}`; @@ -140,6 +160,7 @@ export async function serviceFetch( headers?: Record; service?: ServiceName; actor?: string; + initialUserRole?: InitialUserRole; } ): Promise { const method = init?.method ?? "GET"; @@ -152,7 +173,10 @@ export async function serviceFetch( body: init?.body, actor: init?.actor, }); - const browserCookie = service === "web" ? await testBrowserSessionCookie() : undefined; + const browserCookie = + service === "web" + ? await testBrowserSessionCookie(init?.initialUserRole ?? DEFAULT_INITIAL_USER_ROLE) + : undefined; return SELF.fetch(url, { method, headers: { diff --git a/packages/control-plane/test/integration/image-builds.test.ts b/packages/control-plane/test/integration/image-builds.test.ts index 85480b084c..ef7d35314e 100644 --- a/packages/control-plane/test/integration/image-builds.test.ts +++ b/packages/control-plane/test/integration/image-builds.test.ts @@ -8,7 +8,6 @@ * deployment, and the SCM-less harness split is the same as PR-4/PR-8. */ -import { buildServiceAuthHeaders } from "@open-inspect/shared/service-auth"; import { describe, it, expect, beforeEach } from "vitest"; import { SELF, env } from "cloudflare:test"; import { ImageBuildStore } from "../../src/db/image-builds"; @@ -27,6 +26,7 @@ import type { DeleteImageInput, ImageBuildAdapter } from "../../src/image-builds import { evaluateImageBuildForSpawn } from "../../src/sandbox/lifecycle/image-selection"; import type { Env } from "../../src/types"; import { cleanD1Tables } from "./cleanup"; +import { serviceFetch } from "./helpers"; import { RUNTIME_VERSION, REPOSITORY_SHAS, @@ -62,22 +62,6 @@ const WIRE_KEYS = [ // only forwards token-shaped bearers to the workflow). const MODAL_BUILD_TOKEN = "ab".repeat(32); -/** Call an internal route with a registered service credential. */ -async function serviceFetch(url: string, init?: { method?: string; body?: string }) { - const method = init?.method ?? "GET"; - const headers = { - ...(await buildServiceAuthHeaders({ - service: "linear-bot", - secret: "test-service-secret-linear-bot", - method, - url, - body: init?.body, - })), - ...(init?.body === undefined ? {} : { "Content-Type": "application/json" }), - }; - return SELF.fetch(url, { method, headers, body: init?.body }); -} - function tokenHeaders(token: string): Record { return { Authorization: `Bearer ${token}`, "Content-Type": "application/json" }; } diff --git a/packages/control-plane/test/integration/service-auth.test.ts b/packages/control-plane/test/integration/service-auth.test.ts index 4a7680eadb..c3fd07e26a 100644 --- a/packages/control-plane/test/integration/service-auth.test.ts +++ b/packages/control-plane/test/integration/service-auth.test.ts @@ -11,6 +11,7 @@ import { generateInternalToken } from "@open-inspect/shared/auth"; import { GlobalSecretsStore } from "../../src/db/global-secrets"; import { UserStore } from "../../src/db/user-store"; import { cleanD1Tables } from "./cleanup"; +import { insertCanonicalUser } from "./identity-seed-helpers"; const SERVICE_SECRET: Record = { web: "test-service-secret-web", @@ -46,7 +47,7 @@ async function signedFetch(p: { describe("sig1 service-credential authentication", () => { beforeEach(cleanD1Tables); - it("accepts a signed GET from every non-web service", async () => { + it("rejects actorless service requests on broad routes", async () => { for (const service of Object.keys(SERVICE_SECRET).filter( (candidate): candidate is Exclude => candidate !== "web" )) { @@ -55,12 +56,81 @@ describe("sig1 service-credential authentication", () => { method: "GET", url: "https://test.local/sessions", }); - expect(response.status, service).toBe(200); - const body = await response.json<{ sessions: unknown[] }>(); - expect(body.sessions).toEqual([]); + expect(response.status, service).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_actor_required" }); } }); + it.each([ + ["slack-bot", "/repos", 200], + ["linear-bot", "/repos", 200], + ["github-bot", "/repos/acme/widgets/metadata", 200], + ["slack-bot", "/environments", 200], + ["linear-bot", "/environments", 200], + ["github-bot", "/environments/missing", 404], + ["slack-bot", "/integration-settings/slack", 200], + ["slack-bot", "/integration-settings/slack/watched-channels", 200], + ["slack-bot", "/model-preferences", 200], + ] as const)( + "allows actorless %s metadata/config read %s", + async (service, path, expectedStatus) => { + if (path === "/repos") { + await env.REPOS_CACHE.put( + "repos:list:v2", + JSON.stringify({ + repos: [], + cachedAt: new Date().toISOString(), + freshUntil: Date.now() + 60_000, + }) + ); + } + const response = await signedFetch({ + service, + method: "GET", + url: `https://test.local${path}`, + }); + expect(response.status).toBe(expectedStatus); + } + ); + + it("denies an actorless service without the route's exact grant", async () => { + const response = await signedFetch({ + service: "linear-bot", + method: "GET", + url: "https://test.local/integration-settings/slack", + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_actor_required" }); + }); + + it("denies actorless resolved settings for the wrong integration", async () => { + const response = await signedFetch({ + service: "github-bot", + method: "GET", + url: "https://test.local/integration-settings/linear/resolved/acme/widgets", + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_actor_required" }); + }); + + it.each([ + ["github-bot", "github"], + ["linear-bot", "linear"], + ] as const)( + "authorizes actorless %s only for matching resolved settings", + async (service, id) => { + const response = await signedFetch({ + service, + method: "GET", + url: `https://test.local/integration-settings/${id}/resolved/missing/repository`, + }); + + expect(response.status).not.toBe(403); + } + ); + it("requires a browser session in addition to the web service channel", async () => { const response = await signedFetch({ service: "web", @@ -78,6 +148,7 @@ describe("sig1 service-credential authentication", () => { secret: SERVICE_SECRET["linear-bot"], method: "GET", url: signedUrl, + actor: "linear:query-order", }); const response = await SELF.fetch( `https://test.local/sessions?createdBy=${createdBy}&limit=5`, @@ -88,20 +159,20 @@ describe("sig1 service-credential authentication", () => { expect(response.status).toBe(200); }); - it("delivers the signed body intact to the handler (D1 write lands)", async () => { + it("does not let an actorless service mutate global secrets", async () => { const response = await signedFetch({ service: "linear-bot", method: "PUT", url: "https://test.local/secrets", body: JSON.stringify({ secrets: { SIGNED_BODY_TEST: "intact" } }), }); - expect(response.status).toBe(200); + expect(response.status).toBe(403); const secrets = await new GlobalSecretsStore( env.DB, env.REPO_SECRETS_ENCRYPTION_KEY! ).getDecryptedSecrets(); - expect(secrets.SIGNED_BODY_TEST).toBe("intact"); + expect(secrets.SIGNED_BODY_TEST).toBeUndefined(); }); it("rejects a body tampered after signing", async () => { @@ -113,13 +184,14 @@ describe("sig1 service-credential authentication", () => { method: "PUT", url, body: intactBody, + actor: "linear:tamper-test", }); const intact = await SELF.fetch(url, { method: "PUT", headers: { "Content-Type": "application/json", ...headers }, body: intactBody, }); - expect(intact.status).toBe(200); + expect(intact.status).toBe(403); const tampered = await SELF.fetch(url, { method: "PUT", @@ -190,7 +262,7 @@ describe("sig1 service-credential authentication", () => { expect(response.status).toBe(401); }); - it("persists bot session ownership from the signed actor", async () => { + it("persists bot creator attribution and permits cross-actor collaboration", async () => { const created = await signedFetch({ service: "slack-bot", method: "POST", @@ -202,6 +274,7 @@ describe("sig1 service-credential authentication", () => { }), }); expect(created.status).toBe(201); + const createdBody = await created.json<{ sessionId: string }>(); const identity = await new UserStore(env.DB).getIdentity("slack", "U0001"); expect(identity).not.toBeNull(); @@ -221,6 +294,180 @@ describe("sig1 service-credential authentication", () => { spawnSource: "slack-bot", }) ); + + const collaboratorList = await signedFetch({ + service: "slack-bot", + method: "GET", + url: "https://test.local/sessions", + actor: "slack:U0002", + }); + expect(collaboratorList.status).toBe(200); + await expect(collaboratorList.json()).resolves.toMatchObject({ + sessions: [expect.objectContaining({ title: "Slack-owned session" })], + }); + + const collaborator = await signedFetch({ + service: "slack-bot", + method: "POST", + url: `https://test.local/sessions/${createdBody.sessionId}/prompt`, + actor: "slack:U0002", + body: JSON.stringify({ content: "Cross-session prompt" }), + }); + expect(collaborator.status).toBe(200); + + const deniedByServiceCeiling = await signedFetch({ + service: "slack-bot", + method: "DELETE", + url: `https://test.local/sessions/${createdBody.sessionId}`, + actor: "slack:U0002", + }); + expect(deniedByServiceCeiling.status).toBe(403); + await expect(deniedByServiceCeiling.json()).resolves.toMatchObject({ + code: "service_capability_required", + }); + }); + + it("allows only narrow actorless session callbacks", async () => { + const created = await signedFetch({ + service: "linear-bot", + method: "POST", + url: "https://test.local/sessions", + actor: "linear:U-CREATOR", + body: JSON.stringify({ + title: "Linear callback session", + model: "anthropic/claude-haiku-4-5", + }), + }); + expect(created.status).toBe(201); + const { sessionId } = await created.json<{ sessionId: string }>(); + + const linearStop = await signedFetch({ + service: "linear-bot", + method: "POST", + url: `https://test.local/sessions/${sessionId}/stop`, + }); + expect(linearStop.status).not.toBe(403); + + const slackMedia = await signedFetch({ + service: "slack-bot", + method: "GET", + url: `https://test.local/sessions/${sessionId}/media/missing-artifact`, + }); + expect(slackMedia.status).not.toBe(403); + + const wrongService = await signedFetch({ + service: "github-bot", + method: "POST", + url: `https://test.local/sessions/${sessionId}/stop`, + }); + expect(wrongService.status).toBe(403); + await expect(wrongService.json()).resolves.toMatchObject({ code: "service_actor_required" }); + }); + + it("denies suspended canonical bot actors and actorless broad requests", async () => { + await signedFetch({ + service: "slack-bot", + method: "POST", + url: "https://test.local/sessions", + actor: "slack:U-SUSPENDED", + body: JSON.stringify({ title: "Actor session", model: "anthropic/claude-haiku-4-5" }), + }); + const identity = await new UserStore(env.DB).getIdentity("slack", "U-SUSPENDED"); + await env.DB.prepare("UPDATE users SET suspended_at = 1 WHERE id = ?") + .bind(identity!.userId) + .run(); + + const attributed = await signedFetch({ + service: "slack-bot", + method: "GET", + url: "https://test.local/sessions", + actor: "slack:U-SUSPENDED", + }); + const actorless = await signedFetch({ + service: "slack-bot", + method: "GET", + url: "https://test.local/sessions", + }); + + expect(attributed.status).toBe(403); + await expect(attributed.json()).resolves.toMatchObject({ code: "active_user_required" }); + expect(actorless.status).toBe(403); + await expect(actorless.json()).resolves.toMatchObject({ code: "service_actor_required" }); + }); + + it("intersects an actor role with the service ceiling", async () => { + await signedFetch({ + service: "slack-bot", + method: "GET", + url: "https://test.local/sessions", + actor: "slack:U-VIEWER", + }); + const identity = await new UserStore(env.DB).getIdentity("slack", "U-VIEWER"); + await env.DB.prepare("UPDATE user_role_assignments SET role_id = ? WHERE user_id = ?") + .bind("role_builtin_viewer", identity!.userId) + .run(); + + const response = await signedFetch({ + service: "slack-bot", + method: "POST", + url: "https://test.local/sessions", + actor: "slack:U-VIEWER", + body: JSON.stringify({ title: "Viewer session", model: "anthropic/claude-haiku-4-5" }), + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "sessions.create", + }); + }); + + it("does not authorize a first-contact actor as one user and attribute it to a Viewer", async () => { + await insertCanonicalUser({ + id: "existing-viewer", + email: "viewer@example.com", + emailVerified: 1, + displayName: "Existing Viewer", + }); + await env.DB.prepare("UPDATE user_role_assignments SET role_id = ? WHERE user_id = ?") + .bind("role_builtin_viewer", "existing-viewer") + .run(); + + const body = JSON.stringify({ + title: "First-contact actor", + model: "anthropic/claude-haiku-4-5", + actorEmail: "viewer@example.com", + }); + const first = await signedFetch({ + service: "slack-bot", + method: "POST", + url: "https://test.local/sessions", + actor: "slack:U-EMAIL-VIEWER", + body, + }); + + expect(first.status).toBe(409); + await expect(first.json()).resolves.toMatchObject({ code: "actor_identity_changed" }); + const identity = await new UserStore(env.DB).getIdentity("slack", "U-EMAIL-VIEWER"); + expect(identity?.userId).toBe("existing-viewer"); + + const retry = await signedFetch({ + service: "slack-bot", + method: "POST", + url: "https://test.local/sessions", + actor: "slack:U-EMAIL-VIEWER", + body, + }); + expect(retry.status).toBe(403); + await expect(retry.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "sessions.create", + }); + + const sessions = await env.DB.prepare("SELECT COUNT(*) AS count FROM sessions").first<{ + count: number; + }>(); + expect(sessions?.count).toBe(0); }); it("requires a user or signed actor before any service can create a session", async () => { diff --git a/packages/linear-bot/src/webhook-handler.test.ts b/packages/linear-bot/src/webhook-handler.test.ts index 2c792d9059..09850c3eae 100644 --- a/packages/linear-bot/src/webhook-handler.test.ts +++ b/packages/linear-bot/src/webhook-handler.test.ts @@ -733,6 +733,9 @@ describe("handleAgentSessionEvent environment targets", () => { const promptCall = controlPlaneFetch.mock.calls.find(([input]) => String(input).endsWith("/prompt") ); + const eventsCall = controlPlaneFetch.mock.calls.find(([input]) => + String(input).includes("/events?") + ); const body = JSON.parse(String(promptCall?.[1]?.body)) as Record; // Identity travels via the signed actor assertion, never the body. expect(body).not.toHaveProperty("authorId"); @@ -750,6 +753,50 @@ describe("handleAgentSessionEvent environment targets", () => { }, }); expect(body.callbackContext).not.toHaveProperty("transitionIssueOnStart"); + expect(new Headers(eventsCall?.[1]?.headers).get("X-OpenInspect-Actor")).toBe( + "linear:follow-up-human-user" + ); + expect(new Headers(promptCall?.[1]?.headers).get("X-OpenInspect-Actor")).toBe( + "linear:follow-up-human-user" + ); + }); + + it("fails closed instead of signing as the session creator when follow-up author fields are absent", async () => { + const { kv } = createFakeKV({ + "oauth:client-credentials:org-1": validToken(), + "issue:issue-1": JSON.stringify({ + sessionId: "session-xyz", + issueId: "issue-1", + issueIdentifier: "ENG-42", + repoOwner: "acme", + repoName: "backend", + model: "anthropic/claude-haiku-4-5", + createdAt: Date.now(), + }), + }); + const env = makeLinearBotEnv(kv); + const controlPlaneFetch = (env.CONTROL_PLANE as unknown as { fetch: ReturnType }) + .fetch; + controlPlaneFetch.mockImplementation(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/integration-settings/")) return Response.json({ config: null }); + if (url.endsWith("/events?type=token&limit=20")) return Response.json({ events: [] }); + if (url.endsWith("/prompt")) return Response.json({ ok: true }); + throw new Error(`Unexpected control-plane fetch to ${url}`); + }); + const webhook = makeWebhook(); + webhook.action = "prompted"; + webhook.agentSession.creatorId = "session-creator"; + webhook.agentActivity = { + content: { type: "prompt", body: "Please continue." }, + }; + + await handleAgentSessionEvent(webhook, env, "trace-follow-up-creator-fallback"); + + const sessionCalls = controlPlaneFetch.mock.calls.filter(([input]) => + /\/(events\?|prompt$)/.test(String(input)) + ); + expect(sessionCalls).toHaveLength(0); }); it("adds prior token context from a parsed events response", async () => { @@ -818,10 +865,14 @@ describe("handleAgentSessionEvent environment targets", () => { "https://internal/sessions/session-xyz/stop", expect.objectContaining({ method: "POST" }) ); + const stopInit = controlPlaneFetch.mock.calls[0]?.[1] as RequestInit | undefined; + expect(new Headers(stopInit?.headers).get("X-OpenInspect-Actor")).toBe( + "linear:follow-up-human-user" + ); expect(store.has("issue:issue-1")).toBe(false); }); - it("retains the session mapping when stopping the session fails", async () => { + it("fails closed and retains the session mapping when a stop author is missing", async () => { const { kv, store } = createFakeKV({ "issue:issue-1": JSON.stringify({ sessionId: "session-xyz", @@ -834,7 +885,6 @@ describe("handleAgentSessionEvent environment targets", () => { const env = makeLinearBotEnv(kv); const controlPlaneFetch = (env.CONTROL_PLANE as unknown as { fetch: ReturnType }) .fetch; - controlPlaneFetch.mockResolvedValue(new Response(null, { status: 500 })); const webhook = makeWebhook(); webhook.action = "prompted"; webhook.agentActivity = { @@ -844,7 +894,7 @@ describe("handleAgentSessionEvent environment targets", () => { await handleAgentSessionEvent(webhook, env, "trace-stop-failed"); - expect(controlPlaneFetch).toHaveBeenCalledOnce(); + expect(controlPlaneFetch).not.toHaveBeenCalled(); expect(store.has("issue:issue-1")).toBe(true); }); @@ -937,7 +987,7 @@ describe("handleAgentSessionEvent environment targets", () => { const promptCall = controlPlaneFetch.mock.calls.find(([input]) => String(input).endsWith("/prompt") ); - expect(JSON.parse(String(promptCall?.[1]?.body))).not.toHaveProperty("authorId"); + expect(promptCall).toBeUndefined(); }); }); diff --git a/packages/linear-bot/src/webhook-handler.ts b/packages/linear-bot/src/webhook-handler.ts index 4d5a0b80fb..8067343447 100644 --- a/packages/linear-bot/src/webhook-handler.ts +++ b/packages/linear-bot/src/webhook-handler.ts @@ -224,10 +224,22 @@ async function handleStop(webhook: AgentSessionWebhook, env: Env, traceId: strin const existingSession = await lookupIssueSession(env, issueId); if (existingSession) { const stopUrl = `https://internal/sessions/${existingSession.sessionId}/stop`; + const actorUserId = + webhook.agentActivity?.userId ?? webhook.agentSession.comment?.userId ?? undefined; + if (!actorUserId) { + log.warn("Linear stop rejected because its author is missing", { + event: "agent_session.stop_author_missing", + agent_session_id: agentSessionId, + issue_id: issueId, + trace_id: traceId, + }); + return; + } try { const stopRes = await signedControlPlaneFetch(env, { method: "POST", url: stopUrl, + actor: `linear:${actorUserId}`, traceId, }); if (!stopRes.ok) { @@ -314,7 +326,7 @@ function getFollowUp(webhook: AgentSessionWebhook): { return { content: activityBody, source: "linear_agent_activity", - actorUserId: webhook.agentActivity?.userId, + actorUserId: webhook.agentActivity?.userId ?? undefined, }; } @@ -323,11 +335,15 @@ function getFollowUp(webhook: AgentSessionWebhook): { return { content: comment.body, source: "linear_comment", - actorUserId: comment.userId, + actorUserId: comment.userId ?? undefined, }; } - return { content: "Follow-up on the issue.", source: "linear_fallback" }; + return { + content: "Follow-up on the issue.", + source: "linear_fallback", + actorUserId: undefined, + }; } function buildLinearCallbackContext(params: { @@ -389,6 +405,26 @@ async function handleFollowUp( }); if (!client) return; + if (!followUp.actorUserId) { + log.warn("Linear follow-up rejected because its author is missing", { + event: "agent_session.follow_up_author_missing", + agent_session_id: agentSessionId, + issue_id: issue.id, + organization_id: orgId, + trace_id: traceId, + }); + await emitAgentActivity( + client, + agentSessionId, + { + type: "error", + body: "Cannot process this follow-up because Linear did not identify its author.", + }, + true + ); + return; + } + const existingSession = await lookupIssueSession(env, issue.id); if (!existingSession) return; const existingTarget = await resolveStoredSessionTarget(env, existingSession, traceId); @@ -419,6 +455,7 @@ async function handleFollowUp( const eventsRes = await signedControlPlaneFetch(env, { method: "GET", url: eventsUrl, + actor: `linear:${followUp.actorUserId}`, traceId, }); if (eventsRes.ok) { @@ -440,7 +477,7 @@ async function handleFollowUp( issueIdentifier: issue.identifier, followUpContent: followUp.content, followUpSource: followUp.source, - followUpAuthor: followUp.actorUserId ? "linear" : "unknown", + followUpAuthor: "linear", sessionContextSummary, }), source: "linear", @@ -450,7 +487,7 @@ async function handleFollowUp( method: "POST", url: promptUrl, body: promptBody, - actor: followUp.actorUserId ? `linear:${followUp.actorUserId}` : undefined, + actor: `linear:${followUp.actorUserId}`, traceId, }); diff --git a/packages/slack-bot/src/attachments.test.ts b/packages/slack-bot/src/attachments.test.ts index 6ab59fb5bb..207b43df96 100644 --- a/packages/slack-bot/src/attachments.test.ts +++ b/packages/slack-bot/src/attachments.test.ts @@ -56,7 +56,7 @@ function uploadCreatedResponse(attachmentId = "att-1"): Response { /** Download + upload in one step, as the delivery pipeline runs them. */ async function prepareAndUpload(env: Env, sessionId: string, files: SlackMessageFile[]) { const prepared = await prepareImageAttachments(env, toImageAttachments(files)); - return uploadPreparedAttachments(env, sessionId, prepared); + return uploadPreparedAttachments(env, sessionId, prepared, "slack:U1"); } afterEach(() => { @@ -270,7 +270,7 @@ describe("uploadPreparedAttachments", () => { method: "POST", url: uploadUrl, bodySha256Hex: await sha256Hex(uploadInit.body as Uint8Array), - actor: "", + actor: "slack:U1", }); expect(verified).toMatchObject({ ok: true }); }); diff --git a/packages/slack-bot/src/attachments.ts b/packages/slack-bot/src/attachments.ts index 72e82838e6..71c4168fba 100644 --- a/packages/slack-bot/src/attachments.ts +++ b/packages/slack-bot/src/attachments.ts @@ -253,6 +253,7 @@ async function uploadToSession( env: Env, sessionId: string, file: PreparedImageAttachments["files"][number], + authorId: string, traceId?: string ): Promise<{ reference: SessionAttachmentReference } | { sessionMissing: boolean }> { const { attachment, bytes } = file; @@ -275,6 +276,7 @@ async function uploadToSession( method: "POST", url: `https://internal/sessions/${sessionId}/attachments`, body: { bytes: multipartBytes, contentType }, + actor: authorId.startsWith("slack:") ? authorId : undefined, traceId, }, { signal: AbortSignal.timeout(OUTBOUND_REQUEST_TIMEOUT_MS) } @@ -320,10 +322,11 @@ export async function uploadPreparedAttachments( env: Env, sessionId: string, prepared: PreparedImageAttachments, + authorId: string, traceId?: string ): Promise { const outcomes = await Promise.all( - prepared.files.map((file) => uploadToSession(env, sessionId, file, traceId)) + prepared.files.map((file) => uploadToSession(env, sessionId, file, authorId, traceId)) ); const references: SessionAttachmentReference[] = []; const dropped: SlackAttachmentDropReason[] = [...prepared.dropped]; diff --git a/packages/slack-bot/src/sessions/prompt-delivery.ts b/packages/slack-bot/src/sessions/prompt-delivery.ts index edaa5fab14..5086e847b8 100644 --- a/packages/slack-bot/src/sessions/prompt-delivery.ts +++ b/packages/slack-bot/src/sessions/prompt-delivery.ts @@ -60,7 +60,7 @@ export async function deliverPrompt( threadTs, traceId, } = options; - const upload = await uploadPreparedAttachments(env, sessionId, attachments, traceId); + const upload = await uploadPreparedAttachments(env, sessionId, attachments, authorId, traceId); if (imageOnly && upload.references.length === 0) { // The placeholder prompt would launch a meaningless run with nothing