diff --git a/.changeset/memory-skill-files-path-normalisation.md b/.changeset/memory-skill-files-path-normalisation.md new file mode 100644 index 0000000000..0c4a6a07d3 --- /dev/null +++ b/.changeset/memory-skill-files-path-normalisation.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/server': patch +--- + +`memorySkillFiles.list()` and `memorySkillFiles.read()` normalise the path they are given the same way the record index normalises its keys, so `list('sub/')`, `list('./sub')` and `read('./SKILL.md')` resolve instead of reporting a missing directory or file, and a `..` segment is rejected before it can reach the record. diff --git a/packages/server/src/ext/skills/files.ts b/packages/server/src/ext/skills/files.ts index 8b93955ddf..009500bc76 100644 --- a/packages/server/src/ext/skills/files.ts +++ b/packages/server/src/ext/skills/files.ts @@ -39,6 +39,16 @@ function indexRecord(keys: readonly string[]): MemoryIndex { return { dirs, files }; } +// `''` is the record root; every other path is normalised the same way the +// index normalises record keys, so `sub/` and `./SKILL.md` resolve. +function normaliseDir(dir: string): string { + return dir === '' ? '' : splitRelativePath(dir, 'directory').join('/'); +} + +function normalisePath(path: string): string { + return splitRelativePath(path, 'file path').join('/'); +} + /** * An in-memory adapter. The record is re-read whenever a walk starts — `list('')` — so `reload()` sees later changes. * Other calls use the index from that read (built on first use if no walk has started), so their cost does not grow @@ -52,14 +62,17 @@ export function memorySkillFiles(files: Record): Sk }; return { async list(dir) { - const children = (dir === '' ? reindex() : (index ?? reindex())).dirs.get(dir); + const normalised = normaliseDir(dir); + const children = (normalised === '' ? reindex() : (index ?? reindex())).dirs.get(normalised); if (children !== undefined) return Array.from(children.values(), entry => ({ ...entry })); - if (dir === '') return []; + if (normalised === '') return []; throw new Error(`no such directory: ${dir}`); }, async read(path) { + // Validation comes first: a `..` segment must never reach the record. + const normalised = normalisePath(path); // Keys written as `./a/b` or `a//b` are found through the index. - const key = Object.hasOwn(files, path) ? path : (index ?? reindex()).files.get(path); + const key = Object.hasOwn(files, normalised) ? normalised : (index ?? reindex()).files.get(normalised); const content = key === undefined ? undefined : files[key]; if (content === undefined) throw new Error(`no such file: ${path}`); return typeof content === 'string' ? encodeUtf8(content) : content; diff --git a/packages/server/test/ext/skills/files.test.ts b/packages/server/test/ext/skills/files.test.ts new file mode 100644 index 0000000000..80d5cc03ff --- /dev/null +++ b/packages/server/test/ext/skills/files.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from 'vitest'; + +import { memorySkillFiles } from '../../../src/ext/skills/index'; + +const RECORD = { + 'SKILL.md': '# skill', + 'sub/other.txt': 'other' +}; + +describe('memorySkillFiles', () => { + it('lists a directory given with a trailing slash or a leading dot segment', async () => { + const files = memorySkillFiles(RECORD); + + await expect(files.list('sub/')).resolves.toEqual([{ name: 'other.txt', kind: 'file' }]); + await expect(files.list('./sub')).resolves.toEqual([{ name: 'other.txt', kind: 'file' }]); + }); + + it('lists the root for an empty directory', async () => { + const files = memorySkillFiles(RECORD); + + await expect(files.list('')).resolves.toEqual( + expect.arrayContaining([ + { name: 'SKILL.md', kind: 'file' }, + { name: 'sub', kind: 'directory' } + ]) + ); + }); + + it('reads a file given in a non-canonical form', async () => { + const files = memorySkillFiles(RECORD); + + await expect(files.read('./SKILL.md')).resolves.toEqual(expect.any(Uint8Array)); + await expect(files.read('sub//other.txt')).resolves.toEqual(expect.any(Uint8Array)); + }); + + it('finds a record key that is itself non-canonical', async () => { + const files = memorySkillFiles({ './SKILL.md': '# skill' }); + + await expect(files.read('SKILL.md')).resolves.toEqual(expect.any(Uint8Array)); + }); + + it('rejects a path that could leave the record', async () => { + const files = memorySkillFiles(RECORD); + + await expect(files.list('..')).rejects.toThrow(TypeError); + await expect(files.read('../SKILL.md')).rejects.toThrow(TypeError); + await expect(files.read('')).rejects.toThrow(TypeError); + }); + + it('still reports a missing path', async () => { + const files = memorySkillFiles(RECORD); + + await expect(files.list('missing')).rejects.toThrow('no such directory: missing'); + await expect(files.read('missing.txt')).rejects.toThrow('no such file: missing.txt'); + }); +});