Skip to content

Commit 60f3cd4

Browse files
cliffhallclaude
andcommitted
fix(filesystem): check a retargeted allowed-directory symlink before creating under it
A Root's advertised form can be a symlink, which can be retargeted after the Roots load. resolveUnicodeEquivalentPath started its walk for a missing path from the allowed directory's realpath without checking it, so write_file or create_directory under a retargeted alias landed outside every allowed directory. Check that starting realpath like each component below it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>
1 parent 40a28a8 commit 60f3cd4

2 files changed

Lines changed: 43 additions & 0 deletions

File tree

‎src/filesystem/__tests__/server-roots.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,36 @@ describe("initial roots", () => {
132132
}
133133
});
134134

135+
// #5183: the advertised form names a symlink, which can be retargeted after
136+
// the Roots load. A new file or directory under it must still be refused
137+
// when the symlink now leads outside every allowed directory.
138+
it("refuse creating under a root alias retargeted outside the root (#5183)", async () => {
139+
const link = path.join(cliDir, "link-to-root");
140+
await fs.symlink(rootDir, link);
141+
const { client } = await connectTracked([], {
142+
capabilities: ROOTS,
143+
listRoots: () => [rootOf(link)],
144+
});
145+
await vi.waitFor(async () =>
146+
expect(await allowedDirectoriesOf(client)).toEqual([link, rootDir]),
147+
);
148+
const outside = path.join(cliDir, "outside");
149+
await fs.mkdir(outside);
150+
await fs.rm(link);
151+
await fs.symlink(outside, link);
152+
153+
const written = await call(client, "write_file", {
154+
path: path.join(link, "new.txt"),
155+
content: "x",
156+
});
157+
expect(textOf(written)).toMatch(/^Access denied/);
158+
const created = await call(client, "create_directory", {
159+
path: path.join(link, "a", "b"),
160+
});
161+
expect(textOf(created)).toMatch(/^Access denied/);
162+
expect(await fs.readdir(outside)).toEqual([]);
163+
});
164+
135165
// #5183: the advertised form is kept as the client spelled it. Trimming
136166
// whitespace or quotes from it would name a different directory, here an
137167
// unrelated sibling, and allow it. POSIX only: Windows filenames cannot end

‎src/filesystem/lib.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,19 @@ async function resolveUnicodeEquivalentPath(
267267
}
268268

269269
let currentPath = await fs.realpath(allowedDirectory);
270+
// An allowed directory can be a symlink (the advertised form of a Root or a
271+
// command-line directory), retargeted since it was allowed, so its realpath
272+
// is checked like every component below it (#5183).
273+
if (
274+
!isPathWithinAllowedDirectories(
275+
normalizePathFormat(currentPath),
276+
allowedDirectories,
277+
)
278+
) {
279+
throw new Error(
280+
`Access denied - symlink target outside allowed directories: ${currentPath} not in ${allowedDirectories.join(", ")}`,
281+
);
282+
}
270283
const relativeParts = path
271284
.relative(allowedDirectory, absolutePath)
272285
.split(path.sep)

0 commit comments

Comments
 (0)