From af8a01b598ba5d8037c574a7ef77f2a7bf98c444 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 28 Oct 2026 18:17:28 -0400 Subject: [PATCH 1/3] feat(filesystem): add an append_file tool append_file appends text to the end of a file, exactly as given (no newline is added), creating the file if it does not exist. It is registered right after write_file and takes the same symlink-safe path: exclusive creation for a new file, and for an existing one an O_APPEND handle opened inside the pinned directory and checked to be the file lstat saw, so nothing swapped in after validation is written through. The stdio tests no longer pin the tool count, so a new tool does not break them; server-tools.test.ts still pins the full list. Closes #1208 Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .changeset/filesystem-append-file.md | 5 + src/filesystem/README.md | 13 + src/filesystem/__tests__/append-file.test.ts | 268 ++++++++++++++++++ src/filesystem/__tests__/server-tools.test.ts | 10 +- src/filesystem/__tests__/stdio.test.ts | 14 +- src/filesystem/lib.ts | 31 ++ src/filesystem/pinned-directory.ts | 30 ++ src/filesystem/server.ts | 39 +++ 8 files changed, 407 insertions(+), 3 deletions(-) create mode 100644 .changeset/filesystem-append-file.md create mode 100644 src/filesystem/__tests__/append-file.test.ts diff --git a/.changeset/filesystem-append-file.md b/.changeset/filesystem-append-file.md new file mode 100644 index 0000000000..67de93264c --- /dev/null +++ b/.changeset/filesystem-append-file.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/server-filesystem": minor +--- + +Add an `append_file` tool that appends text to the end of a file, exactly as given, creating the file if it does not exist (#1208). diff --git a/src/filesystem/README.md b/src/filesystem/README.md index dbb89834a9..c1f87779c8 100644 --- a/src/filesystem/README.md +++ b/src/filesystem/README.md @@ -118,6 +118,18 @@ The server's directory access control follows this flow: `get_file_info` reports a file's `hardLinks` count; more than 1 means the file has other names +- **append_file** + - Append text to the end of a file, creating the file if it does not exist + - Inputs: + - `path` (string): File location + - `content` (string): Text to append + - The text is appended exactly as given: no newline is added before or + after it. To build a file line by line, end each `content` with `\n` + - The existing content is never changed. The file is appended to in place, + so it keeps its inode, creation time, hard links and permissions, and a + path that is not a regular file (a directory, say) is refused. The same + hard-link caveat as `write_file` applies + - **edit_file** - Make selective edits using advanced pattern matching and formatting - Features: @@ -261,6 +273,7 @@ The mapping for filesystem tools is: | `list_allowed_directories` | `true` | – | – | Pure read | | `create_directory` | `false` | `true` | `false` | Re‑creating the same dir is a no‑op | | `write_file` | `false` | `true` | `true` | Overwrites existing files | +| `append_file` | `false` | `false` | `false` | Adds to a file; repeating it appends again | | `edit_file` | `false` | `false` | `true` | Re‑applying edits can fail or double‑apply | | `move_file` | `false` | `false` | `true` | Deletes source file | diff --git a/src/filesystem/__tests__/append-file.test.ts b/src/filesystem/__tests__/append-file.test.ts new file mode 100644 index 0000000000..56b0e99b70 --- /dev/null +++ b/src/filesystem/__tests__/append-file.test.ts @@ -0,0 +1,268 @@ +// Protocol-level tests for the append_file tool (#1208), driven through a real +// Client over an in-memory transport. They pin the contract the README states: +// text is appended exactly as given (no newline added), a missing file is +// created, the existing content and the file's identity are kept, and the +// write takes the same symlink-safe path as write_file, so nothing swapped in +// after validation is written through. + +import fs from "fs/promises"; +import fsSync from "fs"; +import path from "path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Client } from "@modelcontextprotocol/client"; +import { + call, + connect, + makeTempDir, + quietStderr, + textOf, + type Connected, +} from "./helpers.js"; + +let dir: string; +let conn: Connected; +let client: Client; + +beforeEach(async () => { + quietStderr(); + dir = await makeTempDir("mcp-fs-append-"); + conn = await connect([dir]); + client = conn.client; +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await conn.close(); + await fs.rm(dir, { recursive: true, force: true }); +}); + +describe("append_file", () => { + it("is listed right after write_file, with truthful annotations", async () => { + const { tools } = await client.listTools(); + const names = tools.map((t) => t.name); + expect(names.indexOf("append_file")).toBe(names.indexOf("write_file") + 1); + const tool = tools.find((t) => t.name === "append_file"); + expect(tool?.title).toBe("Append File"); + expect(tool?.annotations).toEqual({ + readOnlyHint: false, + idempotentHint: false, + destructiveHint: false, + openWorldHint: false, + }); + }); + + it("appends to an existing file exactly as given, adding no newline", async () => { + const file = path.join(dir, "log.txt"); + await fs.writeFile(file, "first"); + const result = await call(client, "append_file", { + path: file, + content: " second", + }); + expect(result.isError).toBeFalsy(); + expect(textOf(result)).toBe(`Successfully appended to ${file}`); + expect(result.structuredContent).toEqual({ content: textOf(result) }); + expect(await fs.readFile(file, "utf-8")).toBe("first second"); + }); + + it("builds a file up line by line when each append ends in a newline", async () => { + const file = path.join(dir, "lines.txt"); + for (const line of ["one\n", "two\n", "three\n"]) { + const result = await call(client, "append_file", { + path: file, + content: line, + }); + expect(result.isError).toBeFalsy(); + } + expect(await fs.readFile(file, "utf-8")).toBe("one\ntwo\nthree\n"); + }); + + it("keeps CRLF and other content byte for byte", async () => { + const file = path.join(dir, "crlf.txt"); + await fs.writeFile(file, "a\r\n"); + await call(client, "append_file", { path: file, content: "b\r\nc" }); + expect(await fs.readFile(file, "utf-8")).toBe("a\r\nb\r\nc"); + }); + + it("creates a file that does not exist yet", async () => { + const file = path.join(dir, "new.txt"); + const result = await call(client, "append_file", { + path: file, + content: "hello", + }); + expect(result.isError).toBeFalsy(); + expect(await fs.readFile(file, "utf-8")).toBe("hello"); + }); + + it("leaves a file unchanged when the content is empty", async () => { + const file = path.join(dir, "same.txt"); + await fs.writeFile(file, "kept"); + const result = await call(client, "append_file", { + path: file, + content: "", + }); + expect(result.isError).toBeFalsy(); + expect(await fs.readFile(file, "utf-8")).toBe("kept"); + }); + + it("appends through the existing file, keeping its inode and hard links", async () => { + const file = path.join(dir, "linked.txt"); + const link = path.join(dir, "other-name.txt"); + await fs.writeFile(file, "base"); + await fs.link(file, link); + const before = await fs.stat(file, { bigint: true }); + await call(client, "append_file", { path: file, content: "+more" }); + const after = await fs.stat(file, { bigint: true }); + expect(after.ino).toBe(before.ino); + expect(after.nlink).toBe(2n); + expect(await fs.readFile(link, "utf-8")).toBe("base+more"); + }); + + it("refuses a path outside the allowed directories", async () => { + const outside = await makeTempDir("mcp-fs-append-outside-"); + try { + const target = path.join(outside, "x.txt"); + const result = await call(client, "append_file", { + path: target, + content: "nope", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toMatch(/^Access denied/); + expect(fsSync.existsSync(target)).toBe(false); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }); + + it("refuses a directory", async () => { + const sub = path.join(dir, "sub"); + await fs.mkdir(sub); + const result = await call(client, "append_file", { + path: sub, + content: "x", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toBe( + `Refusing to append: not a regular file: ${sub}`, + ); + }); + + it("fails, creating nothing, when the parent directory does not exist", async () => { + const result = await call(client, "append_file", { + path: path.join(dir, "missing", "x.txt"), + content: "x", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toMatch(/ENOENT/); + expect(fsSync.existsSync(path.join(dir, "missing"))).toBe(false); + }); + + it("refuses when the file at the path is not the one it opened", async () => { + const file = path.join(dir, "target.txt"); + const decoy = path.join(dir, "decoy.txt"); + await fs.writeFile(file, "original"); + await fs.writeFile(decoy, "decoy"); + // The lstat that names the file to append to describes another file, as + // if the target had been replaced between that lstat and the open. + const realLstat = fs.lstat.bind(fs); + vi.spyOn(fs, "lstat").mockImplementation(((p: string, opts?: unknown) => + realLstat(p === file ? decoy : p, opts as never)) as typeof fs.lstat); + const result = await call(client, "append_file", { + path: file, + content: "pwned", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toBe( + `Refusing to append: ${file} was replaced while it was being opened`, + ); + expect(await fs.readFile(file, "utf-8")).toBe("original"); + expect(await fs.readFile(decoy, "utf-8")).toBe("decoy"); + }); + + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "reports a file it cannot create", + async () => { + const locked = path.join(dir, "locked"); + await fs.mkdir(locked); + await fs.chmod(locked, 0o500); + try { + const result = await call(client, "append_file", { + path: path.join(locked, "x.txt"), + content: "x", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toMatch(/EACCES/); + } finally { + await fs.chmod(locked, 0o700); + } + }, + ); +}); + +// The TOCTOU window, as server-tools.test.ts stages it for write_file: a +// directory on the path is swapped for a symlink to a directory outside the +// allowed one right after validation resolved the path. append_file must not +// write through it, whether it would create the file or append to one. +describe.skipIf(process.platform === "win32")( + "append_file with a directory on the path swapped for a symlink after validation", + () => { + let outside: string; + let flip: string; + let flipReal: string; + + beforeEach(async () => { + outside = await makeTempDir("mcp-fs-append-outside-"); + flip = path.join(dir, "flip"); + flipReal = path.join(dir, "flip-real"); + await fs.mkdir(flip); + }); + + afterEach(async () => { + await fs.rm(outside, { recursive: true, force: true }); + }); + + /** Swap `flip` for a symlink to `outside` once validation realpath'd `target`. */ + const swapAfterRealpath = (target: string) => { + const realRealpath = fs.realpath.bind(fs); + let swapped = false; + vi.spyOn(fs, "realpath").mockImplementation(async (p, opts) => { + const resolved = await realRealpath(p, opts); + if (!swapped && p === target) { + swapped = true; + fsSync.renameSync(flip, flipReal); + fsSync.symlinkSync(outside, flip); + } + return resolved; + }); + }; + + it("does not create a file through it", async () => { + swapAfterRealpath(flip); + const result = await call(client, "append_file", { + path: path.join(flip, "created.txt"), + content: "pwned", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toMatch(/^Access denied/); + expect(await fs.readdir(outside)).toEqual([]); + }); + + it("does not append to a file through it", async () => { + const victim = path.join(flip, "victim.txt"); + await fs.writeFile(victim, "original"); + await fs.writeFile(path.join(outside, "victim.txt"), "SECRET"); + swapAfterRealpath(victim); + const result = await call(client, "append_file", { + path: victim, + content: "pwned", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toMatch(/^Access denied/); + expect(await fs.readFile(path.join(outside, "victim.txt"), "utf-8")).toBe( + "SECRET", + ); + expect( + await fs.readFile(path.join(flipReal, "victim.txt"), "utf-8"), + ).toBe("original"); + }); + }, +); diff --git a/src/filesystem/__tests__/server-tools.test.ts b/src/filesystem/__tests__/server-tools.test.ts index 20bded6aeb..269ace53e6 100644 --- a/src/filesystem/__tests__/server-tools.test.ts +++ b/src/filesystem/__tests__/server-tools.test.ts @@ -31,6 +31,7 @@ const TOOL_NAMES = [ "read_media_file", "read_multiple_files", "write_file", + "append_file", "edit_file", "create_directory", "list_directory", @@ -89,7 +90,7 @@ describe("server identity and tool list", () => { }); }); - it("lists the 14 tools in registration order", async () => { + it("lists the tools in registration order", async () => { const { tools } = await client.listTools(); expect(tools.map((t) => t.name)).toEqual(TOOL_NAMES); }); @@ -114,6 +115,13 @@ describe("server identity and tool list", () => { destructiveHint: true, openWorldHint: false, }, + append_file: { + title: "Append File", + readOnlyHint: false, + idempotentHint: false, + destructiveHint: false, + openWorldHint: false, + }, edit_file: { title: "Edit File", readOnlyHint: false, diff --git a/src/filesystem/__tests__/stdio.test.ts b/src/filesystem/__tests__/stdio.test.ts index 196fa30f59..eb467e3620 100644 --- a/src/filesystem/__tests__/stdio.test.ts +++ b/src/filesystem/__tests__/stdio.test.ts @@ -224,7 +224,13 @@ describe("the stdio transport", () => { session.send(7, "tools/list"); const response = await session.response(7); expect(response.error).toBeUndefined(); - expect(response.result?.tools).toHaveLength(14); + // Not a count: a test that pins the number breaks on every new tool; + // server-tools.test.ts pins the full list. + expect(response.result?.tools).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "list_allowed_directories" }), + ]), + ); }); // #4206: a malformed line is reported to the transport's onerror (unset, @@ -248,7 +254,11 @@ describe("the stdio transport", () => { message: "Method not found", }); session.send(3, "tools/list"); - expect((await session.response(3)).result?.tools).toHaveLength(14); + expect((await session.response(3)).result?.tools).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "list_allowed_directories" }), + ]), + ); }); }); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 775145e708..53e2d366ef 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -7,6 +7,7 @@ import { minimatch } from "minimatch"; import { normalizePathFormat, expandHome } from "./path-utils.js"; import { isPathWithinAllowedDirectories } from "./path-validation.js"; import { + appendFileInDirectory, createDirectoryInPlace, createFileInDirectory, moveFileAcrossDevices, @@ -615,6 +616,36 @@ async function overwriteInPlace( await overwriteFileInDirectory(filePath, content, expected); } +/** + * Append `content` to the file at `filePath`, exactly as given: no newline is + * added before or after it (#1208). A file that does not exist yet is created + * with `content`, the way write_file creates one. + * + * Security: the same path as writeFileContent. Exclusive creation never + * follows a symlink; an existing entry must be a regular file, and is appended + * to through a handle checked to be that very file (see appendFileInDirectory + * in pinned-directory.ts), so nothing swapped in after validation is written + * through. + */ +export async function appendFileContent( + filePath: string, + content: string, +): Promise { + try { + await createFileInDirectory(filePath, content); + return; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") { + throw error; + } + } + const expected = await fs.lstat(filePath, { bigint: true }); + if (!expected.isFile()) { + throw new Error(`Refusing to append: not a regular file: ${filePath}`); + } + await appendFileInDirectory(filePath, content, expected); +} + /** Create a directory and any missing parents (see pinned-directory.ts). */ export async function createDirectory(directoryPath: string): Promise { createDirectoryInPlace(directoryPath); diff --git a/src/filesystem/pinned-directory.ts b/src/filesystem/pinned-directory.ts index 8b2e63ad6d..7517d2a866 100644 --- a/src/filesystem/pinned-directory.ts +++ b/src/filesystem/pinned-directory.ts @@ -280,6 +280,36 @@ export async function overwriteFileInDirectory( } } +/** + * Append `content` to the end of the regular file at `filePath`, which must be + * the file `expected` (its BigInt lstat) describes, through a single + * descriptor (#1208). The same checks as overwriteFileInDirectory guard the + * open; O_APPEND makes every write land at the file's end as it is at that + * moment, so a concurrent append is never overwritten. + */ +export async function appendFileInDirectory( + filePath: string, + content: string, + expected: EntryIdentity, +): Promise { + // O_RDWR rather than O_WRONLY, for the FIFO reason given above. + const { fd, opened } = openInDirectory( + filePath, + fs.constants.O_RDWR | fs.constants.O_APPEND | noFollow(), + onWindows(), + ); + try { + if (!opened.isFile() || !isSameEntry(expected, opened)) { + throw new Error( + `Refusing to append: ${filePath} was replaced while it was being opened`, + ); + } + await writeAll(fd, content); + } finally { + await close(fd); + } +} + /** * Rename `sourcePath` (absolute, outside this fix's scope) to the final * component of `destinationPath`, relative to its anchored directory. diff --git a/src/filesystem/server.ts b/src/filesystem/server.ts index a5127f8dbf..0922040f12 100644 --- a/src/filesystem/server.ts +++ b/src/filesystem/server.ts @@ -19,6 +19,7 @@ import { getFileStats, readFileContent, writeFileContent, + appendFileContent, createDirectory, moveFile, searchFilesWithValidation, @@ -77,6 +78,15 @@ const WriteFileArgsSchema = z.object({ content: z.string(), }); +const AppendFileArgsSchema = z.object({ + path: z.string(), + content: z + .string() + .describe( + "Text to append, exactly as given. No newline is added: end it with \\n if the next append should start on a new line.", + ), +}); + const EditOperation = z.object({ oldText: z.string().describe("Text to search for - must match exactly"), newText: z.string().describe("Text to replace with"), @@ -459,6 +469,35 @@ export function createServer( }, ); + server.registerTool( + "append_file", + { + title: "Append File", + description: + "Append text to the end of a file, creating the file if it does not exist. " + + "The existing content is never changed or replaced. The text is appended exactly " + + "as given, with no newline added before or after it, so end it with a newline " + + "if the next append should start on a new line. Only works within allowed directories.", + inputSchema: jsonSchema2020(AppendFileArgsSchema.shape), + outputSchema: jsonSchema2020({ content: z.string() }), + annotations: { + readOnlyHint: false, + idempotentHint: false, + destructiveHint: false, + openWorldHint: false, + }, + }, + async (args: z.infer) => { + const validPath = await validatePath(args.path, allowedDirectories); + await appendFileContent(validPath, args.content); + const text = `Successfully appended to ${args.path}`; + return { + content: [{ type: "text" as const, text }], + structuredContent: { content: text }, + }; + }, + ); + server.registerTool( "edit_file", { From 2c2be3e4c3b74dad18477aaea1578944efd653cf Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 28 Oct 2026 19:36:38 -0400 Subject: [PATCH 2/3] fix(filesystem): open append_file's new-file write with O_APPEND append_file creates a missing file through the exclusive-create path, whose descriptor was not O_APPEND: another append reaching the new file before the creating write would be overwritten at offset 0. The create now takes an `append` flag, which only append_file sets. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- src/filesystem/__tests__/append-file.test.ts | 26 ++++++++++++++++++++ src/filesystem/lib.ts | 4 ++- src/filesystem/pinned-directory.ts | 8 +++++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/filesystem/__tests__/append-file.test.ts b/src/filesystem/__tests__/append-file.test.ts index 56b0e99b70..93c52c1239 100644 --- a/src/filesystem/__tests__/append-file.test.ts +++ b/src/filesystem/__tests__/append-file.test.ts @@ -93,6 +93,32 @@ describe("append_file", () => { expect(await fs.readFile(file, "utf-8")).toBe("hello"); }); + it("keeps an append that lands on a new file before the creating write", async () => { + const file = path.join(dir, "race.txt"); + // The creating call's write is held back until another caller's append + // has reached the new file; without O_APPEND the held write would land at + // offset 0 over it. + const realWriteFile = fsSync.writeFile; + let raced = false; + vi.spyOn(fsSync, "writeFile").mockImplementation((( + target: number, + ...rest: unknown[] + ) => { + if (!raced && typeof target === "number") { + raced = true; + fsSync.appendFileSync(file, "SECOND"); + } + return (realWriteFile as (...args: unknown[]) => void)(target, ...rest); + }) as typeof fsSync.writeFile); + const result = await call(client, "append_file", { + path: file, + content: "FIRST", + }); + expect(result.isError).toBeFalsy(); + expect(raced).toBe(true); + expect(await fs.readFile(file, "utf-8")).toBe("SECONDFIRST"); + }); + it("leaves a file unchanged when the content is empty", async () => { const file = path.join(dir, "same.txt"); await fs.writeFile(file, "kept"); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 53e2d366ef..982296509b 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -632,7 +632,9 @@ export async function appendFileContent( content: string, ): Promise { try { - await createFileInDirectory(filePath, content); + // O_APPEND here too: another append can reach the file between its + // creation and this write, and must not be overwritten. + await createFileInDirectory(filePath, content, true); return; } catch (error) { if ((error as NodeJS.ErrnoException).code !== "EEXIST") { diff --git a/src/filesystem/pinned-directory.ts b/src/filesystem/pinned-directory.ts index 7517d2a866..8237451684 100644 --- a/src/filesystem/pinned-directory.ts +++ b/src/filesystem/pinned-directory.ts @@ -229,10 +229,15 @@ function openInDirectory( }); } -/** Create `filePath`, which must not exist, and write `content` to it. */ +/** + * Create `filePath`, which must not exist, and write `content` to it. With + * `append`, the descriptor is opened O_APPEND, so an append that lands on the + * new file before this write does is kept rather than overwritten (#1208). + */ export async function createFileInDirectory( filePath: string, content: string, + append: boolean = false, ): Promise { // O_EXCL: an existing file or symlink fails with EEXIST, never followed. const { fd } = openInDirectory( @@ -240,6 +245,7 @@ export async function createFileInDirectory( fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | + (append ? fs.constants.O_APPEND : 0) | noFollow(), false, ); From 7768af5a0e39bd1b0f909b65c20e7a3fe6e3c0fc Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 28 Oct 2026 20:15:20 -0400 Subject: [PATCH 3/3] fix(filesystem): let append_file append to a write-only file append_file opened an existing file O_RDWR, so a write-only file (a 0222 log) failed with EACCES though appending never reads. It now opens O_WRONLY | O_APPEND | O_NONBLOCK: O_NONBLOCK keeps a FIFO swapped in after the lstat from blocking the open, which O_RDWR did before. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- src/filesystem/__tests__/append-file.test.ts | 36 ++++++++++++++++++++ src/filesystem/pinned-directory.ts | 10 ++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/filesystem/__tests__/append-file.test.ts b/src/filesystem/__tests__/append-file.test.ts index 93c52c1239..fc2a99d099 100644 --- a/src/filesystem/__tests__/append-file.test.ts +++ b/src/filesystem/__tests__/append-file.test.ts @@ -143,6 +143,42 @@ describe("append_file", () => { expect(await fs.readFile(link, "utf-8")).toBe("base+more"); }); + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "appends to a write-only file, which it never reads", + async () => { + const file = path.join(dir, "write-only.log"); + await fs.writeFile(file, "a"); + await fs.chmod(file, 0o222); + try { + const result = await call(client, "append_file", { + path: file, + content: "b", + }); + expect(result.isError, textOf(result)).toBeFalsy(); + } finally { + await fs.chmod(file, 0o644); + } + expect(await fs.readFile(file, "utf-8")).toBe("ab"); + }, + ); + + it.skipIf(process.platform === "win32")( + "refuses a FIFO without blocking", + async () => { + const fifo = path.join(dir, "pipe"); + const { execFileSync } = await import("child_process"); + execFileSync("mkfifo", [fifo]); + const result = await call(client, "append_file", { + path: fifo, + content: "x", + }); + expect(result.isError).toBe(true); + expect(textOf(result)).toBe( + `Refusing to append: not a regular file: ${fifo}`, + ); + }, + ); + it("refuses a path outside the allowed directories", async () => { const outside = await makeTempDir("mcp-fs-append-outside-"); try { diff --git a/src/filesystem/pinned-directory.ts b/src/filesystem/pinned-directory.ts index 8237451684..c9e7bd1d36 100644 --- a/src/filesystem/pinned-directory.ts +++ b/src/filesystem/pinned-directory.ts @@ -298,10 +298,16 @@ export async function appendFileInDirectory( content: string, expected: EntryIdentity, ): Promise { - // O_RDWR rather than O_WRONLY, for the FIFO reason given above. + // O_WRONLY, so a write-only file (a 0222 log) can be appended to: append + // never reads. O_NONBLOCK keeps a FIFO swapped in from blocking the open + // (it fails with ENXIO without a reader; a regular file ignores the flag; + // it is undefined on Windows, where the lstat identity checks stand in). const { fd, opened } = openInDirectory( filePath, - fs.constants.O_RDWR | fs.constants.O_APPEND | noFollow(), + fs.constants.O_WRONLY | + fs.constants.O_APPEND | + (fs.constants.O_NONBLOCK ?? 0) | + noFollow(), onWindows(), ); try {