fix: render names a folder composition source instead of crashing (#4613)

* fix(producer): render names a data-composition-src that points at a folder instead of crashing

* fix(lint): name a data-composition-src that points at a folder instead of crashing

* fix(cli): compositions, keyframes and Studio insertion survive a folder composition source

* fix(lint): tell a folder composition source to point at the HTML file inside it

* fix: read a composition source through one file handle so its type check and read cannot race

* fix(parsers): report a named pipe as a composition source instead of blocking on it

* revert(parsers): open composition sources read-only again, which CodeQL reads as no file creation

* fix(parsers): open composition sources with a private mode, skip pipes, and treat bad paths as missing
This commit is contained in:
Miguel Ángel
2026-09-29 07:58:17 +00:00
committed by GitHub
parent c5a69727ff
commit 481dae1ed9
12 changed files with 222 additions and 36 deletions
@@ -86,4 +86,17 @@ describe("parseSubComposition", () => {
elementCount: 1,
});
});
it("treats a data-composition-src that points at a folder as inline instead of crashing", () => {
const baseDir = mkdtempSync(join(tmpdir(), "hyperframes-compositions-"));
try {
mkdirSync(join(baseDir, "compositions", "intro"), { recursive: true });
const html = `<div data-composition-id="intro" data-composition-src="compositions/intro" data-width="1920" data-height="1080"></div>`;
expect(parseCompositions(html, baseDir)).toMatchObject([{ id: "intro" }]);
} finally {
rmSync(baseDir, { recursive: true, force: true });
}
});
});
+5 -4
View File
@@ -1,6 +1,7 @@
import { defineCommand } from "citty";
import type { Example } from "./_examples.js";
import { existsSync, readFileSync } from "node:fs";
import { readFileSync } from "node:fs";
import { readProjectFile } from "@hyperframes/parsers/asset-resolution";
import { resolve, dirname } from "node:path";
export const examples: Example[] = [
@@ -64,9 +65,9 @@ export function parseCompositions(html: string, baseDir: string): CompositionInf
// If this references an external sub-composition, parse that file
if (compositionSrc) {
const subPath = resolve(baseDir, compositionSrc);
if (existsSync(subPath)) {
const subHtml = readFileSync(subPath, "utf-8");
const subInfo = parseSubComposition(subHtml, id, width, height);
const sub = readProjectFile(subPath);
if (sub.kind === "file") {
const subInfo = parseSubComposition(sub.text, id, width, height);
compositions.push({ ...subInfo, source: compositionSrc });
return;
}
+20 -1
View File
@@ -13,7 +13,12 @@ import { beforeAll, describe, expect, it, onTestFinished } from "vitest";
import type { ArgsDef } from "citty";
import { ensureDOMParser } from "../utils/dom.js";
import keyframesCommand from "./keyframes.js";
import { collectShotSelectors, resolveScope, surfaceComposition } from "./keyframes.js";
import {
collectCompositions,
collectShotSelectors,
resolveScope,
surfaceComposition,
} from "./keyframes.js";
import { ensureShotOutputDir } from "./motionShot.js";
// citty types `args` as Resolvable<ArgsDef> (object | promise | thunk); this
@@ -366,3 +371,17 @@ describe("--layout strip help text", () => {
expect(description.toLowerCase()).toContain("only when");
});
});
describe("keyframes project compositions", () => {
it("skips a data-composition-src that points at a folder instead of crashing", () => {
const dir = mkdtempSync(join(tmpdir(), "hf-keyframes-folder-src-"));
onTestFinished(() => rmSync(dir, { recursive: true, force: true }));
mkdirSync(join(dir, "compositions", "intro"), { recursive: true });
writeFileSync(
join(dir, "index.html"),
`<!doctype html><html><body><div data-composition-id="main" data-duration="4"><div data-composition-id="intro" data-composition-src="compositions/intro"></div></div></body></html>`,
);
expect(collectCompositions(join(dir, "index.html"))).toHaveLength(1);
});
});
+9 -6
View File
@@ -1,5 +1,6 @@
import { defineCommand } from "citty";
import { existsSync, readFileSync, statSync } from "node:fs";
import { existsSync, readFileSync } from "node:fs";
import { readProjectFile } from "@hyperframes/parsers/asset-resolution";
import { resolve, dirname, basename, join, relative, sep } from "node:path";
import { parseGsapScript, type GsapAnimation } from "@hyperframes/core/gsap-parser";
import type { Example } from "./_examples.js";
@@ -573,7 +574,7 @@ function groupTraces(tweens: SurfacedTween[]): SurfacedTrace[] {
return traces;
}
function collectCompositions(indexPath: string): SurfacedComposition[] {
export function collectCompositions(indexPath: string): SurfacedComposition[] {
const html = readFileSync(indexPath, "utf-8");
const baseDir = dirname(indexPath);
const out: SurfacedComposition[] = [
@@ -589,9 +590,10 @@ function collectCompositions(indexPath: string): SurfacedComposition[] {
const src = div.getAttribute("data-composition-src");
if (!src) continue;
const subPath = resolve(baseDir, src);
if (!existsSync(subPath)) continue;
const sub = readProjectFile(subPath);
if (sub.kind !== "file") continue;
const id = div.getAttribute("data-composition-id") ?? src;
out.push(surfaceComposition(readFileSync(subPath, "utf-8"), id, src));
out.push(surfaceComposition(sub.text, id, src));
}
return out;
}
@@ -769,9 +771,10 @@ export function resolveScope(args: { target?: string; selector?: string }): {
let projectName: string;
let projectDir: string | undefined;
let entryFile: string | undefined;
if (raw && raw.endsWith(".html") && existsSync(raw) && statSync(raw).isFile()) {
const entry = raw?.endsWith(".html") ? readProjectFile(raw) : undefined;
if (raw && entry?.kind === "file") {
const entryPath = resolve(raw);
comps = [surfaceComposition(readFileSync(entryPath, "utf-8"), basename(entryPath), entryPath)];
comps = [surfaceComposition(entry.text, basename(entryPath), entryPath)];
projectName = basename(entryPath);
projectDir = findProjectRoot(entryPath);
entryFile = relative(projectDir, entryPath).split(sep).join("/");
+14
View File
@@ -269,6 +269,20 @@ describe("missing_or_empty_sub_composition", () => {
expect(finding?.message).toContain("does not exist");
});
it("errors, instead of crashing, when the referenced sub-composition is a folder", async () => {
const project = makeProject(htmlWithSubComp("compositions/scene-title"), {});
mkdirSync(join(project, "compositions", "scene-title"));
writeFileSync(join(project, "compositions", "scene-title", "index.html"), validSubCompHtml());
const { results } = await lintProject(project);
const finding = results
.flatMap((r) => r.result.findings)
.find((f) => f.code === "missing_or_empty_sub_composition");
expect(finding?.message).toContain("compositions/scene-title");
expect(finding?.message).toContain("a folder, not an HTML file");
expect(finding?.fixHint).toContain('"compositions/scene-title/index.html"');
});
it("errors when the referenced sub-composition file has content but no data-composition-id root", async () => {
const { finding, totalErrors } = await lintSubComp("compositions/scene-title.html", {
"scene-title.html": "<!doctype html><html><body><p>TODO: scene content</p></body></html>",
+23 -10
View File
@@ -12,6 +12,7 @@ import {
isUnresolvedAssetPlaceholder,
isWithinProjectRoot,
maskNonScannableRanges,
readProjectFile,
resolveExistingLocalAsset,
resolveLocalAssetCandidates,
resolveProjectRelativeSrc,
@@ -615,7 +616,7 @@ function lintMissingOrEmptySubComposition(
rootHtml: string,
): HyperframeLintFinding[] {
// Dedup by src path — the same reference can appear from nested sub-comps.
const checked = new Map<string, { srcPath: string; problem: string }>();
const checked = new Map<string, { srcPath: string; problem: string; folder?: true }>();
const visited = new Set<string>();
// fallow-ignore-next-line complexity
@@ -636,14 +637,25 @@ function lintMissingOrEmptySubComposition(
if (visited.has(filePath)) continue;
visited.add(filePath);
if (!existsSync(filePath)) {
const read = readProjectFile(filePath);
if (read.kind === "missing") {
if (!checked.has(srcPath)) {
checked.set(srcPath, { srcPath, problem: "the file does not exist" });
}
continue;
}
if (read.kind === "folder") {
if (!checked.has(srcPath)) {
checked.set(srcPath, {
srcPath,
problem: "it is a folder, not an HTML file",
folder: true,
});
}
continue;
}
const fileHtml = readFileSync(filePath, "utf-8");
const fileHtml = read.text;
const validity = checkSubCompositionUsability(fileHtml, parseSubCompHtml);
if (!validity.ok) {
if (!checked.has(srcPath)) {
@@ -664,17 +676,18 @@ function lintMissingOrEmptySubComposition(
walk(rootHtml);
const findings: HyperframeLintFinding[] = [];
for (const { srcPath, problem } of checked.values()) {
for (const { srcPath, problem, folder } of checked.values()) {
findings.push({
code: "missing_or_empty_sub_composition",
severity: "error",
message: `data-composition-src references "${srcPath}", but ${problem}.`,
fixHint:
`Fix this before rendering — the render pre-flight rejects unusable sub-compositions. ` +
`Write valid HTML into "${srcPath}" — it needs a <template> or <body> containing an element with ` +
`data-composition-id, data-width, and data-height. Preview/studio still tolerates and skips the ` +
"scene while you author it. If a scene-authoring step is still running, wait for it to finish " +
"before referencing the file, or re-run the step that generates it.",
fixHint: folder
? `Point data-composition-src at the HTML file inside the folder, such as "${srcPath}/index.html".`
: `Fix this before rendering — the render pre-flight rejects unusable sub-compositions. ` +
`Write valid HTML into "${srcPath}" — it needs a <template> or <body> containing an element with ` +
`data-composition-id, data-width, and data-height. Preview/studio still tolerates and skips the ` +
"scene while you author it. If a scene-authoring step is still running, wait for it to finish " +
"before referencing the file, or re-run the step that generates it.",
});
}
+37 -1
View File
@@ -1,5 +1,6 @@
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { execFileSync } from "node:child_process";
import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, posix, win32 } from "node:path";
import {
@@ -7,6 +8,7 @@ import {
isUnresolvedAssetPlaceholder,
isWithinProjectRoot,
maskNonScannableRanges,
readProjectFile,
resolveProjectRelativeSrc,
} from "./assetResolution.js";
@@ -265,3 +267,37 @@ describe("isWithinProjectRoot", () => {
expect(isWithinProjectRoot("/project", "/project", posix)).toBe(true);
});
});
describe("readProjectFile", () => {
let dir: string;
beforeAll(() => {
dir = mkdtempSync(join(tmpdir(), "hf-read-project-file-"));
writeFileSync(join(dir, "scene.html"), "<div></div>");
mkdirSync(join(dir, "intro"));
});
afterAll(() => rmSync(dir, { recursive: true, force: true }));
it("reads a file's text", () => {
expect(readProjectFile(join(dir, "scene.html"))).toEqual({ kind: "file", text: "<div></div>" });
});
it("names a folder instead of throwing EISDIR", () => {
expect(readProjectFile(join(dir, "intro"))).toEqual({ kind: "folder" });
});
it("reports a missing path, including one under a file", () => {
expect(readProjectFile(join(dir, "gone.html"))).toEqual({ kind: "missing" });
expect(readProjectFile(join(dir, "scene.html", "inner.html"))).toEqual({ kind: "missing" });
});
it("reports a name too long or a symlink loop as missing, as existsSync did", () => {
expect(readProjectFile(join(dir, `${"a".repeat(300)}.html`))).toEqual({ kind: "missing" });
symlinkSync(join(dir, "loop.html"), join(dir, "loop.html"));
expect(readProjectFile(join(dir, "loop.html"))).toEqual({ kind: "missing" });
});
it.skipIf(process.platform === "win32")("reports a named pipe without blocking on it", () => {
execFileSync("mkfifo", [join(dir, "pipe.html")]);
expect(readProjectFile(join(dir, "pipe.html"))).toEqual({ kind: "folder" });
});
});
+26 -1
View File
@@ -1,4 +1,4 @@
import { existsSync } from "node:fs";
import { closeSync, constants, existsSync, fstatSync, openSync, readFileSync } from "node:fs";
import { isAbsolute, join, posix, relative, resolve, sep } from "node:path";
import { decodeUrlPathVariants } from "./composition.js";
@@ -242,3 +242,28 @@ export function maskNonScannableRanges(html: string): string {
out = maskRange(out, /<script\b[^>]*>[\s\S]*?<\/script\b[^>]*>/gi);
return out;
}
export type ProjectFileRead =
| { kind: "file"; text: string }
| { kind: "folder" }
| { kind: "missing" };
/** Reads through one descriptor, so the file-type check and the read see the same file. */
export function readProjectFile(path: string): ProjectFileRead {
let fd: number;
try {
// Non-blocking so a named pipe is reported, not waited on; the mode never applies (no O_CREAT).
fd = openSync(path, constants.O_RDONLY | (constants.O_NONBLOCK ?? 0), 0o600);
} catch (error) {
const code = (error as NodeJS.ErrnoException).code ?? "";
if (["EISDIR", "ENXIO"].includes(code)) return { kind: "folder" };
if (["ENOENT", "ENOTDIR", "ELOOP", "ENAMETOOLONG"].includes(code)) return { kind: "missing" };
throw error;
}
try {
if (!fstatSync(fd).isFile()) return { kind: "folder" };
return { kind: "file", text: readFileSync(fd, "utf-8") };
} finally {
closeSync(fd);
}
}
@@ -823,6 +823,27 @@ describe("detectRenderModeHints", () => {
).rejects.toThrow(/compositions\/intro\.html[\s\S]*compositions\/outro\.html/);
});
it("compileForRender aborts naming a data-composition-src that points at a folder", async () => {
const projectDir = makeSubCompProject(
"hf-folder-subcomp-",
[{ id: "intro", src: "compositions/intro" }],
{},
);
try {
mkdirSync(join(projectDir, "compositions", "intro"));
writeFileSync(
join(projectDir, "compositions", "intro", "index.html"),
validSubCompHtml("intro", "Intro"),
);
await expect(
compileForRender(projectDir, join(projectDir, "index.html"), projectDir),
).rejects.toThrow(/compositions\/intro[\s\S]*a folder, not an HTML file/);
} finally {
rmSync(projectDir, { recursive: true, force: true });
}
});
it("compileForRender aborts when a data-composition-src reference points at a missing file", async () => {
const projectDir = makeSubCompProject(
"hf-missing-subcomp-",
+16 -9
View File
@@ -56,7 +56,10 @@ import {
checkSubCompositionUsability,
type ParsableDocumentLike,
} from "@hyperframes/parsers/sub-composition-validity";
import { isUnresolvedAssetPlaceholder } from "@hyperframes/parsers/asset-resolution";
import {
isUnresolvedAssetPlaceholder,
readProjectFile,
} from "@hyperframes/parsers/asset-resolution";
import { extractMediaMetadata, extractAudioMetadata } from "../utils/ffprobe.js";
import { isPathInside, toExternalAssetKey } from "../utils/paths.js";
import { collectRenderMedia } from "./renderMediaCollector.js";
@@ -209,12 +212,17 @@ function assertSubCompositionsUsable(
// silence here rather than pretend it surfaces an error somewhere else.
if (visited.has(filePath)) continue;
if (!existsSync(filePath)) {
const read = readProjectFile(filePath);
if (read.kind === "missing") {
problems.push({ srcPath, detail: "the file does not exist" });
continue;
}
if (read.kind === "folder") {
problems.push({ srcPath, detail: "it is a folder, not an HTML file" });
continue;
}
const fileHtml = readFileSync(filePath, "utf-8");
const fileHtml = read.text;
const validity = checkSubCompositionUsability(fileHtml, parseSubCompHtmlForValidity);
if (!validity.ok) {
problems.push({
@@ -708,11 +716,12 @@ async function parseSubCompositions(
continue;
}
if (!existsSync(filePath)) {
const read = readProjectFile(filePath);
if (read.kind !== "file") {
continue;
}
const rawSubHtml = readFileSync(filePath, "utf-8");
const rawSubHtml = read.text;
const nestedVisited = new Set(visited);
nestedVisited.add(filePath);
@@ -981,10 +990,8 @@ function inlineSubCompositions(
resolveHtml: (srcPath: string) => {
let compHtml = subCompositions.get(srcPath) || null;
if (!compHtml) {
const filePath = resolve(projectDir, srcPath);
if (existsSync(filePath)) {
compHtml = readFileSync(filePath, "utf-8");
}
const read = readProjectFile(resolve(projectDir, srcPath));
if (read.kind === "file") compHtml = read.text;
}
return compHtml;
},
@@ -1,4 +1,4 @@
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
@@ -175,6 +175,28 @@ describe("insertCompositionIntoSource", () => {
expect(() => insert("../outside.html")).toThrow(CompositionInsertionError);
});
it("rejects a source, or a source's dependency, that is a folder", () => {
const dir = project();
writeFixture(dir);
mkdirSync(join(dir, "intro"));
writeFileSync(
join(dir, "wraps-folder.html"),
`<div data-composition-id="wrap" data-width="1" data-height="1" data-duration="1"><div data-composition-src="intro"></div></div>`,
);
const insert = (sourcePath: string) =>
insertCompositionIntoSource({
projectDir: dir,
targetPath: "index.html",
sourcePath,
parentSource: parent,
start: 0,
desiredTrack: 0,
});
expect(() => insert("intro")).toThrow(/folder, not an HTML file/);
expect(() => insert("wraps-folder.html")).toThrow(/folder, not an HTML file/);
});
it("says the project folder is gone rather than that the source escapes it", () => {
const dir = project();
rmSync(dir, { recursive: true });
@@ -1,4 +1,5 @@
import { existsSync, readFileSync } from "node:fs";
import { existsSync } from "node:fs";
import { readProjectFile } from "@hyperframes/parsers/asset-resolution";
import { randomUUID } from "node:crypto";
import { dirname, relative, resolve, sep } from "node:path";
import { parseHTML } from "linkedom";
@@ -52,6 +53,17 @@ function canonicalProjectPath(projectDir: string, candidate: string | null): str
return canonical;
}
function readCompositionSource(file: string): string {
const read = readProjectFile(file);
if (read.kind === "folder") {
throw new CompositionInsertionError("Composition source is a folder, not an HTML file", 400);
}
if (read.kind === "missing") {
throw new CompositionInsertionError("Composition source was not found", 404);
}
return read.text;
}
function validateSourcePath(sourcePath: string): void {
if (!sourcePath.trim() || sourcePath.includes("\0") || /^[a-z]+:/i.test(sourcePath)) {
throw new CompositionInsertionError("Invalid composition source path", 400);
@@ -83,7 +95,7 @@ function validateDependencyGraph(projectDir: string, targetAbs: string, sourceAb
}
if (visited.has(file)) return;
visiting.add(file);
const source = readFileSync(file, "utf-8");
const source = readCompositionSource(file);
const { document } = compositionRoot(source);
for (const host of descendants(document, "[data-composition-src]")) {
const dependency = host.getAttribute("data-composition-src");
@@ -169,7 +181,7 @@ export function insertCompositionIntoSource(input: {
const sourceAbs = canonicalProjectFile(input.projectDir, input.sourcePath);
validateDependencyGraph(input.projectDir, targetAbs, sourceAbs);
const source = readFileSync(sourceAbs, "utf-8");
const source = readCompositionSource(sourceAbs);
const sourceComposition = compositionRoot(source).root;
const duration = positiveAttribute(
sourceComposition,