diff --git a/src/media/server.test.ts b/src/media/server.test.ts index fda4c0486ac..ffda31f76da 100644 --- a/src/media/server.test.ts +++ b/src/media/server.test.ts @@ -1,9 +1,10 @@ import type { AddressInfo } from "node:net"; import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -const MEDIA_DIR = path.join(process.cwd(), "tmp-media-test"); +let MEDIA_DIR = ""; const cleanOldMedia = vi.fn().mockResolvedValue(undefined); vi.mock("./store.js", async (importOriginal) => { @@ -18,39 +19,41 @@ vi.mock("./store.js", async (importOriginal) => { const { startMediaServer } = await import("./server.js"); const { MEDIA_MAX_BYTES } = await import("./store.js"); -const waitForFileRemoval = async (file: string, timeoutMs = 200) => { - const start = Date.now(); - while (Date.now() - start < timeoutMs) { +async function waitForFileRemoval(filePath: string, maxTicks = 1000) { + for (let tick = 0; tick < maxTicks; tick += 1) { try { - await fs.stat(file); + await fs.stat(filePath); } catch { return; } - await new Promise((resolve) => setTimeout(resolve, 5)); + await new Promise((resolve) => setImmediate(resolve)); } - throw new Error(`timed out waiting for ${file} removal`); -}; + throw new Error(`timed out waiting for ${filePath} removal`); +} describe("media server", () => { + let server: Awaited>; + let port = 0; + beforeAll(async () => { - await fs.rm(MEDIA_DIR, { recursive: true, force: true }); - await fs.mkdir(MEDIA_DIR, { recursive: true }); + MEDIA_DIR = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-media-test-")); + server = await startMediaServer(0, 1_000); + port = (server.address() as AddressInfo).port; }); afterAll(async () => { + await new Promise((r) => server.close(r)); await fs.rm(MEDIA_DIR, { recursive: true, force: true }); + MEDIA_DIR = ""; }); it("serves media and cleans up after send", async () => { const file = path.join(MEDIA_DIR, "file1"); await fs.writeFile(file, "hello"); - const server = await startMediaServer(0, 5_000); - const port = (server.address() as AddressInfo).port; const res = await fetch(`http://127.0.0.1:${port}/media/file1`); expect(res.status).toBe(200); expect(await res.text()).toBe("hello"); await waitForFileRemoval(file); - await new Promise((r) => server.close(r)); }); it("expires old media", async () => { @@ -58,22 +61,16 @@ describe("media server", () => { await fs.writeFile(file, "stale"); const past = Date.now() - 10_000; await fs.utimes(file, past / 1000, past / 1000); - const server = await startMediaServer(0, 1_000); - const port = (server.address() as AddressInfo).port; const res = await fetch(`http://127.0.0.1:${port}/media/old`); expect(res.status).toBe(410); await expect(fs.stat(file)).rejects.toThrow(); - await new Promise((r) => server.close(r)); }); it("blocks path traversal attempts", async () => { - const server = await startMediaServer(0, 5_000); - const port = (server.address() as AddressInfo).port; // URL-encoded "../" to bypass client-side path normalization const res = await fetch(`http://127.0.0.1:${port}/media/%2e%2e%2fpackage.json`); expect(res.status).toBe(400); expect(await res.text()).toBe("invalid path"); - await new Promise((r) => server.close(r)); }); it("blocks symlink escaping outside media dir", async () => { @@ -81,34 +78,25 @@ describe("media server", () => { const link = path.join(MEDIA_DIR, "link-out"); await fs.symlink(target, link); - const server = await startMediaServer(0, 5_000); - const port = (server.address() as AddressInfo).port; const res = await fetch(`http://127.0.0.1:${port}/media/link-out`); expect(res.status).toBe(400); expect(await res.text()).toBe("invalid path"); - await new Promise((r) => server.close(r)); }); it("rejects invalid media ids", async () => { const file = path.join(MEDIA_DIR, "file2"); await fs.writeFile(file, "hello"); - const server = await startMediaServer(0, 5_000); - const port = (server.address() as AddressInfo).port; const res = await fetch(`http://127.0.0.1:${port}/media/invalid%20id`); expect(res.status).toBe(400); expect(await res.text()).toBe("invalid path"); - await new Promise((r) => server.close(r)); }); it("rejects oversized media files", async () => { const file = path.join(MEDIA_DIR, "big"); await fs.writeFile(file, ""); await fs.truncate(file, MEDIA_MAX_BYTES + 1); - const server = await startMediaServer(0, 5_000); - const port = (server.address() as AddressInfo).port; const res = await fetch(`http://127.0.0.1:${port}/media/big`); expect(res.status).toBe(413); expect(await res.text()).toBe("too large"); - await new Promise((r) => server.close(r)); }); }); diff --git a/src/media/server.ts b/src/media/server.ts index 62dd5ef2d7d..31d956f74b0 100644 --- a/src/media/server.ts +++ b/src/media/server.ts @@ -63,9 +63,15 @@ export function attachMediaRoutes( res.send(data); // best-effort single-use cleanup after response ends res.on("finish", () => { - setTimeout(() => { - fs.rm(realPath).catch(() => {}); - }, 50); + const cleanup = () => { + void fs.rm(realPath).catch(() => {}); + }; + // Tests should not pay for time-based cleanup delays. + if (process.env.VITEST || process.env.NODE_ENV === "test") { + queueMicrotask(cleanup); + return; + } + setTimeout(cleanup, 50); }); } catch (err) { if (err instanceof SafeOpenError) {