Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 8 additions & 18 deletions packages/core/src/compiler/htmlBundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,7 @@ import { validateHyperframeHtmlContract } from "./staticGuard";
import { getHyperframeRuntimeScript } from "../generated/runtime-inline";
import { readDeclaredDefaults } from "../runtime/getVariables";
import { inlineSubCompositions } from "./inlineSubCompositions";
import { isSafePath } from "../safePath.js";

/**
* Resolve a relative path within projectDir, rejecting traversal outside it.
* Uses isSafePath so an in-project symlink pointing outside the root can't
* smuggle an external file into the bundle (this fn's result is read+inlined).
*/
function safePath(projectDir: string, relativePath: string): string | null {
const resolved = resolve(projectDir, relativePath);
return isSafePath(projectDir, resolved) ? resolved : null;
}
import { isSafePath, resolveWithinProject } from "../safePath.js";

const DEFAULT_RUNTIME_SCRIPT_URL = "";

Expand Down Expand Up @@ -233,7 +223,7 @@ function maybeInlineRelativeAssetUrl(urlValue: string, projectDir: string): stri
if (!urlValue || !isRelativeUrl(urlValue)) return null;
const { basePath, suffix } = splitUrlSuffix(urlValue.trim());
if (!basePath) return null;
const filePath = safePath(projectDir, basePath);
const filePath = resolveWithinProject(projectDir, basePath);
if (!filePath || !shouldInlineAsDataUrl(filePath)) return null;
const content = safeReadFileBuffer(filePath);
if (content == null) return null;
Expand Down Expand Up @@ -643,7 +633,7 @@ export async function bundleToSingleHtml(
for (const el of [...document.querySelectorAll('link[rel="stylesheet"]')]) {
const href = el.getAttribute("href");
if (!href || !isRelativeUrl(href)) continue;
const cssPath = safePath(projectDir, href);
const cssPath = resolveWithinProject(projectDir, href);
if (!cssPath) continue;
const css = safeReadFile(cssPath);
if (css == null) continue;
Expand Down Expand Up @@ -675,7 +665,7 @@ export async function bundleToSingleHtml(
for (const el of [...document.querySelectorAll("script[src]")]) {
const src = el.getAttribute("src");
if (!src || !isRelativeUrl(src)) continue;
const jsPath = safePath(projectDir, src);
const jsPath = resolveWithinProject(projectDir, src);
const js = jsPath ? safeReadFile(jsPath) : null;
if (js == null) continue;
localJsChunks.push(js);
Expand Down Expand Up @@ -710,7 +700,7 @@ export async function bundleToSingleHtml(
const subCompResult = inlineSubCompositions(document, subCompositionHosts, {
resolveHtml: (srcPath: string) => {
if (!isRelativeUrl(srcPath)) return null;
const compPath = safePath(projectDir, srcPath);
const compPath = resolveWithinProject(projectDir, srcPath);
return compPath ? safeReadFile(compPath) : null;
},
parseHtml: parseHTMLContent,
Expand Down Expand Up @@ -741,7 +731,7 @@ export async function bundleToSingleHtml(
if (seenCompScriptSrcs.has(extSrc)) continue;
seenCompScriptSrcs.add(extSrc);
if (isRelativeUrl(extSrc)) {
const jsPath = safePath(projectDir, extSrc);
const jsPath = resolveWithinProject(projectDir, extSrc);
const js = jsPath ? safeReadFile(jsPath) : null;
if (js != null) {
compScriptChunks.push(js);
Expand Down Expand Up @@ -806,7 +796,7 @@ export async function bundleToSingleHtml(
if (!seenCompScriptSrcs.has(externalSrc)) {
seenCompScriptSrcs.add(externalSrc);
if (isRelativeUrl(externalSrc)) {
const jsPath = safePath(projectDir, externalSrc);
const jsPath = resolveWithinProject(projectDir, externalSrc);
const js = jsPath ? safeReadFile(jsPath) : null;
if (js != null) {
compScriptChunks.push(js);
Expand Down Expand Up @@ -861,7 +851,7 @@ export async function bundleToSingleHtml(
if (!seenCompScriptSrcs.has(externalSrc)) {
seenCompScriptSrcs.add(externalSrc);
if (isRelativeUrl(externalSrc)) {
const jsPath = safePath(projectDir, externalSrc);
const jsPath = resolveWithinProject(projectDir, externalSrc);
const js = jsPath ? safeReadFile(jsPath) : null;
if (js != null) {
compScriptChunks.push(js);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export {
type MediaVisualStyleProperty,
} from "./inline-scripts/parityContract";
export { redactTelemetryString } from "./telemetryRedaction";
export { isSafePath } from "./safePath";
export { isSafePath, resolveWithinProject } from "./safePath";
export type {
HyperframePickerApi,
HyperframePickerBoundingBox,
Expand Down
51 changes: 50 additions & 1 deletion packages/core/src/safePath.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { describe, it, expect, afterEach } from "vitest";
import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { isSafePath } from "./safePath.js";
import { isSafePath, resolveWithinProject } from "./safePath.js";

describe("isSafePath", () => {
const tmpDirs: string[] = [];
Expand Down Expand Up @@ -106,3 +106,52 @@ describe("isSafePath", () => {
expect(isSafePath(base, join(base, "file.txt"))).toBe(false);
});
});

describe("resolveWithinProject", () => {
const tmpDirs: string[] = [];

afterEach(() => {
for (const d of tmpDirs) rmSync(d, { recursive: true, force: true });
tmpDirs.length = 0;
});

function tmpDir(prefix: string): string {
const dir = mkdtempSync(join(tmpdir(), prefix));
tmpDirs.push(dir);
return dir;
}

function tryCreateSymlink(target: string, path: string, type: "dir" | "file"): boolean {
try {
symlinkSync(target, path, type);
return true;
} catch {
return false;
}
}

it("returns the resolved absolute path for an in-project relative path", () => {
const base = tmpDir("rwp-base-");
expect(resolveWithinProject(base, "assets/logo.png")).toBe(join(base, "assets", "logo.png"));
});

it("returns the resolved path for a not-yet-existing write target", () => {
const base = tmpDir("rwp-base-");
expect(resolveWithinProject(base, "new/deep/file.txt")).toBe(
join(base, "new", "deep", "file.txt"),
);
});

it("returns null for a `..` traversal that escapes the project", () => {
const base = tmpDir("rwp-base-");
expect(resolveWithinProject(base, "../../etc/passwd")).toBeNull();
});

it("returns null when the path resolves outside via an in-project symlink", () => {
const base = tmpDir("rwp-base-");
const external = tmpDir("rwp-external-");
writeFileSync(join(external, "secret.txt"), "top secret");
if (!tryCreateSymlink(external, join(base, "link"), "dir")) return;
expect(resolveWithinProject(base, "link/secret.txt")).toBeNull();
});
});
14 changes: 14 additions & 0 deletions packages/core/src/safePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,17 @@ export function isSafePath(base: string, resolved: string): boolean {
return targetReal === baseReal || targetReal.startsWith(baseReal + sep);
}
}

/**
* Resolve `relativePath` against `base` and return the absolute path only if it
* stays within `base` (after symlink resolution); otherwise return `null`.
*
* Prefer this over a bare `resolve()` followed by a separate `isSafePath()`
* check: collapsing the two into one call means a caller cannot resolve a
* project-relative path and then forget the containment guard — the gap that
* let the symlink-escape slip past several call sites historically.
*/
export function resolveWithinProject(base: string, relativePath: string): string | null {
const resolved = resolve(base, relativePath);
return isSafePath(base, resolved) ? resolved : null;
}
2 changes: 1 addition & 1 deletion packages/core/src/studio-api/helpers/safePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { readdirSync } from "node:fs";
// `isSafePath` lives at the package root so non-studio-api layers (compiler,
// CLI, engine) can share it without a backwards dependency on studio-api.
// Re-exported here for back-compat with existing `../helpers/safePath.js` imports.
export { isSafePath } from "../../safePath.js";
export { isSafePath, resolveWithinProject } from "../../safePath.js";

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

Expand Down
22 changes: 11 additions & 11 deletions packages/core/src/studio-api/routes/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { StudioApiAdapter } from "../types.js";
import { isAudioFile } from "../helpers/mime.js";
import { generateWaveformCache } from "../helpers/waveform.js";
import { validateUploadedMediaBuffer } from "../helpers/mediaValidation.js";
import { isSafePath } from "../helpers/safePath.js";
import { isSafePath, resolveWithinProject } from "../helpers/safePath.js";
import { backupPathForResponse, snapshotBeforeWrite } from "../helpers/backupJournal.js";
import {
findUnsafeDomPatchValues,
Expand Down Expand Up @@ -66,8 +66,8 @@ async function resolveProjectPath(
return { error: c.json({ error: "forbidden" }, 403) } as const;
}

const absPath = resolve(project.dir, filePath);
if (!isSafePath(project.dir, absPath)) {
const absPath = resolveWithinProject(project.dir, filePath);
if (!absPath) {
return { error: c.json({ error: "forbidden" }, 403) } as const;
}

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

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

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

const copyPath = generateCopyPath(project.dir, body.path);
const destAbs = resolve(project.dir, copyPath);
if (!isSafePath(project.dir, destAbs)) {
const destAbs = resolveWithinProject(project.dir, copyPath);
if (!destAbs) {
return c.json({ error: "forbidden" }, 403);
}

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

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

const formData = await c.req.formData();
Expand Down
19 changes: 9 additions & 10 deletions packages/core/src/studio-api/routes/preview.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { Hono } from "hono";
import { existsSync, readFileSync, statSync } from "node:fs";
import { join, resolve } from "node:path";
import { join } from "node:path";
import { injectScriptsIntoHtml } from "../../compiler/htmlDocument.js";
import type { StudioApiAdapter } from "../types.js";
import { isSafePath } from "../helpers/safePath.js";
import { resolveWithinProject } from "../helpers/safePath.js";
import { getMimeType } from "../helpers/mime.js";
import { buildSubCompositionHtml } from "../helpers/subComposition.js";
import { createProjectSignature } from "../helpers/projectSignature.js";
Expand Down Expand Up @@ -287,12 +287,8 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi
const compPath = decodeURIComponent(
c.req.path.replace(`/projects/${project.id}/preview/comp/`, "").split("?")[0] ?? "",
);
const compFile = resolve(project.dir, compPath);
if (
!isSafePath(project.dir, compFile) ||
!existsSync(compFile) ||
!statSync(compFile).isFile()
) {
const compFile = resolveWithinProject(project.dir, compPath);
if (!compFile || !existsSync(compFile) || !statSync(compFile).isFile()) {
return c.text("not found", 404);
}

Expand Down Expand Up @@ -321,9 +317,12 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi
const subPath = decodeURIComponent(
c.req.path.replace(`/projects/${project.id}/preview/`, "").split("?")[0] ?? "",
);
const file = resolve(project.dir, subPath);
const file = resolveWithinProject(project.dir, subPath);
if (!file) {
return c.text("not found", 404);
}
const stat = existsSync(file) ? statSync(file) : null;
if (!isSafePath(project.dir, file) || !stat?.isFile()) {
if (!stat?.isFile()) {
return c.text("not found", 404);
}
const contentType = getMimeType(subPath);
Expand Down
13 changes: 6 additions & 7 deletions packages/core/src/studio-api/routes/render.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { Hono } from "hono";
import { streamSSE } from "hono/streaming";
import { existsSync, readFileSync, mkdirSync, unlinkSync, readdirSync, statSync } from "node:fs";
import { join, resolve } from "node:path";
import { join } from "node:path";
import type { StudioApiAdapter, RenderJobState } from "../types.js";
import { VALID_CANVAS_RESOLUTIONS, parseFps, type CanvasResolution } from "../../core.types.js";
import { isSafePath } from "../helpers/safePath.js";
import { resolveWithinProject } from "../helpers/safePath.js";

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

Expand Down Expand Up @@ -80,11 +80,10 @@ export function registerRenderRoutes(api: Hono, adapter: StudioApiAdapter): void
: undefined;
let composition: string | undefined;
if (typeof body.composition === "string" && body.composition.length > 0) {
const resolved = resolve(project.dir, body.composition);
// `body.composition` is attacker-controlled (from c.req.json()). isSafePath
// dereferences symlinks so an in-project symlink pointing outside the root
// can't smuggle the render target out of the project dir.
if (!isSafePath(project.dir, resolved)) {
// `body.composition` is attacker-controlled (from c.req.json()).
// resolveWithinProject dereferences symlinks, so an in-project symlink
// pointing outside the root can't smuggle the render target out.
if (!resolveWithinProject(project.dir, body.composition)) {
return c.json({ error: "composition path must be within the project directory" }, 400);
}
composition = body.composition;
Expand Down
Loading