Skip to content

Commit e7127cd

Browse files
jrusso1020claude
andcommitted
fix(core,cli): route render + play composition paths through isSafePath
Review on #1397 found a third call site with the same vulnerable startsWith pattern. Apply Rule 2: fix every site sharing the contract (gate an attacker-influenced path before a symlink-following fs op). - studio-api routes/render.ts: body.composition (from c.req.json()) was checked with `resolved.startsWith(resolve(project.dir) + sep)`, which doesn't dereference symlinks — an in-project symlink to an external target escaped the project root. Now uses isSafePath(). - cli commands/play.ts: the `/composition/*` server route used `filePath.startsWith(project.dir)` with no trailing-separator guard, so both a sibling dir sharing the prefix (`<dir>-evil`) and symlink escapes passed. Now uses isSafePath() via @hyperframes/core/studio-api (the same lazy-import pattern commands/validate.ts already uses). Tests: render.test.ts gains a "composition path safety" block (in-base allow, `..` reject, in-project-symlink-to-outside reject, in-project symlink staying inside allow). The shared render test adapter now points at a real dir since isSafePath fails closed on an unresolvable base (production project dirs always exist on disk). Not in this change: compiler/htmlBundler.ts has the same class at two sites (safePath helper + inline CSS @import check), but the compiler sits below studio-api in the dependency graph and can't import isSafePath without a backwards edge; that fix needs the helper promoted to a neutral module and is tracked as a follow-up. renderArgs.ts / videoFrameExtractor.ts carry the trailing-sep guard and a local-CLI/engine-internal threat model. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5b868c0 commit e7127cd

3 files changed

Lines changed: 97 additions & 7 deletions

File tree

packages/cli/src/commands/play.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ export default defineCommand({
100100

101101
const { Hono } = await import("hono");
102102
const { createAdaptorServer } = await import("@hono/node-server");
103+
const { isSafePath } = await import("@hyperframes/core/studio-api");
103104

104105
const app = new Hono();
105106

@@ -124,8 +125,11 @@ export default defineCommand({
124125
const reqPath = ctx.req.path.replace("/composition/", "");
125126
const filePath = resolve(project.dir, reqPath);
126127

127-
// Security: don't allow path traversal outside project dir
128-
if (!filePath.startsWith(project.dir)) return ctx.text("Forbidden", 403);
128+
// Security: don't allow path traversal outside project dir. isSafePath
129+
// canonicalizes symlinks and applies a trailing-separator guard, so neither
130+
// an in-project symlink to an external target nor a sibling dir whose name
131+
// shares the project-dir prefix (e.g. `<dir>-evil`) can escape.
132+
if (!isSafePath(project.dir, filePath)) return ctx.text("Forbidden", 403);
129133
if (!existsSync(filePath)) return ctx.text("Not found", 404);
130134

131135
const content = readFileSync(filePath, "utf-8");

packages/core/src/studio-api/routes/render.test.ts

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { describe, expect, it, vi } from "vitest";
1+
import { afterEach, describe, expect, it, vi } from "vitest";
22
import { Hono } from "hono";
3-
import { mkdtempSync, rmSync } from "node:fs";
3+
import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from "node:fs";
44
import { tmpdir } from "node:os";
55
import { join } from "node:path";
66
import { VALID_CANVAS_RESOLUTIONS } from "../../core.types";
@@ -13,7 +13,9 @@ function createAdapter(
1313
): { adapter: StudioApiAdapter; rendersDir: string } {
1414
const adapter: StudioApiAdapter = {
1515
listProjects: () => [],
16-
resolveProject: async (id: string) => ({ id, dir: "/tmp/proj" }),
16+
// Use a real, existing dir: isSafePath() canonicalizes the project dir with
17+
// realpath and fails closed if it doesn't exist (real projects always do).
18+
resolveProject: async (id: string) => ({ id, dir: tmpdir() }),
1719
bundle: async () => null,
1820
lint: async () => ({ findings: [] }),
1921
runtimeUrl: "/api/runtime.js",
@@ -261,3 +263,83 @@ describe("POST /projects/:id/render — fps wire format", () => {
261263
}
262264
});
263265
});
266+
267+
describe("POST /projects/:id/render — composition path safety", () => {
268+
const tmpDirs: string[] = [];
269+
270+
function buildAppWithProjectDir(spy: ReturnType<typeof vi.fn>): {
271+
app: Hono;
272+
projectDir: string;
273+
} {
274+
const projectDir = mkdtempSync(join(tmpdir(), "hf-render-proj-"));
275+
const rendersDir = mkdtempSync(join(tmpdir(), "hf-render-out-"));
276+
tmpDirs.push(projectDir, rendersDir);
277+
const adapter: StudioApiAdapter = {
278+
listProjects: () => [],
279+
resolveProject: async (id: string) => ({ id, dir: projectDir }),
280+
bundle: async () => null,
281+
lint: async () => ({ findings: [] }),
282+
runtimeUrl: "/api/runtime.js",
283+
rendersDir: () => rendersDir,
284+
startRender: (opts) => {
285+
spy(opts);
286+
return { id: opts.jobId, status: "rendering", progress: 0, outputPath: opts.outputPath };
287+
},
288+
};
289+
const app = new Hono();
290+
registerRenderRoutes(app, adapter);
291+
return { app, projectDir };
292+
}
293+
294+
afterEach(() => {
295+
for (const d of tmpDirs) rmSync(d, { recursive: true, force: true });
296+
tmpDirs.length = 0;
297+
});
298+
299+
async function postComposition(app: Hono, composition: string): Promise<Response> {
300+
return app.request("http://localhost/projects/demo/render", {
301+
method: "POST",
302+
headers: { "content-type": "application/json" },
303+
body: JSON.stringify({ fps: 30, quality: "standard", format: "mp4", composition }),
304+
});
305+
}
306+
307+
it("accepts a composition path inside the project directory", async () => {
308+
const spy = vi.fn();
309+
const { app } = buildAppWithProjectDir(spy);
310+
const res = await postComposition(app, "scenes/intro.html");
311+
expect(res.status).toBe(200);
312+
expect(spy).toHaveBeenCalledOnce();
313+
});
314+
315+
it("rejects a `..` traversal in the composition path", async () => {
316+
const spy = vi.fn();
317+
const { app } = buildAppWithProjectDir(spy);
318+
const res = await postComposition(app, "../../etc/passwd");
319+
expect(res.status).toBe(400);
320+
expect(spy).not.toHaveBeenCalled();
321+
});
322+
323+
it("rejects a composition reached through an in-project symlink pointing outside the project", async () => {
324+
const spy = vi.fn();
325+
const { app, projectDir } = buildAppWithProjectDir(spy);
326+
const external = mkdtempSync(join(tmpdir(), "hf-render-external-"));
327+
tmpDirs.push(external);
328+
writeFileSync(join(external, "secret.html"), "<html></html>");
329+
symlinkSync(external, join(projectDir, "link"), "dir");
330+
const res = await postComposition(app, "link/secret.html");
331+
expect(res.status).toBe(400);
332+
expect(spy).not.toHaveBeenCalled();
333+
});
334+
335+
it("allows a composition reached through an in-project symlink that stays inside the project", async () => {
336+
const spy = vi.fn();
337+
const { app, projectDir } = buildAppWithProjectDir(spy);
338+
mkdirSync(join(projectDir, "real"));
339+
writeFileSync(join(projectDir, "real", "scene.html"), "<html></html>");
340+
symlinkSync(join(projectDir, "real"), join(projectDir, "alias"), "dir");
341+
const res = await postComposition(app, "alias/scene.html");
342+
expect(res.status).toBe(200);
343+
expect(spy).toHaveBeenCalledOnce();
344+
});
345+
});

packages/core/src/studio-api/routes/render.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import type { Hono } from "hono";
22
import { streamSSE } from "hono/streaming";
33
import { existsSync, readFileSync, mkdirSync, unlinkSync, readdirSync, statSync } from "node:fs";
4-
import { join, resolve, sep } from "node:path";
4+
import { join, resolve } from "node:path";
55
import type { StudioApiAdapter, RenderJobState } from "../types.js";
66
import { VALID_CANVAS_RESOLUTIONS, parseFps, type CanvasResolution } from "../../core.types.js";
7+
import { isSafePath } from "../helpers/safePath.js";
78

89
const VALID_RESOLUTIONS = new Set<string>(VALID_CANVAS_RESOLUTIONS);
910

@@ -80,7 +81,10 @@ export function registerRenderRoutes(api: Hono, adapter: StudioApiAdapter): void
8081
let composition: string | undefined;
8182
if (typeof body.composition === "string" && body.composition.length > 0) {
8283
const resolved = resolve(project.dir, body.composition);
83-
if (!resolved.startsWith(resolve(project.dir) + sep)) {
84+
// `body.composition` is attacker-controlled (from c.req.json()). isSafePath
85+
// dereferences symlinks so an in-project symlink pointing outside the root
86+
// can't smuggle the render target out of the project dir.
87+
if (!isSafePath(project.dir, resolved)) {
8488
return c.json({ error: "composition path must be within the project directory" }, 400);
8589
}
8690
composition = body.composition;

0 commit comments

Comments
 (0)