From 0ad2413ff83e1a8d5975fe512d3fd4871ed1a8a6 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 22 May 2026 13:03:49 +0000 Subject: [PATCH] Harden plugin invocation actor context --- .../plugins/sdk/src/host-client-factory.ts | 2 +- packages/plugins/sdk/src/protocol.ts | 2 + packages/plugins/sdk/src/ui/types.ts | 1 + packages/plugins/sdk/src/worker-rpc-host.ts | 3 +- .../sdk/tests/host-client-factory.test.ts | 9 ++ .../plugins/sdk/tests/worker-rpc-host.test.ts | 112 +++++++++++++++++- packages/shared/src/constants.ts | 1 + .../src/__tests__/plugin-routes-authz.test.ts | 23 ++++ server/src/routes/plugins.ts | 13 +- 9 files changed, 160 insertions(+), 6 deletions(-) diff --git a/packages/plugins/sdk/src/host-client-factory.ts b/packages/plugins/sdk/src/host-client-factory.ts index e080d33d464..a47582530d0 100644 --- a/packages/plugins/sdk/src/host-client-factory.ts +++ b/packages/plugins/sdk/src/host-client-factory.ts @@ -79,7 +79,7 @@ export class CapabilityDeniedError extends Error { */ export class InvocationScopeDeniedError extends Error { override readonly name = "InvocationScopeDeniedError"; - readonly code = PLUGIN_RPC_ERROR_CODES.CAPABILITY_DENIED; + readonly code = PLUGIN_RPC_ERROR_CODES.INVOCATION_SCOPE_DENIED; constructor(pluginId: string, method: string, requestedCompanyId: string, allowedCompanyId: string) { super( diff --git a/packages/plugins/sdk/src/protocol.ts b/packages/plugins/sdk/src/protocol.ts index 2e1b69cc449..b5c76c5b5f1 100644 --- a/packages/plugins/sdk/src/protocol.ts +++ b/packages/plugins/sdk/src/protocol.ts @@ -227,6 +227,8 @@ export const PLUGIN_RPC_ERROR_CODES = { TIMEOUT: -32003, /** The worker does not implement the requested optional method. */ METHOD_NOT_IMPLEMENTED: -32004, + /** The worker→host call attempted to escape the current invocation company scope. */ + INVOCATION_SCOPE_DENIED: -32005, /** A catch-all for errors that do not fit other categories. */ UNKNOWN: -32099, } as const; diff --git a/packages/plugins/sdk/src/ui/types.ts b/packages/plugins/sdk/src/ui/types.ts index b8216836157..d2a03ec4a80 100644 --- a/packages/plugins/sdk/src/ui/types.ts +++ b/packages/plugins/sdk/src/ui/types.ts @@ -54,6 +54,7 @@ export type { * Error codes: * - `WORKER_UNAVAILABLE` — plugin worker is not running * - `CAPABILITY_DENIED` — plugin lacks the required capability + * - `INVOCATION_SCOPE_DENIED` — plugin call escaped the invocation company scope * - `WORKER_ERROR` — worker returned an error from its handler * - `TIMEOUT` — worker did not respond within the configured timeout * - `UNKNOWN` — unexpected bridge-level failure diff --git a/packages/plugins/sdk/src/worker-rpc-host.ts b/packages/plugins/sdk/src/worker-rpc-host.ts index 7b0d1a4d696..6fa2554aa58 100644 --- a/packages/plugins/sdk/src/worker-rpc-host.ts +++ b/packages/plugins/sdk/src/worker-rpc-host.ts @@ -1454,13 +1454,12 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost const rawActor = params.actorContext && typeof params.actorContext === "object" ? params.actorContext : null; - const legacyCompanyId = stringOrNull(params.params.companyId); const actor = Object.freeze({ type: actorTypeOrSystem(rawActor?.type), userId: stringOrNull(rawActor?.userId), agentId: stringOrNull(rawActor?.agentId), runId: stringOrNull(rawActor?.runId), - companyId: stringOrNull(rawActor?.companyId) ?? legacyCompanyId, + companyId: stringOrNull(rawActor?.companyId), }); return Object.freeze({ actor, diff --git a/packages/plugins/sdk/tests/host-client-factory.test.ts b/packages/plugins/sdk/tests/host-client-factory.test.ts index 1c7d3da0e70..52b601a50a3 100644 --- a/packages/plugins/sdk/tests/host-client-factory.test.ts +++ b/packages/plugins/sdk/tests/host-client-factory.test.ts @@ -5,6 +5,7 @@ import { createHostClientHandlers, InvocationScopeDeniedError, } from "../src/host-client-factory.js"; +import { PLUGIN_RPC_ERROR_CODES } from "../src/protocol.js"; describe("createHostClientHandlers invocation company scope", () => { it("rejects company-scoped host calls outside the current invocation company", async () => { @@ -27,6 +28,14 @@ describe("createHostClientHandlers invocation company scope", () => { { invocationScope: { companyId: "company-a" } }, ), ).rejects.toBeInstanceOf(InvocationScopeDeniedError); + await expect( + handlers["projects.list"]( + { companyId: "company-b" }, + { invocationScope: { companyId: "company-a" } }, + ), + ).rejects.toMatchObject({ + code: PLUGIN_RPC_ERROR_CODES.INVOCATION_SCOPE_DENIED, + }); expect(projectsList).not.toHaveBeenCalled(); }); diff --git a/packages/plugins/sdk/tests/worker-rpc-host.test.ts b/packages/plugins/sdk/tests/worker-rpc-host.test.ts index f8e0a38e5dc..19893c4f61a 100644 --- a/packages/plugins/sdk/tests/worker-rpc-host.test.ts +++ b/packages/plugins/sdk/tests/worker-rpc-host.test.ts @@ -1,11 +1,63 @@ import fs from "node:fs"; import os from "node:os"; +import { PassThrough } from "node:stream"; import path from "node:path"; import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; -import { isWorkerEntrypoint } from "../src/worker-rpc-host.js"; +import { definePlugin } from "../src/define-plugin.js"; +import type { JsonRpcResponse } from "../src/protocol.js"; +import { isWorkerEntrypoint, startWorkerRpcHost } from "../src/worker-rpc-host.js"; + +function createWorkerHarness() { + const stdin = new PassThrough(); + const stdout = new PassThrough(); + let buffer = ""; + const messages: JsonRpcResponse[] = []; + const waiters = new Map void>>(); + + stdout.setEncoding("utf8"); + stdout.on("data", (chunk) => { + buffer += String(chunk); + let newlineIndex = buffer.indexOf("\n"); + while (newlineIndex >= 0) { + const line = buffer.slice(0, newlineIndex).trim(); + buffer = buffer.slice(newlineIndex + 1); + if (line) { + const message = JSON.parse(line) as JsonRpcResponse; + const waiter = waiters.get(message.id)?.shift(); + if (waiter) { + waiter(message); + } else { + messages.push(message); + } + } + newlineIndex = buffer.indexOf("\n"); + } + }); + + async function request(method: string, params: unknown, id: string | number): Promise { + const existingIndex = messages.findIndex((message) => message.id === id); + if (existingIndex >= 0) { + return messages.splice(existingIndex, 1)[0]!; + } + const response = new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error(`Timed out waiting for ${method}`)), 1_000); + const wrappedResolve = (message: JsonRpcResponse) => { + clearTimeout(timer); + resolve(message); + }; + const existing = waiters.get(id) ?? []; + existing.push(wrappedResolve); + waiters.set(id, existing); + }); + stdin.write(JSON.stringify({ jsonrpc: "2.0", id, method, params }) + "\n"); + return response; + } + + return { stdin, stdout, request }; +} describe("isWorkerEntrypoint", () => { const tempRoots: string[] = []; @@ -55,3 +107,61 @@ describe("isWorkerEntrypoint", () => { ).toBe(false); }); }); + +describe("worker performAction context", () => { + it("does not derive context companyId from caller params without host actor context", async () => { + const harness = createWorkerHarness(); + const plugin = definePlugin({ + async setup(ctx) { + ctx.actions.register("inspect", async (params, context) => ({ + paramsCompanyId: params.companyId, + actor: context.actor, + companyId: context.companyId, + })); + }, + }); + const host = startWorkerRpcHost({ + plugin, + stdin: harness.stdin, + stdout: harness.stdout, + }); + + try { + const initialize = await harness.request("initialize", { + manifest: { + id: "paperclip.test-worker-context", + apiVersion: 1, + version: "1.0.0", + displayName: "Worker Context Test", + description: "Test plugin", + author: "Paperclip", + categories: ["automation"], + capabilities: [], + entrypoints: {}, + }, + config: {}, + databaseNamespace: null, + }, 1); + expect("result" in initialize ? initialize.result : null).toMatchObject({ ok: true }); + + const response = await harness.request("performAction", { + key: "inspect", + params: { companyId: "spoofed-company" }, + }, 2); + + expect("result" in response ? response.result : null).toEqual({ + paramsCompanyId: "spoofed-company", + actor: { + type: "system", + userId: null, + agentId: null, + runId: null, + companyId: null, + }, + companyId: null, + }); + } finally { + host.stop(); + } + }); +}); diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index 9c295eae956..7e31bf4c0d5 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -1070,6 +1070,7 @@ export type PluginEventType = (typeof PLUGIN_EVENT_TYPES)[number]; export const PLUGIN_BRIDGE_ERROR_CODES = [ "WORKER_UNAVAILABLE", "CAPABILITY_DENIED", + "INVOCATION_SCOPE_DENIED", "WORKER_ERROR", "TIMEOUT", "UNKNOWN", diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 0b8a14da391..9353ef034ea 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -693,6 +693,29 @@ describe.sequential("plugin tool and bridge authz", () => { }); }); + it("uses null for board actor userId when no authenticated user id is present", async () => { + readyPlugin(); + const call = vi.fn().mockResolvedValue({ ok: true }); + const { app } = await createApp(boardActor({ userId: undefined }), {}, { + bridgeDeps: { + workerManager: { call }, + }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/actions/sync`) + .send({ companyId: companyA }); + + expect(res.status).toBe(200); + expect(call).toHaveBeenCalledWith(pluginId, "performAction", expect.objectContaining({ + actorContext: expect.objectContaining({ + type: "user", + userId: null, + companyId: companyA, + }), + })); + }); + it("allows agent-scoped plugin actions with authenticated actor context", async () => { readyPlugin(); const call = vi.fn().mockResolvedValue({ ok: true }); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index 3914827335d..26fa3e23bbf 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -586,7 +586,7 @@ export function pluginRoutes( if (req.actor.type === "board") { return { type: "user", - userId: req.actor.userId ?? "board", + userId: req.actor.userId ?? null, agentId: null, runId: req.actor.runId ?? null, companyId: scopedCompanyId, @@ -1021,6 +1021,12 @@ export function pluginRoutes( message: err.message, details: err.data, }; + case PLUGIN_RPC_ERROR_CODES.INVOCATION_SCOPE_DENIED: + return { + code: "INVOCATION_SCOPE_DENIED", + message: err.message, + details: err.data, + }; case PLUGIN_RPC_ERROR_CODES.TIMEOUT: return { code: "TIMEOUT", @@ -1641,7 +1647,10 @@ export function pluginRoutes( } catch (err) { const status = typeof (err as { status?: unknown }).status === "number" ? (err as { status: number }).status - : err instanceof JsonRpcCallError && err.code === PLUGIN_RPC_ERROR_CODES.CAPABILITY_DENIED + : err instanceof JsonRpcCallError && ( + err.code === PLUGIN_RPC_ERROR_CODES.CAPABILITY_DENIED || + err.code === PLUGIN_RPC_ERROR_CODES.INVOCATION_SCOPE_DENIED + ) ? 403 : err instanceof JsonRpcCallError && err.code === PLUGIN_RPC_ERROR_CODES.METHOD_NOT_IMPLEMENTED ? 501