fix(plugins): preserve distribution capability approval gates

Co-Authored-By: Paperclip <noreply@paperclip.ing>

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

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: {

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/);
+  });
 });

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 });
+    }
+  });
 });

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())

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<RegistryPluginRow | null>;
     update(
       id: string,
-      data: { version?: string; manifest?: PaperclipPluginManifestV1; packagePath?: string },
+      data: { version?: string; manifest?: PaperclipPluginManifestV1; packagePath?: string; status?: "upgrade_pending" },
     ): Promise<unknown>;
     updateStatus(id: string, input: { status: "ready"; lastError: string | null }): Promise<unknown>;
   };
@@ -263,17 +263,26 @@ async function reconcileBundledPluginManifest(
   deps: BundledPluginProvisionerDeps,
   bundleManifestExists: (localPath: string) => boolean,
   verifiedManifest?: PaperclipPluginManifestV1,
-): Promise<void> {
+): 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;

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) {

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

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;