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
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
#4642fixed. 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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