mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26:47 +00:00
c9627af239
TREK's four channels (in-app, email, webhook, ntfy) were a closed set: notificationService.send() dispatched with four copy-pasted `if` blocks and no provider abstraction, so a fifth channel meant editing eight files by hand. A plugin could produce a notification via ctx.notify.send(), but never deliver one. A plugin now registers a channel with `hooks.notificationChannel` + `hook:notification-channel` on a plain `type: 'integration'` — not a new manifest type, so the TREK-Plugins registry schema and both its CI gates are untouched. Core refactor - New channel registry (services/notifications/): email/webhook/ntfy become ExternalChannel providers wrapping the EXISTING send functions — no delivery logic is rewritten, only relocated. In-app deliberately stays out: it writes typed rows with scope/target/callbacks, not a rendered title+body, the same line shared/ already draws with i18n/externalNotifications. - The event text is now rendered once per recipient instead of once per channel. - The channel set is open: NotifChannel becomes a string, the matrix is registry-derived, and the UI columns are server-driven. The DB column was already bare TEXT and the Zod contract already a string record — only the TypeScript and the two UIs were ever closed. The hook runs USERLESS. Every other hook is user-initiated, so actingUserId falls out of the request; a notification is host-initiated for an ARBITRARY recipient, so ctx.settings.get() would return undefined. The host resolves the recipient's decrypted scope:'user' settings itself and passes them as an argument. That is what lets a channel plugin be handed someone's push token WITHOUT being handed the right to read their trips as them. Enabling the plugin is the opt-in: a plugin channel is not gated on the admin's `notification_channels` list. A built-in always exists in code and needs an explicit switch; a plugin channel only exists because an admin enabled that plugin. (Nothing could write a `plugin:` id into that CSV anyway, and the admin toggle rebuilt it from three booleans, silently dropping anything else — so requiring a second opt-in meant the channel could never be turned on at all.) Also fixed, found while building this: - Plugin settings keys were unvalidated, so a field named `__proto__` or `constructor` resolved off Object.prototype: a REQUIRED field with such a name reported as configured for every user who had configured nothing — enough, for a channel, to be dispatched to everyone with no credentials. Keys are now constrained at install and the config blob is parsed null-prototype, so it is impossible even for an already-installed plugin. - A `select` field's options were cast straight through, so the obvious `["1","5"]` form rendered every dropdown entry BLANK (the client reads value/label). Now coerced, and malformed options are rejected. Also adds: operator-supplied egress hosts (a plugin talking to a self-hosted service can't name the operator's host at publish time, so an admin adds it post-install and the runtime re-spawns the child with the widened allow-list — only for a plugin that DECLARED operatorEgress, and only an admin, never a user); settings-page actions (a "Test connection" button, user-initiated so ctx.settings.get() returns the clicking user's own value); and a Gotify-shaped notification-channel template in the SDK. Verified end to end against a real Gotify container, not just in tests.
136 lines
7.1 KiB
TypeScript
136 lines
7.1 KiB
TypeScript
/**
|
||
* Plugin settings isolation.
|
||
*
|
||
* A plugin's settings live in its OWN storage — instance config in `plugins.config`,
|
||
* per-user config in `plugin_user_config` keyed by (plugin_id, user_id). Plugin code
|
||
* never writes the core `settings` table, and every read helper is host-bound to one
|
||
* plugin id. These tests pin that, plus the settings-key constraint: an unconstrained
|
||
* key (`__proto__`, `constructor`) used to resolve off Object.prototype, so a REQUIRED
|
||
* field with such a name reported as configured for a user who had configured nothing —
|
||
* which for a notification channel meant being dispatched to everyone with no credentials.
|
||
*/
|
||
import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest';
|
||
|
||
const { testDb, dbMock } = vi.hoisted(() => {
|
||
const Database = require('better-sqlite3');
|
||
const db = new Database(':memory:');
|
||
return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: () => null } };
|
||
});
|
||
vi.mock('../../../src/db/database', () => dbMock);
|
||
vi.mock('../../../src/config', () => ({ JWT_SECRET: 'x'.repeat(40), ENCRYPTION_KEY: 'a'.repeat(64), updateJwtSecret: () => {} }));
|
||
|
||
import { createTables } from '../../../src/db/schema';
|
||
import { runMigrations } from '../../../src/db/migrations';
|
||
import { createUser } from '../../helpers/factories';
|
||
import { readUserSettingsDecrypted, hasRequiredUserSettings, readUserSettingDecrypted } from '../../../src/nest/plugins/plugins.service';
|
||
import { parseManifest, ManifestError } from '../../../src/nest/plugins/install/manifest';
|
||
|
||
let uid: number;
|
||
|
||
function declareField(pluginId: string, key: string, opts: { required?: boolean; secret?: boolean } = {}) {
|
||
testDb.prepare(
|
||
`INSERT INTO plugin_settings_fields (plugin_id, field_key, label, input_type, required, secret, scope, sort_order)
|
||
VALUES (?, ?, ?, 'text', ?, ?, 'user', 0)`,
|
||
).run(pluginId, key, key, opts.required ? 1 : 0, opts.secret ? 1 : 0);
|
||
}
|
||
function setUserConfig(pluginId: string, config: Record<string, unknown>) {
|
||
testDb.prepare('INSERT OR REPLACE INTO plugin_user_config (plugin_id, user_id, config) VALUES (?, ?, ?)').run(pluginId, uid, JSON.stringify(config));
|
||
}
|
||
|
||
beforeAll(() => { createTables(testDb); runMigrations(testDb); });
|
||
beforeEach(() => {
|
||
testDb.prepare('DELETE FROM plugin_settings_fields').run();
|
||
testDb.prepare('DELETE FROM plugin_user_config').run();
|
||
testDb.prepare('DELETE FROM settings').run();
|
||
testDb.prepare('DELETE FROM users').run();
|
||
uid = createUser(testDb).user.id;
|
||
});
|
||
|
||
describe('plugin settings are isolated from core and from each other', () => {
|
||
it('PSET-001 — a plugin declaring "webhook_url" cannot touch the CORE settings row', () => {
|
||
testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'webhook_url', 'https://core.example.com/real')").run(uid);
|
||
declareField('evil', 'webhook_url');
|
||
setUserConfig('evil', { webhook_url: 'https://attacker.example.com' });
|
||
|
||
// The user's REAL notification webhook is untouched — the plugin's value lives in
|
||
// its own blob, in its own table. The namespacing is structural, not by key naming.
|
||
const core = testDb.prepare("SELECT value FROM settings WHERE user_id = ? AND key = 'webhook_url'").get(uid) as { value: string };
|
||
expect(core.value).toBe('https://core.example.com/real');
|
||
expect(readUserSettingsDecrypted('evil', uid)).toEqual({ webhook_url: 'https://attacker.example.com' });
|
||
});
|
||
|
||
it('PSET-002 — plugin A cannot read plugin B’s config, even with the same key name', () => {
|
||
declareField('a', 'token', { secret: true });
|
||
declareField('b', 'token', { secret: true });
|
||
setUserConfig('b', { token: 'B-SECRET' });
|
||
|
||
expect(readUserSettingsDecrypted('a', uid)).toEqual({});
|
||
expect(readUserSettingDecrypted('a', uid, 'token')).toBeUndefined();
|
||
expect(readUserSettingDecrypted('b', uid, 'token')).toBe('B-SECRET');
|
||
});
|
||
|
||
it('PSET-003 — a plugin only ever sees its own DECLARED keys', () => {
|
||
declareField('p', 'declared');
|
||
// An undeclared key that somehow reached the blob is not handed to the plugin.
|
||
setUserConfig('p', { declared: 'yes', sneaked: 'no' });
|
||
expect(readUserSettingsDecrypted('p', uid)).toEqual({ declared: 'yes' });
|
||
});
|
||
});
|
||
|
||
describe('settings keys cannot resolve off the prototype chain', () => {
|
||
it.each(['__proto__', 'constructor', 'prototype'])(
|
||
'PSET-004 — a REQUIRED field named "%s" is NOT reported as configured',
|
||
(key) => {
|
||
declareField('evil', key, { required: true });
|
||
// The user has configured nothing at all.
|
||
expect(hasRequiredUserSettings('evil', uid)).toBe(false);
|
||
expect(readUserSettingsDecrypted('evil', uid)).toEqual({});
|
||
},
|
||
);
|
||
|
||
it('PSET-005 — a genuinely configured required field still reports configured', () => {
|
||
declareField('good', 'appToken', { required: true, secret: true });
|
||
expect(hasRequiredUserSettings('good', uid)).toBe(false);
|
||
setUserConfig('good', { appToken: 'T' });
|
||
expect(hasRequiredUserSettings('good', uid)).toBe(true);
|
||
});
|
||
|
||
it('PSET-006 — the manifest rejects such a key at install', () => {
|
||
const base = { id: 'evil', name: 'Evil', version: '1.0.0', apiVersion: 1, type: 'integration', nativeModules: false, permissions: [] };
|
||
for (const key of ['__proto__', 'constructor', 'prototype', 'has space', '1leading', 'a'.repeat(65)]) {
|
||
expect(() => parseManifest({ ...base, settings: [{ key, scope: 'user' }] })).toThrow(ManifestError);
|
||
}
|
||
// …and still accepts an ordinary one.
|
||
expect(parseManifest({ ...base, settings: [{ key: 'appToken', scope: 'user', secret: true, required: true }] }).settings[0].key).toBe('appToken');
|
||
});
|
||
});
|
||
|
||
/**
|
||
* The channel label is a plugin-supplied string that becomes a column header in the
|
||
* user's notification preferences matrix, so the host bounds it like every other
|
||
* plugin string it renders (cf. cap()/stripEmoji in the calendar + photo controllers).
|
||
*/
|
||
describe('a plugin channel label is bounded by the host', () => {
|
||
it('PSET-007 — an oversized, emoji-laden capabilities.title is capped and stripped', async () => {
|
||
const { PluginRuntimeService } = await import('../../../src/nest/plugins/plugin-runtime.service');
|
||
process.env.TREK_PLUGINS_ENABLED = 'true';
|
||
|
||
testDb.prepare(
|
||
`INSERT OR REPLACE INTO plugins (id, name, status, enabled, version, permissions, granted_permissions, capabilities, config)
|
||
VALUES ('loud', 'Loud', 'active', 1, '1.0.0', '[]', '[]', ?, '{}')`,
|
||
).run(JSON.stringify({ notificationChannel: { title: '🎉'.repeat(5) + 'A'.repeat(500) } }));
|
||
|
||
const rt = new PluginRuntimeService();
|
||
// Stand the plugin up as a granted, active notificationChannel provider.
|
||
(rt as unknown as { supervisor: { running: Map<string, unknown> } }).supervisor.running.set('loud', {
|
||
id: 'loud', status: 'active', hooks: ['notificationChannel'], granted: new Set(['hook:notification-channel']),
|
||
});
|
||
|
||
const [channel] = rt.notificationChannels();
|
||
expect(channel.id).toBe('plugin:loud');
|
||
expect(channel.label!.length).toBeLessThanOrEqual(40);
|
||
expect(channel.label).not.toMatch(/\p{Extended_Pictographic}/u);
|
||
delete process.env.TREK_PLUGINS_ENABLED;
|
||
});
|
||
});
|