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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,16 @@ A routine that fails ten times in a row is switched off, and someone has to swit
first-failure message never appeared.
- **Now:** failures are counted from when the routine was last switched on, recorded in a new
`routines.enabled_at` column. The migration sets it to the time of the upgrade, so any failure
streak already under way starts again from zero at that point.

### 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.

streak already under way starts again from zero at that point. Adds migration
`0051_routine_enabled_at`.

Expand Down
36 changes: 29 additions & 7 deletions agent-computer/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -54,6 +55,8 @@ import { startVirtualDisplay } from "./virtual-display";
import {
createWorkspace,
WorkspaceFileError,
WorkspaceFileNotFoundError,
WorkspaceFileTooLargeError,
WorkspacePathError,
} from "./workspace";

Expand All @@ -65,9 +68,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
Expand Down Expand Up @@ -1080,6 +1083,22 @@ serve<StreamData>({

// 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;
Expand Down Expand Up @@ -1494,12 +1513,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;
}
Expand Down
48 changes: 47 additions & 1 deletion agent-computer/src/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
};

/**
Expand All @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions agent-computer/tests/authorisation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ describe("what an unauthenticated caller may reach", () => {
"/snapshot",
"/files/list",
"/files/read",
"/files/download",
"/stream",
"/live",
"/",
Expand Down
164 changes: 164 additions & 0 deletions agent-computer/tests/file-download-http.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof Bun.spawn> | undefined;

async function freePort(): Promise<number> {
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();
});
});
Loading
Loading