fix: bound capture helper failures

.github/workflows/ci.yml

@@ -17,9 +17,9 @@ jobs:
       # skip Playwright's browser download to keep CI fast.
       PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1'
     steps:
-      - uses: actions/checkout@v6
+      - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
 
-      - uses: actions/setup-node@v6
+      - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
         with:
           node-version: 22
           cache: 'npm'

src/launch.js

@@ -59,9 +59,21 @@ async function launchWithExtension({ extensionDir, viewport, recordVideoDir, rec
   const context = await chromium.launchPersistentContext(userDataDir, launchOpts);
 
   // Wait for the service worker so we can read the extension ID off its URL.
-  let [sw] = context.serviceWorkers();
-  if (!sw) sw = await context.waitForEvent('serviceworker', { timeout: 15_000 });
-  const extensionId = sw.url().split('/')[2];
+  // A content-script-only extension (no MV3 worker) or a slow worker makes this
+  // reject — clean up the open context and temp profile before rethrowing so a
+  // failed launch never leaks a live Chromium process or a temp dir.
+  let extensionId;
+  try {
+    let [sw] = context.serviceWorkers();
+    if (!sw) sw = await context.waitForEvent('serviceworker', { timeout: 15_000 });
+    extensionId = sw.url().split('/')[2];
+  } catch (err) {
+    await context.close().catch(() => {});
+    if (userDataDir && fs.existsSync(userDataDir)) {
+      fs.rmSync(userDataDir, { recursive: true, force: true });
+    }
+    throw err;
+  }
 
   return { context, extensionId, userDataDir };
 }

src/serve.js

@@ -48,25 +48,50 @@ function serveDirectory(dir, opts = {}) {
   const root = path.resolve(dir);
   return new Promise((resolve) => {
     const server = http.createServer((req, res) => {
-      const urlPath = decodeURIComponent((req.url || '/').split('?')[0]);
-      // Path-traversal sanitizer (CodeQL js/path-injection): normalize, then
-      // strip any leading `../` segments so the join can't escape `root`.
-      const rel = path.normalize(urlPath === '/' ? '/index.html' : urlPath).replace(/^(\.\.(\/|\\|$))+/, '');
-      let filePath = path.join(root, rel);
-      if (!fs.existsSync(filePath) || fs.statSync(filePath).isDirectory()) {
-        filePath = opts.fallback ? path.join(root, opts.fallback) : null;
-      }
-      if (!filePath || !fs.existsSync(filePath)) {
-        res.writeHead(404, { 'Content-Type': 'text/plain' });
-        res.end('Not found');
+      // A synchronous throw inside this handler would become an uncaughtException
+      // that kills the whole capture process, so guard the entire body. Malformed
+      // percent-encoding (decodeURIComponent), a TOCTOU race between existsSync and
+      // read, or a permission/EACCES error must all yield an HTTP status, not a crash.
+      let urlPath;
+      try {
+        urlPath = decodeURIComponent((req.url || '/').split('?')[0]);
+      } catch {
+        res.writeHead(400, { 'Content-Type': 'text/plain' });
+        res.end('Bad request');
         return;
       }
-      const ext = path.extname(filePath).toLowerCase();
-      res.writeHead(200, {
-        'Content-Type': CONTENT_TYPES[ext] || 'application/octet-stream',
-        'Content-Security-Policy': FIXTURE_CSP,
-      });
-      res.end(fs.readFileSync(filePath));
+      try {
+        // Path-traversal sanitizer (CodeQL js/path-injection): normalize, then
+        // strip any leading `../` segments so the join can't escape `root`.
+        const rel = path.normalize(urlPath === '/' ? '/index.html' : urlPath).replace(/^(\.\.(\/|\\|$))+/, '');
+        let filePath = path.join(root, rel);
+        if (!fs.existsSync(filePath) || fs.statSync(filePath).isDirectory()) {
+          filePath = opts.fallback ? path.join(root, opts.fallback) : null;
+        }
+        if (!filePath || !fs.existsSync(filePath)) {
+          res.writeHead(404, { 'Content-Type': 'text/plain' });
+          res.end('Not found');
+          return;
+        }
+        const ext = path.extname(filePath).toLowerCase();
+        res.writeHead(200, {
+          'Content-Type': CONTENT_TYPES[ext] || 'application/octet-stream',
+          'Content-Security-Policy': FIXTURE_CSP,
+        });
+        res.end(fs.readFileSync(filePath));
+      } catch (err) {
+        // readFileSync can throw after writeHead(200) already flushed headers, so
+        // only write a new status when none has been sent — otherwise abort the socket.
+        if (!res.headersSent) {
+          res.writeHead(err && err.code === 'ENOENT' ? 404 : 500, { 'Content-Type': 'text/plain' });
+          res.end('Server error');
+        } else {
+          res.destroy();
+        }
+      }
+    });
+    server.on('clientError', (err, socket) => {
+      if (socket.writable) socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
     });
     server.listen(0, '127.0.0.1', () => {
       const { port } = server.address();

src/video.js

@@ -20,6 +20,37 @@ const INSTALL_HINT =
   '`apt-get install -y ffmpeg`; GitHub ubuntu runners already have it) or set ' +
   "SHOTKIT_FFMPEG to a binary. Playwright's bundled ffmpeg cannot encode H.264.";
 
+// Bound each ffmpeg invocation so a wedged encoder (bad input, odd codec path,
+// a binary that hangs on stdin) can't block the whole capture run forever.
+// Override for legitimately long encodes via SHOTKIT_FFMPEG_TIMEOUT_MS.
+function ffmpegTimeoutMs(env = process.env) {
+  const raw = Number(env.SHOTKIT_FFMPEG_TIMEOUT_MS);
+  return Number.isFinite(raw) && raw > 0 ? raw : 120_000;
+}
+
+/**
+ * Run ffmpeg with a bounded timeout, turning a timeout into a clear error
+ * (execFileSync otherwise throws a raw ETIMEDOUT/signal error on `.killed`).
+ */
+function runFfmpeg(bin, args, timeoutMs) {
+  try {
+    execFileSync(bin, args, {
+      stdio: ['ignore', 'ignore', 'inherit'],
+      timeout: timeoutMs,
+      killSignal: 'SIGKILL',
+    });
+  } catch (err) {
+    if (err && (err.killed || err.code === 'ETIMEDOUT')) {
+      throw new Error(
+        `shotkit: ffmpeg timed out after ${Math.round(timeoutMs / 1000)}s — the encoder may be ` +
+          'wedged. Raise SHOTKIT_FFMPEG_TIMEOUT_MS for legitimately long encodes.',
+        { cause: err },
+      );
+    }
+    throw err;
+  }
+}
+
 /**
  * Locate a usable ffmpeg. Returns the binary path/name, or null.
  * @param {NodeJS.ProcessEnv} [env]
@@ -28,7 +59,14 @@ function findFfmpeg(env = process.env) {
   for (const bin of [env.SHOTKIT_FFMPEG, 'ffmpeg']) {
     if (!bin) continue;
     try {
-      const r = spawnSync(bin, ['-version'], { stdio: ['ignore', 'pipe', 'ignore'], encoding: 'utf8', env });
+      // `-version` returns near-instantly; a small timeout guards against a
+      // resolved binary that wedges (e.g. hangs waiting on stdin).
+      const r = spawnSync(bin, ['-version'], {
+        stdio: ['ignore', 'pipe', 'ignore'],
+        encoding: 'utf8',
+        env,
+        timeout: 10_000,
+      });
       if (r.status === 0 && /ffmpeg version/i.test(r.stdout || '')) return bin;
     } catch (_e) {
       /* try the next candidate */
@@ -132,14 +170,13 @@ function postProcessDemo({ webmPath, mp4, trim, crop, zoom, thumbnail, log, env
   const bin = findFfmpeg(env);
   if (!bin) throw new Error(`demo mp4/trim/crop/zoom/thumbnail requested but ${INSTALL_HINT}`);
 
+  const timeoutMs = ffmpegTimeoutMs(env);
   const produced = [];
   let finalVideoPath = webmPath;
   if (mp4 || crop || zoom) {
     const crf = typeof mp4 === 'object' && mp4.crf != null ? mp4.crf : undefined;
     const mp4Path = webmPath.replace(/\.webm$/, '.mp4');
-    execFileSync(bin, buildFfmpegArgs({ input: webmPath, output: mp4Path, trim, crf, crop, zoom }), {
-      stdio: ['ignore', 'ignore', 'inherit'],
-    });
+    runFfmpeg(bin, buildFfmpegArgs({ input: webmPath, output: mp4Path, trim, crf, crop, zoom }), timeoutMs);
     produced.push(mp4Path);
     finalVideoPath = mp4Path;
     const notes = ['H.264'];
@@ -150,9 +187,7 @@ function postProcessDemo({ webmPath, mp4, trim, crop, zoom, thumbnail, log, env
   } else if (trim) {
     // Trim-only: stream-copy to a sibling temp file, then swap in place.
     const tmp = `${webmPath}.trim.webm`;
-    execFileSync(bin, buildFfmpegArgs({ input: webmPath, output: tmp, trim, copy: true }), {
-      stdio: ['ignore', 'ignore', 'inherit'],
-    });
+    runFfmpeg(bin, buildFfmpegArgs({ input: webmPath, output: tmp, trim, copy: true }), timeoutMs);
     fs.renameSync(tmp, webmPath);
     log(`✓ ${path.basename(webmPath)} trimmed in place`);
   }
@@ -163,9 +198,7 @@ function postProcessDemo({ webmPath, mp4, trim, crop, zoom, thumbnail, log, env
     const thumbPath = typeof thumbnail === 'object' && thumbnail.name
       ? path.join(path.dirname(webmPath), thumbnail.name)
       : webmPath.replace(/\.webm$/, '-thumbnail.png');
-    execFileSync(bin, buildThumbnailArgs({ input: finalVideoPath, output: thumbPath, at }), {
-      stdio: ['ignore', 'ignore', 'inherit'],
-    });
+    runFfmpeg(bin, buildThumbnailArgs({ input: finalVideoPath, output: thumbPath, at }), timeoutMs);
     // ffmpeg exits 0 even when `at` seeks past the end of a (trimmed) clip,
     // writing no file. Only record the thumbnail when it was actually produced,
     // so the manifest never references a phantom asset.