feat: rewrite as TanStack Start app — a clearer HyperFrames-on-Cloudflare example#10
feat: rewrite as TanStack Start app — a clearer HyperFrames-on-Cloudflare example#10tombeckenham wants to merge 8 commits into
Conversation
Full-stack rewrite of the template on TanStack Start (Vite + @cloudflare/vite-plugin), replacing the hand-rolled Worker router and static preview page: - src/server.ts custom entry: Start fetch handler + RenderContainer DO export (wrangler main now points here) - /api/render, /api/preview, /r/<key> ported to Start server routes; asset fetches use the request origin (dev assets binding is same-origin-only) - /api/generate rebuilt on TanStack AI: chat() + @tanstack/ai-openrouter streams the composition to the browser over SSE; BYOK key travels in forwardedProps via a custom useChat fetcher and is never persisted - lint + self-heal moved to a client-driven loop: lintComposition server function (@hyperframes/core/lint) after each assistant turn, fix requests sent as follow-up chat turns (max 2, temp 0.3) - React UI (routes/index.tsx + components) reproduces the original preview page, adds live streaming view of the generated HTML - scripts/build.mjs now runs via npm prepare-assets before vite dev/build instead of wrangler build.command
Review fixes on top of the TanStack Start rewrite: - Gate custom-HTML /api/render behind ENABLE_AI_GEN so public demos can't run arbitrary HTML through the render container; the bundled "click Render" path stays open - Keep the BYOK key in React state only, never sessionStorage — generated compositions execute same-origin in the player iframe and could read origin storage; security note updated to match - Drop empty preview_image_url that breaks the Deploy to Cloudflare button (re-fix of 02b8a69) - Surface server error messages: the useChat fetcher throws the JSON error body instead of the library's bare status line, RenderControls parses the error envelope, and a lint server-fn failure warns instead of faking success - Wrap the R2 put with contextual logging; serve only renders/* keys from /r/$; validate message roles and clamp durationSec - Extract pure encoding helpers (native Uint8Array.toBase64 with a chunked fallback for Node 22) and add 16 vitest tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds a "Log in with OpenRouter" button that runs OpenRouter's OAuth PKCE flow entirely client-side (src/lib/openrouter-oauth.ts): redirect to openrouter.ai/auth with an S256 code challenge, then exchange the one-time callback code for a runtime API key scoped to this app. No client secret, no server involvement — the key stays in React memory just like the pasted-key flow. Manual key entry remains as a fallback behind a toggle. The PKCE code verifier is the one sanctioned sessionStorage use: it only exists across the redirect round-trip and is deleted (and ?code stripped from the URL) before any generated composition can run. Includes unit tests for the pure helpers (base64url, RFC 7636 appendix B challenge vector, auth URL construction) and README updates.
jrusso1020
left a comment
There was a problem hiding this comment.
Ran an ultra-critical pass per Bin's ask — read the full files, not just the diff, and verified each of the six flagged security surfaces at source (and against the test vectors). No blockers. The security-critical invariants are all correct. Two non-blocking findings and one minor nit are worth the author's consideration; leading with those since scrutiny was the point.
Findings
1. allow-same-origin means "memory-only" is a bar-raiser, not an isolation boundary (non-blocking, but the security note slightly overstates it).
The BYOK key genuinely never touches sessionStorage/localStorage — verified: it lives only in useState/useRef (AiPanel.tsx:49,57) and is forwarded per-request (:108). That correctly removes the trivial sessionStorage.getItem() read vector. But the framing "held in memory only … generated compositions could read origin storage" implies memory placement keeps the key away from the composition, and it doesn't: with allow-scripts allow-same-origin, a generated composition can reach window.parent (same-origin), and from there walk the React fiber tree (__reactFiber$* → memoizedState of the key hooks) or override parent.fetch to capture apiKeyRef.current on the next generate/self-heal turn. So the real blast-radius control is the spend-capped, revocable key (which the UI does recommend — good), not where the key is stored. Two options: (a) tighten the wording so it credits the spend-cap rather than the memory placement, and/or (b) if you want true isolation, run the generated preview in a sandboxed iframe without allow-same-origin (opaque origin) — with the caveat that this likely breaks the <hyperframes-player> runtime, which is presumably why same-origin is used. Fine to ship as-is for a template; worth a precise sentence so adopters don't over-trust it.
2. OAuth-callback effect gates a one-time, irreversible side effect behind cancelled (non-blocking robustness).
consumeOAuthCallback() synchronously consumes the one-time code, removes the verifier, and strips the URL — then awaits the key exchange (openrouter-oauth.ts:58-85). In AiPanel.tsx:64-83 the minted key is applied only if !cancelled. If <AiPanel> unmounts between the synchronous strip and the async resolve, the successfully-minted key is discarded with no recovery (code already consumed, verifier gone, ?code stripped) — login silently no-ops and the user must restart. Today this can't fire (StrictMode is off and AiPanel doesn't remount in the normal flow), but the inline comment at :61-63 ("a second effect run (React strict mode) is a no-op") is subtly wrong: under StrictMode the first run's key is the one dropped by cancelled, so enabling StrictMode — a common hardening — would make OAuth login silently fail in dev. Suggest applying the minted key regardless of the closure's cancelled flag (it's a legitimate one-shot mint), or hoisting the exchange behind a useRef/module one-shot guard so the result survives a remount.
3. durationSec has a lower bound but no upper clamp (minor).
generate.ts:111-114 validates Number.isFinite && > 0 but doesn't cap it. It's only a prompt hint (feeds buildUserPrompt), so blast radius is small — output is capped by MAX_OUTPUT_TOKENS and render is separately gated — but an absurd value could nudge the model toward a very long composition that inflates render time/cost on the deployer's account. A Math.min(durationSec, MAX) closes it cheaply.
Observation (pre-existing, out of scope): the bundled-composition render path (/api/render with no body) is intentionally ungated so the public "Render" button works, so a public demo can be driven to re-run the (bounded, known) bundled render on the deployer's account. Same property as the pre-rewrite version — a per-IP rate limit would harden a public demo, but that's not this PR's job.
Verified correct (the surfaces Bin flagged)
ENABLE_AI_GENrender gate — closes the arbitrary-HTML hole on every custom-HTML path: the gate is the first check insideif (body?.html)(render.ts:40-49), so non-string/truthy html still hits it; omittingcontent-type: application/jsondoesn't smuggle HTML past it (it falls through to the bundled composition, ignoring the body). Matched at/api/generatetoo (:58-63).- R2
/r/$serving —key.startsWith("renders/")gate (r.$.ts:10); R2 keys are literal strings (no filesystem traversal, sorenders/../xis just a non-existent key), andRENDERSis a dedicated bucket (hyperframes-renders) holding only render outputs. No prefix bypass, no traversal. - base64 chunk boundary — the chunked fallback builds one binary string across
0x8000chunks then a singlebtoa(encoding.ts:24-33), so the boundary can't corrupt output;encoding.test.tsexercises below/at/above (0x7fff/0x8000/0x8001) + multi-chunk and asserts native === chunked. Correct and genuinely covered. - OAuth PKCE — S256 challenge matches the RFC 7636 Appendix B test vector (
openrouter-oauth.test.ts:33), verifier is 32 random bytes → RFC-compliant 43-char base64url, and the verifier is removed +?codestripped synchronously before the exchange, so it can't leak to a composition (which can't run until after a key exists anyway). - Boundary validation / errors — message
roleis validated and a client-suppliedsystemrole is rejected (generate.ts:89-92), which nicely blocks client-side system-prompt injection since the real system prompt is server-injected; size caps are present everywhere (key 1KB, prompt 8KB, render HTML 2MB, output 16k tokens); server error messages now propagate to the user in bothAiPanelandRenderControls.
Nice rewrite — the integration points read much more clearly as an example, and the hardening is real. Findings 1 and 2 are the two I'd want the author to weigh before merge; neither blocks.
— Jerrai
| // not just the ones in the adapter's union. OpenRouter rejects unknown | ||
| // ids upstream and the error streams back to the client. | ||
| const model = (typeof props.model === "string" ? props.model : DEFAULT_MODEL) as OpenRouterModel; | ||
| const durationSec = |
There was a problem hiding this comment.
durationSec is validated > 0 && finite but not upper-bounded. It's only a prompt hint so blast radius is small, but an absurd value could push the model toward a very long composition that inflates render cost. A Math.min(durationSec, MAX) closes it. (Summary finding #3, minor.)
| // Same gate as /api/generate: without it, ENABLE_AI_GEN="false" | ||
| // deployments would still let anyone run the render container on | ||
| // arbitrary HTML. The bundled-composition path below stays open. | ||
| if (env.ENABLE_AI_GEN !== "true") { |
There was a problem hiding this comment.
Verified: this gate is the first statement inside if (body?.html), so it covers every custom-HTML path — a non-string/truthy html still hits it, and a request without application/json content-type falls through to the bundled composition (body ignored) rather than smuggling HTML past the gate. This closes the arbitrary-HTML-on-deployer's-account hole cleanly.
| GET: async ({ params }) => { | ||
| const key = params._splat ?? ""; | ||
| // Only serve objects /api/render wrote — not arbitrary bucket keys. | ||
| if (!key.startsWith("renders/")) return new Response("not found", { status: 404 }); |
There was a problem hiding this comment.
Verified path-traversal-safe: R2 keys are literal strings (not filesystem paths), so renders/../x is just a non-existent literal key, and RENDERS is a dedicated bucket holding only render outputs. The startsWith("renders/") gate is sufficient here — nothing sensitive is reachable.
…protection Review feedback on heygen-com#10: memory-only key storage removes the trivial storage read but is not an isolation boundary — same-origin generated code can still reach window.parent and recover the key. Tighten the code comment, UI security note, and README so adopters don't over-trust where the key is stored.
tombeckenham
left a comment
There was a problem hiding this comment.
Thanks for the review @jrusso1020. I've made some changes to the wording and comments around the sign in with openrouter key.
… clamp, OAuth remount safety - Clamp durationSec to 120s. It's only a prompt hint, but an absurd value could steer the model toward a very long composition that inflates render time/cost on the deployer's account. - Memoize the OAuth callback exchange for the page load. The one-time code is consumed synchronously before the async key exchange resolves, so a repeat effect run (React StrictMode double-effect, or an unmount/ remount mid-exchange) previously found the code gone and silently dropped the minted key with no recovery. Repeat calls now await the same promise and receive the key. Corrects the AiPanel comment that claimed the second run was a no-op, and adds a regression test.
miguel-heygen
left a comment
There was a problem hiding this comment.
Additive exact-head review at 4fd6d67b4c51020e81ff3b355c93eaadaa50ff84. Rames already covered the OAuth, same-origin key exposure, render gate, R2 prefix, and base64 boundary surfaces; this pass focused on the freshness delta and clean-checkout reproducibility.
Audited: src/components/AiPanel.tsx, src/lib/openrouter-oauth.ts + tests, src/routes/api/{generate,render,preview}.ts, src/routes/r.$.ts, server asset/lint functions, package scripts, and the full 7a6439c..4fd6d67 delta.
Trusting: generated worker-configuration.d.ts / routeTree.gen.ts, lockfile internals, CSS, and unchanged composition assets.
The freshness fixes are structurally sound: openrouter-oauth.ts:64-70 memoizes the one-shot exchange at module scope, and openrouter-oauth.test.ts:39-83 pins the consumed-code/repeat-call behavior. The revised same-origin warning in AiPanel.tsx:14-21,225-241 is also materially more accurate.
blocker — the advertised typecheck does not work from a clean checkout. package.json:36 runs bare tsc --noEmit, while src/lib/server/assets.ts:2 imports src/composition-manifest.json, which is generated by scripts/build.mjs and explicitly gitignored. I reproduced this from the exact head:
npm ci
npm run typecheck
src/lib/server/assets.ts(2,22): error TS2307: Cannot find module '../../composition-manifest.json'
src/lib/server/assets.ts(23,31): error TS7006: Parameter 'rel' implicitly has an 'any' type.
Running npm run build first creates the manifest and makes typecheck pass, which explains how the PR-body claim could look green locally, but fresh contributors and any standalone typecheck job get a deterministic red command. Make typecheck prepare the required assets (or commit/provide a typed manifest that exists before build) and pin the clean-checkout sequence in CI. This repo currently reports no PR checks, so nothing catches the regression.
Local verification: npm test — 23/23 pass; npm run build — pass; npm run typecheck after build — pass.
Verdict: REQUEST CHANGES
Reasoning: the current runtime/security fixes look sound, but a documented verification command deterministically fails on the exact head from a clean clone because its generated input is missing.
— Magi
Summary
Rewrites the template as a TanStack Start app with TanStack AI powering the BYOK generation flow, and applies a round of security/error-handling hardening from a multi-agent review. Same product surface as before — preview the bundled composition, generate one from a prompt with your own OpenRouter account (OAuth login, or paste a key), render MP4s in the Chromium+FFmpeg container, serve them from R2 — but the code now reads as an example rather than as plumbing.
Why TanStack makes this a clearer HyperFrames-on-Cloudflare example
The point of this template is to show how to integrate HyperFrames on Cloudflare — the player, the lint/self-heal loop, and the render container. The previous version buried those integration points inside hand-rolled infrastructure:
src/index.tsdoing manual URL routing, request parsing, and SSE plumbing, plus a 381-line staticpublic/index.htmlwith inline vanilla JS managing all UI state and stream handling. To find the HyperFrames-specific code, a reader had to wade through everything else first.src/routes/api/render.ts— the render-container call: composition files in, MP4 out, streamed to R2src/routes/api/generate.ts— the AI pipeline: HyperFrames skill prompt +chat()+ OpenRouter adapter over SSEsrc/components/Player.tsx— exactly how to wrap the<hyperframes-player>web component in React, including thesrc→srcdocswap for generated compositionssrc/lib/server-fns.ts—@hyperframes/core/lintas a typed server function the client calls like a local async functionAiPanel.tsx(useChat+onFinish), instead of bespoke SSE parsing and DOM mutation. Anyone copying this template into a real app gets the pattern they'd actually use.@cloudflare/vite-plugin— SSR, typed file routes, server functions, and Vite HMR in dev — so the template doubles as a reference for "modern React app on Workers with a container-backed API," which is the shape most adopters will build.Log in with OpenRouter (OAuth PKCE)
The AI panel's primary path is now a Log in with OpenRouter button instead of pasting an API key (manual key entry remains as a fallback toggle):
src/lib/openrouter-oauth.tsruns OpenRouter's OAuth PKCE flow entirely client-side: S256 code challenge → redirect toopenrouter.ai/auth→ exchange the one-time callback code for a runtime API key scoped to this app (revocable from the OpenRouter dashboard)sessionStorageuse — it exists only across the redirect round-trip and is deleted (with?codestripped from the URL) before any generated composition can runSecurity & robustness fixes (from review)
/api/rendercustom-HTML rendering is now gated byENABLE_AI_GEN— previously a public demo with generation "disabled" still let anyone run arbitrary HTML through the render container on the deployer's accountsessionStorage: generated compositions execute same-origin inside the player iframe (allow-scripts allow-same-origin) and could read origin storage; the UI now says so and recommends a spend-capped keypreview_image_urlthat re-broke the Deploy to Cloudflare button (same issue as 02b8a69)/r/$serves onlyrenders/*keys; R2 writes are logged with context on failure; message roles anddurationSecare validated at the boundaryTesting
stripMarkdownFenceedge cases, fix-message formatting, base64/UTF-8 encoding round-trips across the 32KB chunk boundary — covering both the nativeUint8Array.toBase64()path (Workers/Node 25+) and the chunked fallback (Node 22 LTS) — plus the OAuth PKCE helpers (base64url encoding, the RFC 7636 appendix B challenge test vector, auth URL construction)npm test,npm run typecheck, andnpm run buildall pass; independently re-verified by two review agents against@tanstack/ai-clientinternals (error propagation, stream finalization)🤖 Generated with Claude Code