From 4d563fa7523f0ef2e630d0da214b3b667bf7c982 Mon Sep 17 00:00:00 2001
From: Vance Ingalls <vance@heygen.com>
Date: Fri, 31 Jul 2026 15:56:38 -0700
Subject: [PATCH] fix(engine): guard the rounded frame rate and strict-parse
 rationals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two paths the previous guards still let through.

Rounding could recreate Infinity after the finite check. `raw * 100`
overflows for a finite-but-huge rate — "1e307", "1e307/1" — so `rounded`
became Infinity and passed the positivity check, reaching exactly the
`-r Infinity` failure the finite guard exists to prevent. The rounded
result is now checked too.

The rational operands still used parseFloat. The plain-number path
switched to Number() so trailing garbage fails the whole string, but the
numerator and denominator did not, so "60fps/1", "60/1fps" and
"30garbage/1garbage" returned valid rates while the contract says
malformed frame rates fail closed. Both operands are now parsed strictly,
and an empty operand ("/", "/1", "30/") is rejected rather than coerced.

Tests: 8 malformed inputs and 3 overflow cases in the direct table.
Reverting either fix fails 5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 packages/engine/src/utils/ffprobe.test.ts | 14 ++++++++++++--
 packages/engine/src/utils/ffprobe.ts      | 21 ++++++++++++++++-----
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/packages/engine/src/utils/ffprobe.test.ts b/packages/engine/src/utils/ffprobe.test.ts
index ab1cf211f..40997518b 100644
--- a/packages/engine/src/utils/ffprobe.test.ts
+++ b/packages/engine/src/utils/ffprobe.test.ts
@@ -618,8 +618,18 @@ describe("parseFrameRate", () => {
     expect(parseFrameRate(input)).toBe(0);
   });
 
-  // Fell through to a bare parseFloat that stops at trailing garbage.
-  it.each(["30/1/2", "60fps"])("returns 0 for malformed input %s", (input) => {
+  // Fell through to a bare parseFloat that stops at trailing garbage. The
+  // rational operands had the same defect after the plain path was fixed.
+  it.each(["30/1/2", "60fps", "60fps/1", "60/1fps", "30garbage/1garbage", "/", "/1", "30/"])(
+    "returns 0 for malformed input %s",
+    (input) => {
+      expect(parseFrameRate(input)).toBe(0);
+    },
+  );
+
+  // raw * 100 overflows for a finite-but-huge rate, so the rounded value was
+  // Infinity even though the pre-round guard passed.
+  it.each(["1e307", "1e307/1", "1e308/0.5"])("returns 0 when rounding overflows: %s", (input) => {
     expect(parseFrameRate(input)).toBe(0);
   });
 
diff --git a/packages/engine/src/utils/ffprobe.ts b/packages/engine/src/utils/ffprobe.ts
index 217f0807e..23e8619a7 100644
--- a/packages/engine/src/utils/ffprobe.ts
+++ b/packages/engine/src/utils/ffprobe.ts
@@ -356,21 +356,32 @@ export function parseFrameRate(frameRateStr: string | undefined): number {
   const parts = frameRateStr.split("/");
   if (parts.length > 2) return 0;
 
+  // Number(), never parseFloat — on BOTH the rational operands and the plain
+  // form. parseFloat stops at trailing garbage, so "60fps" parsed as 60 and,
+  // once the plain path was fixed but the operands were not, "60fps/1" and
+  // "30garbage/1garbage" still slipped through the rational branch.
+  const strict = (part: string | undefined): number =>
+    part === undefined || part.trim() === "" ? NaN : Number(part.trim());
+
   const raw =
     parts.length === 2
       ? (() => {
-          const num = parseFloat(parts[0] ?? "");
-          const den = parseFloat(parts[1] ?? "");
+          const num = strict(parts[0]);
+          const den = strict(parts[1]);
           if (!Number.isFinite(num) || !Number.isFinite(den) || den === 0) return NaN;
           return num / den;
         })()
-      : // Not parseFloat: it stops at trailing garbage, so "60fps" parsed as
-        // 60. Number() rejects the whole string.
-        Number(frameRateStr.trim());
+      : strict(frameRateStr);
 
   if (!Number.isFinite(raw) || raw <= 0) return 0;
 
+  // Checked AFTER rounding as well as before. `raw * 100` overflows for a
+  // finite-but-huge rate ("1e307", "1e307/1"), so `rounded` became Infinity
+  // and sailed past the positivity check — reaching exactly the `-r Infinity`
+  // failure the finite guard above exists to prevent.
   const rounded = Math.round(raw * 100) / 100;
+  if (!Number.isFinite(rounded)) return 0;
+
   // A real but very slow rate must not collapse to 0 and inherit the
   // caller's 30fps default.
   return rounded > 0 ? rounded : 0.01;