diff --git a/src/serve.ts b/src/serve.ts index 379d347..e29f5fe 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"; @@ -27,11 +27,16 @@ 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 = {}, ): 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 +72,23 @@ 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 = isWithinRoot(root, unresolvedCandidate); + let candidate: string | undefined; let file: Awaited> | undefined; if (inside && extension !== ".map") { try { - file = await stat(candidate); + const resolvedCandidate = await realpath(unresolvedCandidate); + if (isWithinRoot(root, resolvedCandidate)) { + candidate = resolvedCandidate; + 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(