From 7a5dc22a042cd78753b371cdb56f5ef52353f863 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Mon, 25 May 2026 11:40:39 -0500 Subject: [PATCH] fix(web-bundles): security hardening + strict bundle validation Two issues raised by coderabbit on the latest commit: 1. Shell injection surface: execSync was building the zip command with a template literal that interpolated bundle.slug from JSON. Even with our controlled inputs, a slug with shell metacharacters would break quoting. Switched to execFileSync with an argument array (no shell) and added a strict ^[a-z0-9][a-z0-9-]*$ slug regex enforced before any FS or zip call. 2. Missing bundle directories were [SKIP]-warned but the script still printed the release command, allowing an incomplete release to ship cleanly. Now treated as fatal: any missing or invalid slug blocks the printed gh command and exits non-zero with the offending slugs listed. --- tools/bundle-web-bundles.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/bundle-web-bundles.js b/tools/bundle-web-bundles.js index 899dc203d..470052311 100644 --- a/tools/bundle-web-bundles.js +++ b/tools/bundle-web-bundles.js @@ -13,12 +13,13 @@ const fs = require('node:fs'); const path = require('node:path'); -const { execSync } = require('node:child_process'); +const { execSync, execFileSync } = require('node:child_process'); const REPO_ROOT = path.resolve(__dirname, '..'); const BUNDLES_DIR = path.join(REPO_ROOT, 'web-bundles'); const DIST_DIR = path.join(REPO_ROOT, 'dist', 'web-bundles'); const MANIFEST = path.join(BUNDLES_DIR, 'bundles.json'); +const SLUG_RE = /^[a-z0-9][a-z0-9-]*$/; function fail(msg) { console.error(`[ERROR] ${msg}`); @@ -62,14 +63,18 @@ function main() { console.log(`Packaging ${manifest.bundles.length} bundles for release ${releaseTag}\n`); const zipped = []; + const missing = []; + const invalid = []; for (const bundle of manifest.bundles) { - if (!bundle.slug) { - console.warn(` [SKIP] bundle entry missing slug — ${JSON.stringify(bundle).slice(0, 80)}`); + if (!bundle.slug || !SLUG_RE.test(bundle.slug)) { + invalid.push(bundle.slug || '(no slug)'); + console.error(` [INVALID] slug must match ${SLUG_RE} — got: ${bundle.slug}`); continue; } const src = path.join(BUNDLES_DIR, bundle.slug); if (!fs.existsSync(src)) { - console.warn(` [SKIP] ${bundle.slug} — directory not found`); + missing.push(bundle.slug); + console.error(` [MISSING] ${bundle.slug} — directory not found`); continue; } @@ -77,7 +82,7 @@ function main() { if (fs.existsSync(out)) fs.unlinkSync(out); try { - execSync(`zip -r -X -q "${out}" "${bundle.slug}" -x "*.DS_Store"`, { + execFileSync('zip', ['-r', '-X', '-q', out, bundle.slug, '-x', '*.DS_Store'], { cwd: BUNDLES_DIR, stdio: 'inherit', }); @@ -90,8 +95,14 @@ function main() { zipped.push(bundle.slug); } + if (invalid.length > 0) { + fail(`Refusing to publish: ${invalid.length} bundle(s) have invalid slugs: ${invalid.join(', ')}`); + } + if (missing.length > 0) { + fail(`Refusing to publish an incomplete release: missing directories for ${missing.join(', ')}`); + } if (zipped.length === 0) { - fail('No bundles were packaged. Check bundles.json slugs against web-bundles/ subdirectories.'); + fail('No bundles were packaged. Check bundles.json against web-bundles/ subdirectories.'); } console.log(`\nWrote ${zipped.length} bundles to ${path.relative(REPO_ROOT, DIST_DIR)}/`);