feat(pdf-server): use MCP roots to auto-allow local directories (#479)
When the client advertises roots capability, the server queries roots/list on initialization and subscribes to roots/list_changed notifications. Any file:// roots pointing to existing directories are added to an allowedLocalDirs set, enabling access to all PDFs under those directories without needing to enumerate them via CLI args. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: revert unrelated changes from bad rebase Restores files that were accidentally modified alongside the MCP roots feature: README, pdf-server UI (search feature), threejs example, SDK guards (app.ts, app-bridge.ts), typedoc theme, media assets, and e2e snapshots.
This commit is contained in:
parent
37e127034f
commit
96c0af577d
2 changed files with 150 additions and 10 deletions
|
|
@ -1,6 +1,11 @@
|
||||||
import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test";
|
import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test";
|
||||||
|
import path from "node:path";
|
||||||
import {
|
import {
|
||||||
createPdfCache,
|
createPdfCache,
|
||||||
|
validateUrl,
|
||||||
|
allowedLocalFiles,
|
||||||
|
allowedLocalDirs,
|
||||||
|
pathToFileUrl,
|
||||||
CACHE_INACTIVITY_TIMEOUT_MS,
|
CACHE_INACTIVITY_TIMEOUT_MS,
|
||||||
CACHE_MAX_LIFETIME_MS,
|
CACHE_MAX_LIFETIME_MS,
|
||||||
CACHE_MAX_PDF_SIZE_BYTES,
|
CACHE_MAX_PDF_SIZE_BYTES,
|
||||||
|
|
@ -178,3 +183,65 @@ describe("PDF Cache with Timeouts", () => {
|
||||||
// The timeout behavior is straightforward and can be verified
|
// The timeout behavior is straightforward and can be verified
|
||||||
// through manual testing or E2E tests.
|
// through manual testing or E2E tests.
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("validateUrl with MCP roots (allowedLocalDirs)", () => {
|
||||||
|
const savedFiles = new Set(allowedLocalFiles);
|
||||||
|
const savedDirs = new Set(allowedLocalDirs);
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
allowedLocalFiles.clear();
|
||||||
|
allowedLocalDirs.clear();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
allowedLocalFiles.clear();
|
||||||
|
allowedLocalDirs.clear();
|
||||||
|
for (const f of savedFiles) allowedLocalFiles.add(f);
|
||||||
|
for (const d of savedDirs) allowedLocalDirs.add(d);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow a file under an allowed directory", () => {
|
||||||
|
// Use a real existing directory+file for the existsSync check
|
||||||
|
const dir = path.resolve(import.meta.dirname);
|
||||||
|
allowedLocalDirs.add(dir);
|
||||||
|
|
||||||
|
const filePath = path.join(dir, "server.ts");
|
||||||
|
const result = validateUrl(pathToFileUrl(filePath));
|
||||||
|
expect(result.valid).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should reject a file outside allowed directories", () => {
|
||||||
|
allowedLocalDirs.add("/some/allowed/dir");
|
||||||
|
|
||||||
|
const result = validateUrl("file:///other/dir/test.pdf");
|
||||||
|
expect(result.valid).toBe(false);
|
||||||
|
expect(result.error).toContain("not in allowed list");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should prevent prefix-based directory traversal", () => {
|
||||||
|
// /tmp/safe should NOT allow /tmp/safevil/file.pdf
|
||||||
|
allowedLocalDirs.add("/tmp/safe");
|
||||||
|
|
||||||
|
const result = validateUrl("file:///tmp/safevil/file.pdf");
|
||||||
|
expect(result.valid).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should still allow exact file matches from allowedLocalFiles", () => {
|
||||||
|
const filePath = path.resolve(import.meta.dirname, "server.ts");
|
||||||
|
allowedLocalFiles.add(filePath);
|
||||||
|
|
||||||
|
const result = validateUrl(pathToFileUrl(filePath));
|
||||||
|
expect(result.valid).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should reject non-existent file even if under allowed dir", () => {
|
||||||
|
const dir = path.resolve(import.meta.dirname);
|
||||||
|
allowedLocalDirs.add(dir);
|
||||||
|
|
||||||
|
const result = validateUrl(
|
||||||
|
pathToFileUrl(path.join(dir, "nonexistent-file.pdf")),
|
||||||
|
);
|
||||||
|
expect(result.valid).toBe(false);
|
||||||
|
expect(result.error).toContain("File not found");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -14,14 +14,16 @@ import { randomUUID } from "crypto";
|
||||||
import fs from "node:fs";
|
import fs from "node:fs";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
|
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
|
||||||
|
import type { Server } from "@modelcontextprotocol/sdk/server/index.js";
|
||||||
import {
|
import {
|
||||||
registerAppResource,
|
registerAppResource,
|
||||||
registerAppTool,
|
registerAppTool,
|
||||||
RESOURCE_MIME_TYPE,
|
RESOURCE_MIME_TYPE,
|
||||||
} from "@modelcontextprotocol/ext-apps/server";
|
} from "@modelcontextprotocol/ext-apps/server";
|
||||||
import type {
|
import {
|
||||||
CallToolResult,
|
RootsListChangedNotificationSchema,
|
||||||
ReadResourceResult,
|
type CallToolResult,
|
||||||
|
type ReadResourceResult,
|
||||||
} from "@modelcontextprotocol/sdk/types.js";
|
} from "@modelcontextprotocol/sdk/types.js";
|
||||||
import { z } from "zod";
|
import { z } from "zod";
|
||||||
|
|
||||||
|
|
@ -65,6 +67,9 @@ export const allowedRemoteOrigins = new Set([
|
||||||
/** Allowed local file paths (populated from CLI args) */
|
/** Allowed local file paths (populated from CLI args) */
|
||||||
export const allowedLocalFiles = new Set<string>();
|
export const allowedLocalFiles = new Set<string>();
|
||||||
|
|
||||||
|
/** Allowed local directories (populated from MCP roots) */
|
||||||
|
export const allowedLocalDirs = new Set<string>();
|
||||||
|
|
||||||
// Works both from source (server.ts) and compiled (dist/server.js)
|
// Works both from source (server.ts) and compiled (dist/server.js)
|
||||||
const DIST_DIR = import.meta.filename.endsWith(".ts")
|
const DIST_DIR = import.meta.filename.endsWith(".ts")
|
||||||
? path.join(import.meta.dirname, "dist")
|
? path.join(import.meta.dirname, "dist")
|
||||||
|
|
@ -107,7 +112,17 @@ export function pathToFileUrl(filePath: string): string {
|
||||||
export function validateUrl(url: string): { valid: boolean; error?: string } {
|
export function validateUrl(url: string): { valid: boolean; error?: string } {
|
||||||
if (isFileUrl(url)) {
|
if (isFileUrl(url)) {
|
||||||
const filePath = fileUrlToPath(url);
|
const filePath = fileUrlToPath(url);
|
||||||
if (!allowedLocalFiles.has(filePath)) {
|
const resolved = path.resolve(filePath);
|
||||||
|
|
||||||
|
// Check exact match (CLI args)
|
||||||
|
const exactMatch = allowedLocalFiles.has(filePath);
|
||||||
|
|
||||||
|
// Check directory match (MCP roots)
|
||||||
|
const dirMatch = [...allowedLocalDirs].some(
|
||||||
|
(dir) => resolved === dir || resolved.startsWith(dir + path.sep),
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!exactMatch && !dirMatch) {
|
||||||
return {
|
return {
|
||||||
valid: false,
|
valid: false,
|
||||||
error: `Local file not in allowed list: ${filePath}`,
|
error: `Local file not in allowed list: ${filePath}`,
|
||||||
|
|
@ -342,6 +357,41 @@ export function createPdfCache(): PdfCache {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// =============================================================================
|
||||||
|
// MCP Roots
|
||||||
|
// =============================================================================
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Query the client for roots and update allowedLocalDirs with any file:// roots
|
||||||
|
* that point to existing directories.
|
||||||
|
*/
|
||||||
|
async function refreshRoots(server: Server): Promise<void> {
|
||||||
|
if (!server.getClientCapabilities()?.roots) return;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const { roots } = await server.listRoots();
|
||||||
|
allowedLocalDirs.clear();
|
||||||
|
for (const root of roots) {
|
||||||
|
if (root.uri.startsWith("file://")) {
|
||||||
|
const dir = fileUrlToPath(root.uri);
|
||||||
|
const resolved = path.resolve(dir);
|
||||||
|
try {
|
||||||
|
if (fs.statSync(resolved).isDirectory()) {
|
||||||
|
allowedLocalDirs.add(resolved);
|
||||||
|
console.error(`[pdf-server] Root directory allowed: ${resolved}`);
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// stat failed — skip non-existent roots
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (err) {
|
||||||
|
console.error(
|
||||||
|
`[pdf-server] Failed to list roots: ${err instanceof Error ? err.message : err}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
// MCP Server Factory
|
// MCP Server Factory
|
||||||
// =============================================================================
|
// =============================================================================
|
||||||
|
|
@ -349,6 +399,17 @@ export function createPdfCache(): PdfCache {
|
||||||
export function createServer(): McpServer {
|
export function createServer(): McpServer {
|
||||||
const server = new McpServer({ name: "PDF Server", version: "2.0.0" });
|
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);
|
||||||
|
};
|
||||||
|
server.server.setNotificationHandler(
|
||||||
|
RootsListChangedNotificationSchema,
|
||||||
|
async () => {
|
||||||
|
await refreshRoots(server.server);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
// Create session-local cache (isolated per server instance)
|
// Create session-local cache (isolated per server instance)
|
||||||
const { readPdfRange } = createPdfCache();
|
const { readPdfRange } = createPdfCache();
|
||||||
|
|
||||||
|
|
@ -365,16 +426,27 @@ export function createServer(): McpServer {
|
||||||
pdfs.push({ url: pathToFileUrl(filePath), type: "local" });
|
pdfs.push({ url: pathToFileUrl(filePath), type: "local" });
|
||||||
}
|
}
|
||||||
|
|
||||||
// Note: Remote URLs from allowed origins can be loaded dynamically
|
// Build text
|
||||||
const text =
|
const parts: string[] = [];
|
||||||
pdfs.length > 0
|
if (pdfs.length > 0) {
|
||||||
? `Available PDFs:\n${pdfs.map((p) => `- ${p.url} (${p.type})`).join("\n")}\n\nRemote PDFs from ${[...allowedRemoteOrigins].join(", ")} can also be loaded dynamically.`
|
parts.push(
|
||||||
: `No local PDFs configured. Remote PDFs from ${[...allowedRemoteOrigins].join(", ")} can be loaded dynamically.`;
|
`Available PDFs:\n${pdfs.map((p) => `- ${p.url} (${p.type})`).join("\n")}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (allowedLocalDirs.size > 0) {
|
||||||
|
parts.push(
|
||||||
|
`Allowed local directories (from client roots):\n${[...allowedLocalDirs].map((d) => `- ${d}`).join("\n")}\nAny PDF file under these directories can be displayed.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
parts.push(
|
||||||
|
`Remote PDFs from ${[...allowedRemoteOrigins].join(", ")} can also be loaded dynamically.`,
|
||||||
|
);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
content: [{ type: "text", text }],
|
content: [{ type: "text", text: parts.join("\n\n") }],
|
||||||
structuredContent: {
|
structuredContent: {
|
||||||
localFiles: pdfs.filter((p) => p.type === "local").map((p) => p.url),
|
localFiles: pdfs.filter((p) => p.type === "local").map((p) => p.url),
|
||||||
|
allowedDirectories: [...allowedLocalDirs],
|
||||||
allowedOrigins: [...allowedRemoteOrigins],
|
allowedOrigins: [...allowedRemoteOrigins],
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
@ -470,6 +542,7 @@ export function createServer(): McpServer {
|
||||||
|
|
||||||
Accepts:
|
Accepts:
|
||||||
- Local files explicitly added to the server (use list_pdfs to see available files)
|
- Local files explicitly added to the server (use list_pdfs to see available files)
|
||||||
|
- Local files under directories provided by the client as MCP roots
|
||||||
- Remote PDFs from: ${allowedDomains}`,
|
- Remote PDFs from: ${allowedDomains}`,
|
||||||
inputSchema: {
|
inputSchema: {
|
||||||
url: z.string().default(DEFAULT_PDF).describe("PDF URL"),
|
url: z.string().default(DEFAULT_PDF).describe("PDF URL"),
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue