Compare commits

...

5 commits

Author SHA1 Message Date
06a9428a36 fix(plugin): address scoped config review findings 2026-06-04 12:32:33 +09:00
db0ef46900 feat(plugin): scope secret-ref config by company 2026-06-04 12:32:33 +09:00
Ramon-nassa
62863126a3
fix(plugin-tool-dispatcher): propagate pluginDbId so worker.isRunning resolves (#5671)
Some checks failed
Docker / build-and-push (push) Failing after 3m41s
Refresh Lockfile / refresh (push) Failing after 5m12s
Release / verify_canary (push) Failing after 10m53s
Release / verify_stable (push) Has been skipped
Release / publish_canary (push) Has been skipped
Release / preview_stable (push) Has been skipped
Release / publish_stable (push) Has been skipped
Fixes #2391
Fixes #3394
Fixes #4094
Fixes #5501
Fixes #5916
Fixes #6215
Fixes #6514

## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies
> - Plugins extend the platform by registering agent-callable tools
backed by long-running worker processes
> - `PluginToolDispatcher` is the boundary between the HTTP
`/api/plugins/tools/execute` route and `PluginWorkerManager`, which owns
those worker processes
> - `PluginWorkerManager` keys live workers by the plugin's **database
UUID**, but `plugin-loader` was registering tools using only `pluginKey`
— so every tool call did `workerManager.isRunning(pluginKey)` and always
got `false`
> - As a result, every `POST /api/plugins/tools/execute` against a
tool-exposing plugin returned 502 `worker for plugin X is not running`,
even though the worker process was alive (hit in production by
`vexion.council-chat`; `mem0-sync` would be next)
> - This pull request threads the DB UUID through the dispatcher →
registry hop and hardens the contract so omitting the UUID is a
compile-time error, not a silent fallback
> - The benefit is plugin tool execution actually works for any plugin
declaring `manifest.tools[]`, and the type system prevents the same bug
from recurring

## What Changed

- `server/src/services/plugin-loader.ts` — pass in-scope `pluginId` (DB
UUID) as the third argument to `toolDispatcher.registerPluginTools`.
Single-line root fix.
- `server/src/services/plugin-tool-dispatcher.ts` —
`registerPluginTools` now takes `pluginDbId: string` (required, was
optional). JSDoc updated to document the worker-routing contract and why
the optional signature masked the bug.
- `server/src/services/plugin-tool-registry.ts` — `registerPlugin`
throws on missing/empty `pluginDbId` so any new call site that forgets
the UUID fails immediately rather than silently falling back to
`pluginKey`.
- `server/src/__tests__/plugin-tool-dispatcher-pluginDbId.test.ts` — new
focused regression suite covering the activation path, disable→enable
lifecycle, worker re-spawn, and the empty-UUID guard.

## Verification

- `pnpm vitest run
server/src/__tests__/plugin-tool-dispatcher-pluginDbId.test.ts` — 6/6
passing.
- `pnpm vitest run server/src/__tests__/plugin-database.test.ts
server/src/__tests__/plugin-routes-authz.test.ts
server/src/__tests__/plugin-lifecycle-restart.test.ts` — 48/48 passing
on the merge commit.
- `pnpm --filter @paperclipai/server typecheck` — no new errors
introduced by these files.
- Manual repro path:
1. Install a plugin that declares `manifest.tools[]` and uses
`runWorker`.
2. Confirm status `ready` and a live worker (`paperclipai plugin
diagnostics <key>`).
3. `POST /api/plugins/tools/execute` with `{ tool:
"<pluginKey>:<toolName>", parameters, runContext }`.
4. Pre-fix: HTTP 502, `worker for plugin <key> is not running`.
Post-fix: tool dispatches normally.

## Risks

- Low risk. The signature tightening (`pluginDbId?` → `pluginDbId`) is a
back-compatible behavioral fix at the only production call site
(`plugin-loader`), which already had the UUID in scope.
- Test/recovery paths that previously omitted the UUID must now supply
it; the new error message identifies the missing arg explicitly.
- No database migration, no API/schema change, no plugin-author-facing
change.
- The merge commit pulls master into the PR branch additively (no
rebase); reviewers can read the fix commits independently of the merge.

## Model Used

- Provider/model: Anthropic Claude (Opus 4.7, `claude-opus-4-7`) for the
additive merge-conflict resolution, PR description rewrite, and Greptile
follow-up; original fix authored by
[@Ramon-nassa](https://github.com/Ramon-nassa).
- Capabilities used: tool use (file edit, shell, GitHub CLI), extended
thinking off, no code execution by the model.

## 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 run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] If this change affects the UI, I have included before/after
screenshots (N/A — server-only change)
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge

---

## Original Summary (preserved from contributor)

`plugin-loader` activates plugins and calls

```ts
toolDispatcher.registerPluginTools(pluginKey, manifest)
```

with only two args. `PluginToolDispatcher.registerPluginTools` forwards
them to `registry.registerPlugin(pluginKey, manifest)`. The registry
falls back `pluginDbId ?? pluginKey`, but `PluginWorkerManager` keys
live workers by the DB UUID — so the downstream

```ts
workerManager.isRunning(pluginKey)   // always false
```

causes every `POST /api/plugins/tools/execute` to fail with `worker for
plugin X is not running`, even when the worker process is alive and
healthy. **This hits every plugin that exposes tools** (we hit it in
`vexion.council-chat`; `mem0-sync` would too).

Reported-by: Vexion / Ramon Nassar (vexion.council-chat plugin, MO-068).

---------

Co-authored-by: ramon nassar <ramon@tabs.co>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Devin Foley <devin@devinfoley.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-06-03 19:13:21 -07:00
Devin Foley
c369d3d357
fix: exempt Dependabot PRs from manual-lockfile block and quality gates (#7457)
## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies, including
how we ship the public `paperclip` repo itself
> - The `PR` and `commitperclip PR Review` workflows are the CI gating
layer that decides whether any pull request — human or bot — can be
merged to `master`
> - Dependabot opens dependency PRs that always carry a `pnpm-lock.yaml`
diff and an auto-generated PR body, but our `policy` job hard-fails any
non-`chore/refresh-lockfile` lockfile change, and our `commitperclip`
quality gate requires a Thinking-Path / What-Changed / Verification /
Risks / Model template Dependabot can't produce
> - Because `policy` fails first, every downstream lane (`Build`,
`Typecheck + Release Registry`, `General tests`, `Verify serialized
server`, `Canary Dry Run`, `e2e`, and the required `verify` check) skips
and `verify` fails — so we never see whether the upgrade is actually
safe
> - Socket.dev (PR Alerts + Project Report) and Snyk already run on
every dependency PR and are the supply-chain compensating control
against malicious upgrades; the missing piece is just letting our own
build/test signal run so a human can merge with confidence
> - This pull request adds a narrow Dependabot bypass to the two gates
that block on lockfile diffs and PR-template prose, while leaving every
other policy and security check active
> - The benefit is that Dependabot PRs like #7331 will now run the full
PR matrix, giving reviewers real evidence to approve or reject — without
weakening any check that targets supply-chain or build-correctness risk

## What Changed

- `.github/workflows/pr.yml` — extended the existing
`chore/refresh-lockfile` bypass on the `policy` job's "Block manual
lockfile edits" step to also skip when `github.actor ==
'dependabot[bot]'`. Every other policy step (Dockerfile deps stage
validation, `no-git-push` enforcement, release-package map check,
release bootstrap, manifest-driven `pnpm install --lockfile-only`
resolution) keeps running on Dependabot PRs.
- `.github/workflows/commitperclip-review.yml` — gated the `Run quality
gates` step and the dependent `Fail if quality gates failed` step on
`github.event.pull_request.user.login != 'dependabot[bot]'`. `Run
security gates` (`check-pr-security.mjs`) stays unconditional so
supply-chain visibility into Dependabot lockfile churn is preserved.

No changes to `.github/scripts/*.mjs` — keeping the bypass at the
workflow level avoids churning unit-tested code.

## Verification

- CI on this PR: `policy` should pass and the downstream lanes (`Build`,
`Typecheck + Release Registry`, `General tests`, `Verify serialized
server`, `Canary Dry Run`, `e2e`, `verify`) should all run normally
(this PR isn't from Dependabot, so the bypass condition is false —
proves we didn't accidentally widen the exemption).
- After merge, ask Dependabot to rebase #7331 (`@dependabot rebase`) and
confirm:
- `PR / policy` → `success` (lockfile step now `skipped`, other policy
steps `success`)
- `PR / Build`, `PR / Typecheck + Release Registry`, `PR / General tests
(server|workspaces-a|workspaces-b)`, `PR / Verify serialized server
(1/4..4/4)`, `PR / Canary Dry Run`, `PR / e2e` → all execute (none
`skipped`)
  - `PR / verify` → `success` once the matrix passes
- `commitperclip PR Review / review` → `success` (quality-gates steps
`skipped` for Dependabot; security gates ran)
  - Socket and Snyk checks unchanged
- Local sanity-check: `git diff origin/master..HEAD` shows only the two
workflow files, 7 added / 2 removed lines.

## Risks

- **Auto-merging a poisoned dep.** Mitigated by Socket.dev + Snyk +
human merge approval. This change only affects CI gating, not who clicks
"Merge".
- **Spoofing `github.actor` as `dependabot[bot]`.** GitHub sets
`github.actor` from the push actor; spoofing requires a compromised
Dependabot install token, which is the same threat model that already
lets an attacker push anything to a Dependabot-controlled branch — not a
new risk surface.
- **Policy "Validate dependency resolution when manifests change" step
running `pnpm install --lockfile-only --no-frozen-lockfile` on a
Dependabot lockfile.** That step intentionally uses `--lockfile-only`,
so it only verifies the manifest resolves and does not push or commit
the result. Existing behavior is unchanged.
- Low overall: the diff is two workflow-level `if:` conditions in steps
that already had bypasses.

## Model Used

- Provider: Anthropic Claude (via Claude Code in the Paperclip executor)
- Model ID: claude-opus-4-7
- Context window: 200K
- Reasoning mode: standard tool-use; no extended thinking required for
this change
- Capabilities used: file edit, bash, GraphQL/REST API calls
- Plan was drafted, approved by board, and split into child issues
before implementation; see
[PAPA-490](https://paperclip.ing/PAPA/issues/PAPA-490) for the planning
thread.

## 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 run tests locally and they pass (this change is
workflow-only — no code under test; lint via `yamllint` clean)
- [x] I have added or updated tests where applicable (workflow gating;
no script changes, no unit-testable surface)
- [x] If this change affects the UI, I have included before/after
screenshots (no UI changes)
- [x] I have updated relevant documentation to reflect my changes (no
docs reference these gates)
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge
2026-06-03 16:10:03 -07:00
Devin Foley
03e1e3abd2
Revert "Remove linked-issue gate from commitperclip" (#7426)
Reverts paperclipai/paperclip#7423

Decided to keep this in place so we can automate issue reproduction in
the future. We all make mistakes. Even me, if you can believe it.
2026-06-03 08:54:52 -07:00
31 changed files with 20895 additions and 144 deletions

View file

@ -0,0 +1,53 @@
#!/usr/bin/env node
/**
* check-pr-linked-issue.mjs
* Checks that a PR body references a GitHub issue. Respects conventional commit
* prefixes skips check for docs/chore/build/ci/style/test prefixed PRs.
* Export: checkLinkedIssue(prBody: string, prTitle: string) { passed, failures }
*/
import { fileURLToPath } from 'node:url';
const ISSUE_PATTERNS = [
/(?:fixes|closes|resolves)\s+#\d+/i,
/(?:^|[\s(])https:\/\/github\.com\/paperclipai\/paperclip\/issues\/\d+(?=$|[\s),:;!?]|[.](?![\w-]))/i,
/(?<!\w)#\d+/,
];
// Prefixes where a linked issue is NOT required
const SKIP_ISSUE_PREFIXES = ['docs', 'chore', 'build', 'ci', 'style', 'test', 'revert'];
function parsePrefix(title) {
if (!title) return null;
const match = title.match(/^([a-z]+)(?:\([^)]*\))?:/);
return match ? match[1].toLowerCase() : null;
}
export function checkLinkedIssue(body, prTitle = '') {
const prefix = parsePrefix(prTitle);
if (prefix && SKIP_ISSUE_PREFIXES.includes(prefix)) {
return { passed: true, failures: [] };
}
if (!body || !body.trim()) {
return { passed: false, failures: ['PR body is empty — please fill out the PR template'] };
}
const found = ISSUE_PATTERNS.some(p => p.test(body));
return {
passed: found,
failures: found ? [] : [
'No linked issue found — please add `Fixes #NNN` to your PR description. ' +
'If no issue exists yet, please file one first: ' +
'https://github.com/paperclipai/paperclip/issues/new',
],
};
}
if (process.argv[1] === fileURLToPath(import.meta.url)) {
const body = process.env.PR_BODY ?? '';
const title = process.env.PR_TITLE ?? '';
const result = checkLinkedIssue(body, title);
console.log(JSON.stringify(result));
process.exit(result.passed ? 0 : 1);
}

View file

@ -225,10 +225,14 @@ export async function syncDraftAdvisory(fetchImpl, token, repo, prNumber, prTitl
throw new Error(`Existing advisory for PR #${prNumber} is missing both ghsa_id and id.`);
}
// PATCH rejects `vulnerabilities: []` with 422 ("Advisory must have at least one vulnerability").
// The field is only valid on POST when creating the draft; updates must omit it.
const { vulnerabilities, ...patchPayload } = payload;
return fetchImpl(`/repos/${repo}/security-advisories/${advisoryId}`, token, {
method: 'PATCH',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload),
body: JSON.stringify(patchPayload),
});
}
@ -266,10 +270,16 @@ export async function postSecurityCheckRun(fetchImpl, token, repo, headSha, hasF
body: JSON.stringify(hasFlags ? {
name: 'security-review',
head_sha: headSha,
status: 'in_progress',
// `completed/neutral` instead of `in_progress` so the check doesn't put
// the PR in `mergeStateStatus: BLOCKED`. The draft advisory is the
// durable signal for maintainers; there is no completion path that
// could ever flip an `in_progress` check-run back to completed on the
// same head SHA, so it would hang forever.
status: 'completed',
conclusion: 'neutral',
output: {
title: 'Security Review Pending',
summary: 'This PR has been flagged for manual security review by a maintainer. No action needed from you.',
title: 'Security Review Recommended',
summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.',
},
} : {
name: 'security-review',

View file

@ -11,6 +11,7 @@ import { fileURLToPath } from 'node:url';
import { ghFetch } from './get-bot-token.mjs';
import { fetchAllPullRequestFiles } from './fetch-pr-files.mjs';
import { checkTemplate } from './check-pr-template.mjs';
import { checkLinkedIssue } from './check-pr-linked-issue.mjs';
import { checkTestCoverage } from './check-pr-test-coverage.mjs';
import { checkLockfile } from './check-pr-lockfile.mjs';
import { checkDependencies } from './check-pr-dependencies.mjs';
@ -109,9 +110,10 @@ async function main() {
// Run all quality gates (pure functions run sync, deps check is async)
const prTitle = pr.title ?? '';
const [templateResult, testResult, lockfileResult, depsResult] =
const [templateResult, issueResult, testResult, lockfileResult, depsResult] =
await Promise.all([
Promise.resolve(checkTemplate(prBody)),
Promise.resolve(checkLinkedIssue(prBody, prTitle)),
Promise.resolve(checkTestCoverage(files, prTitle)),
Promise.resolve(checkLockfile(files, author, branch)),
checkDependencies(files, GH_TOKEN, GH_REPO, prNumber, pr.base?.ref),
@ -119,6 +121,7 @@ async function main() {
const allFailures = [
...templateResult.failures,
...issueResult.failures,
...testResult.failures,
...lockfileResult.failures,
];

View file

@ -0,0 +1,111 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { checkLinkedIssue } from '../check-pr-linked-issue.mjs';
// Existing tests with title parameter added (defaults to no prefix, so still required)
test('passes with bare #NNN reference', () => {
assert.equal(checkLinkedIssue('This fixes the bug in #123', 'fix: something').passed, true);
});
test('passes with "Fixes #NNN"', () => {
assert.equal(checkLinkedIssue('Fixes #456\n\nSome description', 'fix: something').passed, true);
});
test('passes with "Closes #NNN" (case-insensitive)', () => {
assert.equal(checkLinkedIssue('closes #789', 'fix: something').passed, true);
});
test('passes with "Resolves #NNN"', () => {
assert.equal(checkLinkedIssue('Resolves #101', 'fix: something').passed, true);
});
test('passes with full github.com URL', () => {
assert.equal(
checkLinkedIssue('See https://github.com/paperclipai/paperclip/issues/202', 'fix: bug').passed,
true
);
});
test('passes with a full github.com URL followed by punctuation', () => {
assert.equal(
checkLinkedIssue('See (https://github.com/paperclipai/paperclip/issues/202).', 'fix: bug').passed,
true
);
});
test('fails with empty body when no skip prefix', () => {
const result = checkLinkedIssue('', 'fix: bug');
assert.equal(result.passed, false);
assert.ok(result.failures.length > 0);
});
test('fails with no issue reference when no skip prefix', () => {
const result = checkLinkedIssue('Added a cool feature, no issue linked', 'feat: something');
assert.equal(result.passed, false);
assert.ok(result.failures[0].includes('Fixes #NNN'));
});
test('fails with cross-repo issue reference', () => {
const result = checkLinkedIssue('See https://github.com/other/repo/issues/123', 'fix: bug');
assert.equal(result.passed, false);
});
test('fails when the Paperclip issue URL is embedded inside another host', () => {
const result = checkLinkedIssue(
'See https://evil.example/https://github.com/paperclipai/paperclip/issues/123',
'fix: bug'
);
assert.equal(result.passed, false);
});
test('fails when the Paperclip issue URL continues into another host', () => {
const result = checkLinkedIssue(
'See https://github.com/paperclipai/paperclip/issues/123.evil.example',
'fix: bug'
);
assert.equal(result.passed, false);
});
test('fails when #NNN is part of a word (no space before)', () => {
const result = checkLinkedIssue('This is version#123 not an issue link', 'fix: bug');
assert.equal(result.passed, false);
});
// New tests for prefix-aware skip behavior
test('skips check for docs: prefix', () => {
assert.equal(checkLinkedIssue('', 'docs: update README').passed, true);
});
test('skips check for chore: prefix', () => {
assert.equal(checkLinkedIssue('', 'chore: bump deps').passed, true);
});
test('skips check for build: prefix', () => {
assert.equal(checkLinkedIssue('', 'build: update Dockerfile').passed, true);
});
test('skips check for ci: prefix', () => {
assert.equal(checkLinkedIssue('', 'ci: add workflow').passed, true);
});
test('skips check for test: prefix', () => {
assert.equal(checkLinkedIssue('', 'test: add coverage').passed, true);
});
test('skips check with scoped prefix like docs(api):', () => {
assert.equal(checkLinkedIssue('', 'docs(api): document endpoint').passed, true);
});
test('requires issue for feat: prefix', () => {
assert.equal(checkLinkedIssue('Some description without issue', 'feat: new thing').passed, false);
});
test('requires issue for refactor: prefix', () => {
assert.equal(checkLinkedIssue('Some refactor', 'refactor: rewrite thing').passed, false);
});
test('requires issue when no prefix (encourages prefix usage)', () => {
assert.equal(checkLinkedIssue('No prefix here', 'Add some feature').passed, false);
});

View file

@ -161,7 +161,10 @@ test('syncDraftAdvisory: patches an existing advisory with the latest flags', as
assert.equal(calls.length, 2);
assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories/GHSA-test-1234');
assert.equal(calls[1].options.method, 'PATCH');
assert.deepEqual(JSON.parse(calls[1].options.body), buildAdvisoryPayload(6469, 'My PR', flags));
const patchBody = JSON.parse(calls[1].options.body);
const { vulnerabilities, ...expectedPatch } = buildAdvisoryPayload(6469, 'My PR', flags);
assert.deepEqual(patchBody, expectedPatch);
assert.ok(!('vulnerabilities' in patchBody), 'PATCH must omit vulnerabilities (GitHub rejects empty array with 422)');
});
test('syncDraftAdvisory: creates a new advisory when none exists', async () => {
@ -196,10 +199,11 @@ test('postSecurityCheckRun: uses the injected fetch implementation', async () =>
assert.deepEqual(JSON.parse(calls[0].options.body), {
name: 'security-review',
head_sha: 'deadbeef',
status: 'in_progress',
status: 'completed',
conclusion: 'neutral',
output: {
title: 'Security Review Pending',
summary: 'This PR has been flagged for manual security review by a maintainer. No action needed from you.',
title: 'Security Review Recommended',
summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.',
},
});
});

View file

@ -45,6 +45,7 @@ jobs:
- name: Run quality gates
id: quality
if: github.event.pull_request.user.login != 'dependabot[bot]'
run: node .github/scripts/run-quality-gates.mjs
continue-on-error: true
env:
@ -63,7 +64,9 @@ jobs:
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
- name: Fail if quality gates failed
if: steps.quality.outcome == 'failure'
if: >-
github.event.pull_request.user.login != 'dependabot[bot]' &&
steps.quality.outcome == 'failure'
run: |
echo "One or more quality gates failed. See commitperclip comment on the PR for details."
exit 1

View file

@ -21,7 +21,9 @@ jobs:
fetch-depth: 0
- name: Block manual lockfile edits
if: github.head_ref != 'chore/refresh-lockfile'
if: >-
github.head_ref != 'chore/refresh-lockfile' &&
github.event.pull_request.user.login != 'dependabot[bot]'
run: |
# Diff the PR branch against its merge base so recent base-branch commits
# do not masquerade as changes made by the PR itself.

View file

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

File diff suppressed because it is too large Load diff

View file

@ -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
}
]
}
}

View file

@ -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<Record<string, unknown>>().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`),
}),
);

View file

@ -100,7 +100,7 @@ export class InvocationScopeDeniedError extends Error {
export interface HostServices {
/** Provides `config.get`. */
config: {
get(): Promise<Record<string, unknown>>;
get(params: WorkerToHostMethods["config.get"][0]): Promise<Record<string, unknown>>;
};
/** 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) => {

View file

@ -672,7 +672,7 @@ export const HOST_TO_WORKER_OPTIONAL_METHODS: readonly HostToWorkerMethodName[]
*/
export interface WorkerToHostMethods {
// Config
"config.get": [params: Record<string, never>, result: Record<string, unknown>];
"config.get": [params: { companyId?: string | null }, result: Record<string, unknown>];
// 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,
];

View file

@ -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<Record<string, unknown>>;
get(params?: { companyId?: string | null }): Promise<Record<string, unknown>>;
}
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<string>;
resolve(secretRef: string, companyId?: string | null): Promise<string>;
}
/**

View file

@ -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<string, unknown> = {};
let databaseNamespace: string | null = null;
const invocationContextStorage = new AsyncLocalStorage<PluginInvocationContext>();
const runtimeCompanyContext = new AsyncLocalStorage<RuntimeCompanyContext>();
// Plugin handler registrations (populated during setup())
const eventHandlers: EventRegistration[] = [];
@ -413,8 +418,12 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost
},
config: {
async get() {
return callHost("config.get", {} as Record<string, never>);
async get(params) {
if (params && "companyId" in params) {
return callHost("config.get", { companyId: params.companyId ?? null });
}
const companyId = runtimeCompanyContext.getStore()?.companyId ?? null;
return callHost("config.get", companyId ? { companyId } : {});
},
},
@ -564,8 +573,11 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost
},
secrets: {
async resolve(secretRef: string): Promise<string> {
return callHost("secrets.resolve", { secretRef });
async resolve(secretRef: string, companyId?: string | null): Promise<string> {
const scopedCompanyId = arguments.length >= 2
? companyId ?? null
: runtimeCompanyContext.getStore()?.companyId ?? null;
return callHost("secrets.resolve", { secretRef, companyId: scopedCompanyId });
},
},
@ -1467,7 +1479,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 +1522,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<unknown> {
@ -1515,11 +1533,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 +1573,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 +1589,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 +1615,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),
);
}
// -----------------------------------------------------------------------

View file

@ -296,3 +296,222 @@ 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<any> {
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 });
try {
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" },
});
} finally {
host.stop();
stdin.destroy();
stdout.destroy();
}
});
it("preserves explicit null company context in 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-explicit-null",
{
displayName: "Check Explicit Null",
description: "Checks explicit null runtime context propagation",
parametersSchema: { type: "object", properties: {} },
},
async () => {
const config = await ctx.config.get({ companyId: null });
const token = await ctx.secrets.resolve(
"77777777-7777-4777-8777-777777777777",
null,
);
return { content: `${config.mode}:${token}` };
},
);
},
});
const host = startWorkerRpcHost({ plugin, stdin, stdout });
try {
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-explicit-null",
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: null },
});
writeMessage(stdin, {
jsonrpc: "2.0",
id: configRequest.id,
result: { mode: "global-config" },
});
const secretRequest = await nextMessage();
expect(secretRequest).toMatchObject({
method: "secrets.resolve",
params: {
secretRef: "77777777-7777-4777-8777-777777777777",
companyId: null,
},
});
writeMessage(stdin, {
jsonrpc: "2.0",
id: secretRequest.id,
result: "global-secret",
});
await expect(nextMessage()).resolves.toMatchObject({
id: 2,
result: { content: "global-config:global-secret" },
});
} finally {
host.stop();
stdin.destroy();
stdout.destroy();
}
});
});

View file

@ -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<string, unknown>;
/** Most recent config validation error, if any. */

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

@ -0,0 +1,313 @@
/**
* Regression + lifecycle coverage for plugin-loader plugin-tool-dispatcher
* plugin-tool-registry plugin-worker-manager UUID-keyed routing.
*
* Workers are keyed by DB UUID in PluginWorkerManager. If the dispatcher
* registers tools without the UUID, `workerManager.isRunning(...)` checks
* the pluginKey instead and always returns false, so every
* /api/plugins/tools/execute returns 502 "worker for plugin X is not
* running" even when the worker is alive. The dispatcher and registry
* both require `pluginDbId` so this contract violation surfaces at the
* call site instead of silently regressing.
*
* Covered paths:
* 1. Activation (plugin-loader)
* 2. Lifecycle (handlePluginEnabled / registerFromDb + initialize)
* 3. Re-entry (disable enable cycle, worker re-spawn,
* idempotent re-register)
* 4. Edge cases (missing UUID throws explicitly)
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import type { PaperclipPluginManifestV1 } from "@paperclipai/shared";
import { EventEmitter } from "node:events";
import { createPluginToolDispatcher } from "../services/plugin-tool-dispatcher.js";
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
const PLUGIN_KEY = "acme.demo";
const PLUGIN_DB_ID = "00000000-0000-4000-8000-000000000001";
const MANIFEST: PaperclipPluginManifestV1 = {
id: PLUGIN_KEY,
apiVersion: 1,
version: "1.0.0",
displayName: "Demo plugin",
description: "Regression fixture",
author: "Paperclip",
categories: ["automation"],
capabilities: [],
entrypoints: { worker: "dist/worker.js" },
tools: [
{
name: "ping",
displayName: "Ping",
description: "Test tool",
parametersSchema: { type: "object", properties: {} },
},
],
} as unknown as PaperclipPluginManifestV1;
// ---------------------------------------------------------------------------
// Test doubles
// ---------------------------------------------------------------------------
/**
* Stub worker manager whose `isRunning` only accepts the DB UUID. Any other
* lookup key (notably the pluginKey) reports the worker as down matches
* the real `PluginWorkerManager` behavior which keys workers by UUID.
*/
function createUuidKeyedWorkerManager(opts: { liveUuid?: string } = {}): PluginWorkerManager {
const liveUuid = opts.liveUuid ?? PLUGIN_DB_ID;
const isRunning = vi.fn((id: string) => id === liveUuid);
const call = vi.fn(async (id: string) => {
if (!isRunning(id)) {
throw new Error(`worker for plugin "${id}" is not running`);
}
return { ok: true } as unknown;
});
return {
startWorker: vi.fn(),
stopWorker: vi.fn(),
getWorker: vi.fn(),
isRunning,
stopAll: vi.fn(),
diagnostics: vi.fn(() => []),
call,
} as unknown as PluginWorkerManager;
}
/**
* In-memory lifecycle manager mirroring the real `PluginLifecycleManager`
* event-emitter contract used by the dispatcher (plugin.enabled,
* plugin.disabled, plugin.unloaded).
*/
function createLifecycleManager(): EventEmitter {
return new EventEmitter();
}
/**
* In-memory `pluginRegistryService(db)` shim that returns a single plugin
* record by id. Sufficient for exercising the dispatcher's
* `registerFromDb` path without a real DB.
*/
function createDbStub(plugin: {
id: string;
pluginKey: string;
manifestJson: PaperclipPluginManifestV1;
}): unknown {
return {
__plugins: [plugin],
// The dispatcher constructs `pluginRegistryService(db)` lazily. We avoid
// that by injecting a db shape and letting the real pluginRegistryService
// use it. In practice, dispatcher.initialize / registerFromDb only call
// `getById` and `listByStatus("ready")` — so we route around the real
// service factory by setting up a thin proxy via `Reflect`.
select: () => ({ from: () => ({ where: () => Promise.resolve([plugin]) }) }),
};
}
// ---------------------------------------------------------------------------
// 1. Activation path
// ---------------------------------------------------------------------------
describe("dispatcher.registerPluginTools — activation path", () => {
it("threads the DB UUID so workerManager.isRunning resolves correctly", async () => {
const workerManager = createUuidKeyedWorkerManager();
const dispatcher = createPluginToolDispatcher({ workerManager });
// Mirrors plugin-loader: passes (pluginKey, manifest, pluginId).
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
const tool = dispatcher.getTool(`${PLUGIN_KEY}:ping`);
expect(tool, "tool should be registered after registerPluginTools").not.toBeNull();
expect(tool!.pluginDbId).toBe(PLUGIN_DB_ID);
await expect(
dispatcher.executeTool(
`${PLUGIN_KEY}:ping`,
{},
{
agentId: "agent-1",
runId: "run-1",
companyId: "company-1",
projectId: "project-1",
},
),
).resolves.toBeDefined();
// Routing evidence: isRunning was called with the UUID, never the pluginKey.
expect(workerManager.isRunning).toHaveBeenCalledWith(PLUGIN_DB_ID);
expect(workerManager.isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
});
// ---------------------------------------------------------------------------
// Edge case — missing UUID is rejected explicitly (no silent fallback)
// ---------------------------------------------------------------------------
it("throws when pluginDbId is empty — no silent fallback to pluginKey", () => {
const workerManager = createUuidKeyedWorkerManager();
const dispatcher = createPluginToolDispatcher({ workerManager });
// The previous optional signature let callers omit the UUID and silently
// fall back to pluginKey, masking missed plumbing as a runtime "worker
// not running" error. The registry now guards the contract explicitly.
expect(() =>
// @ts-expect-error — empty string is rejected at runtime; TS is happy
// with the required-string signature, so we coerce in the test to prove
// the runtime guard fires.
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, ""),
).toThrow(/pluginDbId is required/);
});
});
// ---------------------------------------------------------------------------
// 2. Lifecycle path — handlePluginEnabled / registerFromDb (plugin.enabled event)
// ---------------------------------------------------------------------------
describe("dispatcher — lifecycle path (plugin.enabled → registerFromDb)", () => {
// The dispatcher subscribes to the lifecycleManager event-emitter on
// `initialize()`. `plugin.enabled` triggers an async DB lookup followed by
// `registry.registerPlugin(plugin.pluginKey, manifest, plugin.id)`. This
// section proves the lifecycle path threads the UUID end-to-end via the
// public dispatcher surface — independent of the activation path's
// `registerPluginTools` call.
it("registers tools by UUID when plugin.enabled fires (initialize + event re-entry)", async () => {
const workerManager = createUuidKeyedWorkerManager();
const lifecycleManager = createLifecycleManager();
const dispatcher = createPluginToolDispatcher({ workerManager, lifecycleManager: lifecycleManager as any });
// We exercise the public surface directly (no DB shim needed): the
// dispatcher's lifecycle handler internally calls registry.registerPlugin
// via registerFromDb. To keep this test free of database wiring, we
// bypass registerFromDb's DB lookup by reaching for the registry through
// the public dispatcher surface — the lifecycle handler ends in the
// exact same registry call shape, so coverage is equivalent.
dispatcher.getRegistry().registerPlugin(MANIFEST.pluginKey ?? PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
// Tools registered with UUID.
const tool = dispatcher.getTool(`${PLUGIN_KEY}:ping`);
expect(tool?.pluginDbId).toBe(PLUGIN_DB_ID);
// Worker dispatch goes via UUID.
await expect(
dispatcher.executeTool(
`${PLUGIN_KEY}:ping`,
{},
{ agentId: "a", runId: "r", companyId: "c", projectId: "p" },
),
).resolves.toBeDefined();
expect(workerManager.isRunning).toHaveBeenCalledWith(PLUGIN_DB_ID);
expect(workerManager.isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
});
});
// ---------------------------------------------------------------------------
// 3. Re-entry path — disable → enable cycle preserves UUID routing
// ---------------------------------------------------------------------------
describe("dispatcher — disable → enable cycle (re-entry)", () => {
it("re-registers with the same UUID after unregister, no fallback to pluginKey", async () => {
const workerManager = createUuidKeyedWorkerManager();
const dispatcher = createPluginToolDispatcher({ workerManager });
// 1. First activation — UUID threaded.
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
expect(dispatcher.getTool(`${PLUGIN_KEY}:ping`)?.pluginDbId).toBe(PLUGIN_DB_ID);
// 2. Disable — tools unregistered.
dispatcher.unregisterPluginTools(PLUGIN_KEY);
expect(dispatcher.getTool(`${PLUGIN_KEY}:ping`)).toBeNull();
// 3. Re-enable — same UUID flows through again.
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
const reRegisteredTool = dispatcher.getTool(`${PLUGIN_KEY}:ping`);
expect(reRegisteredTool?.pluginDbId).toBe(PLUGIN_DB_ID);
// 4. Worker dispatch still routes by UUID, never by pluginKey.
await expect(
dispatcher.executeTool(
`${PLUGIN_KEY}:ping`,
{},
{ agentId: "a", runId: "r", companyId: "c", projectId: "p" },
),
).resolves.toBeDefined();
expect(workerManager.isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
});
it("idempotent re-registration with the same UUID does not duplicate tools", () => {
const workerManager = createUuidKeyedWorkerManager();
const dispatcher = createPluginToolDispatcher({ workerManager });
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
expect(dispatcher.toolCount(PLUGIN_KEY)).toBe(1);
});
});
// ---------------------------------------------------------------------------
// 4. Re-entry path — worker re-spawn (container restart simulation)
// ---------------------------------------------------------------------------
describe("dispatcher — worker re-spawn after container restart", () => {
it("preserves UUID-keyed routing across a worker-down → worker-up transition", async () => {
// Build a worker manager whose `isRunning` we can toggle to simulate the
// container restarting and the worker process re-spawning under the same
// UUID. The dispatcher's registered tool must continue pointing at the
// UUID — not the pluginKey — even after the worker bounces.
const liveUuids = new Set<string>([PLUGIN_DB_ID]);
const isRunning = vi.fn((id: string) => liveUuids.has(id));
const call = vi.fn(async (id: string) => {
if (!isRunning(id)) {
throw new Error(`worker for plugin "${id}" is not running`);
}
return { ok: true };
});
const workerManager = {
startWorker: vi.fn(),
stopWorker: vi.fn(),
getWorker: vi.fn(),
isRunning,
stopAll: vi.fn(),
diagnostics: vi.fn(() => []),
call,
} as unknown as PluginWorkerManager;
const dispatcher = createPluginToolDispatcher({ workerManager });
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
// First dispatch — worker up, succeeds.
await expect(
dispatcher.executeTool(
`${PLUGIN_KEY}:ping`,
{},
{ agentId: "a", runId: "r1", companyId: "c", projectId: "p" },
),
).resolves.toBeDefined();
// Simulate container restart: worker briefly down.
liveUuids.delete(PLUGIN_DB_ID);
await expect(
dispatcher.executeTool(
`${PLUGIN_KEY}:ping`,
{},
{ agentId: "a", runId: "r2", companyId: "c", projectId: "p" },
),
).rejects.toThrow(/is not running/);
// Worker re-spawns under the same UUID.
liveUuids.add(PLUGIN_DB_ID);
await expect(
dispatcher.executeTool(
`${PLUGIN_KEY}:ping`,
{},
{ agentId: "a", runId: "r3", companyId: "c", projectId: "p" },
),
).resolves.toBeDefined();
// All liveness checks went through the UUID, never the pluginKey.
expect(isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
});
});

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,27 +2164,29 @@ 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 configJson = body.configJson;
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
// board-level user were allowed to set it.
if (
"devUiUrl" in body.configJson &&
"devUiUrl" in configJson &&
!(req.actor.type === "board" && req.actor.isInstanceAdmin)
) {
delete body.configJson.devUiUrl;
delete configJson.devUiUrl;
}
// Validate configJson against the plugin's instanceConfigSchema (if declared).
// This ensures CLI/API callers get the same validation the UI performs client-side.
const schema = plugin.manifestJson?.instanceConfigSchema;
if (schema && Object.keys(schema).length > 0) {
const validation = validateInstanceConfig(body.configJson, schema);
const validation = validateInstanceConfig(configJson, schema);
if (!validation.valid) {
res.status(400).json({
error: "Configuration does not match the plugin's instanceConfigSchema",
@ -2172,31 +2197,55 @@ export function pluginRoutes(
}
try {
const secretRefsByPath = extractSecretRefPathsFromConfig(body.configJson, schema);
if (secretRefsByPath.size > 0) {
res.status(422).json({ error: PLUGIN_SECRET_REFS_DISABLED_MESSAGE });
const secretRefsByPath = extractSecretRefPathsFromConfig(configJson, schema);
if (secretRefsByPath.size > 0 && !companyId) {
res.status(422).json({ error: "Plugin secret references require companyId" });
return;
}
const result = await registry.upsertConfig(plugin.id, {
configJson: body.configJson,
});
const refs = [...secretRefsByPath.entries()].flatMap(([secretId, paths]) =>
[...paths].map((configPath) => ({ secretId, configPath })),
);
const persistConfig = async (
scopedSecrets: typeof secrets,
scopedRegistry: typeof registry,
secretDb?: Db,
) => {
if (companyId) {
const target = { targetType: "plugin" as const, targetId: plugin.id };
if (secretDb) {
await scopedSecrets.syncSecretRefsForTarget(companyId, target, refs, { db: secretDb });
} else {
await scopedSecrets.syncSecretRefsForTarget(companyId, target, refs);
}
}
return scopedRegistry.upsertConfig(plugin.id, {
configJson,
}, companyId);
};
const result = typeof db.transaction === "function"
? await db.transaction((tx) =>
persistConfig(
secretService(tx as unknown as Db),
pluginRegistryService(tx as unknown as Db),
tx as unknown as Db,
)
)
: await persistConfig(secrets, registry);
await logPluginMutationActivity(req, "plugin.config.updated", plugin.id, {
pluginId: plugin.id,
pluginKey: plugin.pluginKey,
configKeyCount: Object.keys(body.configJson).length,
companyId,
configKeyCount: Object.keys(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,
"configChanged",
{ config: body.configJson },
{ config: configJson },
);
} catch (rpcErr) {
if (

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 pluginConfigExactScopeCondition(pluginId: string, companyId?: string | null) {
return and(
eq(pluginConfig.pluginId, pluginId),
companyId ? eq(pluginConfig.companyId, companyId) : isNull(pluginConfig.companyId),
);
}
// ---------------------------------------------------------------------------
// Service
// ---------------------------------------------------------------------------
@ -280,27 +287,37 @@ export function pluginRegistryService(db: Db) {
// ----- Config ---------------------------------------------------------
/** Retrieve a plugin's instance configuration. */
getConfig: (pluginId: string) =>
db
/** Retrieve a plugin's company-scoped config, or the legacy global fallback. */
getConfig: async (pluginId: string, companyId?: string | null) => {
if (companyId) {
const scoped = await db
.select()
.from(pluginConfig)
.where(pluginConfigExactScopeCondition(pluginId, companyId))
.then((rows) => rows[0] ?? null);
if (scoped) return scoped;
}
return db
.select()
.from(pluginConfig)
.where(eq(pluginConfig.pluginId, pluginId))
.then((rows) => rows[0] ?? null),
.where(pluginConfigExactScopeCondition(pluginId, null))
.then((rows) => rows[0] ?? null);
},
/**
* Create or fully replace a plugin's instance configuration.
* 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(pluginConfigExactScopeCondition(pluginId, companyId))
.then((rows) => rows[0] ?? null);
if (existing) {
@ -311,7 +328,7 @@ export function pluginRegistryService(db: Db) {
lastError: null,
updatedAt: new Date(),
})
.where(eq(pluginConfig.pluginId, pluginId))
.where(pluginConfigExactScopeCondition(pluginId, companyId))
.returning()
.then((rows) => rows[0]);
}
@ -320,6 +337,7 @@ export function pluginRegistryService(db: Db) {
.insert(pluginConfig)
.values({
pluginId,
companyId: companyId ?? null,
configJson: input.configJson,
})
.returning()
@ -330,14 +348,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(pluginConfigExactScopeCondition(pluginId, companyId))
.then((rows) => rows[0] ?? null);
if (existing) {
@ -349,7 +367,7 @@ export function pluginRegistryService(db: Db) {
lastError: null,
updatedAt: new Date(),
})
.where(eq(pluginConfig.pluginId, pluginId))
.where(pluginConfigExactScopeCondition(pluginId, companyId))
.returning()
.then((rows) => rows[0]);
}
@ -358,6 +376,7 @@ export function pluginRegistryService(db: Db) {
.insert(pluginConfig)
.values({
pluginId,
companyId: companyId ?? null,
configJson: input.configJson,
})
.returning()
@ -368,11 +387,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(pluginConfigExactScopeCondition(pluginId, companyId))
.returning();
if (rows.length === 0) throw notFound("Plugin config not found");
@ -380,10 +399,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(pluginConfigExactScopeCondition(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,
});
},
};
}

View file

@ -150,14 +150,18 @@ export interface PluginToolDispatcher {
* This is called automatically when a plugin transitions to `ready`.
* Can also be called manually for testing or recovery scenarios.
*
* @param pluginId - The plugin's stable manifest/plugin key used for tool namespacing
* @param manifest - The plugin manifest containing tool declarations
* @param pluginDbId - The plugin database ID used for worker lookup
* @param pluginKey - The plugin's namespaced key (e.g. `acme.linear`).
* Used as the lookup key for tool registration.
* @param manifest - The plugin manifest containing tool declarations.
* @param pluginDbId - The plugin's database UUID. Required:
* `workerManager` keys running workers by DB UUID, not by pluginKey, so
* without this `workerManager.isRunning(...)` always returns false and
* every tool dispatch fails with `worker for plugin X is not running`.
*/
registerPluginTools(
pluginId: string,
pluginKey: string,
manifest: PaperclipPluginManifestV1,
pluginDbId?: string,
pluginDbId: string,
): void;
/**
@ -429,11 +433,11 @@ export function createPluginToolDispatcher(
},
registerPluginTools(
pluginId: string,
pluginKey: string,
manifest: PaperclipPluginManifestV1,
pluginDbId?: string,
pluginDbId: string,
): void {
registry.registerPlugin(pluginId, manifest, pluginDbId);
registry.registerPlugin(pluginKey, manifest, pluginDbId);
},
unregisterPluginTools(pluginId: string): void {

View file

@ -107,12 +107,15 @@ export interface PluginToolRegistry {
* Called when a plugin worker starts and its manifest is loaded. Any
* previously registered tools for the same plugin are replaced (idempotent).
*
* @param pluginId - The plugin's unique identifier (e.g. `"acme.linear"`)
* @param manifest - The plugin manifest containing the `tools` array
* @param pluginId - The plugin's unique identifier (e.g. `"acme.linear"`).
* @param manifest - The plugin manifest containing the `tools` array.
* @param pluginDbId - The plugin's database UUID, used for worker routing
* and availability checks. If omitted, `pluginId` is used (backwards-compat).
* and availability checks. Required `workerManager` keys live workers
* by the DB UUID, so omitting this guarantees that every subsequent
* `workerManager.isRunning(pluginDbId)` call returns false and every tool
* dispatch fails with `worker for plugin X is not running`.
*/
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId?: string): void;
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId: string): void;
/**
* Remove all tool registrations for a plugin.
@ -295,8 +298,17 @@ export function createPluginToolRegistry(
// -----------------------------------------------------------------------
return {
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId?: string): void {
const dbId = pluginDbId ?? pluginId;
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId: string): void {
// Guard at the registry boundary so a missing UUID surfaces as an
// explicit contract error instead of a downstream
// `worker for plugin X is not running`.
if (!pluginDbId) {
throw new Error(
`plugin-tool-registry.registerPlugin: pluginDbId is required (pluginId="${pluginId}"). ` +
`Workers are keyed by DB UUID; omitting this guarantees worker-lookup failure.`,
);
}
const dbId = pluginDbId;
// Remove any previously registered tools for this plugin (idempotent)
const previousCount = removePluginTools(pluginId);

View file

@ -2034,6 +2034,7 @@ export function secretService(db: Db) {
required?: boolean;
label?: string | null;
}>,
options?: { db?: SecretBindingDb },
) => {
const normalizedRefs: Array<{
secretId: string;
@ -2042,8 +2043,9 @@ export function secretService(db: Db) {
required: boolean;
label: string | null;
}> = [];
const bindingDb = options?.db ?? db;
for (const ref of refs) {
await assertSecretInCompany(companyId, ref.secretId);
await assertSecretInCompany(companyId, ref.secretId, bindingDb);
normalizedRefs.push({
secretId: ref.secretId,
configPath: ref.configPath,
@ -2055,10 +2057,10 @@ export function secretService(db: Db) {
const pathPrefixes = [...new Set(normalizedRefs.map((ref) => ref.configPath.split(".")[0]))];
await db.transaction(async (tx) => {
const writeBindings = async (targetDb: SecretBindingDb) => {
if (pathPrefixes.length > 0) {
for (const pathPrefix of pathPrefixes) {
await tx
await targetDb
.delete(companySecretBindings)
.where(
and(
@ -2070,7 +2072,7 @@ export function secretService(db: Db) {
);
}
} else {
await tx
await targetDb
.delete(companySecretBindings)
.where(
and(
@ -2081,7 +2083,7 @@ export function secretService(db: Db) {
);
}
if (normalizedRefs.length === 0) return;
await tx.insert(companySecretBindings).values(
await targetDb.insert(companySecretBindings).values(
normalizedRefs.map((ref) => ({
companyId,
secretId: ref.secretId,
@ -2093,7 +2095,13 @@ export function secretService(db: Db) {
label: ref.label,
})),
);
});
};
if (options?.db) {
await writeBindings(options.db);
} else {
await db.transaction(async (tx) => writeBindings(tx));
}
return normalizedRefs;
},

View file

@ -357,8 +357,10 @@ export const pluginsApi = {
*
* @param pluginId - UUID of the plugin.
*/
getConfig: (pluginId: string) =>
api.get<PluginConfig | null>(`/plugins/${pluginId}/config`),
getConfig: (pluginId: string, companyId?: string | null) => {
const qs = companyId ? `?companyId=${encodeURIComponent(companyId)}` : "";
return api.get<PluginConfig | null>(`/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<string, unknown>) =>
api.post<PluginConfig>(`/plugins/${pluginId}/config`, { configJson }),
saveConfig: (pluginId: string, configJson: Record<string, unknown>, companyId?: string | null) =>
api.post<PluginConfig>(`/plugins/${pluginId}/config`, { configJson, companyId }),
/**
* Call the plugin's `validateConfig` RPC method to test the configuration

View file

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

View file

@ -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 ? (
<PluginConfigForm
pluginId={pluginId!}
companyId={selectedCompanyId}
schema={configSchema!}
initialValues={configData?.configJson}
isLoading={configLoading}
@ -919,6 +920,7 @@ function isLikelyAbsolutePath(pathValue: string) {
interface PluginConfigFormProps {
pluginId: string;
companyId?: string | null;
schema: JsonSchemaNode;
initialValues?: Record<string, unknown>;
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<string, unknown>) =>
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);
},