test: narrow media fs-safe test seams

This commit is contained in:
Peter Steinberger 2026-04-03 14:22:05 +01:00
parent 3b727d2433
commit ba7297ff21
No known key found for this signature in database
6 changed files with 73 additions and 30 deletions

View File

@ -8,28 +8,23 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vites
const mocks = vi.hoisted(() => ({
readFileWithinRoot: vi.fn(),
cleanOldMedia: vi.fn().mockResolvedValue(undefined),
isSafeOpenError: vi.fn(
(error: unknown) => typeof error === "object" && error !== null && "code" in error,
),
}));
let mediaDir = "";
vi.mock("../infra/fs-safe.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../infra/fs-safe.js")>();
vi.mock("./server.runtime.js", () => {
return {
...actual,
MEDIA_MAX_BYTES: 5 * 1024 * 1024,
readFileWithinRoot: mocks.readFileWithinRoot,
};
});
vi.mock("./store.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("./store.js")>();
return {
...actual,
isSafeOpenError: mocks.isSafeOpenError,
getMediaDir: () => mediaDir,
cleanOldMedia: mocks.cleanOldMedia,
};
});
let SafeOpenError: typeof import("../infra/fs-safe.js").SafeOpenError;
let startMediaServer: typeof import("./server.js").startMediaServer;
let realFetch: typeof import("undici").fetch;
@ -48,7 +43,6 @@ describe("media server outside-workspace mapping", () => {
vi.useRealTimers();
vi.doUnmock("undici");
const require = createRequire(import.meta.url);
({ SafeOpenError } = await import("../infra/fs-safe.js"));
({ startMediaServer } = await import("./server.js"));
({ fetch: realFetch } = require("undici") as typeof import("undici"));
mediaDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-media-outside-workspace-"));
@ -90,9 +84,10 @@ describe("media server outside-workspace mapping", () => {
if (listenBlocked) {
return;
}
mocks.readFileWithinRoot.mockRejectedValueOnce(
new SafeOpenError("outside-workspace", "file is outside workspace root"),
);
mocks.readFileWithinRoot.mockRejectedValueOnce({
code: "outside-workspace",
message: "file is outside workspace root",
});
await expectOutsideWorkspaceServerResponse(`http://127.0.0.1:${port}/media/ok-id`);
});

View File

@ -0,0 +1,27 @@
import { readFileWithinRoot as readFileWithinRootImpl, SafeOpenError } from "../infra/fs-safe.js";
import {
cleanOldMedia as cleanOldMediaImpl,
getMediaDir as getMediaDirImpl,
MEDIA_MAX_BYTES,
} from "./store.js";
export type SafeOpenLikeError = {
code:
| "invalid-path"
| "not-found"
| "outside-workspace"
| "symlink"
| "not-file"
| "path-mismatch"
| "too-large";
message: string;
};
export const readFileWithinRoot = readFileWithinRootImpl;
export const cleanOldMedia = cleanOldMediaImpl;
export const getMediaDir = getMediaDirImpl;
export { MEDIA_MAX_BYTES };
export function isSafeOpenError(error: unknown): error is SafeOpenLikeError {
return error instanceof SafeOpenError;
}

View File

@ -2,10 +2,15 @@ import fs from "node:fs/promises";
import type { Server } from "node:http";
import express, { type Express } from "express";
import { danger } from "../globals.js";
import { SafeOpenError, readFileWithinRoot } from "../infra/fs-safe.js";
import { defaultRuntime, type RuntimeEnv } from "../runtime.js";
import { detectMime } from "./mime.js";
import { cleanOldMedia, getMediaDir, MEDIA_MAX_BYTES } from "./store.js";
import {
cleanOldMedia,
getMediaDir,
isSafeOpenError,
MEDIA_MAX_BYTES,
readFileWithinRoot,
} from "./server.runtime.js";
const DEFAULT_TTL_MS = 2 * 60 * 1000;
const MAX_MEDIA_ID_CHARS = 200;
@ -72,7 +77,7 @@ export function attachMediaRoutes(
setTimeout(cleanup, 50);
});
} catch (err) {
if (err instanceof SafeOpenError) {
if (isSafeOpenError(err)) {
if (err.code === "outside-workspace") {
res.status(400).send("file is outside workspace root");
return;

View File

@ -5,21 +5,21 @@ import { createTempHomeEnv, type TempHomeEnv } from "../test-utils/temp-home.js"
const mocks = vi.hoisted(() => ({
readLocalFileSafely: vi.fn(),
isSafeOpenError: vi.fn(
(error: unknown) => typeof error === "object" && error !== null && "code" in error,
),
}));
vi.mock("../infra/fs-safe.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../infra/fs-safe.js")>();
vi.mock("./store.runtime.js", () => {
return {
...actual,
readLocalFileSafely: mocks.readLocalFileSafely,
isSafeOpenError: mocks.isSafeOpenError,
};
});
type StoreModule = typeof import("./store.js");
type FsSafeModule = typeof import("../infra/fs-safe.js");
let saveMediaSource: StoreModule["saveMediaSource"];
let SafeOpenError: FsSafeModule["SafeOpenError"];
async function expectOutsideWorkspaceStoreFailure(sourcePath: string) {
await expect(saveMediaSource(sourcePath)).rejects.toMatchObject({
@ -34,7 +34,6 @@ describe("media store outside-workspace mapping", () => {
beforeAll(async () => {
({ saveMediaSource } = await import("./store.js"));
({ SafeOpenError } = await import("../infra/fs-safe.js"));
tempHome = await createTempHomeEnv("openclaw-media-store-test-home-");
home = tempHome.home;
});
@ -46,9 +45,10 @@ describe("media store outside-workspace mapping", () => {
it("maps outside-workspace reads to a descriptive invalid-path error", async () => {
const sourcePath = path.join(home, "outside-media.txt");
await fs.writeFile(sourcePath, "hello");
mocks.readLocalFileSafely.mockRejectedValueOnce(
new SafeOpenError("outside-workspace", "file is outside workspace root"),
);
mocks.readLocalFileSafely.mockRejectedValueOnce({
code: "outside-workspace",
message: "file is outside workspace root",
});
await expectOutsideWorkspaceStoreFailure(sourcePath);
});

View File

@ -0,0 +1,16 @@
import {
readLocalFileSafely as readLocalFileSafelyImpl,
SafeOpenError,
type SafeOpenErrorCode,
} from "../infra/fs-safe.js";
export type SafeOpenLikeError = {
code: SafeOpenErrorCode;
message: string;
};
export const readLocalFileSafely = readLocalFileSafelyImpl;
export function isSafeOpenError(error: unknown): error is SafeOpenLikeError {
return error instanceof SafeOpenError;
}

View File

@ -5,11 +5,11 @@ import { request as httpRequest } from "node:http";
import { request as httpsRequest } from "node:https";
import path from "node:path";
import { pipeline } from "node:stream/promises";
import { SafeOpenError, readLocalFileSafely } from "../infra/fs-safe.js";
import { retainSafeHeadersForCrossOriginRedirect } from "../infra/net/redirect-headers.js";
import { resolvePinnedHostname } from "../infra/net/ssrf.js";
import { resolveConfigDir } from "../utils.js";
import { detectMime, extensionForMime } from "./mime.js";
import { isSafeOpenError, readLocalFileSafely, type SafeOpenLikeError } from "./store.runtime.js";
const resolveMediaDir = () => path.join(resolveConfigDir(), "media");
export const MEDIA_MAX_BYTES = 5 * 1024 * 1024; // 5MB default
@ -322,7 +322,7 @@ export class SaveMediaSourceError extends Error {
}
}
function toSaveMediaSourceError(err: SafeOpenError): SaveMediaSourceError {
function toSaveMediaSourceError(err: SafeOpenLikeError): SaveMediaSourceError {
switch (err.code) {
case "symlink":
return new SaveMediaSourceError("invalid-path", "Media path must not be a symlink", {
@ -385,7 +385,7 @@ export async function saveMediaSource(
await writeSavedMediaBuffer({ dir, id, buffer });
return buildSavedMediaResult({ dir, id, size: stat.size, contentType: mime });
} catch (err) {
if (err instanceof SafeOpenError) {
if (isSafeOpenError(err)) {
throw toSaveMediaSourceError(err);
}
throw err;