diff --git a/extensions/diffs/src/browser.test.ts b/extensions/diffs/src/browser.test.ts index 9c3cf1365ea..c0b03d62cc0 100644 --- a/extensions/diffs/src/browser.test.ts +++ b/extensions/diffs/src/browser.test.ts @@ -1,8 +1,8 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import type { OpenClawConfig } from "openclaw/plugin-sdk/diffs"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createTempDiffRoot } from "./test-helpers.js"; const { launchMock } = vi.hoisted(() => ({ launchMock: vi.fn(), @@ -17,10 +17,11 @@ vi.mock("playwright-core", () => ({ describe("PlaywrightDiffScreenshotter", () => { let rootDir: string; let outputPath: string; + let cleanupRootDir: () => Promise; beforeEach(async () => { vi.useFakeTimers(); - rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-diffs-browser-")); + ({ rootDir, cleanup: cleanupRootDir } = await createTempDiffRoot("openclaw-diffs-browser-")); outputPath = path.join(rootDir, "preview.png"); launchMock.mockReset(); const browserModule = await import("./browser.js"); @@ -31,7 +32,7 @@ describe("PlaywrightDiffScreenshotter", () => { const browserModule = await import("./browser.js"); await browserModule.resetSharedBrowserStateForTests(); vi.useRealTimers(); - await fs.rm(rootDir, { recursive: true, force: true }); + await cleanupRootDir(); }); it("reuses the same browser across renders and closes it after the idle window", async () => { diff --git a/extensions/diffs/src/http.test.ts b/extensions/diffs/src/http.test.ts index 5e8c2927691..43216580379 100644 --- a/extensions/diffs/src/http.test.ts +++ b/extensions/diffs/src/http.test.ts @@ -1,32 +1,24 @@ -import fs from "node:fs/promises"; import type { IncomingMessage } from "node:http"; -import os from "node:os"; -import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { createMockServerResponse } from "../../../src/test-utils/mock-http-response.js"; import { createDiffsHttpHandler } from "./http.js"; import { DiffArtifactStore } from "./store.js"; +import { createDiffStoreHarness } from "./test-helpers.js"; describe("createDiffsHttpHandler", () => { - let rootDir: string; let store: DiffArtifactStore; + let cleanupRootDir: () => Promise; beforeEach(async () => { - rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-diffs-http-")); - store = new DiffArtifactStore({ rootDir }); + ({ store, cleanup: cleanupRootDir } = await createDiffStoreHarness("openclaw-diffs-http-")); }); afterEach(async () => { - await fs.rm(rootDir, { recursive: true, force: true }); + await cleanupRootDir(); }); it("serves a stored diff document", async () => { - const artifact = await store.createArtifact({ - html: "viewer", - title: "Demo", - inputKind: "before_after", - fileCount: 1, - }); + const artifact = await createViewerArtifact(store); const handler = createDiffsHttpHandler({ store }); const res = createMockServerResponse(); @@ -45,12 +37,7 @@ describe("createDiffsHttpHandler", () => { }); it("rejects invalid tokens", async () => { - const artifact = await store.createArtifact({ - html: "viewer", - title: "Demo", - inputKind: "before_after", - fileCount: 1, - }); + const artifact = await createViewerArtifact(store); const handler = createDiffsHttpHandler({ store }); const res = createMockServerResponse(); @@ -113,96 +100,52 @@ describe("createDiffsHttpHandler", () => { expect(String(res.body)).toContain("openclawDiffsReady"); }); - it("blocks non-loopback viewer access by default", async () => { - const artifact = await store.createArtifact({ - html: "viewer", - title: "Demo", - inputKind: "before_after", - fileCount: 1, - }); + it.each([ + { + name: "blocks non-loopback viewer access by default", + request: remoteReq, + allowRemoteViewer: false, + expectedStatusCode: 404, + }, + { + name: "blocks loopback requests that carry proxy forwarding headers by default", + request: localReq, + headers: { "x-forwarded-for": "203.0.113.10" }, + allowRemoteViewer: false, + expectedStatusCode: 404, + }, + { + name: "allows remote access when allowRemoteViewer is enabled", + request: remoteReq, + allowRemoteViewer: true, + expectedStatusCode: 200, + }, + { + name: "allows proxied loopback requests when allowRemoteViewer is enabled", + request: localReq, + headers: { "x-forwarded-for": "203.0.113.10" }, + allowRemoteViewer: true, + expectedStatusCode: 200, + }, + ])("$name", async ({ request, headers, allowRemoteViewer, expectedStatusCode }) => { + const artifact = await createViewerArtifact(store); - const handler = createDiffsHttpHandler({ store }); + const handler = createDiffsHttpHandler({ store, allowRemoteViewer }); const res = createMockServerResponse(); const handled = await handler( - remoteReq({ + request({ method: "GET", url: artifact.viewerPath, + headers, }), res, ); expect(handled).toBe(true); - expect(res.statusCode).toBe(404); - }); - - it("blocks loopback requests that carry proxy forwarding headers by default", async () => { - const artifact = await store.createArtifact({ - html: "viewer", - title: "Demo", - inputKind: "before_after", - fileCount: 1, - }); - - const handler = createDiffsHttpHandler({ store }); - const res = createMockServerResponse(); - const handled = await handler( - localReq({ - method: "GET", - url: artifact.viewerPath, - headers: { "x-forwarded-for": "203.0.113.10" }, - }), - res, - ); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(404); - }); - - it("allows remote access when allowRemoteViewer is enabled", async () => { - const artifact = await store.createArtifact({ - html: "viewer", - title: "Demo", - inputKind: "before_after", - fileCount: 1, - }); - - const handler = createDiffsHttpHandler({ store, allowRemoteViewer: true }); - const res = createMockServerResponse(); - const handled = await handler( - remoteReq({ - method: "GET", - url: artifact.viewerPath, - }), - res, - ); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); - expect(res.body).toBe("viewer"); - }); - - it("allows proxied loopback requests when allowRemoteViewer is enabled", async () => { - const artifact = await store.createArtifact({ - html: "viewer", - title: "Demo", - inputKind: "before_after", - fileCount: 1, - }); - - const handler = createDiffsHttpHandler({ store, allowRemoteViewer: true }); - const res = createMockServerResponse(); - const handled = await handler( - localReq({ - method: "GET", - url: artifact.viewerPath, - headers: { "x-forwarded-for": "203.0.113.10" }, - }), - res, - ); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); - expect(res.body).toBe("viewer"); + expect(res.statusCode).toBe(expectedStatusCode); + if (expectedStatusCode === 200) { + expect(res.body).toBe("viewer"); + } }); it("rate-limits repeated remote misses", async () => { @@ -232,6 +175,15 @@ describe("createDiffsHttpHandler", () => { }); }); +async function createViewerArtifact(store: DiffArtifactStore) { + return await store.createArtifact({ + html: "viewer", + title: "Demo", + inputKind: "before_after", + fileCount: 1, + }); +} + function localReq(input: { method: string; url: string; diff --git a/extensions/diffs/src/store.test.ts b/extensions/diffs/src/store.test.ts index d4e6aacd409..8039865b71b 100644 --- a/extensions/diffs/src/store.test.ts +++ b/extensions/diffs/src/store.test.ts @@ -1,21 +1,25 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { DiffArtifactStore } from "./store.js"; +import { createDiffStoreHarness } from "./test-helpers.js"; describe("DiffArtifactStore", () => { let rootDir: string; let store: DiffArtifactStore; + let cleanupRootDir: () => Promise; beforeEach(async () => { - rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-diffs-store-")); - store = new DiffArtifactStore({ rootDir }); + ({ + rootDir, + store, + cleanup: cleanupRootDir, + } = await createDiffStoreHarness("openclaw-diffs-store-")); }); afterEach(async () => { vi.useRealTimers(); - await fs.rm(rootDir, { recursive: true, force: true }); + await cleanupRootDir(); }); it("creates and retrieves an artifact", async () => { diff --git a/extensions/diffs/src/test-helpers.ts b/extensions/diffs/src/test-helpers.ts new file mode 100644 index 00000000000..f97ed9573e1 --- /dev/null +++ b/extensions/diffs/src/test-helpers.ts @@ -0,0 +1,30 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { DiffArtifactStore } from "./store.js"; + +export async function createTempDiffRoot(prefix: string): Promise<{ + rootDir: string; + cleanup: () => Promise; +}> { + const rootDir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + return { + rootDir, + cleanup: async () => { + await fs.rm(rootDir, { recursive: true, force: true }); + }, + }; +} + +export async function createDiffStoreHarness(prefix: string): Promise<{ + rootDir: string; + store: DiffArtifactStore; + cleanup: () => Promise; +}> { + const { rootDir, cleanup } = await createTempDiffRoot(prefix); + return { + rootDir, + store: new DiffArtifactStore({ rootDir }), + cleanup, + }; +} diff --git a/extensions/diffs/src/tool.test.ts b/extensions/diffs/src/tool.test.ts index 97ee6234148..416bdf8dc14 100644 --- a/extensions/diffs/src/tool.test.ts +++ b/extensions/diffs/src/tool.test.ts @@ -1,25 +1,24 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import type { OpenClawPluginApi } from "openclaw/plugin-sdk/diffs"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { DiffScreenshotter } from "./browser.js"; import { DEFAULT_DIFFS_TOOL_DEFAULTS } from "./config.js"; import { DiffArtifactStore } from "./store.js"; +import { createDiffStoreHarness } from "./test-helpers.js"; import { createDiffsTool } from "./tool.js"; import type { DiffRenderOptions } from "./types.js"; describe("diffs tool", () => { - let rootDir: string; let store: DiffArtifactStore; + let cleanupRootDir: () => Promise; beforeEach(async () => { - rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-diffs-tool-")); - store = new DiffArtifactStore({ rootDir }); + ({ store, cleanup: cleanupRootDir } = await createDiffStoreHarness("openclaw-diffs-tool-")); }); afterEach(async () => { - await fs.rm(rootDir, { recursive: true, force: true }); + await cleanupRootDir(); }); it("returns a viewer URL in view mode", async () => {