pdf-server: file roots from MCP client are read-only

allowedLocalFiles conflated two sources: CLI args (user explicitly
named the file when starting the server — overwriting is intentional)
and MCP file roots (client-uploaded copies in ad-hoc hidden folders
that the client doesn't expect to change).

New cliLocalFiles set tracks only the CLI-sourced files. Writable now
requires: file is in cliLocalFiles OR strictly under a directory root.
Directory roots are mounted folders where saving is expected.

save_pdf enforces the same scope server-side — the viewer hides the
button based on _meta.writable, but we must not trust the client.

Author: ochafik

examples/pdf-server/main.ts

@@ -20,6 +20,7 @@ import {
   pathToFileUrl,
   fileUrlToPath,
   allowedLocalFiles,
+  cliLocalFiles,
   DEFAULT_PDF,
   allowedLocalDirs,
 } from "./server.js";
@@ -138,6 +139,7 @@ async function main() {
         const s = fs.statSync(filePath);
         if (s.isFile()) {
           allowedLocalFiles.add(filePath);
+          cliLocalFiles.add(filePath);
           console.error(`[pdf-server] Registered local file: ${filePath}`);
         } else if (s.isDirectory()) {
           allowedLocalDirs.add(filePath);

examples/pdf-server/server.test.ts

@@ -14,6 +14,7 @@ import {
   pathToFileUrl,
   startFileWatch,
   stopFileWatch,
+  cliLocalFiles,
   CACHE_INACTIVITY_TIMEOUT_MS,
   CACHE_MAX_LIFETIME_MS,
   CACHE_MAX_PDF_SIZE_BYTES,
@@ -484,11 +485,13 @@ describe("file watching", () => {
     tmpFile = path.join(tmpDir, "test.pdf");
     fs.writeFileSync(tmpFile, Buffer.from("%PDF-1.4\n%test\n"));
     allowedLocalFiles.add(tmpFile);
+    cliLocalFiles.add(tmpFile); // save_pdf test needs write scope
   });
 
   afterEach(() => {
     stopFileWatch(uuid);
     allowedLocalFiles.delete(tmpFile);
+    cliLocalFiles.delete(tmpFile);
     fs.rmSync(tmpDir, { recursive: true, force: true });
   });
 
@@ -580,6 +583,59 @@ describe("file watching", () => {
     await server.close();
   });
 
+  it("save_pdf refuses file roots from MCP client (not CLI)", async () => {
+    // Simulate: file is readable (in allowedLocalFiles via refreshRoots)
+    // but NOT in cliLocalFiles — it came from the client, not a CLI arg.
+    cliLocalFiles.delete(tmpFile);
+
+    const server = createServer({ enableInteract: true });
+    const client = new Client({ name: "t", version: "1" });
+    const [ct, st] = InMemoryTransport.createLinkedPair();
+    await Promise.all([server.connect(st), client.connect(ct)]);
+
+    const original = fs.readFileSync(tmpFile);
+    const r = await client.callTool({
+      name: "save_pdf",
+      arguments: {
+        url: tmpFile,
+        data: Buffer.from("%PDF-1.4\nshould-not-write").toString("base64"),
+      },
+    });
+    expect(r.isError).toBe(true);
+    const text = (r.content as { text: string }[])[0].text;
+    expect(text).toContain("read-only");
+    // Verify the file was NOT modified
+    expect(fs.readFileSync(tmpFile)).toEqual(original);
+
+    await client.close();
+    await server.close();
+  });
+
+  it("save_pdf allows files under a directory root", async () => {
+    // File NOT in cliLocalFiles but under a mounted directory root
+    cliLocalFiles.delete(tmpFile);
+    allowedLocalDirs.add(tmpDir);
+
+    const server = createServer({ enableInteract: true });
+    const client = new Client({ name: "t", version: "1" });
+    const [ct, st] = InMemoryTransport.createLinkedPair();
+    await Promise.all([server.connect(st), client.connect(ct)]);
+
+    const r = await client.callTool({
+      name: "save_pdf",
+      arguments: {
+        url: tmpFile,
+        data: Buffer.from("%PDF-1.4\nvia-dir-root").toString("base64"),
+      },
+    });
+    expect(r.isError).toBeFalsy();
+    expect(fs.readFileSync(tmpFile, "utf8")).toBe("%PDF-1.4\nvia-dir-root");
+
+    allowedLocalDirs.delete(tmpDir);
+    await client.close();
+    await server.close();
+  });
+
   // fs.watch rename semantics differ between kqueue (macOS) and inotify
   // (Linux) — on Linux, the watcher on the replaced inode may not receive
   // further events, and the re-attach race is inherently flaky in CI.

examples/pdf-server/server.ts

@@ -80,12 +80,20 @@ export const CACHE_MAX_LIFETIME_MS = 60_000; // 60 seconds
 /** Max size for cached PDFs (defensive limit to prevent memory exhaustion) */
 export const CACHE_MAX_PDF_SIZE_BYTES = 50 * 1024 * 1024; // 50MB
 
-/** Allowed local file paths (populated from CLI args) */
+/** Allowed local file paths (CLI args + file roots — read access). */
 export const allowedLocalFiles = new Set<string>();
 
-/** Allowed local directories (populated from MCP roots) */
+/** Allowed local directories (CLI args + directory roots — read access). */
 export const allowedLocalDirs = new Set<string>();
 
+/**
+ * Subset of allowedLocalFiles that came from CLI args (not MCP roots).
+ * Only these individual files are writable. File roots from the client
+ * are uploaded copies in ad-hoc hidden folders — treat as read-only.
+ * Directory roots are mounted folders; files UNDER them are writable.
+ */
+export const cliLocalFiles = new Set<string>();
+
 // Works both from source (server.ts) and compiled (dist/server.js)
 const DIST_DIR = import.meta.filename.endsWith(".ts")
   ? path.join(import.meta.dirname, "dist")
@@ -1350,21 +1358,29 @@ Set \`elicit_form_inputs\` to true to prompt the user to fill form fields before
       const uuid = randomUUID();
 
       // Check writability for local files (governs save button visibility).
-      // Writable only if: (a) the file is explicitly in allowedLocalFiles
-      // (passed as a CLI arg, so the user clearly opted in), OR the file
-      // is STRICTLY UNDER an allowed directory root (isAncestorDir already
-      // excludes rel === "", so a root itself doesn't count); AND
-      // (b) the process has OS write permission (fs.access W_OK).
+      //
+      // Writable only if:
+      //   (a) the file was passed as a CLI arg — the user explicitly named
+      //       it when starting the server, so overwriting is clearly
+      //       intentional; OR
+      //   (b) the file is STRICTLY UNDER a directory root (isAncestorDir
+      //       excludes rel === "", so the root itself doesn't count).
+      //       Directory roots are mounted folders where saving makes sense.
+      //   AND the process has OS write permission.
+      //
+      // NOT writable: file roots from the MCP client. Those are typically
+      // uploaded copies in ad-hoc hidden folders (e.g. Claude Desktop's
+      // uploads directory) — the client doesn't expect them to change.
       let writable = false;
       if (isFileUrl(normalized) || isLocalPath(normalized)) {
         const localPath = isFileUrl(normalized)
           ? fileUrlToPath(normalized)
           : decodeURIComponent(normalized);
         const resolved = path.resolve(localPath);
-        const inAllowedScope =
-          allowedLocalFiles.has(resolved) ||
+        const inWriteScope =
+          cliLocalFiles.has(resolved) ||
           [...allowedLocalDirs].some((dir) => isAncestorDir(dir, resolved));
-        if (inAllowedScope) {
+        if (inWriteScope) {
           try {
             await fs.promises.access(resolved, fs.constants.W_OK);
             writable = true;
@@ -2336,9 +2352,30 @@ Example — add a signature image and a stamp, then screenshot to verify:
           isError: true,
         };
       }
+      const resolved = path.resolve(filePath);
+      // Enforce the same write scope the display_pdf writable flag uses.
+      // The viewer hides the save button for non-writable files, but we
+      // must not trust the client: a direct save_pdf call should also refuse.
+      const inWriteScope =
+        cliLocalFiles.has(resolved) ||
+        [...allowedLocalDirs].some((dir) => isAncestorDir(dir, resolved));
+      if (!inWriteScope) {
+        return {
+          content: [
+            {
+              type: "text",
+              text:
+                "Save refused: file is not under a mounted directory root " +
+                "and was not passed as a CLI argument. MCP file roots are " +
+                "read-only (typically uploaded copies the client doesn't " +
+                "expect to change).",
+            },
+          ],
+          isError: true,
+        };
+      }
       try {
         const bytes = Buffer.from(data, "base64");
-        const resolved = path.resolve(filePath);
         await fs.promises.writeFile(resolved, bytes);
         const { mtimeMs } = await fs.promises.stat(resolved);
         // Don't suppress file_changed here — the saving viewer will recognise