From e9746fcf78c6f005963e2cef071f301fac7b0283 Mon Sep 17 00:00:00 2001 From: Bernd Rodler Date: Tue, 4 Aug 2026 09:03:14 +0200 Subject: [PATCH] feat(plugins): admin review panel for scanner findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overrideWarnings escape hatch added alongside the bundle scan was API-only: an admin uploading a crypto plugin through the web form hit a 400 with canOverride and had no way to act on it, which left S/MIME and PGP bundles uninstallable through the UI. Hold the rejected file client-side and show the findings — pattern per file — with "Install anyway" and "Cancel". Proceeding re-posts the same file with overrideWarnings, so the decision stays explicit and lands in the audit log. The route now echoes accepted findings back on success so the confirmation says how many were waved through rather than reporting a bare install. Also replaces a dead `data.warnings` read with the live `findings` field; the route never returned `warnings` on success, so that branch never ran. Completes B-01. Co-Authored-By: Claude Opus 4.8 --- app/(main)/admin/_tabs/plugins.tsx | 79 +++++++++++++++++++++++++++--- app/api/admin/plugins/route.ts | 4 +- vnc/VNC-CHANGES.md | 3 +- 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/app/(main)/admin/_tabs/plugins.tsx b/app/(main)/admin/_tabs/plugins.tsx index 6569e0c9..2df1c281 100644 --- a/app/(main)/admin/_tabs/plugins.tsx +++ b/app/(main)/admin/_tabs/plugins.tsx @@ -26,6 +26,10 @@ export function PluginsTab() { const [loading, setLoading] = useState(true); const [uploading, setUploading] = useState(false); const [message, setMessage] = useState<{ type: 'success' | 'error'; text: string } | null>(null); + // Bundle held back by the pattern scanner, awaiting an explicit admin decision. + const [pendingScan, setPendingScan] = useState< + { file: File; findings: Array<{ file: string; patterns: string[] }> } | null + >(null); const fileInputRef = useRef(null); const [policy, setPolicy] = useState({ ...DEFAULT_POLICY }); const [policyDirty, setPolicyDirty] = useState(false); @@ -104,15 +108,17 @@ export function PluginsTab() { } } - async function handleUpload(e: React.ChangeEvent) { - const file = e.target.files?.[0]; - if (!file) return; - + // Upload a bundle. The scanner may refuse it for containing patterns that are + // expected in a vendored crypto library (openpgp.js, pkijs); in that case the + // server returns `canOverride` and we hold the file so the admin can review + // the findings and decide. `override` re-posts the same file with consent. + async function uploadPlugin(file: File, override: boolean) { setUploading(true); setMessage(null); const formData = new FormData(); formData.append('file', file); + if (override) formData.append('overrideWarnings', 'true'); try { const res = await apiFetch('/api/admin/plugins', { @@ -122,13 +128,22 @@ export function PluginsTab() { const data = await res.json(); if (res.ok) { - const warnings = data.warnings?.length ? ` (${data.warnings.length} warning(s))` : ''; - setMessage({ type: 'success', text: `Plugin "${data.plugin.name}" installed${warnings}` }); + setPendingScan(null); + const accepted = data.findings?.length + ? ` — ${data.findings.length} scanner finding(s) accepted and logged` + : ''; + setMessage({ type: 'success', text: `Plugin "${data.plugin.name}" installed${accepted}` }); await fetchPlugins(); + } else if (data.canOverride && Array.isArray(data.findings) && !override) { + // Hold the file rather than the error: the admin needs to see WHAT + // tripped, in WHICH file, before deciding. + setPendingScan({ file, findings: data.findings }); } else { + setPendingScan(null); setMessage({ type: 'error', text: data.error || 'Upload failed' }); } } catch { + setPendingScan(null); setMessage({ type: 'error', text: 'Upload failed' }); } finally { setUploading(false); @@ -136,6 +151,13 @@ export function PluginsTab() { } } + async function handleUpload(e: React.ChangeEvent) { + const file = e.target.files?.[0]; + if (!file) return; + setPendingScan(null); + await uploadPlugin(file, false); + } + async function togglePlugin(id: string, enabled: boolean) { setMessage(null); const res = await apiFetch('/api/admin/plugins', { @@ -302,6 +324,51 @@ export function PluginsTab() { )} + {pendingScan && ( +
+
+ +
+

+ Scanner flagged {pendingScan.file.name} +

+

+ These patterns can indicate malicious code, but they also appear in legitimate + minified crypto libraries such as openpgp.js and pkijs. Review the findings before + proceeding — installing anyway is recorded in the audit log. +

+
+
+ +
    + {pendingScan.findings.map(f => ( +
  • + {f.file} + — {f.patterns.join(', ')} +
    • + ))} +
+ +
+ + +
+
+ )} +
diff --git a/app/api/admin/plugins/route.ts b/app/api/admin/plugins/route.ts index b6ad5232..6acf8b14 100644 --- a/app/api/admin/plugins/route.ts +++ b/app/api/admin/plugins/route.ts @@ -255,7 +255,9 @@ export async function POST(request: NextRequest) { logger.warn('Plugin installed with scanner override', { id: plugin.id, findings }); } - return NextResponse.json({ plugin }); + // Echo accepted findings back so the admin UI can confirm exactly what was + // waved through, rather than reporting a bare success. + return NextResponse.json(findings.length > 0 ? { plugin, findings } : { plugin }); } catch (error) { logger.error('Plugin install error', { error: error instanceof Error ? error.message : 'Unknown error' }); return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); diff --git a/vnc/VNC-CHANGES.md b/vnc/VNC-CHANGES.md index 15e34ed5..fc4ab734 100644 --- a/vnc/VNC-CHANGES.md +++ b/vnc/VNC-CHANGES.md @@ -49,6 +49,7 @@ microfrontends integration was also added and reverted the same day._ | 2026-08-03 | `lib/builtin-themes.ts` | add `srcSkin` (MD3 component overrides: shape scale, filled buttons, text fields, cards, dialogs, state layers, switches, login card); add @font-face + typography to `builtin-src`; bump to v1.1.0 | SRC theme: keep colors + fonts, apply MD3 design system | | 2026-08-04 | `lib/plugin-sandbox/loader.ts` | **B-04 security fix** — gate hook registration on granted permissions via new `HOOK_PERMISSIONS` map; refused hooks are skipped, logged and counted | `info.hooks` is self-reported by the sandbox, so an untrusted plugin could claim `onRenderEmailBody` and replace any rendered email body without holding `email:render-takeover`. Consent copy gated what the user was *asked*, not what the host *allowed*. | | 2026-08-04 | `lib/plugin-sandbox/host-api.ts` | export `hasPermission()` (was module-private) | one source of truth for the permission rule — the loader gate and the RPC gate must not drift apart | -| 2026-08-04 | `app/api/admin/plugins/route.ts` | **B-01** — scan all `.js`/`.mjs` in the bundle (was entrypoint only); return structured `findings` + `canOverride`; allow admin `overrideWarnings=true` with a `plugin.install.scan_override` audit entry | hard-reject on `eval(`/`new Function(`/`innerHTML =` made every crypto plugin uninstallable (minified openpgp.js/pkijs trip it), while only scanning the entrypoint left a trivial bypass. Route is already admin-authenticated, so the scan is defence-in-depth, not a trust boundary. | +| 2026-08-04 | `app/api/admin/plugins/route.ts` | **B-01** — scan all `.js`/`.mjs` in the bundle (was entrypoint only); return structured `findings` + `canOverride`; allow admin `overrideWarnings=true` with a `plugin.install.scan_override` audit entry; echo accepted `findings` on success | hard-reject on `eval(`/`new Function(`/`innerHTML =` made every crypto plugin uninstallable (minified openpgp.js/pkijs trip it), while only scanning the entrypoint left a trivial bypass. Route is already admin-authenticated, so the scan is defence-in-depth, not a trust boundary. | +| 2026-08-04 | `app/(main)/admin/_tabs/plugins.tsx` | **B-01 (UI)** — scanner-findings review panel: holds the rejected file, lists pattern-per-file, offers "Install anyway" / "Cancel"; success message reports how many findings were accepted | without this the override was API-only — an admin uploading a crypto bundle through the web form hit a 400 they could not act on. Also replaces a dead `data.warnings` read (never returned by the route) with the live `findings` field. | _(append new rows as you diverge)_