From 4272b311365b574a8d1d59166d81a9753c5f6f1f Mon Sep 17 00:00:00 2001 From: Paperclip Bot Date: Wed, 3 Jun 2026 06:31:01 +0000 Subject: [PATCH] feat(plugin): scope secret-ref config by company --- .../0094_plugin_config_company_scope.sql | 11 ++ packages/db/src/migrations/meta/_journal.json | 9 +- packages/db/src/schema/plugin_config.ts | 22 +++- .../plugins/sdk/src/host-client-factory.ts | 6 +- packages/plugins/sdk/src/protocol.ts | 4 +- packages/plugins/sdk/src/types.ts | 4 +- packages/plugins/sdk/src/worker-rpc-host.ts | 94 ++++++++++---- .../plugins/sdk/tests/worker-rpc-host.test.ts | 121 ++++++++++++++++++ packages/shared/src/types/plugin.ts | 6 +- .../src/__tests__/plugin-routes-authz.test.ts | 108 +++++++++++++++- .../plugin-secrets-handler-runtime.test.ts | 105 +++++++++++++++ .../__tests__/plugin-secrets-handler.test.ts | 6 +- server/src/routes/plugins.ts | 55 ++++++-- server/src/services/plugin-host-services.ts | 6 +- server/src/services/plugin-registry.ts | 37 ++++-- server/src/services/plugin-secrets-handler.ts | 64 ++++++--- ui/src/api/plugins.ts | 10 +- ui/src/lib/queryKeys.ts | 3 +- ui/src/pages/PluginSettings.tsx | 18 +-- 19 files changed, 587 insertions(+), 102 deletions(-) create mode 100644 packages/db/src/migrations/0094_plugin_config_company_scope.sql create mode 100644 server/src/__tests__/plugin-secrets-handler-runtime.test.ts diff --git a/packages/db/src/migrations/0094_plugin_config_company_scope.sql b/packages/db/src/migrations/0094_plugin_config_company_scope.sql new file mode 100644 index 00000000..381fcb7f --- /dev/null +++ b/packages/db/src/migrations/0094_plugin_config_company_scope.sql @@ -0,0 +1,11 @@ +ALTER TABLE "plugin_config" ADD COLUMN IF NOT EXISTS "company_id" uuid;--> statement-breakpoint +DO $$ BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'plugin_config_company_id_companies_id_fk') THEN + ALTER TABLE "plugin_config" ADD CONSTRAINT "plugin_config_company_id_companies_id_fk" FOREIGN KEY ("company_id") REFERENCES "public"."companies"("id") ON DELETE cascade ON UPDATE no action; + END IF; +END $$;--> statement-breakpoint +DROP INDEX IF EXISTS "plugin_config_plugin_id_idx";--> statement-breakpoint +CREATE INDEX IF NOT EXISTS "plugin_config_plugin_id_idx" ON "plugin_config" USING btree ("plugin_id");--> statement-breakpoint +CREATE INDEX IF NOT EXISTS "plugin_config_company_id_idx" ON "plugin_config" USING btree ("company_id");--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "plugin_config_legacy_plugin_id_uq" ON "plugin_config" USING btree ("plugin_id") WHERE "plugin_config"."company_id" is null;--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "plugin_config_company_plugin_uq" ON "plugin_config" USING btree ("plugin_id","company_id") WHERE "plugin_config"."company_id" is not null; diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 991a61ce..48546a4d 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -659,6 +659,13 @@ "when": 1780040470886, "tag": "0093_giant_green_goblin", "breakpoints": true + }, + { + "idx": 94, + "version": "7", + "when": 1780444800000, + "tag": "0094_plugin_config_company_scope", + "breakpoints": true } ] -} \ No newline at end of file +} diff --git a/packages/db/src/schema/plugin_config.ts b/packages/db/src/schema/plugin_config.ts index 24407b97..66144689 100644 --- a/packages/db/src/schema/plugin_config.ts +++ b/packages/db/src/schema/plugin_config.ts @@ -1,10 +1,13 @@ -import { pgTable, uuid, text, timestamp, jsonb, uniqueIndex } from "drizzle-orm/pg-core"; +import { sql } from "drizzle-orm"; +import { pgTable, uuid, text, timestamp, jsonb, uniqueIndex, index } from "drizzle-orm/pg-core"; +import { companies } from "./companies.js"; import { plugins } from "./plugins.js"; /** - * `plugin_config` table — stores operator-provided instance configuration - * for each plugin (one row per plugin, enforced by a unique index on - * `plugin_id`). + * `plugin_config` table — stores operator-provided plugin configuration. + * + * New configuration is company-scoped. Legacy rows may still have a null + * `company_id` so existing installs keep working until re-saved. * * The `config_json` column holds the values that the operator enters in the * plugin settings UI. These values are validated at runtime against the @@ -19,12 +22,21 @@ export const pluginConfig = pgTable( pluginId: uuid("plugin_id") .notNull() .references(() => plugins.id, { onDelete: "cascade" }), + companyId: uuid("company_id") + .references(() => companies.id, { onDelete: "cascade" }), configJson: jsonb("config_json").$type>().notNull().default({}), lastError: text("last_error"), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(), }, (table) => ({ - pluginIdIdx: uniqueIndex("plugin_config_plugin_id_idx").on(table.pluginId), + pluginIdIdx: index("plugin_config_plugin_id_idx").on(table.pluginId), + companyIdIdx: index("plugin_config_company_id_idx").on(table.companyId), + legacyPluginIdUq: uniqueIndex("plugin_config_legacy_plugin_id_uq") + .on(table.pluginId) + .where(sql`${table.companyId} is null`), + companyPluginUq: uniqueIndex("plugin_config_company_plugin_uq") + .on(table.pluginId, table.companyId) + .where(sql`${table.companyId} is not null`), }), ); diff --git a/packages/plugins/sdk/src/host-client-factory.ts b/packages/plugins/sdk/src/host-client-factory.ts index 8fc8fdfa..ec58bf4e 100644 --- a/packages/plugins/sdk/src/host-client-factory.ts +++ b/packages/plugins/sdk/src/host-client-factory.ts @@ -100,7 +100,7 @@ export class InvocationScopeDeniedError extends Error { export interface HostServices { /** Provides `config.get`. */ config: { - get(): Promise>; + get(params: WorkerToHostMethods["config.get"][0]): Promise>; }; /** Provides trusted company-scoped local folder helpers. */ @@ -627,8 +627,8 @@ export function createHostClientHandlers( return { // Config - "config.get": gated("config.get", async () => { - return services.config.get(); + "config.get": gated("config.get", async (params) => { + return services.config.get(params); }), "localFolders.declarations": gated("localFolders.declarations", async (params) => { diff --git a/packages/plugins/sdk/src/protocol.ts b/packages/plugins/sdk/src/protocol.ts index 92fcbf42..4ea8044f 100644 --- a/packages/plugins/sdk/src/protocol.ts +++ b/packages/plugins/sdk/src/protocol.ts @@ -672,7 +672,7 @@ export const HOST_TO_WORKER_OPTIONAL_METHODS: readonly HostToWorkerMethodName[] */ export interface WorkerToHostMethods { // Config - "config.get": [params: Record, result: Record]; + "config.get": [params: { companyId?: string | null }, result: Record]; // Trusted local folders "localFolders.declarations": [ @@ -809,7 +809,7 @@ export interface WorkerToHostMethods { // Secrets "secrets.resolve": [ - params: { secretRef: string }, + params: { secretRef: string; companyId?: string | null }, result: string, ]; diff --git a/packages/plugins/sdk/src/types.ts b/packages/plugins/sdk/src/types.ts index 90af99b4..2ad6361e 100644 --- a/packages/plugins/sdk/src/types.ts +++ b/packages/plugins/sdk/src/types.ts @@ -425,7 +425,7 @@ export interface PluginConfigClient { * Values are validated against the plugin's `instanceConfigSchema` by the * host before being passed to the worker. */ - get(): Promise>; + get(params?: { companyId?: string | null }): Promise>; } export interface PluginLocalFolderProblem { @@ -656,7 +656,7 @@ export interface PluginSecretsClient { * @param secretRef - The secret reference string from plugin config * @returns The resolved secret value */ - resolve(secretRef: string): Promise; + resolve(secretRef: string, companyId?: string | null): Promise; } /** diff --git a/packages/plugins/sdk/src/worker-rpc-host.ts b/packages/plugins/sdk/src/worker-rpc-host.ts index a135d1f0..727441e3 100644 --- a/packages/plugins/sdk/src/worker-rpc-host.ts +++ b/packages/plugins/sdk/src/worker-rpc-host.ts @@ -164,6 +164,10 @@ export interface WorkerRpcHost { stop(): void; } +interface RuntimeCompanyContext { + companyId?: string | null; +} + // --------------------------------------------------------------------------- // Internal: event registration // --------------------------------------------------------------------------- @@ -285,6 +289,7 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost let currentConfig: Record = {}; let databaseNamespace: string | null = null; const invocationContextStorage = new AsyncLocalStorage(); + const runtimeCompanyContext = new AsyncLocalStorage(); // Plugin handler registrations (populated during setup()) const eventHandlers: EventRegistration[] = []; @@ -413,8 +418,10 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost }, config: { - async get() { - return callHost("config.get", {} as Record); + async get(params) { + const companyId = + params?.companyId ?? runtimeCompanyContext.getStore()?.companyId ?? null; + return callHost("config.get", companyId ? { companyId } : {}); }, }, @@ -564,8 +571,9 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost }, secrets: { - async resolve(secretRef: string): Promise { - return callHost("secrets.resolve", { secretRef }); + async resolve(secretRef: string, companyId?: string | null): Promise { + const scopedCompanyId = companyId ?? runtimeCompanyContext.getStore()?.companyId ?? null; + return callHost("secrets.resolve", { secretRef, companyId: scopedCompanyId }); }, }, @@ -1467,7 +1475,10 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost if (registration.filter && !allowsEvent(registration.filter, event)) continue; try { - await registration.fn(event); + await runtimeCompanyContext.run( + { companyId: event.companyId }, + () => registration.fn(event), + ); } catch (err) { // Log error but continue processing other handlers so one failing // handler doesn't prevent the rest from running. @@ -1507,7 +1518,10 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost { code: PLUGIN_RPC_ERROR_CODES.METHOD_NOT_IMPLEMENTED }, ); } - return plugin.definition.onApiRequest(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onApiRequest!(params), + ); } async function handleGetData(params: GetDataParams): Promise { @@ -1515,11 +1529,14 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost if (!handler) { throw new Error(`No data handler registered for key "${params.key}"`); } - return handler({ - ...params.params, - ...(params.companyId === undefined ? {} : { companyId: params.companyId }), - ...(params.renderEnvironment === undefined ? {} : { renderEnvironment: params.renderEnvironment }), - }); + const handlerParams = + params.renderEnvironment === undefined + ? params.params + : { ...params.params, renderEnvironment: params.renderEnvironment }; + return runtimeCompanyContext.run( + { companyId: params.companyId ?? null }, + () => handler(handlerParams), + ); } function stringOrNull(value: unknown): string | null { @@ -1552,13 +1569,14 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost if (!handler) { throw new Error(`No action handler registered for key "${params.key}"`); } - return handler( - { - ...params.params, - ...(params.companyId === undefined ? {} : { companyId: params.companyId }), - ...(params.renderEnvironment === undefined ? {} : { renderEnvironment: params.renderEnvironment }), - }, - actionContextFromParams(params), + const handlerParams = + params.renderEnvironment === undefined + ? params.params + : { ...params.params, renderEnvironment: params.renderEnvironment }; + const actionContext = actionContextFromParams(params); + return runtimeCompanyContext.run( + { companyId: params.companyId ?? actionContext.companyId }, + () => handler(handlerParams ?? {}, actionContext), ); } @@ -1567,7 +1585,10 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost if (!entry) { throw new Error(`No tool handler registered for "${params.toolName}"`); } - return entry.fn(params.parameters, params.runContext); + return runtimeCompanyContext.run( + { companyId: params.runContext.companyId }, + () => entry.fn(params.parameters, params.runContext), + ); } function methodNotImplemented(method: string): Error & { code: number } { @@ -1590,49 +1611,70 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost if (!plugin.definition.onEnvironmentProbe) { throw methodNotImplemented("environmentProbe"); } - return plugin.definition.onEnvironmentProbe(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentProbe!(params), + ); } async function handleEnvironmentAcquireLease(params: PluginEnvironmentAcquireLeaseParams) { if (!plugin.definition.onEnvironmentAcquireLease) { throw methodNotImplemented("environmentAcquireLease"); } - return plugin.definition.onEnvironmentAcquireLease(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentAcquireLease!(params), + ); } async function handleEnvironmentResumeLease(params: PluginEnvironmentResumeLeaseParams) { if (!plugin.definition.onEnvironmentResumeLease) { throw methodNotImplemented("environmentResumeLease"); } - return plugin.definition.onEnvironmentResumeLease(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentResumeLease!(params), + ); } async function handleEnvironmentReleaseLease(params: PluginEnvironmentReleaseLeaseParams) { if (!plugin.definition.onEnvironmentReleaseLease) { throw methodNotImplemented("environmentReleaseLease"); } - return plugin.definition.onEnvironmentReleaseLease(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentReleaseLease!(params), + ); } async function handleEnvironmentDestroyLease(params: PluginEnvironmentDestroyLeaseParams) { if (!plugin.definition.onEnvironmentDestroyLease) { throw methodNotImplemented("environmentDestroyLease"); } - return plugin.definition.onEnvironmentDestroyLease(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentDestroyLease!(params), + ); } async function handleEnvironmentRealizeWorkspace(params: PluginEnvironmentRealizeWorkspaceParams) { if (!plugin.definition.onEnvironmentRealizeWorkspace) { throw methodNotImplemented("environmentRealizeWorkspace"); } - return plugin.definition.onEnvironmentRealizeWorkspace(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentRealizeWorkspace!(params), + ); } async function handleEnvironmentExecute(params: PluginEnvironmentExecuteParams) { if (!plugin.definition.onEnvironmentExecute) { throw methodNotImplemented("environmentExecute"); } - return plugin.definition.onEnvironmentExecute(params); + return runtimeCompanyContext.run( + { companyId: params.companyId }, + () => plugin.definition.onEnvironmentExecute!(params), + ); } // ----------------------------------------------------------------------- diff --git a/packages/plugins/sdk/tests/worker-rpc-host.test.ts b/packages/plugins/sdk/tests/worker-rpc-host.test.ts index d41aa863..1ed698b1 100644 --- a/packages/plugins/sdk/tests/worker-rpc-host.test.ts +++ b/packages/plugins/sdk/tests/worker-rpc-host.test.ts @@ -296,3 +296,124 @@ describe("worker invocation scope propagation", () => { } }); }); + +describe("startWorkerRpcHost runtime company context", () => { + function collectJsonLines(stream: PassThrough) { + const queue: unknown[] = []; + const waiters: Array<(value: unknown) => void> = []; + let buffer = ""; + + stream.on("data", (chunk) => { + buffer += chunk.toString("utf8"); + let newlineIndex = buffer.indexOf("\n"); + while (newlineIndex !== -1) { + const line = buffer.slice(0, newlineIndex).trim(); + buffer = buffer.slice(newlineIndex + 1); + if (line) { + const message = JSON.parse(line); + const waiter = waiters.shift(); + if (waiter) waiter(message); + else queue.push(message); + } + newlineIndex = buffer.indexOf("\n"); + } + }); + + return async function nextMessage(): Promise { + const queued = queue.shift(); + if (queued) return queued; + return new Promise((resolve) => waiters.push(resolve)); + }; + } + + function writeMessage(stream: PassThrough, message: unknown): void { + stream.write(`${JSON.stringify(message)}\n`); + } + + it("passes executeTool company context into config and secret host calls", async () => { + const stdin = new PassThrough(); + const stdout = new PassThrough(); + const nextMessage = collectJsonLines(stdout); + + const plugin = definePlugin({ + async setup(ctx) { + ctx.tools.register( + "check-context", + { + displayName: "Check Context", + description: "Checks runtime context propagation", + parametersSchema: { type: "object", properties: {} }, + }, + async () => { + const config = await ctx.config.get(); + const token = await ctx.secrets.resolve("77777777-7777-4777-8777-777777777777"); + return { content: `${config.mode}:${token}` }; + }, + ); + }, + }); + + const host = startWorkerRpcHost({ plugin, stdin, stdout }); + + writeMessage(stdin, { + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + manifest: { id: "test-plugin", name: "test-plugin", version: "1.0.0" }, + config: {}, + instanceInfo: { instanceId: "inst-1", hostVersion: "0.0.0-test" }, + apiVersion: 1, + }, + }); + await expect(nextMessage()).resolves.toMatchObject({ id: 1, result: { ok: true } }); + + writeMessage(stdin, { + jsonrpc: "2.0", + id: 2, + method: "executeTool", + params: { + toolName: "check-context", + parameters: {}, + runContext: { + agentId: "agent-1", + runId: "run-1", + companyId: "company-1", + projectId: "project-1", + }, + }, + }); + + const configRequest = await nextMessage(); + expect(configRequest).toMatchObject({ + method: "config.get", + params: { companyId: "company-1" }, + }); + writeMessage(stdin, { + jsonrpc: "2.0", + id: configRequest.id, + result: { mode: "company-config" }, + }); + + const secretRequest = await nextMessage(); + expect(secretRequest).toMatchObject({ + method: "secrets.resolve", + params: { + secretRef: "77777777-7777-4777-8777-777777777777", + companyId: "company-1", + }, + }); + writeMessage(stdin, { + jsonrpc: "2.0", + id: secretRequest.id, + result: "company-secret", + }); + + await expect(nextMessage()).resolves.toMatchObject({ + id: 2, + result: { content: "company-config:company-secret" }, + }); + + host.stop(); + }); +}); diff --git a/packages/shared/src/types/plugin.ts b/packages/shared/src/types/plugin.ts index 351b2dda..1086eac1 100644 --- a/packages/shared/src/types/plugin.ts +++ b/packages/shared/src/types/plugin.ts @@ -678,15 +678,17 @@ export interface PluginStateRecord { // --------------------------------------------------------------------------- /** - * Domain type for a plugin's instance configuration as persisted in the + * Domain type for a plugin's configuration as persisted in the * `plugin_config` table. * See PLUGIN_SPEC.md §21.3 for the schema definition. */ export interface PluginConfig { /** UUID primary key. */ id: string; - /** FK to `plugins.id`. Unique — each plugin has at most one config row. */ + /** FK to `plugins.id`. */ pluginId: string; + /** Company scope for this config row. Null only for legacy/global fallback rows. */ + companyId: string | null; /** Operator-provided configuration values (validated against `instanceConfigSchema`). */ configJson: Record; /** Most recent config validation error, if any. */ diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 90371991..28921811 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -8,6 +8,11 @@ const mockRegistry = vi.hoisted(() => ({ upsertConfig: vi.fn(), getCompanySettings: vi.fn(), upsertCompanySettings: vi.fn(), + getConfig: vi.fn(), +})); + +const mockSecrets = vi.hoisted(() => ({ + syncSecretRefsForTarget: vi.fn(), })); const mockLifecycle = vi.hoisted(() => ({ @@ -22,6 +27,10 @@ vi.mock("../services/plugin-registry.js", () => ({ pluginRegistryService: () => mockRegistry, })); +vi.mock("../services/secrets.js", () => ({ + secretService: () => mockSecrets, +})); + vi.mock("../services/plugin-lifecycle.js", () => ({ pluginLifecycleManager: () => mockLifecycle, })); @@ -327,8 +336,105 @@ describe.sequential("plugin install and upgrade authz", () => { }); expect(res.status).toBe(422); - expect(res.body.error).toMatch(/secret references are disabled/i); + expect(res.body.error).toMatch(/secret references require companyId/i); expect(mockRegistry.upsertConfig).not.toHaveBeenCalled(); + expect(mockSecrets.syncSecretRefsForTarget).not.toHaveBeenCalled(); + }, 20_000); + + it("saves company-scoped plugin config secret refs as plugin bindings", async () => { + readyPlugin(); + mockRegistry.upsertConfig.mockResolvedValue({ + id: "99999999-9999-4999-8999-999999999999", + pluginId, + companyId: companyA, + configJson: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + lastError: null, + createdAt: new Date(), + updatedAt: new Date(), + }); + + const { app } = await createApp(boardActor({ + isInstanceAdmin: true, + companyIds: [companyA], + })); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/config`) + .send({ + companyId: companyA, + configJson: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + }); + + expect(res.status).toBe(200); + expect(mockSecrets.syncSecretRefsForTarget).toHaveBeenCalledWith( + companyA, + { targetType: "plugin", targetId: pluginId }, + [{ + secretId: "77777777-7777-4777-8777-777777777777", + configPath: "$", + }], + ); + expect(mockRegistry.upsertConfig).toHaveBeenCalledWith( + pluginId, + { + configJson: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + }, + companyA, + ); + }, 20_000); + + it("rejects company-scoped plugin config secret refs that do not belong to the selected company", async () => { + readyPlugin(); + mockSecrets.syncSecretRefsForTarget.mockRejectedValueOnce(new Error("Secret not found")); + + const { app } = await createApp(boardActor({ + isInstanceAdmin: true, + companyIds: [companyA], + })); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/config`) + .send({ + companyId: companyA, + configJson: { + apiKeyRef: "88888888-8888-4888-8888-888888888888", + }, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/secret not found/i); + expect(mockRegistry.upsertConfig).not.toHaveBeenCalled(); + }, 20_000); + + it("reads plugin config from the requested company scope", async () => { + readyPlugin(); + mockRegistry.getConfig.mockResolvedValue({ + id: "99999999-9999-4999-8999-999999999999", + pluginId, + companyId: companyA, + configJson: { botName: "company-a" }, + lastError: null, + createdAt: new Date(), + updatedAt: new Date(), + }); + + const { app } = await createApp(boardActor({ + isInstanceAdmin: true, + companyIds: [companyA, companyB], + })); + + const res = await request(app) + .get(`/api/plugins/${pluginId}/config?companyId=${companyA}`); + + expect(res.status).toBe(200); + expect(mockRegistry.getConfig).toHaveBeenCalledWith(pluginId, companyA); + expect(res.body.configJson).toEqual({ botName: "company-a" }); }, 20_000); it("allows instance admins to upgrade plugins", async () => { diff --git a/server/src/__tests__/plugin-secrets-handler-runtime.test.ts b/server/src/__tests__/plugin-secrets-handler-runtime.test.ts new file mode 100644 index 00000000..da152656 --- /dev/null +++ b/server/src/__tests__/plugin-secrets-handler-runtime.test.ts @@ -0,0 +1,105 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + getConfig: vi.fn(), + resolveSecretValue: vi.fn(), +})); + +vi.mock("../services/plugin-registry.js", () => ({ + pluginRegistryService: () => ({ + getConfig: mocks.getConfig, + }), +})); + +vi.mock("../services/secrets.js", () => ({ + secretService: () => ({ + resolveSecretValue: mocks.resolveSecretValue, + }), +})); + +import { + createPluginSecretsHandler, + PLUGIN_SECRET_REFS_REQUIRE_COMPANY_MESSAGE, +} from "../services/plugin-secrets-handler.js"; + +const pluginId = "11111111-1111-4111-8111-111111111111"; +const companyId = "22222222-2222-4222-8222-222222222222"; +const secretRef = "77777777-7777-4777-8777-777777777777"; + +const manifest = { + instanceConfigSchema: { + type: "object", + properties: { + apiKeyRef: { + type: "string", + format: "secret-ref", + }, + }, + }, +}; + +describe("createPluginSecretsHandler runtime company scoping", () => { + beforeEach(() => { + mocks.getConfig.mockReset(); + mocks.resolveSecretValue.mockReset(); + }); + + it("fails closed when the runtime call has no company context", async () => { + const handler = createPluginSecretsHandler({ + db: {} as never, + pluginId, + manifest: manifest as never, + }); + + await expect(handler.resolve({ secretRef })).rejects.toThrow( + PLUGIN_SECRET_REFS_REQUIRE_COMPANY_MESSAGE, + ); + expect(mocks.getConfig).not.toHaveBeenCalled(); + expect(mocks.resolveSecretValue).not.toHaveBeenCalled(); + }); + + it("rejects a secret ref that is not referenced by that company's plugin config", async () => { + mocks.getConfig.mockResolvedValue({ + configJson: { + apiKeyRef: "88888888-8888-4888-8888-888888888888", + }, + }); + + const handler = createPluginSecretsHandler({ + db: {} as never, + pluginId, + manifest: manifest as never, + }); + + await expect(handler.resolve({ secretRef, companyId })).rejects.toThrow( + /not referenced by this company's plugin config/i, + ); + expect(mocks.getConfig).toHaveBeenCalledWith(pluginId, companyId); + expect(mocks.resolveSecretValue).not.toHaveBeenCalled(); + }); + + it("resolves only through the company plugin binding context from saved config", async () => { + mocks.getConfig.mockResolvedValue({ + configJson: { + apiKeyRef: secretRef, + }, + }); + mocks.resolveSecretValue.mockResolvedValue("plaintext-token"); + + const handler = createPluginSecretsHandler({ + db: {} as never, + pluginId, + manifest: manifest as never, + }); + + await expect(handler.resolve({ secretRef, companyId })).resolves.toBe("plaintext-token"); + expect(mocks.resolveSecretValue).toHaveBeenCalledWith(companyId, secretRef, "latest", { + consumerType: "plugin", + consumerId: pluginId, + configPath: "apiKeyRef", + actorType: "plugin", + actorId: pluginId, + pluginId, + }); + }); +}); diff --git a/server/src/__tests__/plugin-secrets-handler.test.ts b/server/src/__tests__/plugin-secrets-handler.test.ts index ec89c872..fd0d61bd 100644 --- a/server/src/__tests__/plugin-secrets-handler.test.ts +++ b/server/src/__tests__/plugin-secrets-handler.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from "vitest"; import { createPluginSecretsHandler, - PLUGIN_SECRET_REFS_DISABLED_MESSAGE, + PLUGIN_SECRET_REFS_REQUIRE_COMPANY_MESSAGE, } from "../services/plugin-secrets-handler.js"; describe("createPluginSecretsHandler", () => { - it("fails closed for plugin secret resolution until company scoping lands", async () => { + it("fails closed for plugin secret resolution without company scope", async () => { const handler = createPluginSecretsHandler({ db: {} as never, pluginId: "11111111-1111-4111-8111-111111111111", @@ -13,7 +13,7 @@ describe("createPluginSecretsHandler", () => { await expect( handler.resolve({ secretRef: "77777777-7777-4777-8777-777777777777" }), - ).rejects.toThrow(PLUGIN_SECRET_REFS_DISABLED_MESSAGE); + ).rejects.toThrow(PLUGIN_SECRET_REFS_REQUIRE_COMPANY_MESSAGE); }); it("still rejects malformed secret refs before the feature-disable guard", async () => { diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index c3f6265f..c2d30cc3 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -75,8 +75,8 @@ import { } from "../services/plugin-local-folders.js"; import { extractSecretRefPathsFromConfig, - PLUGIN_SECRET_REFS_DISABLED_MESSAGE, } from "../services/plugin-secrets-handler.js"; +import { secretService } from "../services/secrets.js"; import { badRequest, forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; /** UI slot declaration extracted from plugin manifest */ @@ -481,6 +481,7 @@ export function pluginRoutes( ) { const router = Router(); const registry = pluginRegistryService(db); + const secrets = secretService(db); const lifecycle = pluginLifecycleManager(db, { loader, workerManager: bridgeDeps?.workerManager ?? webhookDeps?.workerManager, @@ -669,6 +670,27 @@ export function pluginRoutes( }))); } + function resolvePluginConfigCompanyId(req: Request): string | null { + const body = req.body as { companyId?: unknown } | undefined; + const rawCompanyId = typeof req.query.companyId === "string" + ? req.query.companyId + : typeof body?.companyId === "string" + ? body.companyId + : null; + + if (rawCompanyId) { + if ( + req.actor.type !== "board" || + (req.actor.source !== "local_implicit" && !req.actor.isInstanceAdmin) + ) { + assertCompanyAccess(req, rawCompanyId); + } + return rawCompanyId; + } + + return null; + } + function assertPluginBridgeScope(req: Request, companyId: unknown): string | undefined { if (companyId === undefined || companyId === null) { assertInstanceAdmin(req); @@ -2111,7 +2133,8 @@ export function pluginRoutes( return; } - const config = await registry.getConfig(plugin.id); + const companyId = resolvePluginConfigCompanyId(req); + const config = await registry.getConfig(plugin.id, companyId); res.json(config); }); @@ -2141,11 +2164,12 @@ export function pluginRoutes( return; } - const body = req.body as { configJson?: Record } | undefined; + const body = req.body as { configJson?: Record; companyId?: string } | undefined; if (!body?.configJson || typeof body.configJson !== "object") { res.status(400).json({ error: '"configJson" is required and must be an object' }); return; } + const companyId = resolvePluginConfigCompanyId(req); // Strip devUiUrl unless the caller is an instance admin. devUiUrl activates // a dev-proxy in the static file route that could be abused for SSRF if any @@ -2173,25 +2197,34 @@ export function pluginRoutes( try { const secretRefsByPath = extractSecretRefPathsFromConfig(body.configJson, schema); - if (secretRefsByPath.size > 0) { - res.status(422).json({ error: PLUGIN_SECRET_REFS_DISABLED_MESSAGE }); + if (secretRefsByPath.size > 0 && !companyId) { + res.status(422).json({ error: "Plugin secret references require companyId" }); return; } + if (companyId) { + const refs = [...secretRefsByPath.entries()].flatMap(([secretId, paths]) => + [...paths].map((configPath) => ({ secretId, configPath })), + ); + await secrets.syncSecretRefsForTarget( + companyId, + { targetType: "plugin", targetId: plugin.id }, + refs, + ); + } const result = await registry.upsertConfig(plugin.id, { configJson: body.configJson, - }); + }, companyId); await logPluginMutationActivity(req, "plugin.config.updated", plugin.id, { pluginId: plugin.id, pluginKey: plugin.pluginKey, + companyId, configKeyCount: Object.keys(body.configJson).length, }); - // Notify the running worker about the config change (PLUGIN_SPEC §25.4.4). - // If the worker implements onConfigChanged, send the new config via RPC. - // If it doesn't (METHOD_NOT_IMPLEMENTED), restart the worker so it picks - // up the new config on re-initialize. If no worker is running, skip. - if (bridgeDeps?.workerManager.isRunning(plugin.id)) { + // Only legacy/global config is still pushed into the process-global worker state. + // Company-scoped config is read at call time through ctx.config.get(). + if (companyId === null && bridgeDeps?.workerManager.isRunning(plugin.id)) { try { await bridgeDeps.workerManager.call( plugin.id, diff --git a/server/src/services/plugin-host-services.ts b/server/src/services/plugin-host-services.ts index 7c2a9156..4ea90454 100644 --- a/server/src/services/plugin-host-services.ts +++ b/server/src/services/plugin-host-services.ts @@ -489,7 +489,7 @@ export function buildHostServices( const registry = pluginRegistryService(db); const stateStore = pluginStateStore(db); const pluginDb = pluginDatabaseService(db); - const secretsHandler = createPluginSecretsHandler({ db, pluginId }); + const secretsHandler = createPluginSecretsHandler({ db, pluginId, manifest: options.manifest }); const companies = companyService(db); const agents = agentService(db); const managedAgents = pluginManagedAgentService(db, { @@ -1053,8 +1053,8 @@ export function buildHostServices( return { config: { - async get() { - const configRow = await registry.getConfig(pluginId); + async get(params) { + const configRow = await registry.getConfig(pluginId, params?.companyId ?? null); return configRow?.configJson ?? {}; }, }, diff --git a/server/src/services/plugin-registry.ts b/server/src/services/plugin-registry.ts index ce544a9c..e2150301 100644 --- a/server/src/services/plugin-registry.ts +++ b/server/src/services/plugin-registry.ts @@ -1,4 +1,4 @@ -import { asc, eq, ne, sql, and } from "drizzle-orm"; +import { asc, eq, ne, sql, and, isNull } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { plugins, @@ -44,6 +44,13 @@ function isPluginKeyConflict(error: unknown): boolean { return err.code === "23505" && constraint === "plugins_plugin_key_idx"; } +function pluginConfigScopeCondition(pluginId: string, companyId?: string | null) { + return and( + eq(pluginConfig.pluginId, pluginId), + companyId ? eq(pluginConfig.companyId, companyId) : isNull(pluginConfig.companyId), + ); +} + // --------------------------------------------------------------------------- // Service // --------------------------------------------------------------------------- @@ -280,12 +287,12 @@ export function pluginRegistryService(db: Db) { // ----- Config --------------------------------------------------------- - /** Retrieve a plugin's instance configuration. */ - getConfig: (pluginId: string) => + /** Retrieve a plugin's company-scoped config, or the legacy global fallback. */ + getConfig: (pluginId: string, companyId?: string | null) => db .select() .from(pluginConfig) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .then((rows) => rows[0] ?? null), /** @@ -293,14 +300,14 @@ export function pluginRegistryService(db: Db) { * If a config row already exists for the plugin it is replaced; * otherwise a new row is inserted. */ - upsertConfig: async (pluginId: string, input: UpsertPluginConfig) => { + upsertConfig: async (pluginId: string, input: UpsertPluginConfig, companyId?: string | null) => { const plugin = await getById(pluginId); if (!plugin) throw notFound("Plugin not found"); const existing = await db .select() .from(pluginConfig) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .then((rows) => rows[0] ?? null); if (existing) { @@ -311,7 +318,7 @@ export function pluginRegistryService(db: Db) { lastError: null, updatedAt: new Date(), }) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .returning() .then((rows) => rows[0]); } @@ -320,6 +327,7 @@ export function pluginRegistryService(db: Db) { .insert(pluginConfig) .values({ pluginId, + companyId: companyId ?? null, configJson: input.configJson, }) .returning() @@ -330,14 +338,14 @@ export function pluginRegistryService(db: Db) { * Partially update a plugin's instance configuration via shallow merge. * If no config row exists yet one is created with the supplied values. */ - patchConfig: async (pluginId: string, input: PatchPluginConfig) => { + patchConfig: async (pluginId: string, input: PatchPluginConfig, companyId?: string | null) => { const plugin = await getById(pluginId); if (!plugin) throw notFound("Plugin not found"); const existing = await db .select() .from(pluginConfig) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .then((rows) => rows[0] ?? null); if (existing) { @@ -349,7 +357,7 @@ export function pluginRegistryService(db: Db) { lastError: null, updatedAt: new Date(), }) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .returning() .then((rows) => rows[0]); } @@ -358,6 +366,7 @@ export function pluginRegistryService(db: Db) { .insert(pluginConfig) .values({ pluginId, + companyId: companyId ?? null, configJson: input.configJson, }) .returning() @@ -368,11 +377,11 @@ export function pluginRegistryService(db: Db) { * Record an error against a plugin's config (e.g. validation failure * against the plugin's instanceConfigSchema). */ - setConfigError: async (pluginId: string, lastError: string | null) => { + setConfigError: async (pluginId: string, lastError: string | null, companyId?: string | null) => { const rows = await db .update(pluginConfig) .set({ lastError, updatedAt: new Date() }) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .returning(); if (rows.length === 0) throw notFound("Plugin config not found"); @@ -380,10 +389,10 @@ export function pluginRegistryService(db: Db) { }, /** Delete a plugin's config row. */ - deleteConfig: async (pluginId: string) => { + deleteConfig: async (pluginId: string, companyId?: string | null) => { const rows = await db .delete(pluginConfig) - .where(eq(pluginConfig.pluginId, pluginId)) + .where(pluginConfigScopeCondition(pluginId, companyId)) .returning(); return rows[0] ?? null; diff --git a/server/src/services/plugin-secrets-handler.ts b/server/src/services/plugin-secrets-handler.ts index ccc5878a..cadb33c7 100644 --- a/server/src/services/plugin-secrets-handler.ts +++ b/server/src/services/plugin-secrets-handler.ts @@ -34,14 +34,17 @@ */ import type { Db } from "@paperclipai/db"; +import type { PaperclipPluginManifestV1 } from "@paperclipai/shared"; import { collectSecretRefPaths, isUuidSecretRef, readConfigValueAtPath, } from "./json-schema-secret-refs.js"; +import { pluginRegistryService } from "./plugin-registry.js"; +import { secretService } from "./secrets.js"; -export const PLUGIN_SECRET_REFS_DISABLED_MESSAGE = - "Plugin secret references are disabled until company-scoped plugin config lands"; +export const PLUGIN_SECRET_REFS_REQUIRE_COMPANY_MESSAGE = + "Plugin secret references require an active company-scoped runtime context"; // --------------------------------------------------------------------------- // Error helpers @@ -125,6 +128,8 @@ export function extractSecretRefPathsFromConfig( export interface PluginSecretsResolveParams { /** The secret reference string (a secret UUID). */ secretRef: string; + /** The company whose scoped plugin config is active for this invocation. */ + companyId?: string | null; } /** @@ -139,6 +144,8 @@ export interface PluginSecretsHandlerOptions { * that reach the plugin worker. */ pluginId: string; + /** Plugin manifest, used to extract schema-declared secret-ref paths. */ + manifest?: PaperclipPluginManifestV1 | null; } /** @@ -199,24 +206,17 @@ function createRateLimiter(maxAttempts: number, windowMs: number) { export function createPluginSecretsHandler( options: PluginSecretsHandlerOptions, ): PluginSecretsService { - const { pluginId } = options; + const { pluginId, manifest } = options; + const registry = pluginRegistryService(options.db); + const secrets = secretService(options.db); - // Rate limit: max 30 resolution attempts per plugin per minute + // Rate limit: max 30 resolution attempts per plugin+company per minute. const rateLimiter = createRateLimiter(30, 60_000); return { async resolve(params: PluginSecretsResolveParams): Promise { const { secretRef } = params; - // --------------------------------------------------------------- - // 0. Rate limiting — prevent brute-force UUID enumeration - // --------------------------------------------------------------- - if (!rateLimiter.check(pluginId)) { - const err = new Error("Rate limit exceeded for secret resolution"); - err.name = "RateLimitExceededError"; - throw err; - } - // --------------------------------------------------------------- // 1. Validate the ref format // --------------------------------------------------------------- @@ -230,9 +230,41 @@ export function createPluginSecretsHandler( throw invalidSecretRef(trimmedRef); } - // Fail closed until plugin config and worker runtime both carry an - // explicit company scope for secret bindings and resolution. - throw new Error(PLUGIN_SECRET_REFS_DISABLED_MESSAGE); + const companyId = typeof params.companyId === "string" ? params.companyId.trim() : ""; + const rateLimitKey = `${pluginId}:${companyId || "__no_company__"}`; + if (!rateLimiter.check(rateLimitKey)) { + const err = new Error("Rate limit exceeded for secret resolution"); + err.name = "RateLimitExceededError"; + throw err; + } + + if (!companyId) { + throw new Error(PLUGIN_SECRET_REFS_REQUIRE_COMPANY_MESSAGE); + } + + const configRow = await registry.getConfig(pluginId, companyId); + const refsBySecret = extractSecretRefPathsFromConfig( + configRow?.configJson ?? {}, + manifest?.instanceConfigSchema, + ); + const paths = [...(refsBySecret.get(trimmedRef) ?? [])]; + if (paths.length === 0) { + throw new Error("Secret is not referenced by this company's plugin config"); + } + if (paths.length > 1) { + throw new Error( + `Secret reference is ambiguous in this company's plugin config at: ${paths.join(", ")}`, + ); + } + + return secrets.resolveSecretValue(companyId, trimmedRef, "latest", { + consumerType: "plugin", + consumerId: pluginId, + configPath: paths[0], + actorType: "plugin", + actorId: pluginId, + pluginId, + }); }, }; } diff --git a/ui/src/api/plugins.ts b/ui/src/api/plugins.ts index b0187560..1572ff11 100644 --- a/ui/src/api/plugins.ts +++ b/ui/src/api/plugins.ts @@ -357,8 +357,10 @@ export const pluginsApi = { * * @param pluginId - UUID of the plugin. */ - getConfig: (pluginId: string) => - api.get(`/plugins/${pluginId}/config`), + getConfig: (pluginId: string, companyId?: string | null) => { + const qs = companyId ? `?companyId=${encodeURIComponent(companyId)}` : ""; + return api.get(`/plugins/${pluginId}/config${qs}`); + }, /** * Save (create or update) the configuration for a plugin. @@ -369,8 +371,8 @@ export const pluginsApi = { * @param pluginId - UUID of the plugin. * @param configJson - Configuration values matching the plugin's `instanceConfigSchema`. */ - saveConfig: (pluginId: string, configJson: Record) => - api.post(`/plugins/${pluginId}/config`, { configJson }), + saveConfig: (pluginId: string, configJson: Record, companyId?: string | null) => + api.post(`/plugins/${pluginId}/config`, { configJson, companyId }), /** * Call the plugin's `validateConfig` RPC method to test the configuration diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index a5517f34..7b4aac75 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -199,7 +199,8 @@ export const queryKeys = { detail: (pluginId: string) => ["plugins", pluginId] as const, health: (pluginId: string) => ["plugins", pluginId, "health"] as const, uiContributions: ["plugins", "ui-contributions"] as const, - config: (pluginId: string) => ["plugins", pluginId, "config"] as const, + config: (pluginId: string, companyId?: string | null) => + ["plugins", pluginId, "config", companyId ?? null] as const, localFolders: (pluginId: string, companyId: string) => ["plugins", pluginId, "companies", companyId, "local-folders"] as const, dashboard: (pluginId: string) => ["plugins", pluginId, "dashboard"] as const, diff --git a/ui/src/pages/PluginSettings.tsx b/ui/src/pages/PluginSettings.tsx index 996b8f8f..9e671c28 100644 --- a/ui/src/pages/PluginSettings.tsx +++ b/ui/src/pages/PluginSettings.tsx @@ -47,8 +47,8 @@ import { * - `GET /api/plugins/:pluginId/health` — health diagnostics (polling). * Only fetched when `plugin.status === "ready"`. * - `GET /api/plugins/:pluginId/dashboard` — aggregated runtime dashboard data (polling). - * - `GET /api/plugins/:pluginId/config` — current config values. - * - `POST /api/plugins/:pluginId/config` — save config values. + * - `GET /api/plugins/:pluginId/config?companyId=...` — current company config values. + * - `POST /api/plugins/:pluginId/config` — save company config values. * - `POST /api/plugins/:pluginId/config/test` — test configuration. * * URL params: @@ -97,9 +97,9 @@ export function PluginSettings() { const hasConfigSchema = configSchema && configSchema.properties && Object.keys(configSchema.properties).length > 0; const { data: configData, isLoading: configLoading } = useQuery({ - queryKey: queryKeys.plugins.config(pluginId!), - queryFn: () => pluginsApi.getConfig(pluginId!), - enabled: !!pluginId && !!hasConfigSchema, + queryKey: queryKeys.plugins.config(pluginId!, selectedCompanyId), + queryFn: () => pluginsApi.getConfig(pluginId!, selectedCompanyId), + enabled: !!pluginId && !!hasConfigSchema && !!selectedCompanyId, }); const { slots } = usePluginSlots({ @@ -245,6 +245,7 @@ export function PluginSettings() { ) : hasConfigSchema ? ( ; isLoading?: boolean; @@ -935,7 +937,7 @@ interface PluginConfigFormProps { * Separated from PluginSettings to isolate re-render scope — only the form * re-renders on field changes, not the entire page. */ -function PluginConfigForm({ pluginId, schema, initialValues, isLoading, pluginStatus, supportsConfigTest }: PluginConfigFormProps) { +function PluginConfigForm({ pluginId, companyId, schema, initialValues, isLoading, pluginStatus, supportsConfigTest }: PluginConfigFormProps) { const queryClient = useQueryClient(); // Form values: start with saved values, fall back to schema defaults @@ -971,11 +973,11 @@ function PluginConfigForm({ pluginId, schema, initialValues, isLoading, pluginSt // Save mutation const saveMutation = useMutation({ mutationFn: (configJson: Record) => - pluginsApi.saveConfig(pluginId, configJson), + pluginsApi.saveConfig(pluginId, configJson, companyId), onSuccess: () => { setSaveMessage({ type: "success", text: "Configuration saved." }); setTestResult(null); - queryClient.invalidateQueries({ queryKey: queryKeys.plugins.config(pluginId) }); + queryClient.invalidateQueries({ queryKey: queryKeys.plugins.config(pluginId, companyId) }); // Clear success message after 3s setTimeout(() => setSaveMessage(null), 3000); },