diff --git a/changelog.d/scratch-folder-open-then-check.md b/changelog.d/scratch-folder-open-then-check.md new file mode 100644 index 0000000000..d5389c106b --- /dev/null +++ b/changelog.d/scratch-folder-open-then-check.md @@ -0,0 +1,4 @@ +### Fixed +- A file opened from a Claude Code session's scratch folder is served, described and drawn as a page + only from the file actually opened inside that folder: a link put in its place after it was found + no longer serves a file from outside the folder. diff --git a/server/claude-store.ts b/server/claude-store.ts index 9513eb3086..145496cfd4 100644 --- a/server/claude-store.ts +++ b/server/claude-store.ts @@ -1182,6 +1182,14 @@ export async function claudeTranscriptFile( return path; } +/** Where `claudeSessionTempDir` looks for this user's `claude-` folder. */ +const claudeTempRoots = (): string[] => [...new Set([tmpdir(), "/tmp"])]; + +/** The `claude-` folders every session's temp folder (`claudeSessionTempDir`) lies in; none on Windows, which has no uid. */ +export function claudeTempBases(uid: number | undefined = process.getuid?.()): string[] { + return uid === undefined ? [] : claudeTempRoots().map((root) => join(root, `claude-${uid}`)); +} + /** * The folder Claude Code keeps for a session's own files outside the project, such as the * `scratchpad/` it names in prose by a relative path: `/claude-///`, @@ -1191,7 +1199,7 @@ export async function claudeTranscriptFile( */ export async function claudeSessionTempDir( transcript: string, - roots: readonly string[] = [...new Set([tmpdir(), "/tmp"])], + roots: readonly string[] = claudeTempRoots(), uid: number | undefined = process.getuid?.(), ): Promise { if (uid === undefined || !transcript.endsWith(".jsonl")) return null; diff --git a/server/file-pool.test.ts b/server/file-pool.test.ts index c20b3b5124..bbaacc1e00 100644 --- a/server/file-pool.test.ts +++ b/server/file-pool.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it } from "bun:test"; -import { closeSync, fstatSync, mkdtempSync, openSync, rmSync, writeFileSync } from "node:fs"; +import { closeSync, constants as fsConstants, fstatSync, mkdtempSync, openSync, rmSync, writeFileSync, writeSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -154,6 +154,25 @@ describe("file threads", () => { expect(() => fstatSync(fd)).toThrow(); }); + it("closes a descriptor a scratch file's description opened, when its thread is ended mid-check", async () => { + setUp({ workers: 1, waiting: 0, deadlineMs: 30_000 }); + const fifo = join(root, "stall"); + expect(Bun.spawnSync(["mkfifo", fifo]).exitCode).toBe(0); + const hold = gate(); + const describing = fileTask({ op: "describe", path: file, folder: root, authorizeGate: { cells: hold.buffer, fifo } } as FileTask); + // the thread holds the file open inside its check, blocked in a system call ending it cannot cut short + const fd = await hold.ran(); + expect(fstatSync(fd).size).toBe(5); + setFilePoolOptions(); + await expect(describing).rejects.toThrow("replaced"); + // the call returns (a write that never waits: the thread opened the FIFO before it said it ran), the thread is gone, and what it opened is closed + const writer = openSync(fifo, fsConstants.O_WRONLY | fsConstants.O_NONBLOCK); + writeSync(writer, "x"); + closeSync(writer); + await fileThreadsClosed(); + expect(() => fstatSync(fd)).toThrow(); + }); + it("keeps a descriptor its thread is reading open until that thread is gone, when the last server stops", async () => { setUp({ workers: 1, waiting: 0, deadlineMs: 30_000 }); const fifo = join(root, "stall"); diff --git a/server/file-pool.ts b/server/file-pool.ts index b3d166bf27..f4fc5e00fc 100644 --- a/server/file-pool.ts +++ b/server/file-pool.ts @@ -138,7 +138,7 @@ export function fileTask(task: FileTask, signal?: Abor if (signal?.aborted) return Promise.reject(new FileBusyError("the request was cancelled")); // the thread names the descriptor it opens here at once, so one never answered (a thread that // failed or was ended mid-task) can still be closed (`abandon`) - if (task.op === "open") task = { ...task, opened: openedCell() }; + if (task.op === "open" || task.op === "describe") task = { ...task, opened: openedCell() }; return new Promise((done, fail) => { const deadline = task.op === "render" || task.op === "walk" ? options.longDeadlineMs : options.deadlineMs; const pending: Pending = { task, done: done as (result: FileTaskResult) => void, fail, settled: false, timer: setTimeout(() => expire(pending, "the file system did not answer in time"), deadline) }; @@ -241,7 +241,7 @@ function openedCell(): Int32Array { /** The descriptor a task's thread opened and will never hand over (it failed or was ended, and is gone), closed. */ function abandon(task: FileTask): void { - if (task.op !== "open" || task.opened === undefined) return; + if ((task.op !== "open" && task.op !== "describe") || task.opened === undefined) return; const fd = Atomics.exchange(task.opened, 0, -1); if (fd < 0) return; try { closeSync(fd); } catch { /* already closed */ } diff --git a/server/file-routes.ts b/server/file-routes.ts index 99100d5625..19b6e4c998 100644 --- a/server/file-routes.ts +++ b/server/file-routes.ts @@ -4,10 +4,12 @@ * The dispatcher in index.ts runs the access gate first and hands over who asks: a page's media * plays only for the client and device that rendered it (file-view.ts). */ +import { sep } from "node:path"; + import type { ProjectFavicon } from "../shared/files.ts"; import type { Access } from "./access.ts"; import { isSecureRequest } from "./auth.ts"; -import { claudeSessionTempDir } from "./claude-store.ts"; +import { claudeSessionTempDir, claudeTempBases } from "./claude-store.ts"; import { claudePaneSession } from "./conversation.ts"; import { fileResponse, findFile, crossSiteFileRead, pageMediaResponse, pageMediaSource, PAGE_MEDIA_PATH, watchPageMedia } from "./file-view.ts"; import { paneGet } from "./herdr/client.ts"; @@ -18,6 +20,9 @@ import { findProjectFavicon } from "./project-favicon.ts"; /** Who asks, as the dispatcher's access gate worked it out. */ export interface FileRequester { bridgeAuthorized: boolean; requestHost: string | null; forwarded: boolean; client: string | null; access: Access } +/** Whether `path` may lie in a Claude Code session's temp folder, told before herdr is asked which one the pane's is. */ +const mayBeClaudeTemp = (path: string): boolean => claudeTempBases().some((base) => path.startsWith(base + sep)); + /** The temp folder of the Claude Code session in a pane, or null when the pane runs none or it has none. */ async function claudeTempFolder(paneId: string): Promise { try { @@ -66,7 +71,7 @@ async function fileRoute(request: Request, url: URL, pathname: string, { bridgeA try { cwd = (await paneContext(paneId)).cwd; } catch { /* an absolute path still opens */ } } const path = url.searchParams.get("path") ?? ""; - const found = await findFile(path, cwd, () => paneId ? claudeTempFolder(paneId) : Promise.resolve(null)); + const found = await findFile(path, cwd, () => paneId ? claudeTempFolder(paneId) : Promise.resolve(null), paneId ? mayBeClaudeTemp : undefined); if (found === null) return jsonResponse({ error: { code: "not_found", message: "no readable file at that path" } }, 404); // several files end in that name: the viewer lists them to choose from if ("candidates" in found) return jsonResponse({ error: { code: "ambiguous_path", message: "several files have that name", candidates: found.candidates } }, 409); @@ -74,5 +79,5 @@ async function fileRoute(request: Request, url: URL, pathname: string, { bridgeA const mediaSource = bridgeAuthorized ? null : pageMediaSource(requestHost, isSecureRequest(request), forwarded ? request.headers.get("x-forwarded-host") : null); // its media plays only for the client address that rendered it, and ends with that device (file-view.ts) const media = mediaSource === null ? null : { source: mediaSource, client, device: access.level === "full" ? access.device?.id ?? null : null }; - return pathname === "/api/fs/stat" ? jsonResponse(found.info) : await fileResponse(found.info, url.searchParams.get("download") === "1", url.searchParams.get("render") === "1", media, request, watch); + return pathname === "/api/fs/stat" ? jsonResponse(found.info) : await fileResponse(found.info, url.searchParams.get("download") === "1", url.searchParams.get("render") === "1", media, request, watch, found.within ?? null); } diff --git a/server/file-tasks.ts b/server/file-tasks.ts index 3d9d80cfb7..b8a945bf09 100644 --- a/server/file-tasks.ts +++ b/server/file-tasks.ts @@ -2,7 +2,7 @@ import { constants, fstatSync, openSync, readSync, realpathSync } from "node:fs" import { sep } from "node:path"; import type { FileInfo } from "../shared/protocol.ts"; -import { byteRange, fileInfo, pageFileHooks, pageWithLocalFiles, pathsByName, release, STREAM_CHUNK_BYTES, type PageMedia, type PageMediaGrant } from "./file-view.ts"; +import { byteRange, describeWithin, fileInfo, openAuthorized, pageFileHooks, pageWithLocalFiles, pathsByName, release, STREAM_CHUNK_BYTES, type PageMedia, type PageMediaGrant } from "./file-view.ts"; import { walkFiles } from "./files.ts"; /** @@ -13,14 +13,19 @@ export type FileTask = | { op: "info"; path: string } | { op: "named"; path: string } | { op: "within"; paths: string[]; folder: string } + /** a file the scratch fallback found, described from what was opened in `folder` (`describeWithin`); `opened` as for `open` */ + | { op: "describe"; path: string; folder: string; opened?: Int32Array } | { op: "walk"; cwd: string } - | { op: "render"; path: string; media: PageMedia | null } + /** `within`: the folder a page found in it must have been opened in (`pageWithLocalFiles`) */ + | { op: "render"; path: string; media: PageMedia | null; within?: string | null } /** * `get`: a GET's Range and whether it came with If-Range (null for a HEAD, which reads nothing); * `opened`: where the thread names the descriptor as soon as it has it (`file-pool.ts`); - * `fastDiskMs`: how quickly the disk must answer for a large body to be sent whole (tests) + * `fastDiskMs`: how quickly the disk must answer for a large body to be sent whole (tests); + * `within`: the folder the file must be opened in (`openAuthorized`), for a file the scratch + * fallback found */ - | { op: "open"; path: string; get: { range: string | null; ifRange: boolean } | null; opened?: Int32Array; fastDiskMs?: number }; + | { op: "open"; path: string; get: { range: string | null; ifRange: boolean } | null; opened?: Int32Array; fastDiskMs?: number; within?: string | null }; /** A page drawn with its own files put into it (null when it cannot be), and the media addresses that render gave out. */ export interface RenderedPage { html: Uint8Array | null; grants: PageMediaGrant[] } @@ -40,16 +45,17 @@ export interface OpenedFile { size: number; range: [number, number] | "unsatisfi export const FAST_DISK_MS = 50; const PROBE_BYTES = 64 * 1024; -export type FileTaskResult = FileInfo | FileInfo[] | string[] | boolean | RenderedPage | OpenedFile | null; +export type FileTaskResult = FileInfo | FileInfo[] | string[] | boolean | RenderedPage | OpenedFile | { info: FileInfo; within: string } | null; export function runFileTask(task: FileTask): FileTaskResult { switch (task.op) { case "info": return fileInfo(task.path); case "named": return pathsByName(task.path).map(fileInfo).filter((info) => info !== null); case "within": return within(task.paths, task.folder); + case "describe": return describeWithin(task.path, task.folder, task.opened); case "walk": return walkFiles(task.cwd); - case "render": return render(task.path, task.media); - case "open": return open(task.path, task.get, task.opened, task.fastDiskMs ?? FAST_DISK_MS); + case "render": return render(task.path, task.media, task.within ?? null); + case "open": return open(task.path, task.get, task.opened, task.fastDiskMs ?? FAST_DISK_MS, task.within ?? null); } } @@ -68,9 +74,9 @@ function within(paths: string[], folder: string): boolean { } catch { return false; } } -function render(path: string, media: PageMedia | null): RenderedPage { +function render(path: string, media: PageMedia | null, within: string | null): RenderedPage { const grants: PageMediaGrant[] = []; - const html = pageWithLocalFiles(path, media, (grant) => grants.push(grant)); + const html = pageWithLocalFiles(path, media, (grant) => grants.push(grant), within); return { html: html === null ? null : new TextEncoder().encode(html), grants }; } @@ -81,14 +87,22 @@ function render(path: string, media: PageMedia | null): RenderedPage { * disk answered at once (Bun sizes only a file body, which it reads on the server's thread, as it * always did); from a slow disk it is streamed from the descriptor handed over, without a length * (Bun sends any stream body chunked), so that disk never holds the server's thread. O_NONBLOCK, - * so a FIFO put where the file was is not waited on. + * so a FIFO put where the file was is not waited on. With `within`, the file is opened only as one + * shown to lie in that folder (`openAuthorized`), and every byte comes from that descriptor: a body + * is never sent from the path, which a link put there since could lead out of the folder. */ -function open(path: string, get: { range: string | null; ifRange: boolean } | null, opened: Int32Array | undefined, fastDiskMs: number): OpenedFile | null { +function open(path: string, get: { range: string | null; ifRange: boolean } | null, opened: Int32Array | undefined, fastDiskMs: number, within: string | null): OpenedFile | null { const began = performance.now(); let fd: number; - try { fd = openSync(path, constants.O_RDONLY | (constants.O_NONBLOCK ?? 0)); } catch { return null; } - if (opened !== undefined) Atomics.store(opened, 0, fd); - pageFileHooks.opened?.(fd); + if (within !== null) { + const authorized = openAuthorized(path, [within], true, opened); + if (authorized === null) return null; + fd = authorized.fd; + } else { + try { fd = openSync(path, constants.O_RDONLY | (constants.O_NONBLOCK ?? 0)); } catch { return null; } + if (opened !== undefined) Atomics.store(opened, 0, fd); + pageFileHooks.opened?.(fd); + } // not handed over: closed by whoever takes it from the cell first, here or `abandon` (file-pool.ts) const close = () => { if (opened === undefined || Atomics.compareExchange(opened, 0, fd, -1) === fd) release(fd); }; let size: number; @@ -104,6 +118,9 @@ function open(path: string, get: { range: string | null; ifRange: boolean } | nu if (length > STREAM_CHUNK_BYTES) { try { (pageFileHooks.readSync ?? readSync)(fd, new Uint8Array(PROBE_BYTES), 0, PROBE_BYTES, start); } catch { close(); return null; } if (performance.now() - began >= fastDiskMs) return { size, range, bytes: null, fd, fast: false }; + // a file the scratch fallback found is never sent from its path: on Linux, from its descriptor's + // own (`/proc/self/fd`, the file it holds, sized as a file body); elsewhere streamed from it + if (within !== null) return { size, range, bytes: null, fd, fast: process.platform === "linux" }; close(); return { size, range, bytes: null, fd: null, fast: true }; } diff --git a/server/file-view.test.ts b/server/file-view.test.ts index 3a0b8b0387..feeaed205a 100644 --- a/server/file-view.test.ts +++ b/server/file-view.test.ts @@ -4,7 +4,7 @@ import { homedir, tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import * as fileView from "./file-view.ts"; -import { crossSiteFileRead, fileInfo, fileResponse, forgetPageMedia, locateFile, locateFileInFolder, PAGE_MEDIA_LIFETIME_MS, PAGE_MEDIA_PER_RENDER, pageFileHooks, pageMediaResponse, pageMediaSource, resolveFilePath, watchPageMedia } from "./file-view.ts"; +import { crossSiteFileRead, fileInfo, fileResponse, forgetPageMedia, locateFile, locateFileInFolder, PAGE_MEDIA_LIFETIME_MS, PAGE_MEDIA_PER_RENDER, pageFileHooks, pageMediaResponse, pageMediaSource, resolveFilePath, STREAM_CHUNK_BYTES, watchPageMedia } from "./file-view.ts"; import { setFilePoolOptions } from "./file-pool.ts"; import { runFileTask } from "./file-tasks.ts"; import { clearFileCache, filesNamed } from "./files.ts"; @@ -1370,3 +1370,215 @@ describe("locateFile", () => { expect(await locateFile("notes.md", root)).toMatchObject({ info: { path: join(root, "docs/notes.md") } }); }); }); + +describe("Claude scratch fallback served from what it opened in its folder (#2259)", () => { + const roots: string[] = []; + beforeEach(trackDescriptors); + afterEach(() => { + descriptorsClosed(); + clearFileCache(); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); + }); + /** A pane, its scratch folder holding `name` (`inside`), and `outside` beside that folder under the same name. */ + const fixture = (name: string, inside: string, outside: string) => { + const root = mkdtempSync(join(tmpdir(), "herdr-scratch-open-")); + roots.push(root); + const cwd = join(root, "pane"); + const scratch = join(root, "scratch"); + mkdirSync(cwd); + mkdirSync(scratch); + writeFileSync(join(scratch, name), inside); + writeFileSync(join(root, name), outside); + /** the scratch file replaced by a link to the one outside */ + const swap = () => { unlinkSync(join(scratch, name)); symlinkSync(join(root, name), join(scratch, name)); }; + return { cwd, scratch, swap }; + }; + /** A body sent from its descriptor's own path keeps that descriptor until the body and its answer are collected: once both are gone, each is closed. */ + const bodiesCollected = async () => { + // within the test's own timeout, so a slow collection fails here, with the descriptors named + const deadline = Date.now() + 3_000; + while (live.size > 0) { + if (Date.now() > deadline) throw new Error(`descriptors still open after collection: ${[...live].join(", ")}`); + Bun.gc(true); + // a turn of the event loop, for the collected bodies' finalizers to run + await new Promise((resolve) => setTimeout(resolve, 0)); + } + }; + const fromScratch = async (name: string, cwd: string, scratch: string) => { + const found = await fileView.findFile(name, cwd, async () => scratch); + if (found === null || !("info" in found)) throw new Error("expected the scratch file"); + return found; + }; + + it("refuses a link swapped in after the file was found", async () => { + const { cwd, scratch, swap } = fixture("note.txt", "INSIDE", "OUTSIDE-SECRET"); + const found = await fromScratch("note.txt", cwd, scratch); + swap(); + const swapped = await fileResponse(found.info, false, false, null, null, undefined, found.within); + expect(await swapped.text()).not.toContain("OUTSIDE-SECRET"); + expect(swapped.status).toBe(404); + }); + + it("refuses a link swapped in between the open's check and the open itself", async () => { + const { cwd, scratch, swap } = fixture("note.txt", "INSIDE", "OUTSIDE-SECRET"); + const found = await fromScratch("note.txt", cwd, scratch); + pageFileHooks.afterResolve = () => { delete pageFileHooks.afterResolve; swap(); }; + const swapped = await fileResponse(found.info, false, false, null, null, undefined, found.within); + expect(pageFileHooks.afterResolve).toBeUndefined(); + expect(await swapped.text()).not.toContain("OUTSIDE-SECRET"); + expect(swapped.status).toBe(404); + }); + + it("describes the file only from what it opened in the folder", async () => { + const { scratch, swap } = fixture("note.txt", "INSIDE", "OUTSIDE-SECRET"); + pageFileHooks.afterResolve = () => { delete pageFileHooks.afterResolve; swap(); }; + expect(await locateFileInFolder("note.txt", scratch)).toBeNull(); + expect(pageFileHooks.afterResolve).toBeUndefined(); + }); + + it("renders a page only from what it opened in the folder", async () => { + const { cwd, scratch, swap } = fixture("page.html", "

INSIDE

", "

OUTSIDE-SECRET

"); + const found = await fromScratch("page.html", cwd, scratch); + expect(await (await fileResponse(found.info, false, true, null, null, undefined, found.within)).text()).toContain("INSIDE"); + swap(); + const swapped = await fileResponse(found.info, false, true, null, null, undefined, found.within); + expect(await swapped.text()).not.toContain("OUTSIDE-SECRET"); + expect(swapped.status).toBe(404); + }); + + it("refuses a link swapped in before the viewer opens, by the path it was given, what it found", async () => { + const { cwd, scratch, swap } = fixture("note.txt", "INSIDE", "OUTSIDE-SECRET"); + const mayBeScratch = (path: string) => path.startsWith(scratch); + const stat = await fromScratch("note.txt", cwd, scratch); + // the viewer's next request names the file by the path stat answered + const byPath = await fileView.findFile(stat.info.path, cwd, async () => scratch, mayBeScratch); + expect(byPath).toMatchObject({ info: { path: stat.info.path }, within: realpathSync(scratch) }); + if (byPath === null || !("info" in byPath)) throw new Error("expected the scratch file"); + expect(await (await fileResponse(byPath.info, false, false, null, null, undefined, byPath.within)).text()).toBe("INSIDE"); + swap(); + expect(await fileView.findFile(stat.info.path, cwd, async () => scratch, mayBeScratch)).toBeNull(); + // a path outside the pane's scratch folder is a direct link, as before + const outside = join(scratch, "..", "note.txt"); + expect(await fileView.findFile(outside, cwd, async () => scratch, () => true)).toMatchObject({ info: { path: outside } }); + }); + + it("finds a path in the folder only there, in any name form, never past it", async () => { + for (const [stored, swapped] of [["NFD", "NFC"], ["NFC", "NFD"]] as const) { + const name = "é.txt".normalize(stored); + const { cwd, scratch } = fixture(name, "INSIDE", "OUTSIDE-SECRET"); + // a file system that stores one form for both cannot hold the swapped name beside it + if (existsSync(join(scratch, name.normalize(swapped)))) return; + const mayBeScratch = (path: string) => path.startsWith(scratch); + const stat = await fromScratch(name, cwd, scratch); + // the file the viewer was told of gone, and a link to the file outside put there under the other form of its name + unlinkSync(join(scratch, name)); + symlinkSync(join(scratch, "..", name), join(scratch, name.normalize(swapped))); + const byPath = await fileView.findFile(stat.info.path, cwd, async () => scratch, mayBeScratch); + if (byPath !== null && "info" in byPath) expect(await (await fileResponse(byPath.info, false, false, null, null, undefined, byPath.within ?? null)).text()).not.toContain("OUTSIDE-SECRET"); + expect(byPath).toBeNull(); + } + }); + + it("finds a path given in full in the folder only where it points, never a file deeper whose path ends in it", async () => { + const { cwd, scratch } = fixture("other.txt", "INSIDE", "OUTSIDE-SECRET"); + mkdirSync(join(scratch, "sub")); + writeFileSync(join(scratch, "sub", "a.txt"), "DEEPER"); + const mayBeScratch = (path: string) => path.startsWith(scratch); + // `/a.txt` is not there: not found, as a missing path anywhere else + expect(await fileView.findFile(join(scratch, "a.txt"), cwd, async () => scratch, mayBeScratch)).toBeNull(); + // a bare name is still found deeper in the scratch folder, as the fallback always did + expect(await fileView.findFile("a.txt", cwd, async () => scratch, mayBeScratch)).toMatchObject({ info: { path: join(scratch, "sub", "a.txt") } }); + }); + + it("keeps sending the file it opened when a link is swapped in while it is sent", async () => { + // larger than one read too: such a body is sent after its headers, never from the path + for (const inside of ["INSIDE", "I".repeat(STREAM_CHUNK_BYTES + 1)]) { + const { cwd, scratch, swap } = fixture("note.txt", inside, "OUTSIDE-SECRET"); + await (async () => { + const found = await fromScratch("note.txt", cwd, scratch); + const responses = await Promise.all([false, true].map((download) => fileResponse(found.info, download, false, null, null, undefined, found.within))); + expect(responses.map((response) => response.status)).toEqual([200, 200]); + swap(); + for (const response of responses) expect(await response.text()).toBe(inside); + })(); + } + await bodiesCollected(); + }); + + it("sends a file larger than one read with its exact length, whole and by range", async () => { + // only Linux names an open file by its descriptor (`/proc/self/fd`); elsewhere it is streamed without one + if (process.platform !== "linux") return; + const size = 2 * STREAM_CHUNK_BYTES + 4096; + // bytes that tell their own offset, so a range sent from the wrong place is seen + const bytes = Buffer.from(Array.from({ length: size }, (_, at) => at % 251)); + const { cwd, scratch } = fixture("big.bin", "", "OUTSIDE-SECRET"); + writeFileSync(join(scratch, "big.bin"), bytes); + // a disk that answers at once, whatever this machine's load: the case a file body is for + pageFileHooks.fastDiskMs = 60_000; + await (async () => { + const found = await fromScratch("big.bin", cwd, scratch); + const server = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: (request) => fileResponse(found.info, true, false, null, request, undefined, found.within) }); + try { + const whole = await fetch(server.url); + expect(whole.headers.get("content-length")).toBe(String(size)); + expect(Buffer.from(await whole.arrayBuffer()).equals(bytes)).toBe(true); + // more than one read: sent as a file body, not read before the headers + const [first, last] = [10, STREAM_CHUNK_BYTES + 5000]; + const part = await fetch(server.url, { headers: { range: `bytes=${first}-${last}` } }); + expect(part.status).toBe(206); + expect(part.headers.get("content-range")).toBe(`bytes ${first}-${last}/${size}`); + expect(part.headers.get("content-length")).toBe(String(last - first + 1)); + expect(Buffer.from(await part.arrayBuffer()).equals(bytes.subarray(first, last + 1))).toBe(true); + } finally { await server.stop(true); } + })(); + await bodiesCollected(); + }); + + it("names the descriptor a file thread opens in the folder in its cell, and takes back one it refuses", async () => { + const { cwd, scratch, swap } = fixture("note.txt", "INSIDE", "OUTSIDE-SECRET"); + const found = await fromScratch("note.txt", cwd, scratch); + const cell = () => { const opened = new Int32Array(new SharedArrayBuffer(4)); opened[0] = -1; return opened; }; + const kept = cell(); + let named = -1; + pageFileHooks.afterAuthorize = () => { named = Atomics.load(kept, 0); }; + const opened = runFileTask({ op: "open", path: found.info.path, get: { range: null, ifRange: false }, opened: kept, within: found.within }); + // named before it was authorized, and handed back with the body read + expect(named).toBeGreaterThanOrEqual(0); + expect(opened).toMatchObject({ bytes: new TextEncoder().encode("INSIDE") }); + expect(Atomics.load(kept, 0)).toBe(-1); + delete pageFileHooks.afterAuthorize; + const refused = cell(); + pageFileHooks.afterOpen = () => { delete pageFileHooks.afterOpen; swap(); }; + expect(runFileTask({ op: "open", path: found.info.path, get: { range: null, ifRange: false }, opened: refused, within: found.within })).toBeNull(); + // taken back before it was closed: the pool never closes that number again (`abandon`) + expect(Atomics.load(refused, 0)).toBe(-1); + }); + + it("names the descriptor a file thread describes a file in the folder from in its cell, and takes it back", async () => { + const { scratch } = fixture("note.txt", "INSIDE", "OUTSIDE-SECRET"); + const described = new Int32Array(new SharedArrayBuffer(4)); + described[0] = -1; + let named = -1; + pageFileHooks.afterAuthorize = () => { named = Atomics.load(described, 0); }; + expect(runFileTask({ op: "describe", path: join(scratch, "note.txt"), folder: scratch, opened: described })).toMatchObject({ info: { size: 6 }, within: realpathSync(scratch) }); + delete pageFileHooks.afterAuthorize; + expect(named).toBeGreaterThanOrEqual(0); + expect(Atomics.load(described, 0)).toBe(-1); + }); + + it("still serves the file in the folder whole, by range and to a HEAD", async () => { + const { cwd, scratch } = fixture("note.txt", "0123456789", "OUTSIDE-SECRET"); + const found = await fromScratch("note.txt", cwd, scratch); + const whole = await fileResponse(found.info, false, false, null, null, undefined, found.within); + expect(whole.status).toBe(200); + expect(whole.headers.get("content-type")).toStartWith("text/plain"); + expect(await whole.text()).toBe("0123456789"); + const range = await fileResponse(found.info, false, false, null, new Request("http://ovis/", { headers: { range: "bytes=2-4" } }), undefined, found.within); + expect(range.status).toBe(206); + expect(range.headers.get("content-range")).toBe("bytes 2-4/10"); + expect(await range.text()).toBe("234"); + const head = await fileResponse(found.info, false, false, null, new Request("http://ovis/", { method: "HEAD" }), undefined, found.within); + expect(head.headers.get("content-length")).toBe("10"); + expect(head.body).toBeNull(); + }); +}); diff --git a/server/file-view.ts b/server/file-view.ts index 960a0ad597..6eb5164258 100644 --- a/server/file-view.ts +++ b/server/file-view.ts @@ -1,7 +1,7 @@ import { randomBytes } from "node:crypto"; import { type BigIntStats, closeSync, constants, existsSync, fstatSync, lstatSync, openSync, read, readdirSync, readlinkSync, readSync, realpathSync, statSync } from "node:fs"; import { homedir } from "node:os"; -import { basename, delimiter, dirname, isAbsolute, join, parse, resolve, sep } from "node:path"; +import { basename, delimiter, dirname, isAbsolute, join, parse, relative as relativePath, resolve, sep } from "node:path"; import type { FileInfo, FileKind } from "../shared/protocol.ts"; import { fileTask } from "./file-pool.ts"; @@ -42,7 +42,7 @@ export function resolveFilePath(input: string, cwd: string | null): string | nul const TEXT_TYPES = /^(?:text\/|application\/(?:json|javascript|typescript|xml|x-sh|toml|yaml|x-yaml|sql)|image\/svg\+xml)/; const SNIFF_BYTES = 8192; -function kindOf(mime: string, path: string): FileKind { +function kindOf(mime: string, path: string, opened: number | null = null): FileKind { if (mime === "image/svg+xml") return "image"; if (mime.startsWith("image/")) return "image"; if (mime.startsWith("video/")) return "video"; @@ -51,11 +51,12 @@ function kindOf(mime: string, path: string): FileKind { if (TEXT_TYPES.test(mime)) return "text"; // no known type (a .log, a dotfile, source without a registered extension): text when // its first bytes hold no NUL + // (`opened`: a descriptor already open on the file, read instead of the path) let fd: number | null = null; try { - fd = openSync(path, "r"); + if (opened === null) fd = openSync(path, "r"); const buffer = Buffer.alloc(SNIFF_BYTES); - const read = readSync(fd, buffer, 0, SNIFF_BYTES, 0); + const read = readSync(opened ?? fd!, buffer, 0, SNIFF_BYTES, 0); return buffer.subarray(0, read).includes(0) ? "binary" : "text"; } catch { return "binary"; @@ -117,24 +118,81 @@ export async function locateFile(input: string, cwd: string | null, anyForm = fa return found.length > 1 ? { candidates: found } : null; } -/** A relative fallback stays in its own folder, including after following symlinks. */ -export async function locateFileInFolder(input: string, folder: string, anyForm = false): ReturnType { +/** A file found, with the folder it must be served from when it was found as one that stays in it (`locateFileInFolder`); or the candidates; or nothing. */ +export type LocatedFile = { info: FileInfo; within?: string } | { candidates: string[] } | null; + +/** + * A relative fallback stays in its own folder, including after following symlinks. Checking a + * path and then following it again could meet a link put there in between, so the file is + * described from a descriptor shown to lie in the folder (`describeWithin`, on a file thread), and + * comes back with that folder (`within`), in which `fileResponse` opens it again the same way. + * Candidates are only names to choose from. Without `walk`, only the place `input` names is + * looked at (in another form of the name too, with `anyForm`), never a file deeper in the folder + * whose path ends in it: a path given in full that is not there is not found, as anywhere else. + */ +export async function locateFileInFolder(input: string, folder: string, anyForm = false, walk = true): Promise { const relative = input.trim(); if (isAbsolute(relative) || relative.startsWith("~") || !resolve(folder, relative).startsWith(resolve(folder) + sep)) return null; - const found = await locateFile(relative, folder, anyForm); + // named in full (no folder to walk), `locateFile` looks only where the path points + const found = walk ? await locateFile(relative, folder, anyForm) : await locateFile(resolve(folder, relative), null, anyForm); if (found === null) return null; // a missing or unreadable target is not a fallback - const paths = "info" in found ? [found.info.path] : found.candidates; - return await fileTask({ op: "within", paths, folder }) ? found : null; + if ("candidates" in found) return await fileTask({ op: "within", paths: found.candidates, folder }) ? found : null; + return await fileTask<{ info: FileInfo; within: string } | null>({ op: "describe", path: found.info.path, folder }); } -/** Try every exact place before opening a name stored in another form. */ -export async function findFile(input: string, cwd: string | null, scratchFolder: () => Promise): ReturnType { +/** + * The file at `path` described from a descriptor shown to lie in `folder` (`openAuthorized`), with + * the folder as it resolves; null for anything else. `cell`: where a file thread names the + * descriptor while it is open (`openAuthorized`), taken back before it is closed. + */ +export function describeWithin(path: string, folder: string, cell?: Int32Array): { info: FileInfo; within: string } | null { + let within: string; + try { within = realpathSync(folder); } catch { return null; } + const file = openAuthorized(path, [within], true, cell); + if (file === null) return null; + // nothing here opens the path again: the type comes from its name, the rest from the descriptor + try { + const mime = mimeOf(path); + const modified = fstatSync(file.fd).mtime.toISOString(); + return { info: { path, name: basename(path), size: file.size, modified, mime, kind: kindOf(mime, path, file.fd) }, within }; + } catch { return null; } finally { + // one the pool already took back from the cell (`abandon`) is closed there + if (cell === undefined || Atomics.compareExchange(cell, 0, file.fd, -1) === file.fd) release(file.fd); + } +} + +/** + * Try every exact place before opening a name stored in another form. A file named by a path in + * the scratch folder (the viewer opens what it found there by the path it was given, and a choice + * among candidates by its own) is still that folder's: found only there, in any form, as + * `locateFileInFolder` finds it, and never by a lookup that is not held to the folder. + * `mayBeScratch` tells, without asking for the folder, whether a path can lie in one. + */ +export async function findFile(input: string, cwd: string | null, scratchFolder: () => Promise, mayBeScratch: (path: string) => boolean = () => false): Promise { + // the folder is asked for once, however many lookups need it + let asked: Promise | undefined; + const scratchOnce = () => asked ??= scratchFolder(); + /** the pane's scratch folder and `path` relative to it, when `path` lies in it */ + const inScratch = async (path: string | null): Promise<{ folder: string; relative: string } | null> => { + if (path === null || !mayBeScratch(path)) return null; + const folder = await scratchOnce(); + return folder !== null && path.startsWith(resolve(folder) + sep) ? { folder, relative: relativePath(resolve(folder), path) } : null; + }; + const named = await inScratch(resolveFilePath(input, cwd)); + if (named !== null) { + // a path given in full (or from ~) names one place; only a relative one may be found deeper in the folder + const walk = !isAbsolute(input.trim()) && !input.trim().startsWith("~"); + return await locateFileInFolder(named.relative, named.folder, false, walk) ?? locateFileInFolder(named.relative, named.folder, true, walk); + } const exact = await locateFile(input, cwd); + // found under the pane's folder, in the scratch folder below it + const walked = exact !== null && "info" in exact ? await inScratch(exact.info.path) : null; + if (walked !== null) return locateFileInFolder(walked.relative, walked.folder); if (exact !== null) return exact; let folder: string | null = null; if (cwd !== null && !isAbsolute(input.trim()) && !input.trim().startsWith("~")) { - folder = await scratchFolder(); + folder = await scratchOnce(); if (folder !== null) { const scratch = await locateFileInFolder(input, folder); if (scratch !== null) return scratch; @@ -488,9 +546,9 @@ const hiddenBelow = (folder: string, path: string): boolean => path.slice(folder * The first of `roots` (resolved folders) that `file` (a resolved path) lies in, when it has no * hidden name below that folder (`.ssh/`, `.env`). */ -function rootOf(file: string, roots: readonly string[]): string | undefined { +function rootOf(file: string, roots: readonly string[], hidden = false): string | undefined { const root = roots.find((candidate) => file.startsWith(candidate.endsWith(sep) ? candidate : candidate + sep)); - if (root === undefined || hiddenBelow(root.endsWith(sep) ? root.slice(0, -1) : root, file)) return undefined; + if (root === undefined || (!hidden && hiddenBelow(root.endsWith(sep) ? root.slice(0, -1) : root, file))) return undefined; return root; } @@ -530,7 +588,7 @@ export function release(fd: number): void { } /** A file a page may read, open: its descriptor, what it was asked as and allowed under, and the file's type, identity, size and modification time. */ -interface Authorized { fd: number; requested: string; file: string; roots: readonly string[]; mime: string; dev: bigint; ino: bigint; size: number; mtimeNs: bigint } +export interface Authorized { fd: number; requested: string; file: string; roots: readonly string[]; mime: string; dev: bigint; ino: bigint; size: number; mtimeNs: bigint } /** * `requested` opened for a page to read, when what was opened is a file in one of `roots` with no @@ -549,18 +607,22 @@ interface Authorized { fd: number; requested: string; file: string; roots: reado * lists: anyone allowed to write to a folder on the way can race them. That is outside the one * user this server serves. * - Anywhere else, nothing is authorized. - * The caller reads only from the descriptor and closes it (`release`). + * `hidden` lets a file with a hidden name below its root through too. `cell`: where a file thread + * names the descriptor the moment it is open (`file-pool.ts`), so a thread ended mid-check leaves + * none open; a refused one is taken back from it before it is closed. The caller reads only from + * the descriptor and closes it (`release`). */ -function openAuthorized(requested: string, roots: readonly string[]): Authorized | null { +export function openAuthorized(requested: string, roots: readonly string[], hidden = false, cell?: Int32Array): Authorized | null { const hooks = pageFileHooks; let file: string; try { file = realpathSync(requested); - if (rootOf(file, roots) === undefined || !statSync(file).isFile()) return null; + if (rootOf(file, roots, hidden) === undefined || !statSync(file).isFile()) return null; } catch { return null; } hooks.afterResolve?.(file); let fd: number; try { fd = openSync(file, OPEN_FLAGS); } catch { return null; } + if (cell !== undefined) Atomics.store(cell, 0, fd); hooks.opened?.(fd); let kept = false; try { @@ -568,12 +630,12 @@ function openAuthorized(requested: string, roots: readonly string[]): Authorized if (!opened.isFile() || opened.ino === 0n) return null; hooks.afterOpen?.(file); const again = realpathSync(requested); - const root = rootOf(again, roots); + const root = rootOf(again, roots, hidden); if (root === undefined) return null; hooks.beforeIdentity?.(again); const named = statSync(again, { bigint: true }); if (named.dev !== opened.dev || named.ino !== opened.ino) return null; - if (!openedWithin(fd, again, root, roots)) return null; + if (!openedWithin(fd, again, root, roots, hidden)) return null; hooks.afterAuthorize?.(requested); // what is handed out (an address's size, a HEAD's length) is the file as it is once authorized: // one written over in place since it was first looked at is refused, not described by that look @@ -584,17 +646,18 @@ function openAuthorized(requested: string, roots: readonly string[]): Authorized } catch { return null; } finally { - if (!kept) release(fd); + // a descriptor the pool already took back from the cell (`abandon`) is closed there + if (!kept && (cell === undefined || Atomics.compareExchange(cell, 0, fd, -1) === fd)) release(fd); } } /** This platform's proof that the file open at `fd` (found at `file`, under `root`) lies in `roots` (`openAuthorized`). */ -function openedWithin(fd: number, file: string, root: string, roots: readonly string[]): boolean { +function openedWithin(fd: number, file: string, root: string, roots: readonly string[], hidden = false): boolean { const platform = pageFileHooks.platform ?? process.platform; if (platform === "linux") { let opened: string; try { opened = (pageFileHooks.readlink ?? readlinkSync)(`/proc/self/fd/${fd}`); } catch { return false; } - return !opened.endsWith(" (deleted)") && rootOf(opened, roots) !== undefined; + return !opened.endsWith(" (deleted)") && rootOf(opened, roots, hidden) !== undefined; } if (platform === "darwin") return ownedFolders(root, file, pageFileHooks.uid ? pageFileHooks.uid() : process.getuid?.()); return platform === "win32"; @@ -700,15 +763,18 @@ interface PageAsset { target: string; mime: string; size: number; data: string | * a video or sound that does not fit is given an address to play from instead * (`PAGE_MEDIA_PATH`, up to `PAGE_MEDIA_PER_RENDER` files), the one thing this server answers the * page; each address is handed to `grant` (by default kept at once, `keepPageMedia`). Null when - * the page is not a file of at most `EMBED_PAGE_BYTES`, or changed while it was read. + * the page is not a file of at most `EMBED_PAGE_BYTES`, or changed while it was read, or, with + * `within` (a page found as one that stays in that folder, `locateFileInFolder`), when the page + * opened is not shown to lie in it as `openAuthorized` shows it. */ -export function pageWithLocalFiles(path: string, media: PageMedia | null = null, grant: (given: PageMediaGrant) => void = (given) => keepPageMedia([given])): string | null { +export function pageWithLocalFiles(path: string, media: PageMedia | null = null, grant: (given: PageMediaGrant) => void = (given) => keepPageMedia([given]), within: string | null = null): string | null { // the page's path is resolved once, before the page is opened (a page that is a link, to the file it // leads to), and the page read must be that file, proven from its descriptor (`pageOpenedAs`), never by // looking a path up again; its files come from that file's folder: a folder swapped for a link between // two lookups would otherwise serve another folder's page, or make it the folder the page's files come from let file: string; try { file = realpathSync(path); } catch { return null; } + if (within !== null && rootOf(file, [within], true) === undefined) return null; const folder = dirname(file); pageFileHooks.pageFolder?.(folder); let html: string; @@ -718,6 +784,7 @@ export function pageWithLocalFiles(path: string, media: PageMedia | null = null, try { const page = fstatBig(fd); if (!page.isFile() || page.size > BigInt(EMBED_PAGE_BYTES) || !pageOpenedAs(fd, file, page)) return null; + if (within !== null && !openedWithin(fd, file, within, [within], true)) return null; // and the asked path still leads there: a folder that was a link only while it was looked up, put // back since, made that lookup another folder's page if (realpathSync(path) !== file) return null; @@ -845,9 +912,31 @@ export function isHtmlPage(info: FileInfo): boolean { * `request` gives the method and Range: a HEAD sends no body, a GET a Range asks for gets that * range (`byteRange`). The page is rendered, and the file opened, on a file thread (`file-pool.ts`). * `watched`: a `watchPageMedia` the caller began before it looked for the file, so a revocation - * meanwhile counts too (without it, one begun here). + * meanwhile counts too (without it, one begun here). `within`: the folder a file found as one that + * stays in it came with (`locateFileInFolder`); such a file is sent only from a descriptor shown to + * lie in that folder (`openAuthorized`), and such a page only when the page opened lies there. */ -export async function fileResponse(info: FileInfo, download: boolean, render = false, media: PageMedia | null = null, request: Request | null = null, watched?: ReturnType): Promise { +/** + * Descriptors a file body names by `/proc/self/fd` (`fileResponse`), each closed once both that body + * and its answer are collected: Bun opens the body's path only when it sends it, so the descriptor + * must outlive that open, or its number could name another file by then; an answer or body still + * held may still be sent. + */ +const bodyDescriptors = new FinalizationRegistry<{ fd: number; left: number }>((held) => { + held.left -= 1; + if (held.left > 0) return; + bodiesHeld -= 1; + try { release(held.fd); } catch { /* already closed */ } +}); +/** + * Such bodies waiting to be collected; past `BODY_DESCRIPTORS` (a player seeking through a long + * video asks for many ranges before a collection) a body is streamed from its descriptor instead, + * so descriptors held for collection stay bounded. + */ +let bodiesHeld = 0; +const BODY_DESCRIPTORS = 256; + +export async function fileResponse(info: FileInfo, download: boolean, render = false, media: PageMedia | null = null, request: Request | null = null, watched?: ReturnType, within: string | null = null): Promise { const name = encodeURIComponent(info.name); const headers = new Headers({ "content-disposition": `${download ? "attachment" : "inline"}; filename*=UTF-8''${name}`, @@ -866,7 +955,7 @@ export async function fileResponse(info: FileInfo, download: boolean, render = f const watch = media === null ? null : (watched ?? watchPageMedia()); let rendered: RenderedPage; try { - rendered = await fileTask({ op: "render", path: info.path, media }, request?.signal); + rendered = await fileTask({ op: "render", path: info.path, media, within }, request?.signal); // gone, grown past the page limit or written over since it was found if (rendered.html === null) return gone(); if (watch !== null) keepPageMedia(rendered.grants, Date.now(), watch.since); @@ -882,7 +971,7 @@ export async function fileResponse(info: FileInfo, download: boolean, render = f const head = request?.method === "HEAD"; // RFC 9110 defines a range for GET alone: a HEAD describes the whole file const get = head ? null : { range: request?.method === "GET" ? request.headers.get("range") : null, ifRange: request?.headers.has("if-range") ?? false }; - const opened = await fileTask({ op: "open", path: info.path, get, fastDiskMs: pageFileHooks.fastDiskMs }, request?.signal); + const opened = await fileTask({ op: "open", path: info.path, get, within, fastDiskMs: pageFileHooks.fastDiskMs }, request?.signal); if (opened === null) return gone(); headers.set("content-type", type); headers.set("accept-ranges", "bytes"); @@ -898,9 +987,23 @@ export async function fileResponse(info: FileInfo, download: boolean, render = f if (head) return new Response(null, { status, headers }); if (opened.bytes !== null) return new Response(opened.bytes, { status, headers }); if (opened.fast) { - // a file body, which Bun sizes itself (and reads on this thread: its disk just answered at once) + // a file body, which Bun sizes itself (and reads on this thread: its disk just answered at once); + // a scratch file (`within`) by its descriptor's own path, the file that descriptor holds headers.delete("content-length"); - return new Response(Bun.file(info.path, { type }).slice(start, end + 1, type), { status, headers }); + if (opened.fd !== null && bodiesHeld >= BODY_DESCRIPTORS) { + // too many such bodies wait to be collected: this one is streamed from its descriptor, without a length + headers.set("content-length", String(length)); + return new Response(descriptorBody(opened.fd, start, length, null), { status, headers }); + } + const body = Bun.file(opened.fd === null ? info.path : `/proc/self/fd/${opened.fd}`, { type }).slice(start, end + 1, type); + const response = new Response(body, { status, headers }); + if (opened.fd !== null) { + const held = { fd: opened.fd, left: 2 }; + bodiesHeld += 1; + bodyDescriptors.register(body, held); + bodyDescriptors.register(response, held); + } + return response; } return new Response(descriptorBody(opened.fd!, start, length, null), { status, headers }); } diff --git a/server/file-worker.fake.ts b/server/file-worker.fake.ts index 816a2567f6..cce6cd615c 100644 --- a/server/file-worker.fake.ts +++ b/server/file-worker.fake.ts @@ -9,16 +9,23 @@ * - with a `readGate` (two Int32 cells and a FIFO's path), stops inside the file read itself: it * records the descriptor being read in cell 1 (`Atomics.notify`), then opens the FIFO for reading, * a real system call that blocks until a writer opens it (which ending the thread cannot cut - * short), and then reads, as a read on a stalled disk would. + * short), and then reads, as a read on a stalled disk would; + * - with an `authorizeGate` (the same shape), stops once the file is authorized, before the task + * answers, as a check on a stalled disk would: it opens the FIFO (read and write, which never + * waits), records the descriptor the task named in its cell (`opened`) in cell 1, then blocks in a + * read of the FIFO. Opened before cell 1 is set, the FIFO always has this reader once the test + * sees it, so the test's release (a write that does not wait) never waits for this thread. */ -import { closeSync, openSync, readSync } from "node:fs"; +import { closeSync, constants, openSync, readSync } from "node:fs"; import { type FileTask, runFileTask, transferablesOf } from "./file-tasks.ts"; import { pageFileHooks } from "./file-view.ts"; declare const self: Worker; -self.onmessage = (event: MessageEvent) => { +type Gate = { cells: SharedArrayBuffer; fifo: string }; + +self.onmessage = (event: MessageEvent) => { const task = event.data; if ("path" in task && (task.path.endsWith("/stuck") || (task.op === "open" && task.path.endsWith("/held-open")))) return; const readGate = task.readGate; @@ -31,8 +38,18 @@ self.onmessage = (event: MessageEvent { + const cells = new Int32Array(authorizeGate.cells); + const fifo = openSync(authorizeGate.fifo, constants.O_RDWR); + Atomics.store(cells, 1, "opened" in task && task.opened !== undefined ? task.opened[0]! : -2); + Atomics.notify(cells, 1); + try { readSync(fifo, new Uint8Array(1), 0, 1, null); } finally { closeSync(fifo); } + }; + } let result; - try { result = runFileTask(task); } finally { delete pageFileHooks.readSync; } + try { result = runFileTask(task); } finally { delete pageFileHooks.readSync; delete pageFileHooks.afterAuthorize; } if (task.gate !== undefined) { const cells = new Int32Array(task.gate); // the descriptor the thread opened, handed over or not (the cell `open` names it in)