diff --git a/doc/plugins/PLUGIN_SPEC.md b/doc/plugins/PLUGIN_SPEC.md index d1bd103486c..523aba36037 100644 --- a/doc/plugins/PLUGIN_SPEC.md +++ b/doc/plugins/PLUGIN_SPEC.md @@ -607,7 +607,11 @@ The plugin UI calls the host bridge, which forwards the request to the worker. T Input includes: - data key (plugin-defined, e.g. `"sync-health"`, `"issue-detail"`) -- context (company id, project id, entity id, etc.) +- browser-supplied context (company id, project id, entity id, etc.) +- trusted actor context from the host, available to SDK handlers as the second + argument (`request.actor`). User-scoped plugins must compare any requested + `userId` with `request.actor.userId`; they must not trust a `userId` supplied + by the plugin UI request body. - optional query parameters ### 13.9 `performAction` @@ -621,6 +625,10 @@ Examples: - "create branch from issue" - "restart process" +Action handlers receive the same trusted host request context as data handlers +as their second argument. User-scoped mutations must authorize against +`request.actor.userId` rather than any caller-supplied `params.userId`. + ### 13.10 `executeTool` Runs a plugin-contributed agent tool during a run. diff --git a/packages/plugins/plugin-briefs/src/worker.ts b/packages/plugins/plugin-briefs/src/worker.ts index 205d6dcc86d..526dea0b138 100644 --- a/packages/plugins/plugin-briefs/src/worker.ts +++ b/packages/plugins/plugin-briefs/src/worker.ts @@ -2,6 +2,7 @@ import { definePlugin, runWorker, type Issue, + type PluginBridgeRequestContext, type PluginContext, type PluginIssueRelationSummary, type ToolResult, @@ -51,6 +52,16 @@ function stringParam(value: unknown, name: string): string { return value.trim(); } +function assertTrustedBridgeUserScope(userId: string, request?: PluginBridgeRequestContext): void { + const actorUserId = request?.actor?.userId; + if (!actorUserId) { + throw new Error("Briefs user-scoped UI calls require a signed-in user"); + } + if (actorUserId !== userId) { + throw new Error("Briefs user scope mismatch"); + } +} + function numberParam(value: unknown): number | null { return typeof value === "number" && Number.isFinite(value) ? Math.max(0, Math.trunc(value)) : null; } @@ -280,26 +291,28 @@ const plugin = definePlugin({ async setup(ctx) { const store = createBriefsStore(ctx.db); - ctx.data.register("cards", async (params) => { + ctx.data.register("cards", async (params, request) => { const input = listBriefCardsInputSchema.parse(params); + assertTrustedBridgeUserScope(input.userId, request); return { cards: await store.listCards(input), }; }); - ctx.data.register("preferences", async (params) => { + ctx.data.register("preferences", async (params, request) => { const input = objectParam(params, "params"); const companyId = typeof input.companyId === "string" ? input.companyId : ""; const userId = typeof input.userId === "string" ? input.userId : ""; if (!companyId || !userId) { throw new Error("companyId and userId are required"); } + assertTrustedBridgeUserScope(userId, request); return { preferences: await store.loadPreferences({ companyId, userId }), }; }); - ctx.data.register("page", async (params) => { + ctx.data.register("page", async (params, request) => { const input = objectParam(params, "params"); const companyId = typeof input.companyId === "string" ? input.companyId : ""; const userId = typeof input.userId === "string" ? input.userId : ""; @@ -307,6 +320,7 @@ const plugin = definePlugin({ if (!companyId || !userId) { throw new Error("companyId and userId are required"); } + assertTrustedBridgeUserScope(userId, request); const [cards, preferences] = await Promise.all([ store.listCards({ companyId, userId, includeHidden }), store.loadPreferences({ companyId, userId }), @@ -373,8 +387,9 @@ const plugin = definePlugin({ return ctx.routines.managed.run(MANUAL_REFRESH_ROUTINE_KEY, stringParam(params.companyId, "companyId")); }); - ctx.actions.register("pin-card", async (params) => { + ctx.actions.register("pin-card", async (params, request) => { const input = pinBriefCardInputSchema.parse(params); + assertTrustedBridgeUserScope(input.userId, request); await store.setPinned(input); await ctx.activity.log({ companyId: input.companyId, @@ -386,8 +401,9 @@ const plugin = definePlugin({ return { ok: true }; }); - ctx.actions.register("update-preferences", async (params) => { + ctx.actions.register("update-preferences", async (params, request) => { const input = updateBriefPreferencesInputSchema.parse(params); + assertTrustedBridgeUserScope(input.userId, request); const preferences = briefPreferencesSchema.parse(input); await store.upsertPreferences(preferences); await ctx.activity.log({ diff --git a/packages/plugins/plugin-briefs/tests/managed-resources.spec.ts b/packages/plugins/plugin-briefs/tests/managed-resources.spec.ts index ad7bae1f8ff..a62be2be1b6 100644 --- a/packages/plugins/plugin-briefs/tests/managed-resources.spec.ts +++ b/packages/plugins/plugin-briefs/tests/managed-resources.spec.ts @@ -11,6 +11,14 @@ import manifest, { import plugin from "../src/worker.js"; const companyId = "11111111-1111-4111-8111-111111111111"; +const bridgeActor = { + actorType: "user" as const, + actorId: "signed-in-user", + userId: "signed-in-user", + agentId: null, + runId: null, + source: "session", +}; describe("Briefs managed resources", () => { it("declares the Briefing Analyst, skills, routines, and agent tools", () => { @@ -71,4 +79,23 @@ describe("Briefs managed resources", () => { expect(routine.missingRefs).toEqual([]); } }); + + it("rejects user-scoped UI bridge calls for a different user", async () => { + const harness = createTestHarness({ manifest }); + await plugin.definition.setup?.(harness.ctx); + const victimParams = { companyId, userId: "victim-user" }; + const context = { actor: bridgeActor }; + + await expect(harness.getData("page", victimParams, context)).rejects.toThrow("Briefs user scope mismatch"); + await expect(harness.getData("preferences", victimParams, context)).rejects.toThrow("Briefs user scope mismatch"); + await expect(harness.performAction("pin-card", { + ...victimParams, + cardId: "card-1", + pinned: true, + }, context)).rejects.toThrow("Briefs user scope mismatch"); + await expect(harness.performAction("update-preferences", { + ...victimParams, + cadence: "daily", + }, context)).rejects.toThrow("Briefs user scope mismatch"); + }); }); diff --git a/packages/plugins/sdk/README.md b/packages/plugins/sdk/README.md index b9a02d5c149..51b1725c102 100644 --- a/packages/plugins/sdk/README.md +++ b/packages/plugins/sdk/README.md @@ -564,6 +564,8 @@ export function DashboardWidget() { Fetches data from the worker's registered `getData` handler. Re-fetches when `params` changes. Returns `{ data, loading, error, refresh }`. +Worker handlers receive browser-supplied `params` as the first argument and trusted host context as the second argument. For user-scoped data, compare any requested `params.userId` with `request.actor.userId`; do not trust `params.userId` by itself. + ```tsx import { usePluginData } from "@paperclipai/plugin-sdk/ui"; @@ -596,6 +598,8 @@ export function SyncStatusWidget({ context }: PluginWidgetProps) { Returns an async function that calls the worker's `performAction` handler. Throws `PluginBridgeError` on failure. +Action handlers also receive trusted host context as their second argument. User-scoped mutations should authorize against `request.actor.userId`. + ```tsx import { useState } from "react"; import { usePluginAction, type PluginBridgeError } from "@paperclipai/plugin-sdk/ui"; diff --git a/packages/plugins/sdk/src/index.ts b/packages/plugins/sdk/src/index.ts index 1fbf172b509..a8f35c98273 100644 --- a/packages/plugins/sdk/src/index.ts +++ b/packages/plugins/sdk/src/index.ts @@ -241,6 +241,10 @@ export type { AgentSessionEvent, AgentSessionSendResult, PluginGoalsClient, + PluginBridgeActorContext, + PluginBridgeRequestContext, + PluginDataHandler, + PluginActionHandler, PluginDataClient, PluginActionsClient, PluginStreamsClient, diff --git a/packages/plugins/sdk/src/protocol.ts b/packages/plugins/sdk/src/protocol.ts index 2c76beab23a..6180d5f117a 100644 --- a/packages/plugins/sdk/src/protocol.ts +++ b/packages/plugins/sdk/src/protocol.ts @@ -65,6 +65,7 @@ import type { PluginAuthorizationDecisionResult, PluginAuthorizationPolicyRecord, PluginAuthorizationPolicySummary, + PluginBridgeActorContext, } from "./types.js"; import type { PluginHealthDiagnostics, @@ -361,6 +362,8 @@ export interface GetDataParams { companyId?: string | null; /** Context and query parameters from the UI. */ params: Record; + /** Trusted actor context supplied by the host, never by the browser. */ + actor?: PluginBridgeActorContext | null; /** Optional launcher/container metadata from the host render environment. */ renderEnvironment?: PluginLauncherRenderContextSnapshot | null; } @@ -401,6 +404,8 @@ export interface PerformActionParams { params: Record; /** Authenticated actor context resolved by the host, never by caller params. */ actorContext?: PluginPerformActionActorContext | null; + /** Trusted actor context supplied by the host, never by the browser. */ + actor?: PluginBridgeActorContext | null; /** Optional launcher/container metadata from the host render environment. */ renderEnvironment?: PluginLauncherRenderContextSnapshot | null; } diff --git a/packages/plugins/sdk/src/testing.ts b/packages/plugins/sdk/src/testing.ts index f8f1fc262aa..a0473d5b2ad 100644 --- a/packages/plugins/sdk/src/testing.ts +++ b/packages/plugins/sdk/src/testing.ts @@ -42,6 +42,9 @@ import type { PrincipalPermissionGrant, PermissionKey, PrincipalType, + PluginBridgeRequestContext, + PluginDataHandler, + PluginActionHandler, } from "./types.js"; import type { PluginEnvironmentValidateConfigParams, @@ -57,8 +60,6 @@ import type { PluginEnvironmentRealizeWorkspaceResult, PluginEnvironmentExecuteParams, PluginEnvironmentExecuteResult, - PluginPerformActionActorContext, - PluginPerformActionContext, } from "./protocol.js"; export interface TestHarnessOptions { @@ -78,10 +79,9 @@ export interface TestHarnessLogEntry { export interface TestHarnessPerformActionOptions { /** - * Authenticated actor context to expose to the action handler. Omitted fields - * default to null, and `type` defaults to `system`. + * Authenticated actor context to expose to the action handler. */ - actor?: Partial | null; + actor?: Partial & { type?: "user" | "agent" | "system"; companyId?: string | null }> | null; /** * Host-authorized company scope. When provided, this is injected into * `params.companyId` so tests match the production bridge's anti-spoofing @@ -112,12 +112,12 @@ export interface TestHarness { /** Execute a previously-registered scheduled job handler. */ runJob(jobKey: string, partial?: Partial): Promise; /** Invoke a `ctx.data.register(...)` handler by key. */ - getData(key: string, params?: Record): Promise; + getData(key: string, params?: Record, context?: Partial): Promise; /** Invoke a `ctx.actions.register(...)` handler by key. */ performAction( key: string, params?: Record, - options?: TestHarnessPerformActionOptions, + options?: TestHarnessPerformActionOptions | Partial, ): Promise; /** Execute a registered tool handler via `ctx.tools.execute(...)`. */ executeTool(name: string, params: unknown, runCtx?: Partial): Promise; @@ -510,11 +510,8 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { const events: EventRegistration[] = []; const jobs = new Map Promise>(); const launchers = new Map(); - const dataHandlers = new Map) => Promise>(); - const actionHandlers = new Map< - string, - (params: Record, context: PluginPerformActionContext) => Promise - >(); + const dataHandlers = new Map(); + const actionHandlers = new Map(); const toolHandlers = new Map Promise>(); function localFolderKey(companyId: string, folderKey: string): string { @@ -529,29 +526,29 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; } - function actorTypeOrSystem(value: unknown): PluginPerformActionActorContext["type"] { - return value === "user" || value === "agent" || value === "system" ? value : "system"; - } - function actionContextFor( params: Record, options?: TestHarnessPerformActionOptions, - ): PluginPerformActionContext { + ): PluginBridgeRequestContext { const actorInput = options?.actor ?? null; const companyId = stringOrNull(options?.companyId) ?? stringOrNull(actorInput?.companyId) ?? stringOrNull(params.companyId); + const actorType = actorInput?.actorType === "agent" || actorInput?.type === "agent" ? "agent" : "user"; + const userId = stringOrNull(actorInput?.userId); + const agentId = stringOrNull(actorInput?.agentId); const actor = Object.freeze({ - type: actorTypeOrSystem(actorInput?.type), - userId: stringOrNull(actorInput?.userId), - agentId: stringOrNull(actorInput?.agentId), + actorType, + actorId: stringOrNull(actorInput?.actorId) ?? (actorType === "agent" ? agentId ?? "agent-test" : userId ?? "user-test"), + userId, + agentId, runId: stringOrNull(actorInput?.runId), - companyId, + source: stringOrNull(actorInput?.source), }); - return Object.freeze({ actor, companyId }); + return Object.freeze({ actor, companyId, renderEnvironment: null }); } function paramsWithHostCompanyScope( params: Record, - context: PluginPerformActionContext, + context: PluginBridgeRequestContext, options?: TestHarnessPerformActionOptions, ): Record { if (Object.prototype.hasOwnProperty.call(options ?? {}, "companyId")) { @@ -560,6 +557,17 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { return params; } + function isBridgeRequestContextOptions( + options: TestHarnessPerformActionOptions | Partial | undefined, + ): options is Partial { + if (!options || typeof options !== "object") return false; + const actor = (options as Partial).actor; + return Boolean( + "renderEnvironment" in options + || (actor && typeof actor === "object" && "actorType" in actor), + ); + } + function normalizeLocalFolderRelativePath(relativePath: string): string { const parts: string[] = []; for (const segment of relativePath.split(/[\\/]+/)) { @@ -2355,20 +2363,33 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { scheduledAt: partial.scheduledAt ?? new Date().toISOString(), }); }, - async getData(key: string, params: Record = {}) { + async getData(key: string, params: Record = {}, context: Partial = {}) { const handler = dataHandlers.get(key); if (!handler) throw new Error(`No data handler registered for '${key}'`); - return await handler(params) as T; + return await handler(params, { + actor: context.actor ?? null, + companyId: context.companyId ?? null, + renderEnvironment: context.renderEnvironment ?? null, + }) as T; }, async performAction( key: string, params: Record = {}, - options?: TestHarnessPerformActionOptions, + options?: TestHarnessPerformActionOptions | Partial, ) { const handler = actionHandlers.get(key); if (!handler) throw new Error(`No action handler registered for '${key}'`); - const context = actionContextFor(params, options); - return await handler(paramsWithHostCompanyScope(params, context, options), context) as T; + const bridgeContext = isBridgeRequestContextOptions(options) + ? { + actor: options.actor ?? null, + companyId: options.companyId ?? null, + renderEnvironment: options.renderEnvironment ?? null, + } + : actionContextFor(params, options as TestHarnessPerformActionOptions | undefined); + return await handler( + paramsWithHostCompanyScope(params, bridgeContext, options as TestHarnessPerformActionOptions | undefined), + bridgeContext, + ) as T; }, async executeTool(name: string, params: unknown, runCtx: Partial = {}) { const handler = toolHandlers.get(name); diff --git a/packages/plugins/sdk/src/types.ts b/packages/plugins/sdk/src/types.ts index 90af99b4803..7e9012d315c 100644 --- a/packages/plugins/sdk/src/types.ts +++ b/packages/plugins/sdk/src/types.ts @@ -46,8 +46,6 @@ import type { PrincipalPermissionGrant, PrincipalType, } from "@paperclipai/shared"; -import type { PluginPerformActionContext } from "./protocol.js"; - // --------------------------------------------------------------------------- // Re-exports from @paperclipai/shared (plugin authors import from one place) // --------------------------------------------------------------------------- @@ -928,6 +926,31 @@ export interface PluginSkillsClient { }; } +export interface PluginBridgeActorContext { + actorType: "user" | "agent"; + actorId: string; + userId: string | null; + agentId: string | null; + runId: string | null; + source: string | null; +} + +export interface PluginBridgeRequestContext { + actor: PluginBridgeActorContext | null; + companyId: string | null; + renderEnvironment?: PluginLauncherRenderContextSnapshot | null; +} + +export type PluginDataHandler = ( + params: Record, + context: PluginBridgeRequestContext, +) => Promise; + +export type PluginActionHandler = ( + params: Record, + context: PluginBridgeRequestContext, +) => Promise; + /** * `ctx.data` — register `getData` handlers that back `usePluginData()` in the * plugin's frontend components. @@ -942,9 +965,9 @@ export interface PluginDataClient { * Register a handler for a plugin-defined data key. * * @param key - Stable string identifier for this data type (e.g. `"sync-health"`) - * @param handler - Async function that receives request params and returns JSON-serializable data + * @param handler - Async function that receives request params plus trusted host context and returns JSON-serializable data */ - register(key: string, handler: (params: Record) => Promise): void; + register(key: string, handler: PluginDataHandler): void; } /** @@ -958,12 +981,9 @@ export interface PluginActionsClient { * Register a handler for a plugin-defined action key. * * @param key - Stable string identifier for this action (e.g. `"resync"`) - * @param handler - Async function that receives action params plus immutable host actor context and returns a result + * @param handler - Async function that receives action params plus trusted host context and returns a result */ - register( - key: string, - handler: (params: Record, context: PluginPerformActionContext) => Promise, - ): void; + register(key: string, handler: PluginActionHandler): void; } /** diff --git a/packages/plugins/sdk/src/worker-rpc-host.ts b/packages/plugins/sdk/src/worker-rpc-host.ts index a135d1f0a9c..713bc00be3d 100644 --- a/packages/plugins/sdk/src/worker-rpc-host.ts +++ b/packages/plugins/sdk/src/worker-rpc-host.ts @@ -64,6 +64,10 @@ import type { ToolResult, EventFilter, AgentSessionEvent, + PluginDataHandler, + PluginActionHandler, + PluginBridgeActorContext, + PluginBridgeRequestContext, } from "./types.js"; import type { JsonRpcId, @@ -78,8 +82,6 @@ import type { RunJobParams, GetDataParams, PerformActionParams, - PluginPerformActionActorContext, - PluginPerformActionContext, ExecuteToolParams, PluginEnvironmentAcquireLeaseParams, PluginEnvironmentDestroyLeaseParams, @@ -290,11 +292,8 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost const eventHandlers: EventRegistration[] = []; const jobHandlers = new Map Promise>(); const launcherRegistrations = new Map(); - const dataHandlers = new Map) => Promise>(); - const actionHandlers = new Map< - string, - (params: Record, context: PluginPerformActionContext) => Promise - >(); + const dataHandlers = new Map(); + const actionHandlers = new Map(); const toolHandlers = new Map; fn: (params: unknown, runCtx: ToolRunContext) => Promise; @@ -1189,10 +1188,7 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost }, actions: { - register( - key: string, - handler: (params: Record, context: PluginPerformActionContext) => Promise, - ): void { + register(key: string, handler: PluginActionHandler): void { actionHandlers.set(key, handler); }, }, @@ -1519,31 +1515,38 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost ...params.params, ...(params.companyId === undefined ? {} : { companyId: params.companyId }), ...(params.renderEnvironment === undefined ? {} : { renderEnvironment: params.renderEnvironment }), - }); + }, bridgeRequestContextFromParams(params)); } function stringOrNull(value: unknown): string | null { return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; } - function actorTypeOrSystem(value: unknown): PluginPerformActionActorContext["type"] { - return value === "user" || value === "agent" || value === "system" ? value : "system"; + function bridgeActorFromLegacyContext(value: unknown): PluginBridgeActorContext | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const rawActor = value as Record; + const actorType = rawActor.type === "agent" ? "agent" : "user"; + const userId = stringOrNull(rawActor.userId); + const agentId = stringOrNull(rawActor.agentId); + return Object.freeze({ + actorType, + actorId: actorType === "agent" ? agentId ?? "unknown-agent" : userId ?? "board", + userId, + agentId, + runId: stringOrNull(rawActor.runId), + source: null, + }); } - function actionContextFromParams(params: PerformActionParams): PluginPerformActionContext { - const rawActor = params.actorContext && typeof params.actorContext === "object" - ? params.actorContext - : null; - const actor = Object.freeze({ - type: actorTypeOrSystem(rawActor?.type), - userId: stringOrNull(rawActor?.userId), - agentId: stringOrNull(rawActor?.agentId), - runId: stringOrNull(rawActor?.runId), - companyId: stringOrNull(rawActor?.companyId), - }); + function bridgeRequestContextFromParams(params: GetDataParams | PerformActionParams): PluginBridgeRequestContext { + const actor = params.actor ?? bridgeActorFromLegacyContext("actorContext" in params ? params.actorContext : null); + const companyId = stringOrNull(params.companyId) + ?? stringOrNull("actorContext" in params && params.actorContext ? params.actorContext.companyId : null); + const renderEnvironment = params.renderEnvironment ?? null; return Object.freeze({ - actor, - companyId: actor.companyId, + actor: actor ? Object.freeze(actor) : null, + companyId, + renderEnvironment, }); } @@ -1558,7 +1561,7 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost ...(params.companyId === undefined ? {} : { companyId: params.companyId }), ...(params.renderEnvironment === undefined ? {} : { renderEnvironment: params.renderEnvironment }), }, - actionContextFromParams(params), + bridgeRequestContextFromParams(params), ); } diff --git a/packages/plugins/sdk/tests/bridge-context.test.ts b/packages/plugins/sdk/tests/bridge-context.test.ts new file mode 100644 index 00000000000..8159c1d302e --- /dev/null +++ b/packages/plugins/sdk/tests/bridge-context.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; + +import { createTestHarness } from "../src/testing.js"; +import { definePlugin } from "../src/define-plugin.js"; +import type { PaperclipPluginManifestV1 } from "../src/types.js"; + +const manifest: PaperclipPluginManifestV1 = { + apiVersion: 1, + id: "paperclip.test.bridge-context", + displayName: "Bridge Context Test", + version: "0.0.0", + description: "Tests trusted bridge request context.", + author: "Paperclip", + categories: ["ui"], + capabilities: [], + entrypoints: { + worker: "./dist/worker.js", + }, +}; + +describe("plugin bridge request context", () => { + it("makes trusted actor context available to data and action handlers", async () => { + const plugin = definePlugin({ + async setup(ctx) { + ctx.data.register("whoami", async (_params, request) => request.actor); + ctx.actions.register("whoami", async (_params, request) => request.actor); + }, + }); + const harness = createTestHarness({ manifest }); + await plugin.definition.setup?.(harness.ctx); + + const actor = { + actorType: "user" as const, + actorId: "user-1", + userId: "user-1", + agentId: null, + runId: "run-1", + source: "session", + }; + + await expect(harness.getData("whoami", {}, { actor })).resolves.toEqual(actor); + await expect(harness.performAction("whoami", {}, { actor })).resolves.toEqual(actor); + }); +}); diff --git a/packages/plugins/sdk/tests/testing-actions.test.ts b/packages/plugins/sdk/tests/testing-actions.test.ts index ab0026b283f..06ac308e52e 100644 --- a/packages/plugins/sdk/tests/testing-actions.test.ts +++ b/packages/plugins/sdk/tests/testing-actions.test.ts @@ -30,11 +30,12 @@ describe("createTestHarness action context", () => { const result = await harness.performAction<{ paramsCompanyId: unknown; actor: { - type: string; + actorType: string; + actorId: string; userId: string | null; agentId: string | null; runId: string | null; - companyId: string | null; + source: string | null; }; companyId: string | null; contextFrozen: boolean; @@ -55,11 +56,12 @@ describe("createTestHarness action context", () => { expect(result.paramsCompanyId).toBe("host-company"); expect(result.companyId).toBe("host-company"); expect(result.actor).toEqual({ - type: "user", + actorType: "user", + actorId: "board-user-1", userId: "board-user-1", agentId: null, runId: "run-1", - companyId: "host-company", + source: null, }); expect(result.contextFrozen).toBe(true); expect(result.actorFrozen).toBe(true); diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 272e20c3575..93f358ea393 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -642,6 +642,50 @@ describe.sequential("plugin tool and bridge authz", () => { key: "health", companyId: companyA, params: { view: "compact" }, + actor: { + actorType: "user", + actorId: "user-1", + userId: "user-1", + agentId: null, + runId: null, + source: "session", + }, + renderEnvironment: null, + }); + }); + + it("passes trusted actor context to plugin data handlers without trusting caller user params", async () => { + readyPlugin(); + const call = vi.fn().mockResolvedValue({ ok: true }); + const { app } = await createApp(boardActor({ + userId: "signed-in-user", + companyIds: [companyA], + }), {}, { + bridgeDeps: { + workerManager: { call }, + }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/data/page`) + .send({ + companyId: companyA, + params: { companyId: companyA, userId: "victim-user" }, + }); + + expect(res.status).toBe(200); + expect(call).toHaveBeenCalledWith(pluginId, "getData", { + key: "page", + companyId: companyA, + params: { companyId: companyA, userId: "victim-user" }, + actor: { + actorType: "user", + actorId: "signed-in-user", + userId: "signed-in-user", + agentId: null, + runId: null, + source: "session", + }, renderEnvironment: null, }); }); @@ -674,6 +718,14 @@ describe.sequential("plugin tool and bridge authz", () => { runId: null, companyId: null, }, + actor: { + actorType: "user", + actorId: "admin-1", + userId: "admin-1", + agentId: null, + runId: null, + source: "session", + }, renderEnvironment: null, }); }); @@ -711,6 +763,14 @@ describe.sequential("plugin tool and bridge authz", () => { runId: runA, companyId: companyA, }, + actor: { + actorType: "user", + actorId: "user-1", + userId: "user-1", + agentId: null, + runId: runA, + source: "session", + }, renderEnvironment: null, }); }); @@ -735,6 +795,11 @@ describe.sequential("plugin tool and bridge authz", () => { userId: null, companyId: companyA, }), + actor: expect.objectContaining({ + actorType: "user", + actorId: "board", + userId: null, + }), })); }); @@ -771,6 +836,14 @@ describe.sequential("plugin tool and bridge authz", () => { runId: runA, companyId: companyA, }, + actor: { + actorType: "agent", + actorId: agentA, + userId: null, + agentId: agentA, + runId: runA, + source: "agent_jwt", + }, renderEnvironment: null, }); @@ -800,6 +873,14 @@ describe.sequential("plugin tool and bridge authz", () => { runId: runA, companyId: companyA, }, + actor: { + actorType: "agent", + actorId: agentA, + userId: null, + agentId: agentA, + runId: runA, + source: "agent_jwt", + }, renderEnvironment: null, }); }); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index d572c90d6fa..57d1d26d865 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -55,7 +55,7 @@ import type { PluginJobStore } from "../services/plugin-job-store.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; import type { PluginStreamBus } from "../services/plugin-stream-bus.js"; import type { PluginToolDispatcher } from "../services/plugin-tool-dispatcher.js"; -import type { PluginPerformActionActorContext, ToolRunContext } from "@paperclipai/plugin-sdk"; +import type { PluginBridgeActorContext, PluginPerformActionActorContext, ToolRunContext } from "@paperclipai/plugin-sdk"; import { JsonRpcCallError, PLUGIN_RPC_ERROR_CODES } from "@paperclipai/plugin-sdk"; import { assertAuthenticated, @@ -601,6 +601,18 @@ export function pluginRoutes( }; } + function trustedPluginBridgeActor(req: Request): PluginBridgeActorContext { + const actor = getActorInfo(req); + return { + actorType: actor.actorType, + actorId: actor.actorId, + userId: req.actor.type === "board" ? req.actor.userId ?? null : null, + agentId: req.actor.type === "agent" ? req.actor.agentId ?? null : null, + runId: actor.runId, + source: req.actor.source ?? null, + }; + } + function actionParamsWithAuthorizedCompanyScope( params: Record | undefined, companyId: string | undefined, @@ -1169,6 +1181,7 @@ export function pluginRoutes( key: body.key, ...(companyId ? { companyId } : {}), params: body.params ?? {}, + actor: trustedPluginBridgeActor(req), renderEnvironment: body.renderEnvironment ?? null, }, ); @@ -1262,6 +1275,7 @@ export function pluginRoutes( key: body.key, params: actionParamsWithAuthorizedCompanyScope(body.params, companyId), actorContext: performActionActorContext(req, companyId), + actor: trustedPluginBridgeActor(req), renderEnvironment: body.renderEnvironment ?? null, }, ); @@ -1356,6 +1370,7 @@ export function pluginRoutes( key, ...(companyId ? { companyId } : {}), params: body?.params ?? {}, + actor: trustedPluginBridgeActor(req), renderEnvironment: body?.renderEnvironment ?? null, }, ); @@ -1446,6 +1461,7 @@ export function pluginRoutes( key, params: actionParamsWithAuthorizedCompanyScope(body?.params, companyId), actorContext: performActionActorContext(req, companyId), + actor: trustedPluginBridgeActor(req), renderEnvironment: body?.renderEnvironment ?? null, }, );