Give audio and video their own write size ceilings - #5733
Conversation
Every non-card write was held to one 5 MB ceiling, which is well under the size of ordinary media: a 7 MB audio track uploaded to a realm came back 413 Payload Too Large. Make the file ceiling a function of the file's inferred content type — audio/* gets 20 MB, video/* gets 50 MB, everything else keeps 5 MB — and resolve it through inferContentType so the `.ts` override wins over mime-types' video/mp2t mapping. Both the realm's write path (which every binary upload and atomic source write funnel through) and the host's client-side precheck read the same resolver, so the two sides agree on which ceiling applies to a given path. The new ceilings are configurable via AUDIO_SIZE_LIMIT_BYTES and VIDEO_SIZE_LIMIT_BYTES, matching the existing CARD_/FILE_ pair, and are published to the host through the index-HTML config meta tag. CS-11335 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7df54ffa78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
isBinaryFilename decides whether a caller reads a file as a UTF-8 string or as raw bytes before uploading it. Video was absent from the list, so `boxel realm push` read an .mp4 with fs.readFile(..., 'utf8') and posted the decoded string — every invalid byte sequence replaced with U+FFFD. That mangling was previously masked for anything over 5 MB: the replacement characters inflate the payload, so the write was rejected as oversized before it could land. The video ceiling removes that accidental backstop, so classify video/* as binary and pin the invariant that any path granted a media ceiling is also carried as bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…file-size-limits # Conflicts: # packages/runtime-common/infer-content-type.ts
There was a problem hiding this comment.
Pull request overview
This PR makes realm write-size validation choose different maximum payload sizes for audio and video assets (20MB / 50MB) while keeping cards (512KB) and other files (5MB) on their existing ceilings. It threads the new limits through runtime-common, realm-server config/meta, and the host’s client-side precheck so both sides agree on which ceiling applies to a given path.
Changes:
- Introduces
fileSizeLimitFor()inruntime-commonto pick the correct file ceiling based oninferContentType(audio/video vs default). - Adds default audio/video limits and plumbs
AUDIO_SIZE_LIMIT_BYTES/VIDEO_SIZE_LIMIT_BYTESthrough realm-server and host configuration. - Expands binary classification to include
video/*and adds unit + endpoint coverage to pin size-limit routing and byte-carrying behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/write-size-validation.ts | Adds FileSizeLimits + fileSizeLimitFor() and uses inferContentType to select audio/video/default ceilings. |
| packages/runtime-common/realm.ts | Applies per-path file ceilings during server-side write-size checks. |
| packages/runtime-common/infer-content-type.ts | Treats video/* as binary so uploads are carried as bytes. |
| packages/runtime-common/index.ts | Re-exports new size-limit helpers/types. |
| packages/runtime-common/constants.ts | Defines default audio/video size ceilings. |
| packages/realm-server/tests/server-config-test.ts | Updates serve-index deps fixture to include audio/video limits. |
| packages/realm-server/tests/serve-index-test.ts | Updates serve-index deps fixture to include audio/video limits. |
| packages/realm-server/tests/index.ts | Registers new file-size limit test module. |
| packages/realm-server/tests/helpers/index.ts | Plumbs audio/video limit options/env defaults into test realm setup helpers. |
| packages/realm-server/tests/file-size-limit-test.ts | Adds unit tests for fileSizeLimitFor() and media↔bytes pairing via isBinaryFilename. |
| packages/realm-server/tests/card-source-endpoints-test.ts | Adds endpoint tests asserting 413 routing for audio/video vs default binary limits. |
| packages/realm-server/server.ts | Reads audio/video limit env vars and passes through serve-index deps. |
| packages/realm-server/main.ts | Reports audio/video limit values to the manager alongside existing limits. |
| packages/realm-server/handlers/serve-index.ts | Publishes audio/video limits via the existing config-meta injection into the host shell. |
| packages/host/config/environment.js | Adds audio/video limits to host build-time config/env parsing. |
| packages/host/app/services/environment-service.ts | Exposes audio/video limits via EnvironmentService. |
| packages/host/app/services/card-service.ts | Uses fileSizeLimitFor() for client-side size precheck of file writes. |
| packages/host/app/config/environment.ts | Extends host config typing to include audio/video limits. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two fixes from review of the media-aware write ceiling. fileSizeLimitFor returned the media limit unconditionally, so raising only FILE_SIZE_LIMIT_BYTES past 20/50 MB silently gave audio and video LESS headroom than a generic file of the same size — with the general limit at 100 MB, a .bin got 100 MB while a .mp3 got 21 MB. The media limits are a floor over the general one now, never a cap. The file-upload service posted the whole body and let the realm's 413 come back, which at a 50 MB ceiling means transferring and buffering a rejected payload in full. It now measures File.size against the same resolver before opening the request, so an over-limit pick fails immediately. This is what makes the audio/video limits published to the host load-bearing; they were only reachable from text-write prechecks before. validateWriteSize grows a validateByteLength sibling so a caller holding a length rather than the bytes gets the identical message. Fold the media endpoint tests into the existing binary size-limit module — the fixture template is copied onto disk rather than written through the realm, so a second module with different ceilings paid a full realm boot and index to produce a byte-identical template. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Settling for "left the picking state" passes on 'uploading', so the assertion held even if the upload then failed. Wait for complete-or-error and assert 'complete'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 27m 8s ⏱️ - 11m 16s Results for commit 55303eb. ± Comparison against earlier commit 7779fb9. Realm Server Test Results 1 files ±0 1 suites ±0 14m 51s ⏱️ +18s Results for commit 55303eb. ± Comparison against earlier commit 7779fb9. |
md5 is linear in content length and runs synchronously on the main thread, so a write's fingerprint cost scaled with the file: 30 ms at 5 MB, 126 ms at 20 MB, 282 ms at 50 MB. Raising the media ceilings put the large end of that within reach of an ordinary upload. Above 5 MB the fingerprint now samples instead of covering everything: byte length, md5 of the leading 4 MB, and md5 of the trailing 1 MB, for a fixed cost at any file size. Content at or below 5 MB is still hashed whole and keeps byte-identical values, so source, cards, images, and every source-cache ETag are untouched — only files large enough to be expensive change shape. The value is content-addressed, not merely a change stamp: it keys the upload dedupe cache in file-def-manager, decides whether a thumbnail still matches the bytes it was rendered from, and serves as a source ETag. A bare truncation would let two files sharing a prefix collide and silently resolve to each other's upload. Pinning length and both ends makes a collision require agreement on all three. The `s1:` marker keeps a sampled value from ever comparing equal to a stored whole one, so rows re-derive rather than compare across two schemes. Three call sites computed this independently and had to agree or the cross-layer verification in recacheContentHash would quietly stop caching. They now share one implementation in runtime-common. The file inspector labels a whole fingerprint "MD5" (it matches md5sum) and a sampled one "Checksum (sampled)", so nobody is invited to a comparison that cannot match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
card-api imported md5 directly, so super-fast-md5 was a static dependency of card-api's format templates and therefore of every card in every realm. Hashing now goes through runtime-common, which is already in that graph, so the module no longer loads per cold prerender or costs bytes per index row. The dependency-guard assertions record the smaller graph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| }; | ||
|
|
||
| // ETag base prefers a content fingerprint (md5 of the file body) over | ||
| // ETag base prefers a content fingerprint (derived from the file body) over |
There was a problem hiding this comment.
[Claude Code 🤖] This comment explains why the fingerprint displaces lastModified — and that reasoning stops holding for the files this PR is built to allow. A middle-only edit to a >5 MB media file now serves a 304 for changed bytes, with the mtime that would have caught it discarded. I'd fix this before merge; it's a one-line change.
The chain, verified end to end.
buildEtag(options?.etagBase ?? ref.lastModified, options?.etagVariant)— the??means a presentetagBasereplaces the timestamp rather than joining it.etagBaseis populated fromcontentHashFromMaterializedRef(cachedRef)(andcached.contentHashon the cache-hit path), and that helper callscomputeContentHashon whatever the ref materialized, with no size or content-type restriction.computeContentHashsamples above 5 MB, so for a >5 MB file the ETag base iss1:<len>:<md5 head 4MB>:<md5 tail 1MB>.serveLocalFilethen doesif (etag && request.headers.get('if-none-match') === etag) → 304, andifNoneMatchMatchesstripsW/, so weak-vs-strong offers no escape hatch.
Failure scenario. A 6 MB .mp4 in a realm. Something rewrites bytes strictly between offset 4 MB and len − 1 MB — an in-place metadata patch that preserves size, a tool that re-stamps a middle segment, a same-length re-render. Length unchanged, head unchanged, tail unchanged → same fingerprint → same ETag → every client holding the old copy gets a 304 and keeps serving stale bytes. I confirmed the collision empirically on a 6 MB buffer: flipping one byte at HEAD + 100 produces an identical hash, while flipping the last byte of the head or the first byte of the tail is detected.
An HTTP cache is the worst place for this to land, because unlike the dedupe cache it isn't process-scoped — a browser disk cache or any intermediary holds the stale answer indefinitely.
Why this is more than "a documented blind spot." The comment above documents the trade-off honestly. But it also records that lastModified was rejected as the ETag base because "the unix-second timestamp collides for two writes that land in the same second." So the two mechanisms fail in disjoint ways: mtime sees every write but can't resolve sub-second pairs; the sampled hash resolves any pair but can't see the middle. Before this PR, etagBase was a whole md5 — a total content identity — so displacing mtime lost nothing. Now, for exactly the >5 MB class this PR enables, displacing it removes the only signal that covers the new gap.
This also narrows a claim in the description. "Source, cards, images, and every source-cache ETag are completely untouched" is exactly right for source, cards, and images — each is held under its own ≤5 MB ceiling, so they're all whole-hashed — but media above 5 MB is served through this same path and does get a sampled ETag. That isn't an edge case; it's the new normal for the files being unblocked.
The fix. Combine the two rather than choosing, which keeps the sub-second protection this comment was written for and closes the sampled gap:
// A sampled fingerprint (see computeContentHash) cannot see a large file's
// middle, so pair it with lastModified: the hash resolves writes within the
// same second, and the timestamp covers a middle-only change the hash misses.
let base = options?.etagBase;
if (base != null && ref.lastModified != null && isSampledContentHash(String(base))) {
base = `${base}:${ref.lastModified}`;
}
let etag = buildEtag(base ?? ref.lastModified, options?.etagVariant);The only remaining hole is a middle-only edit landing in the same unix second as the write before it, which is a genuinely negligible intersection of the two failure modes. Cost: an mtime-only touch of a large media file busts its cache — strictly the safe direction, and it's the behavior every file with no fingerprint already has.
If you'd rather not interleave, the smaller version is to drop etagBase when it's sampled and fall through to lastModified. That's also safe, just gives up sub-second resolution for large media specifically.
While here: this comment is the natural home for the invariant, so it's worth saying that etagBase must be a total identity of the body, and naming the sampled case as the exception. Otherwise the next person to add an etagBase producer has no way to know the rule.
Scope: regression introduced by this PR — the sampling is new, and this consumer treats the value as total. Blocking-ish: I wouldn't ship stale-content 304s on media, but the fix is small enough that it's your call whether it lands here or immediately after.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 7779fb9b96 — but the reachability is narrower than described, and the difference matters for how alarming this is.
Media does not reach this path. getSourceOrRedirect computes bypassCache = noCache || (!pathname.endsWith('.json') && !hasExecutableExtension(pathname)), and executableExtensions is ['.js','.gjs','.ts','.gts']. So for any media extension bypassCache is true, and that branch calls serveLocalFile with no etagBase at all:
if (bypassCache) {
return await this.serveLocalFile(request, handle, requestContext, {
defaultHeaders,
etagVariant: SOURCE_ETAG_VARIANT, // no etagBase
});
} else {
... etagBase: contentHash
}A .mp4 therefore still gets a lastModified-based ETag, exactly as before this PR. Step 2 of your chain is right about contentHashFromMaterializedRef having no size or content-type restriction, but the call site is gated behind bypassCache === false, which excludes media by extension.
What is reachable. Only .json and the four executable extensions get a contentHash base, and those sit under ceilings at or below the whole-hash threshold — so they are always whole-hashed today. The hole opens if FILE_SIZE_LIMIT_BYTES is configured above 5 MB, since a >5 MB .gts or non-card .json would then get a sampled base. Not media, but real, and the floor-not-cap change makes raising that knob more attractive.
Fixed anyway, your way. The invariant shouldn't hold by accident of the bypassCache gate — a future etagBase producer has no way to know the rule. A sampled base is now joined with lastModified instead of replacing it:
function totalEtagBase(etagBase, lastModified) {
if (etagBase == null) return undefined;
if (lastModified == null || !isSampledContentHash(etagBase)) return etagBase;
return `${etagBase}:${lastModified}`;
}I took the joining version over dropping the base, for the reason you gave: it keeps the sub-second resolution the original comment was written for. And I stated the invariant in that comment — an etagBase must be a total identity of the body, with the sampled case named as the exception — so the next producer sees the rule.
You're also right that the description overreached. It now says source, cards, and images are untouched because each is capped at or below the threshold, rather than implying ETags are wholly in the whole-hash regime.
| if (bytes.length <= CONTENT_HASH_WHOLE_LIMIT_BYTES) { | ||
| return md5(bytes); | ||
| } | ||
| // subarray shares the underlying buffer without copying, and md5 honors a | ||
| // view's offset and length rather than reading the whole buffer. | ||
| let head = md5(bytes.subarray(0, CONTENT_HASH_HEAD_BYTES)); | ||
| let tail = md5(bytes.subarray(bytes.length - CONTENT_HASH_TAIL_BYTES)); | ||
| return `${SAMPLED_MARKER}:${bytes.length}:${head}:${tail}`; |
There was a problem hiding this comment.
[Claude Code 🤖] The upload dedupe cache reads this value as "these are the same bytes" and acts on it by not uploading — so a sampled collision doesn't degrade, it substitutes content. Non-blocking, but worth an explicit decision rather than inheriting the general trade-off.
Mechanism. FileDefManager#uploadContentWithCaching is:
const cachedUrl = await this.getCachedUrlForContent(content);
if (cachedUrl) {
return cachedUrl; // ← no upload happens
}and getCachedUrlForContent keys contentHashCache on computeContentHash(content). So on a hash match the caller receives the previously uploaded file's URL. For two large media files that agree on length, first 4 MB, and last 1 MB but differ in the middle, the second attach silently resolves to the first one's bytes — the card ends up pointing at content the user didn't pick, with no error anywhere.
How likely, honestly. Pinning the length is what makes this remote: almost any real edit changes the byte count, and then the s1:<len>: prefix diverges immediately. What survives the length pin is the same-size class — an in-place metadata rewrite, a fixed-width middle field, two outputs of one pipeline differing only in a padded region. Not common. But the description already names media as the worst case for prefix sharing, and "same container, same intro, same trailer, different middle" is the shape media actually takes.
Why I'd treat this consumer differently from the other two. The three consumers have very different costs for a false equality:
| consumer | false-equality outcome |
|---|---|
| upload dedupe | wrong bytes served under a card's file, silently |
| source/file ETag | stale 304 — see the buildEtag thread |
| thumbnail freshness | stale thumbnail; cosmetic |
And the cost of being conservative is asymmetric in the opposite direction: a dedupe miss is one redundant upload of a large file. Slow, bounded, self-correcting. A false hit is silent and wrong.
Two ways out.
Cheapest: make a sampled hash a non-authoritative key for dedupe only — getCachedUrlForContent returns null when isSampledContentHash(hash), and uploadContentWithCaching skips the set. Large media re-uploads each time; everything ≤5 MB (all source, cards, images) keeps the existing dedupe exactly. Three lines, and it leaves the fingerprint itself alone.
Better fingerprint: add a bounded strided sample across the middle — say 64 KB at eight evenly spaced offsets — folded into the hash. Cost stays fixed and small (that's another 512 KB hashed, well inside the current budget), the blind spot stops being a contiguous region, and dedupe keeps working for large media. This is the option I'd pick if dedupe on large media is worth keeping, and it also shrinks the ETag exposure in the other thread.
Either way, the s1 marker is already the right piece of infrastructure for this: bumping to s2 when the sampling shape changes means existing rows re-derive instead of comparing across schemes. Good call including a version from the start.
Smaller thing, same function. The catch fallback a few lines up returns md5(String(content)) — a whole hash, with no size check and no sampled marker. It's near-unreachable (toBytes only throws for a string, on allocation failure, and String(content) on a string is identity, so the fallback would hit the same wall), but as written it's the one path that can produce a whole-content hash of arbitrarily large content — i.e. the exact stall this function exists to bound, in the error path. Worth either routing it through the same size check or dropping it, since a fallback that can't succeed is mostly a place for a future reader to get the wrong idea about the invariant.
Scope: regression introduced by this PR for the dedupe consumer; non-blocking, but the decision should be deliberate.
There was a problem hiding this comment.
[Claude Code 🤖] Took the conservative option — fixed in 7779fb9b96.
Your asymmetry argument is the decisive one: a dedupe miss is one redundant upload, bounded and self-correcting, while a false hit silently substitutes bytes under a card. That isn't a trade worth making for a cache, so a sampled fingerprint is no longer an admissible key at all — getCachedUrlForContent returns null for one, and neither uploadContentWithCaching nor recacheContentHash will store under one. Everything at or below the threshold — all source, cards, images — keeps the existing dedupe byte for byte.
I added the recacheContentHash guard too, which your sketch didn't mention: it populates the same cache from a FileDef's contentHash attribute, so leaving it open would have let a sampled value in through the back door.
On the strided-middle alternative. I considered it and chose not to. It shrinks the blind spot but cannot make the fingerprint total, so both silent-and-wrong consumers would still need their own guard — the ETag one needs lastModified joined regardless, and dedupe would still be trusting a probabilistic key for exactly the content class where a false hit costs most. Once both are guarded, the extra sampling buys only better dedupe hit rates on large media, at the cost of a more complex fingerprint and a wider blind-spot argument to reason about. If large-media dedupe turns out to matter, that's the right change to make then, and the s1 → s2 bump is what makes it safe.
The catch fallback: dropped rather than routed. You're right that it was the one path able to produce a whole hash of arbitrarily large content, and right that it was near-unreachable. Routing it through the size check would have preserved a branch that can't succeed; the encode failure it guarded is an allocation failure on a string, where String(content) is identity and hits the same wall. So it's gone, with a comment saying why — a fallback that can't work is mostly a place to acquire a wrong idea about the invariant.
| // A content fingerprint used to tell one file's bytes from another's: it keys | ||
| // the upload dedupe cache, decides whether a thumbnail still matches the file | ||
| // it was rendered from, and serves as a source ETag. | ||
| // | ||
| // md5 is linear in content length and runs synchronously on the main thread, | ||
| // so hashing whole files makes a single large write a multi-hundred-millisecond | ||
| // stall. Above `CONTENT_HASH_WHOLE_LIMIT_BYTES` the fingerprint is sampled | ||
| // instead: the byte length plus a hash of the head and a hash of the tail, for | ||
| // a fixed cost no matter how large the file is. Two files collide only by | ||
| // matching on all three, which the whole-content hash of a same-length file | ||
| // with identical ends would not have distinguished cheaply either. | ||
| // | ||
| // Content at or below the limit is hashed whole, so the common case — source, | ||
| // cards, images, anything the source cache ETags — keeps exactly the md5 it | ||
| // has always had, and only files large enough to be expensive change shape. |
There was a problem hiding this comment.
[Claude Code 🤖] Confirmation — the two claims this comment makes that a reader can't check locally both hold. Nothing to change here; two additions worth making.
I checked these rather than trusting them because each one silently degrades the whole scheme if it's wrong, and neither is visible from this file.
"md5 honors a view's offset and length rather than reading the whole buffer." True. On a 64-byte buffer filled 0xAA then 0xBB, hashing subarray(0,32) and subarray(32) each matches an independent Uint8Array.from(...) copy, and neither equals the whole-buffer hash:
subarray(0,32) baf72a5be99c3271cada410bd0e19bf6 == copy
subarray(32) f0e98d5c2fb460a90d3e6d9098f331f4 == copy
head===tail ? false tail===whole ? false
Worth having verified: had super-fast-md5 normalized its input via input.buffer, head and tail would both have hashed the entire file and come out equal — you'd have paid full cost and gotten a fingerprint with less information than a plain truncation, and every test in content-hash-test.ts that compares against md5(huge.subarray(...)) would still have passed, because both sides would be wrong identically.
"a fixed cost no matter how large the file is." True, measured:
5 MB -> 10.6 ms (whole)
6 MB -> 7.5 ms (sampled)
20 MB -> 7.5 ms
50 MB -> 8.5 ms
200 MB -> 8.4 ms
Flat across a 33× size range. Also worth noting the sampled path at 6 MB is no more expensive than the whole path at 5 MB, which follows from head+tail summing to the threshold — so there's no cost cliff at the boundary in either direction.
Addition 1: say that HEAD + TAIL === WHOLE_LIMIT is deliberate. It is, arithmetically — 4 MB + 1 MB = 5 MB — and it's load-bearing: it means the unhashed middle opens at zero width exactly at the threshold and grows linearly from there, rather than a 5.001 MB file suddenly having a large hole. Someone tuning these three constants later (raising the whole-limit, shrinking the head) can break that relationship without any test noticing, since no test asserts it. One clause here, or a // invariant: HEAD + TAIL === WHOLE_LIMIT next to the constants, would pin the intent.
Addition 2: the consumer list needs one qualification. This comment and the description both say the ETag case is untouched. That's exact for source, cards, and images — each is capped at ≤5 MB by its own ceiling, so all three are whole-hashed and byte-identical to before. But media above 5 MB is served through the same source path and does get a sampled ETag, which is a live staleness path — details in the thread on buildEtag in realm.ts. Since this comment is where a future reader learns what the value is used for, it's the right place to say "the ETag consumer sees sampled values for media" rather than leaving the impression that ETags are entirely in the whole-hash regime.
The reasoning about why a plain truncation would be worse — shared container headers, codec init segments, common intros — is the right argument and lands well. Pinning both ends plus the length is a real improvement over any single-window scheme.
Scope: confirmation, plus two comment additions. Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Both additions made in 7779fb9b96. Thanks for actually checking the two claims rather than taking them — and your point about why the view-offset one matters is sharper than my comment was: if super-fast-md5 had normalized through input.buffer, head and tail would both have hashed the whole file and come out equal, so I'd have paid full cost for a fingerprint weaker than a plain truncation, and every assertion comparing against md5(huge.subarray(...)) would have passed while both sides were wrong identically. That's a failure mode no test in the file could have caught, which is exactly why it was worth verifying independently.
Addition 1 — the invariant is now stated next to the constants, and asserted. Two tests: one that HEAD + TAIL === WHOLE_LIMIT, and one that pins the consequence you identified — at one byte over the limit the blind spot is exactly one byte wide (index HEAD), with both neighbours covered. That second one is the more useful guard, since it fails if someone retunes the constants in a way that opens a hole at the threshold. Worth noting my first attempt at it was wrong in your direction: I wrote it as "zero-width blind spot", but at WHOLE_LIMIT + 1 the head covers [0, HEAD) and the tail [HEAD+1, len), so index HEAD is uncovered. Width 1, not 0.
Addition 2 — the consumer list is qualified, and I went further than a note about ETags. The comment now names how each consumer treats a sampled value, because they turned out not to be uniform: dedupe refuses them, the ETag joins them with lastModified, thumbnails accept them. That list is where the differing tolerance for a false equality belongs.
One correction to the ETag part of your note, detail in the buildEtag thread: media doesn't get a sampled ETag today, because a non-.json/non-executable GET takes the bypassCache branch that passes no etagBase. The exposure is .json and executable files above the threshold, which needs the general file limit raised. I fixed it regardless.
| export function fileSizeLimitFor(path: string, limits: FileSizeLimits): number { | ||
| let contentType = inferContentType(path.split(/[?#]/)[0]); | ||
| if (contentType.startsWith('audio/')) { | ||
| return Math.max(limits.default, limits.audio); | ||
| } | ||
| if (contentType.startsWith('video/')) { | ||
| return Math.max(limits.default, limits.video); | ||
| } | ||
| return limits.default; |
There was a problem hiding this comment.
[Claude Code 🤖] Math.max propagates NaN, so a typo'd env var doesn't fall back to the general limit — it removes the ceiling for media entirely, which is the opposite of the floor invariant documented directly above. Non-blocking; one guard, and it's independent of the env-var validation you've scoped out.
Mechanism. Math.max(5 * 1024 * 1024, NaN) is NaN, and size > NaN is false, so validateByteLength never throws. Both realm-server (Number(process.env.AUDIO_SIZE_LIMIT_BYTES ?? DEFAULT)) and the host build config produce NaN for a value like 20MB, so AUDIO_SIZE_LIMIT_BYTES=20MB yields no audio ceiling on either side — server and client agree, and both agree on "unlimited."
I agree the underlying env-var parsing is pre-existing and belongs in its own change. What's specific to this function is that the comment three lines up states a guarantee — "The media limits are a floor over the general one, never a cap" — and Math.max is what implements it. With a non-finite input it silently implements the opposite. A NaN audio limit should degrade to the general 5 MB ceiling, which is exactly what the floor semantics promise, and the function can enforce that on its own without knowing where the number came from:
function floorOverDefault(defaultLimit: number, mediaLimit: number): number {
// A non-finite media limit (a mis-parsed env var) must degrade to the general
// ceiling: Math.max would propagate NaN, and `size > NaN` is false, so the
// floor would silently become no limit at all.
return Number.isFinite(mediaLimit)
? Math.max(defaultLimit, mediaLimit)
: defaultLimit;
}That also makes the failure visible, in the useful sense: an operator who typos the audio knob gets 5 MB media uploads rejected and goes looking, instead of getting an unbounded ceiling and never knowing. limits.default itself being NaN is still uncovered by this — that one really is the pre-existing knob and belongs with the validation change.
Confirmation on the rest of this function, since it's the part everything else keys off. The .ts concern is not hypothetical — against the pinned mime-types, lookup('a.ts') returns video/mp2t, so without CONTENT_TYPE_OVERRIDES every TypeScript module in a realm would resolve to the video branch and inherit a 50 MB ceiling. a.gts and a.gjs return nothing from mime-db and are also overridden, so all three land on textual types. Routing through inferContentType rather than a bare mime lookup is therefore doing real work, and the comment's framing — sized by the same identity it's served under — is the right way to describe why.
The query/fragment trim is also correctly reasoned: RealmPaths.local decodes with decodeURI, which leaves %3F/%23 escaped, so a literal ? or # in a filename can't be mistaken for a delimiter here. Good detail to have written down — it's the kind of thing that looks like an over-cautious split until you know why.
Scope: regression introduced by this PR (new code, new invariant), non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Real bug, fixed in 7779fb9b96 with your guard.
Confirmed the mechanism before changing it: Math.max(5242880, NaN) is NaN, and size > NaN is false, so validateByteLength never throws — AUDIO_SIZE_LIMIT_BYTES=20MB yields no audio ceiling on either side. Server and host agree, and both agree on "unlimited". The comment three lines above promises a floor, and Math.max was quietly delivering the opposite.
floorOverDefault now degrades a non-finite media limit to the general ceiling, so a typo rejects oversized media and gets noticed rather than silently accepting everything. A test covers both NaN and Infinity — Infinity matters too, since it's finite-looking in the sense of not being NaN but Number.isFinite correctly rejects it, and it would otherwise also mean no ceiling.
Your framing is what convinced me this is separable from the env-var work I'd scoped out: the parsing is pre-existing and belongs in its own change, but this function documents a guarantee and is the thing that implements it, so it can enforce it without knowing where the number came from. limits.default being NaN is still uncovered and still belongs with the validation change.
Thanks for the two confirmations as well — the .ts → video/mp2t check and the decodeURI reasoning behind the query trim. The latter is the one I most expected to read as an over-cautious split later, so having it independently verified is worth more than the comment I left.
…file-size-limits # Conflicts: # packages/host/tests/helpers/index.gts
A sampled fingerprint answers "probably the same bytes". Two consumers were treating it as "definitely the same bytes", where a false equality is silent rather than merely degraded. The upload dedupe cache acts on a match by skipping the upload and returning the earlier file's URL, so a collision would attach content the user never picked. It now refuses sampled values as keys — on either the upload path or the recache path — and large media re-uploads instead. A miss costs one redundant upload; a false hit is wrong and invisible. The source ETag replaced lastModified with the fingerprint, on the grounds that a whole md5 is a total identity and the timestamp is not. That reasoning does not survive sampling, so a sampled base is now joined with lastModified rather than replacing it: the hash separates two writes inside one second, and the timestamp catches a same-length middle-only edit the hash cannot see. Media does not reach this path today — a non-.json, non-executable GET takes the bypassCache branch, which passes no etagBase at all — but the invariant now holds in the code rather than by accident of that gate, which matters as soon as the general file limit is configured above the whole-hash threshold. fileSizeLimitFor used Math.max to implement "the media limits are a floor, never a cap". Math.max propagates NaN, and `size > NaN` is false, so a mis-parsed AUDIO_SIZE_LIMIT_BYTES removed the media ceiling entirely — the exact inverse of the documented guarantee. A non-finite media limit now degrades to the general ceiling, so a typo rejects oversized media and gets noticed instead of accepting everything silently. Also drop computeContentHash's encode-failure fallback: it hashed the content whole with no size check and no marker, so the one path that could produce an unbounded synchronous stall was the error path, under a value that reads as a whole-content identity. Record that HEAD + TAIL === WHOLE_LIMIT is deliberate — it is what makes the unhashed middle open at one byte wide rather than jumping to a large hole — and name, on the fingerprint itself, how each consumer treats a sampled value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…file-size-limits # Conflicts: # packages/host/tests/helpers/interact-submode-setup.gts
Background
Every write into a realm passes a size check before the bytes hit disk. There are two ceilings: cards get a small one (512 KB — a card is a JSON document, and a card that big is almost always a mistake), and everything else — modules, images, fonts, PDFs, binaries — shares a single 5 MB file ceiling. Over the limit, the realm answers
413 Payload Too Large.5 MB is a reasonable ceiling for source and images. It is not a reasonable ceiling for media. A seven-minute podcast episode at a normal bitrate is comfortably past it, and so a user attaching an ordinary audio track to a card got a 413 for a file that was in no way unusual. Video is worse still: 5 MB buys a few seconds.
What this does
The file ceiling becomes a function of what kind of file is being written, rather than one number for all of them:
.jsoncard documents)The kind is read off the path's extension through
inferContentType— the same inference the realm uses when it decides what content type to serve a file with, so a file is sized by the same identity it is served under. That matters concretely for.ts: a bare mime-types lookup resolves it tovideo/mp2t, which would silently hand every TypeScript module in a realm the video ceiling.inferContentTypeoverrides.ts(and.gts/.gjs) to their textual types, so modules stay on the general file limit.The media numbers are a floor over the general limit, never a cap on it. They exist to give media more room, so an operator who raises
FILE_SIZE_LIMIT_BYTESto 100 MB gets 100 MB for audio and video too, rather than having media singled out for a smaller ceiling than a.binof the same size.The realm resolves this on every write, so it covers both paths a file can arrive on: the octet-stream POST that binary uploads use (the host's file chooser,
boxel realm push's per-file binary upload), and the/_atomicbatch that text writes use.Both ceilings are configurable —
AUDIO_SIZE_LIMIT_BYTESandVIDEO_SIZE_LIMIT_BYTES, matching the existingCARD_SIZE_LIMIT_BYTES/FILE_SIZE_LIMIT_BYTESpair — and are published to the host through the index-HTML config meta tag, so the realm-server's runtime environment is the single place they're set.Rejecting before the upload, not after
The realm's 413 only arrives after the whole body has crossed the wire and been buffered server-side. At a 50 MB ceiling that's an expensive way to say no, and it invites someone to spend minutes uploading a 300 MB video before finding out. The file-upload service now measures
File.sizeagainst the same resolver before it opens the request, so an over-limit pick fails instantly with the same message the server would have produced.That is also what makes the audio and video limits published to the host load-bearing. The host's other size checks guard text writes, which never carry media, so without this the new config would have been plumbing that nothing read.
Carrying media as bytes
Callers that read a file before uploading it choose text or bytes from
isBinaryFilename. A file read as text is UTF-8 decoded, replacing every invalid byte sequence with U+FFFD — which inflates the payload, so oversized media used to be rejected before the mangled bytes could land. A larger ceiling removes that accidental backstop, which means the classification has to be right for the ceiling to be safe. Video reaches the binary side through the unknown-is-binary default in the content-type classifier. A unit test pins the pairing directly: every path granted a media ceiling is also carried as bytes, so an extension added to one side but not the other fails.Bounding the fingerprint cost
Every write computes a content fingerprint, and md5 is linear in length and runs synchronously on the main thread. Measured locally, that was 30 ms at 5 MB, 126 ms at 20 MB, 282 ms at 50 MB — so the new ceilings put a quarter-second event-loop block within reach of one ordinary upload, on a service that already has a 25 s worst-observed stall on record.
Above 5 MB the fingerprint now samples rather than covering everything: the byte length, an md5 of the leading 4 MB, and an md5 of the trailing 1 MB. Cost is fixed at any file size. Content at or below 5 MB is still hashed whole and keeps byte-identical values — and since cards, source, and images are each capped at or below that threshold, all three keep exactly the md5 they had. Only files large enough to have been expensive change shape.
The care here is because this value is content-addressed, not just a change stamp. It keys the upload dedupe cache (a hash match returns the previously uploaded URL), decides whether a thumbnail still matches the bytes it was rendered from, and serves as a source ETag. A plain truncation would let two files sharing a prefix collide and silently resolve to each other's upload — and media is the worst case for that, since a shared container header, codec init segment, or common intro is routine. Pinning the length and both ends means a collision has to agree on all three.
The
s1:marker keeps a sampled value from ever comparing equal to a stored whole one, so existing rows re-derive instead of silently comparing across two schemes. Three call sites used to compute this independently and had to stay in agreement — otherwise the cross-layer check inrecacheContentHashquietly stops caching rather than erroring — so they now share one implementation. The file inspector labels a whole fingerprintMD5(it still matchesmd5sum) and a sampled oneChecksum (sampled), so nobody is invited to a comparison that cannot succeed.Because a sampled value answers "probably the same bytes" rather than "definitely", its consumers don't get to treat it uniformly, and the fingerprint itself now documents the difference:
lastModifiedwith the fingerprint, on the grounds that a whole md5 is a total identity while a unix-second timestamp collides for two writes in the same second. That reasoning doesn't survive sampling, so a sampled base is now joined withlastModified: the hash separates writes inside one second, the timestamp catches a same-length middle-only edit the hash can't see. Media doesn't reach this path today — a non-.json, non-executable GET takes thebypassCachebranch, which passes no ETag base at all — but the invariant now holds in the code rather than by accident of that gate, which starts to matter once the general file limit is configured above the threshold.HEAD + TAIL === WHOLE_LIMITis deliberate and now asserted: it makes the unhashed middle open at exactly one byte wide just past the threshold and widen from there, instead of a file a byte over the limit suddenly carrying a large hole.The accepted trade is that a change confined to the middle of a large file is invisible to the fingerprint. There is a test asserting exactly that, so the boundary is documented rather than discovered.
Worth knowing
Raising the ceiling scales two costs that were previously bounded at 5 MB. Neither is introduced here, but this is what makes the larger numbers reachable:
Requestand again byarrayBuffer()— roughly 2–3× the payload resident per in-flight upload. The client-side precheck above removes the common case, but not a hand-rolled request..json.Deliberately not in scope, worth their own change:
.m4b(audiobook),.m2ts(AVCHD),.divx,.mxf, and friends resolve toapplication/octet-streamand so keep the 5 MB limit. They were capped at 5 MB before this change too — this doesn't regress them, it just doesn't reach them. Closing the gap means teachinginferContentType, which also fixes how those files are served..mtsis a latent landmine.mime-types2.1.35 (the pinned version, and the one bothruntime-commonand the host resolve) maps it tomodel/vnd.mts, so it lands on the general limit today. A bump to 3.x maps it tovideo/mp2t, at which point every.mtsmodule would inherit the video ceiling — the same trap.tsalready has an override for.Number(process.env.X ?? DEFAULT)turnsAUDIO_SIZE_LIMIT_BYTES=20MBintoNaN, andsize > NaNisfalse. The media limits now degrade a non-finite value to the general ceiling — so a typo rejects oversized media and gets noticed, rather than removing the ceiling entirely — butCARD_/FILE_themselves are still unvalidated, and aNaNgeneral limit remains uncovered. That parsing is pre-existing and belongs in its own change.Image, font, and document ceilings are untouched at 5 MB.
Testing
A unit test pins the limit resolution: audio and video extensions route to their own ceilings, non-media (source, JSON, images, fonts, PDFs, extensionless files) route to the general one,
.tsis TypeScript rather than an MPEG transport stream, query strings and fragments don't mask the extension, a general limit raised above the media limits lifts media with it, and every path granted a media ceiling is also classified as binary.Endpoint tests upload real bodies through the binary POST path against a realm configured with small stand-in ceilings, and assert the routing end to end: an audio body over the general file limit is accepted while one over the audio limit is rejected with a 413; a video body over the audio limit is accepted while one over the video limit is rejected; a payload sitting exactly on the ceiling is accepted; and a non-media binary is still held to the general file limit. Each case fails if the resolver picks either of the other two branches.
Unit tests pin the fingerprint: content at or below the whole-hash limit keeps its exact plain md5, a sampled value is self-describing and can never equal a stored whole one, changes to the head, the tail, and the length are each detected, the sampled middle is asserted as a known blind spot, and the hash is proven to be a function of exactly head, tail, and length — which is what bounds the cost, without a timing assertion that would flake under CI load.
Acceptance tests cover the client-side precheck: audio over the general file limit uploads without a size error where a
.binof the same size would not, and audio over the audio limit is rejected immediately with a message naming that ceiling.