fix(plugins): scan all bundle scripts, allow audited scanner override
Two problems with the upload scanner, pulling in opposite directions.
It only scanned the entrypoint, so a bundle with `eval()` in a second
file passed outright — verified against a synthetic bundle whose
vendor/openpgp.js tripped three patterns while index.js stayed clean.
At the same time, a hard 400 on eval()/new Function()/innerHTML= makes
every crypto plugin uninstallable: minified openpgp.js and pkijs
legitimately contain all three. That blocks S/MIME and PGP entirely.
Scan every .js/.mjs in the bundle and return structured findings
({file, patterns[]}) plus canOverride, so the admin can see exactly what
tripped and where. An explicit overrideWarnings=true proceeds and writes
a plugin.install.scan_override audit entry recording which patterns in
which files were accepted — not merely that an override happened.
This route is already admin-authenticated, so the scan is defence in
depth against an accidental or compromised upload, not a trust boundary.
Treating it as the latter is what made crypto plugins uninstallable.
Also log the B-04 and B-01 divergences in vnc/VNC-CHANGES.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ae19ad888b
commit
d91db37b34
@@ -158,15 +158,47 @@ export async function POST(request: NextRequest) {
|
||||
}
|
||||
const code = await entryFile.async('string');
|
||||
|
||||
// Security: block plugins containing dangerous JS patterns
|
||||
const warnings: string[] = [];
|
||||
for (const { pattern, label } of SUSPICIOUS_JS_PATTERNS) {
|
||||
if (pattern.test(code)) warnings.push(`Contains ${label}`);
|
||||
pattern.lastIndex = 0;
|
||||
// Security: scan for dangerous JS patterns across EVERY script in the
|
||||
// bundle, not just the entrypoint - a second .js file was previously never
|
||||
// looked at.
|
||||
//
|
||||
// The result is a reviewable finding rather than an unconditional reject.
|
||||
// Minified crypto libraries (openpgp.js, pkijs) legitimately contain these
|
||||
// patterns, so a hard block makes S/MIME and PGP plugins uninstallable.
|
||||
// This route is already admin-authenticated, so the scan is defence in
|
||||
// depth against an accidental or compromised upload, not a trust boundary:
|
||||
// an admin may proceed with `overrideWarnings`, and the override is
|
||||
// recorded in the audit log with the exact findings.
|
||||
const findings: Array<{ file: string; patterns: string[] }> = [];
|
||||
for (const [filePath, entry] of Object.entries(zip.files)) {
|
||||
if (entry.dir) continue;
|
||||
const ext = filePath.slice(filePath.lastIndexOf('.')).toLowerCase();
|
||||
if (ext !== '.js' && ext !== '.mjs') continue;
|
||||
const source = filePath === root + (manifest.entrypoint as string)
|
||||
? code
|
||||
: await entry.async('string');
|
||||
const hits: string[] = [];
|
||||
for (const { pattern, label } of SUSPICIOUS_JS_PATTERNS) {
|
||||
if (pattern.test(source)) hits.push(label);
|
||||
pattern.lastIndex = 0;
|
||||
}
|
||||
if (hits.length > 0) {
|
||||
findings.push({ file: filePath.slice(root.length), patterns: hits });
|
||||
}
|
||||
}
|
||||
if (warnings.length > 0) {
|
||||
|
||||
const overrideWarnings = formData.get('overrideWarnings') === 'true';
|
||||
if (findings.length > 0 && !overrideWarnings) {
|
||||
const summary = findings
|
||||
.map(f => `${f.file}: ${f.patterns.join(', ')}`)
|
||||
.join('; ');
|
||||
return NextResponse.json(
|
||||
{ error: `Plugin rejected: ${warnings.join(', ')}. These patterns are not allowed for security reasons.` },
|
||||
{
|
||||
error: `Plugin rejected: ${summary}. Review the bundle; if these are expected `
|
||||
+ `(e.g. a vendored crypto library), re-upload with "overrideWarnings" to proceed.`,
|
||||
findings,
|
||||
canOverride: true,
|
||||
},
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
@@ -212,6 +244,16 @@ export async function POST(request: NextRequest) {
|
||||
await savePlugin(plugin, code);
|
||||
invalidateFrameOriginsCache();
|
||||
await auditLog('plugin.install', { id: plugin.id, name: plugin.name, version: plugin.version, frameOrigins: declaredFrameOrigins, httpOrigins: declaredHttpOrigins, apiPostPaths: declaredApiPostPaths }, ip);
|
||||
if (findings.length > 0) {
|
||||
// Record WHAT was waved through, not merely that an override happened -
|
||||
// otherwise the audit trail can't answer "which patterns did we accept?".
|
||||
await auditLog(
|
||||
'plugin.install.scan_override',
|
||||
{ id: plugin.id, version: plugin.version, findings },
|
||||
ip,
|
||||
);
|
||||
logger.warn('Plugin installed with scanner override', { id: plugin.id, findings });
|
||||
}
|
||||
|
||||
return NextResponse.json({ plugin });
|
||||
} catch (error) {
|
||||
|
||||
Reference in New Issue
Block a user