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..fc2a99d099 --- /dev/null +++ b/src/filesystem/__tests__/append-file.test.ts @@ -0,0 +1,330 @@ +// 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("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"); + 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.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 { + 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..982296509b 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,38 @@ 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 { + // 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") { + 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..c9e7bd1d36 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, ); @@ -280,6 +286,42 @@ 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_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_WRONLY | + fs.constants.O_APPEND | + (fs.constants.O_NONBLOCK ?? 0) | + 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", {