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

## Summary - Adds `safeFetch`, a redirect-aware wrapper around `fetch` that re-runs the SSRF denylist on every hop before following a redirect. - Routes `fetchBuffer` and the Lottie media fetch through `safeFetch` so redirect chains can't bounce through a public URL to reach an internal or cloud-metadata host. - Hardens `isPrivateUrl` to also block `0.0.0.0` / `0.0.0.0/8`, IPv6 loopback (`::1`), IPv4-mapped (`::ffff:…`), unique-local (`fc00::/7`), and link-local (`fe80::/10`) ranges. ## Security **F-002 MED** — `fetchBuffer` followed redirects without re-checking the denylist on the destination. A `30x` redirect from an allowlisted public URL to `169.254.169.254` or an internal host would succeed, leaking the response to the caller (e.g. captured page assets written to local disk). **F-003 MED** — `isPrivateUrl` did not cover `0.0.0.0` (maps to localhost on most OSes), IPv6 loopback, or IPv6 private ranges. An asset URL using those addresses would bypass the denylist. Alternate IPv4 encodings (decimal/octal/hex) are already normalized to dotted-quad by WHATWG URL parsing and remain blocked. ## Test plan - [x] Unit tests cover redirect-chain blocking (redirect to metadata IP rejected) - [x] Unit tests cover new `isPrivateUrl` address forms (`0.0.0.0`, `::1`, `fc00::1`, `fe80::1`, `::ffff:192.168.1.1`) - [x] Existing fetch and asset-download tests pass
2026-09-03 23:03:09 +00:00 · 2026-06-05 17:01:00 -07:00
parent bacfb17538
commit 1f37920fe1
4 changed files with 182 additions and 26 deletions
@@ -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();
+  });
+});
@@ -254,40 +254,103 @@ export async function downloadAndRewriteFonts(css: string, outputDir: string): P
  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")) {
@@ -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 @@ export async function saveLottieAnimations(
        // 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")) {