fix: stabilize fetch-urls tests for CI and extend format-matrix timeout

Replace mock HTTP server + vi.mock approach with vi.stubGlobal('fetch') using a public IP (1.2.3.4) that passes real SSRF validation. This eliminates both the fragile vi.mock (broken under V8 coverage) and the localhost network dependency (unreliable in CI). Revert the SSRF_ALLOW_PRIVATE env var that broke ssrf unit tests. Extend timeout for exotic format error resilience tests to 120s to accommodate slow JXL + Image enhancement combination in CI.
2026-08-03 07:46:42 +02:00 · 2026-05-12 03:58:42 +08:00
parent c9c09a96bc
commit 706f78b309
4 changed files with 112 additions and 202 deletions
@@ -33,10 +33,6 @@ function isPrivateIPv6(ip: string): boolean {
 }

 async function resolveAndCheck(hostname: string): Promise<void> {
-  // Allow tests to bypass private-IP checks so a local mock HTTP server can be
-  // used without fragile vi.mock() overrides that break under V8 coverage.
-  if (process.env.SSRF_ALLOW_PRIVATE === "1") return;
-
  const bare = hostname.replace(/^\[|]$/g, "");
  if (isIP(bare)) {
    if (isPrivateIPv4(bare) || isPrivateIPv6(bare)) {
@@ -1,17 +1,15 @@
 /**
 * Integration tests for the fetch-urls route.
 *
- * Spins up a local HTTP server to serve test fixtures, and mocks the SSRF
- * validation to allow localhost connections during tests.
+ * Uses a public IP (1.2.3.4) in test URLs so the real safeFetch SSRF
+ * validation passes without any module mocking. The global `fetch` is
+ * stubbed via vi.stubGlobal to return canned responses, avoiding real
+ * network calls entirely.
 */

 import { readFileSync } from "node:fs";
-import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http";
 import { join } from "node:path";
-import { afterAll, beforeAll, describe, expect, it } from "vitest";
-
-// SSRF private-IP checks are bypassed via the SSRF_ALLOW_PRIVATE=1 env var
-// set in vitest.config.ts, so the real safeFetch works against localhost.
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";

 import { buildTestApp, loginAsAdmin, type TestApp } from "./test-server.js";

@@ -22,80 +20,62 @@ const TIFF = readFileSync(join(FIXTURES, "formats", "sample.tiff"));
 let testApp: TestApp;
 let app: TestApp["app"];
 let adminToken: string;
-let mockServer: Server;
-let mockPort: number;

-function startMockServer(): Promise<{ server: Server; port: number }> {
-  return new Promise((resolve) => {
-    const server = createServer((req: IncomingMessage, res: ServerResponse) => {
-      const url = req.url ?? "";
+// Public IP that passes SSRF private-IP validation (not 10.x, 127.x, 192.168.x, etc.)
+const MOCK_ORIGIN = "http://1.2.3.4:9999";

-      if (url === "/photo.jpg") {
-        res.writeHead(200, { "Content-Type": "image/jpeg" });
-        res.end(JPG);
-        return;
-      }
+function mockResponse(
+  body: Buffer | string | null,
+  init: { status?: number; headers?: Record<string, string> } = {},
+): Response {
+  const status = init.status ?? 200;
+  const buf = body ? (typeof body === "string" ? Buffer.from(body) : body) : Buffer.alloc(0);
+  return new Response(body === null || buf.length === 0 ? null : buf, {
+    status,
+    statusText: status === 200 ? "OK" : status === 404 ? "Not Found" : "Error",
+    headers: new Headers(init.headers),
+  });
+}

-      if (url === "/not-image.txt") {
-        res.writeHead(200, { "Content-Type": "text/plain" });
-        res.end("This is not an image");
-        return;
-      }
+function createMockFetch() {
+  return vi.fn(async (url: string | URL | Request, _init?: RequestInit): Promise<Response> => {
+    const urlStr = typeof url === "string" ? url : url instanceof URL ? url.href : url.url;
+    const path = new URL(urlStr).pathname;

-      if (url === "/redirect") {
-        res.writeHead(302, { Location: "/photo.jpg" });
-        res.end();
-        return;
-      }
-
-      if (url === "/missing.jpg") {
-        res.writeHead(404);
-        res.end("Not Found");
-        return;
-      }
-
-      if (url === "/photo.tiff") {
-        res.writeHead(200, { "Content-Type": "image/tiff" });
-        res.end(TIFF);
-        return;
-      }
-
-      if (url === "/empty") {
-        res.writeHead(200, { "Content-Type": "image/jpeg" });
-        res.end();
-        return;
-      }
-
-      if (url === "/server-error") {
-        res.writeHead(500, { "Content-Type": "text/plain" });
-        res.end("Internal Server Error");
-        return;
-      }
-
-      if (url === "/slow-close") {
-        // Return a valid response with no body stream at all
-        res.writeHead(200, { "Content-Type": "image/jpeg", "Content-Length": "0" });
-        res.end();
-        return;
-      }
-
-      res.writeHead(404);
-      res.end("Not Found");
-    });
-
-    server.listen(0, "127.0.0.1", () => {
-      const addr = server.address();
-      const port = typeof addr === "object" && addr ? addr.port : 0;
-      resolve({ server, port });
-    });
+    switch (path) {
+      case "/photo.jpg":
+        return mockResponse(JPG, { headers: { "Content-Type": "image/jpeg" } });
+      case "/not-image.txt":
+        return mockResponse("This is not an image", {
+          headers: { "Content-Type": "text/plain" },
+        });
+      case "/redirect":
+        return mockResponse(null, {
+          status: 302,
+          headers: { Location: `${new URL(urlStr).origin}/photo.jpg` },
+        });
+      case "/missing.jpg":
+        return mockResponse("Not Found", { status: 404 });
+      case "/photo.tiff":
+        return mockResponse(TIFF, { headers: { "Content-Type": "image/tiff" } });
+      case "/empty":
+        return mockResponse(null, { headers: { "Content-Type": "image/jpeg" } });
+      case "/server-error":
+        return mockResponse("Internal Server Error", {
+          status: 500,
+          headers: { "Content-Type": "text/plain" },
+        });
+      case "/slow-close":
+        return mockResponse(null, {
+          headers: { "Content-Type": "image/jpeg", "Content-Length": "0" },
+        });
+      default:
+        return mockResponse("Not Found", { status: 404 });
+    }
  });
 }

 beforeAll(async () => {
-  const mock = await startMockServer();
-  mockServer = mock.server;
-  mockPort = mock.port;
-
  testApp = await buildTestApp();
  app = testApp.app;
  adminToken = await loginAsAdmin(app);
@@ -103,20 +83,26 @@ beforeAll(async () => {

 afterAll(async () => {
  await testApp.cleanup();
-  await new Promise<void>((resolve) => mockServer.close(() => resolve()));
 }, 10_000);

+let mockFetch: ReturnType<typeof createMockFetch>;
+
+beforeEach(() => {
+  mockFetch = createMockFetch();
+  vi.stubGlobal("fetch", mockFetch);
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
 describe("POST /api/v1/fetch-urls", () => {
  it("fetches a valid image URL and returns metadata + download URL", async () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/photo.jpg`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/photo.jpg`] },
    });

    expect(res.statusCode).toBe(200);
@@ -125,70 +111,56 @@ describe("POST /api/v1/fetch-urls", () => {

    const result = body.results[0];
    expect(result.success).toBe(true);
-    expect(result.url).toBe(`http://127.0.0.1:${mockPort}/photo.jpg`);
+    expect(result.url).toBe(`${MOCK_ORIGIN}/photo.jpg`);
    expect(result.filename).toBe("photo.jpg");
    expect(result.contentType).toBe("image/jpeg");
    expect(result.size).toBeGreaterThan(0);
    expect(result.width).toBe(100);
    expect(result.height).toBe(100);
    expect(result.downloadUrl).toMatch(/^\/api\/v1\/download\/.+\/photo\.jpg$/);
-    expect(result.previewUrl).toBeNull(); // JPEG is browser-previewable
+    expect(result.previewUrl).toBeNull();
  });

  it("returns failure for a 404 URL", async () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/missing.jpg`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/missing.jpg`] },
    });

    expect(res.statusCode).toBe(200);
    const body = JSON.parse(res.body);
    expect(body.results).toHaveLength(1);
-
-    const result = body.results[0];
-    expect(result.success).toBe(false);
-    expect(result.error).toContain("404");
+    expect(body.results[0].success).toBe(false);
+    expect(body.results[0].error).toContain("404");
  });

  it("returns failure for non-image content", async () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/not-image.txt`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/not-image.txt`] },
    });

    expect(res.statusCode).toBe(200);
    const body = JSON.parse(res.body);
    expect(body.results).toHaveLength(1);
-
-    const result = body.results[0];
-    expect(result.success).toBe(false);
-    expect(result.error).toBeTruthy();
+    expect(body.results[0].success).toBe(false);
+    expect(body.results[0].error).toBeTruthy();
  });

  it("handles mixed batch with successes and failures", async () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
      payload: {
        urls: [
-          `http://127.0.0.1:${mockPort}/photo.jpg`,
-          `http://127.0.0.1:${mockPort}/missing.jpg`,
-          `http://127.0.0.1:${mockPort}/not-image.txt`,
+          `${MOCK_ORIGIN}/photo.jpg`,
+          `${MOCK_ORIGIN}/missing.jpg`,
+          `${MOCK_ORIGIN}/not-image.txt`,
        ],
      },
    });
@@ -197,13 +169,10 @@ describe("POST /api/v1/fetch-urls", () => {
    const body = JSON.parse(res.body);
    expect(body.results).toHaveLength(3);

-    // Results preserve order
    expect(body.results[0].success).toBe(true);
    expect(body.results[0].filename).toBe("photo.jpg");
-
    expect(body.results[1].success).toBe(false);
    expect(body.results[1].error).toContain("404");
-
    expect(body.results[2].success).toBe(false);
  });

@@ -211,45 +180,33 @@ describe("POST /api/v1/fetch-urls", () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [] },
    });

    expect(res.statusCode).toBe(400);
-    const body = JSON.parse(res.body);
-    expect(body.error).toBeTruthy();
+    expect(JSON.parse(res.body).error).toBeTruthy();
  });

  it("returns 400 for more than 50 URLs", async () => {
-    const urls = Array.from({ length: 51 }, (_, i) => `http://example.com/img${i}.jpg`);
+    const urls = Array.from({ length: 51 }, (_, i) => `http://1.2.3.4/img${i}.jpg`);
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
      payload: { urls },
    });

    expect(res.statusCode).toBe(400);
-    const body = JSON.parse(res.body);
-    expect(body.error).toBeTruthy();
+    expect(JSON.parse(res.body).error).toBeTruthy();
  });

  it("follows redirects to fetch the final image", async () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/redirect`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/redirect`] },
    });

    expect(res.statusCode).toBe(200);
@@ -263,34 +220,25 @@ describe("POST /api/v1/fetch-urls", () => {
  });

  it("download URL serves the actual image", async () => {
-    // First, fetch the URL to get a downloadUrl
    const fetchRes = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/photo.jpg`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/photo.jpg`] },
    });

    const body = JSON.parse(fetchRes.body);
    const downloadUrl = body.results[0].downloadUrl;
    expect(downloadUrl).toBeTruthy();

-    // Now download the file
    const downloadRes = await app.inject({
      method: "GET",
      url: downloadUrl,
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
    });

    expect(downloadRes.statusCode).toBe(200);
    expect(downloadRes.headers["content-type"]).toBe("image/jpeg");
-    // The downloaded buffer should match the original fixture
    expect(downloadRes.rawPayload.length).toBe(JPG.length);
  });

@@ -298,12 +246,8 @@ describe("POST /api/v1/fetch-urls", () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/photo.jpg`, `http://127.0.0.1:${mockPort}/photo.jpg`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/photo.jpg`, `${MOCK_ORIGIN}/photo.jpg`] },
    });

    expect(res.statusCode).toBe(200);
@@ -313,16 +257,13 @@ describe("POST /api/v1/fetch-urls", () => {
    expect(body.results[0].success).toBe(true);
    expect(body.results[1].success).toBe(true);

-    // Filenames must differ so one does not overwrite the other
    const names = [body.results[0].filename, body.results[1].filename];
    expect(new Set(names).size).toBe(2);
    expect(names).toContain("photo.jpg");
    expect(names).toContain("photo_1.jpg");

-    // Download URLs must also differ
    expect(body.results[0].downloadUrl).not.toBe(body.results[1].downloadUrl);

-    // Both download URLs should serve valid content
    for (const result of body.results) {
      const dl = await app.inject({
        method: "GET",
@@ -338,29 +279,20 @@ describe("POST /api/v1/fetch-urls", () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: ["not-a-valid-url"],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: ["not-a-valid-url"] },
    });

    expect(res.statusCode).toBe(400);
-    const body = JSON.parse(res.body);
-    expect(body.error).toBeTruthy();
+    expect(JSON.parse(res.body).error).toBeTruthy();
  });

  it("generates a preview for non-browser-previewable formats", async () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/photo.tiff`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/photo.tiff`] },
    });

    expect(res.statusCode).toBe(200);
@@ -374,13 +306,10 @@ describe("POST /api/v1/fetch-urls", () => {
    expect(result.previewUrl).toContain("preview-");
    expect(result.previewUrl).toContain(".webp");

-    // Preview URL should serve a valid webp image
    const previewRes = await app.inject({
      method: "GET",
      url: result.previewUrl,
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
    });
    expect(previewRes.statusCode).toBe(200);
  });
@@ -389,12 +318,8 @@ describe("POST /api/v1/fetch-urls", () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/empty`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/empty`] },
    });

    expect(res.statusCode).toBe(200);
@@ -408,12 +333,8 @@ describe("POST /api/v1/fetch-urls", () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/server-error`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/server-error`] },
    });

    expect(res.statusCode).toBe(200);
@@ -424,17 +345,13 @@ describe("POST /api/v1/fetch-urls", () => {
  });

  it("returns failure when fetch throws a network error", async () => {
-    // Port 1 is almost guaranteed to refuse connections, triggering the outer
-    // catch block (lines 275-278 in fetch-urls.ts).
+    mockFetch.mockRejectedValueOnce(new TypeError("fetch failed"));
+
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: ["http://127.0.0.1:1/unreachable.jpg"],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/unreachable.jpg`] },
    });

    expect(res.statusCode).toBe(200);
@@ -448,12 +365,8 @@ describe("POST /api/v1/fetch-urls", () => {
    const res = await app.inject({
      method: "POST",
      url: "/api/v1/fetch-urls",
-      headers: {
-        authorization: `Bearer ${adminToken}`,
-      },
-      payload: {
-        urls: [`http://127.0.0.1:${mockPort}/slow-close`],
-      },
+      headers: { authorization: `Bearer ${adminToken}` },
+      payload: { urls: [`${MOCK_ORIGIN}/slow-close`] },
    });

    expect(res.statusCode).toBe(200);
@@ -735,7 +735,9 @@ describe("Exotic format error resilience", () => {

  for (const fmt of EXOTIC_FORMATS) {
    for (const tool of PROCESSING_TOOLS) {
-      it(`${fmt.name} + ${tool.label}: returns JSON error (no crash)`, async () => {
+      it(`${fmt.name} + ${tool.label}: returns JSON error (no crash)`, {
+        timeout: 120_000,
+      }, async () => {
        const fixturePath = join(FORMATS_DIR, fmt.file);
        if (!existsSync(fixturePath)) return;

@@ -60,7 +60,6 @@ export default defineConfig({
      CONCURRENT_JOBS: "3",
      FILE_MAX_AGE_HOURS: "1",
      CLEANUP_INTERVAL_MINUTES: "60",
-      SSRF_ALLOW_PRIVATE: "1",
    },
    coverage: {
      provider: "v8",