From 64136484ee204be70c5cd2d517bc8e0bfc718f81 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 4 Jul 2026 18:32:26 +0200 Subject: [PATCH] fix(plugins): security hardening from the PR #1415 audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remediates the findings from the adversarial audit (threat model: malicious plugin author + malicious artifact). Highlights: Critical - proxy: force nosniff + Content-Disposition: attachment on every proxied reply and drop location/content-disposition + non-2xx from the passthrough, so a plugin can't serve an HTML document at TREK's origin (sandbox-escape → account takeover) or an open redirect. High - db:own runs synchronously in the host: cap the plugin DB (max_page_count) and row-cap query() via iterate() so a recursive CTE / huge blob can't stall the event loop, OOM, or exhaust the shared volume. - supervisor: measure child RSS host-side (/proc//statm) instead of trusting the spoofable heartbeat; add an activation timeout so a stuck onLoad can't hang activate() or peg a core unreaped. - safe-extract: enforce entry-count + cumulative-size limits INSIDE readZip before inflating (decompression-bomb OOM). - re-consent: activate() never widens granted permissions without explicit consent (409 CONSENT_REQUIRED); the row toggle + "Update All" route through the consent dialog, which now queues instead of overwriting. Medium/low - egress: gate dgram hostnames through the IP-vetting resolver; block the low-level socket escape (process.binding) + lock the wrapped prototypes; canonicalize IPv6 in isBlockedIp (hex-mapped/compressed metadata); reject degenerate `*.` / whole-TLD / spaced outbound hosts in the manifest + CSP. - ws:broadcast is membership-gated to the acting user's trips / own connections; users.getById is scoped to users the acting user can see (no enumeration). - safe-fetch streams + aborts at the byte cap (chunked codeload OOM); isPrivateIp reuses the canonicalizing check. - native-scan throws instead of silently passing past its entry cap. - SDK: dev serves binary assets as raw buffers + handles EADDRINUSE; manifest validator gains the reserved-id + outbound-host checks; wikis corrected. Tests updated for the new membership-gated behaviour + regression tests added (IPv6 canonicalization, wildcard hardening, outbound-host validation, ws/user scoping). 234 plugin tests + 24 SDK tests green. --- client/src/api/client.ts | 2 +- .../components/Admin/AdminPluginsPanel.tsx | 40 ++++++++++---- client/src/components/Plugins/PluginFrame.tsx | 9 ++++ plugin-sdk/src/cli/dev.ts | 18 +++++-- plugin-sdk/src/manifest.ts | 13 +++++ .../src/nest/plugins/host/create-rpc-host.ts | 12 +++++ .../nest/plugins/host/plugin-data.service.ts | 22 +++++++- server/src/nest/plugins/host/rpc-host.ts | 35 ++++++++---- server/src/nest/plugins/install/manifest.ts | 15 ++++++ .../src/nest/plugins/install/native-scan.ts | 4 +- .../src/nest/plugins/install/safe-extract.ts | 25 ++++++--- server/src/nest/plugins/install/safe-fetch.ts | 47 ++++++++++------ .../nest/plugins/plugin-frame.controller.ts | 9 +++- .../nest/plugins/plugin-runtime.service.ts | 29 ++++++++-- .../nest/plugins/plugins-proxy.controller.ts | 17 +++++- server/src/nest/plugins/plugins.controller.ts | 11 ++-- .../src/nest/plugins/runtime/egress-policy.ts | 47 +++++++++++++--- .../nest/plugins/runtime/plugin-host-entry.ts | 33 ++++++++++++ .../plugins/supervisor/plugin-supervisor.ts | 54 ++++++++++++++++++- .../unit/plugins/create-rpc-host.test.ts | 8 +-- .../tests/unit/plugins/egress-policy.test.ts | 17 ++++++ server/tests/unit/plugins/manifest.test.ts | 10 ++++ server/tests/unit/plugins/rpc-host.test.ts | 40 +++++++++++--- wiki/Plugin-Publishing.md | 5 +- wiki/Plugins.md | 5 +- 25 files changed, 442 insertions(+), 85 deletions(-) diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 5d95f542b..f9a388d66 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -464,7 +464,7 @@ export const adminApi = { pluginBrowse: () => apiClient.get('/admin/plugins/registry').then(r => r.data), pluginDetail: (id: string) => apiClient.get(`/admin/plugins/registry/${encodeURIComponent(id)}`).then(r => r.data), pluginInstall: (id: string, version?: string) => apiClient.post('/admin/plugins/install', { id, version }).then(r => r.data), - pluginActivate: (id: string) => apiClient.post(`/admin/plugins/${id}/activate`).then(r => r.data), + pluginActivate: (id: string, consent?: boolean) => apiClient.post(`/admin/plugins/${id}/activate`, consent ? { consent: true } : {}).then(r => r.data), pluginDeactivate: (id: string) => apiClient.post(`/admin/plugins/${id}/deactivate`).then(r => r.data), pluginUpdate: (id: string) => apiClient.post(`/admin/plugins/${id}/update`).then(r => r.data), pluginUninstall: (id: string, deleteData: boolean) => apiClient.post(`/admin/plugins/${id}/uninstall`, { deleteData }).then(r => r.data), diff --git a/client/src/components/Admin/AdminPluginsPanel.tsx b/client/src/components/Admin/AdminPluginsPanel.tsx index 9da7ec5c0..f030adc83 100644 --- a/client/src/components/Admin/AdminPluginsPanel.tsx +++ b/client/src/components/Admin/AdminPluginsPanel.tsx @@ -157,7 +157,9 @@ export default function AdminPluginsPanel() { const [detailFor, setDetailFor] = useState(null) const [errorsFor, setErrorsFor] = useState<{ id: string; rows: Array<{ ts: string; level: string; message: string }> } | null>(null) const [confirmUninstall, setConfirmUninstall] = useState(null) - const [consentUpdate, setConsentUpdate] = useState<{ plugin: PluginRow; version: string; newPermissions: string[]; newEgress: string[] } | null>(null) + // A QUEUE, not one slot: "Update All" can produce several re-consent prompts — + // each must be shown, not silently overwritten by the last one. + const [consentQueue, setConsentQueue] = useState>([]) const [menu, setMenu] = useState(null) // Toolbar state. @@ -209,12 +211,32 @@ export default function AdminPluginsPanel() { const restart = (id: string) => act(id, async () => { await adminApi.pluginDeactivate(id); await adminApi.pluginActivate(id) }, t('admin.plugins.restarted')) const installedIds = new Set(plugins.map(p => p.id)) + // Enable/disable a plugin. Re-enabling one whose update widened its permissions + // must NOT grant them silently — the server answers 409 CONSENT_REQUIRED and we + // route through the same consent dialog the Update button uses. + const toggle = (p: PluginRow) => { + if (busy === p.id) return + if (p.enabled === 1) { void act(p.id, () => adminApi.pluginDeactivate(p.id), t('admin.plugins.deactivated')); return } + setBusy(p.id); setMenu(null) + adminApi.pluginActivate(p.id) + .then(() => toast.success(t('admin.plugins.activated'))) + .catch((e: { response?: { status?: number; data?: { code?: string; error?: string; newPermissions?: string[]; newEgress?: string[] } } }) => { + const d = e?.response?.data + if (e?.response?.status === 409 && d?.code === 'CONSENT_REQUIRED') { + setConsentQueue(qq => [...qq, { plugin: p, version: latest[p.id] ?? p.version ?? '', newPermissions: d.newPermissions ?? [], newEgress: d.newEgress ?? [] }]) + } else { + toast.error(d?.error || t('admin.plugins.actionError')) + } + }) + .finally(() => { setBusy(null); refresh() }) + } + const runUpdate = (p: PluginRow) => { setBusy(p.id); setMenu(null) adminApi.pluginUpdate(p.id) .then((r: { version: string; activated: boolean; newPermissions: string[]; newEgress: string[] }) => { if (r.activated || (r.newPermissions.length === 0 && r.newEgress.length === 0)) toast.success(t('admin.plugins.updated')) - else setConsentUpdate({ plugin: p, version: r.version, newPermissions: r.newPermissions, newEgress: r.newEgress }) + else setConsentQueue(qq => [...qq, { plugin: p, version: r.version, newPermissions: r.newPermissions, newEgress: r.newEgress }]) }) .catch(e => toast.error((e as { response?: { data?: { error?: string } } })?.response?.data?.error || t('admin.plugins.actionError'))) .finally(() => { setBusy(null); refresh() }) @@ -380,8 +402,7 @@ export default function AdminPluginsPanel() { ) : shownInstalled.map(p => ( busy !== p.id && act(p.id, () => p.enabled === 1 ? adminApi.pluginDeactivate(p.id) : adminApi.pluginActivate(p.id), - p.enabled === 1 ? t('admin.plugins.deactivated') : t('admin.plugins.activated'))} + onToggle={() => toggle(p)} onUpdate={() => runUpdate(p)} onRestart={() => restart(p.id)} onErrors={() => openErrors(p.id)} onUninstall={() => { setMenu(null); setConfirmUninstall(p) }} /> ))} @@ -430,14 +451,15 @@ export default function AdminPluginsPanel() { message={t('admin.plugins.uninstallBody')} /> - {consentUpdate && ( + {consentQueue[0] && ( { - const c = consentUpdate; setConsentUpdate(null) - await act(c.plugin.id, () => adminApi.pluginActivate(c.plugin.id), t('admin.plugins.updated')) + const c = consentQueue[0]; setConsentQueue(qq => qq.slice(1)) + // consent:true — the ONLY path that may widen a plugin's granted rights. + await act(c.plugin.id, () => adminApi.pluginActivate(c.plugin.id, true), t('admin.plugins.updated')) }} - onLater={() => { setConsentUpdate(null); toast.success(t('admin.plugins.updateKeptOff')) }} + onLater={() => { setConsentQueue(qq => qq.slice(1)); toast.success(t('admin.plugins.updateKeptOff')) }} /> )} diff --git a/client/src/components/Plugins/PluginFrame.tsx b/client/src/components/Plugins/PluginFrame.tsx index 4be5a32a4..38cdd957e 100644 --- a/client/src/components/Plugins/PluginFrame.tsx +++ b/client/src/components/Plugins/PluginFrame.tsx @@ -35,6 +35,11 @@ type Inbound = export default function PluginFrame({ pluginId, tripId = null, className, title }: PluginFrameProps) { const frameRef = useRef(null) + // A sandboxed frame may navigate ITSELF (connect-src can't stop that). If the + // plugin's document navigates away, its window identity still matches our + // iframe, so we'd keep answering trek:invoke/context to the new (possibly + // attacker) origin. Track loads: after the initial document, refuse the bridge. + const loadsRef = useRef(0) const { locale } = useTranslation() const navigate = useNavigate() const toast = useToast() @@ -59,6 +64,9 @@ export default function PluginFrame({ pluginId, tripId = null, className, title const onMessage = async (ev: MessageEvent) => { // The ONLY trusted identity: the message came from OUR iframe's window. if (ev.source !== frame.contentWindow) return + // …AND that window still holds the original plugin document (loaded once). + // A 2nd load means the frame navigated elsewhere — stop bridging to it. + if (loadsRef.current > 1) return const msg = ev.data as Inbound if (!msg || typeof msg !== 'object') return @@ -104,6 +112,7 @@ export default function PluginFrame({ pluginId, tripId = null, className, title