Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/scratch-folder-open-then-check.md
Original file line number Diff line number Diff line change
@@ -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.
10 changes: 9 additions & 1 deletion server/claude-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,14 @@ export async function claudeTranscriptFile(
return path;
}

/** Where `claudeSessionTempDir` looks for this user's `claude-<uid>` folder. */
const claudeTempRoots = (): string[] => [...new Set([tmpdir(), "/tmp"])];

/** The `claude-<uid>` 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: `<tmp>/claude-<uid>/<project>/<session>/`,
Expand All @@ -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<string | null> {
if (uid === undefined || !transcript.endsWith(".jsonl")) return null;
Expand Down
21 changes: 20 additions & 1 deletion server/file-pool.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions server/file-pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export function fileTask<T extends FileTaskResult>(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<T>((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) };
Expand Down Expand Up @@ -241,7 +241,7 @@ function openedCell(): Int32Array<SharedArrayBuffer> {

/** 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 */ }
Expand Down
11 changes: 8 additions & 3 deletions server/file-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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<string | null> {
try {
Expand Down Expand Up @@ -66,13 +71,13 @@ 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);
// a page another PC's server asks for through its bridge is played from that server's address, which this one cannot name: it is drawn as before
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);
}
45 changes: 31 additions & 14 deletions server/file-tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand All @@ -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<SharedArrayBuffer> }
| { 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<SharedArrayBuffer>; fastDiskMs?: number };
| { op: "open"; path: string; get: { range: string | null; ifRange: boolean } | null; opened?: Int32Array<SharedArrayBuffer>; 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<ArrayBuffer> | null; grants: PageMediaGrant[] }
Expand All @@ -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);
}
}

Expand All @@ -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 };
}

Expand All @@ -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<SharedArrayBuffer> | undefined, fastDiskMs: number): OpenedFile | null {
function open(path: string, get: { range: string | null; ifRange: boolean } | null, opened: Int32Array<SharedArrayBuffer> | 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;
Expand All @@ -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 };
}
Expand Down
Loading
Loading