trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns13g rEventsFlows

Files render in the browser, part 1: a safe media route and one registry of file kinds #7341

Merged masoncrew opened this in answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

This adds `GET /:owner/:name/media/:ref/*path`, which serves a file's bytes under a real media type, and one registry of file kinds (`src/web/render/kinds.ts`) that the blob page now asks how to draw a file. Every renderer in #7329 builds on these two. Type check and design lint pass. The new tests pass, and so do the existing blob, security-floor and cross-file suites. The full gate has not run y…

shipped production is serving it deploy 4db86a7

masoncrew opened this

This adds GET /:owner/:name/media/:ref/*path, which serves a file's bytes under a real media type, and one registry of file kinds (src/web/render/kinds.ts) that the blob page now asks how to draw a file. Every renderer in #7329 builds on these two. Type check and design lint pass. The new tests pass, and so do the existing blob, security-floor and cross-file suites. The full gate has not run yet; it runs on this push.

What moved where

  • src/web/render/kinds.ts (new): fileKind(path, firstBytes) returns one of the 11 kinds, and mediaType(path, firstBytes) gives the type the route sends.

- The extension proposes a type from the allowlist and the file's first bytes must confirm it. A mismatch, or any file not on the list, goes out as application/octet-stream. HTML and JavaScript are never served. - STL has no signature, so its extension alone decides. - RENDER_CAP is the largest file each kind draws inline. - RENDERERS maps a kind to its renderer. Only markdown has one so far.

  • src/web/render/markdown.ts (new): Markdown's prose view, moved out of repo.ts as the first renderer and the pattern later ones follow.
  • src/http/range.ts (new): reads a single bytes= range, a suffix range, or an unsatisfiable one as plain data. A list of ranges is answered with the whole file, which the RFC allows.
  • src/web/pages/media.ts (new): the route.

- Headers: nosniff and the sandbox CSP from the issue, so an SVG with a script is drawn as a picture and never runs. - Ranges: 206 with Content-Range, 416 past the end, and If-Range is honoured. - Caching: the ETag is the blob id and a matching If-None-Match gets 304. A full commit sha caches for a year as immutable; a branch revalidates every time. - Access is the same as /raw. - ?download=1 sends the file as an attachment under its own name, at any size.

  • Streaming:

- blobStream in src/git/read.ts streams files over 1 MB straight off git cat-file, never holding them whole. - Streams draw on their own mediaGate in src/git/env.ts (FORGE_MEDIA_CONCURRENCY, default 16), so long video streams can't use up the git permits every page needs. - A reader that hangs up kills the git process and gives its permit back.

  • renderBlob in repo.ts now asks the registry. A kind with no renderer keeps the view it had. Every file's header has Rendered | Source | Raw | Download, with Rendered only where a renderer exists. The Download button, including the binary and too-large cases, goes through the media route.
  • Docs: how to add a renderer is in docs/features/file-media.md, and the CLAUDE.md map names render/.

What to look at

  • Streamed bodies are sent chunked, with no Content-Length. Bun ignores a length set on a large stream. A 206 still names the full size in Content-Range, which is what a seeking player reads. Buffered bodies (1 MB or less) do carry a length.
  • Bytes before a range's start are read and discarded. Git cannot seek inside a blob, so seeking deep into a large video reads everything before that point.
  • Not done here: the issue mentions serving from a bucket, but nothing in this repository stores blobs in one (there is no LFS), so the route streams from git only. Each renderer is left to #7329.

Tests

  • test/render-kinds.test.ts: every extension maps to its kind and type, mismatched bytes fall back, nothing comes back as HTML or script, and the range parser cases.
  • test/media.test.ts boots a real forge and covers:

- types, and the octet-stream fallback; - the SVG served under the sandbox CSP; - single, suffix and 416 ranges on both a buffered file and a 1.5 MB streamed one; - the permit being returned when a reader hangs up; - the 304 on the ETag, and immutable vs revalidated caching; - downloads; - a private repo returning 404 signed out but 200 to its owner; - the blob page's switch.

To try it: open any file on the preview and use Download, or open /<owner>/<repo>/media/main/<file>.png directly.

🤖 Generated with Claude Code

Closes #7330

masoncrew commented

This adds GET /:owner/:name/media/:ref/*path, which serves a file's bytes under a real media type, and one registry of file kinds (src/web/render/kinds.ts) that the blob page now asks how to draw a file. Every renderer in #7329 builds on these two. Type check and design lint pass. The new tests pass, and so do the existing blob, security-floor and cross-file suites. The full gate has not run yet; it runs on this push.

What moved where

  • src/web/render/kinds.ts (new): fileKind(path, firstBytes) returns one of the 11 kinds, and mediaType(path, firstBytes) gives the type the route sends.

- The extension proposes a type from the allowlist and the file's first bytes must confirm it. A mismatch, or any file not on the list, goes out as application/octet-stream. HTML and JavaScript are never served. - STL has no signature, so its extension alone decides. - RENDER_CAP is the largest file each kind draws inline. - RENDERERS maps a kind to its renderer. Only markdown has one so far.

  • src/web/render/markdown.ts (new): Markdown's prose view, moved out of repo.ts as the first renderer and the pattern later ones follow.
  • src/http/range.ts (new): reads a single bytes= range, a suffix range, or an unsatisfiable one as plain data. A list of ranges is answered with the whole file, which the RFC allows.
  • src/web/pages/media.ts (new): the route.

- Headers: nosniff and the sandbox CSP from the issue, so an SVG with a script is drawn as a picture and never runs. - Ranges: 206 with Content-Range, 416 past the end, and If-Range is honoured. - Caching: the ETag is the blob id and a matching If-None-Match gets 304. A full commit sha caches for a year as immutable; a branch revalidates every time. - Access is the same as /raw. - ?download=1 sends the file as an attachment under its own name, at any size.

  • Streaming:

- blobStream in src/git/read.ts streams files over 1 MB straight off git cat-file, never holding them whole. - Streams draw on their own mediaGate in src/git/env.ts (FORGE_MEDIA_CONCURRENCY, default 16), so long video streams can't use up the git permits every page needs. - A reader that hangs up kills the git process and gives its permit back.

  • renderBlob in repo.ts now asks the registry. A kind with no renderer keeps the view it had. Every file's header has Rendered | Source | Raw | Download, with Rendered only where a renderer exists. The Download button, including the binary and too-large cases, goes through the media route.
  • Docs: how to add a renderer is in docs/features/file-media.md, and the CLAUDE.md map names render/.

What to look at

  • Streamed bodies are sent chunked, with no Content-Length. Bun ignores a length set on a large stream. A 206 still names the full size in Content-Range, which is what a seeking player reads. Buffered bodies (1 MB or less) do carry a length.
  • Bytes before a range's start are read and discarded. Git cannot seek inside a blob, so seeking deep into a large video reads everything before that point.
  • Not done here: the issue mentions serving from a bucket, but nothing in this repository stores blobs in one (there is no LFS), so the route streams from git only. Each renderer is left to #7329.

Tests

  • test/render-kinds.test.ts: every extension maps to its kind and type, mismatched bytes fall back, nothing comes back as HTML or script, and the range parser cases.
  • test/media.test.ts boots a real forge and covers:

- types, and the octet-stream fallback; - the SVG served under the sandbox CSP; - single, suffix and 416 ranges on both a buffered file and a 1.5 MB streamed one; - the permit being returned when a reader hangs up; - the 304 on the ETag, and immutable vs revalidated caching; - downloads; - a private repo returning 404 signed out but 200 to its owner; - the blob page's switch.

To try it: open any file on the preview and use Download, or open /<owner>/<repo>/media/main/<file>.png directly.

🤖 Generated with Claude Code

masoncrew commented

This adds GET /:owner/:name/media/:ref/*path, which serves a file's bytes under a real media type, and one registry of file kinds (src/web/render/kinds.ts) that the blob page now asks how to draw a file. Every renderer in #7329 builds on these two.

What moved where

  • src/web/render/kinds.ts (new): fileKind(path, firstBytes) returns one of the 11 kinds, and mediaType(path, firstBytes) gives the type the route sends.

- The extension proposes a type from the allowlist and the file's first bytes must confirm it. A mismatch, or any file not on the list, goes out as application/octet-stream. HTML and JavaScript are never served. - STL has no signature, so its extension alone decides. - RENDER_CAP is the largest file each kind draws inline. - RENDERERS maps a kind to its renderer. Only markdown has one so far.

  • src/web/render/markdown.ts (new): Markdown's prose view, moved out of repo.ts as the first renderer and the pattern later ones follow.
  • src/http/range.ts (new): reads a single bytes= range, a suffix range, or an unsatisfiable one as plain data. A list of ranges is answered with the whole file, which the RFC allows.
  • src/web/pages/media.ts (new): the route.

- Headers: nosniff and the sandbox CSP from the issue, so an SVG with a script is drawn as a picture and never runs. - Ranges: 206 with Content-Range, 416 past the end, and If-Range is honoured. - Caching: the ETag is the blob id and a matching If-None-Match gets 304. A full commit sha caches for a year as immutable; a branch revalidates every time. - Access is the same as /raw. - ?download=1 sends the file as an attachment under its own name, at any size.

  • Streaming:

- blobStream in src/git/read.ts streams files over 1 MB straight off git cat-file, never holding them whole. - Streams draw on their own mediaGate in src/git/env.ts (FORGE_MEDIA_CONCURRENCY, default 16), so long video streams can't use up the git permits every page needs. - A reader that hangs up kills the git process and gives its permit back. - The new piped spawn is listed in test/spawn-pipes.test.ts with why its stdout is read. That test was right to fail the first check.

  • renderBlob in repo.ts now asks the registry. A kind with no renderer keeps the view it had. Every file's header has Rendered | Source | Raw | Download, with Rendered only where a renderer exists. The Download button, including the binary and too-large cases, goes through the media route.
  • Docs: how to add a renderer is in docs/features/file-media.md, and the CLAUDE.md map names render/.

What to look at

  • Streamed bodies are sent chunked, with no Content-Length. Bun ignores a length set on a large stream. A 206 still names the full size in Content-Range, which is what a seeking player reads. Buffered bodies (1 MB or less) do carry a length.
  • Bytes before a range's start are read and discarded. Git cannot seek inside a blob, so seeking deep into a large video reads everything before that point.
  • Not done here: the issue mentions serving from a bucket, but nothing in this repository stores blobs in one (there is no LFS), so the route streams from git only. Each renderer is left to #7329.

Tests

  • test/render-kinds.test.ts: every extension maps to its kind and type, mismatched bytes fall back, nothing comes back as HTML or script, and the range parser cases.
  • test/media.test.ts boots a real forge and covers:

- types, and the octet-stream fallback; - the SVG served under the sandbox CSP; - single, suffix and 416 ranges on both a buffered file and a 1.5 MB streamed one; - the permit being returned when a reader hangs up; - the 304 on the ETag, and immutable vs revalidated caching; - downloads; - a private repo returning 404 signed out but 200 to its owner; - the blob page's switch.

To try it: open any file on the preview and use Download, or open /<owner>/<repo>/media/main/<file>.png directly.

🤖 Generated with Claude Code

sentinelcrew Changes requested — before the latest push

The route is careful: types come only from an allowlist and are confirmed by the file's first bytes, the sandbox CSP and nosniff are on every answer, access uses loadRepo/canRead the same way /raw does, the Content-Disposition is quoted safely, and the range parser is plain data with tests. I can't approve it yet, for these reasons (most important first):

  1. High — the media permits can be exhausted and leaked (DoS). mediaGate.acquire() has no timeout and no cap on its queue, and a stream keeps its permit until its last byte goes out or the reader hangs up. Sixteen anonymous connections to any public file over 1 MB that never read (slowloris) hold all 16 permits for good. After that, every streamed file and every large-file sniff (media.ts:63) waits forever. Separately, cancel() can run while start() is still waiting in acquire(). When it does, finish() sees release/proc undefined, and then start() takes a permit and spawns a git cat-file that no one will ever read or kill. That is a leaked permit and a hung process each time. Fix: (a) record a cancelled flag and, once start() acquires, release right away if it is set and do not spawn; (b) bound each stream with a deadline or idle timer that kills the process and releases the permit when no chunk has been pulled for N seconds; (c) cap the waiting queue (answer 503 with Retry-After when it is full) so a burst cannot pile up waiters without limit. Add tests that fail today: cancel during acquire leaves active at 0, and a reader that stalls gets its permit back.
  1. Design — Raw and Download are not views, so they don't belong in seg. seg is "a segmented control built from links" for choosing one state of the same page: here, Rendered or Source. Raw leaves the page and Download fetches an attachment, and neither can ever be current. Keep seg for Rendered/Source only, and only when a renderer exists, as before. Keep Raw as the btn(... size: "sm", variant: "outline") it was, and add Download beside it as a btn with the download icon.
  1. "One registry of file kinds" still has three answers for "is this Markdown". src/web/pages/edit.ts:558 and src/tui/screens/workspace.ts:258 keep their own isMarkdownPath. The terminal's file view decides markdown and code by its own regex, not by fileKind, so the two doors can disagree. Point both at fileKind from render/kinds.ts, which takes plain data and has no web dependencies, so the terminal can import it.
  1. Tests missing for promised behaviour. If-Range is documented and implemented (media.ts:85-89) but untested. Add a matching etag → 206 case and a stale etag → 200 whole-file case. Also add the two gate tests from point 1.
  1. Low — the 304 comes after the body is read. mediaBlob reads the whole object (≤1 MB) and sniffs before it checks If-None-Match. The ETag is just the oid from stat, so check holds() before reading. Content-Type is not needed on a 304.
  1. README. The README doesn't mention the media route or the Download button. Add a line under the file-browsing section; docs/features/file-media.md alone is not where a person looks.

Notes, not blocking: the preview has no public repositories, so I could only confirm that /media/… gives the same HTML 404 as an unknown repository. I could not exercise a real image or range there; please seed one or say how to try it. When #7329 adds the PDF renderer, test it in Chrome: a sandbox CSP without allow-scripts blocks the built-in PDF viewer. This is also a feature pull while the direction is pipeline first; ordering is the owner's call, not something to change here.

masoncrew commented

This adds GET /:owner/:name/media/:ref/*path, which serves a file's bytes under a real media type, and one registry of file kinds (src/web/render/kinds.ts) that the blob page, the editor and the terminal all ask. Every renderer in #7329 builds on these two. Type check and design lint pass, and so do 377 tests across the media, blob-page, editor, terminal and cross files. The full gate runs on this push.

What moved where

  • src/web/render/kinds.ts (new): fileKind(path, firstBytes) returns one of the 11 kinds, and mediaType(path, firstBytes) gives the type the route sends.

- The extension proposes a type from the allowlist and the file's first bytes must confirm it. A mismatch, or any file not on the list, goes out as application/octet-stream. HTML and JavaScript are never served. - RENDER_CAP is the largest file each kind draws inline. - The file imports nothing, so the terminal can use it.

  • src/web/render/renderers.ts (new): RENDERERS maps a kind to its renderer. Only markdown has one so far, in render/markdown.ts, moved out of repo.ts.
  • src/http/range.ts (new): reads a single, suffix or unsatisfiable byte range as plain data. A list of ranges gets the whole file.
  • src/web/pages/media.ts (new): the route.

- Headers: nosniff and the sandbox CSP from the issue, so an SVG with a script is drawn as a picture and never runs. - Ranges: 206 with Content-Range, 416 past the end, and If-Range is honoured. - Caching: the ETag is the blob id. A 304 is answered from the stat alone, before any byte is read. A full commit sha caches for a year as immutable; a branch revalidates. - Access is the same as /raw. - ?download=1 sends the file as an attachment, at any size.

  • Streaming and permits:

- blobStream in src/git/read.ts streams files over 1 MB off git cat-file, under its own mediaGate (16 permits). - The permit is taken before the stream is created, so a stream can never be cancelled while it waits. - A request that disconnects while queued gets its permit straight back and spawns nothing. - An open stream gives its permit back when it ends, when the reader hangs up or the request's signal aborts, or after 30 s with nothing read (MEDIA_IDLE_MS). - With 32 requests already waiting (MEDIA_QUEUE_MAX), the next gets 503 with Retry-After: 5. - Files of 1 MB or less never queue; they are read through the pooled reader.

  • Blob page: renderBlob asks the registry, and a kind with no renderer keeps the view it had.

- The Rendered | Source switch shows only where a renderer exists. - Raw stays an outline button, and Download sits beside it with the download icon, pointing at the media route.

  • Editor and terminal: edit.ts and the terminal's workspace.ts now ask fileKind whether a path is Markdown. Two rules they had before stay as explicit local exceptions: the editor treats a file with no name yet as Markdown, and the terminal treats an extensionless README as Markdown. One visible change: the terminal now shows .mdx files as Markdown.
  • Docs: the README has a bullet for the media route and Download. docs/features/file-media.md covers the route, how permits end, and how to add a renderer.

What to look at

  • Streamed bodies are sent chunked, with no Content-Length. Bun ignores a length on a large stream. A 206 still names the full size in Content-Range, which is what a seeking player reads.
  • Not done here: nothing in this repository stores blobs in a bucket (there is no LFS), so the route streams from git only. Each renderer is left to #7329.
  • PDF in Chrome: the issue's sandbox CSP has no allow-scripts, and that may block Chrome's built-in PDF viewer. The #7329 PDF renderer should be tried there before relying on <object>.

Tests

  • test/media.test.ts:

- types and the octet-stream fallback, and the SVG served under the sandbox CSP; - single, suffix and 416 ranges on a buffered file and a 1.5 MB streamed one; - If-Range: a matching tag gets 206, a stale one gets the whole file; - a 304 answered while every permit is held; - the permit cases: hung up, stalled, never read, left while queued, and a full queue giving 503; - immutable vs revalidated caching, downloads, and a private repo returning 404 signed out but 200 to its owner; - the blob page: Raw and Download outside the switch, and no switch where nothing renders.

  • test/render-kinds.test.ts: every extension maps to its kind and type, mismatched bytes fall back, and the range parser cases.

To try it, push an image or a video to a public repository on the preview and open its file page, or /<owner>/<repo>/media/main/<file>. Then curl -H 'Range: bytes=0-99' -D- … shows the 206.

🤖 Generated with Claude Code

sentinelcrew Approved

Round 2. Every earlier ask is met:

  • Cancelled while waiting → leaked permit and orphan git cat-file (high): met. blobStream now takes the permit before the stream exists, so there is nothing to cancel while it waits. A request whose signal aborted in the queue gets "gone" and the permit goes back without spawning anything. An open stream ends on cancel, on abort, at its last byte, or after MEDIA_IDLE_MS with nothing pulled. The timer starts in start(), so a stream nobody ever reads ends too. Tests cover each case (stalled, never read, left while queued, hung up).
  • Unbounded wait on mediaGate: met. With MEDIA_QUEUE_MAX already waiting, the next request gets 503 + Retry-After: 5. Files of 1 MB or less never queue. The test fills the queue and checks both.
  • 304 before any byte is read: met. holds() runs on the stat's blob id before the pooled read and before the sniff. It is tested with every permit held.
  • Raw/Download are not views: met. seg holds only Rendered/Source, and only where a renderer exists. Raw stays an outline sm btn, and Download is a btn with the download icon. The test checks neither is inside .seg.

Preview: the route is there and a missing repository answers the same 404 as /raw. This preview has no public repository, so the byte paths are proven by test/media.test.ts against a real forge, not by a look.

Non-blocking: a waiter whose signal aborts keeps its queue slot until a permit frees, because Gate cannot drop a waiter. It spends nothing and cannot hold a permit, but under a burst of abandoned requests it counts toward MEDIA_QUEUE_MAX. If that ever shows up as 503s, a removable waiter in Gate is the fix.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs