fix(plugins): retain source and privacy boundaries across restarts

Keep pending rollback metadata current without implicitly approving capabilities or enabling a previously disabled plugin. Restrict offline caches to public hashed assets and reject npm fallback for persisted image installs.

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

doc/plugins/DISTRIBUTION-PLUGINS.md

@@ -62,10 +62,19 @@ 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.
+Rolling back an unapproved replacement refreshes the displayed manifest but
+retains `upgrade_pending`. Review the rollback manifest and explicitly enable it
+to resume. A smaller capability set alone cannot prove prior approval: it may
+retain an unapproved permission, and the plugin may originally have been disabled.
+Ordinary upgrades/downgrades of an approved, ready plugin continue automatically.
 
 Keep each key's directory stable across releases. The activation guard also
 covers persisted installs: a plugin removed from the image catalog, or no
 longer selected in managed configuration, cannot activate on restart. Its
+stored image path remains the source marker if the directory disappears; package
+resolution cannot substitute an npm copy. Keep the catalog root stable as well.
+An explicit operator reinstall changes a package's source; editing database rows
+or replacing the catalog root is outside this image-selection contract. Plugin
 database records remain for rollback. Plugin data migrations must themselves
 support the intended rollback window; removing a bundle does not undo them.
 
@@ -96,3 +105,7 @@ UI code is trusted browser code. Host context is display context, never proof
 of server authorization. A distribution backend must independently validate
 the signed-in session and enforce company, tenant and user access rules for
 every read and mutation. Keep provider secrets out of plugin UI and manifests.
+The service worker's offline cache accepts only same-origin, hashed build assets
+under `/assets/`. It does not store or replay application HTML, extension routes
+or API data. This policy remains in effect after worker restarts and does not read
+the arbitrary-response caches created by older workers.

server/src/__tests__/bundled-plugins.test.ts

@@ -483,6 +483,21 @@ describe("ensureBundledPlugins", () => {
     expect(installPlugin).not.toHaveBeenCalled();
   });
 
+  it("refreshes a same-version rollback while retaining the pending operator decision", 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 pending = { ...makeManifest("acme.widget", "0.1.0"), capabilities: ["issues.read", "issues.update"] };
+    const { deps, loadManifest, update, updateStatus } = makeDeps({ rows: { "acme.widget": { id: "row-widget", pluginKey: "acme.widget", status: "upgrade_pending", packagePath: localPath, manifestJson: pending } } });
+    // A smaller capability set is not proof that every remaining capability
+    // was approved, or that the plugin was enabled before the pending upgrade.
+    const rollback = { ...makeManifest("acme.widget", "0.1.0"), capabilities: ["issues.read" as const] };
+    loadManifest.mockResolvedValue(rollback);
+    await ensureBundledPlugins([{ ...distribution, distribution }], deps, { reinstallUninstalled: true });
+    expect(update).toHaveBeenCalledExactlyOnceWith("row-widget", { version: "0.1.0", manifest: rollback });
+    expect(updateStatus).not.toHaveBeenCalled();
+    expect(deps.lifecycle.load).not.toHaveBeenCalled();
+  });
+
   it("swallows a reconcile error and continues boot", async () => {
     const { deps, update } = makeDeps({
       rows: {

server/src/__tests__/distribution-plugin-catalog.test.ts

@@ -60,6 +60,7 @@ describe("image-owned plugin catalogs", () => {
     expect(() => distributionPluginActivationGuard(root, entries, [])({ packageRoot: localPath })).toThrow(/not selected/);
     expect(() => distributionPluginActivationGuard(root, entries, [])(input)).toThrow(/not selected/);
     expect(() => distributionPluginActivationGuard(root, [], [])(input)).toThrow(/absent/);
+    expect(() => distributionPluginActivationGuard(root, [], [])({ ...input, packageRoot: path.join(root, "node_modules/acme"), installedPackagePath: localPath })).toThrow(/absent/);
     expect(() => distributionPluginActivationGuard(root, [], [])({ pluginKey: "ordinary.plugin", packageRoot: path.join(root, "ordinary") })).not.toThrow();
   });
   it("rejects dangling catalog symlinks instead of treating them as absent", () => {

server/src/__tests__/plugin-loader-error-retry.test.ts

@@ -181,4 +181,30 @@ describe("pluginLoader.loadAll error retry", () => {
       rmSync(root, { recursive: true, force: true });
     }
   });
+
+  it("does not import an npm fallback for a removed distribution install", async () => {
+    const root = realpathSync(mkdtempSync(path.join(os.tmpdir(), "distribution-removal-")));
+    try {
+      const packageRoot = path.join(root, "node_modules/@example/broken-plugin");
+      mkdirSync(packageRoot, { recursive: true });
+      const plugin = createPluginRecord({ status: "ready", packagePath: path.join(root, "distribution/removed") });
+      writeFileSync(path.join(packageRoot, "package.json"), JSON.stringify({ name: plugin.packageName, type: "module", paperclipPlugin: { manifest: "manifest.js" } }));
+      writeFileSync(path.join(packageRoot, "manifest.js"), "throw new Error('fallback manifest must never import');");
+      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, [], []),
+      }, runtime);
+      const result = await loader.loadSingle(plugin.id);
+      expect(result.success).toBe(false);
+      expect(result.error).toContain("Distribution plugin is absent or not selected");
+      expect(mockRegistry.update).not.toHaveBeenCalled();
+      expect(startWorker).not.toHaveBeenCalled();
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
 });

server/src/services/bundled-plugins.ts

@@ -278,7 +278,8 @@ async function reconcileBundledPluginManifest(
       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 && !requiresApproval) return;
+    const refreshDistribution = install.distribution && JSON.stringify(bundleManifest) !== JSON.stringify(existing.manifestJson);
+    if (bundleManifest.version === existing.version && !rebindPackage && !requiresApproval && !refreshDistribution) return;
     await deps.registry.update(existing.id, {
       version: bundleManifest.version,
       manifest: bundleManifest,

server/src/services/distribution-plugin-catalog.ts

@@ -55,7 +55,7 @@ export function distributionPluginActivationGuard(
   selectedKeys: readonly string[] | null,
 ) {
   const root = distributionPluginsRoot(catalogRoot);
-  return (input: { pluginKey?: string; packageRoot: string; manifest?: PaperclipPluginManifestV1; previousManifest?: PaperclipPluginManifestV1 }) => {
+  return (input: { pluginKey?: string; packageRoot: string; installedPackagePath?: string | null; manifest?: PaperclipPluginManifestV1; previousManifest?: PaperclipPluginManifestV1 }) => {
     let packageRoot: string;
     try { packageRoot = fs.realpathSync(input.packageRoot); }
     catch (error) {
@@ -65,7 +65,12 @@ export function distributionPluginActivationGuard(
     const entry = entries.find((candidate) => input.pluginKey ? candidate.pluginKey === input.pluginKey : candidate.localPath === packageRoot);
     const relative = path.relative(root, packageRoot);
     const inside = relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
-    if (!inside && !entry) return;
+    // A missing image directory can make package resolution fall back to npm.
+    // Retain the persisted image-path provenance even when that directory no
+    // longer exists; the resolved fallback is not an ordinary installation.
+    const installedRelative = input.installedPackagePath ? path.relative(root, path.resolve(input.installedPackagePath)) : null;
+    const installedInside = installedRelative !== null && (installedRelative === "" || (!installedRelative.startsWith("..") && !path.isAbsolute(installedRelative)));
+    if (!inside && !installedInside && !entry) return;
     if (!entry || (selectedKeys !== null && !selectedKeys.includes(entry.key)) || packageRoot !== entry.localPath) {
       throw new Error("Distribution plugin is absent or not selected in this deployment");
     }

server/src/services/plugin-loader.ts

@@ -273,6 +273,8 @@ export interface PluginLoaderOptions {
   assertPackageActivation?: (input: {
     pluginKey?: string;
     packageRoot: string;
+    /** Persisted source path, even when package resolution used a fallback. */
+    installedPackagePath?: string | null;
     manifest?: PaperclipPluginManifestV1;
     /** Persisted grants, supplied before a runtime manifest refresh is saved. */
     previousManifest?: PaperclipPluginManifestV1;
@@ -1393,7 +1395,7 @@ export function pluginLoader(
       );
     }
 
-    assertPackageActivation?.({ packageRoot, pluginKey: plugin.pluginKey, manifest, previousManifest: plugin.manifestJson });
+    assertPackageActivation?.({ packageRoot, installedPackagePath: plugin.packagePath, pluginKey: plugin.pluginKey, manifest, previousManifest: plugin.manifestJson });
     if (JSON.stringify(manifest) === JSON.stringify(plugin.manifestJson)) {
       return plugin;
     }
@@ -2263,10 +2265,10 @@ export function pluginLoader(
       // 1. Resolve worker entrypoint
       // ------------------------------------------------------------------
       const packageRoot = resolvePluginPackageRoot(activePlugin, localPluginDir);
-      assertPackageActivation?.({ pluginKey, packageRoot });
+      assertPackageActivation?.({ pluginKey, packageRoot, installedPackagePath: activePlugin.packagePath });
       activePlugin = await refreshPluginManifestFromPackage(activePlugin, packageRoot);
       manifest = activePlugin.manifestJson;
-      assertPackageActivation?.({ pluginKey, packageRoot, manifest });
+      assertPackageActivation?.({ pluginKey, packageRoot, installedPackagePath: activePlugin.packagePath, manifest });
       const workerEntrypoint = resolveWorkerEntrypoint(activePlugin, localPluginDir);
 
       // ------------------------------------------------------------------

ui/public/sw.js

@@ -5,7 +5,8 @@
 // reloads parked tabs onto the fresh bundle. Left as the literal placeholder in
 // dev, where HMR (not the worker) drives refreshes.
 const BUILD_ID = "__PAPERCLIP_BUILD_ID__";
-const CACHE_NAME = `paperclip-${BUILD_ID}`;
+// Separate this allowlisted cache from older workers that cached arbitrary URLs.
+const CACHE_NAME = `paperclip-public-assets-${BUILD_ID}`;
 const privateRequests = new Set();
 const privateCacheControl = /(?:^|,)\s*(?:no-store|private)(?:\s*(?:,|=)|\s*$)/i;
 
@@ -32,6 +33,11 @@ self.addEventListener("activate", (event) => {
 self.addEventListener("fetch", (event) => {
   const { request } = event;
   const url = new URL(request.url);
+  // Only immutable Vite build assets have a public offline-cache contract.
+  // Never infer that application/extension responses are public from absent
+  // headers, or from an in-memory classification lost when this worker restarts.
+  const publicAsset = url.origin === self.location.origin && !url.search &&
+    /^\/assets\/[^/]+-[a-zA-Z0-9_-]{8,}\.[a-zA-Z0-9.]+$/.test(url.pathname);
 
   // Explicitly private requests must bypass BOTH cache writes and offline
   // fallback, including extension endpoints outside the host /api namespace.
@@ -44,7 +50,7 @@ self.addEventListener("fetch", (event) => {
     return;
   }
 
-  // Network-first for everything — cache is only an offline fallback
+  // Network-first; only public build assets can use an offline fallback.
   event.respondWith(
     fetch(request)
       .then(async (response) => {
@@ -54,7 +60,7 @@ self.addEventListener("fetch", (event) => {
           // if storage is unavailable so offline fallback still fails closed.
           privateRequests.add(request.url);
           await evictRequest(request).catch(() => {});
-        } else if (response.ok && url.origin === self.location.origin && !privateRequests.has(request.url)) {
+        } else if (response.ok && publicAsset && !privateRequests.has(request.url)) {
           const clone = response.clone();
           await caches.open(CACHE_NAME).then(async (cache) => {
             await cache.put(request, clone);
@@ -66,16 +72,14 @@ self.addEventListener("fetch", (event) => {
       })
       .catch(async () => {
         if (privateRequests.has(request.url)) return Response.error();
-        // caches.match() resolves undefined on a miss (and the promise itself
-        // is always truthy, so `||` can never supply a fallback). respondWith
-        // must always receive a real Response — resolving undefined breaks
-        // the navigation with "Failed to convert value to 'Response'" instead
-        // of showing anything.
-        if (request.mode === "navigate") {
-          if (privateRequests.has(new URL("/", self.location.origin).href)) return new Response("Offline", { status: 503 });
-          return (await caches.match("/")) ?? new Response("Offline", { status: 503 });
-        }
-        return (await caches.match(request)) ?? Response.error();
+        if (!publicAsset) return request.mode === "navigate" ? new Response("Offline", { status: 503 }) : Response.error();
+        // Restrict lookup to this policy's cache; old arbitrary-response caches
+        // must not become fallback candidates if activation cleanup fails.
+        try {
+          const cached = await (await caches.open(CACHE_NAME)).match(request);
+          if (cached && !privateCacheControl.test(cached.headers.get("cache-control") ?? "")) return cached;
+        } catch { /* Unavailable cache storage is an offline miss. */ }
+        return Response.error();
       })
   );
 });

ui/src/lib/service-worker-cache-policy.test.ts

@@ -11,11 +11,11 @@ function worker() {
   const fetch = vi.fn().mockResolvedValue(new Response("public asset"));
   vm.runInNewContext(readFileSync(new URL("../../public/sw.js", import.meta.url), "utf8"), {
     self: { location: { origin: "https://example.test" }, addEventListener: (type: string, fn: (event: unknown) => void) => handlers.set(type, fn) },
-    URL, Response, fetch, caches: { keys: async () => ["paperclip-old", "paperclip-current"], open: async () => ({ put, delete: remove }), match },
+    URL, Response, fetch, caches: { keys: async () => ["paperclip-old", "paperclip-current"], open: async () => ({ put, delete: remove, match }), match },
   });
-  const request = (cache: RequestCache = "default") => {
+  const request = (cache: RequestCache = "default", pathname = "/extension/history") => {
     const respondWith = vi.fn();
-    handlers.get("fetch")!({ request: new Request("https://example.test/extension/history", { cache }), respondWith, waitUntil: vi.fn() });
+    handlers.get("fetch")!({ request: new Request(`https://example.test${pathname}`, { cache }), respondWith, waitUntil: vi.fn() });
     return respondWith;
   };
   return { request, fetch, put, match, remove };
@@ -38,8 +38,11 @@ describe("service worker privacy boundaries", () => {
   });
   it("keeps public asset offline caching", async () => {
     const w = worker();
-    await w.request().mock.calls[0]![0];
+    await w.request("default", "/assets/index-AbCd1234.js").mock.calls[0]![0];
     expect(w.put).toHaveBeenCalledOnce();
+    w.fetch.mockRejectedValue(new Error("offline"));
+    w.match.mockResolvedValue(new Response("cached build asset"));
+    expect(await (await w.request("default", "/assets/index-AbCd1234.js").mock.calls[0]![0]).text()).toBe("cached build asset");
   });
   it.each(["private", "no-store"])("evicts stale entries when a response becomes %s and blocks offline reuse", async directive => {
     const w = worker();
@@ -60,4 +63,25 @@ describe("service worker privacy boundaries", () => {
     expect((await w.request().mock.calls[0]![0]).type).toBe("error");
     expect(w.match).not.toHaveBeenCalled();
   });
+
+  it.each(["/extension/history", "/extensions/support/messages", "/", "/assets/avatar.png", "/assets/index-AbCd1234.js?user=1"])("never caches or falls back to uncertain resource %s after a worker restart", async pathname => {
+    const previous = worker();
+    previous.remove.mockRejectedValue(new Error("storage unavailable"));
+    previous.fetch.mockResolvedValue(new Response("personal content", { headers: { "cache-control": "private" } }));
+    await previous.request("default", pathname).mock.calls[0]![0];
+    const restarted = worker();
+    restarted.match.mockResolvedValue(new Response("stale personal content"));
+    await restarted.request("default", pathname).mock.calls[0]![0];
+    expect(restarted.put).not.toHaveBeenCalled();
+    restarted.fetch.mockRejectedValue(new Error("offline"));
+    expect((await restarted.request("default", pathname).mock.calls[0]![0]).type).toBe("error");
+    expect(restarted.match).not.toHaveBeenCalled();
+  });
+
+  it("rejects a cached asset explicitly marked private even after restart", async () => {
+    const w = worker();
+    w.match.mockResolvedValue(new Response("personal content", { headers: { "cache-control": "private" } }));
+    w.fetch.mockRejectedValue(new Error("offline"));
+    expect((await w.request("default", "/assets/index-AbCd1234.js").mock.calls[0]![0]).type).toBe("error");
+  });
 });

ui/src/lib/sw-offline-fallback.test.ts

@@ -76,7 +76,7 @@ describe("sw.js offline fallback", () => {
     expect(await response!.text()).toBe("Offline");
   });
 
-  it("serves the cached shell for a failed navigation when one exists", async () => {
+  it("does not replay a legacy cached shell for a failed navigation", async () => {
     const shell = new Response("<html>app shell</html>", { status: 200 });
     const listener = loadServiceWorkerFetchListener({
       fetch: () => Promise.reject(new TypeError("network down")),
@@ -89,7 +89,8 @@ describe("sw.js offline fallback", () => {
       mode: "navigate",
     });
 
-    expect(response).toBe(shell);
+    expect(response!.status).toBe(503);
+    expect(await response!.text()).toBe("Offline");
   });
 
   it("returns a network-error Response for a failed asset with no cache entry", async () => {