From 71bbdf5c8aaea4eef19968fd901306d68ebc726b Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 20 Apr 2026 08:08:14 -0500 Subject: [PATCH] Enforce plugin route company scoping Co-Authored-By: Paperclip --- .../src/__tests__/plugin-routes-authz.test.ts | 281 +++++++++++++++++- server/src/routes/plugins.ts | 80 ++++- 2 files changed, 343 insertions(+), 18 deletions(-) diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 95a9281caec..0989a71a3bd 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -35,7 +35,12 @@ vi.mock("../services/live-events.js", () => ({ async function createApp( actor: Record, loaderOverrides: Record = {}, - bridgeDeps?: Record, + routeOverrides: { + db?: unknown; + jobDeps?: unknown; + toolDeps?: unknown; + bridgeDeps?: unknown; + } = {}, ) { const [{ pluginRoutes }, { errorHandler }] = await Promise.all([ import("../routes/plugins.js"), @@ -53,12 +58,58 @@ async function createApp( req.actor = actor as typeof req.actor; next(); }); - app.use("/api", pluginRoutes({} as never, loader as never, undefined, undefined, undefined, bridgeDeps as never)); + app.use("/api", pluginRoutes( + (routeOverrides.db ?? {}) as never, + loader as never, + routeOverrides.jobDeps as never, + undefined, + routeOverrides.toolDeps as never, + routeOverrides.bridgeDeps as never, + )); app.use(errorHandler); return { app, loader }; } +function createSelectQueueDb(rows: Array>>) { + return { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: vi.fn(() => Promise.resolve(rows.shift() ?? [])), + })), + })), + })), + }; +} + +const companyA = "22222222-2222-4222-8222-222222222222"; +const companyB = "33333333-3333-4333-8333-333333333333"; +const agentA = "44444444-4444-4444-8444-444444444444"; +const runA = "55555555-5555-4555-8555-555555555555"; +const projectA = "66666666-6666-4666-8666-666666666666"; +const pluginId = "11111111-1111-4111-8111-111111111111"; + +function boardActor(overrides: Record = {}) { + return { + type: "board", + userId: "user-1", + source: "session", + isInstanceAdmin: false, + companyIds: [companyA], + ...overrides, + }; +} + +function readyPlugin() { + mockRegistry.getById.mockResolvedValue({ + id: pluginId, + pluginKey: "paperclip.example", + version: "1.0.0", + status: "ready", + }); +} + describe("plugin install and upgrade authz", () => { beforeEach(() => { vi.resetAllMocks(); @@ -244,7 +295,7 @@ describe("scoped plugin API routes", () => { companyIds: ["company-1"], }, {}, - { workerManager }, + { bridgeDeps: { workerManager } }, ); const res = await request(app) @@ -265,3 +316,227 @@ describe("scoped plugin API routes", () => { ); }, 20_000); }); + +describe("plugin tool and bridge authz", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("rejects tool execution when the board user cannot access runContext.companyId", async () => { + const executeTool = vi.fn(); + const getTool = vi.fn(); + const { app } = await createApp(boardActor(), {}, { + toolDeps: { + toolDispatcher: { + listToolsForAgent: vi.fn(), + getTool, + executeTool, + }, + }, + }); + + const res = await request(app) + .post("/api/plugins/tools/execute") + .send({ + tool: "paperclip.example:search", + parameters: {}, + runContext: { + agentId: agentA, + runId: runA, + companyId: companyB, + projectId: projectA, + }, + }); + + expect(res.status).toBe(403); + expect(getTool).not.toHaveBeenCalled(); + expect(executeTool).not.toHaveBeenCalled(); + }); + + it.each([ + [ + "agentId", + [ + [{ companyId: companyB }], + ], + ], + [ + "runId company", + [ + [{ companyId: companyA }], + [{ companyId: companyB, agentId: agentA }], + ], + ], + [ + "runId agent", + [ + [{ companyId: companyA }], + [{ companyId: companyA, agentId: "77777777-7777-4777-8777-777777777777" }], + ], + ], + [ + "projectId", + [ + [{ companyId: companyA }], + [{ companyId: companyA, agentId: agentA }], + [{ companyId: companyB }], + ], + ], + ])("rejects tool execution when runContext.%s is outside the company scope", async (_case, rows) => { + const executeTool = vi.fn(); + const { app } = await createApp(boardActor(), {}, { + db: createSelectQueueDb(rows), + toolDeps: { + toolDispatcher: { + listToolsForAgent: vi.fn(), + getTool: vi.fn(() => ({ name: "paperclip.example:search" })), + executeTool, + }, + }, + }); + + const res = await request(app) + .post("/api/plugins/tools/execute") + .send({ + tool: "paperclip.example:search", + parameters: {}, + runContext: { + agentId: agentA, + runId: runA, + companyId: companyA, + projectId: projectA, + }, + }); + + expect(res.status).toBe(403); + expect(executeTool).not.toHaveBeenCalled(); + }); + + it("allows tool execution when agent, run, and project all belong to runContext.companyId", async () => { + const executeTool = vi.fn().mockResolvedValue({ content: "ok" }); + const { app } = await createApp(boardActor(), {}, { + db: createSelectQueueDb([ + [{ companyId: companyA }], + [{ companyId: companyA, agentId: agentA }], + [{ companyId: companyA }], + ]), + toolDeps: { + toolDispatcher: { + listToolsForAgent: vi.fn(), + getTool: vi.fn(() => ({ name: "paperclip.example:search" })), + executeTool, + }, + }, + }); + + const res = await request(app) + .post("/api/plugins/tools/execute") + .send({ + tool: "paperclip.example:search", + parameters: { q: "test" }, + runContext: { + agentId: agentA, + runId: runA, + companyId: companyA, + projectId: projectA, + }, + }); + + expect(res.status).toBe(200); + expect(executeTool).toHaveBeenCalledWith( + "paperclip.example:search", + { q: "test" }, + { + agentId: agentA, + runId: runA, + companyId: companyA, + projectId: projectA, + }, + ); + }); + + it.each([ + ["legacy data", "post", `/api/plugins/${pluginId}/bridge/data`, { key: "health" }], + ["legacy action", "post", `/api/plugins/${pluginId}/bridge/action`, { key: "sync" }], + ["url data", "post", `/api/plugins/${pluginId}/data/health`, {}], + ["url action", "post", `/api/plugins/${pluginId}/actions/sync`, {}], + ] as const)("rejects %s bridge calls without companyId for non-admin users", async (_name, _method, path, body) => { + readyPlugin(); + const call = vi.fn(); + const { app } = await createApp(boardActor(), {}, { + bridgeDeps: { + workerManager: { call }, + }, + }); + + const res = await request(app) + .post(path) + .send(body); + + expect(res.status).toBe(403); + expect(call).not.toHaveBeenCalled(); + }); + + it("allows omitted-company bridge calls for instance admins as global plugin actions", async () => { + readyPlugin(); + const call = vi.fn().mockResolvedValue({ ok: true }); + const { app } = await createApp(boardActor({ + userId: "admin-1", + isInstanceAdmin: true, + companyIds: [], + }), {}, { + bridgeDeps: { + workerManager: { call }, + }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/actions/sync`) + .send({}); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ data: { ok: true } }); + expect(call).toHaveBeenCalledWith(pluginId, "performAction", { + key: "sync", + params: {}, + renderEnvironment: null, + }); + }); + + it("rejects manual job triggers for non-admin board users", async () => { + const scheduler = { triggerJob: vi.fn() }; + const jobStore = { getJobByIdForPlugin: vi.fn() }; + const { app } = await createApp(boardActor(), {}, { + jobDeps: { scheduler, jobStore }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/jobs/job-1/trigger`) + .send({}); + + expect(res.status).toBe(403); + expect(scheduler.triggerJob).not.toHaveBeenCalled(); + expect(jobStore.getJobByIdForPlugin).not.toHaveBeenCalled(); + }); + + it("allows manual job triggers for instance admins", async () => { + readyPlugin(); + const scheduler = { triggerJob: vi.fn().mockResolvedValue({ runId: "run-1", jobId: "job-1" }) }; + const jobStore = { getJobByIdForPlugin: vi.fn().mockResolvedValue({ id: "job-1" }) }; + const { app } = await createApp(boardActor({ + userId: "admin-1", + isInstanceAdmin: true, + companyIds: [], + }), {}, { + jobDeps: { scheduler, jobStore }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/jobs/job-1/trigger`) + .send({}); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ runId: "run-1", jobId: "job-1" }); + expect(scheduler.triggerJob).toHaveBeenCalledWith("job-1", "manual"); + }); +}); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index 19a67391611..43a6e7dd25e 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -26,7 +26,14 @@ import { Router } from "express"; import type { Request, Response } from "express"; import { and, desc, eq, gte } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import { companies, pluginLogs, pluginWebhookDeliveries } from "@paperclipai/db"; +import { + agents, + companies, + heartbeatRuns, + pluginLogs, + pluginWebhookDeliveries, + projects, +} from "@paperclipai/db"; import type { PluginApiRouteDeclaration, PluginStatus, @@ -59,7 +66,7 @@ import { getActorInfo, } from "./authz.js"; import { validateInstanceConfig } from "../services/plugin-config-validator.js"; -import { forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; +import { badRequest, forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; /** UI slot declaration extracted from plugin manifest */ type PluginUiSlotDeclaration = NonNullable["slots"]>[number]; @@ -548,6 +555,52 @@ export function pluginRoutes( }))); } + function assertPluginBridgeScope(req: Request, companyId: unknown): string | undefined { + if (companyId === undefined || companyId === null) { + assertInstanceAdmin(req); + return undefined; + } + if (typeof companyId !== "string" || companyId.trim().length === 0) { + throw badRequest('"companyId" must be a non-empty string when provided'); + } + assertCompanyAccess(req, companyId); + return companyId; + } + + async function validateToolRunContextScope(runContext: ToolRunContext): Promise { + const [agent] = await db + .select({ companyId: agents.companyId }) + .from(agents) + .where(eq(agents.id, runContext.agentId)) + .limit(1); + if (!agent || agent.companyId !== runContext.companyId) { + return '"runContext.agentId" does not belong to "runContext.companyId"'; + } + + const [run] = await db + .select({ companyId: heartbeatRuns.companyId, agentId: heartbeatRuns.agentId }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runContext.runId)) + .limit(1); + if (!run || run.companyId !== runContext.companyId) { + return '"runContext.runId" does not belong to "runContext.companyId"'; + } + if (run.agentId !== runContext.agentId) { + return '"runContext.runId" does not belong to "runContext.agentId"'; + } + + const [project] = await db + .select({ companyId: projects.companyId }) + .from(projects) + .where(eq(projects.id, runContext.projectId)) + .limit(1); + if (!project || project.companyId !== runContext.companyId) { + return '"runContext.projectId" does not belong to "runContext.companyId"'; + } + + return null; + } + /** * GET /api/plugins * @@ -742,6 +795,11 @@ export function pluginRoutes( } assertCompanyAccess(req, runContext.companyId); + const scopeError = await validateToolRunContextScope(runContext); + if (scopeError) { + res.status(403).json({ error: scopeError }); + return; + } // Verify the tool exists const registeredTool = toolDeps.toolDispatcher.getTool(tool); @@ -1020,9 +1078,7 @@ export function pluginRoutes( return; } - if (body.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -1103,9 +1159,7 @@ export function pluginRoutes( return; } - if (body.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -1186,9 +1240,7 @@ export function pluginRoutes( renderEnvironment?: PluginLauncherRenderContextSnapshot | null; } | undefined; - if (body?.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body?.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -1265,9 +1317,7 @@ export function pluginRoutes( renderEnvironment?: PluginLauncherRenderContextSnapshot | null; } | undefined; - if (body?.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body?.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -2140,7 +2190,7 @@ export function pluginRoutes( * - 400 if job not found, not active, already running, or worker unavailable */ router.post("/plugins/:pluginId/jobs/:jobId/trigger", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); if (!jobDeps) { res.status(501).json({ error: "Job scheduling is not enabled" }); return;