diff --git a/src/backend/database/routes/plugins.ts b/src/backend/database/routes/plugins.ts index d37fa1002..73da18085 100644 --- a/src/backend/database/routes/plugins.ts +++ b/src/backend/database/routes/plugins.ts @@ -243,7 +243,9 @@ export async function listPreLoginPlugins(): Promise< * frontends in the same order the server does. * * On a desktop linked to a server, `runsOnServer` marks plugins that - * only run on that server and stay off here on purpose. + * only run on that server and stay off here on purpose, and, for + * admins, `managedByServer` marks plugins whose on/off state the server + * decides. * tags: * - Plugins * responses: @@ -370,6 +372,7 @@ router.get("/", authenticateJWT, async (req: Request, res: Response) => { features, env, signedBy: loaded?.signedBy ?? null, + managedByServer: linked && (await isManagedByLinkedServer(record.id)), capabilities, grantedCapabilities: grantsByPlugin.get(record.id) ?? [], lastError: loaded?.lastError ?? record.lastError ?? null, @@ -393,9 +396,7 @@ router.get("/", authenticateJWT, async (req: Request, res: Response) => { }); async function isManagedByLinkedServer(pluginId: string): Promise { - if (process.env.ELECTRON_EMBEDDED !== "true") return false; - const { getLink } = await import("../../sync/client/link-store.js"); - if (!(await getLink())) return false; + if (!(await isLinkedDesktop())) return false; const { isServerManaged } = await import("../../sync/client/plugins.js"); return isServerManaged(pluginId); } @@ -500,7 +501,7 @@ router.get( * /plugins/onboarding/apply: * post: * summary: Apply the onboarding plugin picker - * description: Sets every listed plugin to enabled, disabled or removed in one pass. Choices are first raised so that every kept plugin still has its hard dependencies (the adjustments list says which and why). Enabling runs dependencies first, disabling and removing run dependents first, and one plugin failing does not stop the rest. Removing a bundled plugin deletes it and its data; it can be reinstalled from the registry later. Clears the fresh-install pending flag. With dryRun nothing changes and the response lists what would. + * description: Sets every listed plugin to enabled, disabled or removed in one pass. Choices are first raised so that every kept plugin still has its hard dependencies (the adjustments list says which and why). Enabling runs dependencies first, disabling and removing run dependents first, and one plugin failing does not stop the rest. Removing a bundled plugin deletes it and its data; it can be reinstalled from the registry later. Clears the fresh-install pending flag. With dryRun nothing changes and the response lists what would. On a linked desktop, locally managed plugins require version and capabilities matching the reviewed manifest when enabling. * tags: * - Plugins * requestBody: @@ -887,7 +888,7 @@ router.delete( * /plugins/{id}/state: * patch: * summary: Enable or disable a plugin - * description: Persists the new state and starts or stops the plugin immediately. Disabling releases everything the plugin owns, including any port it was listening on. Enabling also starts any installed hard dependency that is off, and disabling also stops the running plugins that depend on it. With dryRun nothing changes and the response lists what would. + * description: Persists the new state and starts or stops the plugin immediately. Disabling releases everything the plugin owns, including any port it was listening on. Enabling also starts any installed hard dependency that is off, and disabling also stops the running plugins that depend on it. With dryRun nothing changes and the response lists what would. On a linked desktop, locally managed plugins require version and capabilities matching the reviewed manifest when enabling. * tags: * - Plugins * parameters: @@ -909,6 +910,12 @@ router.delete( * properties: * enabled: * type: boolean + * version: + * type: string + * capabilities: + * type: array + * items: + * type: string * responses: * 200: * description: The plugin's new state. @@ -962,7 +969,26 @@ router.patch( return; } - const result = await setPluginState(pluginId, enabled); + let result; + if (enabled && (await isLinkedDesktop())) { + const version = readVersion(req.body); + const capabilities = readCapabilities(req.body); + if (version === null || capabilities === null) { + res + .status(400) + .json({ error: "Invalid plugin version or capabilities" }); + return; + } + const { enablePluginWithConsent } = + await import("../../plugins/manage.js"); + result = await enablePluginWithConsent(pluginId, { + version, + capabilities, + userId: (req as AuthenticatedRequest).userId as string, + }); + } else { + result = await setPluginState(pluginId, enabled); + } await auditPluginAction( req, enabled ? "enable_plugin" : "disable_plugin", diff --git a/src/backend/plugins/manage.ts b/src/backend/plugins/manage.ts index d4f586bee..6fd33de7e 100644 --- a/src/backend/plugins/manage.ts +++ b/src/backend/plugins/manage.ts @@ -715,6 +715,28 @@ export async function planStateChange( }; } +/** A local admin consents to the exact mirrored version before granting it access. */ +export function enablePluginWithConsent( + pluginId: string, + options: { version?: string; capabilities?: string[]; userId: string }, +): Promise { + return serialized(async () => { + const plugin = getPluginRuntime().loader.get(pluginId); + if (!plugin) + throw new PluginManageError("Plugin not found", 404, "NOT_FOUND"); + if (options.version !== plugin.manifest.version) { + throw new PluginManageError( + "The plugin version changed; review it again", + 409, + "CONSENT_MISMATCH", + ); + } + assertConsent(options.capabilities, plugin.manifest.capabilities); + await grantDeclared(plugin, options.userId); + return setPluginStateUnlocked(pluginId, true); + }); +} + export function setPluginState( pluginId: string, enabled: boolean, diff --git a/src/backend/sync/client/plugins.ts b/src/backend/sync/client/plugins.ts index 8aabf6213..ad841b539 100644 --- a/src/backend/sync/client/plugins.ts +++ b/src/backend/sync/client/plugins.ts @@ -4,7 +4,8 @@ * For every plugin in "mirror" mode, the server decides: a plugin installed * there is installed here, on or off to match, with the capabilities an * admin granted it there. Plugins marked "local" are left to the desktop and - * "server" ones never run on a linked desktop. + * "server" ones never run on a linked desktop. A user plugin no key trusted + * here signed is only ever switched off; the desktop's admin turns it on. */ import fs from "fs"; @@ -123,6 +124,10 @@ async function replaceWithDownload( if (previous) await unloadPlugin(pluginId); try { const installed = await installPluginArtifact(file); + // Approval belongs to the reviewed version, regardless of remote metadata. + if (wasEnabled && installed.source === "user" && !vouchedHere(installed)) { + await setPluginEnabled(pluginId, false); + } await fs.promises.rm(backup, { force: true }); await fs.promises.rm(`${backup}.sig`, { force: true }); return installed; @@ -297,6 +302,7 @@ async function mirrorOne(link: SyncLink, plugin: RemotePlugin): Promise { } export async function mirrorPlugins(link: SyncLink): Promise { + const { serialized } = await import("../../plugins/manage.js"); const { plugins, registryKeys } = await remoteJson<{ plugins: RemotePlugin[]; registryKeys?: unknown[]; @@ -308,7 +314,8 @@ export async function mirrorPlugins(link: SyncLink): Promise { for (const plugin of plugins) { try { - await mirrorOne(link, plugin); + // Share the consent/enable lock so an update cannot swap code mid-approval. + await serialized(() => mirrorOne(link, plugin)); } catch (error) { syncLogger.warn("Could not match a feature to the server", { operation: "sync_plugin_mirror", @@ -328,8 +335,13 @@ export async function mirrorPlugins(link: SyncLink): Promise { } } -/** Whether a plugin's on/off state belongs to the linked server. */ +/** + * Whether a plugin's on/off state belongs to the linked server. Code no key + * trusted here vouches for is the desktop's own admin's call to turn on. + */ export function isServerManaged(pluginId: string): boolean { const local = getPluginRuntime().loader.get(pluginId); - return (local?.manifest.desktop ?? "mirror") !== "local"; + const mode = local?.manifest.desktop ?? "mirror"; + if (mode !== "mirror") return mode === "server"; + return local?.source !== "user" || vouchedHere(local); } diff --git a/src/backend/tests/database/routes/plugins.test.ts b/src/backend/tests/database/routes/plugins.test.ts index 558c27447..a554d4271 100644 --- a/src/backend/tests/database/routes/plugins.test.ts +++ b/src/backend/tests/database/routes/plugins.test.ts @@ -176,6 +176,7 @@ const manageMock = vi.hoisted(() => ({ deletePluginData: vi.fn(), planStateChange: vi.fn(), setPluginState: vi.fn(), + enablePluginWithConsent: vi.fn(), getDeveloperMode: vi.fn(), setDeveloperMode: vi.fn(), stageUpload: vi.fn(), @@ -215,6 +216,12 @@ const onboardingMock = vi.hoisted(() => ({ vi.mock("../../../plugins/onboarding.js", () => onboardingMock); +const syncPluginsMock = vi.hoisted(() => ({ + isServerManaged: vi.fn((_id: string) => true), +})); + +vi.mock("../../../sync/client/plugins.js", () => syncPluginsMock); + const pluginRoutes = (await import("../../../database/routes/plugins.js")) .default; @@ -1139,6 +1146,65 @@ describe("plugins route", () => { expect(manageMock.installPlugin).not.toHaveBeenCalled(); }); + describe("on a linked desktop", () => { + const switchOn = (id: string) => + fetch(`${baseUrl}/plugins/${id}/state`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + enabled: true, + version: "1.0.0", + capabilities: ["kv:own"], + }), + }); + + beforeEach(() => { + manageMock.isLinkedDesktop.mockResolvedValue(true); + state.plugins.set("sample-plugin", makePlugin({ state: "disabled" })); + }); + + afterEach(() => { + syncPluginsMock.isServerManaged.mockImplementation(() => true); + }); + + it("refuses to switch a plugin the server decides", async () => { + const res = await switchOn("sample-plugin"); + expect(res.status).toBe(409); + expect((await res.json()).code).toBe("MANAGED_BY_SERVER"); + expect(manageMock.setPluginState).not.toHaveBeenCalled(); + }); + + it("lets the desktop's admin switch one the server does not", async () => { + syncPluginsMock.isServerManaged.mockImplementation(() => false); + manageMock.enablePluginWithConsent.mockResolvedValue({ + enable: [], + disable: [], + }); + const res = await switchOn("sample-plugin"); + expect(res.status).toBe(200); + expect(manageMock.enablePluginWithConsent).toHaveBeenCalledWith( + "sample-plugin", + expect.objectContaining({ + version: "1.0.0", + capabilities: ["kv:own"], + }), + ); + }); + + it("says per plugin whether the server decides it", async () => { + state.plugins.set("other", makePlugin({ id: "other" })); + syncPluginsMock.isServerManaged.mockImplementation( + (id) => id === "other", + ); + const listed = (await ( + await fetch(`${baseUrl}/plugins`) + ).json()) as Array<{ id: string; managedByServer: boolean }>; + expect( + Object.fromEntries(listed.map((p) => [p.id, p.managedByServer])), + ).toEqual({ "sample-plugin": false, other: true }); + }); + }); + it("previews a state change without making it", async () => { state.plugins.set("sample-plugin", makePlugin()); manageMock.planStateChange.mockResolvedValue({ diff --git a/src/backend/tests/plugins/manage.test.ts b/src/backend/tests/plugins/manage.test.ts index 485f41106..7ea95dba7 100644 --- a/src/backend/tests/plugins/manage.test.ts +++ b/src/backend/tests/plugins/manage.test.ts @@ -91,8 +91,8 @@ vi.mock("../../plugins/data.js", () => ({ forgetTables: () => {}, })); -vi.mock("../../plugins/permissions.js", () => ({ - invalidatePluginPermissionCache: () => {}, +vi.mock("../../plugins/permissions.js", async (importOriginal) => ({ + ...(await importOriginal()), })); vi.mock("../../plugins/registry-index.js", async (importOriginal) => { @@ -1084,3 +1084,48 @@ describe("more registries", () => { ).rejects.toMatchObject({ code: "KEY_CHANGED" }); }); }); + +it("requires exact consent before enabling a mirrored plugin with capabilities", async () => { + createFixturePlugin({ + id: "manual-mirror", + root: path.join(data, "plugins"), + capabilities: ["kv:own"], + backendSource: + 'export async function activate(ctx) { await ctx.kv.set("probe", "value"); }', + }); + await boot(); + const options = { + version: "1.0.0", + capabilities: ["kv:own"], + userId: "admin", + }; + await expect( + manage.enablePluginWithConsent("manual-mirror", { + ...options, + version: "0.9.0", + }), + ).rejects.toMatchObject({ code: "CONSENT_MISMATCH" }); + await expect( + manage.enablePluginWithConsent("manual-mirror", { + ...options, + capabilities: [], + }), + ).rejects.toMatchObject({ code: "CONSENT_MISMATCH" }); + await expect( + manage.enablePluginWithConsent("manual-mirror", { + version: "1.0.0", + userId: "admin", + }), + ).rejects.toMatchObject({ code: "CONSENT_REQUIRED" }); + expect(db.grants).toEqual([]); + expect(db.rows.get("manual-mirror")?.state).toBe("disabled"); + const result = await manage.enablePluginWithConsent("manual-mirror", options); + expect(result.state).toBe("active"); + expect(db.grants).toContainEqual( + expect.objectContaining({ + pluginId: "manual-mirror", + capability: "kv:own", + grantedBy: "admin", + }), + ); +}); diff --git a/src/backend/tests/sync/client/plugins.test.ts b/src/backend/tests/sync/client/plugins.test.ts index 5581a898a..6d33439be 100644 --- a/src/backend/tests/sync/client/plugins.test.ts +++ b/src/backend/tests/sync/client/plugins.test.ts @@ -64,7 +64,8 @@ vi.mock("../../../plugins/index.js", () => ({ }), })); -const { mirrorPlugins } = await import("../../../sync/client/plugins.js"); +const { mirrorPlugins, isServerManaged } = + await import("../../../sync/client/plugins.js"); const { remoteJson } = await import("../../../sync/client/http.js"); const { sendCoreAlert } = await import("../../../notify/core-notify.js"); @@ -185,8 +186,11 @@ describe("mirrorPlugins", () => { }); describe("a user plugin the server has on", () => { - function serverRunsSignedBy(keyId: string) { - const version = keyId === "removed-key" ? "1.0.1" : "1.0.0"; + function serverRunsSignedBy( + keyId: string, + enabled = true, + version = keyId === "removed-key" ? "1.0.1" : "1.0.0", + ) { state.loaded.set("extra", { id: "extra", source: "user", @@ -202,8 +206,8 @@ describe("mirrorPlugins", () => { version, source: "user", artifact: true, - enabled: true, - active: true, + enabled, + active: enabled, desktop: "mirror", syncEntities: [], granted: ["net"], @@ -236,6 +240,67 @@ describe("mirrorPlugins", () => { ); }, ); + + it("leaves the server in charge of one a trusted key signed", () => { + serverRunsSignedBy("local-key"); + expect(isServerManaged("extra")).toBe(true); + }); + + it.each(["server-key", "removed-key"])( + "lets this desktop's admin decide on one signed by %s", + async (keyId) => { + serverRunsSignedBy(keyId); + state.syncedOnlyKeys.add("server-key"); + expect(isServerManaged("extra")).toBe(false); + + state.enabled.add("extra"); + await mirrorPlugins({} as never); + expect(state.enabled.has("extra")).toBe(true); + }, + ); + + it("still switches an untrusted one off when the server does", async () => { + serverRunsSignedBy("server-key", false); + state.syncedOnlyKeys.add("server-key"); + state.enabled.add("extra"); + + await mirrorPlugins({} as never); + + expect(state.enabled.has("extra")).toBe(false); + }); + + it.each([ + ["server-key", false], + ["local-key", true], + ])( + "after an update signed by %s, keeps it on: %s", + async (keyId, stays) => { + serverRunsSignedBy(keyId, true, "2.0.0"); + state.syncedOnlyKeys.add("server-key"); + const reviewed = state.loaded.get("extra")!; + state.loaded.set("extra", { + ...reviewed, + manifest: { id: "extra", version: "1.0.0", capabilities: ["net"] }, + }); + state.enabled.add("extra"); + state.fetch.mockResolvedValue({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(4), + headers: new Headers(), + }); + state.install.mockImplementation(async () => { + const plugin = { ...reviewed }; + state.loaded.set("extra", plugin); + return plugin; + }); + + await mirrorPlugins({} as never); + + expect(state.install).toHaveBeenCalled(); + expect(state.enabled.has("extra")).toBe(stays); + expect(vi.mocked(sendCoreAlert).mock.calls.length > 0).toBe(!stays); + }, + ); }); }); @@ -267,3 +332,78 @@ it("server bundled label must not enable a synced-only user plugin", async () => await mirrorPlugins({} as never); expect(state.enabled.has("extra")).toBe(false); }); + +it("remote bundled label cannot preserve approval across an untrusted update", async () => { + const reviewed = { + id: "extra", + source: "user", + state: "loaded", + signedBy: "server-key", + manifest: { id: "extra", version: "1.0.0", capabilities: [] }, + }; + state.loaded.set("extra", reviewed); + state.syncedOnlyKeys.add("server-key"); + state.enabled.add("extra"); + vi.mocked(remoteJson).mockResolvedValue({ + plugins: [ + { + id: "extra", + name: "Extra", + version: "2.0.0", + source: "bundled", + artifact: true, + enabled: true, + active: true, + desktop: "mirror", + syncEntities: [], + granted: [], + }, + ], + }); + state.fetch.mockResolvedValue({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(4), + headers: new Headers(), + }); + state.install.mockImplementation(async () => { + const plugin = { + ...reviewed, + manifest: { ...reviewed.manifest, version: "2.0.0" }, + }; + state.loaded.set("extra", plugin); + return plugin; + }); + await mirrorPlugins({} as never); + expect(state.install).toHaveBeenCalledOnce(); + expect(state.enabled.has("extra")).toBe(false); +}); + +it("waits for an in-flight local plugin change before applying a remote update", async () => { + const { serialized } = await import("../../../plugins/manage.js"); + let release!: () => void; + let entered!: () => void; + const ready = new Promise((resolve) => { + entered = resolve; + }); + const barrier = new Promise((resolve) => { + release = resolve; + }); + const localChange = serialized(async () => { + entered(); + await barrier; + }); + await ready; + serverHas("1.1.0"); + state.fetch.mockResolvedValue({ ok: false, status: 502 }); + vi.mocked(remoteJson).mockClear(); + const syncing = mirrorPlugins({} as never); + try { + await vi.waitFor(() => expect(remoteJson).toHaveBeenCalled()); + expect(state.fetch).not.toHaveBeenCalled(); + } finally { + release(); + } + await localChange; + await syncing; + expect(state.fetch).toHaveBeenCalledOnce(); +}); diff --git a/src/ui/api/plugins-api.ts b/src/ui/api/plugins-api.ts index ae62c1d81..4d46e3629 100644 --- a/src/ui/api/plugins-api.ts +++ b/src/ui/api/plugins-api.ts @@ -263,6 +263,8 @@ export interface PluginSummary { /** Environment variables the plugin reads. Admins only. */ env?: PluginEnvVar[]; signedBy?: string | null; + /** A linked server decides whether it runs here. Admins only. */ + managedByServer?: boolean; } export async function getPlugins(): Promise { @@ -510,10 +512,12 @@ export async function previewPluginState( export async function setPluginState( pluginId: string, enabled: boolean, + consent?: { version: string; capabilities: string[] }, ): Promise { try { const response = await rbacApi.patch(`${pluginPath(pluginId)}/state`, { enabled, + ...consent, }); return response.data; } finally { diff --git a/src/ui/locales/en.json b/src/ui/locales/en.json index cbefbe5a5..182747207 100644 --- a/src/ui/locales/en.json +++ b/src/ui/locales/en.json @@ -1967,6 +1967,7 @@ "syncEntities_other": "Desktop sync" }, "consent": { + "enableTitle": "Enable {{name}}?", "installTitle": "Install {{name}}", "updateTitle": "Update {{name}}", "agreeUpdate": "Agree and update", diff --git a/src/ui/plugins/PluginConsentPrompt.tsx b/src/ui/plugins/PluginConsentPrompt.tsx index fa54bac7d..333ba83b2 100644 --- a/src/ui/plugins/PluginConsentPrompt.tsx +++ b/src/ui/plugins/PluginConsentPrompt.tsx @@ -8,7 +8,7 @@ import { capabilityRisk, orderByRisk, type PluginEntry } from "./plugin-model"; export interface ConsentRequest { plugin: PluginEntry; - mode: "install" | "update" | "upload"; + mode: "install" | "update" | "upload" | "enable"; version: string; /** What the prompt lists: all of them to install, only the new ones to update. */ capabilities: string[]; @@ -45,17 +45,19 @@ export function PluginConsentPrompt({ onCancel={busy ? undefined : onCancel} icon={} title={ - mode === "update" - ? t("plugins.manager.consent.updateTitle", { name: plugin.name }) - : mode === "upload" - ? t("plugins.manager.consent.uploadTitle", { name: plugin.name }) - : t("plugins.manager.consent.installTitle", { name: plugin.name }) + mode === "enable" + ? t("plugins.manager.consent.enableTitle", { name: plugin.name }) + : mode === "update" + ? t("plugins.manager.consent.updateTitle", { name: plugin.name }) + : mode === "upload" + ? t("plugins.manager.consent.uploadTitle", { name: plugin.name }) + : t("plugins.manager.consent.installTitle", { name: plugin.name }) } description={ {plugin.author && {plugin.author}} - {mode !== "install" && plugin.version + {(mode === "update" || mode === "upload") && plugin.version ? t("plugins.manager.versionChange", { from: plugin.version, to: version, @@ -75,9 +77,11 @@ export function PluginConsentPrompt({ disabled={busy} className="border-accent-brand/40 text-accent-brand hover:bg-accent-brand/10 hover:text-accent-brand dark:border-accent-brand/40 dark:bg-transparent dark:hover:bg-accent-brand/10" > - {mode === "update" - ? t("plugins.manager.consent.agreeUpdate") - : t("plugins.manager.install")} + {mode === "enable" + ? t("plugins.manager.enable") + : mode === "update" + ? t("plugins.manager.consent.agreeUpdate") + : t("plugins.manager.install")} } diff --git a/src/ui/plugins/PluginDetail.tsx b/src/ui/plugins/PluginDetail.tsx index 171b33e04..b31a1bb7e 100644 --- a/src/ui/plugins/PluginDetail.tsx +++ b/src/ui/plugins/PluginDetail.tsx @@ -177,7 +177,7 @@ export function PluginDetail({