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
37 changes: 37 additions & 0 deletions src/api/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,43 @@ export function sendJson(res: ServerResponse, status: number, body: unknown): vo
res.end(data);
}

export function contentTypeWithUtf8Charset(contentType: string): string {
let parameterStart = -1;
let inQuotes = false;
let escaped = false;
for (let i = 0; i < contentType.length; i += 1) {
const character = contentType[i]!;
if (escaped) {
escaped = false;
} else if (inQuotes && character === "\\") {
escaped = true;
} else if (character === '"') {
inQuotes = !inQuotes;
} else if (!inQuotes && character === ";") {
if (parameterStart >= 0 && /^\s*charset\s*=/i.test(contentType.slice(parameterStart, i))) return contentType;
parameterStart = i + 1;
}
}
if (parameterStart >= 0 && /^\s*charset\s*=/i.test(contentType.slice(parameterStart))) return contentType;
const mime = contentType.split(";", 1)[0]!.trim().toLowerCase();
const textual =
mime.startsWith("text/") ||
mime === "application/json" ||
mime === "application/xml" ||
mime === "application/yaml" ||
mime === "application/x-yaml" ||
mime === "application/javascript" ||
mime === "application/x-javascript" ||
mime === "application/graphql" ||
mime === "application/sql" ||
mime === "application/toml" ||
mime === "image/svg+xml" ||
mime.endsWith("+json") ||
mime.endsWith("+xml") ||
mime.endsWith("+yaml");
return textual ? `${contentType}; charset=utf-8` : contentType;
}

export function pipeToResponse(
res: ServerResponse,
stream: NodeJS.ReadableStream & { destroy?: () => void },
Expand Down
4 changes: 2 additions & 2 deletions src/api/routes/admin/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { parseScopeId } from "../../../types.ts";
import { ByteSourceTooLargeError } from "../../../files/durable-byte-store.ts";
import { fileArtifactId } from "../../../files/file-artifact-store.ts";
import { MAX_ATTACHMENT_BYTES, mimeFromName, safeAttachmentName } from "../../../core/attachments.ts";
import { contentDispositionAttachment, pipeToResponse, sendJson } from "../../http.ts";
import { contentDispositionAttachment, contentTypeWithUtf8Charset, pipeToResponse, sendJson } from "../../http.ts";
import { audit, authorizeAdmin, requireScopedAdmin } from "../shared.ts";
import { type ApiCtx } from "../route.ts";
import { discoverScopes, FILES_PAGE_SIZE } from "./common.ts";
Expand Down Expand Up @@ -68,7 +68,7 @@ export async function downloadAdminFile(ctx: ApiCtx): Promise<void> {
const opened = await deps.files.open(id);
if (!opened) return sendJson(res, 404, { error: "not_found" });
res.writeHead(200, {
"content-type": inline ? mime : "application/octet-stream",
"content-type": inline ? contentTypeWithUtf8Charset(art.mimetype) : "application/octet-stream",
"content-length": String(opened.sizeBytes),
"content-disposition": contentDispositionAttachment(art.name, inline ? "inline" : "attachment"),
"x-content-type-options": "nosniff",
Expand Down
4 changes: 2 additions & 2 deletions src/api/routes/surface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { builtInModelCatalog, selectableCatalogForHarness, selectableModelCatalo
import { errMessage } from "../../util/errors.ts";
import { renderAgentApis } from "../agent-api-catalog.ts";
import { mintCapabilityToken, CAPABILITY_TTL_MS } from "../../auth/capability-token.ts";
import { pipeToResponse, sendJson } from "../http.ts";
import { contentTypeWithUtf8Charset, pipeToResponse, sendJson } from "../http.ts";
import { audit, isObj, orgScope } from "./shared.ts";
import { type ApiCtx, type Route } from "./route.ts";
import {
Expand Down Expand Up @@ -252,7 +252,7 @@ async function getFileContent(ctx: ApiCtx): Promise<void> {
const opened = await app.openFileForViewer(id, viewer);
if (!opened) return sendJson(res, 404, { error: "not_found" });
res.writeHead(200, {
"content-type": opened.mimetype || "application/octet-stream",
"content-type": contentTypeWithUtf8Charset(opened.mimetype || "application/octet-stream"),
"content-length": String(opened.sizeBytes),
"content-disposition": `inline; filename*=UTF-8''${encodeURIComponent(opened.name)}`,
});
Expand Down
2 changes: 1 addition & 1 deletion test/admin-observability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ test("an org admin sees conversations, transcripts, files, and runs top-down", a
assert.equal(download.status, 200);
assert.equal(
download.headers.get("content-type"),
"text/plain",
"text/plain; charset=utf-8",
"browser-renderable types open in the browser instead of downloading",
);
assert.match(download.headers.get("content-disposition") ?? "", /^inline/);
Expand Down
58 changes: 54 additions & 4 deletions test/agent-files-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,38 @@ import type { AddressInfo } from "node:net";
import type { Server } from "node:http";
import { buildApp, type BuiltApp } from "../src/wiring.ts";
import { createServer } from "../src/api/server.ts";
import { contentTypeWithUtf8Charset } from "../src/api/http.ts";
import { fileArtifactId } from "../src/files/file-artifact-store.ts";
import { mintCapabilityToken, CAPABILITY_TTL_MS, CONTROL_PLANE_AUD } from "../src/auth/capability-token.ts";
import { scopeId } from "../src/types.ts";
import { testConfig } from "./support/test-config.ts";

const SECRET = "agent-files-secret".repeat(3);

it("detects charset parameters outside quoted parameter values", () => {
assert.equal(
contentTypeWithUtf8Charset('text/plain; note="x;charset=bogus"'),
'text/plain; note="x;charset=bogus"; charset=utf-8',
);
assert.equal(
contentTypeWithUtf8Charset("text/plain; format=flowed; charset=iso-8859-1"),
"text/plain; format=flowed; charset=iso-8859-1",
);
assert.equal(
contentTypeWithUtf8Charset("text/plain; format=flowed; ChArSeT = utf-16"),
"text/plain; format=flowed; ChArSeT = utf-16",
);
assert.equal(contentTypeWithUtf8Charset('text/plain; note="one;two"'), 'text/plain; note="one;two"; charset=utf-8');
});

describe("agent files self-API", async () => {
let server: Server;
let base: string;
let built: BuiltApp;
let mineId: string;
let theirsId: string;
let encodedId: string;
let binaryId: string;

const capFor = (actorId: string) =>
mintCapabilityToken(
Expand All @@ -41,6 +60,8 @@ describe("agent files self-API", async () => {
base = `http://localhost:${(server.address() as AddressInfo).port}`;
mineId = fileArtifactId("mine", "out", 0);
theirsId = fileArtifactId("theirs", "out", 0);
encodedId = fileArtifactId("encoded", "out", 0);
binaryId = fileArtifactId("binary", "out", 0);
await built.files.put({
id: mineId,
ownerScopeId: scopeId("personal", "U1"),
Expand All @@ -61,6 +82,26 @@ describe("agent files self-API", async () => {
data: Buffer.from("theirs"),
direction: "out",
});
await built.files.put({
id: encodedId,
ownerScopeId: scopeId("personal", "U1"),
createdBy: "U1",
name: "encoded.json",
path: `artifacts/${encodedId}/encoded.json`,
mimetype: "application/json; charset=iso-8859-1",
data: Buffer.from("{}"),
direction: "out",
});
await built.files.put({
id: binaryId,
ownerScopeId: scopeId("personal", "U1"),
createdBy: "U1",
name: "binary.png",
path: `artifacts/${binaryId}/binary.png`,
mimetype: "image/png",
data: Buffer.from([0x89, 0x50, 0x4e, 0x47]),
direction: "out",
});
});

after(async () => {
Expand All @@ -75,15 +116,24 @@ describe("agent files self-API", async () => {
const res = await get("/v1/files", await capFor("U1"));
assert.equal(res.status, 200);
const page = (await res.json()) as { owned: Array<{ id: string }>; shared: Array<{ id: string }> };
assert.deepEqual(
page.owned.map((file) => file.id),
[mineId],
);
assert.deepEqual(page.owned.map((file) => file.id).sort(), [mineId, encodedId, binaryId].sort());
assert.deepEqual(page.shared, []);
});

it("returns 404 for another principal's file content", async () => {
const res = await get(`/v1/files/${theirsId}/content`, await capFor("U1"));
assert.equal(res.status, 404);
});

it("adds UTF-8 to textual file content without changing explicit charsets or binary types", async () => {
const token = await capFor("U1");
const text = await get(`/v1/files/${mineId}/content`, token);
const encoded = await get(`/v1/files/${encodedId}/content`, token);
const binary = await get(`/v1/files/${binaryId}/content`, token);

assert.equal(text.headers.get("content-type"), "text/plain; charset=utf-8");
assert.equal(encoded.headers.get("content-type"), "application/json; charset=iso-8859-1");
assert.equal(binary.headers.get("content-type"), "image/png");
await Promise.all([text.arrayBuffer(), encoded.arrayBuffer(), binary.arrayBuffer()]);
});
});