fix(mcp): detect binary files by content instead of extension allowlist (#1633)

src/mcp/binary-detection.ts

@@ -0,0 +1,22 @@
+/**
+ * Decides whether a file has to be committed as a base64 blob instead of being
+ * inlined in the Git tree as UTF-8 text.
+ *
+ * Inlining is only safe for content that survives a UTF-8 decode untouched;
+ * anything else gets its invalid bytes replaced during the decode, which
+ * silently corrupts the committed file. A NUL byte is treated as binary for the
+ * same reason Git does it: no text file carries one, and it is the cheapest
+ * signal available.
+ */
+export function isBinaryContent(content: Buffer): boolean {
+  if (content.includes(0)) {
+    return true;
+  }
+
+  try {
+    new TextDecoder("utf-8", { fatal: true }).decode(content);
+    return false;
+  } catch {
+    return true;
+  }
+}

src/mcp/github-file-ops-server.ts

@@ -8,6 +8,7 @@ import { resolve } from "path";
 import { constants } from "fs";
 import fetch from "node-fetch";
 import { GITHUB_API_URL } from "../github/api/config";
+import { isBinaryContent } from "./binary-detection";
 import { validatePathWithinRepo } from "./path-validation";
 import { updateGitReference } from "./update-git-reference";
 
@@ -258,17 +259,14 @@ server.tool(
           // Get the proper file mode based on file permissions
           const fileMode = await getFileMode(fullPath);
 
-          // Check if file is binary (images, etc.)
-          const isBinaryFile =
-            /\.(png|jpg|jpeg|gif|webp|ico|pdf|zip|tar|gz|exe|bin|woff|woff2|ttf|eot)$/i.test(
-              relativePath,
-            );
+          // Check if the file is binary by inspecting its contents. An
+          // extension allowlist used to decide this, which corrupted every
+          // binary type that wasn't on the list.
+          const fileContent = await readFile(fullPath);
 
-          if (isBinaryFile) {
+          if (isBinaryContent(fileContent)) {
             // For binary files, create a blob first using the Blobs API
-            const binaryContent = await readFile(fullPath);
-
-            // Create blob using Blobs API (supports encoding parameter)
+            // (supports the encoding parameter)
             const blobUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/blobs`;
             const blobResponse = await fetch(blobUrl, {
               method: "POST",
@@ -279,7 +277,7 @@ server.tool(
                 "Content-Type": "application/json",
               },
               body: JSON.stringify({
-                content: binaryContent.toString("base64"),
+                content: fileContent.toString("base64"),
                 encoding: "base64",
               }),
             });
@@ -302,12 +300,11 @@ server.tool(
             };
           } else {
             // For text files, include content directly in tree
-            const content = await readFile(fullPath, "utf-8");
             return {
               path: relativePath,
               mode: fileMode,
               type: "blob",
-              content: content,
+              content: fileContent.toString("utf-8"),
             };
           }
         }),

test/binary-detection.test.ts

@@ -0,0 +1,79 @@
+import { describe, expect, it } from "bun:test";
+import { isBinaryContent } from "../src/mcp/binary-detection";
+
+describe("isBinaryContent", () => {
+  describe("text content", () => {
+    it("treats ASCII as text", () => {
+      expect(isBinaryContent(Buffer.from("hello world\n"))).toBe(false);
+    });
+
+    it("treats multibyte UTF-8 as text", () => {
+      expect(isBinaryContent(Buffer.from("café — 日本語 🎉\n"))).toBe(false);
+    });
+
+    it("treats an empty file as text", () => {
+      expect(isBinaryContent(Buffer.from(""))).toBe(false);
+    });
+
+    it("treats CRLF and tabs as text", () => {
+      expect(isBinaryContent(Buffer.from("a\tb\r\nc\r\n"))).toBe(false);
+    });
+  });
+
+  describe("binary content", () => {
+    // The extensions below are the ones the previous allowlist covered, so
+    // these files were already committed correctly.
+    it("detects PNG", () => {
+      expect(
+        isBinaryContent(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a])),
+      ).toBe(true);
+    });
+
+    // These are the regression cases: binary formats that were not on the
+    // allowlist and got decoded as UTF-8, corrupting the committed bytes.
+    it("detects BMP", () => {
+      expect(
+        isBinaryContent(Buffer.from([0x42, 0x4d, 0x36, 0x00, 0x00, 0x00])),
+      ).toBe(true);
+    });
+
+    it("detects SQLite databases", () => {
+      expect(isBinaryContent(Buffer.from("SQLite format 3\0", "binary"))).toBe(
+        true,
+      );
+    });
+
+    it("detects WebAssembly modules", () => {
+      expect(
+        isBinaryContent(Buffer.from([0x00, 0x61, 0x73, 0x6d, 0x01, 0x00])),
+      ).toBe(true);
+    });
+
+    it("detects arbitrary invalid UTF-8 without NUL bytes", () => {
+      // Lone continuation bytes: no NUL, but not decodable as UTF-8 either.
+      expect(isBinaryContent(Buffer.from([0xc3, 0x28, 0xa0, 0xa1]))).toBe(true);
+    });
+
+    it("detects a truncated multibyte sequence", () => {
+      // First two bytes of a 3-byte character, cut short.
+      expect(isBinaryContent(Buffer.from([0xe6, 0x97]))).toBe(true);
+    });
+  });
+
+  it("round-trips text through UTF-8 without loss", () => {
+    const original = "acentuação, emoji 🚀, símbolos ±≠";
+    const buffer = Buffer.from(original);
+
+    expect(isBinaryContent(buffer)).toBe(false);
+    expect(buffer.toString("utf-8")).toBe(original);
+  });
+
+  it("preserves bytes that a UTF-8 decode would have replaced", () => {
+    const bytes = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x10, 0x4a]);
+
+    expect(isBinaryContent(bytes)).toBe(true);
+    // What the old text path would have produced, versus base64.
+    expect(Buffer.from(bytes.toString("utf-8"), "utf-8")).not.toEqual(bytes);
+    expect(Buffer.from(bytes.toString("base64"), "base64")).toEqual(bytes);
+  });
+});