fix(cli): re-validate SSRF denylist on redirects + harden isPrivateUrl

.github/workflows/windows-render.yml

@@ -84,6 +84,7 @@ jobs:
     steps:
       - name: Disable Windows Defender real-time monitoring
         shell: pwsh
+        continue-on-error: true
         run: Set-MpPreference -DisableRealtimeMonitoring $true
 
       - name: Checkout
@@ -374,6 +375,7 @@ jobs:
     steps:
       - name: Disable Windows Defender real-time monitoring
         shell: pwsh
+        continue-on-error: true
         run: Set-MpPreference -DisableRealtimeMonitoring $true
 
       - name: Checkout

packages/cli/src/capture/assetDownloader.test.ts

@@ -0,0 +1,93 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { isPrivateUrl, safeFetch } from "./assetDownloader.js";
+
+describe("isPrivateUrl — SSRF denylist (security: F-003)", () => {
+  it("blocks loopback, private, and metadata IPv4", () => {
+    for (const u of [
+      "http://127.0.0.1/",
+      "http://10.0.0.5/",
+      "http://172.16.0.1/",
+      "http://192.168.1.1/",
+      "http://169.254.169.254/", // cloud metadata
+    ]) {
+      expect(isPrivateUrl(u), u).toBe(true);
+    }
+  });
+
+  it("blocks 0.0.0.0 and the 0.0.0.0/8 range", () => {
+    expect(isPrivateUrl("http://0.0.0.0/")).toBe(true);
+    expect(isPrivateUrl("http://0.1.2.3/")).toBe(true);
+  });
+
+  it("blocks IPv6 loopback, IPv4-mapped, ULA, and link-local", () => {
+    for (const u of [
+      "http://[::1]/",
+      "http://[::ffff:169.254.169.254]/", // IPv4-mapped metadata
+      "http://[fd00::1]/", // unique-local fc00::/7
+      "http://[fe80::1]/", // link-local fe80::/10
+    ]) {
+      expect(isPrivateUrl(u), u).toBe(true);
+    }
+  });
+
+  it("still blocks alternate IPv4 encodings (WHATWG canonicalization)", () => {
+    expect(isPrivateUrl("http://2130706433/")).toBe(true); // decimal 127.0.0.1
+    expect(isPrivateUrl("http://0x7f000001/")).toBe(true); // hex
+  });
+
+  it("blocks non-http(s) schemes and internal suffixes", () => {
+    expect(isPrivateUrl("file:///etc/passwd")).toBe(true);
+    expect(isPrivateUrl("http://db.internal/")).toBe(true);
+    expect(isPrivateUrl("http://svc.local/")).toBe(true);
+  });
+
+  it("allows ordinary public URLs", () => {
+    expect(isPrivateUrl("https://example.com/logo.png")).toBe(false);
+    expect(isPrivateUrl("https://cdn.jsdelivr.net/a.svg")).toBe(false);
+  });
+});
+
+describe("safeFetch — re-validates the denylist on every redirect hop (security: F-002)", () => {
+  afterEach(() => vi.unstubAllGlobals());
+
+  it("blocks a public URL that redirects to a private/metadata host", async () => {
+    const fetchMock = vi.fn(async (input: string, _init?: RequestInit) => {
+      if (input === "https://public.example/logo.png") {
+        return new Response(null, {
+          status: 302,
+          headers: { location: "http://169.254.169.254/latest/meta-data/" },
+        });
+      }
+      // The metadata host must NEVER be fetched.
+      throw new Error(`safeFetch followed a redirect to a private host: ${input}`);
+    });
+    vi.stubGlobal("fetch", fetchMock);
+
+    const res = await safeFetch("https://public.example/logo.png");
+    expect(res).toBeNull();
+    // First (public) hop fetched; the redirect target was rejected before fetch.
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock.mock.calls[0]?.[1]).toMatchObject({ redirect: "manual" });
+  });
+
+  it("follows a redirect to another public host and returns the final response", async () => {
+    const fetchMock = vi.fn(async (input: string, _init?: RequestInit) => {
+      if (input === "https://a.example/x")
+        return new Response(null, { status: 301, headers: { location: "https://b.example/y" } });
+      return new Response("ok", { status: 200 });
+    });
+    vi.stubGlobal("fetch", fetchMock);
+
+    const res = await safeFetch("https://a.example/x");
+    expect(res?.status).toBe(200);
+    expect(await res?.text()).toBe("ok");
+  });
+
+  it("returns null when the initial URL is private", async () => {
+    const fetchMock = vi.fn(async () => new Response("ok"));
+    vi.stubGlobal("fetch", fetchMock);
+    const res = await safeFetch("http://169.254.169.254/");
+    expect(res).toBeNull();
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+});

packages/cli/src/capture/assetDownloader.ts

@@ -57,7 +57,7 @@
       const localPath = `assets/${name}`;
       const buffer = await fetchBuffer(icon.href);
       if (buffer) {
         writeFileSync(join(outputDir, localPath), buffer);
         assets.push({ url: icon.href, localPath, type: "favicon" });
         break;
       }
@@ -156,7 +156,7 @@
       const localPath = `assets/og-image${ext}`;
       const buffer = await fetchBuffer(tokens.ogImage);
       if (buffer && buffer.length > 5000) {
         writeFileSync(join(outputDir, localPath), buffer);
         assets.push({ url: tokens.ogImage, localPath, type: "image" });
       }
     } catch {
@@ -241,7 +241,7 @@
 
       const buffer = await fetchBuffer(fontUrl);
       if (buffer) {
         writeFileSync(localPath, buffer);
         rewritten = rewritten.split(fontUrl).join(relativePath);
         familyCounts.set(family, familyCount + 1);
         count++;
@@ -254,40 +254,103 @@
   return rewritten;
 }
 
-/** Block requests to private/internal IP ranges to prevent SSRF */
+// Reserved/loopback/private IPv4 blocks as [firstOctet, secondOctetLo, secondOctetHi].
+const PRIVATE_V4_BLOCKS: ReadonlyArray<readonly [number, number, number]> = [
+  [0, 0, 255], // 0.0.0.0/8 (incl. 0.0.0.0, which routes to localhost)
+  [10, 0, 255], // 10.0.0.0/8
+  [127, 0, 255], // 127.0.0.0/8 loopback
+  [172, 16, 31], // 172.16.0.0/12
+  [192, 168, 168], // 192.168.0.0/16
+  [169, 254, 254], // 169.254.0.0/16 link-local (cloud metadata)
+];
+
+/** True for a dotted-quad IPv4 literal in a loopback/private/reserved range. */
+function isPrivateIpv4(host: string): boolean {
+  const octets = host.split(".").map(Number);
+  if (octets.length !== 4) return false;
+  const [a, b] = octets as [number, number, number, number];
+  return PRIVATE_V4_BLOCKS.some(([first, lo, hi]) => a === first && b >= lo && b <= hi);
+}
+
+/** True for a bracketed IPv6 hostname in a loopback/private/reserved range. */
+function isPrivateIpv6(bracketed: string): boolean {
+  const addr = bracketed.replace(/^\[|\]$/g, "").toLowerCase();
+  if (addr === "::1" || addr === "::") return true; // loopback / unspecified
+  const mapped = /^::ffff:(.+)$/.exec(addr); // IPv4-mapped ::ffff:a.b.c.d or ::ffff:hhhh:hhhh
+  if (mapped) {
+    const tail = mapped[1]!;
+    if (tail.includes(".")) return isPrivateIpv4(tail);
+    const hex = tail.split(":");
+    if (hex.length === 2) {
+      const n = ((parseInt(hex[0]!, 16) << 16) | parseInt(hex[1]!, 16)) >>> 0;
+      return isPrivateIpv4(
+        [(n >>> 24) & 255, (n >>> 16) & 255, (n >>> 8) & 255, n & 255].join("."),
+      );
+    }
+  }
+  if (/^f[cd]/.test(addr)) return true; // fc00::/7 unique-local
+  if (/^fe[89ab]/.test(addr)) return true; // fe80::/10 link-local
+  return false;
+}
+
+/**
+ * Block requests to private/internal hosts to prevent SSRF. WHATWG URL parsing
+ * canonicalizes alternate IPv4 encodings (decimal/octal/hex) to dotted-quad
+ * before we see them, so only dotted IPv4 and bracketed IPv6 literals reach the
+ * classifiers below.
+ */
 export function isPrivateUrl(url: string): boolean {
   try {
-    const { hostname } = new URL(url);
-    // Block cloud metadata, localhost, and private IP ranges
-    if (hostname === "localhost" || hostname === "127.0.0.1" || hostname === "[::1]") return true;
-    if (hostname === "169.254.169.254") return true; // AWS/GCP metadata
+    const u = new URL(url);
+    if (u.protocol !== "http:" && u.protocol !== "https:") return true; // no file:, etc.
+    const hostname = u.hostname;
+    if (hostname === "localhost") return true;
     if (hostname.endsWith(".internal") || hostname.endsWith(".local")) return true;
-    // IPv4 private ranges
-    const parts = hostname.split(".").map(Number);
-    if (parts.length === 4 && parts.every((p) => !isNaN(p))) {
-      if (parts[0] === 10) return true; // 10.0.0.0/8
-      if (parts[0] === 172 && parts[1]! >= 16 && parts[1]! <= 31) return true; // 172.16.0.0/12
-      if (parts[0] === 192 && parts[1] === 168) return true; // 192.168.0.0/16
-      if (parts[0] === 169 && parts[1] === 254) return true; // 169.254.0.0/16 (link-local)
-    }
-    // Block non-HTTP(S) schemes
-    const scheme = new URL(url).protocol;
-    if (scheme !== "http:" && scheme !== "https:") return true;
+    if (hostname.startsWith("[")) return isPrivateIpv6(hostname);
+    if (/^\d+(\.\d+){3}$/.test(hostname)) return isPrivateIpv4(hostname);
     return false;
   } catch {
     return true; // reject unparseable URLs
   }
 }
 
+/** Max redirect hops safeFetch will follow before giving up. */
+const MAX_FETCH_REDIRECTS = 5;
+
+/**
+ * fetch() that re-validates the SSRF denylist on EVERY redirect hop. A bare
+ * `redirect: "follow"` only checks the initial URL, so a public URL can 30x to
+ * an internal/metadata host. We resolve redirects manually and re-run
+ * isPrivateUrl on each Location. Returns null when blocked, on too many hops,
+ * or on network error.
+ */
+export async function safeFetch(url: string, init?: RequestInit): Promise<Response | null> {
+  let current = url;
+  for (let hop = 0; hop <= MAX_FETCH_REDIRECTS; hop++) {
+    if (isPrivateUrl(current)) return null;
+    const res = await fetch(current, { ...init, redirect: "manual" });
+    if (res.status >= 300 && res.status < 400) {
+      const loc = res.headers.get("location");
+      if (!loc) return res;
+      try {
+        current = new URL(loc, current).toString();
+      } catch {
+        return null; // malformed Location header
+      }
+      continue;
+    }
+    return res;
+  }
+  return null; // too many redirects
+}
+
 async function fetchBuffer(url: string): Promise<Buffer | null> {
   try {
-    if (isPrivateUrl(url)) return null;
-    const res = await fetch(url, {
+    const res = await safeFetch(url, {
       signal: AbortSignal.timeout(10000),
       headers: { "User-Agent": "HyperFrames/1.0" },
-      redirect: "follow",
     });
-    if (!res.ok) return null;
+    if (!res || !res.ok) return null;
     // Reject XML/HTML error pages disguised as 200 OK (common with S3/CloudFront)
     const ct = res.headers.get("content-type") || "";
     if (ct.includes("text/xml") || ct.includes("text/html") || ct.includes("application/xml")) {

packages/cli/src/capture/mediaCapture.ts

@@ -10,7 +10,7 @@
 import type { Browser, Page } from "puppeteer-core";
 import { mkdirSync, writeFileSync, readdirSync, readFileSync, statSync } from "node:fs";
 import { join } from "node:path";
-import { isPrivateUrl } from "./assetDownloader.js";
+import { safeFetch } from "./assetDownloader.js";
 
 /** Discovered Lottie item from network interception or DOM scan. */
 export interface DiscoveredLottie {
@@ -42,14 +42,12 @@
         // Already have the JSON data from network interception
         jsonData = JSON.stringify(lottieItem.data);
       } else if (lottieItem.url) {
-        // SSRF guard — don't fetch private/internal URLs
-        if (isPrivateUrl(lottieItem.url)) continue;
-        // Download the file
-        const res = await fetch(lottieItem.url, {
+        // SSRF guard — safeFetch re-checks the denylist on every redirect hop
+        const res = await safeFetch(lottieItem.url, {
           signal: AbortSignal.timeout(10000),
           headers: { "User-Agent": "HyperFrames/1.0" },
         });
-        if (!res.ok) continue;
+        if (!res || !res.ok) continue;
         const buf = Buffer.from(await res.arrayBuffer());
 
         if (lottieItem.url.endsWith(".lottie")) {
@@ -74,7 +72,7 @@
             savedHashes.add(hash);
             writeFileSync(join(lottieDir, `animation-${savedCount}.lottie`), buf);
             savedCount++;
             continue;
           }
         } else {
           // Plain JSON file
@@ -98,7 +96,7 @@
 
         writeFileSync(join(lottieDir, `animation-${savedCount}.json`), jsonData, "utf-8");
         savedCount++;
       }
     } catch {
       /* skip */
     }