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
6 changes: 4 additions & 2 deletions src/filesystem/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,12 @@ The server's directory access control follows this flow:
- Cannot specify both `head` and `tail` simultaneously

- **read_media_file**
- Read an image or audio file
- Read a file and return it as a base64-encoded content block with its MIME type
- Inputs:
- `path` (string)
- Streams the file and returns base64 data with the corresponding MIME type
- Streams the file and returns base64 data with the corresponding MIME type. Image and
audio files are returned as `image`/`audio` content; any other file type is returned as
an embedded `resource` (a valid MCP content block for arbitrary binary data)

- **read_multiple_files**
- Read multiple files simultaneously
Expand Down
123 changes: 123 additions & 0 deletions src/filesystem/__tests__/structured-content.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as os from 'os';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js';
import { spawn } from 'child_process';
import { fileURLToPath } from 'url';

/**
* Integration tests to verify that tool handlers return structuredContent
Expand Down Expand Up @@ -155,4 +156,126 @@ describe('structuredContent schema compliance', () => {
expect(Array.isArray(structuredContent.content)).toBe(false);
});
});

// read_media_file must return a VALID MCP content block. Previously it emitted
// type: "blob" for non-image/audio files, which is not in the MCP content-block
// union (text | image | audio | resource_link | resource) and a strict client
// rejects on schema validation. See issue #4029.
describe('read_media_file (issue #4029)', () => {
// Image/audio inputs already returned valid content types before #4029 (only the
// non-media fallback was the invalid "blob"), so these two cases are "still-works"
// coverage — they round-trip the bytes to catch an encoding regression on the media
// paths. The resource + café cases below are the actual #4029 regression guards
// (they fail against the pre-fix "blob" build).
it('returns type "image" for image files, round-tripping the bytes', async () => {
const pngBytes = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
const pngPath = path.join(testDir, 'pixel.png');
// Contents aren't validated by the tool (MIME is by extension), so arbitrary bytes are fine.
await fs.writeFile(pngPath, pngBytes);

const result = await client.callTool({
name: 'read_media_file',
arguments: { path: pngPath }
});

const content = result.content as Array<{ type: string; data?: string; mimeType?: string }>;
expect(Array.isArray(content)).toBe(true);
expect(content).toHaveLength(1);
expect(content[0].type).toBe('image');
expect(content[0].mimeType).toBe('image/png');
expect(Buffer.from(content[0].data!, 'base64').equals(pngBytes)).toBe(true);
// structuredContent must MIRROR content. The SDK validates each field independently
// against the outputSchema union (either arm is valid for either field), so schema
// validation alone cannot catch the two drifting apart — only this equality can.
expect(result.structuredContent).toEqual({ content });
});

it('returns type "audio" for audio files, round-tripping the bytes', async () => {
const mp3Bytes = Buffer.from([0x49, 0x44, 0x33, 0x04]);
const mp3Path = path.join(testDir, 'clip.mp3');
await fs.writeFile(mp3Path, mp3Bytes);

const result = await client.callTool({
name: 'read_media_file',
arguments: { path: mp3Path }
});

const content = result.content as Array<{ type: string; data?: string; mimeType?: string }>;
expect(content[0].type).toBe('audio');
expect(content[0].mimeType).toBe('audio/mpeg');
expect(Buffer.from(content[0].data!, 'base64').equals(mp3Bytes)).toBe(true);
expect(result.structuredContent).toEqual({ content }); // must mirror content (see image case)
});

it('returns an embedded resource (never "blob") for non-media binaries, round-tripping the bytes', async () => {
const binBytes = Buffer.from([0x00, 0x01, 0x02, 0x03, 0xff, 0xfe]);
const binPath = path.join(testDir, 'data.bin');
await fs.writeFile(binPath, binBytes);

const result = await client.callTool({
name: 'read_media_file',
arguments: { path: binPath }
});

const content = result.content as Array<{
type: string;
resource?: { uri: string; mimeType: string; blob: string };
}>;
expect(content).toHaveLength(1);
// ("never blob" is enforced upstream of any assertion here: the SDK client's
// CallToolResultSchema parse rejects a blob block before content is even inspected,
// failing the callTool above — which is exactly how this test fails pre-fix.)
expect(content[0].type).toBe('resource');
expect(content[0].resource).toBeDefined();
expect(content[0].resource!.uri.startsWith('file://')).toBe(true);
// Decode the uri back to a path and confirm it is the file actually read — guards a
// refactor that builds the uri from the wrong variable while still emitting a
// well-formed file:// string. (realpath handles the /tmp -> /private/tmp macOS alias.)
expect(fileURLToPath(content[0].resource!.uri)).toBe(await fs.realpath(binPath));
expect(content[0].resource!.mimeType).toBe('application/octet-stream');
// The base64 blob must round-trip to the original bytes (data integrity, not just a valid
// shape) — this also subsumes a non-empty check.
expect(Buffer.from(content[0].resource!.blob, 'base64').equals(binBytes)).toBe(true);
expect(result.structuredContent).toEqual({ content }); // must mirror content (see image case)
});

it('percent-encodes spaces and non-ASCII chars in the resource uri (pathToFileURL, not a raw file:// concatenation)', async () => {
const fancyPath = path.join(testDir, 'my café.bin');
await fs.writeFile(fancyPath, Buffer.from([0x10, 0x20, 0x30]));

const result = await client.callTool({
name: 'read_media_file',
arguments: { path: fancyPath }
});

const content = result.content as Array<{ type: string; resource?: { uri: string } }>;
expect(content[0].type).toBe('resource');
const uri = content[0].resource!.uri;
// pathToFileURL percent-encodes the space (%20) and the non-ASCII "é" (%C3%A9 for NFC,
// or the base "e" + %CC%81 for the NFD form some filesystems store); a raw
// `file://${validPath}` (as in PR #4044) would leave both literal.
expect(uri).toContain('%20');
expect(uri).not.toContain(' ');
// The non-ASCII "é" must be percent-encoded: %C3%A9 (NFC) or base-"e" + %CC%81 (the NFD
// form some filesystems decompose to). This regex — not a `.not.toContain('é')`, which is
// vacuous on an NFD runner where the precomposed char never appears — is the load-bearing
// assertion that distinguishes pathToFileURL from a raw `file://${path}` concatenation.
expect(uri).toMatch(/%C3%A9|%CC%81/);
expect(result.structuredContent).toEqual({ content }); // must mirror content (see image case)
});

it('advertises the widened outputSchema union via tools/list (both branches serialize)', async () => {
const { tools } = await client.listTools();
const tool = tools.find((t) => t.name === 'read_media_file');
expect(tool).toBeDefined();
// The union outputSchema must serialize to JSON Schema — a DIFFERENT SDK path
// (toJsonSchemaCompat / tools-list) than the callTool structuredContent validation the
// other cases exercise — with BOTH branches present. Guards a nested-union-in-array
// serialization quirk that could drop the resource arm from the advertised schema.
const schemaStr = JSON.stringify(tool!.outputSchema);
expect(schemaStr).toMatch(/resource/);
expect(schemaStr).toMatch(/image/);
expect(schemaStr).toMatch(/audio/);
});
});
});
50 changes: 34 additions & 16 deletions src/filesystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
import {
CallToolResult,
RootsListChangedNotificationSchema,
type Root,
} from "@modelcontextprotocol/sdk/types.js";
import fs from "fs/promises";
import { createReadStream } from "fs";
import path from "path";
import { pathToFileURL } from "url";
import { z } from "zod";
import { minimatch } from "minimatch";
import { normalizePath, expandHome } from './path-utils.js';
Expand Down Expand Up @@ -250,17 +250,29 @@ server.registerTool(
{
title: "Read Media File",
description:
"Read an image or audio file. Returns the base64 encoded data and MIME type. " +
"Only works within allowed directories.",
"Read a file and return it as a base64-encoded content block with its MIME type. " +
"Image and audio files are returned as image/audio content; any other file type is " +
"returned as an embedded resource. Only works within allowed directories.",
inputSchema: {
path: z.string()
},
outputSchema: {
content: z.array(z.object({
type: z.enum(["image", "audio", "blob"]),
data: z.string(),
mimeType: z.string()
}))
content: z.array(z.union([
z.object({
type: z.enum(["image", "audio"]),
data: z.string(),
mimeType: z.string()
}),
z.object({
type: z.literal("resource"),
resource: z.object({
uri: z.string(),
// Optional, matching the SDK's BlobResourceContents shape (the handler always sets it).
mimeType: z.string().optional(),
blob: z.string()
})
})
]))
},
annotations: { readOnlyHint: true }
},
Expand All @@ -283,17 +295,23 @@ server.registerTool(
const mimeType = mimeTypes[extension] || "application/octet-stream";
const data = await readFileAsBase64Stream(validPath);

const type = mimeType.startsWith("image/")
? "image"
: mimeType.startsWith("audio/")
? "audio"
// Fallback for other binary types, not officially supported by the spec but has been used for some time
: "blob";
const contentItem = { type: type as 'image' | 'audio' | 'blob', data, mimeType };
// Map the MIME type to a valid MCP content block. The spec only allows
// text, image, audio, resource_link, and resource — so non-image/audio
// binaries are returned as an embedded resource (NOT type:"blob", which the
// SDK content-block union rejects on schema validation).
const contentItem =
mimeType.startsWith("image/")
? { type: "image" as const, data, mimeType }
: mimeType.startsWith("audio/")
? { type: "audio" as const, data, mimeType }
: {
type: "resource" as const,
resource: { uri: pathToFileURL(validPath).href, mimeType, blob: data }
};
return {
content: [contentItem],
structuredContent: { content: [contentItem] }
} as unknown as CallToolResult;
};
}
);

Expand Down
Loading