diff --git a/server/src/nest/plugins/host/rpc-host.ts b/server/src/nest/plugins/host/rpc-host.ts index c685b4ebd..4d8b5c33b 100644 --- a/server/src/nest/plugins/host/rpc-host.ts +++ b/server/src/nest/plugins/host/rpc-host.ts @@ -1,5 +1,6 @@ import { KNOWN_METHODS, + METHOD_PERMISSION, type KnownMethod, type RpcError, type RpcRequest, @@ -119,7 +120,7 @@ export class PluginRpcHost { req.id, known ? 'PERMISSION_DENIED' : 'UNKNOWN_METHOD', known - ? `${req.method} requires a permission "${this.pluginId}" was not granted` + ? `${req.method} requires the "${(METHOD_PERMISSION as Record)[req.method]}" permission, which was not granted to plugin "${this.pluginId}"` : `unknown method ${req.method}`, ); } diff --git a/server/src/nest/plugins/plugin-runtime.service.ts b/server/src/nest/plugins/plugin-runtime.service.ts index 561860651..b0e8bb138 100644 --- a/server/src/nest/plugins/plugin-runtime.service.ts +++ b/server/src/nest/plugins/plugin-runtime.service.ts @@ -1,4 +1,4 @@ -import { Injectable, type OnModuleInit, type OnModuleDestroy } from '@nestjs/common'; +import { Injectable, type OnApplicationBootstrap, type OnModuleDestroy } from '@nestjs/common'; import semver from 'semver'; import { DatabaseService } from '../database/database.service'; import { pluginsEnabled } from './kill-switch'; @@ -121,7 +121,7 @@ export class PluginDependencyError extends Error { */ @Injectable() -export class PluginRuntimeService implements OnModuleInit, OnModuleDestroy { +export class PluginRuntimeService implements OnApplicationBootstrap, OnModuleDestroy { // The rpc-host factory is bound to `this` as the inter-plugin router, so a // plugin's ctx.plugins.call / ctx.events.emit resolve through callPlugin/ // emitPluginEvent below (which own the dependency-edge authorization). The @@ -182,7 +182,15 @@ export class PluginRuntimeService implements OnModuleInit, OnModuleDestroy { return this.dbs.connection; } - onModuleInit(): void { + // onApplicationBootstrap, NOT onModuleInit: boot activation builds each plugin's + // rpc host synchronously, and the host snapshots PluginRpcRegistryService at + // construction (bindInto). Same-module onModuleInit hooks fire in providers-array + // declaration order, where this service precedes the registry — so booting from + // onModuleInit bound every host to a still-empty registry and every enabled + // plugin's first RPC after a restart came back PERMISSION_DENIED (pinned by + // tests/integration/plugins/boot-registry-order.test.ts). onApplicationBootstrap + // is guaranteed to run after EVERY module's onModuleInit, registry scan included. + onApplicationBootstrap(): void { if (!pluginsEnabled()) return; // If a restore staged plugin trees, swap them into place NOW — before we open any // plugin DB below. This is where a restored backup's plugin data/code actually diff --git a/server/tests/integration/plugins/boot-registry-order.test.ts b/server/tests/integration/plugins/boot-registry-order.test.ts new file mode 100644 index 000000000..d14532cc9 --- /dev/null +++ b/server/tests/integration/plugins/boot-registry-order.test.ts @@ -0,0 +1,142 @@ +/** + * Boot-order regression (#plugins): the RPC registry must be scanned before the + * runtime auto-activates the plugins an admin had enabled. + * + * The bug this pins: PluginRuntimeService booted enabled plugins from + * onModuleInit, and Nest fires same-module onModuleInit hooks in providers-array + * declaration order — where the runtime service is listed BEFORE + * PluginRpcRegistryService. Each boot activation builds its PluginRpcHost + * synchronously (bindInto snapshots the registry at construction), so every host + * was bound from a STILL-EMPTY registry: the plugin's first RPC on activation + * (typically ctx.db.migrate in onLoad) came back PERMISSION_DENIED and the plugin + * landed in error state on every cold boot, until an admin deactivated and + * reactivated it by hand. The fix moves the boot reconcile to + * onApplicationBootstrap, which Nest guarantees runs after EVERY module's + * onModuleInit — so this suite assembles a real Nest module mirroring the + * production declaration order (runtime provider first, registry scan later) and + * asserts the plugin still comes up clean. + */ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { Test } from '@nestjs/testing'; +import type { TestingModule } from '@nestjs/testing'; + +const { testDb } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec(`CREATE TABLE plugins ( + id TEXT PRIMARY KEY, status TEXT, enabled INTEGER DEFAULT 0, version TEXT, api_version INTEGER DEFAULT 1, permissions TEXT DEFAULT '[]', operator_egress INTEGER DEFAULT 0, granted_permissions TEXT DEFAULT '', + config TEXT DEFAULT '{}', dependencies TEXT DEFAULT '{}', capabilities TEXT DEFAULT '{}', last_error TEXT, updated_at TEXT, + trek_range TEXT DEFAULT '>=3.0.0', + source_repo TEXT, author_pubkey TEXT, update_block_code TEXT, update_block_detail TEXT, update_block_version TEXT); + CREATE TABLE plugin_error_log (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT, level TEXT, message TEXT, ts TEXT); + CREATE TABLE plugin_settings_fields (plugin_id TEXT, field_key TEXT, scope TEXT, secret INTEGER); + CREATE TABLE settings (user_id INTEGER, key TEXT, value TEXT); + CREATE TABLE plugin_entity_metadata (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT, entity_type TEXT, entity_id INTEGER, key TEXT, value TEXT, updated_at TEXT); + CREATE TABLE plugin_user_config (plugin_id TEXT, user_id INTEGER, field_key TEXT, value TEXT, PRIMARY KEY (plugin_id, user_id, field_key)); + CREATE TABLE plugin_meta_migrations (plugin_id TEXT, migration_id TEXT, PRIMARY KEY (plugin_id, migration_id)); + CREATE TABLE plugin_capability_audit (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT, acting_user_id INTEGER, method TEXT, resource TEXT, code TEXT, ts TEXT, prev_hash TEXT, hash TEXT); + CREATE TABLE plugin_scheduled_tasks (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT NOT NULL, name TEXT NOT NULL, due_at INTEGER NOT NULL, payload TEXT NOT NULL DEFAULT 'null', every_ms INTEGER, created_at TEXT DEFAULT (datetime('now')), UNIQUE(plugin_id, name)); + CREATE TABLE plugin_user_erasure_queue (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT NOT NULL, user_id INTEGER NOT NULL, created_at TEXT DEFAULT (datetime('now')), UNIQUE(plugin_id, user_id)); + CREATE TABLE addons (id TEXT PRIMARY KEY, enabled INTEGER DEFAULT 0); + CREATE TABLE audit_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + user_id INTEGER, action TEXT NOT NULL, resource TEXT, details TEXT, ip TEXT);`); + return { testDb: db }; +}); +vi.mock('../../../src/db/database', () => ({ db: testDb, canAccessTrip: () => undefined })); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() })); + +import { db as dbConn } from '../../../src/db/database'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AuditService } from '../../../src/nest/audit/audit.service'; +import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service'; +import { PluginUserSettingsService } from '../../../src/nest/plugins/plugin-user-settings.service'; +import { PluginRpcHostFactory } from '../../../src/nest/plugins/host/plugin-rpc-host.factory'; +import { PluginRpcRegistry } from '../../../src/nest/plugins/host/rpc-kit/registry'; +import type { PluginRpcRegistryService } from '../../../src/nest/plugins/host/rpc-kit/registry.service'; +import { DbRpc } from '../../../src/nest/plugins/host/rpc/db.rpc'; + +let codeRoot: string; +let dataRoot: string; +let mod: TestingModule; + +beforeAll(() => { + codeRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'trekplug-boot-code-')); + dataRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'trekplug-boot-data-')); + process.env.TREK_PLUGINS_DIR = codeRoot; + process.env.TREK_PLUGINS_DATA_DIR = dataRoot; + process.env.TREK_PLUGINS_ENABLED = 'true'; + + // The exact shape the bug bit: a plugin that runs its own-db migration as the + // first thing its onLoad does — what every db-backed plugin's activation looks like. + const dir = path.join(codeRoot, 'migrator', 'server'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'index.js'), + `module.exports = { async onLoad(ctx) { await ctx.db.migrate('001', 'CREATE TABLE t (n INTEGER)'); } };`, + ); + // The row a container recreate leaves behind: enabled, already consented to + // db:own. Seeded 'inactive' (not the 'active' a real shutdown leaves) so the + // poll below only terminates on a status the THIS-boot supervisor wrote. + testDb + .prepare("INSERT INTO plugins (id, status, enabled, permissions, granted_permissions, config) VALUES ('migrator','inactive',1,'[\"db:own\"]','[\"db:own\"]','{}')") + .run(); +}); + +afterAll(async () => { + await mod?.close(); + delete process.env.TREK_PLUGINS_DIR; + delete process.env.TREK_PLUGINS_DATA_DIR; + delete process.env.TREK_PLUGINS_ENABLED; + fs.rmSync(codeRoot, { recursive: true, force: true }); + fs.rmSync(dataRoot, { recursive: true, force: true }); +}); + +describe('plugin boot vs registry scan ordering', () => { + it('BOOT-REG-001 a plugin enabled before a restart activates cleanly even though the registry scan runs in a LATER provider\'s onModuleInit', async () => { + const dbs = new DatabaseService(dbConn); + const userSettings = new PluginUserSettingsService(dbs); + // Empty at construction — exactly what PluginRpcRegistryService is before its + // own onModuleInit scan has run. + const registry = new PluginRpcRegistry(); + const hostFactory = new PluginRpcHostFactory(dbs, registry as unknown as PluginRpcRegistryService); + + mod = await Test.createTestingModule({ + providers: [ + // Mirrors plugins-runtime.module.ts: the runtime service is DECLARED BEFORE + // the provider that populates the registry, so Nest fires its onModuleInit + // (if it had one) first. The boot reconcile must therefore not live there. + { + provide: PluginRuntimeService, + useFactory: () => new PluginRuntimeService(dbs, new AuditService(dbs), new AddonsService(dbs), userSettings, undefined, hostFactory), + }, + { + provide: 'REGISTRY_SCAN', + useFactory: () => ({ + onModuleInit() { + registry.register(new DbRpc(userSettings)); + }, + }), + }, + ], + }).compile(); + await mod.init(); + + // Boot activation is fire-and-forget; wait for the supervisor to settle the row. + const runtime = mod.get(PluginRuntimeService); + let row = { status: 'starting', last_error: null as string | null }; + for (let i = 0; i < 100; i++) { + row = testDb.prepare("SELECT status, last_error FROM plugins WHERE id='migrator'").get() as typeof row; + if (row.status === 'active' || row.status === 'error') break; + await new Promise((r) => setTimeout(r, 50)); + } + + expect(row.last_error).toBeNull(); + expect(row.status).toBe('active'); + expect(runtime.isActive('migrator')).toBe(true); + }); +}); diff --git a/server/tests/integration/plugins/plugin-runtime.test.ts b/server/tests/integration/plugins/plugin-runtime.test.ts index 2c7fdf329..0ce24c5ba 100644 --- a/server/tests/integration/plugins/plugin-runtime.test.ts +++ b/server/tests/integration/plugins/plugin-runtime.test.ts @@ -148,9 +148,9 @@ describe('PluginRuntimeService (M2 end-to-end)', () => { // first activate is covered by the 'counter' happy-path test above.) }); - it('onModuleInit is a no-op when the runtime is disabled', () => { + it('onApplicationBootstrap is a no-op when the runtime is disabled', () => { process.env.TREK_PLUGINS_ENABLED = 'false'; - expect(() => createPluginRuntime(new DatabaseService(dbConn)).onModuleInit()).not.toThrow(); + expect(() => createPluginRuntime(new DatabaseService(dbConn)).onApplicationBootstrap()).not.toThrow(); process.env.TREK_PLUGINS_ENABLED = 'true'; }); @@ -246,26 +246,26 @@ describe('PluginRuntimeService (M2 end-to-end)', () => { expect(testDb.prepare("SELECT COUNT(*) c FROM plugin_user_erasure_queue WHERE plugin_id='counter' AND user_id=9").get()).toMatchObject({ c: 0 }); }); - it('onModuleInit boots every ENABLED plugin — even one left in error state', async () => { + it('onApplicationBootstrap boots every ENABLED plugin — even one left in error state', async () => { fs.mkdirSync(path.join(codeRoot, 'booter', 'server'), { recursive: true }); fs.writeFileSync(path.join(codeRoot, 'booter', 'server', 'index.js'), 'module.exports = { async onLoad() {} };'); // status='error' from a previous crash, but enabled=1 → must still boot testDb.prepare("INSERT INTO plugins (id, status, enabled, granted_permissions, config) VALUES ('booter','error',1,'[]','{}')").run(); const rt = createPluginRuntime(new DatabaseService(dbConn)); - rt.onModuleInit(); // fire-and-forget spawn + rt.onApplicationBootstrap(); // fire-and-forget spawn for (let i = 0; i < 40 && !rt.isActive('booter'); i++) await new Promise((r) => setTimeout(r, 50)); expect(rt.isActive('booter')).toBe(true); await rt.deactivate('booter'); }); - it('onModuleInit does NOT boot a disabled plugin', async () => { + it('onApplicationBootstrap does NOT boot a disabled plugin', async () => { fs.mkdirSync(path.join(codeRoot, 'sleeper', 'server'), { recursive: true }); fs.writeFileSync(path.join(codeRoot, 'sleeper', 'server', 'index.js'), 'module.exports = { async onLoad() {} };'); testDb.prepare("INSERT INTO plugins (id, status, enabled, granted_permissions, config) VALUES ('sleeper','inactive',0,'[]','{}')").run(); const rt = createPluginRuntime(new DatabaseService(dbConn)); - rt.onModuleInit(); + rt.onApplicationBootstrap(); await new Promise((r) => setTimeout(r, 300)); expect(rt.isActive('sleeper')).toBe(false); }); @@ -701,7 +701,7 @@ describe('TREK host-version gating', () => { process.env.APP_VERSION = '4.0.0'; seedRanged('boot-broken', '>=3.2.0 <4.0.0', 1); const rt = createPluginRuntime(new DatabaseService(dbConn)); - expect(() => rt.onModuleInit()).not.toThrow(); + expect(() => rt.onApplicationBootstrap()).not.toThrow(); for (let i = 0; i < 40; i++) { const row = testDb.prepare("SELECT enabled FROM plugins WHERE id='boot-broken'").get() as { enabled: number }; if (row.enabled === 0) break; diff --git a/server/tests/unit/plugins/rpc-host.test.ts b/server/tests/unit/plugins/rpc-host.test.ts index ce30579fe..d7b4f6cd1 100644 --- a/server/tests/unit/plugins/rpc-host.test.ts +++ b/server/tests/unit/plugins/rpc-host.test.ts @@ -76,9 +76,14 @@ describe('PluginRpcHost — capability enforcement', () => { expect((res as RpcError).error.message).toBe('unknown method fs.readFile'); }); - it('RPCHOST-003 a denied known method names the plugin, so the log says who asked', async () => { + it('RPCHOST-003 a denied known method names the missing permission AND the plugin', async () => { + // The old template interpolated the plugin id where the permission belonged + // ('tags.list requires a permission "p" was not granted'), so an operator read + // their plugin's NAME as an unknown permission. The message must name both. const res = await makeHost([]).dispatch(req('tags.list'), 42); - expect((res as RpcError).error.message).toBe('tags.list requires a permission "p" was not granted'); + expect((res as RpcError).error.message).toBe( + 'tags.list requires the "db:read:tags" permission, which was not granted to plugin "p"', + ); }); it('RPCHOST-004 with no permissions at all, every permissioned method is denied', async () => {