fix(plugins): persist plugin enable/config and restore configuration on restart

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

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' });
+  });
+});

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<void> {
     const plugin = this.plugins.get(pluginId);
     if (!plugin) {
@@ -411,16 +442,24 @@ export class PluginLoaderService implements OnModuleInit {
   // ============================================================================
 
   registerBuiltInPlugin(manifest: PluginManifest, instance: IPlugin, config: Record<string, unknown> = {}): 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',

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',