Skip to content

Commit 809a978

Browse files
jrusso1020claude
andcommitted
refactor(core): route project paths through a single resolveWithinProject chokepoint
Structural follow-up to the symlink-escape fix. The recurring miss (#465 fixed isSafePath but left render.ts; the sweep then turned up play.ts, htmlBundler, ...) is because containment was enforced by convention — "remember to call isSafePath after every resolve()" — which a new call site can silently skip. Add resolveWithinProject(base, relativePath) -> string | null (resolve + containment in one call) and route the studio-api + bundler sites through it, so a caller cannot resolve a project-relative path without the guard: - studio-api routes/files.ts (read, rename, duplicate, upload-dir), preview.ts (sub-comp + static asset), render.ts (composition) — all the resolve()+isSafePath() pairs collapse to a single call. - compiler/htmlBundler.ts: its local safePath helper was exactly this; drop it for the shared one. Left intentionally on isSafePath: files.ts upload (resolves a name against a validated sub-dir but contains against the project root) and htmlBundler's CSS @import (resolves against the CSS file's dir, contains against the root) — these resolve and contain against *different* bases, which the single-base chokepoint doesn't model. Exported from @hyperframes/core and re-exported from studio-api/helpers for back-compat. Adds resolveWithinProject unit tests; all existing studio-api route tests pass unchanged (behavior is identical — same resolve, same containment, same reject paths). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5b6c62e commit 809a978

8 files changed

Lines changed: 100 additions & 49 deletions

File tree

packages/core/src/compiler/htmlBundler.ts

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,7 @@ import { validateHyperframeHtmlContract } from "./staticGuard";
1717
import { getHyperframeRuntimeScript } from "../generated/runtime-inline";
1818
import { readDeclaredDefaults } from "../runtime/getVariables";
1919
import { inlineSubCompositions } from "./inlineSubCompositions";
20-
import { isSafePath } from "../safePath.js";
21-
22-
/**
23-
* Resolve a relative path within projectDir, rejecting traversal outside it.
24-
* Uses isSafePath so an in-project symlink pointing outside the root can't
25-
* smuggle an external file into the bundle (this fn's result is read+inlined).
26-
*/
27-
function safePath(projectDir: string, relativePath: string): string | null {
28-
const resolved = resolve(projectDir, relativePath);
29-
return isSafePath(projectDir, resolved) ? resolved : null;
30-
}
20+
import { isSafePath, resolveWithinProject } from "../safePath.js";
3121

3222
const DEFAULT_RUNTIME_SCRIPT_URL = "";
3323

@@ -233,7 +223,7 @@ function maybeInlineRelativeAssetUrl(urlValue: string, projectDir: string): stri
233223
if (!urlValue || !isRelativeUrl(urlValue)) return null;
234224
const { basePath, suffix } = splitUrlSuffix(urlValue.trim());
235225
if (!basePath) return null;
236-
const filePath = safePath(projectDir, basePath);
226+
const filePath = resolveWithinProject(projectDir, basePath);
237227
if (!filePath || !shouldInlineAsDataUrl(filePath)) return null;
238228
const content = safeReadFileBuffer(filePath);
239229
if (content == null) return null;
@@ -643,7 +633,7 @@ export async function bundleToSingleHtml(
643633
for (const el of [...document.querySelectorAll('link[rel="stylesheet"]')]) {
644634
const href = el.getAttribute("href");
645635
if (!href || !isRelativeUrl(href)) continue;
646-
const cssPath = safePath(projectDir, href);
636+
const cssPath = resolveWithinProject(projectDir, href);
647637
if (!cssPath) continue;
648638
const css = safeReadFile(cssPath);
649639
if (css == null) continue;
@@ -675,7 +665,7 @@ export async function bundleToSingleHtml(
675665
for (const el of [...document.querySelectorAll("script[src]")]) {
676666
const src = el.getAttribute("src");
677667
if (!src || !isRelativeUrl(src)) continue;
678-
const jsPath = safePath(projectDir, src);
668+
const jsPath = resolveWithinProject(projectDir, src);
679669
const js = jsPath ? safeReadFile(jsPath) : null;
680670
if (js == null) continue;
681671
localJsChunks.push(js);
@@ -710,7 +700,7 @@ export async function bundleToSingleHtml(
710700
const subCompResult = inlineSubCompositions(document, subCompositionHosts, {
711701
resolveHtml: (srcPath: string) => {
712702
if (!isRelativeUrl(srcPath)) return null;
713-
const compPath = safePath(projectDir, srcPath);
703+
const compPath = resolveWithinProject(projectDir, srcPath);
714704
return compPath ? safeReadFile(compPath) : null;
715705
},
716706
parseHtml: parseHTMLContent,
@@ -741,7 +731,7 @@ export async function bundleToSingleHtml(
741731
if (seenCompScriptSrcs.has(extSrc)) continue;
742732
seenCompScriptSrcs.add(extSrc);
743733
if (isRelativeUrl(extSrc)) {
744-
const jsPath = safePath(projectDir, extSrc);
734+
const jsPath = resolveWithinProject(projectDir, extSrc);
745735
const js = jsPath ? safeReadFile(jsPath) : null;
746736
if (js != null) {
747737
compScriptChunks.push(js);
@@ -806,7 +796,7 @@ export async function bundleToSingleHtml(
806796
if (!seenCompScriptSrcs.has(externalSrc)) {
807797
seenCompScriptSrcs.add(externalSrc);
808798
if (isRelativeUrl(externalSrc)) {
809-
const jsPath = safePath(projectDir, externalSrc);
799+
const jsPath = resolveWithinProject(projectDir, externalSrc);
810800
const js = jsPath ? safeReadFile(jsPath) : null;
811801
if (js != null) {
812802
compScriptChunks.push(js);
@@ -861,7 +851,7 @@ export async function bundleToSingleHtml(
861851
if (!seenCompScriptSrcs.has(externalSrc)) {
862852
seenCompScriptSrcs.add(externalSrc);
863853
if (isRelativeUrl(externalSrc)) {
864-
const jsPath = safePath(projectDir, externalSrc);
854+
const jsPath = resolveWithinProject(projectDir, externalSrc);
865855
const js = jsPath ? safeReadFile(jsPath) : null;
866856
if (js != null) {
867857
compScriptChunks.push(js);

packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export {
163163
type MediaVisualStyleProperty,
164164
} from "./inline-scripts/parityContract";
165165
export { redactTelemetryString } from "./telemetryRedaction";
166-
export { isSafePath } from "./safePath";
166+
export { isSafePath, resolveWithinProject } from "./safePath";
167167
export type {
168168
HyperframePickerApi,
169169
HyperframePickerBoundingBox,

packages/core/src/safePath.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, it, expect, afterEach } from "vitest";
22
import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
5-
import { isSafePath } from "./safePath.js";
5+
import { isSafePath, resolveWithinProject } from "./safePath.js";
66

77
describe("isSafePath", () => {
88
const tmpDirs: string[] = [];
@@ -106,3 +106,52 @@ describe("isSafePath", () => {
106106
expect(isSafePath(base, join(base, "file.txt"))).toBe(false);
107107
});
108108
});
109+
110+
describe("resolveWithinProject", () => {
111+
const tmpDirs: string[] = [];
112+
113+
afterEach(() => {
114+
for (const d of tmpDirs) rmSync(d, { recursive: true, force: true });
115+
tmpDirs.length = 0;
116+
});
117+
118+
function tmpDir(prefix: string): string {
119+
const dir = mkdtempSync(join(tmpdir(), prefix));
120+
tmpDirs.push(dir);
121+
return dir;
122+
}
123+
124+
function tryCreateSymlink(target: string, path: string, type: "dir" | "file"): boolean {
125+
try {
126+
symlinkSync(target, path, type);
127+
return true;
128+
} catch {
129+
return false;
130+
}
131+
}
132+
133+
it("returns the resolved absolute path for an in-project relative path", () => {
134+
const base = tmpDir("rwp-base-");
135+
expect(resolveWithinProject(base, "assets/logo.png")).toBe(join(base, "assets", "logo.png"));
136+
});
137+
138+
it("returns the resolved path for a not-yet-existing write target", () => {
139+
const base = tmpDir("rwp-base-");
140+
expect(resolveWithinProject(base, "new/deep/file.txt")).toBe(
141+
join(base, "new", "deep", "file.txt"),
142+
);
143+
});
144+
145+
it("returns null for a `..` traversal that escapes the project", () => {
146+
const base = tmpDir("rwp-base-");
147+
expect(resolveWithinProject(base, "../../etc/passwd")).toBeNull();
148+
});
149+
150+
it("returns null when the path resolves outside via an in-project symlink", () => {
151+
const base = tmpDir("rwp-base-");
152+
const external = tmpDir("rwp-external-");
153+
writeFileSync(join(external, "secret.txt"), "top secret");
154+
if (!tryCreateSymlink(external, join(base, "link"), "dir")) return;
155+
expect(resolveWithinProject(base, "link/secret.txt")).toBeNull();
156+
});
157+
});

packages/core/src/safePath.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,17 @@ export function isSafePath(base: string, resolved: string): boolean {
5656
return targetReal === baseReal || targetReal.startsWith(baseReal + sep);
5757
}
5858
}
59+
60+
/**
61+
* Resolve `relativePath` against `base` and return the absolute path only if it
62+
* stays within `base` (after symlink resolution); otherwise return `null`.
63+
*
64+
* Prefer this over a bare `resolve()` followed by a separate `isSafePath()`
65+
* check: collapsing the two into one call means a caller cannot resolve a
66+
* project-relative path and then forget the containment guard — the gap that
67+
* let the symlink-escape slip past several call sites historically.
68+
*/
69+
export function resolveWithinProject(base: string, relativePath: string): string | null {
70+
const resolved = resolve(base, relativePath);
71+
return isSafePath(base, resolved) ? resolved : null;
72+
}

packages/core/src/studio-api/helpers/safePath.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { readdirSync } from "node:fs";
44
// `isSafePath` lives at the package root so non-studio-api layers (compiler,
55
// CLI, engine) can share it without a backwards dependency on studio-api.
66
// Re-exported here for back-compat with existing `../helpers/safePath.js` imports.
7-
export { isSafePath } from "../../safePath.js";
7+
export { isSafePath, resolveWithinProject } from "../../safePath.js";
88

99
const IGNORE_DIRS = new Set([".thumbnails", "node_modules", ".git"]);
1010

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type { StudioApiAdapter } from "../types.js";
1616
import { isAudioFile } from "../helpers/mime.js";
1717
import { generateWaveformCache } from "../helpers/waveform.js";
1818
import { validateUploadedMediaBuffer } from "../helpers/mediaValidation.js";
19-
import { isSafePath } from "../helpers/safePath.js";
19+
import { isSafePath, resolveWithinProject } from "../helpers/safePath.js";
2020
import { backupPathForResponse, snapshotBeforeWrite } from "../helpers/backupJournal.js";
2121
import {
2222
findUnsafeDomPatchValues,
@@ -66,8 +66,8 @@ async function resolveProjectPath(
6666
return { error: c.json({ error: "forbidden" }, 403) } as const;
6767
}
6868

69-
const absPath = resolve(project.dir, filePath);
70-
if (!isSafePath(project.dir, absPath)) {
69+
const absPath = resolveWithinProject(project.dir, filePath);
70+
if (!absPath) {
7171
return { error: c.json({ error: "forbidden" }, 403) } as const;
7272
}
7373

@@ -1037,8 +1037,8 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
10371037
return c.json({ error: "newPath required" }, 400);
10381038
}
10391039

1040-
const newAbs = resolve(res.project.dir, body.newPath);
1041-
if (!isSafePath(res.project.dir, newAbs)) {
1040+
const newAbs = resolveWithinProject(res.project.dir, body.newPath);
1041+
if (!newAbs) {
10421042
return c.json({ error: "forbidden" }, 403);
10431043
}
10441044
if (existsSync(newAbs)) {
@@ -1065,14 +1065,14 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
10651065
return c.json({ error: "path required" }, 400);
10661066
}
10671067

1068-
const srcAbs = resolve(project.dir, body.path);
1069-
if (!isSafePath(project.dir, srcAbs) || !existsSync(srcAbs)) {
1068+
const srcAbs = resolveWithinProject(project.dir, body.path);
1069+
if (!srcAbs || !existsSync(srcAbs)) {
10701070
return c.json({ error: "not found" }, 404);
10711071
}
10721072

10731073
const copyPath = generateCopyPath(project.dir, body.path);
1074-
const destAbs = resolve(project.dir, copyPath);
1075-
if (!isSafePath(project.dir, destAbs)) {
1074+
const destAbs = resolveWithinProject(project.dir, copyPath);
1075+
if (!destAbs) {
10761076
return c.json({ error: "forbidden" }, 403);
10771077
}
10781078

@@ -1098,8 +1098,8 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
10981098

10991099
// Optional subdirectory within the project (e.g. "assets/audio")
11001100
const subDir = c.req.query("dir") ?? "";
1101-
const targetDir = subDir ? resolve(project.dir, subDir) : project.dir;
1102-
if (!isSafePath(project.dir, targetDir)) return c.json({ error: "forbidden" }, 403);
1101+
const targetDir = subDir ? resolveWithinProject(project.dir, subDir) : project.dir;
1102+
if (!targetDir) return c.json({ error: "forbidden" }, 403);
11031103
if (subDir && !existsSync(targetDir)) mkdirSync(targetDir, { recursive: true });
11041104

11051105
const formData = await c.req.formData();

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import type { Hono } from "hono";
22
import { existsSync, readFileSync, statSync } from "node:fs";
3-
import { join, resolve } from "node:path";
3+
import { join } from "node:path";
44
import { injectScriptsIntoHtml } from "../../compiler/htmlDocument.js";
55
import type { StudioApiAdapter } from "../types.js";
6-
import { isSafePath } from "../helpers/safePath.js";
6+
import { resolveWithinProject } from "../helpers/safePath.js";
77
import { getMimeType } from "../helpers/mime.js";
88
import { buildSubCompositionHtml } from "../helpers/subComposition.js";
99
import { createProjectSignature } from "../helpers/projectSignature.js";
@@ -287,12 +287,8 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi
287287
const compPath = decodeURIComponent(
288288
c.req.path.replace(`/projects/${project.id}/preview/comp/`, "").split("?")[0] ?? "",
289289
);
290-
const compFile = resolve(project.dir, compPath);
291-
if (
292-
!isSafePath(project.dir, compFile) ||
293-
!existsSync(compFile) ||
294-
!statSync(compFile).isFile()
295-
) {
290+
const compFile = resolveWithinProject(project.dir, compPath);
291+
if (!compFile || !existsSync(compFile) || !statSync(compFile).isFile()) {
296292
return c.text("not found", 404);
297293
}
298294

@@ -321,9 +317,12 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi
321317
const subPath = decodeURIComponent(
322318
c.req.path.replace(`/projects/${project.id}/preview/`, "").split("?")[0] ?? "",
323319
);
324-
const file = resolve(project.dir, subPath);
320+
const file = resolveWithinProject(project.dir, subPath);
321+
if (!file) {
322+
return c.text("not found", 404);
323+
}
325324
const stat = existsSync(file) ? statSync(file) : null;
326-
if (!isSafePath(project.dir, file) || !stat?.isFile()) {
325+
if (!stat?.isFile()) {
327326
return c.text("not found", 404);
328327
}
329328
const contentType = getMimeType(subPath);

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +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 } from "node:path";
4+
import { join } 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";
7+
import { resolveWithinProject } from "../helpers/safePath.js";
88

99
const VALID_RESOLUTIONS = new Set<string>(VALID_CANVAS_RESOLUTIONS);
1010

@@ -80,11 +80,10 @@ export function registerRenderRoutes(api: Hono, adapter: StudioApiAdapter): void
8080
: undefined;
8181
let composition: string | undefined;
8282
if (typeof body.composition === "string" && body.composition.length > 0) {
83-
const resolved = resolve(project.dir, body.composition);
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)) {
83+
// `body.composition` is attacker-controlled (from c.req.json()).
84+
// resolveWithinProject dereferences symlinks, so an in-project symlink
85+
// pointing outside the root can't smuggle the render target out.
86+
if (!resolveWithinProject(project.dir, body.composition)) {
8887
return c.json({ error: "composition path must be within the project directory" }, 400);
8988
}
9089
composition = body.composition;

0 commit comments

Comments
 (0)