Skip to content

fix(studio-server): keep the desktop app's hand-off and chat history out of the file routes - #5113

Merged
WaterrrForever merged 6 commits into
mainfrom
miao/studio-private-dir
Oct 6, 2026
Merged

WaterrrForever merged 6 commits into
mainfrom
miao/studio-private-dir

Conversation

@WaterrrForever

@WaterrrForever WaterrrForever commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

The desktop app keeps three files in each project that hold the person's agent session and their chat:

  • .hyperframes/agent-handoff.json, which hyperframes open and Studio's Edit with Framey write.
  • .hyperframes/agent-runs.jsonl, the app's own log of every chat run.
  • .hyperframes/app-history.jsonl, the record of chat turns that the terminal agent catches up on (hyperframes catch-up, coming next).

Studio's file and preview routes reach any path inside the project, so anything that reached the preview port could read those files, or plant a fake hand-off or history.

What changed

  • isPrivateProjectFile (safePath.ts): names those three files, in any case and through a link (lexical and real path).
  • resolveWithinProject: studio-server's copy now wraps core's and refuses them, so every route that resolves a request path does too: read, write, delete, rename, duplicate.
  • Uploads: an upload never lands on either file.
  • Preview routes: the static preview route and preview/comp return 404 for them.
  • Rest of .hyperframes/: served as before, including Studio's motion file and prepared GIF assets.

Split out of the closed #5073. Unlike that version, this blocks only these three files, not all of .hyperframes/, so prepared GIF assets keep loading.

What I measured

  • safePath.test.ts, files.pathSafety.test.ts and preview.test.ts cover every route by plain path, by another case, and through a link, plus the files that stay reachable.
  • Whole studio-server suite: 1159 passed.
  • tsc, oxlint, the comment ratchet, and the pre-commit hooks (fallow, typecheck): all pass.

What I did NOT exercise

  • A running Studio in a browser.
  • Windows and Linux.

…out of the file routes

The file and preview routes reach any path inside the project. The desktop
app keeps two files there that hold the person's agent session and their
chat: .hyperframes/agent-handoff.json and .hyperframes/app-history.jsonl.
No route now reads, writes, renames onto, duplicates or uploads over them,
by any spelling or through a link. The rest of .hyperframes/ (Studio's
motion file, prepared GIF assets) is served as before.
…utes too

The app records every chat run, the person's words included, in
.hyperframes/agent-runs.jsonl. Only the app's main process reads it.
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2055 (base branch 2055), smooth 1625 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 (0)

Unstable (1)

  • crop-none-pct-r30-root-z100: tracking 0.04, pressJump 0, drop 40.08, reload 40.08, render 34.74, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.07, reload 0.07, render 0.27, renderKey -, undo true, teleport true / tracking 0.04, pressJump 0, drop 0.07, reload 0.07, render 0.27, renderKey -, undo true, teleport true

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The direct-read, preview and symlink guards work, and their tests fail without them. But the claim that routes "never read or write the hand-off and chat records" doesn't hold yet.

Blocking

  1. A record is missing from the list. safePath.ts:34-38 leaves out .hyperframes/agent-handoff-read.json, the read cursor #2928 writes. The file routes can still read and overwrite it. #5114's .hyperframes/app-history-seen.json isn't listed either.
  2. Renaming the folder escapes the guard. isPrivateProjectFile matches exact file paths only. The rename route (files.ts:3403-3425) accepts .hyperframes itself. After PATCH .../files/.hyperframes {newPath: "x"}, x/app-history.jsonl reads through the file routes. Matching .hyperframes/ as a prefix for the private names (or denying a rename of .hyperframes or any folder above the records) would close it.
  3. The project listing still shows the records. GET /projects/:id (projects.ts:49-55) returns walkDir unfiltered, and walkDir only ignores .hyperframes/backup, so the record paths are still listed. Filter them with the same predicate.

Non-blocking

  • The upload test sends a .jsonl payload that media validation may reject on its own, so it may pass even without the guard. Use a file type uploads accept, so the private-path guard is the only reason for the 403.

— Jerrai

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes at 23b76be7. Wrapping the shared resolveWithinProject is the right place: every route that resolves a request path now passes through one check, and the tests cover GET, PUT, DELETE, PATCH-rename, duplicate, upload and both preview routes. This review adds to Jerrai's (missing files, renaming the folder, the listing).

Blocker: make this an allow-list for .hyperframes/, not a list of three names

  • Probe on this head through registerFileRoutes: GET .hyperframes/agent-handoff-read.json returns 200, and so do .hyperframes/app-history-seen.json and .hyperframes/share.json. All three were written by the desktop app or CLI. The first holds the same sessionId this PR hides in agent-handoff.json.
  • Every new record file will need another entry, and the sibling changes already add two. The holistic shape is the reverse: Studio's own entries under .hyperframes/ are a small, known set, and every other path under .hyperframes/ is refused. That set is studio-motion.json, studio-manual-edits.json, prepared-assets/, and whichever of backup/, preview/, history-id and hf-ids-stamped.json the routes actually need. It also closes the folder-rename gap Jerrai found, as long as the check runs on the real path too, as isPrivateProjectFile already does.

Should-fix

  • Studio's own copy of the folder name and file list should come from one exported constant, not literals. Then the desktop app and the CLI can import the same set, and a new record file can't be added on one side only.

The earlier concern that the file API served a private project file with a token in it is gone: nothing in this design carries a token. What's left is the session id and the person's chat, which is what this PR covers.

Checks at this head:

  • safePath, files.pathSafety and preview tests: 169/169.
  • CI is green.

— Rames

…s own files

Review (Jerrai, Rames): a list of three names missed the read marks the
desktop app and the CLI also keep there, and renaming the folder reached
them under a new name. The check is now an allow-list: no route reaches
.hyperframes/ or anything in it, by any spelling or through a link, except
Studio's studio-motion.json, studio-manual-edits.json and prepared-assets/.
Studio's own internal files (backups, preview documents, history id) are
read by path inside the server, not through a request path.

The project's file tree leaves out the same paths. The upload test now
sends a file uploads accept elsewhere, so the folder is the only reason
it is refused.
@WaterrrForever

Copy link
Copy Markdown
Collaborator Author

Thanks, both. At 51550e42:

  • Allow-list (Rames): nothing under .hyperframes/ is reachable now, the folder itself included, except Studio's studio-motion.json, studio-manual-edits.json and prepared-assets/. This covers agent-handoff-read.json, app-history-seen.json, share.json and any record added later. The check still runs on both the lexical and the real path.
    • Studio's other files there (backups, preview documents, history id, hf-ids-stamped.json) are read and written by path inside the server, never through a request path, so they don't need to be on the list.
    • Neither Studio nor the desktop UI requests anything else under .hyperframes/ through these routes.
  • Folder rename (Jerrai): PATCH .hyperframes {newPath: "x"} is refused, and x doesn't appear. There's a test.
  • Listing (Jerrai): GET /projects/:id drops the same paths from files. The test now checks that studio-motion.json stays listed while examples/, backup/ and app-history.jsonl don't.
  • Upload test: it now uploads a .txt, and the same test shows that file uploading fine at the root. So the folder is the only reason for the 403.
  • Shared constant: with an allow-list, the CLI and the desktop app don't need a list of their own: anything they put under .hyperframes/ is private by default. So the only set is Studio's own, in safePath.ts.

studio-server suite: 1160 passed.

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 51550e42. The allow-list itself is right. The direct file routes, the folder rename and the project listing are all closed. Two paths still reach private records without going through it.

Should-fix

  1. The render-file route checks against the renders folder, not the project. routes/render.ts:271 resolves the filename against rendersDir (<project>/renders, cli/src/server/studioServer.ts:553). The private check then sees a path relative to that folder. In a project that ships renders -> .hyperframes, GET /projects/:id/renders/file/app-history.jsonl resolves to app-history.jsonl under the base, isPrivateProjectPath says no, and the route serves it. A project's own preview script can make that request. Fix: also refuse when isPrivateProjectFile(project.dir, fp) is true.
  2. A rename still reads and can rewrite private JSON. updateReferences (routes/files.ts:789-815) walks the whole project with walkFiles (:654-670). That walk doesn't skip .hyperframes, and the rewrite checks only isSafePath. Renaming a referenced file rewrites any matching path inside .hyperframes/*.json records (for example agent-handoff.json and app-history-seen.json). Today's records hold no project paths, so the practical impact is small, but it breaks the "nothing under .hyperframes/ is read or written" guarantee. Fix: skip private paths in the walk.

Confirmed fixed

  • The allow-list exposes only studio-motion.json, studio-manual-edits.json and prepared-assets/, and the folder itself is denied (helpers/safePath.ts:33-55).
  • PATCH checks both the source and the destination. The same resolver covers read, write, delete, duplicate and upload.
  • Case variants, ./.., decoded segments and links into private records are checked against both the typed and the real path (safePath.ts:18-20,42-67).
  • The project listing filters private paths (routes/projects.ts:49-55), and the upload test now has a real success control.
  • The allow-list excludes the files #5114 writes.

Not run locally (no installed dependencies in my checkout).

— Jerrai

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes at 51550e42. The allow-list is the right shape, and it closes the leak on the file, preview, upload, rename and listing routes. One thing it now blocks is a path Studio's own preview route has to serve. One write still reaches the private records.

Fixed

  • Allow-list: safePath.ts refuses .hyperframes and everything under it except studio-motion.json, studio-manual-edits.json and prepared-assets/. It checks the lowercased path both as asked and after resolving links.
  • Reads, via createStudioApi (base → head):
    • files/ returns 403 for agent-handoff-read.json, app-history-seen.json, share.json, app-history.jsonl and agent-handoff.json. preview/ returns 404 for the same files.
    • Other spellings, all 403: .HyperFrames/, ./, %2ehyperframes, %2EHYPERFRAMES%2F, //, /./, assets/../, prepared-assets/../share.json and ..%2f.
    • A linked folder or a linked file is refused.
    • The three allowed entries still return 200.
  • Writes: PUT, create, delete, rename in or out, and upload into .hyperframes or a link to it all return 403. A rename of the folder itself returns 403, and the listing shows only the three entries.

Blocker: request pictures under .hyperframes/requests/ stop loading.

  • HyperFrames Desktop saves each chat request's picture at .hyperframes/requests/<n>/before.png, and its request cards load it through Studio's /api/projects/:id/preview/<path> route.
  • Probe: GET preview/.hyperframes/requests/1/before.png is 200 at the base and 404 at this head. When Desktop picks up this release, those cards show a broken picture.
  • These are crops of the person's own video, not records, so requests/ belongs on the allow-list, with a test.
  • This also means "nothing else under .hyperframes/ is requested" isn't quite true. It's worth a grep of what else Desktop's renderer loads through preview/ before the list is final.

Should-fix (same as Jerrai's second point): a rename still rewrites private records.

  • updateReferences (files.ts) walks every .json/.md in the project, .hyperframes/ included.
  • Probe: renaming assets/a.png → assets/b.png rewrote the ref in .hyperframes/share.json and returned updatedReferences: 4. That count also tells the caller which private records mention a path.
  • Skipping isPrivateProjectFile(projectDir, file) in that walk closes it.

Jerrai's other point, the render-file route checking against renders/ instead of the project, is also a real bypass of the same class. Putting the check in the one place every route resolves a path would cover both.

Non-blocking

  • Reuse: STUDIO_FILES retypes two paths that already exist as exported constants, STUDIO_MOTION_PATH (studioMotionRenderScript.ts) and STUDIO_MANUAL_EDITS_PATH (manualEditsRenderScript.ts). Importing them keeps the list and the writers from drifting apart.
  • The allowed folder itself (.hyperframes/prepared-assets, no trailing slash) is refused, so an upload into it returns 403. I found no caller that does this today.
  • Product note: skill output under .hyperframes/ disappears from the file tree. That includes expanded-prompt.md, which the prompt-expansion skill tells the person to review, plus anim-map/, contrast/ and frame-packets/. Probably fine, but it's a visible change.

Checks at 51550e42:

  • The four changed test files pass 176 of 176.
  • The studio-server suite passes 1157, with 2 skipped.
  • CI: the required checks that finished are green, and the edit-accuracy shards were still pending.
  • Not tested: Windows path forms and macOS case-insensitivity. The lowercasing plus realpathSync.native should cover them, but I only reasoned that through.

— Rames

…rs and rename paths

Review (Rames, Jerrai):
- Desktop loads each request's picture from .hyperframes/requests/ through
  the preview route, so that folder joins the allow-list. An allowed
  folder's own path is allowed too.
- The render-file route resolves against renders/, so a renders/ linked
  into .hyperframes/ reached the records. It now also checks the file
  against the project.
- A rename's reference update walked every JSON/MD file, records
  included, and could rewrite them. It now skips private paths.
- The allow-list reuses STUDIO_MOTION_PATH and STUDIO_MANUAL_EDITS_PATH.
@WaterrrForever

Copy link
Copy Markdown
Collaborator Author

Thanks, both. At 58a7a86e:

  • Request pictures (Rames): .hyperframes/requests/ joins the allow-list, so preview/.hyperframes/requests/1/before.png returns 200 again. There's a test.
    • An allowed folder's own path (.hyperframes/prepared-assets, .hyperframes/requests) is allowed too.
    • I grepped Desktop's renderer and main for other .hyperframes/ paths loaded through preview/ or files/, and found none.
  • Render-file route (Jerrai): the file is now also checked against the project, so a renders -> .hyperframes link returns 403. There's a test.
  • Rename references (both): the reference update skips private paths. Renaming inside.txt leaves .hyperframes/share.json untouched and reports updatedReferences: 0. There's a test.
  • Reuse: the allow-list now uses STUDIO_MOTION_PATH and STUDIO_MANUAL_EDITS_PATH.
  • Skill outputs leaving the tree (expanded-prompt.md, anim-map/, contrast/, frame-packets/): this is intended. They are the agent's working files, and the agent reads them by path. I'd rather keep the list to Studio's own entries than grow it per skill.

studio-server suite: 1163 passed.

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 58a7a86e. Both fixes from my last review hold. One more render path still serves private files.

Should-fix: renders listed from disk are served without the private check. GET /projects/:id/renders (routes/render.ts:298-346) lists every .mp4/.webm/.mov in renders/. It statSyncs each one, following links, and registers each in renderJobs with outputPath: join(rendersDir, filename). GET /render/:jobId/view and /download (:214-245) then readFileSync(job.outputPath) with no project or real-path check. So a project that ships renders/leak.mp4 -> ../.hyperframes/app-history.jsonl gets its chat history served from /render/leak/view once the renders list loads. Suggested fix: skip any entry where isPrivateProjectFile(project.dir, fp) is true (or any entry whose real path leaves renders/) before registering it, and add a test like the renders -> .hyperframes one.

Confirmed fixed

  • /renders/file/* checks the resolved file against project.dir, on both the typed and the real path (render.ts:281-285, safePath.ts:50-54). The new test covers renders -> .hyperframes.
  • The rename reference update skips private files before reading or rewriting them (files.ts:799-812).
  • The other path-helper callers pass the project dir: file mutations, media, thumbnails, previews and freeze frames. Static preview assets also apply isPrivateProjectFile (preview.ts:615-629).
  • .hyperframes/requests/ is allowed as a whole subtree. A link inside it to ../app-history.jsonl resolves to a private real path and is refused.

— Jerrai

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes at 58a7a86e. Everything from the last round is fixed. One route in render.ts still serves private records through a link. Jerrai's review raised the same issue, and I confirmed it with a separate test.

Blocking: the render list and the view/download routes follow links

  • GET /projects/:id/renders (render.ts:297-352) finds every .mp4/.webm/.mov in renders/ with join and statSync, which follow links. It saves that path as the job's outputPath.
  • GET /render/:jobId/view and /download (render.ts:214-250) then call readFileSync(job.outputPath) with no check.
  • Test at this head, in a project that ships renders/leak.mp4 -> ../.hyperframes/app-history.jsonl:
    • The list returns 200 with leak.
    • render/leak/view and render/leak/download both return 200 with the chat history.
    • A link to a file outside the project is served the same way.
    • renders/file/leak.mp4 correctly returns 403.
  • The same code is on main, so this predates the PR. But it is the shipped-link case the new renders/ test covers, one route over in the same file.
  • Suggested fix: build the list with the check renders/file/* already uses. Keep an entry only when resolveWithinProject(rendersDir, f) returns a path and isPrivateProjectFile(project.dir, path) is false, and save that resolved path. Checking again in view/download would also cover a link swapped after the list was built.
  • I looked at every file read in routes/. These two routes are the only ones that serve a path that never goes through the shared resolveWithinProject check, so this closes the gap everywhere.

Fixed since 51550e42

  • Request pictures: preview/.hyperframes/requests/3/before.png now returns 200 (it was 404).
    • after.png, edit-1/was-0.jpg and rect.json return 200 on both preview/ and files/.
    • The match is the folder name plus /, so .hyperframes/requestsX/a is still refused.
    • Desktop writes only picture crops and a crop-box rect.json under requests/<n>/. No prompt or chat text is stored there, so the allow-list stays narrow.
  • Escapes from requests/ are refused:
    • requests/../app-history.jsonl, requests/3/..%2f..%2fapp-history.jsonl and requests%2f..%2fshare.json return 403 on files/ and 404 on preview/.
    • A link inside requests/ to a record file or to the records folder is refused, because the real path is checked too.
    • A PUT through a folder link returns 403.
    • Renaming requests into .hyperframes/ returns 403.
  • Rename: renaming assets/a.png rewrote only index.html (updatedReferences: 1). share.json, agent-handoff.json, comments.jsonl and references/moments/m.json were left as they were.
  • Render-file route: it now also checks isPrivateProjectFile(project.dir, fp).
  • Reuse: STUDIO_FILES now imports STUDIO_MOTION_PATH and STUDIO_MANUAL_EDITS_PATH, and .hyperframes/prepared-assets itself is allowed.
  • The three new tests fail against the 51550e42 source and pass here.

Non-blocking

  • The allow-list grants write as well as read, so files under requests/ can now be overwritten, deleted and renamed through the file routes. That is probably fine for the person's own crops, but it is a choice worth knowing about.
  • Request pictures now show in the Studio file tree.

Checks at 58a7a86e

  • The four changed test files pass 213 of 213.
  • CI had no failures when I looked. "Tests on windows-latest" passes. The edit-accuracy shards were still running.

— Rames

…ile route would serve

Review (Jerrai, Rames): the render list found files in renders/ with join
and statSync, which follow links, and registered them for /render/:id/view
and /download, which read them with no check. A project shipping
renders/leak.mp4 -> ../.hyperframes/app-history.jsonl had its chat served.
The list now keeps an entry only when resolveWithinProject accepts it and
it isn't private, and registers the resolved path. View and download serve
only a plain file, so a link swapped in after the list is refused. This
predates the PR; it's the same shipped-link case.
@WaterrrForever

Copy link
Copy Markdown
Collaborator Author

Thanks, both. At dc304cd9:

  • Render list, view and download:
    • The list keeps a file only when resolveWithinProject(rendersDir, f) accepts it and isPrivateProjectFile(project.dir, path) is false, and it registers that resolved path as the job's output.
    • View and download serve only a plain file (lstat), so a link swapped in after the list was built is refused too.
    • Test: a project shipping renders/leak.mp4 -> ../.hyperframes/app-history.jsonl lists only real. render/leak/view and /download return 404 without the chat, and real still returns 200.
  • Writes under requests/: that's fine. They're the person's own crops, and the file tree shows them now.

studio-server suite: 1164 passed.

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at dc304cd9.

The bypass from my last review is closed, in two layers:

  • GET /projects/:id/renders now builds each entry's path with resolveWithinProject and drops anything isPrivateProjectFile flags. That predicate also compares real paths, so a renders/leak.mp4 link into .hyperframes/ is never listed and never registered in renderJobs.
  • /render/:jobId/view and /download now lstat the output path and serve only a plain file.

The new test proves the list never registers the link, and that real still views and downloads.

Non-blocking: the test can't tell the two layers apart. leak is never registered, so its 404 holds even without isPlainFile. A case that seeds renderJobs with a symlinked outputPath directly would pin the second layer. Not needed for this PR.

— Jerrai

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at dc304cd9. The blocker from my last review is fixed.

What I checked
dc304cd9 is the only commit since 58a7a86e, and it touches only render.ts and its test. I wrote a scratch test against createStudioApi and checked each case on GET /projects/:id/renders, /render/:id/view and /render/:id/download:

  • renders/leak.mp4 linking to .hyperframes/app-history.jsonl, by absolute and by relative path: left out of the list, and view/download return 404.
  • renders/out.mp4 linking to a file outside the project: left out, 404.
  • renders/x.mp4 linking to the .hyperframes folder: left out, 404. A renders/sub folder link isn't listed, and renders/file/sub/app-history.jsonl returns 403.
  • A link swapped in after the list was built (list a.mp4 and b.mp4 as plain files, then replace them with links to the history and to an outside file): view/download return 404 for both.
  • A plain render in renders/ still lists, views, downloads and deletes.
  • A job started by POST /projects/:id/render still views and downloads. I checked three setups: renders inside the project, renders outside it (the ../renders setup in vite.adapter.ts), and a project opened through a symlinked path.

The list now uses the same check as renders/file/* (resolveWithinProject plus isPrivateProjectFile), so the two routes agree on what's in scope. The full studio-server suite passes (1161 tests), along with tsc, oxlint and oxfmt. In CI, every check passes, including "Tests on windows-latest", the edit-accuracy gate and CodeQL.

Non-blocking

  1. A hard link in renders/ to a private file is still listed and served. A hard link can't be told apart from a real file, so this is a known limit, not something to fix here.
  2. View and download run lstat only on the last part of the path. If the whole renders/ folder is swapped for a link after the list, a file with the same name in the new target gets served. That needs a .mp4/.webm/.mov named like a listed render, and the app's own records don't use those names, so it's fine as is.
  3. A link inside renders/ to another render (alias.mp4 -> real.mp4) is listed, but view and download return 404 for it, while renders/file/alias.mp4 serves it. Studio plays renders through renders/file/*, so users won't see this. The list and view/download just don't quite agree.
  4. Still open from last time: the .hyperframes/requests/ allow-list also allows writes, and request pictures show in the file tree.

— Rames

@WaterrrForever
WaterrrForever added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit 32516b5 Oct 6, 2026
92 checks passed
@WaterrrForever
WaterrrForever deleted the miao/studio-private-dir branch October 6, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants