Preserve integrity when materializing and reading skill files

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 {

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,
   };
 }

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(),

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"),
+  };
 }

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() })

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]),
+  );
+});