From 706f78b309391139d57c5ad0f926d0e8859df528 Mon Sep 17 00:00:00 2001 From: SnapOtter Date: Tue, 12 May 2026 03:58:42 +0800 Subject: [PATCH] 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. --- apps/api/src/lib/ssrf.ts | 4 - tests/integration/fetch-urls.test.ts | 305 +++++++++--------------- tests/integration/format-matrix.test.ts | 4 +- vitest.config.ts | 1 - 4 files changed, 112 insertions(+), 202 deletions(-) diff --git a/apps/api/src/lib/ssrf.ts b/apps/api/src/lib/ssrf.ts index f5ade1c5..e49a5c93 100644 --- a/apps/api/src/lib/ssrf.ts +++ b/apps/api/src/lib/ssrf.ts @@ -33,10 +33,6 @@ function isPrivateIPv6(ip: string): boolean { } async function resolveAndCheck(hostname: string): Promise { - // 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)) { diff --git a/tests/integration/fetch-urls.test.ts b/tests/integration/fetch-urls.test.ts index 1a416685..692f6bb4 100644 --- a/tests/integration/fetch-urls.test.ts +++ b/tests/integration/fetch-urls.test.ts @@ -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 } = {}, +): 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 => { + 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((resolve) => mockServer.close(() => resolve())); }, 10_000); +let mockFetch: ReturnType; + +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); diff --git a/tests/integration/format-matrix.test.ts b/tests/integration/format-matrix.test.ts index d4b40ac0..31406dd1 100644 --- a/tests/integration/format-matrix.test.ts +++ b/tests/integration/format-matrix.test.ts @@ -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; diff --git a/vitest.config.ts b/vitest.config.ts index 58acabec..5c88c66c 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -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",