feat(plugin): scope secret-ref config by company

This commit is contained in:
Paperclip Bot 2026-06-03 06:31:01 +00:00 committed by Alkim Ake Gozen
parent 62863126a3
commit db0ef46900
19 changed files with 587 additions and 102 deletions

View file

@ -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 () => {

View file

@ -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,
});
});
});

View file

@ -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 () => {

View file

@ -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<string, unknown> } | undefined;
const body = req.body as { configJson?: Record<string, unknown>; 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,

View file

@ -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 ?? {};
},
},

View file

@ -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;

View file

@ -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<string> {
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,
});
},
};
}