fix(preview): serve external symlink assets

[jrusso1020]

What

Allow Studio Preview to serve an asset reached through a project-local symlink whose target is in a shared directory outside the project, including browser-hostile video assets that need an authoring proxy.

Why

Preview rejected these assets with a 404 while the renderer accepted the same path. The initial static-route fix still failed for HEVC, ProRes, AV1, and VP9 assets because the proxy transcoder rejected the external target.

How

Use lexical project-root containment for the read-only static asset route and proxy source request. The transcoder canonicalizes the target for ffmpeg and includes that identity in its cache key, while keeping the proxy cache inside the project. Composition source paths retain canonical containment because preview can persist their data-hf-id values.

Test plan

• Unit tests added/updated
• bun run --cwd packages/studio-server test (397 tests)
• Studio Server typecheck, oxlint, and oxfmt
• External-symlinked hostile-video proxy route regression
• Static-route traversal regression
• Documentation updated (not applicable)

[jrusso1020]

• fix(preview): serve external symlink assets #2764 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miguel-heygen]

Reviewed exact head 354a3ac538190e78a410621a78c2c8c0f58d7e70 against the static preview route, shared asset resolution, renderer FileServer behavior, and the transparent media-proxy path.

Blocker

🔴 External symlinked browser-hostile videos are still broken under the default preview configuration.

This patch makes the static asset route lexically admit assets/shared/clip.mov, and scanProjectMediaCodecMap() uses the same lexical resolver, so HEVC/ProRes/AV1/VP9 assets reached through that symlink are inserted into window.__HF_MEDIA_CODEC_MAP__. media.autoProxy defaults on, so the runtime rewrites the element to ...?hf-proxy=h264|vp8. But the route then calls:

resolveProxy(project.dir, file, proxyVariant)

and proxyTranscoder.ts:145-161 canonicalizes both paths and throws ProxySourceOutsideProjectError whenever the symlink target is outside project.dir. The route converts that into a 502. In other words, SVG/CSS/H.264 parity is fixed, but the default preview path for precisely the codecs that need authoring proxies still fails, even though render accepts the source.

Please carry the new symlink policy through the proxy boundary safely (while keeping the cache under the project), and add an end-to-end route regression: an external-symlinked hostile video is discovered by the codec map and ?hf-proxy= resolves successfully. If the intended scope is intentionally only non-proxied assets, narrow the PR claim and test the exclusion explicitly — but that would leave Preview/Render parity incomplete for a supported asset class.

Security boundary / regression coverage

🟡 The lexical containment choice matches the renderer's deliberately accepted local-author threat model from #486, and resolve() + isWithinProjectRoot() still rejects URL .. traversal. Because this intentionally relaxes the resolveWithinProject chokepoint introduced by #1398, add a static-route integration assertion that a normal traversal request (not only the existing proxy-specific case) remains 404. That pins the exact boundary being retained.

CI

The required Test job is red, but the failure is an unrelated CLI renderLocal browser GPU config hook timeout at packages/cli/src/commands/render.test.ts:196; the changed Studio Server and lint suites are not the failing package. Windows tests were still running at review time. This CI flake is not the reason for the requested change above.

Verdict: REQUEST CHANGES — the lexical static-serving change is correct for non-proxied assets, but the default auto-proxy path rejects the same external symlink target, so the advertised Preview/Render parity is not complete.

[vanceingalls]

R1 adversarial review — #2764 (fix/preview-external-symlink @ 354a3ac)

Adversarial lens: assume this is a subtle security bypass until proven otherwise. Change is narrowly scoped and the parity claim holds, but there's one comment-vs-code mismatch worth clarifying and a couple of test-coverage gaps.

Verdict: COMMENT. Not blocking on the change itself — verified narrow scope, no bleed to writes, matches renderer's static server (packages/cli/src/utils/staticProjectServer.ts L187-207). But one P1 doc/comment fix + a couple of P2 test gaps before merge.

Scope audit — clean

• Only the /projects/:id/preview/* static-asset GET is loosened (preview.ts L526-544). No bleed to render.ts, media.ts, storyboard.ts, files.ts, helpers/compositionInsertion.ts, or helpers/htmlBundler.ts — all continue to use resolveWithinProject (the realpath-canonicalizing helper in packages/core/src/safePath.ts).
• Composition sub-comp route above (L488-522) still uses resolveWithinProject, matching the comment.
• Renderer static server (packages/cli/src/utils/staticProjectServer.ts L189-191) already does the same lexical resolve + relative + startsWith("..") check with a loopback-only bind — parity claim verified.

P1 — code comment overstates what the check restricts

packages/studio-server/src/routes/preview.ts L532-535:

// Assets are read-only and should mirror the renderer: permit a path that
// is lexically inside the project even if an explicit project symlink
// targets a shared directory outside it.

The words "explicit project symlink targets a shared directory outside it" read as if the check restricts the symlink target to a shared directory. It doesn't. isWithinProjectRoot is purely lexical on the pre-realpath candidate — the target can be anywhere the process can readFileSync (e.g. /etc/passwd, ~/.ssh/id_rsa, any file in $HOME). Only the request path is contained; the filesystem target is unbounded.

That's fine for the current threat model (see below) but the comment should stop implying a target-side restriction that isn't in the code. Suggested rewrite:

// Assets are read-only and should mirror the renderer: permit a lexically
// in-project request path even when the resolved filesystem target is
// outside the project (e.g. an in-project symlink to a shared assets
// directory). We trust any symlink the developer placed inside the project.
// Composition source files still use resolveWithinProject because preview
// mutates their data-hf-id values.

Related: it'd be worth calling out in the PR description (or a code comment) which threat models were considered and dismissed:

1. Default deployment (HYPERFRAMES_PREVIEW_HOST unset → 127.0.0.1): local dev only; developer already has fs read access — this change adds nothing they couldn't already get. Fine.
2. LAN-exposed opt-in (HYPERFRAMES_PREVIEW_HOST=0.0.0.0, documented in packages/cli/src/server/portUtils.ts L355-359): previously symlink-safe (realpath would 404), now a LAN attacker who knows a symlink's request path can exfiltrate whatever the symlink targets. If this env var is only for "internal debugging on a trusted network," a one-line doc note near the env var (or a warning at server startup) that symlinks are no longer contained would be the right mitigation.
3. Untrusted project imports (someone hyperframes opens a project shipped in a tar/zip they downloaded): a hostile in-tree symlink ./assets/steal -> ~/.ssh/id_rsa becomes exfiltratable via GET /projects/<id>/preview/assets/steal from any localhost browser tab the user has open. If Hyperframes never intentionally extracts symlinks from third-party archives, this is theoretical. If it does — worth checking whether archive import strips symlinks or preserves them.

Neither of (2) or (3) has to be solved in this PR, but confirming out loud that they're out-of-scope keeps the review record clean.

P2 — test doesn't cover the trust boundary you actually widened

The added preview.test.ts test (L519-538) creates a symlink to a plausibly-named "shared assets" directory. That doesn't distinguish the new behavior from the old — with a canonical check, the same test would fail; with the new check, it passes. But the interesting property is "any symlink target is trusted, not just ones under a designated shared root." A second test with a symlink target that is unambiguously not under any expected shared-assets directory (e.g. mkdtempSync(join(tmpdir(), "hf-unrelated-")) — no "shared" in the name) would make the trust boundary explicit in the test suite and lock the design intent against future well-meaning tightening.

If the design intent is narrower than "any symlink target trusted" — say "only symlinks whose target sits under a configured shared-assets root," which is what the current comment reads like — then the code is missing that restriction and this becomes a P0.

P2 — no cycle handling

Symlink cycle (ln -s a b && ln -s b a) or self-loop resolves via path.resolve (lexical, no fs touch) fine, but readFileSync at L608-610 will throw ELOOP, which Hono surfaces as a 500 by default (no try/catch around the read). Prior canonical check would have rejected these with a realpathSync throw before ever reaching the read. Low-risk (developer would notice their own broken symlink quickly), but a try { readFileSync } catch { return 404 } around the two reads would keep the surface tidy.

P2 — Windows coverage effectively zero

symlinkSync(externalDir, ..., "dir") on Windows requires either admin or Developer Mode; without either, the tryCreateSymlink helper silently returns false and the test early-returns (if (!tryCreateSymlink(...)) return). If Windows CI runners don't have Developer Mode enabled (this repo's Windows tests historically don't), the new tests never actually exercise this on Windows. Not blocking — but a console.warn inside the catch (or an explicit test.skipIf) would surface silent skips rather than hide them behind a passing green.

Praise

• Route scoping is textbook: the loosened check applies to exactly one read-only GET, with the write-mutating composition-source route kept on canonical containment and the reason explicitly commented. That's the kind of documented invariant that survives future refactors — nice.
• The change matches the renderer's existing behavior (verified in staticProjectServer.ts), so the "preview / render parity" claim in the PR body isn't hand-wavy; it maps to actual lexical-containment code with the same shape.
• Test correctly guards on symlink-creation failure (tryCreateSymlink returning false) so CI on privilege-restricted hosts doesn't flake.

---

R1 by Via

[jrusso1020]

Addressed in cdcfd93.

• resolveProxy() now accepts a source only when its requested path is lexically within the project, while canonicalizing the target for ffmpeg. External targets add their canonical identity to the cache key, and the cache remains under the canonical project directory.
• Added coverage for an external-symlinked hostile video being injected into the codec map and served through ?hf-proxy=h264.
• Added the requested ordinary static-route traversal assertion (404).

Verified with bun run --cwd packages/studio-server test (397 tests), Studio Server typecheck, oxlint, and oxfmt.

[miguel-heygen]

R2 at exact head cdcfd93863b6431f80568cff4de58ee58c01589a.

The blocker is resolved cleanly:

• packages/studio-server/src/helpers/proxyTranscoder.ts:150-178 now enforces lexical containment on the project-addressed request path, then canonicalizes the target for ffmpeg. A direct outside path still fails, while an in-project symlink may reach the trusted external target.
• packages/studio-server/src/helpers/proxyTranscoder.ts:181-196 includes the canonical external identity in the cache key and still places the proxy under the canonical project's .transcode-cache, so retargeted symlinks cannot reuse an unrelated proxy and the cache does not escape.
• packages/studio-server/src/helpers/proxyTranscoder.test.ts:488-520 pins both halves of that boundary: direct outside sources remain rejected, and external targets addressed through an in-project symlink transcode using the canonical target with an in-project cache.
• packages/studio-server/src/routes/preview.test.ts:538-541 pins ordinary traversal as 404; packages/studio-server/src/routes/preview.test.ts:1105-1148 pins hostile-codec map/proxy route wiring.

Via's comment wording, ELOOP handling, and Windows skip-transparency notes remain useful non-blocking follow-ups; I found no new blocker in the delta. All required CI is green, including Windows. Focused local verification passed: proxy transcoder 20/20 and preview route 47/47 after building workspace deps.

Verdict: APPROVE
Reasoning: The proxy boundary now carries the same deliberate lexical trust policy as static preview/render while preserving direct-path rejection, canonical ffmpeg input, cache isolation, and regression coverage.

— Magi

[jrusso1020]

Merge activity

• Jul 24, 5:35 PM UTC: @jrusso1020 merged this pull request with Graphite.