Repository navigation
fix(files): serve the Claude scratch fallback only from what was opened in its folder (#2259) - #2265
Conversation
|
Review findings and how each was resolved (this PR touches an HTTP contract and file-access confinement, so the code-review agents' reviews are recorded here; CodeRabbit's will be added if it runs). omo-native-code-reviewer on 346a36c: FAIL
Codex Sol 6.1 Fast xhigh
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Identify scratch-folder matches server/claude-store.ts, server/file-view.ts, server/file-routes.ts, server/file-view.test.ts |
Shared Claude temporary roots identify possible scratch paths. Lookup returns matching files with their folder boundary, and the route passes that boundary to file serving. |
Authorize and serve files within the folder server/file-view.ts, server/file-tasks.ts, server/file-view.test.ts, changelog.d/scratch-folder-open-then-check.md |
Constrained file tasks authorize opened descriptors within the folder. Rendering and responses receive the folder boundary. Fast responses use the authorized descriptor when available. Tests cover replacement links, content, and whole-file, range, and HEAD responses. |
Track and release opened descriptors server/file-tasks.ts, server/file-pool.ts, server/file-worker.fake.ts, server/file-pool.test.ts, server/file-view.test.ts |
Describe and open tasks record opened descriptors in shared cells. Abandoned tasks close recorded descriptors. Tests check descriptor cleanup after worker replacement and during authorization. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- devswha/ovis#2259: Describes the scratch-folder containment race addressed by authorizing and serving the opened file within the folder.
Possibly related PRs
- devswha/ovis#1980: Changes the lookup contract that this PR extends with folder-bound results.
- devswha/ovis#1696: Adds the file-route flow that this PR updates to pass scratch-folder predicates and folder boundaries.
- devswha/ovis#1927: Covers descriptor-bound serving for rendered pages, which this PR also applies to scratch-folder fallback files.
Suggested reviewers: jensr93
Merge Risk: 
e0da0
-
No actionable issue was established in the scratch-folder containment change; it is mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 76.47% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the primary change: limiting Claude scratch-folder fallback responses to the file opened within that folder. Description check The description explains the problem, implementation, scope, known limits, linked issue, and detailed validation results. It uses "## Verification" instead of the template's "## Validation" heading, b… Linked Issues check [ #2259] requires the Claude scratch-folder fallback to open the file, authorize the opened descriptor, and serve only from that descriptor. Verified at head:openAuthorized(server/file-view.ts) now…Out of Scope Changes check Every changed area maps to #2259.openAuthorized/openedWithin/rootOfgainhidden,cell, and exported types so the fallback can use the page-asset check.describeandopen/renderfile t…Full details: Description check
-
Explanation
The description explains the problem, implementation, scope, known limits, linked issue, and detailed validation results. It uses "## Verification" instead of the template's "## Validation" heading, but it provides the required validation information and is otherwise complete.
Full details: Linked Issues check
-
Explanation
[
#2259] requires the Claude scratch-folder fallback to open the file, authorize the opened descriptor, and serve only from that descriptor. Verified at head:openAuthorized(server/file-view.ts) now takeshiddenandcellparameters. It opens the file, then proves the descriptor lies under the root (/proc/self/fd/Non Linux, owned-folder check on macOS).describeWithinderives size, mtime, and text/binary sniff from that descriptor (kindOfreadsopened).locateFileInFoldernow returns adescriberesult withwithin, andfileResponsetakeswithinand opens throughopenAuthorized. The PR reports that the small body is read from the descriptor, the large Linux fast-disk body is sent from/proc/self/fd/N(same inode as the authorized descriptor), and other bodies stream from the descriptor. Rendered pages are confined bypageWithLocalFiles(..., within), which checksopenedWithinon the page descriptor. The viewer's exact-absolute-path request is confined byfindFilewithmayBeScratch. Regression tests are reported as failing on#2256and passing here. Remaining uncertainty: thefileResponsebody of the diff was truncated in my read, so the fast-disk branch was checked only through the PR description and summary, not the final code lines. The previous PASS is still supported by the changed code.
Full details: Out of Scope Changes check
-
Explanation
Every changed area maps to
#2259.openAuthorized/openedWithin/rootOfgainhidden,cell, and exported types so the fallback can use the page-asset check.describeandopen/renderfile tasks in server/file-tasks.ts and server/file-pool.ts (cell,abandon) carry the descriptor from the thread to the caller, which is needed for mid-check cleanup.findFile'smayBeScratchandclaudeTempBases(server/claude-store.ts, used by server/file-routes.ts) keep the viewer's second request, which uses an exact absolute path, inside the same folder confinement. The behavior changes (a scratch symlink leading out is refused by its full path; a(deleted)name and a non-hidden scratch subfolder are refused) follow the fallback's existing rules. The fake-worker and FIFO test changes (server/file-worker.fake.ts, server/file-pool.test.ts) support the regression tests for these paths. The changelog entry documents the fix. Direct links outside the scratch folder keep their existing behavior, so the PR does not widen unrelated behavior.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 76.47% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 17 functions across 8 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…ed in its folder (#2259) locateFileInFolder checked the scratch folder's containment on a path, and the file thread's open then followed that path again (and a large body was sent from the path by Bun.file), so a symlink put in place of the file in between served a file outside the folder. The viewer widened the window: it stats by name, then fetches by the absolute path that answered, which matched exactly and was served unconfined. Open first, then check what was opened (openAuthorized: O_NOFOLLOW, then the descriptor's kernel path on Linux, owned folders on macOS), on the file thread: - the 'describe' task describes a fallback file from that descriptor and returns the folder as 'within'; - the 'open' task, given 'within', opens through openAuthorized (naming the descriptor in the pool's cell at once, and taking a refused one back before closing it) and never sends the body from the path; - the 'render' task draws a page found there only when the page it opened lies in the folder; - findFile confines an exact path that lies in the pane's scratch folder (claudeTempBases tells such a path before herdr is asked). Direct file links outside the scratch folder keep opening any readable file.
… form (#2259) A path in the pane's scratch folder with no exact match fell through to the any-form lookup that is not held to the folder, so a link put there under the other Unicode form of the file's name (NFD for NFC) was served from outside (omo review MEDIUM on the port). Such a path is now found only through locateFileInFolder, exact and then in any form, and never past it; the scratch folder is asked for once per lookup. describeWithin no longer opens the path again through fileInfo: the type comes from the name, the rest from the descriptor.
…read is ended (#2259) The describe task opened the scratch file through openAuthorized without the pool's cell, so a thread ended inside the check left it open (Codex review MEDIUM). describe now names its descriptor in a cell as open does, takes it back before closing it, and abandon closes it once an ended thread is gone. The name-form test covers both swap directions (NFD for NFC and back).
… waits (#2259) The fake thread said it ran before it opened the FIFO, so a test that ended it in between then blocked forever opening the FIFO's writer with no reader (Codex review MEDIUM, reproduced on one core). The thread now opens the FIFO read and write before it says it ran and blocks in a read; the test releases it with a non-blocking write.
5cc3aa9 to
cd6cb60
Compare
|
Rebased onto main after #2256 (d75d738) and ported into the file threads; head cd6cb60. Review rounds since the first comment (omo-native-code-reviewer and Codex Sol 6.1 Fast xhigh): After rebasing onto #2256 (file-thread pool): the fix was ported into the file thread's
|
|
@coderabbitai review |
|
|
CodeRabbit did not review head cd6cb60 (its "@coderabbitai review" answered "Review rate limited"). The code-review agents' reviews stand in: omo-native-code-reviewer and Codex Sol 6.1 Fast xhigh, final rounds PASS on these exact four commits (git range-diff against the reviewed ones shows them identical); their findings and how each was resolved are in the two comments above. |
|
Tip For best results, initiate chat on the files or code changes. @devswha Understood. CodeRabbit did not review head You are interacting with an AI system. |
… found (#2259) Two regressions the merge pass's review found in this PR: - A scratch file larger than one read was streamed from its descriptor without a Content-Length (Bun sends any stream chunked), so a download showed no size. On Linux a fast disk now sends it as a file body named by the descriptor's own path (/proc/self/fd/N, the file that descriptor holds, whatever its path names now), which Bun sizes and slices exactly. Bun opens that path only when it sends the answer, so the descriptor is closed once both the body and its answer are collected. A slow disk, and macOS and Windows, still stream from the descriptor. - A full path into the scratch folder that is not there was matched against files deeper in the folder whose path ends in it. Like a full path anywhere else, it is now looked for only where it points (in either Unicode form); a bare name is still found deeper.
…ected (#2259) Each fast scratch body larger than one read holds its descriptor until the body and its answer are collected, so a player seeking through a long video could pile them up between collections (Codex review LOW). Past 256 such bodies waiting, one is streamed from its descriptor instead. The range test now asks for more than one read with bytes that tell their offset, so it exercises the file body rather than the read before the headers.
|
Fix round for the merge pass's review of cd6cb60 (two MEDIUMs), now at e0da0aa:
Reviews of this round:
|
|
@coderabbitai review |
✅ Action performed
|
|
Merging e0da0aa (closes #2259). File serving and access, so it got a strict review of its own from the merge pass's reviewer (route-coverage QA with a real createServer and the real file pool), on top of the lane's omo and Codex reviews. When a Claude pane names a file in its scratch folder, the server now opens it first and checks what it opened (the descriptor's identity and real path, a regular file inside the folder), then serves that same descriptor, also through #2256's worker pool via openAuthorized. Verified: on main a file swapped for a link before the open, or a linked file or folder, or an NFC/NFD variant, served an outside file; on this head every such case is refused (404), a swap after the check still serves the checked file's bytes (60 forced-GC tries, never the outside file), a FIFO gets 404 without holding a worker, and descriptors do not leak. This round (Dori's call): large scratch files keep an exact Content-Length and Content-Range (307200; a 1000-299999 range gives 206 with 299000 bytes), sent from the checked descriptor; a missing absolute path is not found, while a bare name is still looked up in the folder. Descriptors waiting to be collected stay under the 256 cap plus bodies in flight, and return to zero after a GC. Not filed: up to about 256 descriptors can stay open for seconds to ~45 s after a burst (a deleted scratch file's space stays allocated until then), and near a hard fd limit of about 300 they can make other requests fail; an aborted fast body keeps its descriptor until a GC; the |
…ading (#2296) (#2297) Closes #2296 ## Problem `server/file-pool.test.ts` hung CI's Fast checks on run [40779586477](https://github.com/devswha/ovis/actions/runs/40779586477) (#2293, which does not touch it): the job was cancelled at its 10-minute limit with this file the only one never reported. The test "keeps a descriptor its thread is reading open until that thread is gone, when the last server stops" holds its file thread with the fake worker's `readGate` (`server/file-worker.fake.ts`, from #2256). That gate said the thread was reading (`Atomics.notify` on cell 1) **before** it opened the FIFO for reading, a blocking `open(2)`. The test then ends the thread (`release()`) and opened the FIFO's writer with a blocking `openSync(fifo, "w")` on its main thread. A thread ended between the notify and its own open never opens the reader, so the writer's open waits forever, and Bun's per-test timeout cannot fire while the main thread is blocked in a system call. #2265's PR body named this gap as a candidate flake. ## Change (test-only) - `readGate` opens the FIFO read-write (an open that never waits) **before** it records the descriptor and notifies, then blocks in a one-byte read of it, as `authorizeGate` (#2265) already does. Once the test sees the thread running, the FIFO always has a reader, even after that thread is ended. - The test releases the read with `O_WRONLY | O_NONBLOCK` and a one-byte write, which never waits (and would fail with `ENXIO` rather than hang if there were no reader). - Both FIFO gates (`readGate`, `authorizeGate`, now one `holdOnFifo`) name their FIFO descriptor in a third gate cell, and the test closes it once the thread is gone: a worker ended inside that read never runs its `finally` on Bun, so before this each run left 2 FIFO descriptors open (found from CodeRabbit's review; `/proc/self/fd` count at the end of the file: 2 -> 0). `server/file-pool.ts` is not changed: the server's thread never waits on a file thread (it posts a task, ends a thread with `terminate()` without awaiting it, and closes descriptors), so the product cannot block this way. Only the tests await `fileThreadsClosed()`. ## Verification - Widened window, scratch only (a 500 ms `Atomics.wait` right after the notify in `readGate`): - on main 72f2edf: `timeout 30 bun test server/file-pool.test.ts` is killed at 30 s (exit 124), stopped in that test; - with this fix: exit 0 in 1 s, 12 pass. - 50 runs of `bun test server/file-pool.test.ts`, 10 at a time pinned to 2 cores (`taskset -c 0,1`), each under `timeout 60`: 50 pass, 0 hangs. Again pinned to one core (`taskset -c 0`): 50 pass, 0 hangs. - Leaked FIFO descriptors at the end of the file (`/proc/self/fd`, scratch probe): 2 before, 0 after; on 80b8cb2 the widened window again passes in 1 s and 50/50 runs pinned to one core pass. - `bun run check fast`: green on 80b8cb2 (typecheck, build, generated files, workflow syntax; unit 8380 pass, 0 fail, 34 skip in 535 files). ## Review omo-native-code-reviewer: PASS (APPROVE), no CRITICAL/HIGH/MEDIUM. It found no path left that blocks the test's main thread (the remaining waits are the non-blocking open and write, `hold.ran()`'s 10 s bounded wait, and awaited promises), confirmed the release cannot hit ENXIO/EAGAIN, and checked that a failing assertion while the thread is parked fails fast instead of hanging. A second omo round on 80b8cb2 (the FIFO-descriptor take-back): PASS (APPROVE), reproduced 2 leaked FIFO descriptors at 65a8a71 and none at 80b8cb2. No Codex review: test-only, `server/file-pool.ts` unchanged. ## Known limits (LOW, not fixed here) - If the thread is ended after its notify but before it enters the FIFO read, the test still passes but does not exercise a read in flight in that run (a false pass, never a hang). Every gate that notifies before it blocks has this window, `authorizeGate` included. The reviewer's findings and how each was resolved are in a PR comment.
…ollected (#2286) (#2317) Closes #2286 ## Problem A Claude Code scratch file larger than one read (256 KiB) on a fast Linux disk is sent as `Bun.file("/proc/self/fd/N").slice(...)` (#2265), a file body Bun sizes itself. Its descriptor must outlive Bun's lazy open, so a `FinalizationRegistry` closes it once both the body and its answer are collected. Past 256 such bodies waiting for collection, `fileResponse` streamed the body from its descriptor instead, and Bun drops the Content-Length of any stream body. Many parallel downloads reach that cap before a collection runs. Reproduced on main with 300 KiB scratch files, 1,800 requests over 16 parallel connections from another process: **73 to 98 of 1,800** answers came back chunked, with no size or progress. ## Change - **Cap:** raised from 256 to a quarter of this process's open-file limit, at most 32,768 descriptor-named bodies. - Only bodies actually being sent are really held, and those are bounded by open connections. The rest are sent bodies waiting for a collection. - Sent bodies that lived through a collection while they were sent wait for one of Bun's own full collections, so the held count rises and falls with those. The peaks, measured with no cap on a 1M-object heap, were 1,942 over 60,000 downloads 16 at a time and 6,657 over 30,000 downloads 64 at a time. - The limit is read once from `/proc/self/limits`; only Linux takes this path. Bun raises the soft limit to the hard one at start (a soft limit of 1,024 became 524,288 here), so most hosts get 32,768. - A host whose hard limit is 1,024 keeps main's 256, leaving the rest for the server's own sockets, terminals and file threads. When the limit cannot be read the cap is 256. - **Collection requested, never forced:** past 256 held bodies (a quarter of the cap when that is lower), a collection is requested on the collector's own threads (`Bun.gc(false)`), at most once every 250 ms, and no answer waits for it. - That frees only bodies made since the last collection: about half the peak in a burst, not ones that lived through a collection. - There is never a forced collection (`Bun.gc(true)`), which holds every terminal, key and request on the server's thread for as long as it takes on the whole heap. - Past the cap, a body is still streamed from its descriptor, as before. - **Unchanged:** descriptor handling and #2259's confinement. A descriptor is still closed only once both the body and its answer are collected. The first version of this PR forced a collection at the cap. The merge pass measured it stopping the event loop for 215–300 ms per collection on a ~300 MB heap during a seek burst, so it was replaced. Alternatives measured on Bun 1.4.3 and rejected: - **A sized `ReadableStream`:** Bun drops Content-Length for every stream body. - **`Bun.file(fd).slice()`:** Bun sends the whole file's size as Content-Length with only the slice's bytes, a wrong header. - **Closing the descriptor when the body is done:** there is no hook for it, because `request.signal` does not abort after a sent or a cut answer. - **Requesting a collection early while keeping the 256 cap:** 88–97 of 1,800 were still chunked, because the collection lands too late. - **The same, plus a non-blocking wait at the cap:** 52–64 of 1,800 chunked, about 2.5 times slower. ## Verification - **Load test** (separate client process, the real file-thread pool, 300 KiB scratch files, 16 at a time): - main: 73–98 of 1,800 chunked in every run. - This branch: 0 of 1,800 in every run, and 0 of 10,000 on a 1M-object heap. Every body had the right bytes and length. - **Sustained load** (64 at a time, 30,000 downloads, 1M-object heap): 0 of 30,000 chunked, where a 4,096 ceiling lost 3,166 to 8,086 and main lost 28,976. - **Seek burst** (1M-object heap, 256 bodies truly held, 512 range requests 16 at a time, the server's event-loop lag sampled every 2 ms), three alternating rounds: | Tree | Stalls over 50 ms per burst | Answers without a length | Burst time | |---|---|---|---| | main | 1 (99–191 ms) | 512 of 512 | 254–373 ms | | first version (0f2a44a) | 9 (786–860 ms in total) | 512 of 512 | 844–1,161 ms | | this branch | 1 (132–184 ms, the heap's own collection, as on main) | 0 of 512 | 265–478 ms | - **New tests** in `server/file-view.test.ts`, neither using sleeps: - "keeps a large file's exact length while many answers let go of wait to be collected (#2286)": it lets go of 1,024 answers, makes the next one before the event loop turns, and fetches it over HTTP. It fails 3 of 3 on main (`content-length` null) and passes here. - "holds no more such bodies than a quarter of this process's open files (#2286)": with the limit set to 1,024, the answer after 256 let-go answers is streamed, whole. It fails 3 of 3 on the round-2 version (6eda965), which ignored the limit, and passes here. - "answers a burst of ranges while many bodies are held without stopping the event loop for a collection (#2286)": the seek-burst bound. It wraps `Bun.gc` and asserts that no forced collection runs while 1,024 ranges are held. It fails 3 of 3 on the first version and passes here. - The bound is not a wall-clock event-loop limit, which would flake on loaded runners, as this repo's timing ratios have (#2084, #1901). - **A host whose hard limit is 1,024** (Codex's reproduction: `ulimit -Sn 1024; ulimit -Hn 1024`, 256 reserved descriptors, 10,000 range reads, a 1M-object heap): all 10,000 answered 206 with the right bytes, peaking at 542 descriptors, the same as main's 542. The round-2 version returned 7,968 404s with `EMFILE`. Each collection request cost 1.5–5.9 ms on the thread. - `bun test server/file-view.test.ts server/file-pool.test.ts server/file-routes.test.ts server/claude-store.test.ts`: 197 pass, 0 fail. `bun run typecheck` is clean. - No `REMOTE_BUNDLE_VERSION` change: only a release PR raises it. ## Reviews The round-1 reviews (omo, Codex Sol 6.1 Fast xhigh) and the merge pass's MEDIUM are in the PR comments. The round-2 reviews of this design are in a comment below. ## Known limits (LOW, not fixed here) - **Slow disks and other platforms still stream without a length.** On a slow disk, a scratch file larger than one read is still streamed without a Content-Length, and so is every such file on macOS and Windows, as before (#2265). - **Past the cap, answers stream without a length again.** Under sustained load with no full collection the held count keeps growing (about 270 a second at 64-way in omo's 96 s run, 25,626 held after 150,000 downloads), so the 32,768 cap is reached after roughly 200,000 downloads at that rate, about 800 MB/s for minutes. The answers past it still arrive whole. - **A larger cap keeps deleted scratch files' disk space a while longer.** Up to 32,768 descriptors, against 256 before, can keep a deleted file's space in use until a full collection. On a host whose hard limit is 1,024, the cap stays 256, so heavy parallel load there loses lengths as on main. - **Held descriptors wait for a collection.** Up to a quarter of the open-file limit (at most 32,768) can stay open until a collection runs, where up to 256 could before.
Closes #2259
Problem
The Claude Code scratch-folder fallback of
/api/fs/statand/api/fs/file(findFile→locateFileInFolder) checked the folder's containment on a path, andfileResponsethen followed that path again (Bun.file(info.path)). A symlink put in place of the file between the check and the open served a file outside the folder. The file viewer made the window wider still: it asks/api/fs/statby the name, then fetches/api/fs/fileby the absolute path that answered, which matched exactly and was served unconfined.Change
Built on #2256 (file reads on bounded file threads). Open first, then check what was opened, with the helper rendered pages already use for their own files (
openAuthorized:O_NOFOLLOW | O_NONBLOCK, then the opened descriptor's kernel path from/proc/self/fd/Non Linux, owned non-group/other-writable folders on macOS), on the file thread:locateFileInFolderdescribes a single file (size, modification time, text/binary sniff) on a file thread (describetask,describeWithin) from a descriptor shown to lie in the folder, and returns that folder aswithin.fileResponse(..., watched, within): theopentask opens such a file throughopenAuthorizedand every byte comes from that descriptor. A small body is read before the headers as on fix(files): read the disk for the file routes on bounded background threads (#790) #2256. A larger one is never sent asBun.file(path)(the "fast disk" branch would follow the path again): on Linux with a fast disk it is sent as a file body named by the descriptor's own path,Bun.file("/proc/self/fd/N").slice(...), which reopens the very file that descriptor holds whatever the path names now, so the answer keeps its exact Content-Length / Content-Range; Bun opens that path only when it sends the answer, so the descriptor is released by aFinalizationRegistryonce both the body and the answer are collected, and past 256 such bodies waiting for collection one is streamed from its descriptor instead, so held descriptors stay bounded. On a slow disk, and on macOS and Windows (no/proc), it is streamed from the descriptor (chunked), as fix(files): read the disk for the file routes on bounded background threads (#790) #2256 streams every slow file. The descriptor is named in the pool's cell the moment it is open, so a thread ended mid-check leaves none open (abandon), and a refused one is taken back from the cell before it is closed.render=1) found there is drawn only when the page it opened lies in the folder (rendertask →pageWithLocalFiles(path, media, grant, within)).findFiletakes amayBeScratchpredicate: a path that lies in the pane's scratch folder (the viewer's second request, and a choice among candidates) is found only there, exact and then in another Unicode form, as the scratch fallback finds it, and never by a lookup that is not held to the folder. A path given in full is looked for only where it points (a missing one stays not found, as anywhere else); only a bare name is also found deeper in the folder.file-routes.tsbuilds it fromclaudeTempBases()(new inclaude-store.ts), so herdr is asked for the pane's session only for a path that can be in one.rootOf/openAuthorized/openedWithintake ahiddenflag (defaultfalse, the old behaviour for page assets) so the fallback keeps allowing hidden names below the folder as before.Direct file links outside the scratch folder keep opening any readable file (existing contract). One behaviour change inside the folder: a symlink in the scratch folder that leads out of it no longer opens by its full path either (it already did not by its relative name).
Verification
Rebased onto main after #2256 (d75d738);
git range-diffshowed the first four commits unchanged from the ones reviewed on #2256's head, and main changed none of the files they touch since. The last two commits answer the merge pass's review of cd6cb60 (M1: a large scratch file lost its Content-Length; M2: a missing full path matched a deeper file).server/file-view.test.ts, "Claude scratch fallback served from what it opened in its folder (fix(files): a symlink swapped in after the scratch-folder containment check serves a file outside the folder #2259)", andserver/file-pool.test.ts), deterministic: swaps happen directly or throughpageFileHookshooks between the check and the open, the held thread waits on atomics and a FIFO, no sleeps. Each fails without its fix:OUTSIDE-SECRET, the page<p>OUTSIDE-SECRET</p>);Bun.file(path);<scratch>/sub/a.txtfor a missing<scratch>/a.txt);openanddescribe, and the pool test that ends a thread held inside the check: fail when the cell take-back orabandon'sdescribecase is removed (mutation checks); the pool test passes 50 runs pinned to one core.bun test server/file-view.test.ts server/file-routes.test.ts server/claude-store.test.ts server/file-pool.test.ts: 185 pass, 0 fail.bun run typecheckclean.bun run check fast: green on e0da0aa (unit 8225 pass, 0 fail; typecheck, build, generated types and themes, workflow syntax ok).Bun.serverunning the route's own steps with the real file-thread pool against a temp scratch folder under<tmp>/claude-<uid>/…: stat by name 200 → file by the returned absolute path 200;Range: bytes=2-4206bytes 2-4/10; HEADcontent-length: 10; a 300 KiB body arrives whole from the descriptor;render=1page 200. After swapping the three files for links outside: file, stat, the large body and the page all 404; a direct link to the outside file still 200 (contract).REMOTE_BUNDLE_VERSIONchange: only a release PR raises it.Reviews
omo-native-code-reviewer and Codex Sol 6.1 Fast xhigh, on the first version and again on the port onto #2256; every HIGH and MEDIUM fixed, final rounds PASS from both. Findings and resolutions are in the PR comment.
Known limits (LOW, not fixed here)
readGate) says it is reading before it opens the FIFO, and the test then opens the FIFO's writer blocking, so a thread ended in between hangs the test (Bun's timeout cannot fire while the main thread is blocked). Codex reproduced that gap withtaskset -c 0in this PR's first version of the new pool test; this PR'sauthorizeGateopens the FIFO read-write before it says it ran and is released by a non-blocking write, and the same change would fitreadGate./proc/self/fd/Nchecks permissions again: a scratch file made unreadable (chmod 000) after it was opened answers 500, as does a Linux system without/proc. Both fail closed (no other bytes are sent).~or has leading/trailing spaces is not found by its full path (the fallback's own name rules).(deleted), and on macOS a scratch subfolder other users can write to, are now refused by the fallback (same rules as page assets).