RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(plugin-sdk): thread companyId through configChanged + fail-closed cross-tenant guard (LOOA-687) (#10096) 

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - First-party **plugins** run as isolated workers spawned by the host
`plugin-loader`, reading company-scoped config through a governed
`ctx.config.get(companyId)` channel.
> - The host→worker `configChanged` RPC carries `{ config, companyId }`,
but the SDK dispatch dropped the scope — `onConfigChanged(newConfig)`
was companyId-blind by design — so a **proactive** worker kept a single
worker-global config.
> - #10092 added a startup replay that fans out **every** stored
company's config through `configChanged`. With no deterministic
ordering, a plugin configured for more than one distinct company ends up
running as whichever DB row was delivered last.
> - That is a latent cross-tenant identity/secret confusion bug: one
company's bot token could be applied to another company's traffic.
> - This pull request threads `companyId` through `onConfigChanged` and
adds a fail-closed cross-tenant guard at the SDK layer, so a
single-tenant worker can never silently collapse to a second company's
config.
> - The benefit is that the config-delivery class is fixed at the SDK
boundary — before any genuinely multi-company proactive plugin ships —
without changing today's single-tenant behavior.

## Linked Issues or Issue Description

No public GitHub issue — describing in-PR (hardening / latent security):

**Latent cross-tenant config collapse.** The worker-side `configChanged`
dispatch forwarded only `config` and dropped `companyId`, so a proactive
plugin kept a single worker-global config. #10092's startup replay
delivers every configured company's config sequentially with no `ORDER
BY`, so a plugin with configs for more than one distinct company would
apply a nondeterministic last-write-wins global config (one tenant's
credential applied to another's traffic).

- Builds on and must merge after #10092.
- Not exploitable today: the only proactive consumer (the chat gateway)
has single-tenant config rows, so last-write-wins is a no-op. This is a
hardening pre-condition before any multi-company proactive plugin ships.

## What Changed

- **Thread scope through:** `onConfigChanged(newConfig, context)` with a
new exported `PluginConfigChangeContext { companyId }`. Backward
compatible — the second arg is optional; existing single-arg
implementations are unaffected.
- **Fail-closed cross-tenant guard** (`worker-rpc-host.ts`): a
single-tenant plugin that receives `configChanged` for a second,
distinct company with a *different* config is rejected with the new
`PLUGIN_RPC_ERROR_CODES.CROSS_TENANT_CONFIG` instead of silently
overwriting the applied tenant's config. Idempotent replays of the
*same* config under a different scope row remain allowed.
- **Opt-in `multiCompanyConfig: true`** on the plugin definition for
plugins that genuinely serve multiple companies from one worker (keying
per-company state on `context.companyId`); the guard is bypassed for
those.
- **Deterministic `ORDER BY companyId`** on `registry.listConfigs`, so
the startup replay binds a single-tenant worker to a stable company
across restarts.
- **Loader visibility:** a `CROSS_TENANT_CONFIG` rejection is logged at
`warn` (was best-effort `debug`) so the misconfiguration is surfaced.
- **Regression test**
(`packages/plugins/sdk/tests/worker-rpc-host.test.ts`): two distinct
companies delivered via the startup-replay path fail closed and stay
bound to the first company; an idempotent same-config replay under a
different scope row is allowed; a `multiCompanyConfig` plugin receives
per-company config with the correct `context.companyId`.

## Verification

- SDK `tsc --noEmit`: clean.
- SDK vitest `worker-rpc-host.test.ts`: 7/7 pass (incl. 3 new). The
two-distinct-company case **fails against pre-fix code** and passes
after the fix.
- #10092 embedded-postgres `plugin-config-startup-delivery.test.ts`: 3/3
pass (unaffected by the new `ORDER BY`).
- Full server `tsc --noEmit` against this SDK: clean.

## Risks

- **Low functional risk.** The second `onConfigChanged` arg is optional
and existing implementations are unchanged. Today's single-tenant
gateway keeps working — idempotent same-config replays are explicitly
allowed, so the go-live is preserved.
- **Behavioral shift on misconfig:** a genuinely multi-company plugin
that has NOT opted into `multiCompanyConfig` now fails closed
(`CROSS_TENANT_CONFIG`) rather than silently collapsing to one tenant.
This is the intended safer default; opt in with `multiCompanyConfig:
true` to serve multiple companies from one worker.
- **Not in scope (residual).** Per-company workers/connections for a
genuinely multi-company gateway increase resource use and are tracked
separately (ties into the #10092 fan-out/timeout follow-up). This PR
fixes the class and fails closed; it does not build multi-tenant
connection management.

## Model Used

Claude — Anthropic `claude-opus-4-8` (Opus 4.8), extended thinking, with
tool use / code execution via Claude Code.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes:` / `Closes`
/ `Refs` OR (b) described the issue in-PR following the relevant issue
template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [ ] My branch name describes the change and contains no internal
Paperclip ticket id — branch predates this rule; not renaming an open PR
mid-review
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes — no
doc surface; internal SDK/host behavior only
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: anicca <annica@Michaels-Mac-Studio.local>
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Michael Nguyen 2026-07-23 09:50:48 -07:00 committed by GitHub
parent 1ebf5254b6
commit a7186dce4b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 348 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
packages/plugins/sdk/src/define-plugin.ts
View File

@ -166,6 +166,31 @@ export interface PluginApiResponse {
body?: unknown;
}
// ---------------------------------------------------------------------------
// Config change context
// ---------------------------------------------------------------------------
/**
* Scope metadata delivered alongside a `configChanged` RPC so the worker knows
* *which company's* configuration changed.
*
* The host→worker `configChanged` message has always carried the company scope,
* but the SDK historically dropped it before invoking `onConfigChanged`, leaving
* proactive plugins to keep a single worker-global config. That is safe for a
* single-tenant plugin but silently collapses a multi-company plugin onto
* whichever company's config was delivered last. Threading the scope through
* lets a `multiCompanyConfig` plugin maintain per-company state.
*
* @see PLUGIN_SPEC.md §13.4 — `configChanged`
*/
export interface PluginConfigChangeContext {
/**
* The company whose configuration changed, or `null` for an instance/global
* save that is not bound to a specific company.
*/
companyId: string | null;
}
// ---------------------------------------------------------------------------
// Plugin definition
// ---------------------------------------------------------------------------
@ -207,6 +232,22 @@ export interface PluginDefinition {
*/
onHealth?(): Promise<PluginHealthDiagnostics>;
/**
* When true, this plugin's worker correctly serves configuration from more
* than one company inside a single worker process — for example by keying its
* state on `context.companyId` in `onConfigChanged` and running one connection
* / subscription set per company.
*
* When false or omitted (the default), the plugin is treated as single-tenant.
* The host then **fails closed** if `configChanged` would ever deliver a
* second, distinct company's configuration to the same worker: instead of
* silently collapsing the worker onto whichever company arrived last (a
* cross-tenant identity/secret confusion bug), the delivery is rejected with
* `PLUGIN_RPC_ERROR_CODES.CROSS_TENANT_CONFIG`. Re-delivering an unchanged
* config for a different company (idempotent replay) is still allowed.
*/
multiCompanyConfig?: boolean;
/**
* Called when the operator updates this plugin's company-scoped configuration
* at runtime, without restarting the worker.
@ -214,9 +255,16 @@ export interface PluginDefinition {
* If not implemented, the host restarts the worker to apply the new config.
*
* @param newConfig - The newly resolved configuration
* @param context - Scope of the change. `context.companyId` identifies the
* company whose config changed (null for an instance/global save). A
* multi-company plugin (`multiCompanyConfig: true`) MUST key its per-company
* state on this value rather than assuming a single global config.
* @see PLUGIN_SPEC.md §13.4 — `configChanged`
*/
onConfigChanged?(newConfig: Record<string, unknown>): Promise<void>;
onConfigChanged?(
newConfig: Record<string, unknown>,
context?: PluginConfigChangeContext,
): Promise<void>;
/**
* Called when the host is about to shut down the plugin worker.

1
packages/plugins/sdk/src/index.ts
View File

@ -94,6 +94,7 @@ export type {
PluginDefinition,
PaperclipPlugin,
PluginHealthDiagnostics,
PluginConfigChangeContext,
PluginConfigValidationResult,
PluginWebhookInput,
PluginApiRequestInput,

8
packages/plugins/sdk/src/protocol.ts
View File

@ -257,6 +257,14 @@ export const PLUGIN_RPC_ERROR_CODES = {
METHOD_NOT_IMPLEMENTED: -32004,
/** The worker→host call attempted to escape the current invocation company scope. */
INVOCATION_SCOPE_DENIED: -32005,
/**
* A `configChanged` delivery would have collapsed a single-tenant worker onto
* a second, distinct company's configuration. The worker fails closed instead
* of silently overwriting the already-applied tenant's config. A plugin that
* genuinely serves multiple companies from one worker must opt in via
* `multiCompanyConfig: true` on its definition.
*/
CROSS_TENANT_CONFIG: -32006,
/** A catch-all for errors that do not fit other categories. */
UNKNOWN: -32099,
} as const;

74
packages/plugins/sdk/src/worker-rpc-host.ts
View File

@ -201,6 +201,32 @@ function realpathOrResolvedPath(filePath: string): string {
}
}
/**
* Order-independent structural equality for two plugin config objects.
*
* Config arrives as parsed JSON, so plain `JSON.stringify` comparison is
* sensitive to key ordering across independent saves. Canonicalizing with
* recursively sorted object keys makes an idempotent replay of the same config
* compare equal regardless of serialization order.
*/
function configsEqual(a: unknown, b: unknown): boolean {
return canonicalize(a) === canonicalize(b);
}
function canonicalize(value: unknown): string {
if (value === null || typeof value !== "object") {
return JSON.stringify(value) ?? "null";
}
if (Array.isArray(value)) {
return `[${value.map(canonicalize).join(",")}]`;
}
const entries = Object.entries(value as Record<string, unknown>)
.filter(([, v]) => v !== undefined)
.sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0))
.map(([key, v]) => `${JSON.stringify(key)}:${canonicalize(v)}`);
return `{${entries.join(",")}}`;
}
export function isWorkerEntrypoint(entry: string, moduleUrl: string): boolean {
const thisFile = realpathOrResolvedPath(fileURLToPath(moduleUrl));
const entryPath = realpathOrResolvedPath(entry);
@ -294,6 +320,10 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost
let initialized = false;
let manifest: PaperclipPluginManifestV1 | null = null;
let currentConfig: Record<string, unknown> = {};
// The company whose config was last applied via configChanged. Used to fail
// closed when a single-tenant plugin would be collapsed onto a second,
// distinct company's config. `null` until the first company-scoped delivery.
let configCompanyId: string | null = null;
let databaseNamespace: string | null = null;
const invocationContextStorage = new AsyncLocalStorage<PluginInvocationContext>();
@ -1584,10 +1614,52 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost
}
async function handleConfigChanged(params: ConfigChangedParams): Promise<void> {
const incomingCompanyId = params.companyId ?? null;
// Fail-closed cross-tenant guard.
//
// A worker is spawned once per plugin (not per company), so a proactive
// plugin that keeps a single worker-global config would silently collapse
// onto whichever company's config was delivered last if configChanged is
// called for more than one distinct company — for example the startup
// config replay fanning out every stored company's config, or two operators
// saving configs for different companies. That is a cross-tenant identity /
// secret confusion bug (one company's bot token applied to another's work).
//
// Reject the second, distinct company unless the plugin explicitly declares
// it handles multiple companies in one worker (multiCompanyConfig). An
// idempotent replay of the *same* config for a different company id is
// harmless (single-tenant plugins commonly have duplicate scope rows that
// all embed the same config), so it is allowed.
if (
!plugin.definition.multiCompanyConfig &&
incomingCompanyId !== null &&
configCompanyId !== null &&
configCompanyId !== incomingCompanyId &&
!configsEqual(params.config, currentConfig)
) {
throw Object.assign(
new Error(
`configChanged: refusing to overwrite configuration for company ` +
`"${configCompanyId}" with a different configuration for company ` +
`"${incomingCompanyId}". This plugin is single-tenant and cannot ` +
`safely serve multiple companies from one worker. If multi-company ` +
`support is intended, set multiCompanyConfig: true on the plugin ` +
`definition and key per-company state on context.companyId.`,
),
{ code: PLUGIN_RPC_ERROR_CODES.CROSS_TENANT_CONFIG },
);
}
currentConfig = params.config;
if (incomingCompanyId !== null) {
configCompanyId = incomingCompanyId;
}
if (plugin.definition.onConfigChanged) {
await plugin.definition.onConfigChanged(params.config);
await plugin.definition.onConfigChanged(params.config, {
companyId: incomingCompanyId,
});
}
}

188
packages/plugins/sdk/tests/worker-rpc-host.test.ts
View File

@ -296,3 +296,191 @@ describe("worker invocation scope propagation", () => {
}
});
});
describe("worker configChanged cross-tenant guard", () => {
// Spin up a worker-rpc-host wired to in-memory streams and expose a
// request/response `callWorker` plus `initialize`/`stop` helpers.
function makeWorker(plugin: ReturnType<typeof definePlugin>) {
const hostToWorker = new PassThrough();
const workerToHost = new PassThrough();
const hostReadline = createInterface({ input: workerToHost });
const pending = new Map<string, (response: JsonRpcResponse) => void>();
let nextRequestId = 1;
const worker = startWorkerRpcHost({
plugin,
stdin: hostToWorker,
stdout: workerToHost,
});
function callWorker(method: string, params: unknown) {
const id = `host-${nextRequestId++}`;
const result = new Promise<unknown>((resolve, reject) => {
pending.set(id, (response) => {
if ("error" in response && response.error) {
reject(
Object.assign(new Error(response.error.message), {
code: response.error.code,
}),
);
return;
}
resolve((response as { result?: unknown }).result);
});
});
hostToWorker.write(serializeMessage(createRequest(method, params, id)));
return result;
}
hostReadline.on("line", (line) => {
const message = parseMessage(line);
if (!isJsonRpcResponse(message)) return;
pending.get(String(message.id))?.(message);
pending.delete(String(message.id));
});
async function initialize() {
await callWorker("initialize", {
manifest: {
id: "paperclip.config-guard-test",
apiVersion: 1,
version: "1.0.0",
displayName: "Config Guard Test",
description: "Test plugin",
author: "Paperclip",
categories: ["automation"],
capabilities: [],
entrypoints: {},
},
config: {},
databaseNamespace: null,
});
}
function stop() {
worker.stop();
hostReadline.close();
hostToWorker.destroy();
workerToHost.destroy();
}
return { callWorker, initialize, stop };
}
it("fails closed when a second, distinct company's config would overwrite a single-tenant worker", async () => {
const applied: Array<{ companyId: string | null; token: unknown }> = [];
const plugin = definePlugin({
async setup() {},
async onConfigChanged(newConfig, context) {
applied.push({
companyId: context?.companyId ?? null,
token: newConfig.slackBotToken,
});
},
});
const { callWorker, initialize, stop } = makeWorker(plugin);
try {
await initialize();
// Company A's config is delivered first (deterministic ORDER BY companyId
// in the loader) and applied.
await expect(
callWorker("configChanged", {
config: { companyId: "company-a", slackBotToken: "xoxb-A" },
companyId: "company-a",
}),
).resolves.toBeNull();
// Company B's *distinct* config must be rejected rather than silently
// collapsing the single worker onto B's bot token (the vulnerability).
await expect(
callWorker("configChanged", {
config: { companyId: "company-b", slackBotToken: "xoxb-B" },
companyId: "company-b",
}),
).rejects.toMatchObject({
code: PLUGIN_RPC_ERROR_CODES.CROSS_TENANT_CONFIG,
});
// The worker stayed bound to company A; company B never reached the
// plugin. Against the pre-fix code this array would be
// [company-a, company-b] (last-write-wins collapse).
expect(applied).toEqual([{ companyId: "company-a", token: "xoxb-A" }]);
} finally {
stop();
}
});
it("allows an idempotent replay of the same config under a different scope row", async () => {
// Mirrors the live single-tenant gateway: several plugin_config rows keyed
// by distinct row companyIds but all embedding the same config. Replaying
// them must be a no-op, not a fail-closed rejection.
const appliedScopes: Array<string | null> = [];
const plugin = definePlugin({
async setup() {},
async onConfigChanged(_newConfig, context) {
appliedScopes.push(context?.companyId ?? null);
},
});
const { callWorker, initialize, stop } = makeWorker(plugin);
try {
await initialize();
const embedded = { companyId: "company-a", slackBotToken: "xoxb-A" };
await callWorker("configChanged", {
config: { ...embedded },
companyId: "row-scope-1",
});
await expect(
callWorker("configChanged", {
config: { ...embedded },
companyId: "row-scope-2",
}),
).resolves.toBeNull();
expect(appliedScopes).toEqual(["row-scope-1", "row-scope-2"]);
} finally {
stop();
}
});
it("threads per-company config to a plugin that opts into multiCompanyConfig", async () => {
const applied: Array<{ companyId: string | null; token: unknown }> = [];
const plugin = definePlugin({
multiCompanyConfig: true,
async setup() {},
async onConfigChanged(newConfig, context) {
applied.push({
companyId: context?.companyId ?? null,
token: newConfig.slackBotToken,
});
},
});
const { callWorker, initialize, stop } = makeWorker(plugin);
try {
await initialize();
await callWorker("configChanged", {
config: { companyId: "company-a", slackBotToken: "xoxb-A" },
companyId: "company-a",
});
await expect(
callWorker("configChanged", {
config: { companyId: "company-b", slackBotToken: "xoxb-B" },
companyId: "company-b",
}),
).resolves.toBeNull();
// Both companies' configs delivered, each tagged with its own scope.
expect(applied).toEqual([
{ companyId: "company-a", token: "xoxb-A" },
{ companyId: "company-b", token: "xoxb-B" },
]);
} finally {
stop();
}
});
});

31
server/src/services/plugin-loader.ts
View File

@ -32,6 +32,7 @@ import path from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { promisify } from "node:util";
import type { Db } from "@paperclipai/db";
import { PLUGIN_RPC_ERROR_CODES } from "@paperclipai/plugin-sdk";
import type {
PaperclipPluginManifestV1,
PluginLauncherDeclaration,
@ -2195,15 +2196,27 @@ export function pluginLoader(
companyId: row.companyId,
});
} catch (configErr) {
log.debug(
{
pluginId,
pluginKey,
companyId: row.companyId,
err: configErr instanceof Error ? configErr.message : String(configErr),
},
"plugin-loader: startup config delivery skipped for company",
);
// A single-tenant worker fails closed (CROSS_TENANT_CONFIG) rather
// than collapse onto a second company's config — surface that at
// warn so the misconfiguration (multiple distinct companies
// configured for a single-tenant plugin) is visible, instead of
// being lost in the best-effort debug stream.
const code = (configErr as { code?: number } | null)?.code;
const details = {
pluginId,
pluginKey,
companyId: row.companyId,
code,
err: configErr instanceof Error ? configErr.message : String(configErr),
};
if (code === PLUGIN_RPC_ERROR_CODES.CROSS_TENANT_CONFIG) {
log.warn(
details,
"plugin-loader: startup config delivery rejected — single-tenant plugin configured for multiple companies",
);
} else {
log.debug(details, "plugin-loader: startup config delivery skipped for company");
}
}
}
} catch (listErr) {

8
server/src/services/plugin-registry.ts
View File

@ -297,12 +297,18 @@ export function pluginRegistryService(db: Db) {
* proactive plugin that never runs inside a company-scoped invocation (and
* therefore cannot resolve `ctx.config.get(companyId)`) still receives its
* configuration at startup.
*
* Ordered deterministically by companyId: the startup replay delivers these
* rows to a single worker via `configChanged`, and a single-tenant worker
* binds to the first company it sees. Without a stable order the worker
* would bind to a nondeterministic (DB-dependent) company across restarts.
*/
listConfigs: (pluginId: string) =>
db
.select()
.from(pluginConfig)
.where(eq(pluginConfig.pluginId, pluginId)),
.where(eq(pluginConfig.pluginId, pluginId))
.orderBy(asc(pluginConfig.companyId)),
/**
* Create or fully replace a plugin's company-scoped configuration.