From bc7f98d9fd730bbb9734ee6e2a20e8d147efb5b9 Mon Sep 17 00:00:00 2001 From: BlueX888 <140241684+BlueX888@users.noreply.github.com> Date: Sat, 19 Sep 2026 18:58:07 +0800 Subject: [PATCH] fix(memory): preserve memory file permissions across atomic save saveGraph writes a temp file and renames it over the memory file, which replaces the inode with one created under the process umask. A memory file the operator narrowed with chmod 600 therefore came back 0644 after the next mutation. Capture the existing permission bits before the write and restore them after the rename, matching what the filesystem server does for the same temp-file+rename pattern. --- src/memory/__tests__/file-permissions.test.ts | 57 +++++++++++++++++++ src/memory/index.ts | 14 +++++ 2 files changed, 71 insertions(+) create mode 100644 src/memory/__tests__/file-permissions.test.ts diff --git a/src/memory/__tests__/file-permissions.test.ts b/src/memory/__tests__/file-permissions.test.ts new file mode 100644 index 0000000000..eb3b53aae0 --- /dev/null +++ b/src/memory/__tests__/file-permissions.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { KnowledgeGraphManager } from '../index.js'; + +/** + * Regression tests for the memory file's permission bits. + * + * saveGraph() writes a temporary file and renames it over the memory file. + * The rename replaces the inode, and the temp file is created under the + * process umask, so any permission bits the operator set on the memory file + * (chmod 600, restored from a 600 backup, written by another tool) are lost + * on the next mutation and the file comes back 0644. The filesystem server + * (src/filesystem/lib.ts) restores the original bits after the same + * temp-file + rename sequence; the memory server must do the same. + */ +describe('KnowledgeGraphManager file permissions', () => { + let testDir: string; + let testFilePath: string; + + // Octal, so a failure reports the modes an operator would have typed. + const modeOf = async (file: string): Promise => + ((await fs.stat(file)).mode & 0o777).toString(8); + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-memory-perms-')); + testFilePath = path.join(testDir, 'memory.jsonl'); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it.each([ + { label: 'owner-only (0600)', mode: 0o600 }, + { label: 'group-readable (0640)', mode: 0o640 }, + ])('keeps a $label memory file at that mode across a mutation', async ({ mode }) => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }, + ]); + + await fs.chmod(testFilePath, mode); + expect(await modeOf(testFilePath), 'mode set by the operator').toBe(mode.toString(8)); + + await manager.createEntities([ + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]); + + expect(await modeOf(testFilePath), 'mode after a save').toBe(mode.toString(8)); + + // The mutation must still have been persisted. + const reloaded = await new KnowledgeGraphManager(testFilePath).readGraph(); + expect(reloaded.entities.map(e => e.name).sort()).toEqual(['Alice', 'Bob']); + }); +}); diff --git a/src/memory/index.ts b/src/memory/index.ts index d9f814877b..b21a553160 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -183,6 +183,15 @@ export class KnowledgeGraphManager { // complete old file or the complete new one, never a partial state. // The temp file is kept in the same directory so the rename stays on one // filesystem — renaming across mount points fails with EXDEV. + // + // The rename also replaces the target's inode, and the temp file is + // created under the process umask, so a memory file an operator narrowed + // with chmod would come back 0644. Capture the existing bits and restore + // them after the rename, as src/filesystem/lib.ts does for the same + // write pattern. A memory file that does not exist yet has nothing to + // preserve. + const existingStats = await fs.stat(this.memoryFilePath).catch(() => undefined); + const directory = path.dirname(this.memoryFilePath); const tempFilePath = path.join( directory, @@ -197,6 +206,11 @@ export class KnowledgeGraphManager { await fs.unlink(tempFilePath).catch(() => {}); throw error; } + + if (existingStats) { + // The data is already durable, so a failed chmod must not fail the save. + await fs.chmod(this.memoryFilePath, existingStats.mode & 0o777).catch(() => {}); + } } async createEntities(entities: Entity[]): Promise {