diff --git a/doc/plugins/DISTRIBUTION-PLUGINS.md b/doc/plugins/DISTRIBUTION-PLUGINS.md index d2fa5fd02a..77fd55782e 100644 --- a/doc/plugins/DISTRIBUTION-PLUGINS.md +++ b/doc/plugins/DISTRIBUTION-PLUGINS.md @@ -56,7 +56,12 @@ The manifest's worker and optional UI entrypoints must match the verified At boot, selected distribution entries adopt the current image's package path even when a previous npm or local install has the same version. Reconciliation -keeps the registry ID, configuration, stored state and operator-disabled status. +keeps the registry ID, configuration and stored state. With unchanged permissions, +operator-disabled status is retained. A replacement that adds capabilities is +saved atomically in `upgrade_pending`, even for same-version bundles. It cannot +activate until an operator reviews the manifest and enables it through the normal +plugin lifecycle. Invalid capability declarations are rejected before persistence. +Runtime refreshes also reject unapproved capability additions before starting code. Keep each key's directory stable across releases. The activation guard also covers persisted installs: a plugin removed from the image catalog, or no diff --git a/server/src/__tests__/bundled-plugins.test.ts b/server/src/__tests__/bundled-plugins.test.ts index 57cb3e7c6f..fef938c3a7 100644 --- a/server/src/__tests__/bundled-plugins.test.ts +++ b/server/src/__tests__/bundled-plugins.test.ts @@ -221,7 +221,7 @@ type LooseRow = { // Build a minimal manifest for a persisted row or a shipped bundle. The reconcile // step compares the bundle version with the persisted version. function makeManifest(pluginKey: string, version: string) { - return { id: pluginKey, apiVersion: 1, version } as unknown as import("@paperclipai/shared").PaperclipPluginManifestV1; + return { id: pluginKey, apiVersion: 1, version, capabilities: [] } as unknown as import("@paperclipai/shared").PaperclipPluginManifestV1; } function makeDeps(overrides?: { @@ -438,6 +438,40 @@ describe("ensureBundledPlugins", () => { expect(deps.logger.error).toHaveBeenCalled(); }); + it.each(["ready", "error", "disabled"])("gates same-version distribution capability additions from %s atomically", async (status) => { + const localPath = path.join(CATALOG_ROOT, "distribution/widget"); + const distribution = { key: "widget", pluginKey: "acme.widget", version: "0.1.0", directory: "widget", digest: `sha256:${"a".repeat(64)}`, localPath, entrypoints: { worker: "dist/worker.js" } }; + const oldManifest = makeManifest("acme.widget", "0.1.0"); + const { deps, loadManifest, update, updateStatus, installPlugin } = makeDeps({ + rows: { "acme.widget": { id: "row-widget", pluginKey: "acme.widget", status, packagePath: localPath, manifestJson: { ...oldManifest } } }, + }); + const replacement = { ...oldManifest, capabilities: ["issues.read" as const] }; + loadManifest.mockResolvedValue(replacement); + await ensureBundledPlugins([{ ...distribution, distribution }], deps, { reinstallUninstalled: true }); + expect(update).toHaveBeenCalledExactlyOnceWith("row-widget", { version: "0.1.0", manifest: replacement, status: "upgrade_pending" }); + expect(updateStatus).not.toHaveBeenCalled(); + expect(installPlugin).not.toHaveBeenCalled(); + expect(deps.lifecycle.load).not.toHaveBeenCalled(); + + // A later boot must leave the approval gate in place. + vi.mocked(deps.registry.getByKey).mockResolvedValue({ id: "row-widget", pluginKey: "acme.widget", status: "upgrade_pending", version: "0.1.0", packagePath: localPath, manifestJson: replacement }); + await ensureBundledPlugins([{ ...distribution, distribution }], deps, { reinstallUninstalled: true }); + expect(update).toHaveBeenCalledTimes(1); + expect(updateStatus).not.toHaveBeenCalled(); + }); + + it("does not save an inconsistent distribution manifest", async () => { + const localPath = path.join(CATALOG_ROOT, "distribution/widget"); + const distribution = { key: "widget", pluginKey: "acme.widget", version: "0.1.0", directory: "widget", digest: `sha256:${"a".repeat(64)}`, localPath, entrypoints: { worker: "dist/worker.js" } }; + const { deps, loadManifest, update } = makeDeps({ rows: { "acme.widget": { id: "row-widget", pluginKey: "acme.widget", status: "ready", packagePath: localPath } } }); + const manifest = makeManifest("acme.widget", "0.1.0"); + manifest.ui = { slots: [{ type: "appShellOverlay", id: "overlay", displayName: "Overlay", exportName: "Overlay" }] }; + loadManifest.mockResolvedValue(manifest); + await ensureBundledPlugins([{ ...distribution, distribution }], deps, { reinstallUninstalled: true }); + expect(update).not.toHaveBeenCalled(); + expect(deps.logger.error).toHaveBeenCalled(); + }); + it("swallows a reconcile error and continues boot", async () => { const { deps, update } = makeDeps({ rows: { diff --git a/server/src/__tests__/distribution-plugin-catalog.test.ts b/server/src/__tests__/distribution-plugin-catalog.test.ts index a03afe8856..904d3e21aa 100644 --- a/server/src/__tests__/distribution-plugin-catalog.test.ts +++ b/server/src/__tests__/distribution-plugin-catalog.test.ts @@ -81,7 +81,7 @@ describe("image-owned plugin catalogs", () => { const { root, localPath } = fixture(); const entries = readDistributionPluginCatalog(root, BUNDLED_PLUGIN_CATALOG); const guard = distributionPluginActivationGuard(root, entries, ["acme.widget"]); - const manifest = { id: "acme.widget", version: "1.0.0", entrypoints: { worker: "dist/worker.js", ui: "./dist/ui" } } as PaperclipPluginManifestV1; + const manifest = { id: "acme.widget", version: "1.0.0", capabilities: [], entrypoints: { worker: "dist/worker.js", ui: "./dist/ui" } } as unknown as PaperclipPluginManifestV1; expect(() => guard({ packageRoot: localPath, manifest })).not.toThrow(); for (const name of ["worker", "ui"] as const) { for (const value of ["/outside/worker.js", "../outside", "dist/../worker.js", "C:/outside", "dist\\worker.js", "dist/other", "", undefined]) { @@ -96,9 +96,20 @@ describe("image-owned plugin catalogs", () => { writeFileSync(path.join(localPath, "package.json"), JSON.stringify({ version: "1.0.0", paperclipPlugin: { manifest: "./dist/manifest.js", worker: "./dist/worker.js" } })); save([{ ...entry, digest: distributionBundleDigest(localPath) }]); const guard = distributionPluginActivationGuard(root, readDistributionPluginCatalog(root, BUNDLED_PLUGIN_CATALOG), null); - const manifest = { id: "acme.widget", version: "1.0.0", entrypoints: { worker: "./dist/worker.js" } } as PaperclipPluginManifestV1; + const manifest = { id: "acme.widget", version: "1.0.0", capabilities: [], entrypoints: { worker: "./dist/worker.js" } } as unknown as PaperclipPluginManifestV1; expect(() => guard({ packageRoot: localPath, manifest })).not.toThrow(); manifest.entrypoints.ui = "./dist/ui"; expect(() => guard({ packageRoot: localPath, manifest })).toThrow(/verified package/); }); + + it("rejects inconsistent capabilities and unapproved runtime refreshes", () => { + const { root, localPath } = fixture(); + const guard = distributionPluginActivationGuard(root, readDistributionPluginCatalog(root, BUNDLED_PLUGIN_CATALOG), null); + const previousManifest = { id: "acme.widget", version: "1.0.0", capabilities: [], entrypoints: { worker: "./dist/worker.js", ui: "./dist/ui" } } as unknown as PaperclipPluginManifestV1; + const manifest = { ...previousManifest, capabilities: ["issues.read" as const] }; + expect(() => guard({ packageRoot: localPath, manifest, previousManifest })).toThrow(/require approval/); + expect(() => guard({ packageRoot: localPath, manifest, previousManifest: manifest })).not.toThrow(); + previousManifest.ui = { slots: [{ type: "appShellOverlay", id: "overlay", displayName: "Overlay", exportName: "Overlay" }] }; + expect(() => guard({ packageRoot: localPath, manifest: previousManifest })).toThrow(/missing required capabilities: ui.action.register/); + }); }); diff --git a/server/src/__tests__/plugin-loader-error-retry.test.ts b/server/src/__tests__/plugin-loader-error-retry.test.ts index bcde5be425..bc26888ad4 100644 --- a/server/src/__tests__/plugin-loader-error-retry.test.ts +++ b/server/src/__tests__/plugin-loader-error-retry.test.ts @@ -12,6 +12,10 @@ */ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Db } from "@paperclipai/db"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, realpathSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { distributionBundleDigest, distributionPluginActivationGuard, readDistributionPluginCatalog } from "../services/distribution-plugin-catalog.js"; const mockRegistry = vi.hoisted(() => ({ getById: vi.fn(), @@ -148,4 +152,33 @@ describe("pluginLoader.loadAll error retry", () => { expect(result).toEqual({ total: 0, succeeded: 0, failed: 0, results: [] }); }); + + it("rejects distribution capability escalation before saving a runtime refresh or starting a worker", async () => { + const root = realpathSync(mkdtempSync(path.join(os.tmpdir(), "distribution-refresh-"))); + try { + const packageRoot = path.join(root, "distribution", "example"); + mkdirSync(path.join(packageRoot, "dist"), { recursive: true }); + const plugin = createPluginRecord({ status: "ready", packagePath: packageRoot }); + const replacement = { ...plugin.manifestJson, categories: ["ui"], capabilities: ["issues.read"] }; + writeFileSync(path.join(packageRoot, "package.json"), JSON.stringify({ name: plugin.packageName, version: "1.0.0", type: "module", paperclipPlugin: { manifest: "dist/manifest.js", worker: "dist/worker.js" } })); + writeFileSync(path.join(packageRoot, "dist/manifest.js"), `export default ${JSON.stringify(replacement)};`); + writeFileSync(path.join(packageRoot, "dist/worker.js"), "throw new Error('unapproved worker must not start');"); + writeFileSync(path.join(root, "distribution/catalog.json"), JSON.stringify({ schemaVersion: 1, plugins: [{ key: "example", pluginKey: plugin.pluginKey, version: "1.0.0", directory: "example", digest: distributionBundleDigest(packageRoot) }] })); + const runtime = createRuntimeServices(); + const startWorker = vi.fn(); + runtime.workerManager.startWorker = startWorker; + mockRegistry.getById.mockResolvedValue(plugin); + const loader = pluginLoader({} as Db, { + localPluginDir: root, + assertPackageActivation: distributionPluginActivationGuard(root, readDistributionPluginCatalog(root, []), ["example"]), + }, runtime); + const result = await loader.loadSingle(plugin.id); + expect(result.success).toBe(false); + expect(result.error).toContain("capabilities require approval: issues.read"); + expect(mockRegistry.update).not.toHaveBeenCalled(); + expect(startWorker).not.toHaveBeenCalled(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); }); diff --git a/server/src/app.ts b/server/src/app.ts index 3d941c06cf..a1d603c5b3 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -1272,7 +1272,8 @@ export async function createApp( { registry: pluginRegistry, loader, lifecycle, logger }, // Managed mode reinstalls soft-uninstalled bundles (the control plane // owns provisioning); self-hosted leaves an operator's uninstall alone. - // Operator-DISABLED plugins are never touched in either mode. + // Disabled plugins never start automatically. Added distribution permissions + // still enter upgrade_pending so enabling them requires an operator decision. { reinstallUninstalled: managedAutoInstallKeys !== null }, ) .then(() => loader.loadAll()) diff --git a/server/src/services/bundled-plugins.ts b/server/src/services/bundled-plugins.ts index 571b4e1d67..be321ce9a2 100644 --- a/server/src/services/bundled-plugins.ts +++ b/server/src/services/bundled-plugins.ts @@ -1,7 +1,7 @@ import path from "node:path"; import fs from "node:fs"; import type { PaperclipPluginManifestV1 } from "@paperclipai/shared"; -import { readDistributionPluginCatalog, type DistributionPlugin } from "./distribution-plugin-catalog.js"; +import { assertDistributionManifestCapabilities, readDistributionPluginCatalog, type DistributionPlugin } from "./distribution-plugin-catalog.js"; /** * Bundled plugin auto-provisioning. @@ -217,7 +217,7 @@ export interface BundledPluginProvisionerDeps { getByKey(pluginKey: string): Promise; update( id: string, - data: { version?: string; manifest?: PaperclipPluginManifestV1; packagePath?: string }, + data: { version?: string; manifest?: PaperclipPluginManifestV1; packagePath?: string; status?: "upgrade_pending" }, ): Promise; updateStatus(id: string, input: { status: "ready"; lastError: string | null }): Promise; }; @@ -263,17 +263,26 @@ async function reconcileBundledPluginManifest( deps: BundledPluginProvisionerDeps, bundleManifestExists: (localPath: string) => boolean, verifiedManifest?: PaperclipPluginManifestV1, -): Promise { +): Promise<"upgrade_pending" | undefined> { try { if (!verifiedManifest && !bundleManifestExists(install.localPath)) return; const bundleManifest = verifiedManifest ?? await deps.loader.loadManifest(install.localPath); if (!bundleManifest) return; + let requiresApproval = false; + if (install.distribution) { + assertDistributionManifestCapabilities(bundleManifest); + const approved = new Set(existing.manifestJson.capabilities ?? []); + requiresApproval = bundleManifest.capabilities.some((capability) => !approved.has(capability)); + } const rebindPackage = install.distribution && existing.packagePath !== install.localPath && existing.status !== "uninstalled"; - if (bundleManifest.version === existing.version && !rebindPackage) return; + if (bundleManifest.version === existing.version && !rebindPackage && !requiresApproval) return; await deps.registry.update(existing.id, { version: bundleManifest.version, manifest: bundleManifest, ...(rebindPackage ? { packagePath: install.localPath } : {}), + // Persist the replacement and its approval gate in one write. A crash + // between separate manifest/status updates must never grant capabilities. + ...(requiresApproval ? { status: "upgrade_pending" as const } : {}), }); deps.logger.info( { @@ -284,6 +293,7 @@ async function reconcileBundledPluginManifest( }, "reconciled bundled plugin manifest to the shipped bundle version", ); + if (requiresApproval) return "upgrade_pending"; } catch (err) { deps.logger.error( { err, pluginKey: install.pluginKey }, @@ -391,7 +401,8 @@ export async function ensureBundledPlugins( // existing install, because the auto-install below skips a present // plugin. Distribution entries also replace a legacy/npm package path // before loadAll resolves the worker. Configuration and status stay put. - await reconcileBundledPluginManifest(existing, install, deps, bundleManifestExists, verifiedManifest); + const reconciledStatus = await reconcileBundledPluginManifest(existing, install, deps, bundleManifestExists, verifiedManifest); + if (reconciledStatus === "upgrade_pending") continue; if (existing.status === "error") { await reenableErroredBundledPlugin(existing, install, deps); continue; diff --git a/server/src/services/distribution-plugin-catalog.ts b/server/src/services/distribution-plugin-catalog.ts index 40b9e5431c..710c1935a8 100644 --- a/server/src/services/distribution-plugin-catalog.ts +++ b/server/src/services/distribution-plugin-catalog.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { createHash } from "node:crypto"; import { z } from "zod"; import type { PaperclipPluginManifestV1 } from "@paperclipai/shared"; +import { pluginCapabilityValidator } from "./plugin-capability-validator.js"; const segment = z.string().regex(/^[a-z][a-z0-9.-]{0,99}$/); export const distributionPluginCatalogSchema = z.object({ @@ -30,6 +31,13 @@ function bundleEntrypoint(declared: unknown): string { return relative; } +export function assertDistributionManifestCapabilities(manifest: PaperclipPluginManifestV1): void { + const result = pluginCapabilityValidator().validateManifestCapabilities(manifest); + if (!result.allowed) { + throw new Error(`Distribution manifest is missing required capabilities: ${result.missing.join(", ")}`); + } +} + export function distributionPluginsRoot(catalogRoot: string): string { // Match canonical paths persisted by local-path installs (for example, // macOS /tmp -> /private/tmp), without permitting a symlinked catalog itself. @@ -47,7 +55,7 @@ export function distributionPluginActivationGuard( selectedKeys: readonly string[] | null, ) { const root = distributionPluginsRoot(catalogRoot); - return (input: { pluginKey?: string; packageRoot: string; manifest?: PaperclipPluginManifestV1 }) => { + return (input: { pluginKey?: string; packageRoot: string; manifest?: PaperclipPluginManifestV1; previousManifest?: PaperclipPluginManifestV1 }) => { let packageRoot: string; try { packageRoot = fs.realpathSync(input.packageRoot); } catch (error) { @@ -65,6 +73,12 @@ export function distributionPluginActivationGuard( throw new Error("Distribution manifest does not match its catalog identity/version"); } if (input.manifest) { + assertDistributionManifestCapabilities(input.manifest); + if (input.previousManifest) { + const approved = new Set(input.previousManifest.capabilities); + const added = input.manifest.capabilities.filter((capability) => !approved.has(capability)); + if (added.length) throw new Error(`Distribution plugin capabilities require approval: ${added.join(", ")}`); + } const worker = bundleEntrypoint(input.manifest.entrypoints.worker); const ui = input.manifest.entrypoints.ui === undefined ? undefined : bundleEntrypoint(input.manifest.entrypoints.ui); if (worker !== entry.entrypoints.worker || ui !== entry.entrypoints.ui) { diff --git a/server/src/services/plugin-loader.ts b/server/src/services/plugin-loader.ts index 56e95590af..be853ea937 100644 --- a/server/src/services/plugin-loader.ts +++ b/server/src/services/plugin-loader.ts @@ -274,6 +274,8 @@ export interface PluginLoaderOptions { pluginKey?: string; packageRoot: string; manifest?: PaperclipPluginManifestV1; + /** Persisted grants, supplied before a runtime manifest refresh is saved. */ + previousManifest?: PaperclipPluginManifestV1; }) => void; /** * Path to the local plugin directory to scan. @@ -1391,7 +1393,7 @@ export function pluginLoader( ); } - assertPackageActivation?.({ packageRoot, pluginKey: plugin.pluginKey, manifest }); + assertPackageActivation?.({ packageRoot, pluginKey: plugin.pluginKey, manifest, previousManifest: plugin.manifestJson }); if (JSON.stringify(manifest) === JSON.stringify(plugin.manifestJson)) { return plugin; } diff --git a/server/src/services/plugin-registry.ts b/server/src/services/plugin-registry.ts index 8c3b024d5e..f86a550b94 100644 --- a/server/src/services/plugin-registry.ts +++ b/server/src/services/plugin-registry.ts @@ -200,6 +200,7 @@ export function pluginRegistryService(db: Db) { data: { packageName?: string; packagePath?: string; + status?: "upgrade_pending"; version?: string; manifest?: PaperclipPluginManifestV1; }, @@ -212,6 +213,10 @@ export function pluginRegistryService(db: Db) { }; if (data.packageName !== undefined) setClause.packageName = data.packageName; if (data.packagePath !== undefined) setClause.packagePath = data.packagePath; + if (data.status !== undefined) { + setClause.status = data.status; + setClause.lastError = null; + } if (data.version !== undefined) setClause.version = data.version; if (data.manifest !== undefined) { setClause.manifestJson = data.manifest;