diff --git a/server/src/__tests__/adapter-routes-authz.test.ts b/server/src/__tests__/adapter-routes-authz.test.ts new file mode 100644 index 00000000000..e2b976ccace --- /dev/null +++ b/server/src/__tests__/adapter-routes-authz.test.ts @@ -0,0 +1,255 @@ +import express from "express"; +import request from "supertest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ServerAdapterModule } from "../adapters/index.js"; + +const mocks = vi.hoisted(() => { + const externalRecords = new Map(); + + return { + externalRecords, + execFile: vi.fn((_file: string, _args: string[], optionsOrCallback: unknown, maybeCallback?: unknown) => { + const callback = typeof optionsOrCallback === "function" ? optionsOrCallback : maybeCallback; + if (typeof callback === "function") { + callback(null, "", ""); + } + return { + kill: vi.fn(), + on: vi.fn(), + }; + }), + listAdapterPlugins: vi.fn(), + addAdapterPlugin: vi.fn((record: any) => { + externalRecords.set(record.type, record); + }), + removeAdapterPlugin: vi.fn((type: string) => { + externalRecords.delete(type); + }), + getAdapterPluginByType: vi.fn((type: string) => externalRecords.get(type)), + getAdapterPluginsDir: vi.fn(), + getDisabledAdapterTypes: vi.fn(), + setAdapterDisabled: vi.fn(), + loadExternalAdapterPackage: vi.fn(), + buildExternalAdapters: vi.fn(async () => []), + reloadExternalAdapter: vi.fn(), + getUiParserSource: vi.fn(), + getOrExtractUiParserSource: vi.fn(), + }; +}); + +vi.mock("node:child_process", () => ({ + execFile: mocks.execFile, +})); + +vi.mock("../services/adapter-plugin-store.js", () => ({ + listAdapterPlugins: mocks.listAdapterPlugins, + addAdapterPlugin: mocks.addAdapterPlugin, + removeAdapterPlugin: mocks.removeAdapterPlugin, + getAdapterPluginByType: mocks.getAdapterPluginByType, + getAdapterPluginsDir: mocks.getAdapterPluginsDir, + getDisabledAdapterTypes: mocks.getDisabledAdapterTypes, + setAdapterDisabled: mocks.setAdapterDisabled, +})); + +vi.mock("../adapters/plugin-loader.js", () => ({ + buildExternalAdapters: mocks.buildExternalAdapters, + loadExternalAdapterPackage: mocks.loadExternalAdapterPackage, + getUiParserSource: mocks.getUiParserSource, + getOrExtractUiParserSource: mocks.getOrExtractUiParserSource, + reloadExternalAdapter: mocks.reloadExternalAdapter, +})); + +const EXTERNAL_ADAPTER_TYPE = "external_admin_test"; +const EXTERNAL_PACKAGE_NAME = "paperclip-external-adapter"; +let adapterRoutes: typeof import("../routes/adapters.js").adapterRoutes; +let errorHandler: typeof import("../middleware/index.js").errorHandler; +let registerServerAdapter: typeof import("../adapters/registry.js").registerServerAdapter; +let unregisterServerAdapter: typeof import("../adapters/registry.js").unregisterServerAdapter; +let setOverridePaused: typeof import("../adapters/registry.js").setOverridePaused; + +function createAdapter(type = EXTERNAL_ADAPTER_TYPE): ServerAdapterModule { + return { + type, + models: [], + execute: async () => ({ exitCode: 0, signal: null, timedOut: false }), + testEnvironment: async () => ({ + adapterType: type, + status: "pass", + checks: [], + testedAt: new Date(0).toISOString(), + }), + }; +} + +function installedRecord(type = EXTERNAL_ADAPTER_TYPE) { + return { + packageName: EXTERNAL_PACKAGE_NAME, + type, + installedAt: new Date(0).toISOString(), + }; +} + +function createApp(actor: Express.Request["actor"]) { + if (!adapterRoutes || !errorHandler) { + throw new Error("adapter route test dependencies were not loaded"); + } + + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = actor; + next(); + }); + app.use("/api", adapterRoutes()); + app.use(errorHandler); + return app; +} + +function boardMember(membershipRole: "admin" | "operator" | "viewer"): Express.Request["actor"] { + return { + type: "board", + userId: `${membershipRole}-user`, + userName: null, + userEmail: null, + source: "session", + isInstanceAdmin: false, + companyIds: ["company-1"], + memberships: [ + { + companyId: "company-1", + membershipRole, + status: "active", + }, + ], + }; +} + +const instanceAdmin: Express.Request["actor"] = { + type: "board", + userId: "instance-admin", + userName: null, + userEmail: null, + source: "session", + isInstanceAdmin: true, + companyIds: [], + memberships: [], +}; + +function sendMutatingRequest(app: express.Express, name: string) { + switch (name) { + case "install": + return request(app) + .post("/api/adapters/install") + .send({ packageName: EXTERNAL_PACKAGE_NAME }); + case "disable": + return request(app) + .patch(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}`) + .send({ disabled: true }); + case "override": + return request(app) + .patch("/api/adapters/claude_local/override") + .send({ paused: true }); + case "delete": + return request(app).delete(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}`); + case "reload": + return request(app).post(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}/reload`); + case "reinstall": + return request(app).post(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}/reinstall`); + default: + throw new Error(`Unknown mutating adapter route: ${name}`); + } +} + +function seedInstalledExternalAdapter() { + mocks.externalRecords.set(EXTERNAL_ADAPTER_TYPE, installedRecord()); + unregisterServerAdapter(EXTERNAL_ADAPTER_TYPE); + registerServerAdapter(createAdapter()); +} + +describe("adapter management route authorization", () => { + beforeAll(async () => { + const [routes, middleware, registry] = await Promise.all([ + import("../routes/adapters.js"), + import("../middleware/index.js"), + import("../adapters/registry.js"), + ]); + adapterRoutes = routes.adapterRoutes; + errorHandler = middleware.errorHandler; + registerServerAdapter = registry.registerServerAdapter; + unregisterServerAdapter = registry.unregisterServerAdapter; + setOverridePaused = registry.setOverridePaused; + }, 20_000); + + beforeEach(() => { + vi.clearAllMocks(); + mocks.externalRecords.clear(); + + unregisterServerAdapter(EXTERNAL_ADAPTER_TYPE); + setOverridePaused("claude_local", false); + mocks.listAdapterPlugins.mockImplementation(() => [...mocks.externalRecords.values()]); + mocks.getAdapterPluginsDir.mockReturnValue("/tmp/paperclip-adapter-route-authz-test"); + mocks.getDisabledAdapterTypes.mockReturnValue([]); + mocks.setAdapterDisabled.mockReturnValue(true); + mocks.buildExternalAdapters.mockResolvedValue([]); + mocks.loadExternalAdapterPackage.mockResolvedValue(createAdapter()); + mocks.reloadExternalAdapter.mockImplementation(async (type: string) => createAdapter(type)); + }); + + afterEach(() => { + unregisterServerAdapter(EXTERNAL_ADAPTER_TYPE); + setOverridePaused("claude_local", false); + }); + + it.each([ + "install", + "disable", + "override", + "delete", + "reload", + "reinstall", + ])("rejects %s for a non-instance-admin board user with company membership", async (routeName) => { + seedInstalledExternalAdapter(); + const app = createApp(boardMember("admin")); + + const res = await sendMutatingRequest(app, routeName); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + }); + + it.each([ + ["install", 201], + ["disable", 200], + ["override", 200], + ["delete", 200], + ["reload", 200], + ["reinstall", 200], + ] as const)("allows instance admins to reach %s", async (routeName, expectedStatus) => { + if (routeName !== "install") { + seedInstalledExternalAdapter(); + } + const app = createApp(instanceAdmin); + + const res = await sendMutatingRequest(app, routeName); + + expect(res.status, JSON.stringify(res.body)).toBe(expectedStatus); + }); + + it.each(["viewer", "operator"] as const)( + "does not let a company %s trigger adapter npm install or reload", + async (membershipRole) => { + seedInstalledExternalAdapter(); + const app = createApp(boardMember(membershipRole)); + + const install = await request(app) + .post("/api/adapters/install") + .send({ packageName: EXTERNAL_PACKAGE_NAME }); + const reload = await request(app).post(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}/reload`); + + expect(install.status, JSON.stringify(install.body)).toBe(403); + expect(reload.status, JSON.stringify(reload.body)).toBe(403); + expect(mocks.execFile).not.toHaveBeenCalled(); + expect(mocks.loadExternalAdapterPackage).not.toHaveBeenCalled(); + expect(mocks.reloadExternalAdapter).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/server/src/routes/adapters.ts b/server/src/routes/adapters.ts index a01f3132ba0..40e5800ef99 100644 --- a/server/src/routes/adapters.ts +++ b/server/src/routes/adapters.ts @@ -6,7 +6,9 @@ * - Installing external adapters from npm packages or local paths * - Unregistering external adapters * - * All routes require board-level authentication (assertBoard middleware). + * Read-only routes require board org access. Mutating adapter management + * routes require instance-admin access because they can install, reload, or + * toggle server-side adapter code for the whole Paperclip instance. * * @module server/routes/adapters */ @@ -41,7 +43,7 @@ import type { AdapterPluginRecord } from "../services/adapter-plugin-store.js"; import type { ServerAdapterModule, AdapterConfigSchema } from "../adapters/types.js"; import { loadExternalAdapterPackage, getUiParserSource, getOrExtractUiParserSource, reloadExternalAdapter } from "../adapters/plugin-loader.js"; import { logger } from "../middleware/logger.js"; -import { assertBoardOrgAccess } from "./authz.js"; +import { assertBoardOrgAccess, assertInstanceAdmin } from "./authz.js"; import { BUILTIN_ADAPTER_TYPES } from "../adapters/builtin-adapter-types.js"; const execFileAsync = promisify(execFile); @@ -192,6 +194,9 @@ export function adapterRoutes() { * its model count, and load status. */ router.get("/adapters", async (_req, res) => { + // Adapter inventory is needed by ordinary board members when creating or + // editing company agents. Mutating adapter management routes below remain + // instance-admin only because they affect the whole server runtime. assertBoardOrgAccess(_req); const registeredAdapters = listServerAdapters(); @@ -218,7 +223,7 @@ export function adapterRoutes() { * - version?: string — target version for npm packages */ router.post("/adapters/install", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const { packageName, isLocalPath = false, version } = req.body as AdapterInstallRequest; @@ -350,7 +355,7 @@ export function adapterRoutes() { * Request body: { "disabled": boolean } */ router.patch("/adapters/:type", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const adapterType = req.params.type; const { disabled } = req.body as { disabled?: boolean }; @@ -385,7 +390,7 @@ export function adapterRoutes() { * keep the adapter they started with. */ router.patch("/adapters/:type/override", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const adapterType = req.params.type; const { paused } = req.body as { paused?: boolean }; @@ -413,7 +418,7 @@ export function adapterRoutes() { * Unregister an external adapter. Built-in adapters cannot be removed. */ router.delete("/adapters/:type", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const adapterType = req.params.type; @@ -488,7 +493,7 @@ export function adapterRoutes() { * Cannot be used on built-in adapter types. */ router.post("/adapters/:type/reload", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const type = req.params.type; @@ -540,7 +545,7 @@ export function adapterRoutes() { // This is a convenience shortcut for remove + install with the same // package name, but without the risk of losing the store record. router.post("/adapters/:type/reinstall", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const type = req.params.type; @@ -613,6 +618,8 @@ export function adapterRoutes() { const CONFIG_SCHEMA_TTL_MS = 30_000; router.get("/adapters/:type/config-schema", async (req, res) => { + // Config schemas are read-only form metadata used when org members create + // or edit agents; they do not install or execute new adapter code. assertBoardOrgAccess(req); const { type } = req.params; @@ -651,6 +658,8 @@ export function adapterRoutes() { // The adapter package must export a "./ui-parser" entry in package.json // pointing to a self-contained ESM module with zero runtime dependencies. router.get("/adapters/:type/ui-parser.js", (req, res) => { + // UI parsers are read-only assets for displaying existing run output. + // Runtime-changing adapter management routes above require instance admin. assertBoardOrgAccess(req); const { type } = req.params; const source = getOrExtractUiParserSource(type);