From dfb8fc798e1a98ff989c587a166b75010bfe2639 Mon Sep 17 00:00:00 2001 From: Henrique Pires Date: Tue, 11 Aug 2026 20:35:46 -0300 Subject: [PATCH] fix(mcp): detect binary files by content instead of extension allowlist (#1633) --- src/mcp/binary-detection.ts | 22 +++++++++ src/mcp/github-file-ops-server.ts | 21 ++++---- test/binary-detection.test.ts | 79 +++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 12 deletions(-) create mode 100644 src/mcp/binary-detection.ts create mode 100644 test/binary-detection.test.ts diff --git a/src/mcp/binary-detection.ts b/src/mcp/binary-detection.ts new file mode 100644 index 00000000..8a87aa55 --- /dev/null +++ b/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; + } +} diff --git a/src/mcp/github-file-ops-server.ts b/src/mcp/github-file-ops-server.ts index a8d5c72b..2b4906b6 100644 --- a/src/mcp/github-file-ops-server.ts +++ b/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"), }; } }), diff --git a/test/binary-detection.test.ts b/test/binary-detection.test.ts new file mode 100644 index 00000000..3737c8b5 --- /dev/null +++ b/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); + }); +});