From 0a4818e182d0f080607c4a69d34fa018c4b1371d Mon Sep 17 00:00:00 2001 From: rmyndharis Date: Fri, 19 Jun 2026 16:10:02 +0700 Subject: [PATCH] fix(plugins): persist plugin enable/config and restore configuration on restart Enabling, disabling, or configuring a plugin only mutated in-memory state: the storage setters update an existing registry entry, but no entry was ever created, so every write silently no-op'd and the change was lost on restart while the API still reported success. A registry entry is now created when a plugin loads, so enable/disable/config writes persist, and a plugin's saved configuration is restored on the next start. Boot never auto-enables a plugin (the entry's status is reset to INSTALLED to match the freshly-loaded runtime), so a previously enabled plugin is re-enabled by an explicit admin action rather than re-executing plugin code at startup; its config is preserved. Built-in engine config stays env-driven each boot (the live default merges over persisted operator overrides), so a changed .env value is never frozen to a first-boot snapshot. registry.json is written owner-only since plugin config can hold secrets. --- CHANGELOG.md | 10 ++ .../plugins/plugin-loader.service.spec.ts | 96 ++++++++++++++++++- src/core/plugins/plugin-loader.service.ts | 47 ++++++++- src/core/plugins/plugin-storage.service.ts | 11 ++- 4 files changed, 156 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66bcc8f6..94e92882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Plugin enable/disable and configuration now persist.** Enabling, disabling, or configuring a plugin + updated only in-memory state — the registry on disk was never written, so the change was silently lost + on restart (an enabled plugin came back disabled and its saved configuration, including secrets such as + an API key, was gone), while the API still reported success. A registry entry is now created when a + plugin loads, so these writes are durable, and a plugin's saved configuration is restored on the next + start. For safety, plugins are not auto-enabled on boot — re-enable them after a restart; their + configuration is preserved. + ## [0.4.2] - 2026-06-19 Bug-fix and hardening release: access-control tightening, session-lifecycle resilience, data-migration diff --git a/src/core/plugins/plugin-loader.service.spec.ts b/src/core/plugins/plugin-loader.service.spec.ts index 18f0fbd4..a625aebd 100644 --- a/src/core/plugins/plugin-loader.service.spec.ts +++ b/src/core/plugins/plugin-loader.service.spec.ts @@ -25,17 +25,23 @@ describe('resolvePluginMainPath', () => { }); }); +import * as fs from 'fs'; +import * as os from 'os'; import { PluginLoaderService } from './plugin-loader.service'; import { ConfigService } from '@nestjs/config'; import { ModuleRef } from '@nestjs/core'; import { HookManager } from '../hooks'; import { PluginStorageService } from './plugin-storage.service'; -import { IPlugin, PluginManifest, PluginType } from './plugin.interfaces'; +import { IPlugin, PluginManifest, PluginStatus, PluginType } from './plugin.interfaces'; describe('PluginLoaderService.registerBuiltInPlugin config', () => { function makeLoader(): PluginLoaderService { const configService = { get: jest.fn().mockReturnValue(undefined) } as unknown as ConfigService; - const pluginStorage = {} as unknown as PluginStorageService; + const pluginStorage = { + getPluginEntry: jest.fn().mockReturnValue(undefined), + setPluginEntry: jest.fn(), + getPluginConfig: jest.fn().mockReturnValue(null), + } as unknown as PluginStorageService; return new PluginLoaderService(configService, new HookManager(), pluginStorage, {} as unknown as ModuleRef); } const manifest: PluginManifest = { @@ -59,3 +65,89 @@ describe('PluginLoaderService.registerBuiltInPlugin config', () => { expect(loader.getPlugin('cfg-test')?.config).toEqual({}); }); }); + +describe('PluginLoaderService — enable/config persistence', () => { + let tmpDir: string; + let config: ConfigService; + let storage: PluginStorageService; + let loader: PluginLoaderService; + + const manifest: PluginManifest = { + id: 'persist-test', + name: 'Persist Test', + version: '1.0.0', + type: PluginType.EXTENSION, + main: 'index.js', + }; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'owa-plugin-')); + config = { get: (k: string) => (k === 'dataDir' ? tmpDir : undefined) } as unknown as ConfigService; + storage = new PluginStorageService(config); + loader = new PluginLoaderService(config, new HookManager(), storage, {} as unknown as ModuleRef); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('creates a complete INSTALLED registry entry on register so a status write persists across a restart', () => { + loader.registerBuiltInPlugin(manifest, {}, { apiKey: 'default' }); + const entry = storage.getPluginEntry('persist-test'); + expect(entry).toMatchObject({ + id: 'persist-test', + status: PluginStatus.INSTALLED, + builtIn: true, + }); + + // The status write now lands (previously a silent no-op because no entry existed). + storage.setPluginStatus('persist-test', PluginStatus.ENABLED); + + // Durable: a fresh storage instance re-reads registry.json (simulates a restart). + expect(new PluginStorageService(config).getPluginStatus('persist-test')).toBe(PluginStatus.ENABLED); + }); + + it('keeps using live env config for a built-in across restarts (the first snapshot must not freeze it)', () => { + // Boot 1: register with one env-derived default, no operator edit. + loader.registerBuiltInPlugin(manifest, {}, { execPath: '/old/chromium', headless: true }); + + // Boot 2: env changed (e.g. operator set PUPPETEER_EXECUTABLE_PATH on a new image) → the live value wins. + const storage2 = new PluginStorageService(config); + const loader2 = new PluginLoaderService(config, new HookManager(), storage2, {} as unknown as ModuleRef); + loader2.registerBuiltInPlugin(manifest, {}, { execPath: '/new/chromium', headless: true }); + + expect(loader2.getPlugin('persist-test')?.config).toEqual({ execPath: '/new/chromium', headless: true }); + }); + + it('reports a re-registered plugin as installed after restart even if it was enabled (no boot auto-enable, no divergence)', () => { + loader.registerBuiltInPlugin(manifest, {}, {}); + storage.setPluginStatus('persist-test', PluginStatus.ENABLED); // operator enabled it + + // Restart: re-register the built-in. + const storage2 = new PluginStorageService(config); + const loader2 = new PluginLoaderService(config, new HookManager(), storage2, {} as unknown as ModuleRef); + loader2.registerBuiltInPlugin(manifest, {}, {}); + + // Runtime is INSTALLED (not auto-enabled) AND the registry agrees (no enabled/installed divergence). + expect(loader2.getPlugin('persist-test')?.status).toBe(PluginStatus.INSTALLED); + expect(storage2.getPluginStatus('persist-test')).toBe(PluginStatus.INSTALLED); + }); + + it('writes registry.json without group/other access (plugin config can hold secrets)', () => { + loader.registerBuiltInPlugin(manifest, {}, { apiKey: 'secret' }); + const mode = fs.statSync(path.join(tmpDir, 'plugins', 'registry.json')).mode & 0o777; + expect(mode & 0o077).toBe(0); + }); + + it('restores the operator config on the next load instead of resetting to the default', () => { + loader.registerBuiltInPlugin(manifest, {}, { apiKey: 'default' }); + loader.updatePluginConfig('persist-test', { apiKey: 'operator-secret' }); + expect(storage.getPluginConfig('persist-test')).toEqual({ apiKey: 'operator-secret' }); + + // Restart: re-register the built-in with its default config — the persisted operator config wins. + const storage2 = new PluginStorageService(config); + const loader2 = new PluginLoaderService(config, new HookManager(), storage2, {} as unknown as ModuleRef); + loader2.registerBuiltInPlugin(manifest, {}, { apiKey: 'default' }); + expect(loader2.getPlugin('persist-test')?.config).toEqual({ apiKey: 'operator-secret' }); + }); +}); diff --git a/src/core/plugins/plugin-loader.service.ts b/src/core/plugins/plugin-loader.service.ts index 35afc3f9..81253fee 100644 --- a/src/core/plugins/plugin-loader.service.ts +++ b/src/core/plugins/plugin-loader.service.ts @@ -121,19 +121,22 @@ export class PluginLoaderService implements OnModuleInit { throw new Error(`Plugin ${manifest.id} is already loaded`); } - // Load stored config - const storedConfig = this.pluginStorage.getPluginConfig(manifest.id); + // Load any persisted config so an operator's settings survive a restart. + const storedConfig = this.pluginStorage.getPluginConfig(manifest.id) ?? {}; const pluginInstance: PluginInstance = { manifest, status: PluginStatus.INSTALLED, - config: storedConfig ?? {}, + config: storedConfig, instance: null, loadedAt: new Date(), }; this.plugins.set(manifest.id, pluginInstance); + // Ensure a registry entry exists so later enable/disable/config writes persist. + this.ensureRegistryEntry(manifest, false); + this.logger.log(`Plugin loaded: ${manifest.name} v${manifest.version}`, { pluginId: manifest.id, type: manifest.type, @@ -143,6 +146,34 @@ export class PluginLoaderService implements OnModuleInit { return pluginInstance; } + /** + * Ensure a freshly-loaded plugin has a persisted registry entry, so later enable/disable/config + * writes (which only update an EXISTING entry) actually persist instead of silently no-op'ing. + * Creates a complete INSTALLED entry when none exists; an existing entry's persisted status/config + * is left untouched. Best-effort (saveRegistry swallows fs errors, so a disk failure never turns a + * load into a 500). Does NOT enable or run the plugin — boot never auto-executes plugin code. + */ + private ensureRegistryEntry(manifest: PluginManifest, builtIn: boolean): void { + // Reconcile the persisted entry with the freshly-loaded runtime: the runtime always loads + // INSTALLED and is never auto-enabled on boot (enabling must stay an explicit ADMIN action that + // runs the lifecycle), so the entry's status is (re)set to INSTALLED to match — a previously + // enabled plugin must be re-enabled after a restart. The operator's persisted config is preserved + // so secrets/settings survive. Best-effort: saveRegistry swallows fs errors, so a disk failure + // never turns a load into a 500. + const existing = this.pluginStorage.getPluginEntry(manifest.id); + this.pluginStorage.setPluginEntry({ + id: manifest.id, + type: manifest.type, + name: manifest.name, + version: manifest.version, + status: PluginStatus.INSTALLED, + config: existing?.config ?? {}, + builtIn, + installedAt: existing?.installedAt ?? new Date(), + updatedAt: new Date(), + }); + } + async enablePlugin(pluginId: string): Promise { const plugin = this.plugins.get(pluginId); if (!plugin) { @@ -411,16 +442,24 @@ export class PluginLoaderService implements OnModuleInit { // ============================================================================ registerBuiltInPlugin(manifest: PluginManifest, instance: IPlugin, config: Record = {}): void { + // Merge: env-derived defaults stay live each boot (so a changed .env wins), while an operator's + // persisted overrides win for the keys they actually set. Engine config is wholly env-derived + // (no persisted overrides), so it is never frozen to a first-boot snapshot. + const effectiveConfig = { ...config, ...(this.pluginStorage.getPluginConfig(manifest.id) ?? {}) }; + const pluginInstance: PluginInstance = { manifest, status: PluginStatus.INSTALLED, - config, + config: effectiveConfig, instance, loadedAt: new Date(), }; this.plugins.set(manifest.id, pluginInstance); + // Ensure a registry entry exists so later enable/disable/config writes persist. + this.ensureRegistryEntry(manifest, true); + this.logger.debug(`Built-in plugin registered: ${manifest.name}`, { pluginId: manifest.id, action: 'builtin_plugin_registered', diff --git a/src/core/plugins/plugin-storage.service.ts b/src/core/plugins/plugin-storage.service.ts index 1e9b5f30..c1fe47a5 100644 --- a/src/core/plugins/plugin-storage.service.ts +++ b/src/core/plugins/plugin-storage.service.ts @@ -39,11 +39,18 @@ export class PluginStorageService { try { const dir = path.dirname(this.registryPath); if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); } const entries = Array.from(this.registry.values()); - fs.writeFileSync(this.registryPath, JSON.stringify(entries, null, 2)); + // Owner-only: plugin config can hold secrets (e.g. an API key). writeFileSync's mode only + // applies on CREATE, so chmod an already-existing, looser file too (best-effort). + fs.writeFileSync(this.registryPath, JSON.stringify(entries, null, 2), { mode: 0o600 }); + try { + fs.chmodSync(this.registryPath, 0o600); + } catch { + /* best-effort hardening */ + } } catch (error) { this.logger.error('Failed to save plugin registry', String(error), { action: 'registry_save_failed',