From cb4d54a9210e23296c22044231b9f58f94dc0888 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 13:24:53 +0000 Subject: [PATCH 01/15] pdf-server: support folders in cli + files in roots --- examples/pdf-server/main.ts | 11 +++++++++-- examples/pdf-server/server.ts | 8 +++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/examples/pdf-server/main.ts b/examples/pdf-server/main.ts index 50dc43aa0..d9b50f8ed 100644 --- a/examples/pdf-server/main.ts +++ b/examples/pdf-server/main.ts @@ -5,6 +5,7 @@ */ import fs from "node:fs"; +import path from "node:path"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { createMcpExpressApp } from "@modelcontextprotocol/sdk/server/express.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; @@ -122,8 +123,14 @@ async function main() { if (isFileUrl(url)) { const filePath = fileUrlToPath(url); if (fs.existsSync(filePath)) { - allowedLocalFiles.add(filePath); - console.error(`[pdf-server] Registered local file: ${filePath}`); + const s = fs.statSync(filePath); + if (s.isFile()) { + allowedLocalFiles.add(filePath); + console.error(`[pdf-server] Registered local file: ${filePath}`); + } else if (s.isDirectory()) { + allowedLocalDirs.add(filePath); + console.error(`[pdf-server] Registered local directory: ${filePath}`); + } } else { console.error(`[pdf-server] Warning: File not found: ${filePath}`); } diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index e68fdde47..09870567f 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -357,7 +357,13 @@ async function refreshRoots(server: Server): Promise { const dir = fileUrlToPath(root.uri); const resolved = path.resolve(dir); try { - if (fs.statSync(resolved).isDirectory()) { + const s = fs.statSync(resolved); + if (s.isFile()) { + console.error( + `[pdf-server] Root is a file, not a directory (skipped): ${resolved}`, + ); + allowedLocalFiles.add(resolved); + } else if (s.isDirectory()) { allowedLocalDirs.add(resolved); console.error(`[pdf-server] Root directory allowed: ${resolved}`); } From ccd32c3a01c9f88c2e9a3b179a3ddeef2df1430c Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:00:44 +0000 Subject: [PATCH 02/15] fix(pdf-server): use path.relative for robust ancestor dir matching - Replace string prefix matching (startsWith) with path.relative-based isAncestorDir() which correctly handles trailing slashes, path normalization, and prevents both .. traversal and prefix attacks. - Use path.resolve consistently when adding paths to allowedLocalFiles and allowedLocalDirs (main.ts was missing this). - Use resolved path for exact file match check in validateUrl. - Improve error message formatting for easier debugging. - Add tests for trailing slashes, grandparent dirs, and isAncestorDir. --- examples/pdf-server/main.ts | 77 ++++++++++++++++++++++++++---- examples/pdf-server/server.test.ts | 62 ++++++++++++++++++++++++ examples/pdf-server/server.ts | 44 ++++++++++++----- 3 files changed, 163 insertions(+), 20 deletions(-) diff --git a/examples/pdf-server/main.ts b/examples/pdf-server/main.ts index d9b50f8ed..21c0d4de1 100644 --- a/examples/pdf-server/main.ts +++ b/examples/pdf-server/main.ts @@ -10,6 +10,11 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js" import { createMcpExpressApp } from "@modelcontextprotocol/sdk/server/express.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; +import type { + Transport, + TransportSendOptions, +} from "@modelcontextprotocol/sdk/shared/transport.js"; +import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js"; import cors from "cors"; import type { Request, Response } from "express"; import { @@ -21,8 +26,59 @@ import { fileUrlToPath, allowedLocalFiles, DEFAULT_PDF, + allowedLocalDirs, } from "./server.js"; +// ============================================================================= +// JSONL Interaction Logger +// ============================================================================= + +const LOG_PATH = path.join(import.meta.dirname, "interactions.jsonl"); + +/** Append a JSONL line to the log file. */ +function logInteraction(entry: Record): void { + const line = JSON.stringify({ + timestamp: new Date().toISOString(), + ...entry, + }); + fs.appendFile(LOG_PATH, line + "\n", (err) => { + if (err) console.error("[pdf-server] Failed to write log:", err.message); + }); +} + +/** + * Wraps an MCP transport to log every incoming and outgoing JSON-RPC message + * to a JSONL file at `interactions.jsonl` next to this script. + */ +function withLogging(transport: T): T { + const originalSend = transport.send.bind(transport); + + transport.send = async ( + message: JSONRPCMessage, + options?: TransportSendOptions, + ): Promise => { + logInteraction({ direction: "server→client", message }); + return originalSend(message, options); + }; + + // Intercept the onmessage setter so we can log before the real handler runs + let realOnMessage = transport.onmessage; + Object.defineProperty(transport, "onmessage", { + get: () => realOnMessage, + set: (handler: typeof transport.onmessage) => { + realOnMessage = handler + ? (message, extra) => { + logInteraction({ direction: "client→server", message }); + handler(message, extra); + } + : handler; + }, + configurable: true, + }); + + return transport; +} + /** * Starts an MCP server with Streamable HTTP transport in stateless mode. */ @@ -36,9 +92,11 @@ export async function startStreamableHTTPServer( app.all("/mcp", async (req: Request, res: Response) => { const server = createServer(); - const transport = new StreamableHTTPServerTransport({ - sessionIdGenerator: undefined, - }); + const transport = withLogging( + new StreamableHTTPServerTransport({ + sessionIdGenerator: undefined, + }), + ); res.on("close", () => { transport.close().catch(() => {}); @@ -85,7 +143,7 @@ export async function startStreamableHTTPServer( export async function startStdioServer( createServer: () => McpServer, ): Promise { - await createServer().connect(new StdioServerTransport()); + await createServer().connect(withLogging(new StdioServerTransport())); } function parseArgs(): { urls: string[]; stdio: boolean } { @@ -121,7 +179,7 @@ async function main() { // Register local files in whitelist for (const url of urls) { if (isFileUrl(url)) { - const filePath = fileUrlToPath(url); + const filePath = path.resolve(fileUrlToPath(url)); if (fs.existsSync(filePath)) { const s = fs.statSync(filePath); if (s.isFile()) { @@ -137,12 +195,15 @@ async function main() { } } - console.error(`[pdf-server] Ready (${urls.length} URL(s) configured)`); + console.error( + `[pdf-server] Ready (${urls.length} URL(s) configured, logging to ${LOG_PATH})`, + ); + const factory = () => createServer(logInteraction); if (stdio) { - await startStdioServer(createServer); + await startStdioServer(factory); } else { - await startStreamableHTTPServer(createServer); + await startStreamableHTTPServer(factory); } } diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index 95efa7df7..e0a4c39cc 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { createPdfCache, validateUrl, + isAncestorDir, allowedLocalFiles, allowedLocalDirs, pathToFileUrl, @@ -271,4 +272,65 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { expect(result.valid).toBe(false); expect(result.error).toContain("File not found"); }); + + it("should allow a file under an allowed dir with trailing slash", () => { + const dir = path.resolve(import.meta.dirname); + // Simulate a dir stored with a trailing slash (e.g. from CLI path) + allowedLocalDirs.add(dir + "/"); + + const filePath = path.join(dir, "server.ts"); + const result = validateUrl(pathToFileUrl(filePath)); + expect(result.valid).toBe(true); + }); + + it("should allow a file under a grandparent allowed dir", () => { + // Allow a directory two levels up from the file + const grandparent = path.resolve(path.join(import.meta.dirname, "..")); + allowedLocalDirs.add(grandparent); + + const filePath = path.join(import.meta.dirname, "server.ts"); + const result = validateUrl(pathToFileUrl(filePath)); + expect(result.valid).toBe(true); + }); +}); + +describe("isAncestorDir", () => { + it("should return true for a direct child", () => { + expect(isAncestorDir("/Users/test/dir", "/Users/test/dir/file.pdf")).toBe( + true, + ); + }); + + it("should return true for a nested child", () => { + expect(isAncestorDir("/Users/test", "/Users/test/sub/dir/file.pdf")).toBe( + true, + ); + }); + + it("should return false for a file outside the dir", () => { + expect(isAncestorDir("/Users/test/dir", "/Users/test/other/file.pdf")).toBe( + false, + ); + }); + + it("should return false for the dir itself", () => { + expect(isAncestorDir("/Users/test/dir", "/Users/test/dir")).toBe(false); + }); + + it("should prevent .. traversal", () => { + expect( + isAncestorDir("/Users/test/dir", "/Users/test/dir/../other/file.pdf"), + ).toBe(false); + }); + + it("should prevent prefix-based traversal", () => { + // /tmp/safe should NOT match /tmp/safevil/file.pdf + expect(isAncestorDir("/tmp/safe", "/tmp/safevil/file.pdf")).toBe(false); + }); + + it("should handle dirs with trailing slash", () => { + expect(isAncestorDir("/Users/test/dir/", "/Users/test/dir/file.pdf")).toBe( + true, + ); + }); }); diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 09870567f..f8d5d340d 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -89,27 +89,39 @@ export function pathToFileUrl(filePath: string): string { return `file://${encodeURIComponent(absolutePath).replace(/%2F/g, "/")}`; } +/** + * Check if `dir` is an ancestor of `filePath` using path.relative, + * which is more robust than string prefix matching (handles normalization). + */ +export function isAncestorDir(dir: string, filePath: string): boolean { + const rel = path.relative(dir, filePath); + // Must be non-empty (not the dir itself when checking files), + // must not start with ".." (escaping), and must not be absolute (different root). + return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel); +} + export function validateUrl(url: string): { valid: boolean; error?: string } { if (isFileUrl(url)) { const filePath = fileUrlToPath(url); const resolved = path.resolve(filePath); - // Check exact match (CLI args) - const exactMatch = allowedLocalFiles.has(filePath); + // Check exact match (CLI args / roots) + const exactMatch = allowedLocalFiles.has(resolved); - // Check directory match (MCP roots) - const dirMatch = [...allowedLocalDirs].some( - (dir) => resolved === dir || resolved.startsWith(dir + path.sep), + // Check directory match (MCP roots / CLI dirs) using path.relative + // which is more robust than string prefix matching. + const dirMatch = [...allowedLocalDirs].some((dir) => + isAncestorDir(dir, resolved), ); if (!exactMatch && !dirMatch) { return { valid: false, - error: `Local file not in allowed list: ${filePath}`, + error: `Local file not in allowed list: \n${resolved}\nAllowed files: ${[...allowedLocalFiles].join(", ")}\nAllowed directories:\n${[...allowedLocalDirs].join("\n")}`, }; } - if (!fs.existsSync(filePath)) { - return { valid: false, error: `File not found: ${filePath}` }; + if (!fs.existsSync(resolved)) { + return { valid: false, error: `File not found: ${resolved}` }; } return { valid: true }; } @@ -342,15 +354,23 @@ export function createPdfCache(): PdfCache { // MCP Roots // ============================================================================= +/** Optional structured logger for JSONL interaction logging. */ +export type InteractionLogger = (entry: Record) => void; + /** * Query the client for roots and update allowedLocalDirs with any file:// roots * that point to existing directories. */ -async function refreshRoots(server: Server): Promise { +async function refreshRoots( + server: Server, + trigger: "initialized" | "roots/list_changed", + log?: InteractionLogger, +): Promise { if (!server.getClientCapabilities()?.roots) return; try { const { roots } = await server.listRoots(); + log?.({ event: "roots/list", trigger, roots }); allowedLocalDirs.clear(); for (const root of roots) { if (root.uri.startsWith("file://")) { @@ -383,17 +403,17 @@ async function refreshRoots(server: Server): Promise { // MCP Server Factory // ============================================================================= -export function createServer(): McpServer { +export function createServer(log?: InteractionLogger): McpServer { const server = new McpServer({ name: "PDF Server", version: "2.0.0" }); // Fetch roots on initialization and subscribe to changes server.server.oninitialized = () => { - refreshRoots(server.server); + refreshRoots(server.server, "initialized", log); }; server.server.setNotificationHandler( RootsListChangedNotificationSchema, async () => { - await refreshRoots(server.server); + await refreshRoots(server.server, "roots/list_changed", log); }, ); From d1ea22bff2291cc495414f6eb65b68d7bca1858f Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:05:45 +0000 Subject: [PATCH 03/15] chore(pdf-server): remove JSONL interaction logging Remove the withLogging transport wrapper, logInteraction helper, InteractionLogger type, and all log parameter threading that were added for debugging. --- examples/pdf-server/main.ts | 74 ++++------------------------------- examples/pdf-server/server.ts | 16 ++------ 2 files changed, 11 insertions(+), 79 deletions(-) diff --git a/examples/pdf-server/main.ts b/examples/pdf-server/main.ts index 21c0d4de1..ddf26c2be 100644 --- a/examples/pdf-server/main.ts +++ b/examples/pdf-server/main.ts @@ -10,11 +10,6 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js" import { createMcpExpressApp } from "@modelcontextprotocol/sdk/server/express.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; -import type { - Transport, - TransportSendOptions, -} from "@modelcontextprotocol/sdk/shared/transport.js"; -import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js"; import cors from "cors"; import type { Request, Response } from "express"; import { @@ -29,56 +24,6 @@ import { allowedLocalDirs, } from "./server.js"; -// ============================================================================= -// JSONL Interaction Logger -// ============================================================================= - -const LOG_PATH = path.join(import.meta.dirname, "interactions.jsonl"); - -/** Append a JSONL line to the log file. */ -function logInteraction(entry: Record): void { - const line = JSON.stringify({ - timestamp: new Date().toISOString(), - ...entry, - }); - fs.appendFile(LOG_PATH, line + "\n", (err) => { - if (err) console.error("[pdf-server] Failed to write log:", err.message); - }); -} - -/** - * Wraps an MCP transport to log every incoming and outgoing JSON-RPC message - * to a JSONL file at `interactions.jsonl` next to this script. - */ -function withLogging(transport: T): T { - const originalSend = transport.send.bind(transport); - - transport.send = async ( - message: JSONRPCMessage, - options?: TransportSendOptions, - ): Promise => { - logInteraction({ direction: "server→client", message }); - return originalSend(message, options); - }; - - // Intercept the onmessage setter so we can log before the real handler runs - let realOnMessage = transport.onmessage; - Object.defineProperty(transport, "onmessage", { - get: () => realOnMessage, - set: (handler: typeof transport.onmessage) => { - realOnMessage = handler - ? (message, extra) => { - logInteraction({ direction: "client→server", message }); - handler(message, extra); - } - : handler; - }, - configurable: true, - }); - - return transport; -} - /** * Starts an MCP server with Streamable HTTP transport in stateless mode. */ @@ -92,11 +37,9 @@ export async function startStreamableHTTPServer( app.all("/mcp", async (req: Request, res: Response) => { const server = createServer(); - const transport = withLogging( - new StreamableHTTPServerTransport({ - sessionIdGenerator: undefined, - }), - ); + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: undefined, + }); res.on("close", () => { transport.close().catch(() => {}); @@ -143,7 +86,7 @@ export async function startStreamableHTTPServer( export async function startStdioServer( createServer: () => McpServer, ): Promise { - await createServer().connect(withLogging(new StdioServerTransport())); + await createServer().connect(new StdioServerTransport()); } function parseArgs(): { urls: string[]; stdio: boolean } { @@ -195,15 +138,12 @@ async function main() { } } - console.error( - `[pdf-server] Ready (${urls.length} URL(s) configured, logging to ${LOG_PATH})`, - ); + console.error(`[pdf-server] Ready (${urls.length} URL(s) configured)`); - const factory = () => createServer(logInteraction); if (stdio) { - await startStdioServer(factory); + await startStdioServer(createServer); } else { - await startStreamableHTTPServer(factory); + await startStreamableHTTPServer(createServer); } } diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index f8d5d340d..cadd42131 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -354,23 +354,15 @@ export function createPdfCache(): PdfCache { // MCP Roots // ============================================================================= -/** Optional structured logger for JSONL interaction logging. */ -export type InteractionLogger = (entry: Record) => void; - /** * Query the client for roots and update allowedLocalDirs with any file:// roots * that point to existing directories. */ -async function refreshRoots( - server: Server, - trigger: "initialized" | "roots/list_changed", - log?: InteractionLogger, -): Promise { +async function refreshRoots(server: Server): Promise { if (!server.getClientCapabilities()?.roots) return; try { const { roots } = await server.listRoots(); - log?.({ event: "roots/list", trigger, roots }); allowedLocalDirs.clear(); for (const root of roots) { if (root.uri.startsWith("file://")) { @@ -403,17 +395,17 @@ async function refreshRoots( // MCP Server Factory // ============================================================================= -export function createServer(log?: InteractionLogger): McpServer { +export function createServer(): McpServer { const server = new McpServer({ name: "PDF Server", version: "2.0.0" }); // Fetch roots on initialization and subscribe to changes server.server.oninitialized = () => { - refreshRoots(server.server, "initialized", log); + refreshRoots(server.server); }; server.server.setNotificationHandler( RootsListChangedNotificationSchema, async () => { - await refreshRoots(server.server, "roots/list_changed", log); + await refreshRoots(server.server); }, ); From 1881c571f71624d97fa9af763d95e0d7de171b77 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:10:39 +0000 Subject: [PATCH 04/15] fix(pdf-server): support computer:// URL scheme for local files Some clients (e.g. Claude Code) use computer:// instead of file:// for local file references. Handle both schemes in isFileUrl and fileUrlToPath. --- examples/pdf-server/server.test.ts | 10 ++++++++++ examples/pdf-server/server.ts | 9 ++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index e0a4c39cc..26f9d4e60 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -292,6 +292,16 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { const result = validateUrl(pathToFileUrl(filePath)); expect(result.valid).toBe(true); }); + + it("should accept computer:// URLs as local files", () => { + const dir = path.resolve(import.meta.dirname); + allowedLocalDirs.add(dir); + + const filePath = path.join(dir, "server.ts"); + const encoded = encodeURIComponent(filePath).replace(/%2F/g, "/"); + const result = validateUrl(`computer://${encoded}`); + expect(result.valid).toBe(true); + }); }); describe("isAncestorDir", () => { diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index cadd42131..4ca04a9f6 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -60,7 +60,7 @@ const DIST_DIR = import.meta.filename.endsWith(".ts") // ============================================================================= export function isFileUrl(url: string): boolean { - return url.startsWith("file://"); + return url.startsWith("file://") || url.startsWith("computer://"); } export function isArxivUrl(url: string): boolean { @@ -81,7 +81,10 @@ export function normalizeArxivUrl(url: string): string { } export function fileUrlToPath(fileUrl: string): string { - return decodeURIComponent(fileUrl.replace("file://", "")); + // Support both file:// and computer:// (used by some clients for local files) + return decodeURIComponent( + fileUrl.replace(/^(?:file|computer):\/\//, ""), + ); } export function pathToFileUrl(filePath: string): string { @@ -365,7 +368,7 @@ async function refreshRoots(server: Server): Promise { const { roots } = await server.listRoots(); allowedLocalDirs.clear(); for (const root of roots) { - if (root.uri.startsWith("file://")) { + if (isFileUrl(root.uri)) { const dir = fileUrlToPath(root.uri); const resolved = path.resolve(dir); try { From 166895193e6a303aeeea5acd94a7fc040589cea5 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:16:55 +0000 Subject: [PATCH 05/15] fix(pdf-server): add debug logging for rejected files, ignore .jsonl logs --- examples/pdf-server/.gitignore | 1 + examples/pdf-server/.mcpbignore | 3 +++ examples/pdf-server/server.ts | 7 ++++--- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/examples/pdf-server/.gitignore b/examples/pdf-server/.gitignore index 849ddff3b..a856bb2b7 100644 --- a/examples/pdf-server/.gitignore +++ b/examples/pdf-server/.gitignore @@ -1 +1,2 @@ dist/ +*.jsonl diff --git a/examples/pdf-server/.mcpbignore b/examples/pdf-server/.mcpbignore index fe11c64e7..3f7859c20 100644 --- a/examples/pdf-server/.mcpbignore +++ b/examples/pdf-server/.mcpbignore @@ -7,6 +7,9 @@ src/ # Tests *.test.* +# Debug logs +*.jsonl + # Development assets screenshot.png grid-cell.png diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 4ca04a9f6..eb7aa148c 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -82,9 +82,7 @@ export function normalizeArxivUrl(url: string): string { export function fileUrlToPath(fileUrl: string): string { // Support both file:// and computer:// (used by some clients for local files) - return decodeURIComponent( - fileUrl.replace(/^(?:file|computer):\/\//, ""), - ); + return decodeURIComponent(fileUrl.replace(/^(?:file|computer):\/\//, "")); } export function pathToFileUrl(filePath: string): string { @@ -118,6 +116,9 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { ); if (!exactMatch && !dirMatch) { + console.error( + `[pdf-server] validateUrl REJECTED: url=${url}\n filePath=${filePath}\n resolved=${resolved}\n allowedDirs=${JSON.stringify([...allowedLocalDirs])}\n dirChecks=${JSON.stringify([...allowedLocalDirs].map((d) => ({ dir: d, rel: path.relative(d, resolved), match: isAncestorDir(d, resolved) })))}`, + ); return { valid: false, error: `Local file not in allowed list: \n${resolved}\nAllowed files: ${[...allowedLocalFiles].join(", ")}\nAllowed directories:\n${[...allowedLocalDirs].join("\n")}`, From 3d6ba159bd6b5c400d641228066b3822339ef992 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:30:14 +0000 Subject: [PATCH 06/15] fix(pdf-server): accept bare file paths in addition to file:// URLs Clients like Claude Desktop may pass raw absolute paths (e.g. /Users/.../file.pdf) instead of file:// URLs. Handle these in validateUrl and readPdfRange. --- examples/pdf-server/server.test.ts | 9 +++++++++ examples/pdf-server/server.ts | 30 ++++++++++++++++++++++-------- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index 26f9d4e60..63463f81e 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -302,6 +302,15 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { const result = validateUrl(`computer://${encoded}`); expect(result.valid).toBe(true); }); + + it("should accept bare absolute paths as local files", () => { + const dir = path.resolve(import.meta.dirname); + allowedLocalDirs.add(dir); + + const filePath = path.join(dir, "server.ts"); + const result = validateUrl(filePath); + expect(result.valid).toBe(true); + }); }); describe("isAncestorDir", () => { diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index eb7aa148c..970044f07 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -101,9 +101,17 @@ export function isAncestorDir(dir: string, filePath: string): boolean { return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel); } +/** + * Check if `url` looks like an absolute local file path (not a URL scheme). + * Handles Unix paths (/...), home-relative (~), and Windows drive letters (C:\...). + */ +function isLocalPath(url: string): boolean { + return url.startsWith("/") || url.startsWith("~") || /^[A-Za-z]:[/\\]/.test(url); +} + export function validateUrl(url: string): { valid: boolean; error?: string } { - if (isFileUrl(url)) { - const filePath = fileUrlToPath(url); + if (isFileUrl(url) || isLocalPath(url)) { + const filePath = isFileUrl(url) ? fileUrlToPath(url) : url; const resolved = path.resolve(filePath); // Check exact match (CLI args / roots) @@ -116,12 +124,16 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { ); if (!exactMatch && !dirMatch) { + const diagnostics = [...allowedLocalDirs].map((d) => { + const rel = path.relative(d, resolved); + return `dir=${d} rel=${rel} match=${isAncestorDir(d, resolved)}`; + }); console.error( - `[pdf-server] validateUrl REJECTED: url=${url}\n filePath=${filePath}\n resolved=${resolved}\n allowedDirs=${JSON.stringify([...allowedLocalDirs])}\n dirChecks=${JSON.stringify([...allowedLocalDirs].map((d) => ({ dir: d, rel: path.relative(d, resolved), match: isAncestorDir(d, resolved) })))}`, + `[pdf-server] validateUrl REJECTED:\n url=${url}\n resolved=${resolved}\n diagnostics:\n ${diagnostics.join("\n ")}`, ); return { valid: false, - error: `Local file not in allowed list: \n${resolved}\nAllowed files: ${[...allowedLocalFiles].join(", ")}\nAllowed directories:\n${[...allowedLocalDirs].join("\n")}`, + error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}\nDiagnostics: ${diagnostics.join(" | ")}`, }; } if (!fs.existsSync(resolved)) { @@ -254,8 +266,10 @@ export function createPdfCache(): PdfCache { const normalized = isArxivUrl(url) ? normalizeArxivUrl(url) : url; const clampedByteCount = Math.min(byteCount, MAX_CHUNK_BYTES); - if (isFileUrl(normalized)) { - const filePath = fileUrlToPath(normalized); + if (isFileUrl(normalized) || isLocalPath(normalized)) { + const filePath = isFileUrl(normalized) + ? fileUrlToPath(normalized) + : normalized; const stats = await fs.promises.stat(filePath); const totalBytes = stats.size; @@ -463,7 +477,7 @@ export function createServer(): McpServer { title: "Read PDF Bytes", description: "Read a range of bytes from a PDF (max 512KB per request)", inputSchema: { - url: z.string().describe("PDF URL"), + url: z.string().describe("PDF URL or local file path"), offset: z.number().min(0).default(0).describe("Byte offset"), byteCount: z .number() @@ -542,7 +556,7 @@ Accepts: - Local files under directories provided by the client as MCP roots - Any remote PDF accessible via HTTPS`, inputSchema: { - url: z.string().default(DEFAULT_PDF).describe("PDF URL"), + url: z.string().default(DEFAULT_PDF).describe("PDF URL or local file path"), page: z.number().min(1).default(1).describe("Initial page"), }, outputSchema: z.object({ From 5e6d03bb60576be64ef2ab594931059141153715 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:37:44 +0000 Subject: [PATCH 07/15] debug(pdf-server): add hex char diagnostics for path mismatch --- examples/pdf-server/server.ts | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 970044f07..c145ca109 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -106,7 +106,9 @@ export function isAncestorDir(dir: string, filePath: string): boolean { * Handles Unix paths (/...), home-relative (~), and Windows drive letters (C:\...). */ function isLocalPath(url: string): boolean { - return url.startsWith("/") || url.startsWith("~") || /^[A-Za-z]:[/\\]/.test(url); + return ( + url.startsWith("/") || url.startsWith("~") || /^[A-Za-z]:[/\\]/.test(url) + ); } export function validateUrl(url: string): { valid: boolean; error?: string } { @@ -125,11 +127,20 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { if (!exactMatch && !dirMatch) { const diagnostics = [...allowedLocalDirs].map((d) => { - const rel = path.relative(d, resolved); - return `dir=${d} rel=${rel} match=${isAncestorDir(d, resolved)}`; + // Find first char that differs to diagnose encoding issues + const prefix = resolved.substring(0, d.length); + let firstDiff = ""; + for (let i = 0; i < Math.min(d.length, prefix.length); i++) { + if (d[i] !== prefix[i]) { + firstDiff = `firstDiff@${i}: dir=0x${d.charCodeAt(i).toString(16)} file=0x${prefix.charCodeAt(i).toString(16)}`; + break; + } + } + if (!firstDiff && d.length === prefix.length) firstDiff = "IDENTICAL_PREFIX"; + return `match=${isAncestorDir(d, resolved)} ${firstDiff} resolvedLen=${resolved.length} dirLen=${d.length}`; }); console.error( - `[pdf-server] validateUrl REJECTED:\n url=${url}\n resolved=${resolved}\n diagnostics:\n ${diagnostics.join("\n ")}`, + `[pdf-server] REJECTED url=${JSON.stringify(url)}\n resolved=${JSON.stringify(resolved)}\n dirs=${JSON.stringify([...allowedLocalDirs])}\n ${diagnostics.join("\n ")}`, ); return { valid: false, @@ -556,7 +567,10 @@ Accepts: - Local files under directories provided by the client as MCP roots - Any remote PDF accessible via HTTPS`, inputSchema: { - url: z.string().default(DEFAULT_PDF).describe("PDF URL or local file path"), + url: z + .string() + .default(DEFAULT_PDF) + .describe("PDF URL or local file path"), page: z.number().min(1).default(1).describe("Initial page"), }, outputSchema: z.object({ From 527f42fffce4ad5afc7de720f03c32ec93ef3f69 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:43:43 +0000 Subject: [PATCH 08/15] fix(pdf-server): resolve symlinks to handle sandbox path remapping Use fs.realpathSync to resolve symlinks/bind mounts when comparing file paths against allowed directories. This fixes the path namespace mismatch where Claude Desktop sends container paths (e.g. /sessions/...) while MCP roots use host paths (e.g. /Users/...). Both the file path and directory paths are resolved through symlinks before comparison, so either side can be a symlink. Also stores realpath of roots in allowedLocalDirs/Files so matching works in both directions. Removes verbose hex diagnostics (no longer needed). --- examples/pdf-server/server.test.ts | 49 ++++++++++++++++++++++ examples/pdf-server/server.ts | 67 +++++++++++++++++++----------- 2 files changed, 91 insertions(+), 25 deletions(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index 63463f81e..f99fca0cb 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -311,6 +311,55 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { const result = validateUrl(filePath); expect(result.valid).toBe(true); }); + + it("should allow file accessed via symlink when real dir is allowed", () => { + const fs = require("node:fs"); + const os = require("node:os"); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdf-test-")); + const realDir = path.join(tmpDir, "real"); + const linkDir = path.join(tmpDir, "link"); + const testFile = path.join(realDir, "test.txt"); + + try { + fs.mkdirSync(realDir); + fs.writeFileSync(testFile, "hello"); + fs.symlinkSync(realDir, linkDir); + + // Allow the REAL directory + allowedLocalDirs.add(realDir); + + // Access via the SYMLINK path — should still be allowed + const symlinkPath = path.join(linkDir, "test.txt"); + const result = validateUrl(symlinkPath); + expect(result.valid).toBe(true); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it("should allow file when allowed dir is a symlink to real parent", () => { + const fs = require("node:fs"); + const os = require("node:os"); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdf-test-")); + const realDir = path.join(tmpDir, "real"); + const linkDir = path.join(tmpDir, "link"); + const testFile = path.join(realDir, "test.txt"); + + try { + fs.mkdirSync(realDir); + fs.writeFileSync(testFile, "hello"); + fs.symlinkSync(realDir, linkDir); + + // Allow the SYMLINK directory + allowedLocalDirs.add(linkDir); + + // Access via the REAL path — should still be allowed + const result = validateUrl(testFile); + expect(result.valid).toBe(true); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); }); describe("isAncestorDir", () => { diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index c145ca109..26b28b976 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -101,6 +101,18 @@ export function isAncestorDir(dir: string, filePath: string): boolean { return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel); } +/** + * Try to resolve a path through symlinks using fs.realpathSync. + * Returns the original path if resolution fails (e.g., file doesn't exist yet). + */ +function tryRealpath(p: string): string { + try { + return fs.realpathSync(p); + } catch { + return p; + } +} + /** * Check if `url` looks like an absolute local file path (not a URL scheme). * Handles Unix paths (/...), home-relative (~), and Windows drive letters (C:\...). @@ -115,36 +127,34 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { if (isFileUrl(url) || isLocalPath(url)) { const filePath = isFileUrl(url) ? fileUrlToPath(url) : url; const resolved = path.resolve(filePath); - - // Check exact match (CLI args / roots) - const exactMatch = allowedLocalFiles.has(resolved); - - // Check directory match (MCP roots / CLI dirs) using path.relative - // which is more robust than string prefix matching. - const dirMatch = [...allowedLocalDirs].some((dir) => - isAncestorDir(dir, resolved), - ); + // Resolve through symlinks/bind mounts to handle sandbox path remapping + // (e.g., client sends /sessions/... but roots use /Users/...) + const real = tryRealpath(resolved); + + // Check exact match (CLI args / roots) — try both resolved and realpath + const exactMatch = + allowedLocalFiles.has(resolved) || allowedLocalFiles.has(real); + + // Check directory match (MCP roots / CLI dirs) using path.relative. + // Try both the resolved path and its realpath against both the raw dir + // and its realpath, to handle sandbox path remapping in either direction. + const dirMatch = [...allowedLocalDirs].some((dir) => { + const realDir = tryRealpath(dir); + return ( + isAncestorDir(dir, resolved) || + isAncestorDir(dir, real) || + isAncestorDir(realDir, resolved) || + isAncestorDir(realDir, real) + ); + }); if (!exactMatch && !dirMatch) { - const diagnostics = [...allowedLocalDirs].map((d) => { - // Find first char that differs to diagnose encoding issues - const prefix = resolved.substring(0, d.length); - let firstDiff = ""; - for (let i = 0; i < Math.min(d.length, prefix.length); i++) { - if (d[i] !== prefix[i]) { - firstDiff = `firstDiff@${i}: dir=0x${d.charCodeAt(i).toString(16)} file=0x${prefix.charCodeAt(i).toString(16)}`; - break; - } - } - if (!firstDiff && d.length === prefix.length) firstDiff = "IDENTICAL_PREFIX"; - return `match=${isAncestorDir(d, resolved)} ${firstDiff} resolvedLen=${resolved.length} dirLen=${d.length}`; - }); console.error( - `[pdf-server] REJECTED url=${JSON.stringify(url)}\n resolved=${JSON.stringify(resolved)}\n dirs=${JSON.stringify([...allowedLocalDirs])}\n ${diagnostics.join("\n ")}`, + `[pdf-server] Local file not in allowed list: ${resolved} (real: ${real})\n Allowed dirs: ${[...allowedLocalDirs].join(", ")}`, ); return { valid: false, - error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}\nDiagnostics: ${diagnostics.join(" | ")}`, + error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}`, }; } if (!fs.existsSync(resolved)) { @@ -399,14 +409,21 @@ async function refreshRoots(server: Server): Promise { const resolved = path.resolve(dir); try { const s = fs.statSync(resolved); + // Use realpath to resolve symlinks/bind mounts, so sandbox paths + // (e.g., /sessions/...) and host paths (e.g., /Users/...) both match. + const real = tryRealpath(resolved); if (s.isFile()) { console.error( `[pdf-server] Root is a file, not a directory (skipped): ${resolved}`, ); allowedLocalFiles.add(resolved); + if (real !== resolved) allowedLocalFiles.add(real); } else if (s.isDirectory()) { allowedLocalDirs.add(resolved); - console.error(`[pdf-server] Root directory allowed: ${resolved}`); + if (real !== resolved) allowedLocalDirs.add(real); + console.error( + `[pdf-server] Root directory allowed: ${resolved}${real !== resolved ? ` (real: ${real})` : ""}`, + ); } } catch { // stat failed — skip non-existent roots From 6d6559900faf58c50291c67d0d7d9430daa48ec5 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:50:21 +0000 Subject: [PATCH 09/15] fix(pdf-server): decode percent-encoded bare paths (e.g. %20 for spaces) Clients may send bare file paths with percent-encoded characters (e.g., Application%20Support). Apply decodeURIComponent to bare paths before resolving, matching the behavior already used for file:// URLs. --- examples/pdf-server/server.test.ts | 19 +++++++++++++++++++ examples/pdf-server/server.ts | 9 +++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index f99fca0cb..dd5ac4474 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -312,6 +312,25 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { expect(result.valid).toBe(true); }); + it("should decode percent-encoded bare paths (e.g. %20 for spaces)", () => { + const fs = require("node:fs"); + const os = require("node:os"); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdf test ")); + const testFile = path.join(tmpDir, "file.txt"); + + try { + fs.writeFileSync(testFile, "hello"); + allowedLocalDirs.add(tmpDir); + + // Encode spaces as %20 in the path (as some clients do) + const encoded = testFile.replace(/ /g, "%20"); + const result = validateUrl(encoded); + expect(result.valid).toBe(true); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + it("should allow file accessed via symlink when real dir is allowed", () => { const fs = require("node:fs"); const os = require("node:os"); diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 26b28b976..982ad7ebf 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -104,6 +104,7 @@ export function isAncestorDir(dir: string, filePath: string): boolean { /** * Try to resolve a path through symlinks using fs.realpathSync. * Returns the original path if resolution fails (e.g., file doesn't exist yet). + * Useful when sandbox/container path remapping uses symlinks. */ function tryRealpath(p: string): string { try { @@ -125,7 +126,11 @@ function isLocalPath(url: string): boolean { export function validateUrl(url: string): { valid: boolean; error?: string } { if (isFileUrl(url) || isLocalPath(url)) { - const filePath = isFileUrl(url) ? fileUrlToPath(url) : url; + // fileUrlToPath already decodes percent-encoding; for bare paths, + // decode here in case the client sends %20 for spaces etc. + const filePath = isFileUrl(url) + ? fileUrlToPath(url) + : decodeURIComponent(url); const resolved = path.resolve(filePath); // Resolve through symlinks/bind mounts to handle sandbox path remapping // (e.g., client sends /sessions/... but roots use /Users/...) @@ -290,7 +295,7 @@ export function createPdfCache(): PdfCache { if (isFileUrl(normalized) || isLocalPath(normalized)) { const filePath = isFileUrl(normalized) ? fileUrlToPath(normalized) - : normalized; + : decodeURIComponent(normalized); const stats = await fs.promises.stat(filePath); const totalBytes = stats.size; From 96ed79ba15682b13f63d0d8f5bc0525c71050acc Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 15:56:05 +0000 Subject: [PATCH 10/15] fix(pdf-server): add full diagnostics to client-facing error message Include url, resolved path, realpath, allowedFiles, and per-directory path.relative results in the error returned to the client, so we can diagnose why isAncestorDir fails when paths look identical. --- examples/pdf-server/server.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 982ad7ebf..c55f9f274 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -154,12 +154,25 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { }); if (!exactMatch && !dirMatch) { - console.error( - `[pdf-server] Local file not in allowed list: ${resolved} (real: ${real})\n Allowed dirs: ${[...allowedLocalDirs].join(", ")}`, - ); + // Build detailed diagnostics for debugging + const dirDetails = [...allowedLocalDirs].map((d) => { + const realD = tryRealpath(d); + const rel = path.relative(d, resolved); + return `dir=${JSON.stringify(d)} rel=${JSON.stringify(rel)} ancestor=${isAncestorDir(d, resolved)}${realD !== d ? ` realDir=${JSON.stringify(realD)}` : ""}`; + }); + const diag = [ + `url=${JSON.stringify(url)}`, + `resolved=${JSON.stringify(resolved)}`, + real !== resolved ? `real=${JSON.stringify(real)}` : null, + `allowedFiles=[${[...allowedLocalFiles].map((f) => JSON.stringify(f)).join(", ")}]`, + ...dirDetails, + ] + .filter(Boolean) + .join("\n"); + console.error(`[pdf-server] REJECTED:\n${diag}`); return { valid: false, - error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}`, + error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}\nDiagnostics:\n${diag}`, }; } if (!fs.existsSync(resolved)) { From c2fa8219743cf1a450f040911e247263bbb005fc Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 16:02:04 +0000 Subject: [PATCH 11/15] debug(pdf-server): add hex dump to diagnose invisible char differences path.relative returns 10 levels of .. despite paths looking identical, meaning they differ at the byte level. Add hex dump of first 30 chars of both paths to catch invisible Unicode or encoding differences. --- examples/pdf-server/server.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index c55f9f274..285693e02 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -154,21 +154,18 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { }); if (!exactMatch && !dirMatch) { - // Build detailed diagnostics for debugging + // Hex dump first 30 chars to catch invisible Unicode differences + const hex = (s: string, n = 30) => + [...s.slice(0, n)].map((c) => c.charCodeAt(0).toString(16).padStart(4, "0")).join(" "); const dirDetails = [...allowedLocalDirs].map((d) => { - const realD = tryRealpath(d); const rel = path.relative(d, resolved); - return `dir=${JSON.stringify(d)} rel=${JSON.stringify(rel)} ancestor=${isAncestorDir(d, resolved)}${realD !== d ? ` realDir=${JSON.stringify(realD)}` : ""}`; + return `dir_hex=[${hex(d)}]\nres_hex=[${hex(resolved)}]\nrel=${JSON.stringify(rel)} ancestor=${isAncestorDir(d, resolved)}`; }); const diag = [ - `url=${JSON.stringify(url)}`, `resolved=${JSON.stringify(resolved)}`, - real !== resolved ? `real=${JSON.stringify(real)}` : null, - `allowedFiles=[${[...allowedLocalFiles].map((f) => JSON.stringify(f)).join(", ")}]`, + `allowedFiles=${JSON.stringify([...allowedLocalFiles])}`, ...dirDetails, - ] - .filter(Boolean) - .join("\n"); + ].join("\n"); console.error(`[pdf-server] REJECTED:\n${diag}`); return { valid: false, From 9c68624378a7456b78b12a93a1b01392002070b4 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 16:15:53 +0000 Subject: [PATCH 12/15] fix(pdf-server): use inode comparison for bind-mount path matching The cowork VM remaps paths via bind mounts: the tool input uses VM-internal paths (/sessions/...) while MCP roots use host paths (/Users/...). Since path.relative can't match across bind mounts, compare directory inodes instead (dev+ino from fs.statSync). Walks up the file's parent directories checking each against allowed dirs by inode, which works regardless of path namespace differences. --- examples/pdf-server/server.ts | 89 ++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 285693e02..70c632612 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -102,16 +102,40 @@ export function isAncestorDir(dir: string, filePath: string): boolean { } /** - * Try to resolve a path through symlinks using fs.realpathSync. - * Returns the original path if resolution fails (e.g., file doesn't exist yet). - * Useful when sandbox/container path remapping uses symlinks. + * Check if two paths refer to the same directory by comparing device+inode. + * Works across bind mounts and symlinks where path strings differ but the + * underlying filesystem location is the same. */ -function tryRealpath(p: string): string { +function isSameDir(a: string, b: string): boolean { try { - return fs.realpathSync(p); + const sa = fs.statSync(a); + const sb = fs.statSync(b); + return sa.dev === sb.dev && sa.ino === sb.ino; } catch { - return p; + return false; + } +} + +/** + * Check if `filePath` is under `allowedDir`, handling bind mounts. + * First tries fast string-based comparison (path.relative), then + * falls back to inode comparison by walking up the directory tree. + */ +function isUnderAllowedDir(filePath: string, allowedDir: string): boolean { + // Fast path: string comparison works when paths are in the same namespace + if (isAncestorDir(allowedDir, filePath)) return true; + + // Slow path: walk up the directory tree and compare inodes. + // Handles bind mounts where paths differ but point to the same location. + let current = path.dirname(filePath); + const root = path.parse(current).root; + while (current !== root) { + if (isSameDir(current, allowedDir)) return true; + const parent = path.dirname(current); + if (parent === current) break; // safety: reached root + current = parent; } + return false; } /** @@ -132,44 +156,26 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { ? fileUrlToPath(url) : decodeURIComponent(url); const resolved = path.resolve(filePath); - // Resolve through symlinks/bind mounts to handle sandbox path remapping - // (e.g., client sends /sessions/... but roots use /Users/...) - const real = tryRealpath(resolved); - // Check exact match (CLI args / roots) — try both resolved and realpath + // Check exact match (CLI args / roots) const exactMatch = - allowedLocalFiles.has(resolved) || allowedLocalFiles.has(real); - - // Check directory match (MCP roots / CLI dirs) using path.relative. - // Try both the resolved path and its realpath against both the raw dir - // and its realpath, to handle sandbox path remapping in either direction. - const dirMatch = [...allowedLocalDirs].some((dir) => { - const realDir = tryRealpath(dir); - return ( - isAncestorDir(dir, resolved) || - isAncestorDir(dir, real) || - isAncestorDir(realDir, resolved) || - isAncestorDir(realDir, real) - ); - }); + allowedLocalFiles.has(resolved) || + [...allowedLocalFiles].some((f) => isSameDir(f, resolved)); + + // Check directory match (MCP roots / CLI dirs). + // Uses inode comparison to handle bind mounts (e.g., VM sandbox paths + // like /sessions/... that map to host paths like /Users/...). + const dirMatch = [...allowedLocalDirs].some((dir) => + isUnderAllowedDir(resolved, dir), + ); if (!exactMatch && !dirMatch) { - // Hex dump first 30 chars to catch invisible Unicode differences - const hex = (s: string, n = 30) => - [...s.slice(0, n)].map((c) => c.charCodeAt(0).toString(16).padStart(4, "0")).join(" "); - const dirDetails = [...allowedLocalDirs].map((d) => { - const rel = path.relative(d, resolved); - return `dir_hex=[${hex(d)}]\nres_hex=[${hex(resolved)}]\nrel=${JSON.stringify(rel)} ancestor=${isAncestorDir(d, resolved)}`; - }); - const diag = [ - `resolved=${JSON.stringify(resolved)}`, - `allowedFiles=${JSON.stringify([...allowedLocalFiles])}`, - ...dirDetails, - ].join("\n"); - console.error(`[pdf-server] REJECTED:\n${diag}`); + console.error( + `[pdf-server] Local file not in allowed list: ${resolved}\n Allowed dirs: ${[...allowedLocalDirs].join(", ")}`, + ); return { valid: false, - error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}\nDiagnostics:\n${diag}`, + error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}`, }; } if (!fs.existsSync(resolved)) { @@ -424,20 +430,15 @@ async function refreshRoots(server: Server): Promise { const resolved = path.resolve(dir); try { const s = fs.statSync(resolved); - // Use realpath to resolve symlinks/bind mounts, so sandbox paths - // (e.g., /sessions/...) and host paths (e.g., /Users/...) both match. - const real = tryRealpath(resolved); if (s.isFile()) { console.error( `[pdf-server] Root is a file, not a directory (skipped): ${resolved}`, ); allowedLocalFiles.add(resolved); - if (real !== resolved) allowedLocalFiles.add(real); } else if (s.isDirectory()) { allowedLocalDirs.add(resolved); - if (real !== resolved) allowedLocalDirs.add(real); console.error( - `[pdf-server] Root directory allowed: ${resolved}${real !== resolved ? ` (real: ${real})` : ""}`, + `[pdf-server] Root directory allowed: ${resolved}`, ); } } catch { From d1c3fdef60deadf63e4b688a84082fcc5cc8c2a2 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 16:27:10 +0000 Subject: [PATCH 13/15] fix(pdf-server): resolve VM-internal paths via directory basename matching When the MCP server runs on the host but receives unrewritten VM paths (e.g., /sessions/name/mnt/uploads/file.pdf), the tool call url argument isn't rewritten by the cowork VM proxy because it doesn't know which arguments are file paths. Fix: when path validation fails, try to match the file by finding the directory basename (e.g., 'uploads') in the VM path and looking for the relative path suffix under each allowed directory on the host. Also returns the resolved host path so readPdfRange reads the correct file on the host filesystem. --- examples/pdf-server/server.test.ts | 27 +++++ examples/pdf-server/server.ts | 161 ++++++++++++++++++----------- 2 files changed, 129 insertions(+), 59 deletions(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index dd5ac4474..73fa95680 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -379,6 +379,33 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { fs.rmSync(tmpDir, { recursive: true }); } }); + + it("should resolve VM-internal paths via directory basename matching", () => { + const fs = require("node:fs"); + const os = require("node:os"); + // Simulate: host has /tmp/xxx/uploads/file.txt + // VM sends /sessions/name/mnt/uploads/file.txt + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdf-vm-test-")); + const hostUploads = path.join(tmpDir, "uploads"); + fs.mkdirSync(hostUploads); + fs.writeFileSync(path.join(hostUploads, "report.pdf"), "fake-pdf"); + + try { + // Allow the host uploads directory + allowedLocalDirs.add(hostUploads); + + // Simulate VM-internal path that shares the "uploads" basename + const vmPath = "/sessions/gallant-tender-mayer/mnt/uploads/report.pdf"; + const result = validateUrl(vmPath); + expect(result.valid).toBe(true); + // The resolved path should point to the host file + expect(result.resolvedPath).toBe( + path.join(hostUploads, "report.pdf"), + ); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); }); describe("isAncestorDir", () => { diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index 70c632612..ebaca0fc8 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -102,40 +102,34 @@ export function isAncestorDir(dir: string, filePath: string): boolean { } /** - * Check if two paths refer to the same directory by comparing device+inode. - * Works across bind mounts and symlinks where path strings differ but the - * underlying filesystem location is the same. - */ -function isSameDir(a: string, b: string): boolean { - try { - const sa = fs.statSync(a); - const sb = fs.statSync(b); - return sa.dev === sb.dev && sa.ino === sb.ino; - } catch { - return false; - } -} - -/** - * Check if `filePath` is under `allowedDir`, handling bind mounts. - * First tries fast string-based comparison (path.relative), then - * falls back to inode comparison by walking up the directory tree. + * Try to resolve a VM-internal path to a host path by matching the path suffix + * against allowed directories. + * + * When the MCP server runs on the host but receives unrewritten VM paths + * (e.g., /sessions/name/mnt/uploads/file.pdf), this finds the equivalent host + * path (e.g., /Users/.../uploads/file.pdf) by matching directory basenames. + * + * Returns the host path if found, or null. */ -function isUnderAllowedDir(filePath: string, allowedDir: string): boolean { - // Fast path: string comparison works when paths are in the same namespace - if (isAncestorDir(allowedDir, filePath)) return true; - - // Slow path: walk up the directory tree and compare inodes. - // Handles bind mounts where paths differ but point to the same location. - let current = path.dirname(filePath); - const root = path.parse(current).root; - while (current !== root) { - if (isSameDir(current, allowedDir)) return true; - const parent = path.dirname(current); - if (parent === current) break; // safety: reached root - current = parent; +function resolveVmPath( + vmPath: string, + allowedDirs: ReadonlySet, +): string | null { + const parts = vmPath.split(path.sep); + for (const dir of allowedDirs) { + const dirBasename = path.basename(dir); + // Find the mount point in the VM path by matching the directory basename + const idx = parts.lastIndexOf(dirBasename); + if (idx >= 0) { + // Join allowed dir with the relative path after the mount point + const relativeParts = parts.slice(idx + 1); + const candidate = path.join(dir, ...relativeParts); + if (fs.existsSync(candidate)) { + return candidate; + } + } } - return false; + return null; } /** @@ -148,7 +142,11 @@ function isLocalPath(url: string): boolean { ); } -export function validateUrl(url: string): { valid: boolean; error?: string } { +export function validateUrl(url: string): { + valid: boolean; + error?: string; + resolvedPath?: string; +} { if (isFileUrl(url) || isLocalPath(url)) { // fileUrlToPath already decodes percent-encoding; for bare paths, // decode here in case the client sends %20 for spaces etc. @@ -158,30 +156,64 @@ export function validateUrl(url: string): { valid: boolean; error?: string } { const resolved = path.resolve(filePath); // Check exact match (CLI args / roots) - const exactMatch = - allowedLocalFiles.has(resolved) || - [...allowedLocalFiles].some((f) => isSameDir(f, resolved)); + if (allowedLocalFiles.has(resolved)) { + if (!fs.existsSync(resolved)) { + return { valid: false, error: `File not found: ${resolved}` }; + } + return { valid: true, resolvedPath: resolved }; + } // Check directory match (MCP roots / CLI dirs). - // Uses inode comparison to handle bind mounts (e.g., VM sandbox paths - // like /sessions/... that map to host paths like /Users/...). - const dirMatch = [...allowedLocalDirs].some((dir) => - isUnderAllowedDir(resolved, dir), - ); + // Try both the raw path and its realpath (resolves symlinks). + let realResolved: string | undefined; + try { + realResolved = fs.realpathSync(resolved); + } catch { + // File may not exist yet at this path + } + if ( + [...allowedLocalDirs].some((dir) => { + let realDir: string | undefined; + try { + realDir = fs.realpathSync(dir); + } catch { + // Dir may not exist + } + return ( + isAncestorDir(dir, resolved) || + (realResolved != null && isAncestorDir(dir, realResolved)) || + (realDir != null && isAncestorDir(realDir, resolved)) || + (realDir != null && + realResolved != null && + isAncestorDir(realDir, realResolved)) + ); + }) + ) { + if (!fs.existsSync(resolved)) { + return { valid: false, error: `File not found: ${resolved}` }; + } + return { valid: true, resolvedPath: resolved }; + } - if (!exactMatch && !dirMatch) { + // VM path fallback: the path may be a VM-internal path + // (e.g., /sessions/.../mnt/uploads/file.pdf) that wasn't rewritten + // to the host path. Try to find the file in allowed dirs by matching + // the directory basename as a mount point. + const hostPath = resolveVmPath(resolved, allowedLocalDirs); + if (hostPath) { console.error( - `[pdf-server] Local file not in allowed list: ${resolved}\n Allowed dirs: ${[...allowedLocalDirs].join(", ")}`, + `[pdf-server] Resolved VM path: ${resolved} → ${hostPath}`, ); - return { - valid: false, - error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}`, - }; - } - if (!fs.existsSync(resolved)) { - return { valid: false, error: `File not found: ${resolved}` }; + return { valid: true, resolvedPath: hostPath }; } - return { valid: true }; + + console.error( + `[pdf-server] Local file not in allowed list: ${resolved}\n Allowed dirs: ${[...allowedLocalDirs].join(", ")}`, + ); + return { + valid: false, + error: `Local file not in allowed list: ${resolved}\nAllowed directories: ${[...allowedLocalDirs].join(", ")}`, + }; } // Remote URL - require HTTPS @@ -437,9 +469,7 @@ async function refreshRoots(server: Server): Promise { allowedLocalFiles.add(resolved); } else if (s.isDirectory()) { allowedLocalDirs.add(resolved); - console.error( - `[pdf-server] Root directory allowed: ${resolved}`, - ); + console.error(`[pdf-server] Root directory allowed: ${resolved}`); } } catch { // stat failed — skip non-existent roots @@ -550,8 +580,16 @@ export function createServer(): McpServer { } try { - const normalized = isArxivUrl(url) ? normalizeArxivUrl(url) : url; - const { data, totalBytes } = await readPdfRange(url, offset, byteCount); + // Use resolved host path if VM path was remapped + const effectiveUrl = validation.resolvedPath ?? url; + const normalized = isArxivUrl(effectiveUrl) + ? normalizeArxivUrl(effectiveUrl) + : effectiveUrl; + const { data, totalBytes } = await readPdfRange( + effectiveUrl, + offset, + byteCount, + ); // Base64 encode for JSON transport const bytes = Buffer.from(data).toString("base64"); @@ -624,13 +662,18 @@ Accepts: }; } + // Use resolved host path if VM path was remapped + const effectiveUrl = validation.resolvedPath ?? normalized; + // Probe file size so the client can set up range transport without an extra fetch - const { totalBytes } = await readPdfRange(normalized, 0, 1); + const { totalBytes } = await readPdfRange(effectiveUrl, 0, 1); return { - content: [{ type: "text", text: `Displaying PDF: ${normalized}` }], + content: [ + { type: "text", text: `Displaying PDF: ${effectiveUrl}` }, + ], structuredContent: { - url: normalized, + url: effectiveUrl, initialPage: page, totalBytes, }, From a2625c1658d084dde093b439761ff6f9c42e9462 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 16:36:35 +0000 Subject: [PATCH 14/15] revert(pdf-server): remove resolveVmPath hack The VM path mismatch (VM-internal paths not rewritten in tool call arguments) should be fixed in the cowork VM proxy layer, not worked around in the MCP server. Removes: - resolveVmPath() function - resolvedPath field from validateUrl return type - effectiveUrl remapping in tool handlers - VM path resolution test --- examples/pdf-server/server.test.ts | 26 ------------ examples/pdf-server/server.ts | 67 ++++-------------------------- 2 files changed, 7 insertions(+), 86 deletions(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index 73fa95680..aeaa3a466 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -380,32 +380,6 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { } }); - it("should resolve VM-internal paths via directory basename matching", () => { - const fs = require("node:fs"); - const os = require("node:os"); - // Simulate: host has /tmp/xxx/uploads/file.txt - // VM sends /sessions/name/mnt/uploads/file.txt - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdf-vm-test-")); - const hostUploads = path.join(tmpDir, "uploads"); - fs.mkdirSync(hostUploads); - fs.writeFileSync(path.join(hostUploads, "report.pdf"), "fake-pdf"); - - try { - // Allow the host uploads directory - allowedLocalDirs.add(hostUploads); - - // Simulate VM-internal path that shares the "uploads" basename - const vmPath = "/sessions/gallant-tender-mayer/mnt/uploads/report.pdf"; - const result = validateUrl(vmPath); - expect(result.valid).toBe(true); - // The resolved path should point to the host file - expect(result.resolvedPath).toBe( - path.join(hostUploads, "report.pdf"), - ); - } finally { - fs.rmSync(tmpDir, { recursive: true }); - } - }); }); describe("isAncestorDir", () => { diff --git a/examples/pdf-server/server.ts b/examples/pdf-server/server.ts index ebaca0fc8..ccc400f0d 100644 --- a/examples/pdf-server/server.ts +++ b/examples/pdf-server/server.ts @@ -101,37 +101,6 @@ export function isAncestorDir(dir: string, filePath: string): boolean { return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel); } -/** - * Try to resolve a VM-internal path to a host path by matching the path suffix - * against allowed directories. - * - * When the MCP server runs on the host but receives unrewritten VM paths - * (e.g., /sessions/name/mnt/uploads/file.pdf), this finds the equivalent host - * path (e.g., /Users/.../uploads/file.pdf) by matching directory basenames. - * - * Returns the host path if found, or null. - */ -function resolveVmPath( - vmPath: string, - allowedDirs: ReadonlySet, -): string | null { - const parts = vmPath.split(path.sep); - for (const dir of allowedDirs) { - const dirBasename = path.basename(dir); - // Find the mount point in the VM path by matching the directory basename - const idx = parts.lastIndexOf(dirBasename); - if (idx >= 0) { - // Join allowed dir with the relative path after the mount point - const relativeParts = parts.slice(idx + 1); - const candidate = path.join(dir, ...relativeParts); - if (fs.existsSync(candidate)) { - return candidate; - } - } - } - return null; -} - /** * Check if `url` looks like an absolute local file path (not a URL scheme). * Handles Unix paths (/...), home-relative (~), and Windows drive letters (C:\...). @@ -145,7 +114,6 @@ function isLocalPath(url: string): boolean { export function validateUrl(url: string): { valid: boolean; error?: string; - resolvedPath?: string; } { if (isFileUrl(url) || isLocalPath(url)) { // fileUrlToPath already decodes percent-encoding; for bare paths, @@ -160,7 +128,7 @@ export function validateUrl(url: string): { if (!fs.existsSync(resolved)) { return { valid: false, error: `File not found: ${resolved}` }; } - return { valid: true, resolvedPath: resolved }; + return { valid: true }; } // Check directory match (MCP roots / CLI dirs). @@ -192,19 +160,7 @@ export function validateUrl(url: string): { if (!fs.existsSync(resolved)) { return { valid: false, error: `File not found: ${resolved}` }; } - return { valid: true, resolvedPath: resolved }; - } - - // VM path fallback: the path may be a VM-internal path - // (e.g., /sessions/.../mnt/uploads/file.pdf) that wasn't rewritten - // to the host path. Try to find the file in allowed dirs by matching - // the directory basename as a mount point. - const hostPath = resolveVmPath(resolved, allowedLocalDirs); - if (hostPath) { - console.error( - `[pdf-server] Resolved VM path: ${resolved} → ${hostPath}`, - ); - return { valid: true, resolvedPath: hostPath }; + return { valid: true }; } console.error( @@ -580,13 +536,9 @@ export function createServer(): McpServer { } try { - // Use resolved host path if VM path was remapped - const effectiveUrl = validation.resolvedPath ?? url; - const normalized = isArxivUrl(effectiveUrl) - ? normalizeArxivUrl(effectiveUrl) - : effectiveUrl; + const normalized = isArxivUrl(url) ? normalizeArxivUrl(url) : url; const { data, totalBytes } = await readPdfRange( - effectiveUrl, + normalized, offset, byteCount, ); @@ -662,18 +614,13 @@ Accepts: }; } - // Use resolved host path if VM path was remapped - const effectiveUrl = validation.resolvedPath ?? normalized; - // Probe file size so the client can set up range transport without an extra fetch - const { totalBytes } = await readPdfRange(effectiveUrl, 0, 1); + const { totalBytes } = await readPdfRange(normalized, 0, 1); return { - content: [ - { type: "text", text: `Displaying PDF: ${effectiveUrl}` }, - ], + content: [{ type: "text", text: `Displaying PDF: ${normalized}` }], structuredContent: { - url: effectiveUrl, + url: normalized, initialPage: page, totalBytes, }, From f93d87923645b9e7d3921070b6dd8026e1ed5785 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 24 Feb 2026 17:55:47 +0000 Subject: [PATCH 15/15] nit --- examples/pdf-server/server.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/pdf-server/server.test.ts b/examples/pdf-server/server.test.ts index aeaa3a466..dd5ac4474 100644 --- a/examples/pdf-server/server.test.ts +++ b/examples/pdf-server/server.test.ts @@ -379,7 +379,6 @@ describe("validateUrl with MCP roots (allowedLocalDirs)", () => { fs.rmSync(tmpDir, { recursive: true }); } }); - }); describe("isAncestorDir", () => {