From 332e5c3c4233d11991e4665caaae2813266ab5bf Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 4 Jul 2026 00:31:01 +0200 Subject: [PATCH] =?UTF-8?q?feat(plugins):=20sandbox=20hardening=20?= =?UTF-8?q?=E2=80=94=20OS=20permission=20model,=20egress=20choke=20point,?= =?UTF-8?q?=20bound=20acting=20user,=20author=20signatures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the four gaps the security review surfaced: - OS permission model on the prod plugin child (Node --permission with fs-read scoped to the compiled server dir + the plugin's own code dir, no fs-write/child_process/worker/native). A plugin can no longer read trek.db or the .jwt_secret/.encryption_key files, nor shell out — the direct-fs and RCE escapes that bypassed the RPC layer. Opt-out via TREK_PLUGIN_PERMISSIONS=off. - Egress guard extended from fetch to the net.Socket connect choke point, so node:http/https/net/tls obey the declared-egress allowlist too (no declared egress = no outbound). Under the permission model there is no clean escape to an unwrapped runtime. Kernel/network-namespace containment remains the container step. - Trip reads are membership-checked against the acting user the HOST binds from the authenticated invocation, not an asUserId the plugin supplies; a job/onLoad (no user) can't read user-scoped trips. - Optional minisign (Ed25519) author signatures verified offline, TOFU-pinned (migration 158). Unsigned plugins install on sha256 alone; a signed plugin can't silently drop its signature or swap its author key. Server suite green (permission-model activation verified against the Koffi reference plugin on dev1). --- server/src/db/migrations.ts | 7 ++ server/src/nest/plugins/host/rpc-host.ts | 34 +++++--- .../nest/plugins/install/verify-signature.ts | 87 +++++++++++++++++++ server/src/nest/plugins/paths.ts | 43 ++++++++- .../nest/plugins/plugin-runtime.service.ts | 4 +- .../nest/plugins/plugins-proxy.controller.ts | 25 ++++-- .../nest/plugins/registry/registry.service.ts | 43 +++++++++ .../nest/plugins/runtime/plugin-host-entry.ts | 72 +++++++++++---- server/src/nest/plugins/runtime/plugin-sdk.ts | 26 ++++-- .../plugins/supervisor/plugin-supervisor.ts | 44 ++++++++-- .../integration/plugins/registry.test.ts | 67 +++++++++++++- server/tests/unit/plugins/plugin-sdk.test.ts | 16 ++-- .../tests/unit/plugins/plugins-proxy.test.ts | 8 +- .../tests/unit/plugins/protocol-paths.test.ts | 22 ++++- server/tests/unit/plugins/rpc-host.test.ts | 23 +++-- .../unit/plugins/verify-signature.test.ts | 81 +++++++++++++++++ wiki/Plugin-Publishing.md | 43 +++++++++ 17 files changed, 565 insertions(+), 80 deletions(-) create mode 100644 server/src/nest/plugins/install/verify-signature.ts create mode 100644 server/tests/unit/plugins/verify-signature.test.ts diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index f09621786..91d93aa02 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -3370,6 +3370,13 @@ function runMigrations(db: Database.Database): void { () => { try { db.exec("ALTER TABLE plugins ADD COLUMN capabilities TEXT NOT NULL DEFAULT '{}';"); } catch (err) { console.warn('[migrations] Non-fatal migration step failed:', err); } }, + // Migration 158: TOFU pin for a plugin's author signing key (#plugins, #4). + // Set on first install of a signed plugin; a later install whose registry key + // differs is a hard stop (author change / key rotation / attack) unless an + // admin re-trusts. NULL for unsigned plugins (signing is opt-in). + () => { + try { db.exec("ALTER TABLE plugins ADD COLUMN author_pubkey TEXT;"); } catch (err) { console.warn('[migrations] Non-fatal migration step failed:', err); } + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/nest/plugins/host/rpc-host.ts b/server/src/nest/plugins/host/rpc-host.ts index ca0a78efc..9059f4ede 100644 --- a/server/src/nest/plugins/host/rpc-host.ts +++ b/server/src/nest/plugins/host/rpc-host.ts @@ -41,7 +41,7 @@ export interface HostDeps { broadcastToUser(userId: number, payload: Record): void; } -type Handler = (params: Record) => unknown; +type Handler = (params: Record, actingUserId: number | undefined) => unknown; const num = (v: unknown, name: string): number => { const n = typeof v === 'string' ? Number(v) : v; @@ -71,14 +71,14 @@ export class PluginRpcHost { } if (has('db:read:trips')) { - this.methods.set('trips.getById', (p) => - this.tripRead(p, () => deps.db.prepare('SELECT * FROM trips WHERE id = ?').get(num(p.tripId, 'tripId'))), + this.methods.set('trips.getById', (p, uid) => + this.tripRead(p, uid, () => deps.db.prepare('SELECT * FROM trips WHERE id = ?').get(num(p.tripId, 'tripId'))), ); - this.methods.set('trips.getPlaces', (p) => - this.tripRead(p, () => deps.db.prepare('SELECT * FROM places WHERE trip_id = ? ORDER BY day_id, position').all(num(p.tripId, 'tripId'))), + this.methods.set('trips.getPlaces', (p, uid) => + this.tripRead(p, uid, () => deps.db.prepare('SELECT * FROM places WHERE trip_id = ? ORDER BY day_id, position').all(num(p.tripId, 'tripId'))), ); - this.methods.set('trips.getReservations', (p) => - this.tripRead(p, () => deps.db.prepare('SELECT * FROM reservations WHERE trip_id = ? ORDER BY reservation_time').all(num(p.tripId, 'tripId'))), + this.methods.set('trips.getReservations', (p, uid) => + this.tripRead(p, uid, () => deps.db.prepare('SELECT * FROM reservations WHERE trip_id = ? ORDER BY reservation_time').all(num(p.tripId, 'tripId'))), ); } @@ -104,17 +104,25 @@ export class PluginRpcHost { } } - /** Membership-check every trip read against the acting user; forbid otherwise. */ - private tripRead(p: Record, read: () => unknown): unknown { + /** + * Membership-check every trip read against the acting user. The acting user is + * bound by the HOST from the authenticated invocation (see the supervisor's + * invocation map) — NOT taken from a plugin-supplied `asUserId`, which a plugin + * could set to any id to read another user's trips. If no acting user is bound + * (a job / onLoad, or a forged call), the read is forbidden. + */ + private tripRead(p: Record, actingUserId: number | undefined, read: () => unknown): unknown { const tripId = num(p.tripId, 'tripId'); - const asUserId = num(p.asUserId, 'asUserId'); - if (!this.deps.canAccessTrip(tripId, asUserId)) { + if (actingUserId === undefined) { + throw new ForbiddenResource('trip reads require an authenticated user context'); + } + if (!this.deps.canAccessTrip(tripId, actingUserId)) { throw new ForbiddenResource(`no access to trip ${tripId}`); } return read(); } - async dispatch(req: RpcRequest): Promise { + async dispatch(req: RpcRequest, actingUserId?: number): Promise { const handler = this.methods.get(req.method); if (!handler) { const known = (KNOWN_METHODS as readonly string[]).includes(req.method as KnownMethod); @@ -128,7 +136,7 @@ export class PluginRpcHost { } const params = (req.params ?? {}) as Record; try { - const result = await handler(params); + const result = await handler(params, actingUserId); return { k: 'res', id: req.id, ok: true, result }; } catch (e) { if (e instanceof BadParams) return this.err(req.id, 'BAD_PARAMS', e.message); diff --git a/server/src/nest/plugins/install/verify-signature.ts b/server/src/nest/plugins/install/verify-signature.ts new file mode 100644 index 000000000..d858fd7c5 --- /dev/null +++ b/server/src/nest/plugins/install/verify-signature.ts @@ -0,0 +1,87 @@ +import crypto from 'node:crypto'; + +/** + * Author-signature verification for the plugin installer (#plugins, #4). + * + * sha256 pinning proves the bytes match what the REGISTRY vouches for; a whoever + * controls the registry repo controls that. A minisign (Ed25519) author + * signature proves the bytes were signed by the holder of the plugin AUTHOR's + * private key — so a compromised registry cannot ship attacker code under an + * author's name without also stealing that key. TREK verifies offline with + * node:crypto (no Fulcio/Rekor phone-home), consistent with the "no telemetry + * beyond the update check" stance. + * + * Signing is OPT-IN: an entry without an author key + signature installs exactly + * as before (sha256 only). When both are present, verification is mandatory and + * a mismatch aborts the install. + */ + +// SPKI DER prefix for a raw 32-byte Ed25519 public key (RFC 8410). +const ED25519_SPKI_PREFIX = Buffer.from('302a300506032b6570032100', 'hex'); + +export class SignatureError extends Error {} + +/** Wrap a raw 32-byte Ed25519 public key as a node KeyObject. */ +function ed25519KeyFromRaw(raw: Buffer): crypto.KeyObject { + if (raw.length !== 32) throw new SignatureError('public key is not a 32-byte Ed25519 key'); + return crypto.createPublicKey({ key: Buffer.concat([ED25519_SPKI_PREFIX, raw]), format: 'der', type: 'spki' }); +} + +/** + * Parse a minisign public key. Accepts either the two-line minisign format + * (comment line + base64 payload) or a bare base64 payload. The 10-byte payload + * header is `Ed` + an 8-byte key id; the trailing 32 bytes are the Ed25519 key. + * A bare 32-byte base64 key (44 chars) is also accepted for simple deployments. + */ +function parseMinisignPubKey(pub: string): { key: crypto.KeyObject; keyId: Buffer | null } { + const line = pub.trim().split(/\r?\n/).map((l) => l.trim()).filter((l) => l && !l.startsWith('untrusted comment')).pop(); + if (!line) throw new SignatureError('empty public key'); + const buf = Buffer.from(line, 'base64'); + if (buf.length === 32) return { key: ed25519KeyFromRaw(buf), keyId: null }; + if (buf.length === 42 && buf.subarray(0, 2).toString('latin1') === 'Ed') { + return { key: ed25519KeyFromRaw(buf.subarray(10)), keyId: buf.subarray(2, 10) }; + } + throw new SignatureError('unrecognized public key format'); +} + +/** + * Parse a minisign signature (.minisig). The relevant line is the first base64 + * payload: 2-byte algorithm (`Ed` legacy | `ED` prehashed) + 8-byte key id + + * 64-byte Ed25519 signature. A bare 64-byte base64 signature (legacy over the + * raw bytes) is also accepted. + */ +function parseMinisignSignature(sig: string): { algo: 'Ed' | 'ED' | 'raw'; keyId: Buffer | null; signature: Buffer } { + const line = sig.trim().split(/\r?\n/).map((l) => l.trim()).filter((l) => l && !l.startsWith('untrusted comment')).shift(); + if (!line) throw new SignatureError('empty signature'); + const buf = Buffer.from(line, 'base64'); + if (buf.length === 64) return { algo: 'raw', keyId: null, signature: buf }; + if (buf.length === 74) { + const algo = buf.subarray(0, 2).toString('latin1'); + if (algo !== 'Ed' && algo !== 'ED') throw new SignatureError(`unsupported signature algorithm ${algo}`); + return { algo, keyId: buf.subarray(2, 10), signature: buf.subarray(10) }; + } + throw new SignatureError('unrecognized signature format'); +} + +/** + * Verify that `bytes` were signed by the holder of `publicKeyB64`'s private key. + * Supports minisign legacy (`Ed`, signs the raw file) and prehashed (`ED`, signs + * BLAKE2b-512 of the file), plus bare raw Ed25519 over the file bytes. Returns + * true on a valid signature, throws SignatureError on a malformed input, and + * returns false on a well-formed but non-matching signature. + */ +export function verifyAuthorSignature(bytes: Buffer, signatureB64: string, publicKeyB64: string): boolean { + const { key, keyId } = parseMinisignPubKey(publicKeyB64); + const { algo, keyId: sigKeyId, signature } = parseMinisignSignature(signatureB64); + + // If both carry a key id, they must match the same key (catches a wrong-key mixup). + if (keyId && sigKeyId && !keyId.equals(sigKeyId)) return false; + + const message = + algo === 'ED' ? crypto.createHash('blake2b512').update(bytes).digest() : bytes; + try { + return crypto.verify(null, message, key, signature); + } catch { + return false; + } +} diff --git a/server/src/nest/plugins/paths.ts b/server/src/nest/plugins/paths.ts index 7e0de6dfd..2263a5a4d 100644 --- a/server/src/nest/plugins/paths.ts +++ b/server/src/nest/plugins/paths.ts @@ -45,7 +45,7 @@ export function pluginDbFile(id: string): string { * so no `.js` sibling exists — fall back to the `.ts` source loaded via tsx (a * prod dependency). This keeps the fork path identical in tests and production. */ -export function resolveChildEntry(): { entry: string; execArgv: string[]; forkCwd?: string } { +export function resolveChildEntry(): { entry: string; execArgv: string[]; forkCwd?: string; jsMode: boolean } { const js = path.join(__dirname, 'runtime', 'plugin-host-entry.js'); const ts = path.join(__dirname, 'runtime', 'plugin-host-entry.ts'); if (!fs.existsSync(js) && fs.existsSync(ts)) { @@ -53,8 +53,43 @@ export function resolveChildEntry(): { entry: string; execArgv: string[]; forkCw // must run from a dir where `tsx` is on the node_modules chain (the server // root) — NOT the plugin dir. The plugin itself is still loaded by absolute // path via createRequire, so this doesn't weaken the prod isolation, where - // the .js branch below keeps cwd at the plugin dir. - return { entry: ts, execArgv: ['--import', 'tsx'], forkCwd: process.cwd() }; + // the .js branch below keeps cwd at the plugin dir. tsx compiles TS on the + // fly and walks node_modules, so the OS-level permission model can't be + // applied here — it is a prod-only hardening (jsMode). + return { entry: ts, execArgv: ['--import', 'tsx'], forkCwd: process.cwd(), jsMode: false }; } - return { entry: js, execArgv: ['--max-old-space-size=192'] }; + return { entry: js, execArgv: ['--max-old-space-size=192'], jsMode: true }; +} + +/** + * The compiled server code root (…/dist), computed from this module's location. + * The plugin child needs read access to it to load its bootstrap + SDK; it must + * NOT be granted the data root (trek.db, .jwt_secret, .encryption_key live there). + */ +export function serverCodeRoot(): string { + // __dirname = …/dist/nest/plugins → …/dist + return path.resolve(__dirname, '../..'); +} + +/** + * Node OS-level permission-model flags for a plugin child (#plugins, #2 security + * hardening). `--permission` denies filesystem write, child_process, worker + * threads and native addons outright; `--allow-fs-read` is scoped to exactly the + * code the child must load (the compiled server dir + this plugin's own code + * dir). Result: the child can no longer read trek.db / the secret files or shell + * out, closing the direct-filesystem and RCE escapes that bypass the RPC layer. + * Empty (opt-out) when TREK_PLUGIN_PERMISSIONS=off. + */ +export function pluginPermissionArgs(pluginId: string): string[] { + if ((process.env.TREK_PLUGIN_PERMISSIONS ?? 'on').toLowerCase() === 'off') return []; + const codeRoot = serverCodeRoot(); + return [ + '--permission', + `--allow-fs-read=${codeRoot}`, + // The server package.json sits one level above dist and Node reads it to + // resolve the child's module type. Grant just that file — never its dir, + // which also holds the `data` symlink to trek.db and the secret files. + `--allow-fs-read=${path.join(codeRoot, '..', 'package.json')}`, + `--allow-fs-read=${pluginCodeDir(pluginId)}`, + ]; } diff --git a/server/src/nest/plugins/plugin-runtime.service.ts b/server/src/nest/plugins/plugin-runtime.service.ts index 8ae176f47..164719139 100644 --- a/server/src/nest/plugins/plugin-runtime.service.ts +++ b/server/src/nest/plugins/plugin-runtime.service.ts @@ -128,8 +128,8 @@ export class PluginRuntimeService implements OnModuleInit, OnModuleDestroy { routesOf(id: string): PluginRouteInfo[] { return this.supervisor.routesOf(id); } - invoke(id: string, method: string, params: Record): Promise { - return this.supervisor.invoke(id, method, params); + invoke(id: string, method: string, params: Record, actingUserId?: number): Promise { + return this.supervisor.invoke(id, method, params, { actingUserId }); } } diff --git a/server/src/nest/plugins/plugins-proxy.controller.ts b/server/src/nest/plugins/plugins-proxy.controller.ts index 010a93843..fb1aa83d5 100644 --- a/server/src/nest/plugins/plugins-proxy.controller.ts +++ b/server/src/nest/plugins/plugins-proxy.controller.ts @@ -48,16 +48,23 @@ export class PluginsProxyController { } try { - const reply = (await this.runtime.invoke(pluginId, 'invoke.route', { - routeId: route.i, - req: { - method: req.method, - path: sub, - query: req.query, - body: req.body ?? null, - user: user ? { id: user.id, username: user.username, isAdmin: !!user.is_admin } : null, + const reply = (await this.runtime.invoke( + pluginId, + 'invoke.route', + { + routeId: route.i, + req: { + method: req.method, + path: sub, + query: req.query, + body: req.body ?? null, + user: user ? { id: user.id, username: user.username, isAdmin: !!user.is_admin } : null, + }, }, - })) as { status?: number; headers?: Record; body?: unknown }; + // Bind the authenticated session user as the acting user for any trip + // reads this invocation makes — the plugin cannot override it. + user?.id, + )) as { status?: number; headers?: Record; body?: unknown }; res.status(reply?.status ?? 200); for (const [k, v] of Object.entries(reply?.headers ?? {})) { diff --git a/server/src/nest/plugins/registry/registry.service.ts b/server/src/nest/plugins/registry/registry.service.ts index 4d2cfe481..54feaeb14 100644 --- a/server/src/nest/plugins/registry/registry.service.ts +++ b/server/src/nest/plugins/registry/registry.service.ts @@ -4,6 +4,7 @@ import path from 'node:path'; import { db } from '../../../db/database'; import { pluginCodeDir, pluginsCodeRoot, pluginsDataRoot } from '../paths'; import { safeDownload, sha256Matches } from '../install/safe-fetch'; +import { verifyAuthorSignature } from '../install/verify-signature'; import { extractArchive } from '../install/safe-extract'; import { scanForNativeBinaries } from '../install/native-scan'; import { parseManifest } from '../install/manifest'; @@ -36,6 +37,8 @@ interface RegistryVersion { apiVersion?: number; nativeModules?: boolean; publishedAt?: string; + /** base64 minisign signature (.minisig payload) over the artifact bytes. */ + signature?: string; } export interface RegistryEntry { id: string; @@ -47,6 +50,8 @@ export interface RegistryEntry { tags?: string[]; type: string; reviewedAt?: string | null; + /** base64 minisign author public key — stable across versions, TOFU-pinned. */ + authorPublicKey?: string; versions: RegistryVersion[]; } interface Registry { @@ -160,6 +165,11 @@ export class PluginRegistryService { const { bytes, sha256 } = await safeDownload(ver.downloadUrl, (ver.size ?? 50 * 1024 * 1024) + 4096); if (!sha256Matches(sha256, ver.sha256)) throw new RegistryError('integrity check failed (sha256 mismatch)'); + // 2b. author signature (opt-in) + TOFU key pin. Unsigned plugins skip this + // and install on sha256 alone (unchanged behaviour); a signed plugin must + // verify, and its author key must match the one pinned on first install. + this.verifySignatureAndTofu(id, bytes, entry, ver); + // 3. zip/tar-slip-safe extract to staging const staging = path.join(pluginsDataRoot(), '.staging', `${id}-${ver.version}-${Date.now()}`); try { @@ -183,11 +193,44 @@ export class PluginRegistryService { db.prepare('UPDATE plugins SET source_repo = ?, source_commit = ?, sha256 = ?, reviewed_at = ? WHERE id = ?').run( entry.repo, ver.commitSha, ver.sha256, entry.reviewedAt ?? null, id, ); + // Pin the author key on first successful install of a signed plugin (TOFU). + if (entry.authorPublicKey) { + db.prepare('UPDATE plugins SET author_pubkey = ? WHERE id = ?').run(entry.authorPublicKey, id); + } return { id, version: ver.version }; } finally { fs.rmSync(staging, { recursive: true, force: true }); } } + + /** + * Verify the author signature (if the entry declares a key + the version a + * signature) and enforce Trust-On-First-Use on the author key. + * + * - Neither key nor signature → unsigned plugin, skip (opt-in). + * - Key present but no signature (or vice versa) → hard stop; a half-signed + * entry is a misconfiguration we refuse rather than silently downgrade. + * - Signature invalid → hard stop. + * - Registry key differs from the key pinned on a prior install → hard stop + * (author change / rotation / attack; needs an explicit admin re-trust). + */ + private verifySignatureAndTofu(id: string, bytes: Buffer, entry: RegistryEntry, ver: RegistryVersion): void { + const pinned = (db.prepare('SELECT author_pubkey FROM plugins WHERE id = ?').get(id) as { author_pubkey?: string } | undefined)?.author_pubkey ?? null; + + if (!entry.authorPublicKey && !ver.signature) { + if (pinned) throw new RegistryError('this plugin was signed before but the update is unsigned — refusing'); + return; // unsigned throughout: sha256 is the only pin + } + if (!entry.authorPublicKey || !ver.signature) { + throw new RegistryError('incomplete signature: an author key and a version signature must both be present'); + } + if (pinned && pinned !== entry.authorPublicKey) { + throw new RegistryError("the plugin's author signing key changed since it was installed — re-trust it explicitly to continue"); + } + if (!verifyAuthorSignature(bytes, ver.signature, entry.authorPublicKey)) { + throw new RegistryError('author signature verification failed'); + } + } } function rawFileUrl(repo: string, commitSha: string, file: string): string { diff --git a/server/src/nest/plugins/runtime/plugin-host-entry.ts b/server/src/nest/plugins/runtime/plugin-host-entry.ts index d467248cd..6bde692d9 100644 --- a/server/src/nest/plugins/runtime/plugin-host-entry.ts +++ b/server/src/nest/plugins/runtime/plugin-host-entry.ts @@ -11,6 +11,7 @@ */ import path from 'node:path'; +import net from 'node:net'; import { createRequire } from 'node:module'; import { createPluginContext, type ChildTransport, type PluginContext, type PluginDefinition } from './plugin-sdk'; import type { Envelope, RpcError } from '../protocol/envelope'; @@ -18,6 +19,8 @@ import type { Envelope, RpcError } from '../protocol/envelope'; const pluginId = process.argv[2] || process.env.TREK_PLUGIN_ID || 'unknown'; const pluginDir = process.argv[3] || ''; +let pluginConfig: Record = {}; + const pending = new Map void; reject: (e: Error) => void }>(); let seq = 0; @@ -73,18 +76,21 @@ async function handleInvoke(req: { id: string; method: string; params: Record>[0]; - const result = await route.handler(pluginReq, ctx); + const result = await route.handler(pluginReq, invCtx); respond(true, result); } else if (req.method === 'invoke.job') { const jobId = req.params.jobId as string; const job = def.jobs?.find((j) => j.id === jobId); if (!job) throw new Error(`no job ${jobId}`); - await job.handler(ctx); + await job.handler(invCtx); respond(true, { ok: true }); } else { respond(false, `unknown invoke ${req.method}`); @@ -127,17 +133,26 @@ process.on('message', (raw: unknown) => { if (msg.k === 'evt') { if (msg.topic === 'init') { const d = msg.data as { config?: Record; egress?: string[] }; + pluginConfig = d.config ?? {}; installEgressGuard(d.egress ?? []); - void boot(d.config ?? {}); + void boot(pluginConfig); } else if (msg.topic === 'shutdown') void shutdown(); } }); /** * Restrict the plugin's outbound network to its declared egress hosts. With no - * declared egress, ALL outbound fetch is blocked. Wildcards like `*.host` match - * any subdomain. This is process-level defense in depth; the container runtime - * (v2) enforces it at the network layer. + * declared egress, ALL outbound is blocked. Wildcards like `*.host` match any + * subdomain. + * + * Two layers, so a plugin can't just sidestep `fetch` with a raw socket: + * 1. `globalThis.fetch` is wrapped (undici path). + * 2. `net.Socket.prototype.connect` is wrapped — the single TCP choke point that + * node:http / node:https / node:net / node:tls all funnel through — so + * `require('node:https').request(...)` is subject to the same allowlist. + * Under the OS permission model the child also cannot spawn a fresh process or + * load a native addon to escape these wrappers. This is strong defense in depth; + * a kernel/network-namespace guarantee still belongs to the container runtime. */ function installEgressGuard(egress: string[]): void { const patterns = egress.map((h) => h.trim().toLowerCase()).filter(Boolean); @@ -145,19 +160,42 @@ function installEgressGuard(egress: string[]): void { const h = hostname.toLowerCase(); return patterns.some((p) => (p.startsWith('*.') ? h === p.slice(2) || h.endsWith(p.slice(1)) : h === p)); }; + const realFetch = globalThis.fetch; - if (typeof realFetch !== 'function') return; - globalThis.fetch = ((input: unknown, init?: unknown) => { - const url = typeof input === 'string' ? input : (input as { url?: string })?.url ?? String(input); - let host: string; - try { - host = new URL(url).hostname; - } catch { - return Promise.reject(new Error('egress: invalid url')); + if (typeof realFetch === 'function') { + globalThis.fetch = ((input: unknown, init?: unknown) => { + const url = typeof input === 'string' ? input : (input as { url?: string })?.url ?? String(input); + let host: string; + try { + host = new URL(url).hostname; + } catch { + return Promise.reject(new Error('egress: invalid url')); + } + if (!allowed(host)) return Promise.reject(new Error(`egress: ${host} is not in the plugin's declared hosts`)); + return (realFetch as (i: unknown, n: unknown) => Promise)(input, init); + }) as typeof fetch; + } + + // TCP choke point: http/https/net/tls all end up here. A unix-socket (path) + // connect is local IPC, not network egress, so it's left alone. + const proto = net.Socket.prototype as unknown as { connect: (...a: unknown[]) => unknown }; + const realConnect = proto.connect; + proto.connect = function (this: unknown, ...args: unknown[]): unknown { + const first = args[0]; + let host: string | undefined; + if (first && typeof first === 'object') { + const o = first as { host?: string; path?: string }; + if (o.path) return realConnect.apply(this, args); // unix socket — local + host = o.host ?? 'localhost'; + } else if (typeof first === 'number' || typeof first === 'string') { + // connect(port[, host]) — a numeric-only path connects to localhost. + host = typeof args[1] === 'string' ? (args[1] as string) : 'localhost'; } - if (!allowed(host)) return Promise.reject(new Error(`egress: ${host} is not in the plugin's declared hosts`)); - return (realFetch as (i: unknown, n: unknown) => Promise)(input, init); - }) as typeof fetch; + if (host && !allowed(host)) { + throw new Error(`egress: ${host} is not in the plugin's declared hosts`); + } + return realConnect.apply(this, args); + }; } // Ask the host for the init payload (instance config), then wait for it. diff --git a/server/src/nest/plugins/runtime/plugin-sdk.ts b/server/src/nest/plugins/runtime/plugin-sdk.ts index de658b815..f10104ed9 100644 --- a/server/src/nest/plugins/runtime/plugin-sdk.ts +++ b/server/src/nest/plugins/runtime/plugin-sdk.ts @@ -17,9 +17,14 @@ export interface PluginContext { migrate(id: string, sql: string): Promise<{ applied: boolean }>; }; trips: { - getById(tripId: number, asUserId: number): Promise; - getPlaces(tripId: number, asUserId: number): Promise; - getReservations(tripId: number, asUserId: number): Promise; + // `asUserId` is accepted for source compatibility but IGNORED by the host — + // trip reads are always membership-checked against the authenticated user of + // the current invocation (the request's `req.user`), which the plugin cannot + // override. Only reachable from a route handler (a user context); a job has + // no user and its trip reads are refused. + getById(tripId: number, asUserId?: number): Promise; + getPlaces(tripId: number, asUserId?: number): Promise; + getReservations(tripId: number, asUserId?: number): Promise; }; users: { getById(id: number): Promise; @@ -77,11 +82,18 @@ export interface ChildTransport { emit(topic: string, data: unknown): void; } -/** Build the ctx the plugin's handlers receive — every method is an RPC call. */ +/** + * Build the ctx the plugin's handlers receive — every method is an RPC call. + * `invocationId` (the host's reqId for the route/job currently being handled) is + * attached to trip reads as `_inv` so the host can bind the acting user to THIS + * invocation. It is undefined for the load-time ctx (onLoad), where trip reads + * have no user and are refused. + */ export function createPluginContext( id: string, config: Record, t: ChildTransport, + invocationId?: string, ): PluginContext { return { id, @@ -92,9 +104,9 @@ export function createPluginContext( migrate: (mid, sql) => t.rpc('db.migrate', { id: mid, sql }) as Promise<{ applied: boolean }>, }, trips: { - getById: (tripId, asUserId) => t.rpc('trips.getById', { tripId, asUserId }), - getPlaces: (tripId, asUserId) => t.rpc('trips.getPlaces', { tripId, asUserId }) as Promise, - getReservations: (tripId, asUserId) => t.rpc('trips.getReservations', { tripId, asUserId }) as Promise, + getById: (tripId) => t.rpc('trips.getById', { tripId, _inv: invocationId }), + getPlaces: (tripId) => t.rpc('trips.getPlaces', { tripId, _inv: invocationId }) as Promise, + getReservations: (tripId) => t.rpc('trips.getReservations', { tripId, _inv: invocationId }) as Promise, }, users: { getById: (uid) => t.rpc('users.getById', { id: uid }), diff --git a/server/src/nest/plugins/supervisor/plugin-supervisor.ts b/server/src/nest/plugins/supervisor/plugin-supervisor.ts index 6bfba6e74..c66c7d7de 100644 --- a/server/src/nest/plugins/supervisor/plugin-supervisor.ts +++ b/server/src/nest/plugins/supervisor/plugin-supervisor.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'node:crypto'; import { fork, type ChildProcess } from 'node:child_process'; -import { resolveChildEntry, pluginCodeDir } from '../paths'; +import { resolveChildEntry, pluginCodeDir, pluginPermissionArgs } from '../paths'; import type { Envelope, RpcError, RpcRequest } from '../protocol/envelope'; import type { PluginRpcHost } from '../host/rpc-host'; @@ -48,6 +48,7 @@ interface Supervised { routes: PluginRouteInfo[]; jobs: string[]; pending: Map; // host→child invokes awaiting a response + invocations: Map; // reqId -> acting user of that invoke (undefined = no user, e.g. a job) activation?: { resolve: () => void; reject: (e: Error) => void }; } @@ -96,6 +97,7 @@ export class PluginSupervisor { routes: [], jobs: [], pending: new Map(), + invocations: new Map(), }; this.running.set(id, sup); this.ensureSweep(); @@ -126,8 +128,20 @@ export class PluginSupervisor { return this.running.get(id)?.routes ?? []; } - /** Send a host→child request (invoke a route or job) and await its response. */ - invoke(id: string, method: string, params: Record, timeoutMs = 30_000): Promise { + /** + * Send a host→child request (invoke a route or job) and await its response. + * `actingUserId` binds the authenticated user of this invocation on the HOST + * side: any trip read the child makes while handling it is membership-checked + * against THIS user, never against an id the plugin passes. A job carries no + * user (undefined) and therefore cannot read user-scoped data. + */ + invoke( + id: string, + method: string, + params: Record, + opts: { timeoutMs?: number; actingUserId?: number } = {}, + ): Promise { + const { timeoutMs = 30_000, actingUserId } = opts; const sup = this.running.get(id); if (!sup || sup.status !== 'active' || !sup.child) { return Promise.reject(new Error(`plugin ${id} is not active`)); @@ -136,11 +150,15 @@ export class PluginSupervisor { const reqId = randomUUID(); const timer = setTimeout(() => { sup.pending.delete(reqId); + sup.invocations.delete(reqId); reject(new Error('plugin invoke timed out')); }, timeoutMs); timer.unref?.(); sup.pending.set(reqId, { resolve, reject, timer }); - sup.child!.send({ k: 'req', id: reqId, method, params } satisfies Envelope); + // The child echoes this reqId as `_inv` on its trip reads; the host resolves + // the acting user from here, so the plugin cannot name an arbitrary user. + sup.invocations.set(reqId, actingUserId); + sup.child!.send({ k: 'req', id: reqId, method, params: { ...params, _inv: reqId } } satisfies Envelope); }); } @@ -156,10 +174,13 @@ export class PluginSupervisor { // ── internals ────────────────────────────────────────────────────────────── private spawn(sup: Supervised): void { - const { entry, execArgv, forkCwd } = resolveChildEntry(); + const { entry, execArgv, forkCwd, jsMode } = resolveChildEntry(); + // Prod (compiled) children get the OS permission model — a real kernel-level + // fs/child_process/native jail on top of the env scrub and RPC boundary. + const argv = jsMode ? [...execArgv, ...pluginPermissionArgs(sup.id)] : execArgv; const child = fork(entry, [sup.id, pluginCodeDir(sup.id)], { cwd: forkCwd ?? pluginCodeDir(sup.id), - execArgv, + execArgv: argv, stdio: ['ignore', 'pipe', 'pipe', 'ipc'], // Whitelist env — nothing inherited. No JWT_SECRET, no DB creds, no PATH-leaked secrets. env: { @@ -183,8 +204,13 @@ export class PluginSupervisor { if (!msg || typeof msg !== 'object') return; if (msg.k === 'req') { - // A ctx.* call from the plugin — dispatch through its capability host. - const res = await sup.rpcHost.dispatch(msg as RpcRequest); + // A ctx.* call from the plugin — dispatch through its capability host. The + // acting user comes from the invocation the child is currently handling + // (its `_inv` reqId → our invocation map), NOT from anything the plugin can + // set in the call params. + const inv = (msg as RpcRequest).params as { _inv?: unknown } | undefined; + const actingUserId = typeof inv?._inv === 'string' ? sup.invocations.get(inv._inv) : undefined; + const res = await sup.rpcHost.dispatch(msg as RpcRequest, actingUserId); sup.child?.send(res); return; } @@ -194,6 +220,7 @@ export class PluginSupervisor { const p = sup.pending.get(msg.id); if (!p) return; sup.pending.delete(msg.id); + sup.invocations.delete(msg.id); clearTimeout(p.timer); if (msg.ok) p.resolve(msg.result); else p.reject(new Error((msg as RpcError).error.message)); @@ -244,6 +271,7 @@ export class PluginSupervisor { p.reject(new Error(reason)); } sup.pending.clear(); + sup.invocations.clear(); } private onExit(sup: Supervised, code: number | null, signal: string | null): void { diff --git a/server/tests/integration/plugins/registry.test.ts b/server/tests/integration/plugins/registry.test.ts index 9120b6842..a8d2e7058 100644 --- a/server/tests/integration/plugins/registry.test.ts +++ b/server/tests/integration/plugins/registry.test.ts @@ -8,7 +8,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import zlib from 'node:zlib'; -import { createHash } from 'node:crypto'; +import { createHash, generateKeyPairSync, sign as edSign } from 'node:crypto'; const { safeDownload } = vi.hoisted(() => ({ safeDownload: vi.fn() })); vi.mock('../../../src/nest/plugins/install/safe-fetch', async (orig) => ({ @@ -22,7 +22,7 @@ const { testDb } = vi.hoisted(() => { db.exec(` CREATE TABLE plugins (id TEXT PRIMARY KEY, name TEXT, description TEXT, type TEXT, icon TEXT, version TEXT, api_version INTEGER, min_trek_version TEXT, permissions TEXT, capabilities TEXT DEFAULT '{}', granted_permissions TEXT, status TEXT, config TEXT, - source_repo TEXT, source_commit TEXT, sha256 TEXT, reviewed_at TEXT, updated_at TEXT); + source_repo TEXT, source_commit TEXT, sha256 TEXT, reviewed_at TEXT, author_pubkey TEXT, updated_at TEXT); CREATE TABLE plugin_settings_fields (plugin_id TEXT, field_key TEXT, label TEXT, input_type TEXT, placeholder TEXT, hint TEXT, required INTEGER, secret INTEGER, scope TEXT, options TEXT, oauth_config TEXT, sort_order INTEGER); CREATE TABLE plugin_error_log (id INTEGER PRIMARY KEY AUTOINCREMENT, plugin_id TEXT, level TEXT, message TEXT, ts TEXT);`); return { testDb: db }; @@ -86,12 +86,31 @@ beforeEach(() => { afterEach(() => { vi.unstubAllGlobals(); safeDownload.mockReset(); + delete (REGISTRY.plugins[0] as { authorPublicKey?: string }).authorPublicKey; + delete (REGISTRY.plugins[0].versions[0] as { signature?: string }).signature; delete process.env.TREK_PLUGINS_DATA_DIR; delete process.env.TREK_PLUGINS_DIR; fs.rmSync(dataRoot, { recursive: true, force: true }); fs.rmSync(codeRoot, { recursive: true, force: true }); }); +/** Ed25519 keypair as (raw 32-byte pubkey base64, raw-signature signer). */ +function signingKey() { + const { publicKey, privateKey } = generateKeyPairSync('ed25519'); + return { + pubB64: (publicKey.export({ format: 'der', type: 'spki' }).subarray(-32) as Buffer).toString('base64'), + sign: (b: Buffer) => (edSign(null, b, privateKey) as Buffer).toString('base64'), + }; +} +function stageSignedArtifact(pubB64: string, sig: (b: Buffer) => string): Buffer { + const artifact = makeArtifact({ id: 'flight-tracker', name: 'Flight', version: '1.0.0', type: 'widget', permissions: ['db:own'] }); + REGISTRY.plugins[0].versions[0].sha256 = createHash('sha256').update(artifact).digest('hex'); + (REGISTRY.plugins[0] as { authorPublicKey?: string }).authorPublicKey = pubB64; + (REGISTRY.plugins[0].versions[0] as { signature?: string }).signature = sig(artifact); + safeDownload.mockResolvedValue({ bytes: artifact, sha256: REGISTRY.plugins[0].versions[0].sha256 }); + return artifact; +} + describe('PluginRegistryService', () => { it('browse maps the aggregated registry to metadata', async () => { const list = await svc.browse(); @@ -235,4 +254,48 @@ describe('PluginRegistryService', () => { safeDownload.mockResolvedValue({ bytes: artifact, sha256: REGISTRY.plugins[0].versions[0].sha256 }); await expect(svc.install('flight-tracker')).rejects.toThrow(/!=/); }); + + it('installs a signed plugin and pins the author key (TOFU)', async () => { + const k = signingKey(); + stageSignedArtifact(k.pubB64, k.sign); + await expect(svc.install('flight-tracker')).resolves.toEqual({ id: 'flight-tracker', version: '1.0.0' }); + const row = testDb.prepare("SELECT author_pubkey FROM plugins WHERE id='flight-tracker'").get() as { author_pubkey: string }; + expect(row.author_pubkey).toBe(k.pubB64); + }); + + it('rejects a signed plugin whose signature does not verify', async () => { + const k = signingKey(); + stageSignedArtifact(k.pubB64, () => k.sign(Buffer.from('different bytes'))); + await expect(svc.install('flight-tracker')).rejects.toThrow(/signature verification failed/); + }); + + it('rejects an update whose author key differs from the pinned one (TOFU mismatch)', async () => { + const a = signingKey(); + stageSignedArtifact(a.pubB64, a.sign); + await svc.install('flight-tracker'); + const b = signingKey(); + stageSignedArtifact(b.pubB64, b.sign); // valid signature, but a different author key + await expect(svc.install('flight-tracker')).rejects.toThrow(/author signing key changed/); + }); + + it('rejects a half-signed entry (key without a signature)', async () => { + const artifact = makeArtifact({ id: 'flight-tracker', name: 'Flight', version: '1.0.0', type: 'widget', permissions: ['db:own'] }); + REGISTRY.plugins[0].versions[0].sha256 = createHash('sha256').update(artifact).digest('hex'); + (REGISTRY.plugins[0] as { authorPublicKey?: string }).authorPublicKey = signingKey().pubB64; // no version signature + safeDownload.mockResolvedValue({ bytes: artifact, sha256: REGISTRY.plugins[0].versions[0].sha256 }); + await expect(svc.install('flight-tracker')).rejects.toThrow(/incomplete signature/); + }); + + it('refuses to downgrade a previously-signed plugin to an unsigned update', async () => { + const k = signingKey(); + stageSignedArtifact(k.pubB64, k.sign); + await svc.install('flight-tracker'); + // now offer an unsigned update + const artifact = makeArtifact({ id: 'flight-tracker', name: 'Flight', version: '1.0.0', type: 'widget', permissions: ['db:own'] }); + REGISTRY.plugins[0].versions[0].sha256 = createHash('sha256').update(artifact).digest('hex'); + delete (REGISTRY.plugins[0] as { authorPublicKey?: string }).authorPublicKey; + delete (REGISTRY.plugins[0].versions[0] as { signature?: string }).signature; + safeDownload.mockResolvedValue({ bytes: artifact, sha256: REGISTRY.plugins[0].versions[0].sha256 }); + await expect(svc.install('flight-tracker')).rejects.toThrow(/unsigned/); + }); }); diff --git a/server/tests/unit/plugins/plugin-sdk.test.ts b/server/tests/unit/plugins/plugin-sdk.test.ts index 95e0cd17a..14da01e19 100644 --- a/server/tests/unit/plugins/plugin-sdk.test.ts +++ b/server/tests/unit/plugins/plugin-sdk.test.ts @@ -16,7 +16,9 @@ function fakeTransport() { describe('createPluginContext', () => { it('maps each ctx method onto the right RPC call', async () => { const { transport, rpc } = fakeTransport(); - const ctx = createPluginContext('p', {}, transport); + // An invocation-scoped ctx: trip reads carry `_inv` so the host can bind the + // acting user to this invocation. + const ctx = createPluginContext('p', {}, transport, 'inv-1'); await ctx.db.query('SELECT 1', 'a'); expect(rpc).toHaveBeenCalledWith('db.query', { sql: 'SELECT 1', args: ['a'] }); @@ -24,8 +26,8 @@ describe('createPluginContext', () => { await ctx.db.migrate('001', 'CREATE TABLE t (x)'); expect(rpc).toHaveBeenCalledWith('db.migrate', { id: '001', sql: 'CREATE TABLE t (x)' }); - await ctx.trips.getById(1, 42); - expect(rpc).toHaveBeenCalledWith('trips.getById', { tripId: 1, asUserId: 42 }); + await ctx.trips.getById(1); + expect(rpc).toHaveBeenCalledWith('trips.getById', { tripId: 1, _inv: 'inv-1' }); await ctx.ws.broadcastToTrip(1, 'ping', { a: 1 }); expect(rpc).toHaveBeenCalledWith('ws.broadcastToTrip', { tripId: 1, event: 'ping', data: { a: 1 } }); @@ -36,11 +38,11 @@ describe('createPluginContext', () => { await ctx.db.exec('DELETE FROM t'); expect(rpc).toHaveBeenCalledWith('db.exec', { sql: 'DELETE FROM t', args: [] }); - await ctx.trips.getPlaces(1, 42); - expect(rpc).toHaveBeenCalledWith('trips.getPlaces', { tripId: 1, asUserId: 42 }); + await ctx.trips.getPlaces(1); + expect(rpc).toHaveBeenCalledWith('trips.getPlaces', { tripId: 1, _inv: 'inv-1' }); - await ctx.trips.getReservations(1, 42); - expect(rpc).toHaveBeenCalledWith('trips.getReservations', { tripId: 1, asUserId: 42 }); + await ctx.trips.getReservations(1); + expect(rpc).toHaveBeenCalledWith('trips.getReservations', { tripId: 1, _inv: 'inv-1' }); await ctx.ws.broadcastToUser(9, 'poke', { x: 2 }); expect(rpc).toHaveBeenCalledWith('ws.broadcastToUser', { userId: 9, event: 'poke', data: { x: 2 } }); diff --git a/server/tests/unit/plugins/plugins-proxy.test.ts b/server/tests/unit/plugins/plugins-proxy.test.ts index e1d43d5e4..36e402182 100644 --- a/server/tests/unit/plugins/plugins-proxy.test.ts +++ b/server/tests/unit/plugins/plugins-proxy.test.ts @@ -81,11 +81,12 @@ describe('PluginsProxyController', () => { expect(res.statusCode).toBe(200); expect(res.headers['content-type']).toBe('application/json'); expect(res.body).toBe('{"ok":true}'); - // the child receives only a whitelisted user view, never the token/cookie + // the child receives only a whitelisted user view, never the token/cookie; + // the acting user (5) is bound server-side as the 4th invoke arg expect(runtime.invoke).toHaveBeenCalledWith('p', 'invoke.route', expect.objectContaining({ routeId: 0, req: expect.objectContaining({ user: { id: 5, username: 'ada', isAdmin: false } }), - })); + }), 5); }); it('a public (auth:false) route skips the session check', async () => { @@ -94,9 +95,10 @@ describe('PluginsProxyController', () => { await new PluginsProxyController(runtime).proxy('p', fakeReq('POST', '/webhook'), res as never); expect(res.statusCode).toBe(200); expect(extractTokenMock).not.toHaveBeenCalled(); + // a public route has no session user → no bound acting user (undefined) expect(runtime.invoke).toHaveBeenCalledWith('p', 'invoke.route', expect.objectContaining({ req: expect.objectContaining({ user: null }), - })); + }), undefined); }); it('strips unsafe response headers from the child', async () => { diff --git a/server/tests/unit/plugins/protocol-paths.test.ts b/server/tests/unit/plugins/protocol-paths.test.ts index c46e3cb80..d9178580d 100644 --- a/server/tests/unit/plugins/protocol-paths.test.ts +++ b/server/tests/unit/plugins/protocol-paths.test.ts @@ -5,7 +5,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { isKnownPermission, METHOD_PERMISSION, KNOWN_METHODS } from '../../../src/nest/plugins/protocol/envelope'; import path from 'node:path'; -import { pluginsCodeRoot, pluginsDataRoot, pluginCodeDir, pluginDbFile, resolveChildEntry } from '../../../src/nest/plugins/paths'; +import { pluginsCodeRoot, pluginsDataRoot, pluginCodeDir, pluginDbFile, resolveChildEntry, serverCodeRoot, pluginPermissionArgs } from '../../../src/nest/plugins/paths'; afterEach(() => { delete process.env.TREK_PLUGINS_DIR; @@ -47,5 +47,25 @@ describe('paths', () => { const r = resolveChildEntry(); expect(r.entry).toMatch(/plugin-host-entry\.(js|ts)$/); expect(Array.isArray(r.execArgv)).toBe(true); + expect(typeof r.jsMode).toBe('boolean'); + }); + + it('builds scoped OS-permission flags for a plugin child (default on)', () => { + delete process.env.TREK_PLUGIN_PERMISSIONS; + const args = pluginPermissionArgs('flight-tracker'); + expect(args).toContain('--permission'); + // read is scoped to the compiled server dir + this plugin's own code dir… + expect(args.some((a) => a === `--allow-fs-read=${serverCodeRoot()}`)).toBe(true); + expect(args.some((a) => a === `--allow-fs-read=${pluginCodeDir('flight-tracker')}`)).toBe(true); + // …and never grants fs-write / child_process / the data root. + expect(args.some((a) => a.startsWith('--allow-fs-write'))).toBe(false); + expect(args.some((a) => a.startsWith('--allow-child-process'))).toBe(false); + expect(serverCodeRoot()).not.toContain(`${require('node:path').sep}data`); + }); + + it('lets an operator opt out of the permission model', () => { + process.env.TREK_PLUGIN_PERMISSIONS = 'off'; + expect(pluginPermissionArgs('flight-tracker')).toEqual([]); + delete process.env.TREK_PLUGIN_PERMISSIONS; }); }); diff --git a/server/tests/unit/plugins/rpc-host.test.ts b/server/tests/unit/plugins/rpc-host.test.ts index 2512c307b..30b51a7f3 100644 --- a/server/tests/unit/plugins/rpc-host.test.ts +++ b/server/tests/unit/plugins/rpc-host.test.ts @@ -64,7 +64,8 @@ describe('PluginRpcHost — capability enforcement', () => { it('db:read:trips reads a trip the acting user can access', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); - const res = await host.dispatch(req('trips.getById', { tripId: 1, asUserId: 42 })); + // The acting user is bound by the HOST (2nd dispatch arg), never from params. + const res = await host.dispatch(req('trips.getById', { tripId: 1 }), 42); expect(ok(res)).toBe(true); // returns the ACTUAL trip row (title/start_date), not the access-check object expect((res as RpcResponse).result).toMatchObject({ id: 1, title: 'Japan', start_date: '2027-01-01' }); @@ -73,17 +74,25 @@ describe('PluginRpcHost — capability enforcement', () => { it('db:read:trips is still RESOURCE_FORBIDDEN when the user is not a member', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); - const res = await host.dispatch(req('trips.getById', { tripId: 1, asUserId: 99 })); + const res = await host.dispatch(req('trips.getById', { tripId: 1 }), 99); expect((res as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); }); + it('a trip read with NO bound acting user is RESOURCE_FORBIDDEN (jobs / forged calls)', async () => { + const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); + // A plugin-supplied asUserId is ignored; without a host-bound user, deny. + const res = await host.dispatch(req('trips.getById', { tripId: 1, asUserId: 42 }), undefined); + expect((res as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); + expect(deps.canAccessTrip).not.toHaveBeenCalled(); + }); + it('trips.getPlaces is membership-checked before the core read', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); - const forbidden = await host.dispatch(req('trips.getPlaces', { tripId: 2, asUserId: 42 })); + const forbidden = await host.dispatch(req('trips.getPlaces', { tripId: 2 }), 42); expect((forbidden as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); expect(deps.db.prepare).not.toHaveBeenCalled(); - const allowed = await host.dispatch(req('trips.getPlaces', { tripId: 1, asUserId: 42 })); + const allowed = await host.dispatch(req('trips.getPlaces', { tripId: 1 }), 42); expect(ok(allowed)).toBe(true); expect((allowed as RpcResponse).result).toEqual([{ id: 7, name: 'Place' }]); }); @@ -142,7 +151,7 @@ describe('PluginRpcHost — capability enforcement', () => { it('trips.getReservations is membership-checked and reads reservations', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); - const res = await host.dispatch(req('trips.getReservations', { tripId: 1, asUserId: 42 })); + const res = await host.dispatch(req('trips.getReservations', { tripId: 1 }), 42); expect(ok(res)).toBe(true); expect(deps.db.prepare).toHaveBeenCalledWith(expect.stringContaining('FROM reservations')); }); @@ -169,8 +178,8 @@ describe('PluginRpcHost — capability enforcement', () => { it('coerces numeric string params and tolerates a missing params object', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips', 'db:own']), deps); - // tripId/asUserId as strings -> coerced to numbers - const res = await host.dispatch(req('trips.getById', { tripId: '1', asUserId: '42' })); + // tripId as a string -> coerced to a number; acting user is the bound host arg + const res = await host.dispatch(req('trips.getById', { tripId: '1' }), 42); expect(ok(res)).toBe(true); // a request with no params object at all -> BAD_PARAMS (sql missing), not a crash const noParams = await host.dispatch({ k: 'req', id: 'y', method: 'db.query', params: undefined }); diff --git a/server/tests/unit/plugins/verify-signature.test.ts b/server/tests/unit/plugins/verify-signature.test.ts new file mode 100644 index 000000000..6fc2c4146 --- /dev/null +++ b/server/tests/unit/plugins/verify-signature.test.ts @@ -0,0 +1,81 @@ +/** + * Author-signature verification (#plugins, #4). Ed25519 via node:crypto, in the + * minisign shapes TREK accepts: bare raw key+sig, minisign legacy ('Ed', signs + * the file), and minisign prehashed ('ED', signs BLAKE2b-512 of the file). + */ +import { describe, it, expect } from 'vitest'; +import crypto from 'node:crypto'; +import { verifyAuthorSignature, SignatureError } from '../../../src/nest/plugins/install/verify-signature'; + +// A raw Ed25519 keypair, exported as the 32-byte raw public key + a signer. +function makeKeypair() { + const { publicKey, privateKey } = crypto.generateKeyPairSync('ed25519'); + const rawPub = publicKey.export({ format: 'der', type: 'spki' }).subarray(-32); // last 32 bytes = the key + return { + rawPub, + sign: (msg: Buffer) => crypto.sign(null, msg, privateKey), + }; +} + +const KEY_ID = Buffer.from('0011223344556677', 'hex'); +const b64 = (b: Buffer) => b.toString('base64'); + +describe('verifyAuthorSignature', () => { + const bytes = Buffer.from('the exact plugin.zip bytes'); + + it('accepts a bare raw Ed25519 key + 64-byte signature over the file', () => { + const { rawPub, sign } = makeKeypair(); + expect(verifyAuthorSignature(bytes, b64(sign(bytes)), b64(rawPub))).toBe(true); + }); + + it('rejects a valid signature from a different key', () => { + const a = makeKeypair(); + const b = makeKeypair(); + expect(verifyAuthorSignature(bytes, b64(a.sign(bytes)), b64(b.rawPub))).toBe(false); + }); + + it('rejects a signature over tampered bytes', () => { + const { rawPub, sign } = makeKeypair(); + const sig = b64(sign(bytes)); + expect(verifyAuthorSignature(Buffer.from('tampered'), sig, b64(rawPub))).toBe(false); + }); + + it('verifies the minisign legacy format (Ed — signs the raw file)', () => { + const { rawPub, sign } = makeKeypair(); + const pub = b64(Buffer.concat([Buffer.from('Ed'), KEY_ID, rawPub])); + const sig = b64(Buffer.concat([Buffer.from('Ed'), KEY_ID, sign(bytes)])); + expect(verifyAuthorSignature(bytes, sig, pub)).toBe(true); + }); + + it('verifies the minisign prehashed format (ED — signs BLAKE2b-512 of the file)', () => { + const { rawPub, sign } = makeKeypair(); + const hash = crypto.createHash('blake2b512').update(bytes).digest(); + const pub = b64(Buffer.concat([Buffer.from('Ed'), KEY_ID, rawPub])); + const sig = b64(Buffer.concat([Buffer.from('ED'), KEY_ID, sign(hash)])); + expect(verifyAuthorSignature(bytes, sig, pub)).toBe(true); + }); + + it('rejects when the key ids in key and signature disagree', () => { + const { rawPub, sign } = makeKeypair(); + const pub = b64(Buffer.concat([Buffer.from('Ed'), KEY_ID, rawPub])); + const otherId = Buffer.from('8899aabbccddeeff', 'hex'); + const sig = b64(Buffer.concat([Buffer.from('Ed'), otherId, sign(bytes)])); + expect(verifyAuthorSignature(bytes, sig, pub)).toBe(false); + }); + + it('parses a two-line minisign key (comment + base64)', () => { + const { rawPub, sign } = makeKeypair(); + const pub = `untrusted comment: minisign public key ABC\n${b64(Buffer.concat([Buffer.from('Ed'), KEY_ID, rawPub]))}`; + const sig = `untrusted comment: signature\n${b64(Buffer.concat([Buffer.from('Ed'), KEY_ID, sign(bytes)]))}`; + expect(verifyAuthorSignature(bytes, sig, pub)).toBe(true); + }); + + it('throws SignatureError on a malformed key', () => { + expect(() => verifyAuthorSignature(bytes, b64(Buffer.alloc(64)), b64(Buffer.alloc(7)))).toThrow(SignatureError); + }); + + it('throws SignatureError on a malformed signature', () => { + const { rawPub } = makeKeypair(); + expect(() => verifyAuthorSignature(bytes, b64(Buffer.alloc(10)), b64(rawPub))).toThrow(SignatureError); + }); +}); diff --git a/wiki/Plugin-Publishing.md b/wiki/Plugin-Publishing.md index 728eefdf4..aa1a57c16 100644 --- a/wiki/Plugin-Publishing.md +++ b/wiki/Plugin-Publishing.md @@ -81,6 +81,49 @@ TREK verifies the downloaded bytes against `sha256` and refuses to install on a mismatch. A `reviewedAt` date on your entry means a maintainer looked at that exact commit — it is **not** an ongoing guarantee. +## Signing your releases (optional, recommended) + +`sha256` proves the bytes are the ones the *registry* vouches for. An author +signature additionally proves the bytes were signed by **you** — so a compromised +registry cannot ship attacker code under your name. TREK verifies the signature +offline (Ed25519, no external service), and pins your key on first install +(trust-on-first-use): a later release signed with a different key is refused until +an admin re-trusts it. + +**One-time — create a key:** + +``` +minisign -G # writes minisign.key (keep secret) + minisign.pub +``` + +Put the public key in your registry entry as `authorPublicKey` (the base64 payload +line from `minisign.pub`). It is stable across versions. + +**Each release — sign the artifact:** + +``` +minisign -Sm plugin.zip # writes plugin.zip.minisig +``` + +Add the base64 signature line from `plugin.zip.minisig` to that version as +`signature`, next to its `sha256`. Example entry: + +```jsonc +{ + "id": "flight-tracker", + "authorPublicKey": "RWQ…base64 minisign public key…", + "versions": [{ + "version": "1.2.0", + "sha256": "3b2a…", + "signature": "RUR…base64 .minisig payload…" + }] +} +``` + +Signing is **opt-in**: an entry without `authorPublicKey`/`signature` installs on +`sha256` alone, exactly as before. But once a plugin has shipped signed, an +*unsigned* update for it is refused — don't drop the signature between versions. + ## Updating Add a new version entry (new `version`/`gitTag`/`commitSha`/`sha256`) to your