ignore client roots by default in pdf-server (#510)
* security: ignore client roots by default in pdf-server When the pdf-server is started with --stdio, MCP clients may advertise roots that refer to directories on the *client's* file system. Because the server resolves those paths locally, accepting them by default would give the remote client access to arbitrary directories on the server's machine. This commit makes client roots opt-in via the --use-client-roots flag. Without the flag, the server logs a notice and skips roots setup entirely. The createServer() function now accepts a CreateServerOptions object with a `useClientRoots` boolean (defaults to false). https://claude.ai/code/session_014ohk5NMEPe8TBKpqp4ZRSw * Enable client roots by default for HTTP, keep off for stdio HTTP mode serves a local client so roots are safe. Stdio mode may have a remote client whose roots would resolve against the server's filesystem, so roots stay off unless --use-client-roots is passed. https://claude.ai/code/session_014ohk5NMEPe8TBKpqp4ZRSw * fix: correct transport-aware defaults for useClientRoots stdio = local client (e.g. Claude Desktop) → auto-enable roots HTTP = remote client → ignore roots unless --use-client-roots passed The previous commit had the logic inverted. https://claude.ai/code/session_014ohk5NMEPe8TBKpqp4ZRSw * refactor: inline useClientRoots per transport branch Remove the intermediate effectiveUseClientRoots variable — just pass `true` in the stdio branch and the flag value in the HTTP branch. https://claude.ai/code/session_014ohk5NMEPe8TBKpqp4ZRSw * docs: update README to match transport-aware client roots defaults Stdio always enables roots (client is local); HTTP ignores them by default. The previous README text had these reversed. https://claude.ai/code/session_014ohk5NMEPe8TBKpqp4ZRSw --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
87e30edb03
commit
30f79b9e3f
4 changed files with 103 additions and 17 deletions
|
|
@ -149,9 +149,27 @@ bun examples/pdf-server/main.ts ./local.pdf https://arxiv.org/pdf/2401.00001.pdf
|
|||
bun examples/pdf-server/main.ts --stdio ./papers/
|
||||
```
|
||||
|
||||
## Security: Client Roots
|
||||
|
||||
MCP clients may advertise **roots** — `file://` URIs pointing to directories on the client's file system. The server uses these to allow access to local files under those directories.
|
||||
|
||||
- **Stdio mode** (`--stdio`): Client roots are **always enabled** — the client is typically on the same machine (e.g. Claude Desktop), so the roots are safe.
|
||||
- **HTTP mode** (default): Client roots are **ignored** by default — the client may be remote, and its roots would be resolved against the server's filesystem. To opt in, pass `--use-client-roots`:
|
||||
|
||||
```bash
|
||||
# Trust that the HTTP client is local and its roots are safe
|
||||
bun examples/pdf-server/main.ts --use-client-roots
|
||||
```
|
||||
|
||||
When roots are ignored the server logs:
|
||||
|
||||
```
|
||||
[pdf-server] Client roots are ignored (default for remote transports). Pass --use-client-roots to allow the client to expose local directories.
|
||||
```
|
||||
|
||||
## Allowed Sources
|
||||
|
||||
- **Local files**: Must be passed as CLI arguments
|
||||
- **Local files**: Must be passed as CLI arguments (or via client roots when enabled)
|
||||
- **Remote URLs**: arxiv.org, biorxiv.org, medrxiv.org, chemrxiv.org, zenodo.org, osf.io, hal.science, ssrn.com, and more
|
||||
|
||||
## Tools
|
||||
|
|
|
|||
|
|
@ -89,14 +89,21 @@ export async function startStdioServer(
|
|||
await createServer().connect(new StdioServerTransport());
|
||||
}
|
||||
|
||||
function parseArgs(): { urls: string[]; stdio: boolean } {
|
||||
function parseArgs(): {
|
||||
urls: string[];
|
||||
stdio: boolean;
|
||||
useClientRoots: boolean;
|
||||
} {
|
||||
const args = process.argv.slice(2);
|
||||
const urls: string[] = [];
|
||||
let stdio = false;
|
||||
let useClientRoots = false;
|
||||
|
||||
for (const arg of args) {
|
||||
if (arg === "--stdio") {
|
||||
stdio = true;
|
||||
} else if (arg === "--use-client-roots") {
|
||||
useClientRoots = true;
|
||||
} else if (!arg.startsWith("-")) {
|
||||
// Convert local paths to file:// URLs, normalize arxiv URLs
|
||||
let url = arg;
|
||||
|
|
@ -113,11 +120,15 @@ function parseArgs(): { urls: string[]; stdio: boolean } {
|
|||
}
|
||||
}
|
||||
|
||||
return { urls: urls.length > 0 ? urls : [DEFAULT_PDF], stdio };
|
||||
return {
|
||||
urls: urls.length > 0 ? urls : [DEFAULT_PDF],
|
||||
stdio,
|
||||
useClientRoots,
|
||||
};
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const { urls, stdio } = parseArgs();
|
||||
const { urls, stdio, useClientRoots } = parseArgs();
|
||||
|
||||
// Register local files in whitelist
|
||||
for (const url of urls) {
|
||||
|
|
@ -141,9 +152,11 @@ async function main() {
|
|||
console.error(`[pdf-server] Ready (${urls.length} URL(s) configured)`);
|
||||
|
||||
if (stdio) {
|
||||
await startStdioServer(createServer);
|
||||
// stdio → client is local (e.g. Claude Desktop), roots are safe
|
||||
await startStdioServer(() => createServer({ useClientRoots: true }));
|
||||
} else {
|
||||
await startStreamableHTTPServer(createServer);
|
||||
// HTTP → client is remote, only honour roots with explicit opt-in
|
||||
await startStreamableHTTPServer(() => createServer({ useClientRoots }));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test";
|
|||
import path from "node:path";
|
||||
import {
|
||||
createPdfCache,
|
||||
createServer,
|
||||
validateUrl,
|
||||
isAncestorDir,
|
||||
allowedLocalFiles,
|
||||
|
|
@ -421,3 +422,27 @@ describe("isAncestorDir", () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createServer useClientRoots option", () => {
|
||||
it("should not set up roots handlers by default", () => {
|
||||
const server = createServer();
|
||||
// When useClientRoots is false (default), oninitialized should NOT
|
||||
// be overridden by our roots logic.
|
||||
expect(server.server.oninitialized).toBeUndefined();
|
||||
server.close();
|
||||
});
|
||||
|
||||
it("should not set up roots handlers when useClientRoots is false", () => {
|
||||
const server = createServer({ useClientRoots: false });
|
||||
expect(server.server.oninitialized).toBeUndefined();
|
||||
server.close();
|
||||
});
|
||||
|
||||
it("should set up roots handlers when useClientRoots is true", () => {
|
||||
const server = createServer({ useClientRoots: true });
|
||||
// When useClientRoots is true, oninitialized should be set to
|
||||
// the roots refresh handler.
|
||||
expect(server.server.oninitialized).toBeFunction();
|
||||
server.close();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -443,19 +443,49 @@ async function refreshRoots(server: Server): Promise<void> {
|
|||
// MCP Server Factory
|
||||
// =============================================================================
|
||||
|
||||
export function createServer(): McpServer {
|
||||
export interface CreateServerOptions {
|
||||
/**
|
||||
* Whether to honour MCP roots sent by the client.
|
||||
*
|
||||
* When a server is exposed over HTTP, the connecting client is
|
||||
* typically remote and may advertise `roots` that refer to
|
||||
* directories on the **client's** file system. Because the server
|
||||
* resolves those paths locally, accepting them by default would give
|
||||
* the remote client access to arbitrary directories on the
|
||||
* **server's** machine.
|
||||
*
|
||||
* For stdio the client is typically local (e.g. Claude Desktop on the
|
||||
* same machine), so roots are safe and enabled by default.
|
||||
*
|
||||
* Set this to `true` for HTTP only when you trust the client, or
|
||||
* pass the `--use-client-roots` CLI flag.
|
||||
*
|
||||
* @default false
|
||||
*/
|
||||
useClientRoots?: boolean;
|
||||
}
|
||||
|
||||
export function createServer(options: CreateServerOptions = {}): McpServer {
|
||||
const { useClientRoots = false } = options;
|
||||
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);
|
||||
},
|
||||
);
|
||||
if (useClientRoots) {
|
||||
// Fetch roots on initialization and subscribe to changes
|
||||
server.server.oninitialized = () => {
|
||||
refreshRoots(server.server);
|
||||
};
|
||||
server.server.setNotificationHandler(
|
||||
RootsListChangedNotificationSchema,
|
||||
async () => {
|
||||
await refreshRoots(server.server);
|
||||
},
|
||||
);
|
||||
} else {
|
||||
console.error(
|
||||
"[pdf-server] Client roots are ignored (default for remote transports). " +
|
||||
"Pass --use-client-roots to allow the client to expose local directories.",
|
||||
);
|
||||
}
|
||||
|
||||
// Create session-local cache (isolated per server instance)
|
||||
const { readPdfRange } = createPdfCache();
|
||||
|
|
|
|||
Loading…
Reference in a new issue