fix: harden file serving

This commit is contained in:
Peter Steinberger
2026-01-26 20:05:03 +00:00
parent 8b56f0e68d
commit 5eee991913
5 changed files with 213 additions and 50 deletions
+32 -4
View File
@@ -7,12 +7,17 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
const MEDIA_DIR = path.join(process.cwd(), "tmp-media-test");
const cleanOldMedia = vi.fn().mockResolvedValue(undefined);
vi.mock("./store.js", () => ({
getMediaDir: () => MEDIA_DIR,
cleanOldMedia,
}));
vi.mock("./store.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("./store.js")>();
return {
...actual,
getMediaDir: () => MEDIA_DIR,
cleanOldMedia,
};
});
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();
@@ -84,4 +89,27 @@ describe("media server", () => {
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://localhost:${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://localhost:${port}/media/big`);
expect(res.status).toBe(413);
expect(await res.text()).toBe("too large");
await new Promise((r) => server.close(r));
});
});
+37 -15
View File
@@ -1,13 +1,23 @@
import fs from "node:fs/promises";
import type { Server } from "node:http";
import path from "node:path";
import express, { type Express } from "express";
import { danger } from "../globals.js";
import { defaultRuntime, type RuntimeEnv } from "../runtime.js";
import { SafeOpenError, openFileWithinRoot } from "../infra/fs-safe.js";
import { detectMime } from "./mime.js";
import { cleanOldMedia, getMediaDir } from "./store.js";
import { cleanOldMedia, getMediaDir, MEDIA_MAX_BYTES } from "./store.js";
const DEFAULT_TTL_MS = 2 * 60 * 1000;
const MAX_MEDIA_ID_CHARS = 200;
const MEDIA_ID_PATTERN = /^[\p{L}\p{N}._-]+$/u;
const MAX_MEDIA_BYTES = MEDIA_MAX_BYTES;
const isValidMediaId = (id: string) => {
if (!id) return false;
if (id.length > MAX_MEDIA_ID_CHARS) return false;
if (id === "." || id === "..") return false;
return MEDIA_ID_PATTERN.test(id);
};
export function attachMediaRoutes(
app: Express,
@@ -18,26 +28,28 @@ export function attachMediaRoutes(
app.get("/media/:id", async (req, res) => {
const id = req.params.id;
const mediaRoot = (await fs.realpath(mediaDir)) + path.sep;
const file = path.resolve(mediaRoot, id);
if (!isValidMediaId(id)) {
res.status(400).send("invalid path");
return;
}
try {
const lstat = await fs.lstat(file);
if (lstat.isSymbolicLink()) {
res.status(400).send("invalid path");
const { handle, realPath, stat } = await openFileWithinRoot({
rootDir: mediaDir,
relativePath: id,
});
if (stat.size > MAX_MEDIA_BYTES) {
await handle.close().catch(() => {});
res.status(413).send("too large");
return;
}
const realPath = await fs.realpath(file);
if (!realPath.startsWith(mediaRoot)) {
res.status(400).send("invalid path");
return;
}
const stat = await fs.stat(realPath);
if (Date.now() - stat.mtimeMs > ttlMs) {
await handle.close().catch(() => {});
await fs.rm(realPath).catch(() => {});
res.status(410).send("expired");
return;
}
const data = await fs.readFile(realPath);
const data = await handle.readFile();
await handle.close().catch(() => {});
const mime = await detectMime({ buffer: data, filePath: realPath });
if (mime) res.type(mime);
res.send(data);
@@ -47,7 +59,17 @@ export function attachMediaRoutes(
fs.rm(realPath).catch(() => {});
}, 50);
});
} catch {
} catch (err) {
if (err instanceof SafeOpenError) {
if (err.code === "invalid-path") {
res.status(400).send("invalid path");
return;
}
if (err.code === "not-found") {
res.status(404).send("not found");
return;
}
}
res.status(404).send("not found");
}
});
+11 -11
View File
@@ -10,7 +10,8 @@ import { resolvePinnedHostname } from "../infra/net/ssrf.js";
import { detectMime, extensionForMime } from "./mime.js";
const resolveMediaDir = () => path.join(resolveConfigDir(), "media");
const MAX_BYTES = 5 * 1024 * 1024; // 5MB default
export const MEDIA_MAX_BYTES = 5 * 1024 * 1024; // 5MB default
const MAX_BYTES = MEDIA_MAX_BYTES;
const DEFAULT_TTL_MS = 2 * 60 * 1000; // 2 minutes
/**
@@ -19,10 +20,9 @@ const DEFAULT_TTL_MS = 2 * 60 * 1000; // 2 minutes
* Keeps: alphanumeric, dots, hyphens, underscores, Unicode letters/numbers.
*/
function sanitizeFilename(name: string): string {
// Remove: < > : " / \ | ? * and control chars (U+0000-U+001F)
// oxlint-disable-next-line no-control-regex -- Intentionally matching control chars
const unsafe = /[<>:"/\\|?*\x00-\x1f]/g;
const sanitized = name.trim().replace(unsafe, "_").replace(/\s+/g, "_"); // Replace whitespace runs with underscore
const trimmed = name.trim();
if (!trimmed) return "";
const sanitized = trimmed.replace(/[^\p{L}\p{N}._-]+/gu, "_");
// Collapse multiple underscores, trim leading/trailing, limit length
return sanitized.replace(/_+/g, "_").replace(/^_|_$/g, "").slice(0, 60);
}
@@ -56,7 +56,7 @@ export function getMediaDir() {
export async function ensureMediaDir() {
const mediaDir = resolveMediaDir();
await fs.mkdir(mediaDir, { recursive: true });
await fs.mkdir(mediaDir, { recursive: true, mode: 0o700 });
return mediaDir;
}
@@ -123,7 +123,7 @@ async function downloadToFile(
let total = 0;
const sniffChunks: Buffer[] = [];
let sniffLen = 0;
const out = createWriteStream(dest);
const out = createWriteStream(dest, { mode: 0o600 });
res.on("data", (chunk) => {
total += chunk.length;
if (sniffLen < 16384) {
@@ -168,7 +168,7 @@ export async function saveMediaSource(
): Promise<SavedMedia> {
const baseDir = resolveMediaDir();
const dir = subdir ? path.join(baseDir, subdir) : baseDir;
await fs.mkdir(dir, { recursive: true });
await fs.mkdir(dir, { recursive: true, mode: 0o700 });
await cleanOldMedia();
const baseId = crypto.randomUUID();
if (looksLikeUrl(source)) {
@@ -198,7 +198,7 @@ export async function saveMediaSource(
const ext = extensionForMime(mime) ?? path.extname(source);
const id = ext ? `${baseId}${ext}` : baseId;
const dest = path.join(dir, id);
await fs.writeFile(dest, buffer);
await fs.writeFile(dest, buffer, { mode: 0o600 });
return { id, path: dest, size: stat.size, contentType: mime };
}
@@ -213,7 +213,7 @@ export async function saveMediaBuffer(
throw new Error(`Media exceeds ${(maxBytes / (1024 * 1024)).toFixed(0)}MB limit`);
}
const dir = path.join(resolveMediaDir(), subdir);
await fs.mkdir(dir, { recursive: true });
await fs.mkdir(dir, { recursive: true, mode: 0o700 });
const uuid = crypto.randomUUID();
const headerExt = extensionForMime(contentType?.split(";")[0]?.trim() ?? undefined);
const mime = await detectMime({ buffer, headerMime: contentType });
@@ -231,6 +231,6 @@ export async function saveMediaBuffer(
}
const dest = path.join(dir, id);
await fs.writeFile(dest, buffer);
await fs.writeFile(dest, buffer, { mode: 0o600 });
return { id, path: dest, size: buffer.byteLength, contentType: mime };
}