mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 00:30:19 +02:00
Enforce plugin route company scoping
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
a839769f55
commit
71bbdf5c8a
2 files changed
+343
-18
No files matched your search
@@ -35,7 +35,12 @@ vi.mock("../services/live-events.js", () => ({
|
||||
async function createApp(
|
||||
actor: Record<string, unknown>,
|
||||
loaderOverrides: Record<string, unknown> = {},
|
||||
bridgeDeps?: Record<string, unknown>,
|
||||
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<Array<Record<string, unknown>>>) {
|
||||
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<string, unknown> = {}) {
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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<NonNullable<PaperclipPluginManifestV1["ui"]>["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<string | null> {
|
||||
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;
|
||||
|
||||
Reference in new issue
Block a user