PAP-1646 restrict adapter management to instance admins

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<string, any>();
+
+  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();
+    },
+  );
+});

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);