Skip to content

Commit db2a4fc

Browse files
cliffhallclaude
andcommitted
fix(filesystem): report a missing parent directory, not a raw chdir error
A pinned write (write_file, append_file, move_file, copy_file) into a directory that does not exist failed with Node's chdir error, which named chdir and the server's own working directory. Entering the anchor now maps ENOENT/ENOTDIR to "Parent directory does not exist: <directory>", and any other failure to "<code>: cannot enter directory: <directory>", keeping the code on the error. Closes #5432 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: cliffhall <[email protected]>
1 parent 281b167 commit db2a4fc

6 files changed

Lines changed: 219 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@modelcontextprotocol/server-filesystem": patch
3+
---
4+
5+
A write into a directory that does not exist (`write_file`, `append_file`, `move_file`, `copy_file`) now fails with `Parent directory does not exist: <directory>` instead of a raw `chdir` error that showed the server's working directory. Any other failure to enter a directory names only that directory.

‎src/filesystem/README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ Files inside the allowed directories are also [resources](https://modelcontextpr
147147
- Inputs:
148148
- `path` (string): File location
149149
- `content` (string): File content
150+
- Does not create directories: when the file's directory does not exist, it
151+
fails with `Parent directory does not exist: <directory>`; create the
152+
directory first with `create_directory`. `append_file`, `move_file` and
153+
`copy_file` fail the same way for a missing destination directory
150154
- An existing file is overwritten in place, so it keeps its inode, creation
151155
time, hard links and permissions (`edit_file` writes the same way). The
152156
overwrite is not crash-atomic: a crash mid-write can leave the file partly

‎src/filesystem/__tests__/append-file.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ describe("append_file", () => {
214214
content: "x",
215215
});
216216
expect(result.isError).toBe(true);
217-
expect(textOf(result)).toMatch(/ENOENT/);
217+
expect(textOf(result)).toBe(
218+
`Parent directory does not exist: ${path.join(dir, "missing")}`,
219+
);
218220
expect(fsSync.existsSync(path.join(dir, "missing"))).toBe(false);
219221
});
220222

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// A write whose parent directory is missing reports the documented
2+
// "Parent directory does not exist: <path>" error (#5432), never the raw
3+
// chdir error of the pinned write, which named chdir (an implementation
4+
// detail) and the server's own working directory (wherever it was launched,
5+
// possibly outside the allowed directories). The protocol-level tests drive
6+
// each pinned write through a client; the unit tests below them cover the
7+
// error mapping's other branches in pinned-directory.ts directly.
8+
9+
import fs from "fs/promises";
10+
import fsSync from "fs";
11+
import path from "path";
12+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
13+
import type { Client } from "@modelcontextprotocol/client";
14+
import { createFileInDirectory } from "../pinned-directory.js";
15+
import {
16+
call,
17+
connect,
18+
makeTempDir,
19+
quietStderr,
20+
textOf,
21+
type Connected,
22+
} from "./helpers.js";
23+
24+
let dir: string;
25+
let cwd: string;
26+
let conn: Connected;
27+
let client: Client;
28+
29+
beforeEach(async () => {
30+
quietStderr();
31+
cwd = process.cwd();
32+
dir = await makeTempDir("mcp-fs-missing-parent-");
33+
conn = await connect([dir]);
34+
client = conn.client;
35+
});
36+
37+
afterEach(async () => {
38+
vi.restoreAllMocks();
39+
process.chdir(cwd);
40+
await conn.close();
41+
await fs.rm(dir, { recursive: true, force: true });
42+
});
43+
44+
/** The error text must not leak the server's working directory or chdir. */
45+
function expectNoLeak(text: string): void {
46+
expect(text).not.toContain(cwd);
47+
expect(text).not.toMatch(/chdir/);
48+
}
49+
50+
describe("a write into a missing directory (#5432)", () => {
51+
it("write_file reports the missing parent", async () => {
52+
const missing = path.join(dir, "nope");
53+
const result = await call(client, "write_file", {
54+
path: path.join(missing, "x.txt"),
55+
content: "z",
56+
});
57+
expect(result.isError).toBe(true);
58+
expect(textOf(result)).toBe(`Parent directory does not exist: ${missing}`);
59+
expect(fsSync.existsSync(missing)).toBe(false);
60+
});
61+
62+
it("write_file reports the missing parent several levels down", async () => {
63+
const missing = path.join(dir, "no", "such");
64+
const result = await call(client, "write_file", {
65+
path: path.join(missing, "file.txt"),
66+
content: "z",
67+
});
68+
expect(result.isError).toBe(true);
69+
expect(textOf(result)).toBe(`Parent directory does not exist: ${missing}`);
70+
});
71+
72+
it("append_file reports the missing parent", async () => {
73+
const missing = path.join(dir, "nope");
74+
const result = await call(client, "append_file", {
75+
path: path.join(missing, "x.txt"),
76+
content: "z",
77+
});
78+
expect(result.isError).toBe(true);
79+
expect(textOf(result)).toBe(`Parent directory does not exist: ${missing}`);
80+
expect(fsSync.existsSync(missing)).toBe(false);
81+
});
82+
83+
it("move_file reports the destination's missing parent", async () => {
84+
const source = path.join(dir, "a.txt");
85+
await fs.writeFile(source, "a");
86+
const missing = path.join(dir, "nope");
87+
const result = await call(client, "move_file", {
88+
source,
89+
destination: path.join(missing, "a.txt"),
90+
});
91+
expect(result.isError).toBe(true);
92+
expect(textOf(result)).toBe(`Parent directory does not exist: ${missing}`);
93+
expect(await fs.readFile(source, "utf-8")).toBe("a");
94+
});
95+
96+
it("copy_file reports the destination's missing parent", async () => {
97+
const source = path.join(dir, "a.txt");
98+
await fs.writeFile(source, "a");
99+
const missing = path.join(dir, "nope");
100+
const result = await call(client, "copy_file", {
101+
source,
102+
destination: path.join(missing, "a.txt"),
103+
});
104+
expect(result.isError).toBe(true);
105+
expect(textOf(result)).toBe(`Parent directory does not exist: ${missing}`);
106+
expect(fsSync.existsSync(missing)).toBe(false);
107+
});
108+
109+
it("edit_file on a file in a missing directory leaks nothing", async () => {
110+
const result = await call(client, "edit_file", {
111+
path: path.join(dir, "nope", "x.txt"),
112+
edits: [{ oldText: "a", newText: "b" }],
113+
});
114+
expect(result.isError).toBe(true);
115+
expectNoLeak(textOf(result));
116+
});
117+
118+
it("create_directory still creates missing parents, and leaks nothing on a file in the way", async () => {
119+
const created = await call(client, "create_directory", {
120+
path: path.join(dir, "a", "b"),
121+
});
122+
expect(created.isError).toBeFalsy();
123+
await fs.writeFile(path.join(dir, "file"), "");
124+
const result = await call(client, "create_directory", {
125+
path: path.join(dir, "file", "below"),
126+
});
127+
expect(result.isError).toBe(true);
128+
expectNoLeak(textOf(result));
129+
});
130+
});
131+
132+
describe("the pinned write's error for a directory it cannot enter", () => {
133+
it("reports a file in place of the parent as a missing parent", async () => {
134+
await fs.writeFile(path.join(dir, "file"), "");
135+
const parent = path.join(dir, "file");
136+
await expect(
137+
createFileInDirectory(path.join(parent, "x.txt"), "z"),
138+
).rejects.toThrow(
139+
expect.objectContaining({
140+
message: `Parent directory does not exist: ${parent}`,
141+
code: "ENOTDIR",
142+
}),
143+
);
144+
});
145+
146+
it("keeps any other failure's code and names only the directory", async () => {
147+
const realChdir = process.chdir.bind(process);
148+
vi.spyOn(process, "chdir").mockImplementation((directory) => {
149+
if (directory === dir) {
150+
throw Object.assign(
151+
new Error(`EACCES: permission denied, chdir '${cwd}' -> '${dir}'`),
152+
{ code: "EACCES" },
153+
);
154+
}
155+
realChdir(directory);
156+
});
157+
await expect(
158+
createFileInDirectory(path.join(dir, "x.txt"), "z"),
159+
).rejects.toThrow(
160+
expect.objectContaining({
161+
message: `EACCES: cannot enter directory: ${dir}`,
162+
code: "EACCES",
163+
}),
164+
);
165+
});
166+
167+
it("reports a failure with no code without leaking its message", async () => {
168+
const realChdir = process.chdir.bind(process);
169+
vi.spyOn(process, "chdir").mockImplementation((directory) => {
170+
if (directory === dir) {
171+
throw new Error(`chdir '${cwd}' -> '${dir}'`);
172+
}
173+
realChdir(directory);
174+
});
175+
await expect(
176+
createFileInDirectory(path.join(dir, "x.txt"), "z"),
177+
).rejects.toThrow(`Error: cannot enter directory: ${dir}`);
178+
});
179+
});

‎src/filesystem/__tests__/server-tools.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,13 +854,15 @@ describe("write_file", () => {
854854
expect(textOf(result)).toBe("EIO: i/o error");
855855
});
856856

857-
it("reports a missing parent directory as ENOENT", async () => {
857+
it("reports a missing parent directory (#5432)", async () => {
858858
const result = await call(client, "write_file", {
859859
path: path.join(dir, "no", "such", "file.txt"),
860860
content: "x",
861861
});
862862
expect(result.isError).toBe(true);
863-
expect(textOf(result)).toMatch(/^ENOENT: no such file or directory/);
863+
expect(textOf(result)).toBe(
864+
`Parent directory does not exist: ${path.join(dir, "no", "such")}`,
865+
);
864866
});
865867
});
866868

‎src/filesystem/pinned-directory.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,31 @@ function verifyWorkingDirectory(directory: string): void {
162162
}
163163
}
164164

165+
/**
166+
* The error for a directory the process could not enter (#5432). Node's chdir
167+
* error names the working directory it left, which is wherever the server was
168+
* launched from and none of the client's business, and names chdir, an
169+
* implementation detail of the anchor. A missing directory (ENOENT, or
170+
* ENOTDIR for a file in its place) is the parent of the entry being written,
171+
* so it gets validatePath's message for a missing parent; anything else keeps
172+
* its code and names only the directory. The code stays on the error, so a
173+
* caller that branches on it (createDirectoryInPlace) still can.
174+
*/
175+
function cannotEnterError(directory: string, error: unknown): Error {
176+
const code = (error as NodeJS.ErrnoException).code;
177+
const message =
178+
code === "ENOENT" || code === "ENOTDIR"
179+
? `Parent directory does not exist: ${directory}`
180+
: `${code ?? "Error"}: cannot enter directory: ${directory}`;
181+
return Object.assign(new Error(message), { code });
182+
}
183+
165184
function enterDirectory(directory: string): void {
166-
process.chdir(directory);
185+
try {
186+
process.chdir(directory);
187+
} catch (error) {
188+
throw cannotEnterError(directory, error);
189+
}
167190
verifyWorkingDirectory(directory);
168191
}
169192

0 commit comments

Comments
 (0)