Merge pull request #856 from heygen-com/05-15-fix_security_close_codeql_critical_bad-code-sanitization

fix(security): close CodeQL critical command-line-injection and bad-code-sanitization

Author: jrusso1020

packages/cli/src/commands/upgrade.ts

@@ -1,7 +1,7 @@
 import { defineCommand } from "citty";
 import type { Example } from "./_examples.js";
 import * as clack from "@clack/prompts";
-import { execSync } from "node:child_process";
+import { execFileSync } from "node:child_process";
 import { c } from "../ui/colors.js";
 
 export const examples: Example[] = [
@@ -67,13 +67,29 @@ export default defineCommand({
       }
     }
 
-    const installCmd = `npm install -g hyperframes@${result.latest}`;
+    // Reject anything that isn't a strict semver-shaped string before it reaches
+    // the install command. A poisoned npm registry response could otherwise put
+    // shell metacharacters into `result.latest`; rejecting up front means the
+    // version flows through execFile (and the displayed command) as an opaque
+    // token, not something the shell might re-parse.
+    const SAFE_VERSION = /^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/;
+    if (!SAFE_VERSION.test(result.latest)) {
+      clack.outro(c.dim("Refusing to install: unexpected version string from npm registry."));
+      process.exitCode = 1;
+      return;
+    }
+
+    const installArgs = ["install", "-g", `hyperframes@${result.latest}`];
+    const installCmd = `npm ${installArgs.join(" ")}`;
     if (autoYes) {
       console.log();
       console.log(`   ${c.dim("Running:")} ${c.accent(installCmd)}`);
       console.log();
       try {
-        execSync(installCmd, { stdio: "inherit" });
+        // execFileSync with shell:false — the version is now provably safe per
+        // SAFE_VERSION above, but keep the no-shell call so future edits can't
+        // regress the shell-injection surface area.
+        execFileSync("npm", installArgs, { stdio: "inherit", shell: false });
         clack.outro(c.success(`Upgraded to v${result.latest}`));
       } catch {
         clack.outro(c.dim("Install failed. Try running manually:"));

packages/engine/src/services/frameCapture.ts

@@ -349,6 +349,29 @@ async function pollPageExpression(
   return Boolean(await page.evaluate(expression));
 }
 
+async function pollVideosReady(
+  page: Page,
+  skipIds: readonly string[],
+  timeoutMs: number,
+  intervalMs: number = 100,
+): Promise<boolean> {
+  const check = async (): Promise<boolean> => {
+    return Boolean(
+      await page.evaluate((skipIdList: readonly string[]) => {
+        const skip = new Set(skipIdList);
+        const vids = Array.from(document.querySelectorAll("video")).filter((v) => !skip.has(v.id));
+        return vids.length === 0 || vids.every((v) => (v as HTMLVideoElement).readyState >= 2);
+      }, skipIds),
+    );
+  };
+  const deadline = Date.now() + timeoutMs;
+  while (Date.now() < deadline) {
+    if (await check()) return true;
+    await new Promise((resolve) => setTimeout(resolve, intervalMs));
+  }
+  return check();
+}
+
 async function applyVideoMetadataHints(
   page: Page,
   hints: readonly CaptureVideoMetadataHint[] | undefined,
@@ -490,10 +513,9 @@ export async function initializeSession(session: CaptureSession): Promise<void>
     // sources) whose frames come from ffmpeg out-of-band. videoMetadataHints
     // supply intrinsic dimensions for skipped videos whose layout depends on
     // aspect ratio, while Chromium may still fail to decode/load metadata.
-    const skipIdsLiteral = JSON.stringify(session.options.skipReadinessVideoIds ?? []);
-    const videosReady = await pollPageExpression(
+    const videosReady = await pollVideosReady(
       page,
-      `(() => { const skip = new Set(${skipIdsLiteral}); const vids = Array.from(document.querySelectorAll("video")).filter(v => !skip.has(v.id)); return vids.length === 0 || vids.every(v => v.readyState >= 2); })()`,
+      session.options.skipReadinessVideoIds ?? [],
       pageReadyTimeout,
     );
     if (!videosReady) {
@@ -596,16 +618,11 @@ export async function initializeSession(session: CaptureSession): Promise<void>
   await applyVideoMetadataHints(page, session.options.videoMetadataHints);
 
   // Same readyState contract as the screenshot path above (>= 2 / HAVE_CURRENT_DATA).
-  const beginframeSkipIdsLiteral = JSON.stringify(session.options.skipReadinessVideoIds ?? []);
-  const videoDeadline =
-    Date.now() + (session.config?.playerReadyTimeout ?? DEFAULT_CONFIG.playerReadyTimeout);
-  while (Date.now() < videoDeadline) {
-    const videosReady = await page.evaluate(
-      `(() => { const skip = new Set(${beginframeSkipIdsLiteral}); const vids = Array.from(document.querySelectorAll("video")).filter(v => !skip.has(v.id)); return vids.length === 0 || vids.every(v => v.readyState >= 2); })()`,
-    );
-    if (videosReady) break;
-    await new Promise((r) => setTimeout(r, 100));
-  }
+  await pollVideosReady(
+    page,
+    session.options.skipReadinessVideoIds ?? [],
+    session.config?.playerReadyTimeout ?? DEFAULT_CONFIG.playerReadyTimeout,
+  );
 
   // Font check (no rAF dependency — uses fonts.ready API directly)
   await page.evaluate(`document.fonts?.ready`);