[jrusso1020]

What

buildSubCompositionHtml returns null for a compPath that resolves outside projectDir, the same answer it gives for a missing file. It also leaves out the project index.html head when that file resolves outside the project, for example through a symlink.

Why

The helper is exported from @hyperframes/studio-server (and re-exported by @hyperframes/core/studio-api), and it read join(projectDir, compPath) without a containment check, trusting each caller to confine the path first. Studio's own preview route does (resolveWithinProject in routes/preview.ts). An embedder that forgets would serve any file the process can read. Moving the check into the helper means no caller has to remember it.

Related work

Defense in depth for an embedder-side fix that landed separately; no public issue.

How

compFile comes from core's resolveWithinProject (@hyperframes/core/safe-path), the helper the rest of Studio uses, which also follows symlinks before the containment check. A null result returns null before any existsSync or readFileSync.

Test plan

• New tests in subComposition.test.ts: a ../<sibling>/secret.txt path and an absolute path both return null (fails on main); an index.html symlinked to a file outside the project contributes no head content (fails at the previous head). All 27 pass here.

• Behavior change for local Studio: a project whose index.html is a symlink to a file outside the project loses that head in sub-composition previews, matching how the preview route already treats a composition file symlinked outside.

• vitest run src/routes/preview in studio-server: 159 pass (186 with the helper tests). That route already confines its own path, so its behavior is unchanged.

• oxlint and oxfmt --check are clean on both files, and check:package-subpaths passes.

• Unit tests added/updated

• Manual testing performed

• Documentation updated (if applicable)

• Comments follow CONTRIBUTING.md "Comments"

— Rames Jusso

🤖 Generated with Claude Code

[github-actions]

Edit accuracy: accurate 267 (base branch 267), smooth 105 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

• seqplainkeys-none-center-r0-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-center-r0-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-center-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-pct-r0-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-pct-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-px-r0-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-px-r30-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-px-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run

Unstable (2)

• seqab-none-px-r0-nested-z100: tracking 0.07, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.07, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.07, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
• seqplainkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

[jerrai-bot-heygen]

Security re-review at 036c976: the new resolveWithinProject guard confines the requested composition file, including an existing outside-pointing symlink. Hosted support tests exercised the new spec (26 passing); no local test or baseline run by me.

Residual read path: packages/studio-server/src/helpers/subComposition.ts:336–339 separately reads the project's index.html via join/readFileSync, without applying resolveWithinProject. If index.html is a symlink to a file outside the project, a valid in-project composition passes the new check, yet the helper incorporates the outside file's <head> content in its returned HTML. This was present before this PR, but the stated defense-in-depth guarantee that the helper only reads within the project remains incomplete. Please confine that secondary read too, and cover an existing outside-pointing symlink with a test. I have source-verified the call chain but not run this case. Also the new sibling-path test builds a path using outside.split('/'); on Windows it may not construct the actual sibling path, so its green result does not prove the old escape was tested there.

Findings-only COMMENT, not an approval or merge. — Jerrai

[jerrai-bot-heygen]

Re-review at 28b52977: the prior secondary-read finding is addressed. buildSubCompositionHtml now resolves both the requested composition and the separate project index.html through resolveWithinProject before either file read. The new regression uses a valid in-project scene plus an outside-pointing index.html symlink, asserts the scene remains present and the external head marker is absent, and all 27 helper tests executed successfully in the exact-head hosted support job. I found no new code blocker in this incremental fix. The guard checks existing paths/symlinks but is not an atomic open against concurrent symlink replacement; no such attacker-controlled race was established here. No deployed test or local test by me. Findings-only COMMENT, not approval or merge. — Jerrai

[heygengenesis]

Verdict: APPROVE · 0 blocker · 0 important · 0 nits · 28b52977

Both reads in buildSubCompositionHtml (the composition and the root index.html head) now go through resolveWithinProject, so .., absolute paths and outside-pointing symlinks return null or no head. Studio's own preview route already confines compPath the same way, so local Studio behaves the same apart from the symlinked-index.html case the body already describes. Required CI is green at this head, and the new tests ran in Test (support) (27 pass).

Scope

I read subComposition.ts, its test, core/src/safePath.ts and the only in-repo caller (routes/preview.ts). Jerrai's earlier finding about the secondary index.html read is fixed at this head. Out of scope: assetExists still probes join(projectDir, …) for asset refs. That only checks whether a file exists and doesn't return its contents, and it was there before this PR.

— Genesis