diff --git a/cli/package.mjs b/cli/package.mjs index f528131..104d794 100644 --- a/cli/package.mjs +++ b/cli/package.mjs @@ -75,13 +75,21 @@ export async function materialize(bundle, root) { try { await rename(tmp, target); } catch (e) { - if (e.code === "ENOTEMPTY" || e.code === "EEXIST") { - /* Never trust an existing mutable cache directory: return this freshly verified copy. */ const fresh = - target + "-" + randomUUID(); - await rename(tmp, fresh); - return fresh; + const collision = e.code === "ENOTEMPTY" || e.code === "EEXIST"; + let targetExists = false; + if (e.code === "EPERM") { + try { + await lstat(target); + targetExists = true; + } catch { + // EPERM without an existing target is a real rename failure. + } } - throw e; + if (!collision && !targetExists) throw e; + /* Never trust an existing mutable cache directory: return this freshly verified copy. */ const fresh = + target + "-" + randomUUID(); + await rename(tmp, fresh); + return fresh; } return target; } finally { diff --git a/src/server/library.ts b/src/server/library.ts index 823ad36..5555731 100644 --- a/src/server/library.ts +++ b/src/server/library.ts @@ -5,6 +5,7 @@ import { } from "../skill-references"; import { packageMetrics } from "../package-metrics"; import { parseSkillIcon, type SkillIcon } from "../skill-icons"; +import { decodeTextContent } from "../skill-manifest"; import { createHash, randomUUID } from "node:crypto"; import matter from "gray-matter"; import { and, eq, desc, inArray, sql } from "drizzle-orm"; @@ -464,15 +465,16 @@ export async function readFile( if (f.size > 160_000) throw new Problem(413, "File exceeds inline limit; fetch the bundle"); const bytes = Buffer.from(f.content, "base64"); - if (bytes.includes(0)) - throw new Problem(415, "Binary file; fetch the bundle"); + const text = decodeTextContent(bytes); + if (text === null) + throw new Problem(415, "Binary or invalid UTF-8 file; fetch the bundle"); await record(p, "read_file", id, { revision: r.id, path }); return { id, revision: r.id, path, - text: bytes.toString("utf8"), - ...(await referenceDetails(p, bytes.toString("utf8"))), + text, + ...(await referenceDetails(p, text)), sha256: f.sha256, }; } diff --git a/src/server/mcp.ts b/src/server/mcp.ts index ceb2665..eef457f 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -230,7 +230,7 @@ export function createMcp( "read_skill_file", { description: - "Read a specific reference or script from the exact loaded skill revision. Text files only, up to 160 KB. For binary or larger files use skillbox fetch on the machine that needs them.", + "Read a specific reference or script from the exact loaded skill revision. UTF-8 text files only, up to 160 KB. For binary, non-UTF-8 or larger files use skillbox fetch on the machine that needs them.", inputSchema: z.object({ id: z.string(), revision: z.string(), diff --git a/src/skill-manifest.ts b/src/skill-manifest.ts index c104aaa..497dd55 100644 --- a/src/skill-manifest.ts +++ b/src/skill-manifest.ts @@ -291,24 +291,31 @@ export function mimeTypeForPath(path: string) { ); } -export function resourceContent(uri: string, file: SkillFile) { - const bytes = verifiedFileBytes(file); +export function decodeTextContent(bytes: Uint8Array): string | null { try { const text = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true, }).decode(bytes); - if (text.includes("\0")) throw new Error("Binary content"); - return { - uri, - mimeType: mimeTypeForPath(file.path), - text, - }; + return text.includes("\0") ? null : text; } catch { + return null; + } +} + +export function resourceContent(uri: string, file: SkillFile) { + const bytes = verifiedFileBytes(file); + const text = decodeTextContent(bytes); + if (text !== null) { return { uri, mimeType: mimeTypeForPath(file.path), - blob: bytes.toString("base64"), + text, }; } + return { + uri, + mimeType: mimeTypeForPath(file.path), + blob: bytes.toString("base64"), + }; } diff --git a/tests/library.test.ts b/tests/library.test.ts index 2d31289..d3a74c9 100644 --- a/tests/library.test.ts +++ b/tests/library.test.ts @@ -388,6 +388,21 @@ test("missing auth and invalid tokens fail closed", async () => { expect(r.status).toBe(401); } }); + +test("text-only file reads reject malformed UTF-8 instead of replacing bytes", async () => { + const id = "test-invalid-utf8-" + randomUUID().slice(0, 8); + ids.push(id); + const revision = await publish( + ADMIN, + id, + [...files(id), makeFile("references/invalid.txt", Buffer.from([0xff]))], + null, + ); + const { readFile } = await import("../src/server/library"); + await expect( + readFile(ADMIN, id, revision.revision, "references/invalid.txt"), + ).rejects.toMatchObject({ status: 415 }); +}); test("allowlist filters browse and blocks direct content, history and bundles", async () => { const list = await ( await app.request("/api/skills", { headers: headers() }) diff --git a/tests/skill-manifest.test.ts b/tests/skill-manifest.test.ts index 4558fc0..45c396a 100644 --- a/tests/skill-manifest.test.ts +++ b/tests/skill-manifest.test.ts @@ -5,6 +5,7 @@ import { parseSkillResourceUri, skillResourceUri, resourceContent, + decodeTextContent, verifiedFileBytes, } from "../src/skill-manifest"; import type { SkillFile } from "../src/shared"; @@ -153,3 +154,19 @@ test("resource reads preserve UTF-8 and binary bytes and fail on tampering", () ]); expect(corrupted.compatible).toBe(false); }); + +test("text-only decoding rejects malformed UTF-8 and NUL content", () => { + const valid = Buffer.from("valid 🌋 text"); + expect(decodeTextContent(valid)).toBe("valid 🌋 text"); + expect(decodeTextContent(Buffer.from([0xff, 0x61]))).toBeNull(); + expect(decodeTextContent(Buffer.from([0x61, 0x00, 0x62]))).toBeNull(); + + const invalid = file("refs/invalid.txt", Buffer.from([0xff, 0x61])); + const content = resourceContent( + skillResourceUri(uuid, "example", invalid.path), + invalid, + ); + expect("blob" in content ? Buffer.from(content.blob!, "base64") : null).toEqual( + Buffer.from([0xff, 0x61]), + ); +});