From 9c2da96546f1eabd276bb0387834853c83353c9a Mon Sep 17 00:00:00 2001 From: Hako Date: Fri, 23 Oct 2026 11:40:33 +0900 Subject: [PATCH 1/2] test(server): pin the file routes before they move A unit-lane test through a real createServer on a temp project folder, with paneGet stubbed where a pane is involved, so it runs without herdr: GET and HEAD only and no other site's no-cors read; a file's facts and bytes by its absolute path, inline or as a download, and 404 for none; a relative path found in the named pane's folder, or not when herdr cannot say it; a folder's project icon or none, and 400 without a root; and no page media this server never gave out. --- server/file-routes.test.ts | 76 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 server/file-routes.test.ts diff --git a/server/file-routes.test.ts b/server/file-routes.test.ts new file mode 100644 index 0000000000..31ec45d7b2 --- /dev/null +++ b/server/file-routes.test.ts @@ -0,0 +1,76 @@ +import { afterAll, beforeAll, describe, expect, it, spyOn } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import * as herdr from "./herdr/client.ts"; +import { HerdrError } from "./herdr/client.ts"; +import { createServer } from "./index.ts"; + +describe("file routes", () => { + let server: ReturnType; + let stateDir: string; + let folder: string; + let notes: string; + const url = (path: string) => `http://127.0.0.1:${server.port}${path}`; + const q = (value: string) => encodeURIComponent(value); + const error = async (response: Response) => [response.status, ((await response.json()) as { error: { code: string } }).error.code]; + + beforeAll(() => { + stateDir = mkdtempSync(join(tmpdir(), "ovis-file-routes-")); + folder = join(stateDir, "project"); + mkdirSync(join(folder, "public"), { recursive: true }); + mkdirSync(join(folder, "docs")); + notes = join(folder, "notes.txt"); + writeFileSync(notes, "hello"); + writeFileSync(join(folder, "public", "favicon.ico"), new Uint8Array([0, 0, 1, 0, 1, 0, 16, 16])); + server = createServer({ port: 0, hostname: "127.0.0.1", stateDir, token: "", machines: false, sidecar: false }); + }); + afterAll(() => { + server.stop(); + rmSync(stateDir, { recursive: true, force: true }); + }); + + it("wants GET or HEAD, and no other site's page", async () => { + expect(await error(await fetch(url(`/api/fs/stat?path=${q(notes)}`), { method: "POST", headers: { origin: url("") } }))).toEqual([400, "method_not_allowed"]); + expect(await error(await fetch(url(`/api/fs/file?path=${q(notes)}`), { headers: { "sec-fetch-site": "cross-site", "sec-fetch-mode": "no-cors" } }))).toEqual([403, "cross_site"]); + expect((await fetch(url(`/api/fs/file?path=${q(notes)}`), { headers: { "sec-fetch-site": "cross-site", "sec-fetch-mode": "navigate" } })).status).toBe(200); + expect((await fetch(url(`/api/fs/file?path=${q(notes)}`), { method: "HEAD" })).status).toBe(200); + }); + + it("describes and serves a file by its absolute path, and says when there is none", async () => { + const stat = await fetch(url(`/api/fs/stat?path=${q(notes)}`)); + expect(stat.status).toBe(200); + expect(await stat.json()).toMatchObject({ name: "notes.txt", kind: "text", size: 5 }); + const file = await fetch(url(`/api/fs/file?path=${q(notes)}`)); + expect(await file.text()).toBe("hello"); + expect(file.headers.get("content-disposition")).toStartWith("inline;"); + expect((await fetch(url(`/api/fs/file?path=${q(notes)}&download=1`))).headers.get("content-disposition")).toStartWith("attachment;"); + expect(await error(await fetch(url(`/api/fs/stat?path=${q(join(folder, "missing.txt"))}`)))).toEqual([404, "not_found"]); + }); + + it("finds a relative path in the named pane's folder, and none when herdr cannot say it", async () => { + const get = spyOn(herdr, "paneGet").mockImplementation(async (paneId) => ({ pane_id: paneId, workspace_id: "w1", tab_id: "w1:t1", cwd: folder, agent_status: "idle" }) as unknown as Awaited>); + try { + const found = await fetch(url("/api/fs/stat?pane_id=w1:p1&path=notes.txt")); + expect(found.status).toBe(200); + expect(await found.json()).toMatchObject({ name: "notes.txt", size: 5 }); + get.mockImplementation(async (paneId) => { throw new HerdrError("pane_not_found", `pane ${paneId} not found`); }); + expect(await error(await fetch(url("/api/fs/stat?pane_id=w1:p1&path=notes.txt")))).toEqual([404, "not_found"]); + expect((await fetch(url(`/api/fs/stat?pane_id=w1:p1&path=${q(notes)}`))).status).toBe(200); + } finally { + get.mockRestore(); + } + }); + + it("answers a folder's project icon, or none", async () => { + expect(await error(await fetch(url("/api/fs/favicon")))).toEqual([400, "missing_root"]); + expect(await (await fetch(url(`/api/fs/favicon?root=${q(folder)}`))).json()).toMatchObject({ icon: { name: "favicon.ico", kind: "image" } }); + expect(await (await fetch(url(`/api/fs/favicon?root=${q(join(folder, "docs"))}`))).json()).toEqual({ icon: null }); + }); + + it("plays no page media it never gave out", async () => { + expect(await error(await fetch(url("/api/fs/page-media/not-a-token")))).toEqual([404, "not_found"]); + expect(await error(await fetch(url("/api/fs/page-media/not-a-token"), { method: "POST", headers: { origin: url("") } }))).toEqual([400, "method_not_allowed"]); + }); +}); From b75a80eb7d7148a165d39a910006bc4cd9b8d6c8 Mon Sep 17 00:00:00 2001 From: Hako Date: Fri, 23 Oct 2026 11:40:33 +0900 Subject: [PATCH 2/2] refactor(server): move the file routes and types into their own slice The last inline feature slice out of server/index.ts and shared/protocol.ts (plan: .omo/plans/feature-slices-plan.md). GET/HEAD /api/fs/stat, /api/fs/file, /api/fs/favicon and /api/fs/page-media/:media_id move, unchanged, into handleFileRequest in server/file-routes.ts with claudeTempFolder, which only the file read used. It is called at the same place in the dispatch order and given who asks as the access gate worked it out (bridge token, host, proxy, client address, access), since page media plays only for the client and device that rendered it. The page-media registry stays in file-view.ts, where sign-out and device revocation still forget it. FileKind, FileInfo and ProjectFavicon move verbatim into shared/files.ts (types only), and protocol.ts re-exports them. --- server/file-routes.ts | 69 +++++++++++++++++++++++++++++++++++++++++++ server/index.ts | 59 ++++-------------------------------- shared/files.ts | 29 ++++++++++++++++++ shared/protocol.ts | 30 +------------------ 4 files changed, 105 insertions(+), 82 deletions(-) create mode 100644 server/file-routes.ts create mode 100644 shared/files.ts diff --git a/server/file-routes.ts b/server/file-routes.ts new file mode 100644 index 0000000000..fd029154d0 --- /dev/null +++ b/server/file-routes.ts @@ -0,0 +1,69 @@ +/** + * The file routes: what a file is and its bytes (GET/HEAD /api/fs/stat, /api/fs/file), a folder's + * project icon (/api/fs/favicon) and a rendered page's own media (/api/fs/page-media/:media_id). + * 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 type { ProjectFavicon } from "../shared/files.ts"; +import type { Access } from "./access.ts"; +import { isSecureRequest } from "./auth.ts"; +import { claudeSessionTempDir } from "./claude-store.ts"; +import { claudePaneSession } from "./conversation.ts"; +import { fileResponse, findFile, crossSiteFileRead, pageMediaResponse, pageMediaSource, PAGE_MEDIA_PATH } from "./file-view.ts"; +import { paneGet } from "./herdr/client.ts"; +import { badRequest, jsonResponse, setRequestRoute } from "./http.ts"; +import { paneContext, resolveAgent } from "./pane-context.ts"; +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 } + +/** 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 { + const pane = await paneGet(paneId); + if (resolveAgent(pane).agent !== "claude") return null; + const session = await claudePaneSession(pane); + return session === null ? null : await claudeSessionTempDir(session.path); + } catch { return null; /* herdr busy or the pane gone: the path stays not found */ } +} + +/** The answer for a file route, or null when the path is not one. */ +export async function handleFileRequest(request: Request, url: URL, pathname: string, { bridgeAuthorized, requestHost, forwarded, client, access }: FileRequester): Promise { + if (pathname === "/api/fs/stat" || pathname === "/api/fs/file" || pathname === "/api/fs/favicon") { + setRequestRoute(pathname); + if (request.method !== "GET" && request.method !== "HEAD") return badRequest("method_not_allowed", "use GET"); + if (crossSiteFileRead(request)) return jsonResponse({ error: { code: "cross_site", message: "A file is read from this app or opened in a tab, not loaded by another page" } }, 403); + // a folder's project icon: every place it may be kept, in one answer that a miss is part of + if (pathname === "/api/fs/favicon") { + const root = url.searchParams.get("root") ?? ""; + if (root.trim() === "") return badRequest("missing_root", "root is required"); + return jsonResponse({ icon: await findProjectFavicon(root) } satisfies ProjectFavicon); + } + const paneId = url.searchParams.get("pane_id"); + let cwd: string | null = null; + if (paneId) { + 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)); + 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) : fileResponse(found.info, url.searchParams.get("download") === "1", url.searchParams.get("render") === "1", media); + } + + // a rendered page's own video, by the address its render gave it: the page has no origin and + // no cookie, so the address is all it holds (requiresAuth lets this path through) + if (pathname.startsWith(PAGE_MEDIA_PATH)) { + setRequestRoute("/api/fs/page-media/:media_id"); + if (request.method !== "GET" && request.method !== "HEAD") return badRequest("method_not_allowed", "use GET"); + const media = pageMediaResponse(pathname.slice(PAGE_MEDIA_PATH.length), client); + return media ?? jsonResponse({ error: { code: "not_found", message: "no such page media" } }, 404); + } + return null; +} diff --git a/server/index.ts b/server/index.ts index 926b6308a3..9f8c3c27d1 100644 --- a/server/index.ts +++ b/server/index.ts @@ -5,16 +5,15 @@ import { ClientConnectionLog } from "./client-connection-log.ts"; import { RequestLog } from "./request-log.ts"; import { handlePaneRequest } from "./pane-routes.ts"; import { handleIntegrationsRequest } from "./integrations-routes.ts"; -import { paneContext, resolveAgent } from "./pane-context.ts"; import { randomBytes, randomUUID } from "node:crypto"; import { OrderSequence } from "./order-sequence.ts"; import { readHerdrProfiles, type HerdrMachineProfile } from "./herdr-profiles.ts"; import { accessSync, constants, readFileSync } from "node:fs"; import { dirname, join } from "node:path"; -import type { HealthAuth, PendingMessage, ProjectFavicon } from "../shared/protocol.ts"; +import type { HealthAuth, PendingMessage } from "../shared/protocol.ts"; import { DEFAULT_PORT } from "../shared/protocol.ts"; -import { authClient, deviceCookieValue, bearerMatches, handleAuthRequest, isAuthenticated, isSecureRequest, parseCookies, presentedToken, presentedTokenHeld, recordPresentedTokenFailure, requiresAuth, unauthorizedJson } from "./auth.ts"; +import { authClient, deviceCookieValue, bearerMatches, handleAuthRequest, isAuthenticated, parseCookies, presentedToken, presentedTokenHeld, recordPresentedTokenFailure, requiresAuth, unauthorizedJson } from "./auth.ts"; import { cameThroughProxy, decideAccess, isLoopbackAddress, isLoopbackBind, isServeOwnerRequest, lanExposed } from "./access.ts"; import { DeviceStore, handleDeviceRequest, type DeviceMatch } from "./devices.ts"; import { handlePreferencesRequest, PreferenceStore } from "./preferences.ts"; @@ -23,17 +22,14 @@ import { remoteAccess, TailnetIdentitySource } from "./tailscale.ts"; import { badRequest, errorResponse, jsonResponse, setRequestRoute } from "./http.ts"; import { requestOrigin, serveStatic } from "./static.ts"; import { compressResponse } from "./compress.ts"; -import { claudePaneSession } from "./conversation.ts"; import { handleTasksRequest } from "./tasks-routes.ts"; import { handlePlanRequest } from "./plan-routes.ts"; import { ShellAgentLocator } from "./shell-agent.ts"; import { handleAgentsRequest } from "./agents-routes.ts"; -import { claudeSessionTempDir } from "./claude-store.ts"; -import { fileResponse, findFile, crossSiteFileRead, forgetPageMedia, pageMediaResponse, pageMediaSource, PAGE_MEDIA_PATH } from "./file-view.ts"; -import { findProjectFavicon } from "./project-favicon.ts"; +import { forgetPageMedia } from "./file-view.ts"; +import { handleFileRequest } from "./file-routes.ts"; import { herdrSocketPath, - paneGet, ping, } from "./herdr/client.ts"; import { type AlertTiming, defaultStateDir, handlePushRequest } from "./push.ts"; @@ -88,16 +84,6 @@ const DRAFT_CLEAR_WAIT_MS = 1_500; const DRAFT_CLEAR_POLL_MS = 50; -/** 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 { - const pane = await paneGet(paneId); - if (resolveAgent(pane).agent !== "claude") return null; - const session = await claudePaneSession(pane); - return session === null ? null : await claudeSessionTempDir(session.path); - } catch { return null; /* herdr busy or the pane gone: the path stays not found */ } -} - /** * What Bun.serve admits before a route runs (64 MiB). The ~55.9 MB body of a 40 MiB file is under it, * so the attachment route answers it with its JSON reason. The browser refuses a file over @@ -497,41 +483,8 @@ export function createServer( const integrationsResponse = await handleIntegrationsRequest(request, pathname); if (integrationsResponse) return integrationsResponse; - if (pathname === "/api/fs/stat" || pathname === "/api/fs/file" || pathname === "/api/fs/favicon") { - setRequestRoute(pathname); - if (request.method !== "GET" && request.method !== "HEAD") return badRequest("method_not_allowed", "use GET"); - if (crossSiteFileRead(request)) return jsonResponse({ error: { code: "cross_site", message: "A file is read from this app or opened in a tab, not loaded by another page" } }, 403); - // a folder's project icon: every place it may be kept, in one answer that a miss is part of - if (pathname === "/api/fs/favicon") { - const root = url.searchParams.get("root") ?? ""; - if (root.trim() === "") return badRequest("missing_root", "root is required"); - return jsonResponse({ icon: await findProjectFavicon(root) } satisfies ProjectFavicon); - } - const paneId = url.searchParams.get("pane_id"); - let cwd: string | null = null; - if (paneId) { - 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)); - 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) : fileResponse(found.info, url.searchParams.get("download") === "1", url.searchParams.get("render") === "1", media); - } - - // a rendered page's own video, by the address its render gave it: the page has no origin and - // no cookie, so the address is all it holds (requiresAuth lets this path through) - if (pathname.startsWith(PAGE_MEDIA_PATH)) { - setRequestRoute("/api/fs/page-media/:media_id"); - if (request.method !== "GET" && request.method !== "HEAD") return badRequest("method_not_allowed", "use GET"); - const media = pageMediaResponse(pathname.slice(PAGE_MEDIA_PATH.length), client); - return media ?? jsonResponse({ error: { code: "not_found", message: "no such page media" } }, 404); - } + const filesResponse = await handleFileRequest(request, url, pathname, { bridgeAuthorized, requestHost, forwarded, client, access }); + if (filesResponse) return filesResponse; const workspaceResponse = await handleWorkspaceRequest(request, url, pathname, bunServer, broadcastAll, queueFirstMessage, () => { // the receipt first, then the roster read that can show it: not waited for, and its failure changes no answer diff --git a/shared/files.ts b/shared/files.ts new file mode 100644 index 0000000000..bec999374e --- /dev/null +++ b/shared/files.ts @@ -0,0 +1,29 @@ +/** How the file viewer shows a file. */ +export type FileKind = "image" | "video" | "audio" | "pdf" | "text" | "binary"; + +/** + * GET /api/fs/stat: a file the viewer can open (GET /api/fs/file streams it). An HTML page is + * `text` with `mime` "text/html": `/api/fs/file` serves it as plain text, and as the page itself + * only with `render=1`, in a sandbox with no origin and no network of its own. + */ +export interface FileInfo { + path: string; + name: string; + size: number; + /** ISO time of the last change */ + modified: string; + mime: string; + kind: FileKind; +} + +/** + * GET /api/fs/favicon?root=: the icon the project in folder `root` keeps, or null. The PC looks + * through every place a project keeps one (`FAVICON_CANDIDATES` in server/project-favicon.ts), + * best first, each read as /api/fs/stat reads `/`, and answers once: a folder with + * no icon is `{ icon: null }`, never a 404. 400 missing_root; the access rules of /api/fs/stat + * (no watching device, no other site's page). + */ +export interface ProjectFavicon { + /** the image found; `path` opens it through /api/fs/file */ + icon: FileInfo | null; +} diff --git a/shared/protocol.ts b/shared/protocol.ts index 07cc8b249d..f7029036b4 100644 --- a/shared/protocol.ts +++ b/shared/protocol.ts @@ -666,35 +666,7 @@ export interface DirectoryCreated { path: string; } -/** How the file viewer shows a file. */ -export type FileKind = "image" | "video" | "audio" | "pdf" | "text" | "binary"; - -/** - * GET /api/fs/stat: a file the viewer can open (GET /api/fs/file streams it). An HTML page is - * `text` with `mime` "text/html": `/api/fs/file` serves it as plain text, and as the page itself - * only with `render=1`, in a sandbox with no origin and no network of its own. - */ -export interface FileInfo { - path: string; - name: string; - size: number; - /** ISO time of the last change */ - modified: string; - mime: string; - kind: FileKind; -} - -/** - * GET /api/fs/favicon?root=: the icon the project in folder `root` keeps, or null. The PC looks - * through every place a project keeps one (`FAVICON_CANDIDATES` in server/project-favicon.ts), - * best first, each read as /api/fs/stat reads `/`, and answers once: a folder with - * no icon is `{ icon: null }`, never a 404. 400 missing_root; the access rules of /api/fs/stat - * (no watching device, no other site's page). - */ -export interface ProjectFavicon { - /** the image found; `path` opens it through /api/fs/file */ - icon: FileInfo | null; -} +export type { FileKind, FileInfo, ProjectFavicon } from "./files.ts"; /** Which two states of a repository are compared. */ export type RepoDiffScope = "workspace" | "staged" | "commit";