fix: don't auto-enable synced plugins signed only by a server-sent key (#1623)

* fix: don't auto-enable synced plugins signed only by a server-sent key

Registry keys a linked server sends still let its plugins load on the desktop, but no longer count as a signer for switching them on or copying their grants. Keys the desktop added itself keep their trust.

* fix: enforce local plugin trust across remote source labels

* fix: require positive local trust for synced plugin keys

src/backend/plugins/registries.ts

@@ -19,6 +19,9 @@ import {
   type RegistrySource,
 } from "./registry-index.js";
 import {
+  COMMUNITY_PLUGIN_KEYS,
+  TRUSTED_PLUGIN_KEYS,
+  getCustomPluginKeys,
   isPluginPublicKey,
   pluginKeyId,
   setCustomPluginKeys,
@@ -28,6 +31,8 @@ import {
 const CONFIG_KEY = "plugin_registries";
 /** Keys a linked desktop copied from its server, so mirrored plugins load. */
 const SYNCED_KEYS_KEY = "plugin_registry_synced_keys";
+/** `addedIn` of a key only the linked server vouches for. */
+const SYNCED = "synced";
 const ID_PATTERN = /^[a-z0-9][a-z0-9-]*$/;
 const MAX_ID_LENGTH = 40;
 const MAX_NAME_LENGTH = 80;
@@ -117,8 +122,8 @@ async function writeRegistriesConfig(config: RegistriesConfig): Promise<void> {
   await applyCustomKeys(config);
 }
 
-function toKey(publicKey: string): TrustedPluginKey {
-  return { id: pluginKeyId(publicKey), publicKey, addedIn: "admin" };
+function toKey(publicKey: string, addedIn = "admin"): TrustedPluginKey {
+  return { id: pluginKeyId(publicKey), publicKey, addedIn };
 }
 
 async function readSyncedKeys(): Promise<string[]> {
@@ -134,19 +139,32 @@ async function readSyncedKeys(): Promise<string[]> {
 
 /**
  * Trusts the key of every custom registry, on or off: a plugin installed
- * from a registry that was later turned off still has to load.
+ * from a registry that was later turned off still has to load. A key added
+ * here wins over the same key synced from a server.
  */
 async function applyCustomKeys(config: RegistriesConfig): Promise<void> {
   const keys = new Map<string, TrustedPluginKey>();
+  for (const publicKey of await readSyncedKeys()) {
+    keys.set(pluginKeyId(publicKey), toKey(publicKey, SYNCED));
+  }
   for (const entry of config.custom) {
     keys.set(entry.keyId, toKey(entry.publicKey));
   }
-  for (const publicKey of await readSyncedKeys()) {
-    keys.set(pluginKeyId(publicKey), toKey(publicKey));
-  }
   setCustomPluginKeys([...keys.values()]);
 }
 
+/** Whether this app trusts the key independently of its linked server. */
+export function isLocallyTrustedPluginKey(keyId: string): boolean {
+  return (
+    [...TRUSTED_PLUGIN_KEYS, ...COMMUNITY_PLUGIN_KEYS].some(
+      (key) => key.id === keyId,
+    ) ||
+    getCustomPluginKeys().some(
+      (key) => key.id === keyId && key.addedIn !== SYNCED,
+    )
+  );
+}
+
 /** Called before the boot scan so plugins from custom registries load. */
 export async function loadCustomRegistryKeys(): Promise<void> {
   try {

src/backend/sync/client/plugins.ts

@@ -21,7 +21,10 @@ import {
   unloadPlugin,
 } from "../../plugins/index.js";
 import { getPluginsDir } from "../../plugins/paths.js";
-import { setSyncedRegistryKeys } from "../../plugins/registries.js";
+import {
+  isLocallyTrustedPluginKey,
+  setSyncedRegistryKeys,
+} from "../../plugins/registries.js";
 import type { LoadedPlugin } from "../../plugins/loader.js";
 import {
   createCurrentPluginPermissionGrantRepository,
@@ -57,6 +60,14 @@ async function isEnabledLocally(pluginId: string): Promise<boolean> {
   return record?.state === "enabled";
 }
 
+/**
+ * Signed by a key this desktop trusts on its own. A key the server sent
+ * would let the server's admin sign anything, so it does not count.
+ */
+function vouchedHere(local: LoadedPlugin): boolean {
+  return !!local.signedBy && isLocallyTrustedPluginKey(local.signedBy);
+}
+
 const PLUGIN_ID_PATTERN = /^[a-z0-9][a-z0-9-]*$/;
 
 /** Downloads next to the installed copy, which stays until the swap. */
@@ -155,7 +166,7 @@ async function matchGrants(plugin: RemotePlugin): Promise<void> {
   if (!local || local.source !== "user") return;
   // Grants are only copied for code a trusted key signed. Anything else the
   // desktop's own admin has to review, as with any plugin dropped on disk.
-  if (!local.signedBy) return;
+  if (!vouchedHere(local)) return;
   const declared = new Set(local.manifest.capabilities);
   const wanted = new Set(plugin.granted.filter((cap) => declared.has(cap)));
   const existing = await repository.listByPlugin(plugin.id);
@@ -196,7 +207,7 @@ async function alertUnsigned(plugin: RemotePlugin): Promise<void> {
   alertedVersions.add(key);
   await sendCoreAlert({
     title: "Review a feature from your server",
-    body: `${plugin.name} (${plugin.version}) was copied from your server but is not signed, so it stays off here until you turn it on.`,
+    body: `${plugin.name} (${plugin.version}) was copied from your server but is not signed by a key this app trusts, so it stays off here until you turn it on.`,
     severity: "warning",
     category: "termix.sync.unsigned_plugin",
     dedupeKey: `sync:${key}`,
@@ -239,7 +250,9 @@ async function mirrorOne(link: SyncLink, plugin: RemotePlugin): Promise<void> {
         await alertOlderDesktop(plugin);
         throw error;
       }
-      if (wasEnabled) await setPluginEnabled(plugin.id, true);
+      if (wasEnabled && local.source === "bundled") {
+        await setPluginEnabled(plugin.id, true);
+      }
     } else if (newer) {
       await alertOlderDesktop(plugin);
     }
@@ -263,19 +276,19 @@ async function mirrorOne(link: SyncLink, plugin: RemotePlugin): Promise<void> {
       }
     }
     await matchGrants(plugin);
-    // Unsigned code is only ever switched off from here, never on.
-    if (!local.signedBy) {
-      const enabledHere = await isEnabledLocally(plugin.id);
-      if (!plugin.enabled && enabledHere) {
-        await setPluginEnabled(plugin.id, false);
-      } else if (plugin.enabled && !enabledHere) {
-        await alertUnsigned(plugin);
-      }
-      return;
-    }
   }
 
   if (!local) return;
+  // Remote metadata cannot confer bundled trust on a locally loaded user plugin.
+  if (local.source === "user" && !vouchedHere(local)) {
+    const enabledHere = await isEnabledLocally(plugin.id);
+    if (!plugin.enabled && enabledHere) {
+      await setPluginEnabled(plugin.id, false);
+    } else if (plugin.enabled && !enabledHere) {
+      await alertUnsigned(plugin);
+    }
+    return;
+  }
   if (plugin.enabled !== (await isEnabledLocally(plugin.id))) {
     await setPluginEnabled(plugin.id, plugin.enabled);
   } else if (plugin.enabled && local.state !== "active") {

src/backend/tests/plugins/registries.test.ts

@@ -256,4 +256,25 @@ describe("keys from a linked server", () => {
     await registries.setSyncedRegistryKeys([]);
     expect(getCustomPluginKeys()).toEqual([]);
   });
+
+  it("marks them as synced unless an admin added the same key here", async () => {
+    const synced = newKey();
+    const shared = newKey();
+    serve(URL_A, { registry: "acme", publicKey: shared });
+    await registries.addRegistry(URL_A, pluginKeyId(shared));
+    await registries.setSyncedRegistryKeys([synced, shared]);
+
+    expect(registries.isLocallyTrustedPluginKey(pluginKeyId(synced))).toBe(
+      false,
+    );
+    expect(registries.isLocallyTrustedPluginKey(pluginKeyId(shared))).toBe(
+      true,
+    );
+    expect(registries.isLocallyTrustedPluginKey("ec40e29bcc4986b2")).toBe(true);
+    await registries.setSyncedRegistryKeys([]);
+    expect(registries.isLocallyTrustedPluginKey(pluginKeyId(synced))).toBe(
+      false,
+    );
+    expect(registries.isLocallyTrustedPluginKey("unknown-key")).toBe(false);
+  });
 });

src/backend/tests/sync/client/plugins.test.ts

@@ -11,6 +11,8 @@ const state = vi.hoisted(() => ({
   fetch: vi.fn(),
   loadBundled: vi.fn(),
   syncedKeys: vi.fn(async (_keys: unknown[]) => {}),
+  syncedOnlyKeys: new Set<string>(),
+  grants: [] as string[],
 }));
 
 vi.mock("../../../utils/logger.js", () => ({
@@ -28,10 +30,16 @@ vi.mock("../../../database/repositories/factory.js", () => ({
       state: state.enabled.has(id) ? "enabled" : "disabled",
     }),
   }),
-  createCurrentPluginPermissionGrantRepository: () => ({}),
+  createCurrentPluginPermissionGrantRepository: () => ({
+    listByPlugin: async () => [],
+    grant: async ({ capability }: { capability: string }) =>
+      void state.grants.push(capability),
+  }),
 }));
 vi.mock("../../../plugins/registries.js", () => ({
   setSyncedRegistryKeys: state.syncedKeys,
+  isLocallyTrustedPluginKey: (keyId: string) =>
+    keyId === "local-key" && !state.syncedOnlyKeys.has(keyId),
 }));
 vi.mock("../../../sync/client/http.js", () => ({
   remoteFetch: state.fetch,
@@ -58,6 +66,7 @@ vi.mock("../../../plugins/index.js", () => ({
 
 const { mirrorPlugins } = await import("../../../sync/client/plugins.js");
 const { remoteJson } = await import("../../../sync/client/http.js");
+const { sendCoreAlert } = await import("../../../notify/core-notify.js");
 
 const bundled = {
   id: "ssh-terminal",
@@ -92,6 +101,9 @@ beforeEach(() => {
   state.install.mockReset();
   state.fetch.mockReset();
   state.loadBundled.mockReset();
+  state.syncedOnlyKeys = new Set();
+  state.grants = [];
+  vi.mocked(sendCoreAlert).mockClear();
   state.loadBundled.mockImplementation(async (id: string) => {
     const plugin = { ...bundled, id };
     state.loaded.set(id, plugin);
@@ -171,4 +183,87 @@ describe("mirrorPlugins", () => {
     expect(state.loaded.has("ssh-terminal")).toBe(true);
     expect(state.enabled.has("ssh-terminal")).toBe(true);
   });
+
+  describe("a user plugin the server has on", () => {
+    function serverRunsSignedBy(keyId: string) {
+      const version = keyId === "removed-key" ? "1.0.1" : "1.0.0";
+      state.loaded.set("extra", {
+        id: "extra",
+        source: "user",
+        state: "loaded",
+        signedBy: keyId,
+        manifest: { id: "extra", version, capabilities: ["net"] },
+      });
+      vi.mocked(remoteJson).mockResolvedValue({
+        plugins: [
+          {
+            id: "extra",
+            name: "Extra",
+            version,
+            source: "user",
+            artifact: true,
+            enabled: true,
+            active: true,
+            desktop: "mirror",
+            syncEntities: [],
+            granted: ["net"],
+          },
+        ],
+      });
+    }
+
+    it("turns it on when a key this desktop trusts signed it", async () => {
+      serverRunsSignedBy("local-key");
+
+      await mirrorPlugins({} as never);
+
+      expect(state.enabled.has("extra")).toBe(true);
+      expect(state.grants).toEqual(["net"]);
+    });
+
+    it.each(["server-key", "removed-key"])(
+      "leaves it off for untrusted key %s",
+      async (keyId) => {
+        serverRunsSignedBy(keyId);
+        state.syncedOnlyKeys.add("server-key");
+
+        await mirrorPlugins({} as never);
+
+        expect(state.enabled.has("extra")).toBe(false);
+        expect(state.grants).toEqual([]);
+        expect(sendCoreAlert).toHaveBeenCalledWith(
+          expect.objectContaining({ category: "termix.sync.unsigned_plugin" }),
+        );
+      },
+    );
+  });
+});
+
+it("server bundled label must not enable a synced-only user plugin", async () => {
+  state.syncedOnlyKeys.add("server-key");
+  state.loaded.set("extra", {
+    id: "extra",
+    source: "user",
+    state: "loaded",
+    signedBy: "server-key",
+    manifest: { id: "extra", version: "1.0.0", capabilities: [] },
+  });
+  vi.mocked(remoteJson).mockResolvedValue({
+    plugins: [
+      {
+        id: "extra",
+        name: "Extra",
+        version: "1.0.0",
+        source: "bundled",
+        artifact: true,
+        enabled: true,
+        active: true,
+        desktop: "mirror",
+        syncEntities: [],
+        granted: [],
+      },
+    ],
+  });
+  await mirrorPlugins({} as never);
+  expect(state.enabled.has("extra")).toBe(false);
 });