From d8558dc1742970c32ce9ac0bd4a419078d8ff579 Mon Sep 17 00:00:00 2001 From: DeryFerd Date: Sun, 20 Sep 2026 18:18:13 +0700 Subject: [PATCH] fix(server): contain static file requests within their served roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static asset handlers joined the raw request path onto their root directory without checking where it landed. Hono decodes percent escapes while routing, so an encoded backslash (%5c) arrives as a real separator on Windows and dot segments arrive already collapsed — "/assets/..%5c..%5csecret" and "/%2e%2e%2fsecret" could read files above the studio bundle root (embedded Studio server) and above compiledDir/projectDir (engine render file server). Browsers never request those shapes, but anything that can reach the bound port can: raw local clients, LAN peers when the preview binds 0.0.0.0, and a shared-machine process hitting the render server. Both handlers now resolve the request path first and refuse anything that collapses outside its root, using the same lexical containment the preview asset route already applies — bundle/project assets behind symlinked directories keep being served, only dot-segment escapes are rejected. Route-level pathSafety tests pin the behavior in both packages, including the Windows backslash shape and the symlinked-asset cases from the descriptor-pinning regressions. Validated: new containment suites fail on the prior handlers (200 + marker file content served from outside the root) and pass with the guards; cli server (140), engine services (1024+, matching the main baseline incl. pre-existing browserManager failures), typecheck, oxlint, oxfmt and the full workspace build all clean. --- .../server/studioServer.pathSafety.test.ts | 75 +++++++++++++++++++ packages/cli/src/server/studioServer.ts | 7 ++ .../services/fileServer.pathSafety.test.ts | 63 ++++++++++++++++ packages/engine/src/services/fileServer.ts | 19 ++++- 4 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/server/studioServer.pathSafety.test.ts create mode 100644 packages/engine/src/services/fileServer.pathSafety.test.ts diff --git a/packages/cli/src/server/studioServer.pathSafety.test.ts b/packages/cli/src/server/studioServer.pathSafety.test.ts new file mode 100644 index 0000000000..017ef53d3c --- /dev/null +++ b/packages/cli/src/server/studioServer.pathSafety.test.ts @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { tmpdir } from "node:os"; +import { createStudioServer } from "./studioServer.js"; + +/** + * The static SPA handlers resolve the raw request path against the bundle + * directory. Hono decodes percent escapes when routing, so an encoded + * separator or dot segment in the request must never walk out of the bundle + * root — browsers would not send those shapes, but anything that can reach + * the bound port can. These tests pin containment at the handler. + */ + +const hooks = vi.hoisted(() => ({ studioDir: "" })); + +// The bundle directory is resolved from __dirname at server construction, so +// point that one `resolve(<...>/server, "studio")` call at a temp tree. +vi.mock("node:path", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + resolve: (...parts: string[]) => + hooks.studioDir && parts.length === 2 && parts[0]?.endsWith("server") && parts[1] === "studio" + ? hooks.studioDir + : actual.resolve(...parts), + }; +}); + +let root: string; +let server: Awaited>; + +beforeEach(async () => { + root = fs.mkdtempSync(path.join(tmpdir(), "hf-studio-containment-")); + hooks.studioDir = path.join(root, "studio"); + const projectDir = path.join(root, "project"); + fs.mkdirSync(projectDir); + fs.mkdirSync(path.join(hooks.studioDir, "assets"), { recursive: true }); + fs.writeFileSync(path.join(root, "studio-marker.txt"), "STUDIO-MARKER"); + fs.writeFileSync( + path.join(hooks.studioDir, "index.html"), + "Studio", + ); + server = await createStudioServer({ projectDir }); +}); + +afterEach(() => { + server.watcher.close(); + fs.rmSync(root, { recursive: true, force: true }); + hooks.studioDir = ""; +}); + +describe("static file path containment", () => { + // A percent-encoded backslash decodes to a real separator on Windows, so + // two encoded hops walk from the bundle root to the temp root above it. + it.each(["/assets/..%5c..%5cstudio-marker.txt", "/icons/..%5c..%5cstudio-marker.txt"])( + "rejects encoded traversal %s", + async (requestPath) => { + const response = await server.app.request(requestPath); + expect(response.status).toBe(404); + expect(await response.text()).not.toContain("STUDIO-MARKER"); + }, + ); + + it("still serves the SPA shell and bundle assets", async () => { + const shell = await server.app.request("/"); + expect(shell.status).toBe(200); + expect(await shell.text()).toContain("Studio"); + + fs.writeFileSync(path.join(hooks.studioDir, "assets", "app.js"), "export {};"); + const asset = await server.app.request("/assets/app.js"); + expect(asset.status).toBe(200); + expect(await asset.text()).toContain("export {};"); + }); +}); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 1dd6ecfac6..d8577ab6d1 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -49,6 +49,7 @@ import { resolveAutoProxy } from "../utils/projectConfig.js"; import { getElementScreenshotClip } from "@hyperframes/studio-server/screenshot-clip"; import type { ScreenshotClip } from "@hyperframes/studio-server/screenshot-clip"; import type { RenderJob } from "@hyperframes/producer"; +import { isWithinProjectRoot } from "@hyperframes/parsers/asset-resolution"; import { seekCompositionTimeline } from "../capture/captureCompositionFrame.js"; import { assertWebGpuAdapterAvailable, @@ -881,6 +882,12 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { // Studio SPA static files const serveStudioStaticFile = (cacheControl: string) => (c: Context) => { const filePath = resolve(studioDir, c.req.path.slice(1)); + // Percent escapes can decode into separators and dot segments before this + // resolve, so a hostile request can name files above the bundle + // directory. Containment stays lexical on purpose: bundle assets may sit + // behind symlinked directories (same tradeoff as the preview asset + // route), but dot segments must never collapse outside the bundle root. + if (!isWithinProjectRoot(studioDir, filePath)) return c.text("not found", 404); const content = readBundleFile(filePath); if (content === null) return c.text("not found", 404); return new Response(content, { diff --git a/packages/engine/src/services/fileServer.pathSafety.test.ts b/packages/engine/src/services/fileServer.pathSafety.test.ts new file mode 100644 index 0000000000..4df08092d3 --- /dev/null +++ b/packages/engine/src/services/fileServer.pathSafety.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { tmpdir } from "node:os"; +import { createFileServer, type FileServerHandle } from "./fileServer.js"; + +/** + * The server derives each request path from the URL and joins it onto + * compiledDir/projectDir. Hono decodes percent escapes when routing, so an + * encoded separator or dot segment in the request must never walk out of the + * served roots — Chrome never requests those shapes, but anything that can + * reach the port (LAN bind, other local processes) can. On Windows an encoded + * backslash (%5c) decodes to a real separator, so `..%5c..%5c` escapes even + * though `..%2f` is normalized away by join(). These tests pin containment + * at the handler. + */ +describe("fileServer path containment", () => { + let root: string; + let projectDir: string; + let outsideFile: string; + let server: FileServerHandle; + + beforeEach(async () => { + root = fs.mkdtempSync(path.join(tmpdir(), "hf-fileserver-containment-")); + projectDir = path.join(root, "project"); + fs.mkdirSync(projectDir); + fs.writeFileSync(path.join(projectDir, "index.html"), ""); + outsideFile = path.join(root, "outside-secret.txt"); + fs.writeFileSync(outsideFile, "OUTSIDE"); + server = await createFileServer({ + headScripts: [], + bodyScripts: [], + projectDir, + }); + }); + + afterEach(async () => { + await server.close(); + fs.rmSync(root, { recursive: true, force: true }); + }); + + const escapeAttempts = [ + // Encoded separators: %2f collapses with a leading dot-segment only via a + // nested hop, %5c is a live separator on Windows. + "/..%2foutside-secret.txt", + "/%2e%2e%2foutside-secret.txt", + "/nested%2f..%2f..%2foutside-secret.txt", + "/..%5coutside-secret.txt", + "/..%5c..%5coutside-secret.txt", + ]; + + it.each(escapeAttempts)("rejects encoded traversal %s", async (requestPath) => { + const response = await fetch(`${server.url}${requestPath}`); + expect(response.status).toBe(404); + expect(await response.text()).not.toContain("OUTSIDE"); + }); + + it("still serves files inside the project", async () => { + const response = await fetch(`${server.url}/index.html`); + expect(response.status).toBe(200); + expect(await response.text()).toContain(""); + }); +}); diff --git a/packages/engine/src/services/fileServer.ts b/packages/engine/src/services/fileServer.ts index 04b4a62fe2..9e11419b95 100644 --- a/packages/engine/src/services/fileServer.ts +++ b/packages/engine/src/services/fileServer.ts @@ -11,6 +11,7 @@ import { serve } from "@hono/node-server"; import { readFileSync, openSync, fstatSync, closeSync, statSync, constants } from "node:fs"; import { join, extname } from "node:path"; import { injectScriptsIntoHtml } from "@hyperframes/core/compiler"; +import { isWithinProjectRoot } from "@hyperframes/parsers/asset-resolution"; const MIME_TYPES: Record = { ".html": "text/html; charset=utf-8", @@ -98,10 +99,22 @@ export function createFileServer(options: FileServerOptions): Promise