Skip to content

Commit dc72bea

Browse files
committed
feat(pdf-server): treat uploads-root dirs as read-only by default
Dir roots whose basename is 'uploads' (e.g. Claude Desktop's attachment drop folder) are now treated as read-only unless --writeable-uploads-root is passed. This prevents the save button from appearing for attached PDFs that the client doesn't expect to be overwritten. Also gates the _debug diagnostic block on --debug and adds save/restore of writeFlags.allowUploadsRoot in test beforeEach/afterEach.
1 parent 275fe59 commit dc72bea

2 files changed

Lines changed: 93 additions & 1 deletion

File tree

examples/pdf-server/server.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
stopFileWatch,
1717
cliLocalFiles,
1818
isWritablePath,
19+
writeFlags,
1920
CACHE_INACTIVITY_TIMEOUT_MS,
2021
CACHE_MAX_LIFETIME_MS,
2122
CACHE_MAX_PDF_SIZE_BYTES,
@@ -459,6 +460,7 @@ describe("isWritablePath", () => {
459460
let savedFiles: Set<string>;
460461
let savedDirs: Set<string>;
461462
let savedCli: Set<string>;
463+
let savedAllowUploadsRoot: boolean;
462464

463465
beforeEach(() => {
464466
savedFiles = new Set(allowedLocalFiles);
@@ -467,6 +469,8 @@ describe("isWritablePath", () => {
467469
allowedLocalFiles.clear();
468470
allowedLocalDirs.clear();
469471
cliLocalFiles.clear();
472+
savedAllowUploadsRoot = writeFlags.allowUploadsRoot;
473+
writeFlags.allowUploadsRoot = false;
470474
});
471475

472476
afterEach(() => {
@@ -476,6 +480,7 @@ describe("isWritablePath", () => {
476480
for (const x of savedFiles) allowedLocalFiles.add(x);
477481
for (const x of savedDirs) allowedLocalDirs.add(x);
478482
for (const x of savedCli) cliLocalFiles.add(x);
483+
writeFlags.allowUploadsRoot = savedAllowUploadsRoot;
479484
});
480485

481486
it("nothing is writable when no roots and no CLI files", () => {
@@ -526,6 +531,43 @@ describe("isWritablePath", () => {
526531
expect(isWritablePath("/home/user/other/file.pdf")).toBe(false);
527532
expect(isWritablePath("/home/user/docsevil/file.pdf")).toBe(false);
528533
});
534+
535+
it("dir root named 'uploads' is read-only by default", () => {
536+
// Claude Desktop mounts the conversation's attachment drop folder as a
537+
// directory root literally named 'uploads'. The attached PDF lives
538+
// directly under it.
539+
allowedLocalDirs.add("/var/folders/xy/T/claude/uploads");
540+
expect(isWritablePath("/var/folders/xy/T/claude/uploads/Form.pdf")).toBe(
541+
false,
542+
);
543+
// Deep nesting under the uploads root — still the same root, still no.
544+
expect(
545+
isWritablePath("/var/folders/xy/T/claude/uploads/sub/deep.pdf"),
546+
).toBe(false);
547+
});
548+
549+
it("uploads-root guard matches basename, not substring", () => {
550+
allowedLocalDirs.add("/home/user/my-uploads"); // contains 'uploads' but ≠
551+
allowedLocalDirs.add("/home/user/uploads-archive");
552+
expect(isWritablePath("/home/user/my-uploads/f.pdf")).toBe(true);
553+
expect(isWritablePath("/home/user/uploads-archive/f.pdf")).toBe(true);
554+
});
555+
556+
it("--writeable-uploads-root opts back in", () => {
557+
allowedLocalDirs.add("/var/folders/xy/T/claude/uploads");
558+
writeFlags.allowUploadsRoot = true;
559+
expect(isWritablePath("/var/folders/xy/T/claude/uploads/Form.pdf")).toBe(
560+
true,
561+
);
562+
});
563+
564+
it("CLI file under an uploads root is still writable", () => {
565+
// Explicit CLI intent beats the uploads-basename heuristic.
566+
allowedLocalDirs.add("/tmp/uploads");
567+
allowedLocalFiles.add("/tmp/uploads/explicit.pdf");
568+
cliLocalFiles.add("/tmp/uploads/explicit.pdf");
569+
expect(isWritablePath("/tmp/uploads/explicit.pdf")).toBe(true);
570+
});
529571
});
530572

531573
describe("file watching", () => {

examples/pdf-server/server.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,21 @@ export const allowedLocalDirs = new Set<string>();
9494
*/
9595
export const cliLocalFiles = new Set<string>();
9696

97+
/**
98+
* Write-permission flags. Object wrapper (not a bare `let`) so main.ts can
99+
* mutate via the exported binding without re-import gymnastics — same
100+
* pattern as the Sets above.
101+
*/
102+
export const writeFlags = {
103+
/**
104+
* Claude Desktop mounts its per-conversation drop folder as a directory
105+
* root whose basename is literally `uploads`. Files in there are one-shot
106+
* copies the client doesn't expect us to overwrite. Default: read-only.
107+
* `--writeable-uploads-root` flips this for local testing.
108+
*/
109+
allowUploadsRoot: false,
110+
};
111+
97112
/**
98113
* Saving is allowed iff:
99114
* (a) the file was passed as a CLI arg — the user explicitly named it
@@ -105,13 +120,24 @@ export const cliLocalFiles = new Set<string>();
105120
* treat that signal as authoritative even when the path happens
106121
* to fall inside a mounted directory.
107122
*
123+
* EXCEPTION to (b): a dir root whose basename is `uploads` is treated
124+
* as read-only unless `writeFlags.allowUploadsRoot` is set. This is how
125+
* Claude Desktop surfaces attached files — writing back to them
126+
* surprises the user (the attachment doesn't update).
127+
*
108128
* With no directory roots and no CLI files, nothing is writable.
109129
*/
110130
export function isWritablePath(resolved: string): boolean {
111131
if (cliLocalFiles.has(resolved)) return true;
112132
// MCP file root → always read-only, regardless of ancestry
113133
if (allowedLocalFiles.has(resolved)) return false;
114-
return [...allowedLocalDirs].some((dir) => isAncestorDir(dir, resolved));
134+
return [...allowedLocalDirs].some((dir) => {
135+
if (!isAncestorDir(dir, resolved)) return false;
136+
if (!writeFlags.allowUploadsRoot && path.basename(dir) === "uploads") {
137+
return false;
138+
}
139+
return true;
140+
});
115141
}
116142

117143
// Works both from source (server.ts) and compiled (dist/server.js)
@@ -1158,10 +1184,17 @@ export interface CreateServerOptions {
11581184
* @default false
11591185
*/
11601186
useClientRoots?: boolean;
1187+
1188+
/**
1189+
* Emit debug metadata to the viewer (currently: allowed roots shown
1190+
* in a floating bubble). Toggled by the `--debug` CLI flag.
1191+
*/
1192+
debug?: boolean;
11611193
}
11621194

11631195
export function createServer(options: CreateServerOptions = {}): McpServer {
11641196
const { enableInteract = false, useClientRoots = false } = options;
1197+
const debug = options.debug ?? false;
11651198
const disableInteract = !enableInteract;
11661199
const server = new McpServer({ name: "PDF Server", version: "2.0.0" });
11671200

@@ -1432,11 +1465,13 @@ Set \`elicit_form_inputs\` to true to prompt the user to fill form fields before
14321465
// Check writability (governs save button; see isWritablePath doc).
14331466
// Also requires OS-level W_OK so we don't lie on read-only mounts.
14341467
let writable = false;
1468+
let debugResolved: string | undefined; // only used when --debug
14351469
if (isFileUrl(normalized) || isLocalPath(normalized)) {
14361470
const localPath = isFileUrl(normalized)
14371471
? fileUrlToPath(normalized)
14381472
: decodeURIComponent(normalized);
14391473
const resolved = path.resolve(localPath);
1474+
debugResolved = resolved;
14401475
if (isWritablePath(resolved)) {
14411476
try {
14421477
await fs.promises.access(resolved, fs.constants.W_OK);
@@ -1595,6 +1630,21 @@ Set \`elicit_form_inputs\` to true to prompt the user to fill form fields before
15951630
viewUUID: uuid,
15961631
interactEnabled: !disableInteract,
15971632
writable,
1633+
// Debug: viewer renders this in a floating bubble (--debug flag).
1634+
...(debug
1635+
? {
1636+
_debug: {
1637+
resolved: debugResolved,
1638+
writable,
1639+
isWritablePath: debugResolved
1640+
? isWritablePath(debugResolved)
1641+
: undefined,
1642+
cliLocalFiles: [...cliLocalFiles],
1643+
allowedLocalFiles: [...allowedLocalFiles],
1644+
allowedLocalDirs: [...allowedLocalDirs],
1645+
},
1646+
}
1647+
: {}),
15981648
},
15991649
};
16001650
},

0 commit comments

Comments
 (0)