From 626e24426fc445f9bfb021f9a6f6142a159573d5 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sun, 26 Jul 2026 12:53:29 -0400 Subject: [PATCH 1/2] fix: contain static assets across symlinks Closes #4 --- src/serve.ts | 15 ++++++++++----- tests/node.test.ts | 27 ++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/serve.ts b/src/serve.ts index 379d347..16fe1ee 100644 --- a/src/serve.ts +++ b/src/serve.ts @@ -1,5 +1,5 @@ import { createReadStream } from "node:fs"; -import { stat } from "node:fs/promises"; +import { realpath, stat } from "node:fs/promises"; import { createServer } from "node:http"; import { extname, resolve, sep } from "node:path"; import type { ServerApp } from "@askrjs/server"; @@ -31,7 +31,7 @@ export async function serve( app: ServerApp & { close?: () => void | Promise }, options: ServeOptions = {}, ): Promise { - const root = options.assets ? resolve(options.assets.root) : undefined; + const root = options.assets ? await realpath(resolve(options.assets.root)) : undefined; const handlerOptions = { allowedHosts: [options.host ?? "127.0.0.1", "localhost"], }; @@ -67,17 +67,22 @@ export async function serve( const method = request.method ?? "GET"; if (root && (method === "GET" || method === "HEAD") && isAssetPath(pathname)) { const extension = extname(pathname).toLowerCase(); - const candidate = resolve(root, `.${pathname}`); - const inside = candidate.startsWith(`${root}${sep}`); + const unresolvedCandidate = resolve(root, `.${pathname}`); + const inside = unresolvedCandidate.startsWith(`${root}${sep}`); + let candidate: string | undefined; let file: Awaited> | undefined; if (inside && extension !== ".map") { try { + candidate = await realpath(unresolvedCandidate); + if (!candidate.startsWith(`${root}${sep}`)) candidate = undefined; + if (!candidate) throw new Error("Asset path escapes the configured root."); file = await stat(candidate); } catch { + candidate = undefined; file = undefined; } } - if (!file?.isFile()) { + if (!candidate || !file?.isFile()) { response .writeHead(404, { "content-type": "text/plain; charset=utf-8", diff --git a/tests/node.test.ts b/tests/node.test.ts index d78ea92..b124c1b 100644 --- a/tests/node.test.ts +++ b/tests/node.test.ts @@ -1,5 +1,5 @@ import { EventEmitter, once } from "node:events"; -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; import { get, request as nodeRequest, type ServerResponse } from "node:http"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -380,6 +380,31 @@ describe("serve", () => { } }); + it.runIf(process.platform !== "win32")( + "should not follow static asset symlinks outside the configured root", + async () => { + const root = await mkdtemp(join(tmpdir(), "askr-node-assets-")); + const outside = await mkdtemp(join(tmpdir(), "askr-node-outside-")); + await writeFile(join(outside, "secret.txt"), "secret"); + await symlink(outside, join(root, "escape")); + const served = await serve( + { fetch: async () => new Response("application") }, + { assets: { root }, signals: false }, + ); + try { + const response = await fetch(`${served.url}/escape/secret.txt`); + expect(response.status).toBe(404); + expect(await response.text()).toBe("Not Found"); + } finally { + await served.close(); + await Promise.all([ + rm(root, { recursive: true, force: true }), + rm(outside, { recursive: true, force: true }), + ]); + } + }, + ); + it("should close the application exactly once across concurrent shutdown", async () => { let closes = 0; const served = await serve( From d036c59dfc06fab39ebdcc70354b519cc84b9a35 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sun, 26 Jul 2026 12:59:08 -0400 Subject: [PATCH 2/2] fix: normalize asset root containment --- src/serve.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/serve.ts b/src/serve.ts index 16fe1ee..e29f5fe 100644 --- a/src/serve.ts +++ b/src/serve.ts @@ -27,6 +27,11 @@ function isAssetPath(pathname: string): boolean { return extname(pathname) !== ""; } +function isWithinRoot(root: string, candidate: string): boolean { + const prefix = root.endsWith(sep) ? root : `${root}${sep}`; + return candidate === root || candidate.startsWith(prefix); +} + export async function serve( app: ServerApp & { close?: () => void | Promise }, options: ServeOptions = {}, @@ -68,15 +73,16 @@ export async function serve( if (root && (method === "GET" || method === "HEAD") && isAssetPath(pathname)) { const extension = extname(pathname).toLowerCase(); const unresolvedCandidate = resolve(root, `.${pathname}`); - const inside = unresolvedCandidate.startsWith(`${root}${sep}`); + const inside = isWithinRoot(root, unresolvedCandidate); let candidate: string | undefined; let file: Awaited> | undefined; if (inside && extension !== ".map") { try { - candidate = await realpath(unresolvedCandidate); - if (!candidate.startsWith(`${root}${sep}`)) candidate = undefined; - if (!candidate) throw new Error("Asset path escapes the configured root."); - file = await stat(candidate); + const resolvedCandidate = await realpath(unresolvedCandidate); + if (isWithinRoot(root, resolvedCandidate)) { + candidate = resolvedCandidate; + file = await stat(candidate); + } } catch { candidate = undefined; file = undefined;