From 0bd2792b40815b42011b856c00db8585f6ff9700 Mon Sep 17 00:00:00 2001 From: kzahiri1 Date: Sat, 5 Sep 2026 14:57:47 -0700 Subject: [PATCH] fix(memory): require a separator after the /memories prefix _validate_path accepted any path starting with the characters /memories and then sliced that prefix off, so /memoriesX became the relative path X and landed inside the store. create /memoriesX wrote /X, and delete /memoriessub removed /sub and returned Successfully deleted /memoriessub, naming a path that exists nowhere. Nothing escaped the store, since the escape check below still held, but a command aimed outside it altered something inside it and said otherwise. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01E8X6AL5NTTrQ458Y5anofR --- .../lib/tools/_beta_builtin_memory_tool.py | 10 ++++- .../lib/tools/memory_tools/test_filesystem.py | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/anthropic/lib/tools/_beta_builtin_memory_tool.py b/src/anthropic/lib/tools/_beta_builtin_memory_tool.py index dd37de615..63ce8f366 100644 --- a/src/anthropic/lib/tools/_beta_builtin_memory_tool.py +++ b/src/anthropic/lib/tools/_beta_builtin_memory_tool.py @@ -395,7 +395,10 @@ def __init__(self, base_path: str = "./memory"): def _validate_path(self, path: str) -> Path: """Validate and resolve memory paths""" - if not path.startswith("/memories"): + # The separator matters: without it "/memoriesX" passes the prefix check and + # then slices down to "X", so a path that names something outside the store + # is silently re-pointed at /X. + if path != "/memories" and not path.startswith("/memories/"): raise ToolError(f"Path must start with /memories, got: {path}") relative_path = path[len("/memories") :].lstrip("/") @@ -681,7 +684,10 @@ async def _ensure_memory_root(self) -> None: async def _validate_path(self, path: str) -> AsyncPath: """Validate and resolve memory paths""" - if not path.startswith("/memories"): + # The separator matters: without it "/memoriesX" passes the prefix check and + # then slices down to "X", so a path that names something outside the store + # is silently re-pointed at /X. + if path != "/memories" and not path.startswith("/memories/"): raise ToolError(f"Path must start with /memories, got: {path}") relative_path = path[len("/memories") :].lstrip("/") diff --git a/tests/lib/tools/memory_tools/test_filesystem.py b/tests/lib/tools/memory_tools/test_filesystem.py index 8462ffd7a..a9d4a070c 100644 --- a/tests/lib/tools/memory_tools/test_filesystem.py +++ b/tests/lib/tools/memory_tools/test_filesystem.py @@ -449,6 +449,27 @@ def test_delete_not_allow_deleting_memories_directory( with pytest.raises(ToolError, match="Cannot delete the /memories directory itself"): sync_local_filesystem_tool.delete(BetaMemoryTool20250818DeleteCommand(command="delete", path="/memories")) + @pytest.mark.parametrize("path", ["/memoriesX", "/memoriesXY/z.md", "/memoriessub", "/memories."]) + def test_paths_sharing_the_prefix_without_a_separator_are_rejected( + self, sync_local_filesystem_tool: BetaLocalFilesystemMemoryTool, temp_directory: str, path: str + ) -> None: + """Paths that share the prefix without a separator name nothing inside the + store. Unchecked, they slice down to a relative path and land on a real + entry: delete "/memoriessub" removes /sub and reports success.""" + sync_local_filesystem_tool.create( + BetaMemoryTool20250818CreateCommand(command="create", file_text="keep me", path="/memories/sub/a.txt") + ) + + with pytest.raises(ToolError, match="Path must start with /memories"): + sync_local_filesystem_tool.delete(BetaMemoryTool20250818DeleteCommand(command="delete", path=path)) + + with pytest.raises(ToolError, match="Path must start with /memories"): + sync_local_filesystem_tool.create( + BetaMemoryTool20250818CreateCommand(command="create", file_text="written", path=path) + ) + + assert get_directory_snapshot(temp_directory) == {"memories/sub/a.txt": "keep me"} + def test_rename(self, sync_local_filesystem_tool: BetaLocalFilesystemMemoryTool) -> None: sync_local_filesystem_tool.create( BetaMemoryTool20250818CreateCommand( @@ -988,6 +1009,27 @@ async def test_delete_not_allow_deleting_memories_directory( BetaMemoryTool20250818DeleteCommand(command="delete", path="/memories") ) + @pytest.mark.parametrize("path", ["/memoriesX", "/memoriesXY/z.md", "/memoriessub", "/memories."]) + async def test_paths_sharing_the_prefix_without_a_separator_are_rejected( + self, async_local_filesystem_tool: BetaAsyncLocalFilesystemMemoryTool, temp_directory: str, path: str + ) -> None: + """Paths that share the prefix without a separator name nothing inside the + store. Unchecked, they slice down to a relative path and land on a real + entry: delete "/memoriessub" removes /sub and reports success.""" + await async_local_filesystem_tool.create( + BetaMemoryTool20250818CreateCommand(command="create", file_text="keep me", path="/memories/sub/a.txt") + ) + + with pytest.raises(ToolError, match="Path must start with /memories"): + await async_local_filesystem_tool.delete(BetaMemoryTool20250818DeleteCommand(command="delete", path=path)) + + with pytest.raises(ToolError, match="Path must start with /memories"): + await async_local_filesystem_tool.create( + BetaMemoryTool20250818CreateCommand(command="create", file_text="written", path=path) + ) + + assert get_directory_snapshot(temp_directory) == {"memories/sub/a.txt": "keep me"} + async def test_rename(self, async_local_filesystem_tool: BetaAsyncLocalFilesystemMemoryTool) -> None: await async_local_filesystem_tool.create( BetaMemoryTool20250818CreateCommand(