modelcontextprotocol / modelcontextprotocol/servers

server-memory: saveGraph rewrites memory.jsonl via temp-file + rename, dropping an existing file's permission bits (0600 -> 0644) and silently overwriting read-only files

Open
#4,827 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
90.5k
Forks
11.7k
Avg merge
2d 2h
Merged PRs (30d)
5

Description

server-memory: saveGraph rewrites memory.jsonl through a temp file + rename, so an existing file's permission bits are silently dropped (0600 -> 0644) and read-only files are silently overwritten

Environment
  • Package: @modelcontextprotocol/server-memory (src/memory), version 0.6.3
  • Repo: modelcontextprotocol/servers @ d73f99efbfd40c3aa1b61e88728b3d49fb52608f (main)
  • Node: v22.23.2
  • OS: macOS 26.6.2 (Darwin 25.6.0), process umask 022
  • The rename-based write was introduced by 642a9112 "fix(memory): write the knowledge graph atomically (#4642)".
Minimal reproduction
// repro_permissions.test.ts
import { describe, it, expect } from 'vitest';
import { promises as fs } from 'fs';
import os from 'os';
import path from 'path';
import { KnowledgeGraphManager } from './src/memory/index.ts';

describe('memory file permissions', () => {
  it('keeps the mode of an existing memory.jsonl across a mutation', async () => {
    const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'mem-mode-'));
    const file = path.join(dir, 'memory.jsonl');
    const manager = new KnowledgeGraphManager(file);

    await manager.createEntities([{ name: 'Alice', entityType: 'person', observations: [] }]);
    await fs.chmod(file, 0o600); // the operator hardened their memory file
    const before = ((await fs.stat(file)).mode & 0o777).toString(8);

    await manager.createEntities([{ name: 'Bob', entityType: 'person', observations: [] }]);
    const after = ((await fs.stat(file)).mode & 0o777).toString(8);

    console.log(`REPRO mode before=${before} after=${after} umask=${process.umask().toString(8)}`);
    expect(after, 'permission bits after a save').toBe(before);
    await fs.rm(dir, { recursive: true, force: true });
  });
});

The same happens for every mutation (create_entities, add_observations, and delete_entities / delete_observations / delete_relations), since they all funnel through saveGraph.

Actual output

Verifier 1 (mode 0600 on an existing file, umask 022):

VERIFIER-A umask=22 before=600 after=644
AssertionError: expected '644' to be '600'
umask=22 initial=644 before=600 after=644
plain-writeFile-on-0444=threw EACCES | manager-on-0444: 444 -> 644

Verifier 2 (chmod 600 then each mutation; and a 0444 read-only file):

umask = 22
after initial create_entities : 644
operator chmod 600             : 600
after add_observations  : 644  isError=undefined
after delete_entities   : 644  isError=undefined
after 0444 -> add_observations  : 644 isError= undefined text= [{"entityName":"A","addedObservations":["y"]}]

Reproduced again on the pinned commit while filing this:

REPRO .../src/memory/index.ts:194 mode before=600 after=644 umask=22
AssertionError: permission bits after a save: expected '644' to be '600'
Expected behaviour

A mutation should preserve the permission bits of an existing memory file. A memory.jsonl the operator narrowed to 0600, restored from a 0600 backup, or created by another tool at 0600 must stay 0600 across create_entities / add_observations / delete_*. A write must not silently succeed on a 0444 read-only file that the pre-#4642 code refused to write.

Concrete basis: this same repository already established the contract, in a sibling implementation merged two days earlier. src/filesystem/lib.ts:215-231 uses the identical temp-file + rename pattern and then restores the mode:

const origStats = await fs.stat(filePath);
...
// Restore original permission bits since the atomic rename replaces the
// inode and the temp file has default (0644) permissions. Mask off the
// file-type bits; POSIX leaves them unspecified for chmod. A chmod
// failure must not fail the write, which has already succeeded.
try {
  await fs.chmod(filePath, origStats.mode & 0o777);
} catch {}

That landed as 562feeb2 "fix(filesystem): preserve file permissions during write and edit operations (#4115)" (2026-08-27), whose message reads: "The atomic write pattern (write temp file + rename) replaces the original inode, causing the new file to have default 0644 permissions regardless of what the original file had." #4642 (642a9112, 2026-08-29) applied the same pattern to src/memory/index.ts without the chmod; its message discusses only truncation/atomicity and never mentions permissions. Before #4642, the code was await fs.writeFile(this.memoryFilePath, lines.join("\n")), which preserves an existing file's mode — so this is a regression against the function's own prior behaviour and against the in-repo precedent.

Root cause

src/memory/index.ts:193-194 (KnowledgeGraphManager.saveGraph):

await fs.writeFile(tempFilePath, lines.join("\n") + "\n");
await fs.rename(tempFilePath, this.memoryFilePath);

rename(2) replaces the target inode with the temp file's inode, and the temp file was created under the process umask, so its mode is 0666 & ~umask (0644). The original file's mode is discarded with its inode; nothing restores it. rename also succeeds over a read-only (0444) target when the containing directory is writable, so a read-only memory file is overwritten rather than rejected.

Related issues / PRs
  • #4614 (closed) — "saveGraph() in src/memory uses non-atomic fs.writeFile"; the issue that #4642 fixed. This report is a regression introduced by that fix.
  • 562feeb2 / #4115 — the filesystem server's permission-preserving version of the same pattern (the precedent above).
  • #4117 (open) — broad "safer persistence defaults, atomic writes, …" proposal for the memory server; covers related persistence hardening, not this permissions regression.
Offer

Happy to open a PR that stats the target before the rename (tolerating ENOENT for a fresh file) and restores the mode after a successful rename with a best-effort chmod, matching src/filesystem/lib.ts exactly — or happy to be assigned if you'd prefer to handle it in-tree.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/memory/index.ts at KnowledgeGraphManager.saveGraph and compare its atomic write with the permission-preserving implementation in src/filesystem/lib.ts:215-231. Run the provided permissions reproduction, or add it as a focused Vitest regression test, covering existing 0600 and 0444 files. Done means mutations preserve existing permission bits and do not silently overwrite read-only files.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.