From 8380aa33a7102763d473a26277cc66de1455661d Mon Sep 17 00:00:00 2001 From: Harbor404 <2657212322@qq.com> Date: Thu, 1 Oct 2026 22:35:23 +0800 Subject: [PATCH 1/3] feat(computer): download generated workspace files --- agent-computer/src/index.ts | 40 ++++- agent-computer/src/workspace.ts | 48 ++++- agent-computer/tests/authorisation.test.ts | 1 + .../tests/file-download-http.test.ts | 164 ++++++++++++++++++ .../tests/workspace-download.test.ts | 107 ++++++++++++ agent-computer/tests/workspace.test.ts | 2 + server/src/computer/client.ts | 94 +++++++++- server/src/computer/gateway.ts | 60 ++++++- server/src/computer/policy.ts | 5 +- server/src/computer/routes.ts | 90 +++++++--- server/src/computer/schema.ts | 13 +- server/tests/computer-client.test.ts | 53 ++++++ server/tests/computer-download-routes.test.ts | 159 +++++++++++++++++ server/tests/computer-gateway.test.ts | 101 +++++++++++ shared/file-download.test.ts | 32 ++++ shared/file-download.ts | 65 +++++++ 16 files changed, 985 insertions(+), 49 deletions(-) create mode 100644 agent-computer/tests/file-download-http.test.ts create mode 100644 agent-computer/tests/workspace-download.test.ts create mode 100644 server/tests/computer-download-routes.test.ts create mode 100644 shared/file-download.test.ts create mode 100644 shared/file-download.ts diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index 1eebdb851..279b9b2b4 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -2,6 +2,7 @@ import { mkdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { serve } from "bun"; import type { Page } from "playwright"; +import { downloadHeaders } from "../../shared/file-download"; import { cutAtCodeUnits, parseAriaSnapshot, @@ -9,9 +10,9 @@ import { } from "./aria-snapshot"; import { actsOnTheComputer, - mutatesBrowser, isOpenPath, matchesToken, + mutatesBrowser, offeredToken, } from "./authorisation"; import { isPlainBotId } from "./bot-id"; @@ -20,8 +21,8 @@ import { detectChallenge } from "./challenge"; import { ControlError, ControlRequestError, - SnapshotRequiredError, NO_SECRET_PENDING, + SnapshotRequiredError, TAKE_CONTROL_FIRST, } from "./control"; import { identity } from "./identity"; @@ -44,6 +45,8 @@ import { startVirtualDisplay } from "./virtual-display"; import { createWorkspace, WorkspaceFileError, + WorkspaceFileNotFoundError, + WorkspaceFileTooLargeError, WorkspacePathError, } from "./workspace"; @@ -55,9 +58,9 @@ import { * audit row before calling this process. This process has no policy engine and no audit trail of its * own; its direct-port boundary is the computer token. * - * `/files/read` and `/files/write` reach the durable workspace volume, confined to - * it by workspace.ts. Reading and writing are the two operations a Bot needs to keep notes between - * turns. + * `/files/read`, `/files/download` and `/files/write` reach the durable workspace volume, confined + * to it by workspace.ts. Reading and writing are the two operations a Bot needs to keep notes between + * turns; download returns the exact bytes so a PDF, image or archive is not damaged by text decoding. * * Elements are addressed by reference, not by pixel. `/snapshot` stamps every interactive element * with a ref and hands back a compact list; `/click` and `/type` take one of those refs. That is the @@ -1006,6 +1009,22 @@ serve({ // The Bot's files. Confined to the workspace by workspace.ts. Nothing here decides whether a Bot // MAY touch a path: the gateway in front of this process does that. + if (url.pathname === "/files/download" && request.method === "GET") { + try { + const file = await workspace.download( + url.searchParams.get("path") ?? "", + ); + return new Response(file.body, { + headers: downloadHeaders(file.name, file.bytes), + }); + } catch (error) { + return json( + { error: describe(error, "The file could not be downloaded.") }, + fileStatus(error), + ); + } + } + if (url.pathname === "/files/read" && request.method === "POST") { const body = (await request.json().catch(() => null)) as { path?: unknown; @@ -1420,12 +1439,15 @@ function describe(error: unknown, fallback: string): string { * Which status a file failure deserves. * * A path outside the workspace is the caller asking for something it may never have, so 403: retrying - * it unchanged will never work, and it is not a fault. A missing file or an oversized write is a 400, - * because a different request would succeed. Collapsing both into 500 would tell the Bot the computer - * is broken and invite it to try the same thing again. + * it unchanged will never work, and it is not a fault. A missing download is 404, an oversized + * download is 413, and an ordinary bad file request is 400, because a different request could + * succeed. Collapsing these into 500 would tell the caller the computer is broken and invite a retry + * of the same request. */ -function fileStatus(error: unknown): 400 | 403 | 500 { +function fileStatus(error: unknown): 400 | 403 | 404 | 413 | 500 { if (error instanceof WorkspacePathError) return 403; + if (error instanceof WorkspaceFileTooLargeError) return 413; + if (error instanceof WorkspaceFileNotFoundError) return 404; if (error instanceof WorkspaceFileError) return 400; return 500; } diff --git a/agent-computer/src/workspace.ts b/agent-computer/src/workspace.ts index 38390ef71..ad9c90c7c 100644 --- a/agent-computer/src/workspace.ts +++ b/agent-computer/src/workspace.ts @@ -55,6 +55,10 @@ export class WorkspaceFileError extends Error { } } +export class WorkspaceFileNotFoundError extends WorkspaceFileError {} + +export class WorkspaceFileTooLargeError extends WorkspaceFileError {} + export type WorkspaceLimits = { /** * Most bytes a read hands back. @@ -67,6 +71,8 @@ export type WorkspaceLimits = { writeBytes: number; /** Most entries a listing describes, so a Bot cannot paste a whole disk into its own context. */ listEntries: number; + /** Most bytes one download may stream, so a large artifact cannot exhaust the connection. */ + downloadBytes: number; }; /** @@ -88,6 +94,7 @@ export const DEFAULT_WORKSPACE_LIMITS: WorkspaceLimits = { readBytes: 64_000, writeBytes: 1_000_000, listEntries: 500, + downloadBytes: 100 * 1024 * 1024, }; export function createWorkspace( @@ -134,7 +141,7 @@ export function createWorkspace( realAnchor = await realpath(anchor); } catch { if (!forWrite) { - throw new WorkspaceFileError(`There is no file at ${wanted}.`); + throw new WorkspaceFileNotFoundError(`There is no file at ${wanted}.`); } // The parent directory does not exist yet. Walk up to the nearest one that does and verify it, // so a write into a new subdirectory is allowed but cannot be aimed through a symlink. @@ -281,6 +288,45 @@ export function createWorkspace( }; }, + /** + * Open a file as raw bytes for a download. + * + * The same `resolvePath` used by text reads enforces the workspace boundary, including symlink + * resolution. The file is streamed from disk rather than read into a string, so PDFs, images and + * archives are returned byte-for-byte and the text read limit does not apply. + */ + async download(requested: string): Promise<{ + path: string; + name: string; + bytes: number; + body: Blob; + }> { + const full = await resolvePath(requested, false); + const info = await stat(full).catch(() => null); + if (!info) { + throw new WorkspaceFileNotFoundError( + `There is no file at ${requested}.`, + ); + } + if (!info.isFile()) { + throw new WorkspaceFileError(`${requested} is not a file.`); + } + if (info.size > limits.downloadBytes) { + throw new WorkspaceFileTooLargeError( + `That file is ${info.size} bytes and the download limit is ${limits.downloadBytes}.`, + ); + } + + return { + path: requested, + name: basename(requested.replace(/\\/g, "/")) || "download", + bytes: info.size, + // Bun serves this file without reading it into memory, and preserves the byte length a + // ReadableStream response loses. The caller still receives the raw bytes unchanged. + body: Bun.file(full), + }; + }, + /** Write a text file, creating parent directories inside the workspace as needed. */ async write( requested: string, diff --git a/agent-computer/tests/authorisation.test.ts b/agent-computer/tests/authorisation.test.ts index 34fcfdc4b..02927ebcc 100644 --- a/agent-computer/tests/authorisation.test.ts +++ b/agent-computer/tests/authorisation.test.ts @@ -85,6 +85,7 @@ describe("what an unauthenticated caller may reach", () => { "/snapshot", "/files/list", "/files/read", + "/files/download", "/stream", "/live", "/", diff --git a/agent-computer/tests/file-download-http.test.ts b/agent-computer/tests/file-download-http.test.ts new file mode 100644 index 000000000..3a025d72c --- /dev/null +++ b/agent-computer/tests/file-download-http.test.ts @@ -0,0 +1,164 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { + mkdir, + mkdtemp, + rm, + symlink, + truncate, + writeFile, +} from "node:fs/promises"; +import { createServer } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { DEFAULT_WORKSPACE_LIMITS } from "../src/workspace"; + +const asked = process.env.OPENBOT_FILE_DOWNLOAD_HTTP === "1"; +const TOKEN = "file-download-http-test-token"; +const BOT = "file-download-http"; + +let root = ""; +let base = ""; +let child: ReturnType | undefined; + +async function freePort(): Promise { + return new Promise((resolve, reject) => { + const probe = createServer(); + probe.on("error", reject); + probe.listen(0, "127.0.0.1", () => { + const address = probe.address(); + if (!address || typeof address === "string") { + reject(new Error("The port probe did not return a TCP address.")); + return; + } + probe.close(() => resolve(address.port)); + }); + }); +} + +function api(path: string, authenticated = true) { + return fetch(`${base}${path}`, { + headers: authenticated + ? { + "x-openbot-bot-id": BOT, + "x-openbot-computer-token": TOKEN, + } + : {}, + }); +} + +beforeAll(async () => { + if (!asked) return; + root = await mkdtemp(join(tmpdir(), "file-download-http-")); + const workspace = join(root, "workspace"); + await mkdir(join(workspace, "folder"), { recursive: true }); + const port = await freePort(); + base = `http://127.0.0.1:${port}`; + child = Bun.spawn([process.execPath, "src/index.ts"], { + cwd: join(import.meta.dir, ".."), + env: { + ...process.env, + COMPUTER_TOKEN: TOKEN, + COMPUTER_BROWSER_BACKEND: "managed", + COMPUTER_BROWSER_MODE: "headless", + PORT: String(port), + PROFILES_DIR: join(root, "profiles"), + WORKSPACE_DIR: workspace, + }, + stdout: "ignore", + stderr: "inherit", + }); + const end = Date.now() + 10_000; + for (;;) { + if (child.exitCode !== null) + throw new Error(`Computer fixture exited with ${child.exitCode}.`); + try { + if ((await fetch(`${base}/health`)).ok) break; + } catch {} + if (Date.now() > end) + throw new Error("Timed out waiting for the computer fixture."); + await Bun.sleep(10); + } +}); + +afterAll(async () => { + if (!asked) return; + child?.kill(); + if (child) await child.exited; + await rm(root, { recursive: true, force: true }); +}); + +describe.skipIf(!asked)("the computer file download endpoint", () => { + test("streams binary bytes and safe attachment headers", async () => { + const payload = Buffer.alloc(70_000); + for (let index = 0; index < payload.length; index += 1) { + payload[index] = index % 251; + } + await mkdir(join(root, "workspace", "reports"), { recursive: true }); + await writeFile(join(root, "workspace", "reports", "data.bin"), payload); + + const response = await api( + `/files/download?path=${encodeURIComponent("reports/data.bin")}`, + ); + + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe( + "application/octet-stream", + ); + expect(response.headers.get("content-length")).toBe(String(payload.length)); + expect(response.headers.get("content-disposition")).toContain( + 'filename="data.bin"', + ); + expect(response.headers.get("x-content-type-options")).toBe("nosniff"); + const bytes = Buffer.from(await response.arrayBuffer()); + expect(bytes.equals(payload)).toBeTrue(); + }); + + test("refuses unauthenticated downloads", async () => { + const response = await api("/files/download?path=anything.bin", false); + expect(response.status).toBe(401); + }); + + test.each([ + ["a missing file", "missing.bin", 404], + ["a directory", "folder", 400], + ["parent traversal", "../outside.txt", 403], + ["an absolute path", "/etc/passwd", 403], + ])("refuses %s with %i", async (_name, path, expected) => { + const response = await api( + `/files/download?path=${encodeURIComponent(path)}`, + ); + expect(response.status).toBe(expected); + }); + + test("refuses a symlink escape with 403", async () => { + const outside = join(root, "outside.txt"); + await writeFile(outside, "private"); + await symlink(outside, join(root, "workspace", "leak.bin")); + + const response = await api("/files/download?path=leak.bin"); + expect(response.status).toBe(403); + }); + + test("refuses a file above the cap with 413", async () => { + const oversized = join(root, "workspace", "oversized.bin"); + await writeFile(oversized, ""); + await truncate(oversized, DEFAULT_WORKSPACE_LIMITS.downloadBytes + 1); + + const response = await api("/files/download?path=oversized.bin"); + expect(response.status).toBe(413); + }); + + test("neutralises CRLF in the filename header", async () => { + await writeFile( + join(root, "workspace", "evil\r\nX-Injected: yes.txt"), + "safe", + ); + const response = await api( + `/files/download?path=${encodeURIComponent("evil\r\nX-Injected: yes.txt")}`, + ); + + expect(response.status).toBe(200); + expect(response.headers.get("content-disposition")).not.toMatch(/[\r\n]/); + expect(response.headers.get("x-injected")).toBeNull(); + }); +}); diff --git a/agent-computer/tests/workspace-download.test.ts b/agent-computer/tests/workspace-download.test.ts new file mode 100644 index 000000000..214a6fc30 --- /dev/null +++ b/agent-computer/tests/workspace-download.test.ts @@ -0,0 +1,107 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + createWorkspace, + WorkspaceFileError, + WorkspaceFileNotFoundError, + WorkspaceFileTooLargeError, + WorkspacePathError, +} from "../src/workspace"; + +let root: string; +let outside: string; + +beforeEach(async () => { + const base = await mkdtemp(join(tmpdir(), "openbot-workspace-download-")); + root = join(base, "workspace"); + outside = join(base, "outside"); + await mkdir(root, { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(join(outside, "secret.txt"), "a private key", "utf8"); +}); + +afterEach(async () => { + await rm(join(root, ".."), { recursive: true, force: true }); +}); + +function workspace() { + return createWorkspace(root); +} + +async function downloadedBytes(body: Blob) { + return Buffer.from(await new Response(body).arrayBuffer()); +} + +describe("downloading files from the workspace", () => { + test("returns every byte without the text read limit", async () => { + const payload = Buffer.alloc(70_000); + for (let index = 0; index < payload.length; index += 1) { + payload[index] = index % 251; + } + await mkdir(join(root, "reports"), { recursive: true }); + await writeFile(join(root, "reports", "binary.dat"), payload); + + const download = await workspace().download("reports/binary.dat"); + + expect(download.path).toBe("reports/binary.dat"); + expect(download.name).toBe("binary.dat"); + expect(download.bytes).toBe(payload.length); + expect((await downloadedBytes(download.body)).equals(payload)).toBeTrue(); + }); + + test("refuses a file above the download limit before returning a stream", async () => { + const ws = createWorkspace(root, { + readBytes: 64_000, + writeBytes: 1_000, + listEntries: 500, + downloadBytes: 8, + }); + await writeFile(join(root, "too-big.bin"), "123456789"); + + await expect(ws.download("too-big.bin")).rejects.toThrow( + WorkspaceFileTooLargeError, + ); + }); + + test("refuses a directory rather than streaming a platform-defined error", async () => { + await mkdir(join(root, "folder")); + await expect(workspace().download("folder")).rejects.toThrow( + WorkspaceFileError, + ); + }); + + test("reports a missing file as not found", async () => { + await expect(workspace().download("missing.bin")).rejects.toThrow( + WorkspaceFileNotFoundError, + ); + }); + + test("refuses a symlink that escapes the workspace", async () => { + await symlink(join(outside, "secret.txt"), join(root, "leak.bin")); + + await expect(workspace().download("leak.bin")).rejects.toThrow( + WorkspacePathError, + ); + }); + + test("allows a symlink that still points inside the workspace", async () => { + await writeFile(join(root, "real.bin"), "inside"); + await symlink(join(root, "real.bin"), join(root, "alias.bin")); + + const download = await workspace().download("alias.bin"); + expect((await downloadedBytes(download.body)).toString("utf8")).toBe( + "inside", + ); + }); + + test("refuses parent traversal and absolute paths", async () => { + await expect(workspace().download("../outside/secret.txt")).rejects.toThrow( + WorkspacePathError, + ); + await expect(workspace().download("/etc/passwd")).rejects.toThrow( + WorkspacePathError, + ); + }); +}); diff --git a/agent-computer/tests/workspace.test.ts b/agent-computer/tests/workspace.test.ts index f95001e71..67cb389df 100644 --- a/agent-computer/tests/workspace.test.ts +++ b/agent-computer/tests/workspace.test.ts @@ -80,6 +80,7 @@ describe("reading and writing inside the workspace", () => { readBytes: 10, writeBytes: 1000, listEntries: 500, + downloadBytes: 1000, }); await ws.write("long.txt", "0123456789ABCDEF"); const read = await ws.read("long.txt"); @@ -95,6 +96,7 @@ describe("reading and writing inside the workspace", () => { readBytes: 1000, writeBytes: 8, listEntries: 500, + downloadBytes: 1000, }); await expect(ws.write("big.txt", "far too long")).rejects.toThrow( WorkspaceFileError, diff --git a/server/src/computer/client.ts b/server/src/computer/client.ts index 058f0d4b9..3fb61ec4e 100644 --- a/server/src/computer/client.ts +++ b/server/src/computer/client.ts @@ -58,6 +58,28 @@ export class WorkspaceRequestError extends Error { } } +/** The workspace has no file at the requested path. */ +export class WorkspaceNotFoundError extends Error { + constructor(reason: string) { + super(reason); + this.name = "WorkspaceNotFoundError"; + } +} + +/** The requested workspace file is larger than the download limit. */ +export class WorkspaceTooLargeError extends Error { + constructor(reason: string) { + super(reason); + this.name = "WorkspaceTooLargeError"; + } +} + +/** Raw bytes and their size, kept as a stream until the caller sends them onward. */ +export type ComputerDownload = { + body: ReadableStream; + bytes: number; +}; + /** The page changed after the caller received its element references. */ export class StaleSnapshotError extends Error { constructor( @@ -138,6 +160,13 @@ export interface ComputerTransport { url: string, toolCallId?: string, ): Promise; + download( + baseUrl: string, + botId: string, + path: string, + caller?: AbortSignal, + timeoutMs?: number, + ): Promise; } /** @@ -152,14 +181,14 @@ export function createComputerTransport( const doFetch = options.fetchImpl ?? fetch; const defaultTimeoutMs = options.timeoutMs ?? 45_000; - async function call( + async function request( baseUrl: string, botId: string, path: string, init?: RequestInit, caller?: AbortSignal, timeoutMsOverride?: number, - ): Promise { + ): Promise { if (caller?.aborted) { throw new ComputerStoppedError("The action was stopped."); } @@ -173,7 +202,6 @@ export function createComputerTransport( * this becomes the backstop rather than the limit. */ const timeoutMs = timeoutMsOverride ?? defaultTimeoutMs; - const target = baseUrl.replace(/\/$/, ""); let response: Response; try { @@ -209,7 +237,25 @@ export function createComputerTransport( : "The assistant's computer is not running.", ); } + return response; + } + async function call( + baseUrl: string, + botId: string, + path: string, + init?: RequestInit, + caller?: AbortSignal, + timeoutMsOverride?: number, + ): Promise { + const response = await request( + baseUrl, + botId, + path, + init, + caller, + timeoutMsOverride, + ); const body = (await response.json().catch(() => null)) as Record< string, unknown @@ -220,6 +266,40 @@ export function createComputerTransport( return body as T; } + async function download( + baseUrl: string, + botId: string, + path: string, + caller?: AbortSignal, + timeoutMs?: number, + ): Promise { + const response = await request( + baseUrl, + botId, + path, + { method: "GET" }, + caller, + timeoutMs, + ); + if (!response.ok) { + const body = (await response.json().catch(() => null)) as Record< + string, + unknown + > | null; + throwMappedError(response.status, body); + } + + const length = response.headers.get("content-length"); + const bytes = + length !== null && /^\d+$/.test(length) ? Number(length) : Number.NaN; + if (!response.body || !Number.isSafeInteger(bytes) || bytes < 0) { + throw new ComputerUnavailableError( + "The assistant's computer returned an invalid file download.", + ); + } + return { body: response.body, bytes }; + } + function post( baseUrl: string, botId: string, @@ -260,7 +340,7 @@ export function createComputerTransport( }); } - return { call, post, navigate }; + return { call, post, navigate, download }; } /** Map agent-computer responses to errors that a caller can act on. */ @@ -294,6 +374,12 @@ function throwMappedError( if (status === 400) { throw new WorkspaceRequestError(detail); } + if (status === 404) { + throw new WorkspaceNotFoundError(detail); + } + if (status === 413) { + throw new WorkspaceTooLargeError(detail); + } if (/waiting for locator|Timeout .* exceeded/i.test(detail)) { const ref = detail.match(/aria-ref=([A-Za-z0-9_-]+)/)?.[1]; throw new ElementNotFoundError( diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index c1be93812..1cee2382e 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -29,12 +29,14 @@ import { checkComputerAddress } from "./target"; export { ComputerUnavailableError, ElementNotFoundError, - HumanHasControlError, HandoffRequestError, + HumanHasControlError, NavigationRefusedError, StaleSnapshotError, + WorkspaceNotFoundError, WorkspaceRefusedError, WorkspaceRequestError, + WorkspaceTooLargeError, } from "./client"; import type { PageFrameStore } from "./page-frames"; @@ -42,8 +44,8 @@ import { type ActionPolicy, evaluateActionPolicy, type PolicyContext, - policyInitiator, type PolicyDecision, + policyInitiator, } from "./policy"; import type { ComputerProvider } from "./provider"; import type { @@ -51,6 +53,7 @@ import type { ClickInput, ComputerStatus, ControlState, + DownloadFileInput, HumanInput, HumanInputResult, KeyInput, @@ -166,6 +169,16 @@ export interface ComputerGateway { actor: ActionActor, input: ReadFileInput, ): Promise; + downloadFile( + botId: string, + actor: ActionActor, + input: DownloadFileInput, + signal?: AbortSignal, + ): Promise<{ + body: ReadableStream; + bytes: number; + name: string; + }>; listFiles( botId: string, actor: ActionActor, @@ -344,6 +357,13 @@ export function createComputerGateway( */ const COMMAND_BACKSTOP_MS = 615_000; + /* + * A download has no command-like budget of its own. The computer refuses oversized files before + * streaming, but a large permitted file can still outlive the ordinary request deadline on a slow + * link. Five minutes is a backstop, not a size limit. + */ + const DOWNLOAD_BACKSTOP_MS = 300_000; + /** Read-only, so it passes straight through. Nothing has changed and there is nothing to decide. */ async function screenshot(botId: string): Promise { return get(botId, "/screenshot"); @@ -993,6 +1013,40 @@ export function createComputerGateway( ); }, + /** + * The bytes leave the workspace, which is a different permission from letting a Bot read them. + * + * The decision and audit row use `computer_download_file` / `download_file`; a deployment can + * therefore allow `computer_read_file` while refusing this, or the reverse. The computer returns + * a stream, so the size limit is enforced there before any bytes are sent. + */ + downloadFile( + botId: string, + actor: ActionActor, + input: DownloadFileInput, + signal?: AbortSignal, + ) { + return govern( + "computer_download_file", + botId, + actor, + { filePath: input.path, ...(signal ? { signal } : {}) }, + async () => { + const download = await transport.download( + await locate(botId), + botId, + `/files/download?path=${encodeURIComponent(input.path)}`, + signal, + DOWNLOAD_BACKSTOP_MS, + ); + return { + ...download, + name: describeFile(input.path).name || "download", + }; + }, + ); + }, + /** * Listing is governed too, and for the same reason the read is: what a Bot has accumulated over * every task it has run is worth being able to restrict. A rule denying a folder hides it from the @@ -1122,6 +1176,8 @@ export function intentOf( return "read"; case "computer_read_file": return "read_file"; + case "computer_download_file": + return "download_file"; case "computer_write_file": return "write_file"; case "computer_run_command": diff --git a/server/src/computer/policy.ts b/server/src/computer/policy.ts index 0318de503..9d5a9c9f9 100644 --- a/server/src/computer/policy.ts +++ b/server/src/computer/policy.ts @@ -78,7 +78,7 @@ export type PolicyContext = { * `type`, text going into a field, including any other keypress. * `navigate`, opening a page. * `read`, looking at the page or listing what is on it. - * `write_file` / `read_file` / `list_files`, the workspace. + * `write_file` / `read_file` / `download_file` / `list_files`, the workspace. * * It still cannot see whether a keypress will submit a form, only that one is coming: a type * carrying `submit` reports `activate` because it ends in Enter, but a browser submits @@ -93,6 +93,7 @@ export type PolicyContext = { | "navigate" | "read" | "read_file" + | "download_file" | "write_file" | "list_files" // A tool on somebody else's MCP server. Split by effect for the same reason as the browser @@ -102,7 +103,7 @@ export type PolicyContext = { | "write_tool" | "run_command"; /** - * The file a `computer_read_file` or `computer_write_file` call is aimed at. + * The file a `computer_read_file`, `computer_download_file` or `computer_write_file` call is aimed at. * * The path is as the Bot asked for it, relative to its workspace. Containment is not policy: a path * that tries to escape is refused by the computer itself and is not negotiable. A rule here is about diff --git a/server/src/computer/routes.ts b/server/src/computer/routes.ts index 5c49df936..3f4eee625 100644 --- a/server/src/computer/routes.ts +++ b/server/src/computer/routes.ts @@ -1,5 +1,6 @@ import type { Context, MiddlewareHandler } from "hono"; import { Hono } from "hono"; +import { downloadHeaders } from "../../../shared/file-download"; import type { BotAccessCheck } from "../agents/profile-policy"; import type { AuditReader } from "../audit"; import type { AppVariables } from "../auth/guards"; @@ -11,12 +12,14 @@ import { type ComputerGateway, ComputerUnavailableError, ElementNotFoundError, - HumanHasControlError, HandoffRequestError, + HumanHasControlError, NavigationRefusedError, StaleSnapshotError, + WorkspaceNotFoundError, WorkspaceRefusedError, WorkspaceRequestError, + WorkspaceTooLargeError, } from "./gateway"; import type { PageFrameStore } from "./page-frames"; import { dryRunAgainstHistory, REPLAYABLE_EVENT_TYPES } from "./policy-dry-run"; @@ -600,6 +603,35 @@ export function createComputerRoutes( }), ); + /** + * A person downloading a file the Bot generated. + * + * This is a GET because a browser download is a navigation, not a JSON action. It still goes + * through the gateway, so the separate `computer_download_file` intent is decided and audited + * before the computer is asked for bytes. Ownership is enforced by the `/:botId/*` middleware + * above, and the response is always an opaque attachment so HTML cannot render on this origin. + */ + routes.get("/:botId/files/download", async (context) => { + const path = context.req.query("path"); + if (!path?.trim()) { + return context.json({ error: "A file path is required." }, 400); + } + + try { + const file = await gateway.downloadFile( + context.req.param("botId"), + actorOf(context.var.actor), + { path: path.trim() }, + context.req.raw.signal, + ); + return new Response(file.body, { + headers: downloadHeaders(file.name, file.bytes), + }); + } catch (error) { + return actionFailure(context, error); + } + }); + /* * A command on the Bot's computer. * @@ -842,13 +874,7 @@ async function act( try { const result = await handler( botId, - { - id: record.id, - // Only a real users row may go in the audit table's foreign key column. The local development - // actor is not one, so writing it there fails the constraint and loses the row entirely. Who - // it was is recorded in the payload regardless. See gateway.ts. - ...(record.email === DEV_ACTOR_EMAIL ? {} : { userId: record.id }), - }, + actorOf(record), body, context.req.raw.signal, ); @@ -857,23 +883,7 @@ async function act( } return context.json(result as Record); } catch (error) { - // A policy refusal is the product working. 403 with the rule that refused it, so the surface can - // tell the person which boundary they met rather than reporting a malfunction. - if (error instanceof ActionRefusedError) { - return context.json({ error: error.message, rule: error.rule }, 403); - } - // The computer refused the path itself, which is a different thing from the policy refusing this - // Bot. Same status, no rule attached, because there is no rule to go and edit. - if (error instanceof WorkspaceRefusedError) { - return context.json({ error: error.message }, 403); - } - // A 400, deliberately, NOT a 403. The surface treats 403 as "a boundary refused you" and renders - // it as Blocked, so returning it for "there is no file at notes.md" told both the person and the - // model that a policy had intervened when none had. - if (error instanceof WorkspaceRequestError) { - return context.json({ error: error.message }, 400); - } - return context.json(errorBody(error), statusFor(error)); + return actionFailure(context, error); } } @@ -885,6 +895,30 @@ async function act( */ const DEV_ACTOR_EMAIL = "dev@openbot.local"; +/** The audit identity derived from the signed-in actor, with the same FK rule as every acting call. */ +function actorOf(record: AppVariables["actor"]): ActionActor { + return { + id: record.id, + // Only a real users row may go in the audit table's foreign key column. The local development + // actor is not one, so writing it there fails the constraint and loses the row entirely. Who + // it was is recorded in the payload regardless. See gateway.ts. + ...(record.email === DEV_ACTOR_EMAIL ? {} : { userId: record.id }), + }; +} + +/** + * One failure shape for JSON acting and binary download routes. + * + * A policy refusal names the rule; a workspace refusal does not, because there is no rule to edit. + * Keeping both here means the download route cannot accidentally turn a boundary into a 500. + */ +function actionFailure(context: ComputerContext, error: unknown) { + if (error instanceof ActionRefusedError) { + return context.json({ error: error.message, rule: error.rule }, 403); + } + return context.json(errorBody(error), statusFor(error)); +} + function isBadRequest(value: unknown): value is BadRequest { return ( !!value && @@ -965,7 +999,7 @@ function handoffId(body: Record | null): string { return body.requestId; } -function statusFor(error: unknown): 400 | 404 | 409 | 500 | 503 { +function statusFor(error: unknown): 400 | 403 | 404 | 409 | 413 | 500 | 503 { if (error instanceof HandoffRequestError) return error.status; if (error instanceof StaleSnapshotError) return 409; // Same status as a stale snapshot and for the same reason: nothing is broken, the caller has to do @@ -983,6 +1017,10 @@ function statusFor(error: unknown): 400 | 404 | 409 | 500 | 503 { ) { return 409; } + if (error instanceof WorkspaceRefusedError) return 403; + if (error instanceof WorkspaceRequestError) return 400; + if (error instanceof WorkspaceNotFoundError) return 404; + if (error instanceof WorkspaceTooLargeError) return 413; if (error instanceof ComputerUnavailableError) return 503; return 500; } diff --git a/server/src/computer/schema.ts b/server/src/computer/schema.ts index 57055252f..eb5c729bc 100644 --- a/server/src/computer/schema.ts +++ b/server/src/computer/schema.ts @@ -30,6 +30,7 @@ export const COMPUTER_TOOLS = [ "computer_key", "computer_scroll", "computer_read_file", + "computer_download_file", "computer_write_file", "computer_list_files", ] as const; @@ -40,11 +41,11 @@ export const COMPUTER_TOOLS = [ * These are the calls the gateway must decide on. Reading a PAGE a Bot has already been allowed to * open is not a new decision; clicking "Confirm payment" on it is. * - * Both file tools are here, including the read. That differs from `computer_read`, which - * is ungoverned, and it is deliberate: a page was already permitted when it was opened, whereas the - * workspace accumulates whatever a Bot has put in it across every task it has ever run, so "which - * files may this Bot read" is a question a deployment genuinely needs to be able to answer. The build - * doc says both file tools go through the gateway, and this is why that is right. + * Every workspace file tool is here, including the read and the download. That differs from + * `computer_read`, which is ungoverned, and it is deliberate: a page was already permitted when it + * was opened, whereas the workspace accumulates whatever a Bot has put in it across every task it + * has ever run, so "which files may this Bot read, list, download or write" is a question a + * deployment genuinely needs to be able to answer. */ export const COMPUTER_ACTING_TOOLS = [ // Navigation is governed too, and not only guarded. The client's target guard refuses a forbidden @@ -57,6 +58,7 @@ export const COMPUTER_ACTING_TOOLS = [ "computer_key", "computer_scroll", "computer_read_file", + "computer_download_file", "computer_write_file", "computer_list_files", ] as const; @@ -206,6 +208,7 @@ export type ListFilesResult = { }; export type ReadFileInput = { path: string }; +export type DownloadFileInput = { path: string }; export type ReadFileResult = { path: string; text: string; diff --git a/server/tests/computer-client.test.ts b/server/tests/computer-client.test.ts index ce9059eec..5e57f461e 100644 --- a/server/tests/computer-client.test.ts +++ b/server/tests/computer-client.test.ts @@ -7,6 +7,8 @@ import { HumanHasControlError, NavigationRefusedError, StaleSnapshotError, + WorkspaceNotFoundError, + WorkspaceTooLargeError, } from "../src/computer/client"; function clientWith( @@ -25,6 +27,12 @@ function clientWith( screenshot: () => transport.call(baseUrl, botId, "/screenshot"), click: (input: unknown, signal?: AbortSignal) => transport.post(baseUrl, botId, "/click", input, signal), + download: (path: string) => + transport.download( + baseUrl, + botId, + `/files/download?path=${encodeURIComponent(path)}`, + ), }; } @@ -35,6 +43,51 @@ const ok = (body: unknown) => }); describe("computer client", () => { + describe("computer file downloads", () => { + test("returns raw bytes without decoding them as JSON", async () => { + const payload = Uint8Array.from([0, 255, 1, 2, 3]); + const seen: string[] = []; + const client = clientWith((url) => { + seen.push(url); + return new Response(payload, { + headers: { "content-length": String(payload.byteLength) }, + }); + }); + + const download = await client.download("reports/data.bin"); + + expect(download.bytes).toBe(payload.byteLength); + expect( + new Uint8Array(await new Response(download.body).arrayBuffer()), + ).toEqual(payload); + expect(seen).toEqual([ + "http://agent-computer:4100/files/download?path=reports%2Fdata.bin", + ]); + }); + + test("refuses a successful response with no byte length", async () => { + const client = clientWith(() => new Response(Uint8Array.from([1, 2, 3]))); + + await expect(client.download("x")).rejects.toThrow( + ComputerUnavailableError, + ); + }); + + test.each([ + [404, WorkspaceNotFoundError], + [413, WorkspaceTooLargeError], + ])("maps HTTP %i to its workspace error", async (status, errorType) => { + const client = clientWith(() => + Response.json( + { error: "The file could not be downloaded." }, + { status }, + ), + ); + + await expect(client.download("x")).rejects.toBeInstanceOf(errorType); + }); + }); + test("navigates and returns where it landed", async () => { const seen: string[] = []; const client = clientWith((url, init) => { diff --git a/server/tests/computer-download-routes.test.ts b/server/tests/computer-download-routes.test.ts new file mode 100644 index 000000000..8647b3195 --- /dev/null +++ b/server/tests/computer-download-routes.test.ts @@ -0,0 +1,159 @@ +import { describe, expect, test } from "bun:test"; +import type { MiddlewareHandler } from "hono"; +import type { AppVariables, AuthenticatedActor } from "../src/auth/guards"; +import type { ComputerGateway } from "../src/computer/gateway"; +import { + ActionRefusedError, + ComputerUnavailableError, + WorkspaceNotFoundError, + WorkspaceRefusedError, + WorkspaceRequestError, + WorkspaceTooLargeError, +} from "../src/computer/gateway"; +import type { PolicyStore } from "../src/computer/policy-store"; +import { createComputerRoutes } from "../src/computer/routes"; + +const member: AuthenticatedActor = { + id: "user-1", + email: "member@openbot.test", + role: "user", +}; + +function asActor(): MiddlewareHandler<{ Variables: AppVariables }> { + return async (context, next) => { + context.set("actor", member); + await next(); + }; +} + +function appFor( + gateway: ComputerGateway, + canUseBot: ( + actor: AuthenticatedActor, + botId: string, + ) => Promise = async () => true, +) { + return createComputerRoutes(gateway, {} as PolicyStore, asActor(), canUseBot); +} + +function gatewayThatReturns(name: string, bytes: Uint8Array) { + const calls: Array<{ + botId: string; + actorId: string; + path: string; + }> = []; + const gateway = { + downloadFile: async ( + botId: string, + actor: { id: string }, + input: { path: string }, + ) => { + calls.push({ botId, actorId: actor.id, path: input.path }); + return { + body: bytes, + bytes: bytes.byteLength, + name, + }; + }, + } as unknown as ComputerGateway; + return { gateway, calls }; +} + +describe("the public computer file download route", () => { + test("returns raw bytes under safe attachment headers", async () => { + const bytes = Uint8Array.from([0, 1, 2, 255]); + const { gateway, calls } = gatewayThatReturns("report.pdf", bytes); + const response = await appFor(gateway).request( + "http://openbot.test/bot-17/files/download?path=%20reports%2Freport.pdf%20", + ); + + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe( + "application/octet-stream", + ); + expect(response.headers.get("content-length")).toBe("4"); + expect(response.headers.get("content-disposition")).toBe( + "attachment; filename=\"report.pdf\"; filename*=UTF-8''report.pdf", + ); + expect(response.headers.get("x-content-type-options")).toBe("nosniff"); + expect(response.headers.get("cache-control")).toBe("private, no-store"); + expect(new Uint8Array(await response.arrayBuffer())).toEqual(bytes); + expect(calls).toEqual([ + { botId: "bot-17", actorId: "user-1", path: "reports/report.pdf" }, + ]); + }); + + test("cannot add response headers through the filename", async () => { + const { gateway } = gatewayThatReturns( + "evil\r\nX-Injected: yes.txt", + Uint8Array.from([1]), + ); + const response = await appFor(gateway).request( + "http://openbot.test/bot-17/files/download?path=evil.txt", + ); + + expect(response.status).toBe(200); + expect(response.headers.get("content-disposition")).not.toMatch(/[\r\n]/); + expect(response.headers.get("x-injected")).toBeNull(); + }); + + test("refuses a missing path before asking the gateway", async () => { + const { gateway, calls } = gatewayThatReturns("x", Uint8Array.from([1])); + const response = await appFor(gateway).request( + "http://openbot.test/bot-17/files/download", + ); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: "A file path is required.", + }); + expect(calls).toHaveLength(0); + }); + + test("applies Bot ownership before asking the gateway", async () => { + const { gateway, calls } = gatewayThatReturns("x", Uint8Array.from([1])); + const response = await appFor(gateway, async () => false).request( + "http://openbot.test/somebody-elses-bot/files/download?path=x", + ); + + expect(response.status).toBe(404); + await expect(response.json()).resolves.toEqual({ + error: "There is no such Bot.", + }); + expect(calls).toHaveLength(0); + }); + + test.each([ + [ + "a policy refusal", + new ActionRefusedError("Blocked by policy.", "file.extension == 'env'"), + 403, + ], + ["a workspace refusal", new WorkspaceRefusedError("Outside."), 403], + ["a bad path", new WorkspaceRequestError("A path is required."), 400], + ["a missing file", new WorkspaceNotFoundError("No such file."), 404], + ["an oversized file", new WorkspaceTooLargeError("Too large."), 413], + [ + "an unavailable computer", + new ComputerUnavailableError("The computer is not running."), + 503, + ], + ["an unexpected failure", new Error("Boom."), 500], + ])("maps %s to %i", async (_name, error, status) => { + const gateway = { + downloadFile: async () => { + throw error; + }, + } as unknown as ComputerGateway; + const response = await appFor(gateway).request( + "http://openbot.test/bot-17/files/download?path=x", + ); + + expect(response.status).toBe(status); + const body = (await response.json()) as Record; + expect(body).toMatchObject({ error: error.message }); + if (error instanceof ActionRefusedError) { + expect(body).toMatchObject({ rule: error.rule }); + } + }); +}); diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index 9515a6615..776d9ee89 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -4,7 +4,9 @@ import { StaleSnapshotError } from "../src/computer/client"; import { ActionRefusedError, createComputerGateway, + WorkspaceNotFoundError, WorkspaceRefusedError, + WorkspaceTooLargeError, } from "../src/computer/gateway"; import type { ActionPolicy } from "../src/computer/policy"; import { @@ -100,6 +102,11 @@ function fakeComputer(options?: { image: "aGVsbG8=", mimeType: "image/png", }); + case "/files/download": + calls.push("downloadFile"); + return new Response(Uint8Array.from([0, 1, 2, 255]), { + headers: { "content-length": "4" }, + }); case "/files/read": calls.push("readFile"); return Response.json({ @@ -843,6 +850,100 @@ describe("the computer gateway", () => { }); }); + test("downloads raw bytes through the separate download intent and records it", async () => { + const { gateway, calls, rows, requests } = await gatewayWith(PERMISSIVE, { + token: "computer-secret", + }); + + const download = await gateway.downloadFile("bot-1", ACTOR, { + path: "reports/Q3 file.pdf", + }); + + expect(download.name).toBe("Q3 file.pdf"); + expect(download.bytes).toBe(4); + expect( + new Uint8Array(await new Response(download.body).arrayBuffer()), + ).toEqual(Uint8Array.from([0, 1, 2, 255])); + expect(calls).toContain("downloadFile"); + expect(rows[0]?.eventType).toBe("computer.action_allowed"); + expect(rows[0]?.payload).toMatchObject({ + action: "computer_download_file", + bot: "bot-1", + file: "reports/Q3 file.pdf", + }); + const request = requests.find(({ url }) => url.includes("/files/download")); + expect(request?.init?.method).toBe("GET"); + expect(request?.init?.headers).toMatchObject({ + "x-openbot-bot-id": "bot-1", + "x-openbot-computer-token": "computer-secret", + }); + expect(new URL(request?.url ?? "").searchParams.get("path")).toBe( + "reports/Q3 file.pdf", + ); + }); + + test("an absent policy refuses a download before the computer is asked", async () => { + const { gateway, calls, rows } = await gatewayWith(undefined); + + await expect( + gateway.downloadFile("bot-1", ACTOR, { path: "report.pdf" }), + ).rejects.toThrow(ActionRefusedError); + expect(calls).not.toContain("downloadFile"); + expect(rows[0]?.eventType).toBe("computer.action_refused"); + expect(rows[0]?.payload.action).toBe("computer_download_file"); + }); + + test("a download rule does not reuse the read-file permission", async () => { + const separate = await gatewayWith({ + ...PERMISSIVE, + deny: ['tool.name == "computer_read_file"'], + }); + await expect( + separate.gateway.downloadFile("bot-1", ACTOR, { path: "report.pdf" }), + ).resolves.toMatchObject({ name: "report.pdf" }); + expect(separate.calls).toContain("downloadFile"); + + const denied = await gatewayWith({ + ...PERMISSIVE, + deny: ['intent == "download_file"'], + }); + await expect( + denied.gateway.downloadFile("bot-1", ACTOR, { path: "report.pdf" }), + ).rejects.toThrow(ActionRefusedError); + expect(denied.calls).not.toContain("downloadFile"); + expect(denied.rows[0]?.eventType).toBe("computer.action_refused"); + }); + + test("records a failed download attempt after the allowed decision", async () => { + const { gateway, rows } = await gatewayWith(PERMISSIVE, { + routes: { + "/files/download": () => + Response.json({ error: "No such file." }, { status: 404 }), + }, + }); + + await expect( + gateway.downloadFile("bot-1", ACTOR, { path: "missing.pdf" }), + ).rejects.toThrow(WorkspaceNotFoundError); + expect(rows.map(({ eventType }) => eventType)).toEqual([ + "computer.action_allowed", + "computer.action_failed", + ]); + }); + + test("maps an oversized computer response to a 413-equivalent error", async () => { + const { gateway } = await gatewayWith(PERMISSIVE, { + routes: { + "/files/download": () => + Response.json({ error: "Too large." }, { status: 413 }), + }, + }); + + await expect( + gateway.downloadFile("bot-1", ACTOR, { path: "big.zip" }), + ).rejects.toThrow(WorkspaceTooLargeError); + }); + test("routes acting, control, file, secret, and human input calls to the correct endpoint paths", async () => { const { gateway, requests } = await gatewayWith(PERMISSIVE); diff --git a/shared/file-download.test.ts b/shared/file-download.test.ts new file mode 100644 index 000000000..c5a02cf9b --- /dev/null +++ b/shared/file-download.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, test } from "bun:test"; +import { contentDisposition, downloadHeaders } from "./file-download"; + +describe("workspace download headers", () => { + test("declares an opaque attachment and its exact byte length", () => { + const headers = downloadHeaders("report.pdf", 42); + + expect(headers).toMatchObject({ + "Content-Type": "application/octet-stream", + "Content-Length": "42", + "Content-Disposition": + "attachment; filename=\"report.pdf\"; filename*=UTF-8''report.pdf", + "X-Content-Type-Options": "nosniff", + "Cache-Control": "private, no-store", + }); + }); + + test("keeps a unicode name in filename* and a safe ASCII fallback", () => { + expect(contentDisposition("报告.pdf")).toBe( + "attachment; filename=\"__.pdf\"; filename*=UTF-8''%E6%8A%A5%E5%91%8A.pdf", + ); + }); + + test("neutralises CRLF so a filename cannot add response headers", () => { + const headers = new Headers( + downloadHeaders("evil\r\nX-Injected: yes.txt", 4), + ); + + expect(headers.get("content-disposition")).not.toMatch(/[\r\n]/); + expect(headers.get("x-injected")).toBeNull(); + }); +}); diff --git a/shared/file-download.ts b/shared/file-download.ts new file mode 100644 index 000000000..d4dddbc13 --- /dev/null +++ b/shared/file-download.ts @@ -0,0 +1,65 @@ +/** + * The response contract for a workspace file leaving the computer. + * + * Downloads are always opaque attachments. A generated HTML, SVG or JavaScript file must not be + * rendered on the application's origin, and an extension is not evidence of what the bytes contain. + * One declaration is shared by the computer that reads the file and the server that exposes it, so + * the two sides cannot drift on whether inline rendering is allowed. + */ + +export const DOWNLOAD_CONTENT_TYPE = "application/octet-stream"; + +function headerSafeFilename(name: string): string { + const cleaned = Array.from(name.normalize("NFC"), (character) => { + const code = character.codePointAt(0) ?? 0; + // Header injection is removed before any quoting: a quote or backslash would otherwise escape + // the quoted parameter and let a filename carry its own directive. + return code <= 0x1f || + code === 0x7f || + character === '"' || + character === "\\" + ? "_" + : character; + }) + .join("") + .trim(); + // Keep the fallback and RFC 5987 form bounded. A filename is a label, not a transport for an + // unbounded path. + return Array.from(cleaned).slice(0, 255).join("") || "download"; +} + +function asciiFallback(name: string): string { + const fallback = Array.from(name, (character) => { + const code = character.codePointAt(0) ?? 0; + return code >= 0x20 && code <= 0x7e ? character : "_"; + }).join(""); + return fallback || "download"; +} + +function rfc5987Encode(value: string): string { + return encodeURIComponent(value).replace( + /['()*]/g, + (character) => `%${character.charCodeAt(0).toString(16).toUpperCase()}`, + ); +} + +export function contentDisposition(name: string): string { + const safe = headerSafeFilename(name); + return `attachment; filename="${asciiFallback(safe)}"; filename*=UTF-8''${rfc5987Encode(safe)}`; +} + +export function downloadHeaders( + name: string, + bytes: number, +): Record { + if (!Number.isSafeInteger(bytes) || bytes < 0) { + throw new Error("A download must have a non-negative byte length."); + } + return { + "Content-Type": DOWNLOAD_CONTENT_TYPE, + "Content-Length": String(bytes), + "Content-Disposition": contentDisposition(name), + "X-Content-Type-Options": "nosniff", + "Cache-Control": "private, no-store", + }; +} From 0b75cb458dcc109e5d152bde5a3378310abc1cff Mon Sep 17 00:00:00 2001 From: Harbor404 <2657212322@qq.com> Date: Thu, 1 Oct 2026 22:35:54 +0800 Subject: [PATCH 2/3] docs: note generated file downloads --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33f721218..a12c09fe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Generated workspace files can be downloaded intact + +`GET /api/computers/:botId/files/download?path=...` streams a generated file as an opaque +attachment instead of returning the 64 KB UTF-8 text extract. Downloads use the separate +`computer_download_file` / `download_file` permission, remain confined to the Bot workspace, are +capped at 100 MiB with `413`, and are recorded on the computer audit trail. Existing read, list and +write APIs are unchanged. + ### `start.sh` names the port to change on macOS When the API server's or the app's port was held by another process, `start.sh` was meant to say From 7b09593f037770fd05f9e48f0995223a2c24892d Mon Sep 17 00:00:00 2001 From: David McKay Date: Fri, 2 Oct 2026 13:47:14 -0700 Subject: [PATCH 3/3] Let the Cloud computer use switch refuse workspace downloads download_file was not one of the intents the switch gates, so a member could still download workspace files after an administrator turned Cloud computer use off. It now needs the same capability as reading or writing those files. --- server/src/admin/controls.ts | 1 + server/tests/enterprise-controls.test.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/server/src/admin/controls.ts b/server/src/admin/controls.ts index ad5cf8906..181804a64 100644 --- a/server/src/admin/controls.ts +++ b/server/src/admin/controls.ts @@ -133,6 +133,7 @@ const COMPUTER_INTENTS = new Set([ "read_file", "write_file", "list_files", + "download_file", "run_command", ]); diff --git a/server/tests/enterprise-controls.test.ts b/server/tests/enterprise-controls.test.ts index eaed9c51b..2249e928e 100644 --- a/server/tests/enterprise-controls.test.ts +++ b/server/tests/enterprise-controls.test.ts @@ -141,6 +141,19 @@ describe("the computer boundary overlay", () => { ); }); + test("the computer switch refuses downloading a workspace file", () => { + const off = (capability: string) => + snapshot({ rows: [row("organization", "", capability, false)] }); + const download = context({ + tool: { name: "computer_download_file" }, + intent: "download_file", + }); + expect(decideFromSnapshot(off("cloudComputer"), download)?.matched).toBe( + "capability:cloudComputer", + ); + expect(decideFromSnapshot(off("cloudBrowser"), download)).toBeNull(); + }); + test("a group grant reaches the snapshot's members", () => { const current = snapshot({ rows: [