From 9408daaf5e48445928842f17057c2e0d057cedbf Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:25:45 +0200 Subject: [PATCH] fix: keep an extension's admin state on re-upload and remove its config when it is deleted --- app/api/admin/plugins/__tests__/route.test.ts | 27 ++++++++++++++++++- app/api/admin/plugins/route.ts | 9 ++++++- app/api/admin/themes/route.ts | 7 +++-- lib/admin/extension-cleanup.ts | 11 ++++++++ lib/admin/plugin-config.ts | 6 +++++ 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/app/api/admin/plugins/__tests__/route.test.ts b/app/api/admin/plugins/__tests__/route.test.ts index 264a0badd..9d2ce079b 100644 --- a/app/api/admin/plugins/__tests__/route.test.ts +++ b/app/api/admin/plugins/__tests__/route.test.ts @@ -38,7 +38,7 @@ vi.mock('@/lib/admin/csp-frame-origins', async (orig) => ({ invalidateFrameOriginsCache: vi.fn(), })); -import { POST } from '../route'; +import { DELETE, POST } from '../route'; const MANIFEST = { id: 'demo-plugin', name: 'Demo', version: '1.0.0', author: 'me', description: 'd', @@ -100,3 +100,28 @@ describe('POST /api/admin/plugins', () => { expect(res.status).toBe(400); }); }); + +describe('re-uploading and deleting', () => { + it('keeps a plugin the admin disabled disabled when a new version is uploaded', async () => { + registry.set('demo-plugin', { id: 'demo-plugin', enabled: false, forceEnabled: true }); + + await upload({ 'manifest.json': JSON.stringify({ ...MANIFEST, version: '1.1.0' }), 'index.js': '' }); + + expect(registry.get('demo-plugin')).toMatchObject({ version: '1.1.0', enabled: false, forceEnabled: true }); + }); + + it('drops the config and the forced flag when a plugin is deleted', async () => { + registry.set('demo-plugin', { id: 'demo-plugin' }); + policy = { forceEnabledPlugins: ['demo-plugin', 'other'] }; + + const res = await DELETE(new NextRequest('http://localhost/api/admin/plugins', { + method: 'DELETE', + body: JSON.stringify({ id: 'demo-plugin' }), + headers: { 'content-type': 'application/json' }, + })); + + expect(res.status).toBe(200); + expect(removedConfig).toEqual(['demo-plugin']); + expect(policy.forceEnabledPlugins).toEqual(['other']); + }); +}); diff --git a/app/api/admin/plugins/route.ts b/app/api/admin/plugins/route.ts index 41542a25b..2573921ab 100644 --- a/app/api/admin/plugins/route.ts +++ b/app/api/admin/plugins/route.ts @@ -3,6 +3,7 @@ import { requireAdminAuth, getClientIP } from '@/lib/admin/session'; import { auditLog } from '@/lib/admin/audit'; import { logger } from '@/lib/logger'; import { + getPlugin, getPluginRegistry, savePlugin, deletePlugin as removePlugin, @@ -10,6 +11,7 @@ import { } from '@/lib/admin/plugin-registry'; import { listDevPlugins } from '@/lib/admin/plugin-dev'; import { invalidateFrameOriginsCache } from '@/lib/admin/csp-frame-origins'; +import { cleanUpDeletedPlugin } from '@/lib/admin/extension-cleanup'; import { ExtensionPackageError, readPluginPackage, suspiciousCode } from '@/lib/extension-package'; import { MAX_PLUGIN_SIZE } from '@/lib/plugin-types'; @@ -86,6 +88,9 @@ export async function POST(request: NextRequest) { ); } + // A new upload of an installed plugin is an update: keep whether the + // admin had it enabled or forced instead of switching it on for everyone. + const existing = await getPlugin(manifest.id); const now = new Date().toISOString(); const plugin: ServerPlugin = { id: manifest.id, @@ -97,7 +102,8 @@ export async function POST(request: NextRequest) { ...(manifest.tier === 'privileged' ? { tier: 'privileged' } : {}), permissions: manifest.permissions, entrypoint: manifest.entrypoint, - enabled: true, + enabled: existing?.enabled ?? true, + ...(existing?.forceEnabled !== undefined ? { forceEnabled: existing.forceEnabled } : {}), ...(manifest.configSchema ? { configSchema: manifest.configSchema } : {}), ...(manifest.settingsSchema ? { settingsSchema: manifest.settingsSchema } : {}), ...(manifest.locales ? { locales: manifest.locales } : {}), @@ -193,6 +199,7 @@ export async function DELETE(request: NextRequest) { if (!removed) { return NextResponse.json({ error: 'Plugin not found' }, { status: 404 }); } + await cleanUpDeletedPlugin(id); invalidateFrameOriginsCache(); await auditLog('plugin.delete', { id }, ip); diff --git a/app/api/admin/themes/route.ts b/app/api/admin/themes/route.ts index 9f07bf34f..3950021aa 100644 --- a/app/api/admin/themes/route.ts +++ b/app/api/admin/themes/route.ts @@ -3,6 +3,7 @@ import { requireAdminAuth, getClientIP } from '@/lib/admin/session'; import { auditLog } from '@/lib/admin/audit'; import { logger } from '@/lib/logger'; import { + getTheme, getThemeRegistry, saveTheme, deleteTheme as removeTheme, @@ -55,6 +56,7 @@ export async function POST(request: NextRequest) { } const { manifest, css, skin, warnings } = read; + const existing = await getTheme(manifest.id); const now = new Date().toISOString(); const theme: ServerTheme = { id: manifest.id, @@ -63,8 +65,9 @@ export async function POST(request: NextRequest) { author: manifest.author, description: manifest.description || '', variants: manifest.variants, - enabled: true, - installedAt: now, + enabled: existing?.enabled ?? true, + ...(existing?.forceEnabled !== undefined ? { forceEnabled: existing.forceEnabled } : {}), + installedAt: existing?.installedAt ?? now, updatedAt: now, }; diff --git a/lib/admin/extension-cleanup.ts b/lib/admin/extension-cleanup.ts index 052e4a2f8..00b70529c 100644 --- a/lib/admin/extension-cleanup.ts +++ b/lib/admin/extension-cleanup.ts @@ -1,4 +1,5 @@ import { configManager } from './config-manager'; +import { deletePluginConfig } from './plugin-config'; /** * Drop a deleted plugin or theme from the policy lists that refer to it by @@ -13,3 +14,13 @@ export async function forgetExtensionInPolicy(kind: 'plugin' | 'theme', id: stri if (!list.includes(id)) return; await configManager.setPolicy({ ...policy, [key]: list.filter((x) => x !== id) }); } + +/** + * Everything the server keeps about an admin plugin besides its registry + * entry: the forced flag and its config (which can hold secrets). A plugin + * installed later under the same id starts clean. + */ +export async function cleanUpDeletedPlugin(id: string): Promise { + await forgetExtensionInPolicy('plugin', id); + await deletePluginConfig(id); +} diff --git a/lib/admin/plugin-config.ts b/lib/admin/plugin-config.ts index ca78e1d9d..3af07d254 100644 --- a/lib/admin/plugin-config.ts +++ b/lib/admin/plugin-config.ts @@ -51,6 +51,12 @@ export async function setPluginConfig(pluginId: string, key: string, value: unkn await rename(tmpPath, filePath); } +/** Remove all stored config, secrets included, when a plugin is deleted. */ +export async function deletePluginConfig(pluginId: string): Promise { + assertWritable('delete plugin config'); + try { await unlink(configPath(pluginId)); } catch { /* none stored */ } +} + /** * Delete a single config key for a plugin. */