From 43f5469b7827253743b1d853bdb7b32081dd2257 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 16 Jun 2026 00:52:19 +0000 Subject: [PATCH] fix(core): route renders/file serving through resolveWithinProject chokepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/projects/:id/renders/file/*` route joined attacker-controlled wildcard input straight onto rendersDir with a bare join() + readFileSync and no containment check — the only project-scoped filesystem route that skipped the resolveWithinProject chokepoint every sibling route uses. Literal/encoded `../` traversal is collapsed upstream by Hono's WHATWG URL normalization (verified empirically), so the plain LFI is not reachable over HTTP. But a symlink living inside rendersDir and pointing outside it was still followed and served verbatim (verified: leaked an external secret, 200 OK). Routing through resolveWithinProject canonicalizes with realpath before serving, closing the symlink escape and making the route's safety independent of the URL layer's normalization behavior. Adds regression coverage: serves an in-dir file, rejects an escaping symlink (403), and still serves a symlink that stays inside rendersDir. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/src/studio-api/routes/render.test.ts | 74 +++++++++++++++++++ packages/core/src/studio-api/routes/render.ts | 8 +- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/packages/core/src/studio-api/routes/render.test.ts b/packages/core/src/studio-api/routes/render.test.ts index 0d92e49c90..ef299cd780 100644 --- a/packages/core/src/studio-api/routes/render.test.ts +++ b/packages/core/src/studio-api/routes/render.test.ts @@ -354,3 +354,77 @@ describe("POST /projects/:id/render — composition path safety", () => { expect(spy).toHaveBeenCalledOnce(); }); }); + +describe("GET /projects/:id/renders/file/* — path safety", () => { + const tmpDirs: string[] = []; + + function buildApp(): { app: Hono; rendersDir: string } { + const rendersDir = mkdtempSync(join(tmpdir(), "hf-renders-out-")); + tmpDirs.push(rendersDir); + const adapter: StudioApiAdapter = { + listProjects: () => [], + resolveProject: async (id: string) => ({ id, dir: tmpdir() }), + bundle: async () => null, + lint: async () => ({ findings: [] }), + runtimeUrl: "/api/runtime.js", + rendersDir: () => rendersDir, + startRender: (opts) => ({ + id: opts.jobId, + status: "rendering", + progress: 0, + outputPath: opts.outputPath, + }), + }; + const app = new Hono(); + registerRenderRoutes(app, adapter); + return { app, rendersDir }; + } + + // Mirror the repo convention (preview.test.ts / composition tests above): + // skip symlink cases on non-symlink-privileged Windows runners. + function tryCreateSymlink(target: string, path: string, type: "dir" | "file"): boolean { + try { + symlinkSync(target, path, type); + return true; + } catch { + return false; + } + } + + afterEach(() => { + for (const d of tmpDirs) rmSync(d, { recursive: true, force: true }); + tmpDirs.length = 0; + }); + + it("serves a render file that lives inside rendersDir", async () => { + const { app, rendersDir } = buildApp(); + writeFileSync(join(rendersDir, "demo.mp4"), "render-bytes"); + const res = await app.request("http://localhost/projects/demo/renders/file/demo.mp4"); + expect(res.status).toBe(200); + expect(await res.text()).toBe("render-bytes"); + }); + + it("rejects a file reached through a symlink inside rendersDir pointing outside it", async () => { + const { app, rendersDir } = buildApp(); + // A bare join()+readFileSync followed the symlink and leaked the target; + // the resolveWithinProject chokepoint canonicalizes with realpath first. + const external = mkdtempSync(join(tmpdir(), "hf-renders-external-")); + tmpDirs.push(external); + writeFileSync(join(external, "secret.txt"), "TOP-SECRET"); + if (!tryCreateSymlink(join(external, "secret.txt"), join(rendersDir, "leak.txt"), "file")) + return; + const res = await app.request("http://localhost/projects/demo/renders/file/leak.txt"); + expect(res.status).toBe(403); + expect(await res.text()).not.toContain("TOP-SECRET"); + }); + + it("serves a render file reached through a symlink that stays inside rendersDir", async () => { + const { app, rendersDir } = buildApp(); + mkdirSync(join(rendersDir, "nested")); + writeFileSync(join(rendersDir, "nested", "clip.mp4"), "nested-bytes"); + if (!tryCreateSymlink(join(rendersDir, "nested"), join(rendersDir, "alias"), "dir")) return; + const res = await app.request("http://localhost/projects/demo/renders/file/alias/clip.mp4"); + expect(res.status).toBe(200); + expect(await res.text()).toBe("nested-bytes"); + }); +}); diff --git a/packages/core/src/studio-api/routes/render.ts b/packages/core/src/studio-api/routes/render.ts index 6b6bb058b2..bf62bbb55e 100644 --- a/packages/core/src/studio-api/routes/render.ts +++ b/packages/core/src/studio-api/routes/render.ts @@ -216,7 +216,13 @@ export function registerRenderRoutes(api: Hono, adapter: StudioApiAdapter): void const filename = c.req.path.split("/renders/file/")[1]; if (!filename) return c.json({ error: "missing filename" }, 400); const rendersDir = adapter.rendersDir(project); - const fp = join(rendersDir, filename); + // Containment guard: the filename is attacker-controlled wildcard input, so + // route it through the same chokepoint every other project-scoped path uses. + // Literal `..` is collapsed upstream by the URL parser, but a bare join() + + // readFileSync still followed an in-rendersDir symlink pointing outside the + // dir; resolveWithinProject canonicalizes with realpath before serving. + const fp = resolveWithinProject(rendersDir, filename); + if (!fp) return c.json({ error: "forbidden" }, 403); if (!existsSync(fp)) return c.json({ error: "not found" }, 404); const contentType = renderContentType(fp); const content = readFileSync(fp);