Skip to content

Commit 3e3b027

Browse files
phernandezclaude
andcommitted
fix: update tests and fix bugs for project-prefixed permalinks
- Fix update_entity overwriting project-prefixed permalink with non-prefixed one during metadata merge - Fix memory:// URL path traversal validation bypass in read_note and read_content tools - Fix pyright type error in claude_projects_importer - Update test assertions across 12 test files to use project-prefixed permalinks (test-project/path instead of path) - Fix monkeypatch in search test to use resolve_project_and_path instead of removed get_active_project Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 49c702f commit 3e3b027

16 files changed

Lines changed: 94 additions & 63 deletions

src/basic_memory/importers/claude_projects_importer.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ async def import_data(
7171
)
7272
permalink, file_path = self.build_import_paths(prompt_path)
7373
prompt_entity = self._format_prompt_markdown(project, permalink)
74-
await self.write_entity(prompt_entity, file_path)
74+
if prompt_entity:
75+
await self.write_entity(prompt_entity, file_path)
7576
prompts_imported += 1
7677

7778
# Import project documents

src/basic_memory/mcp/tools/read_content.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from basic_memory.mcp.server import mcp
2020
from basic_memory.mcp.async_client import get_client
2121
from basic_memory.mcp.tools.utils import call_get, resolve_entity_id
22+
from basic_memory.schemas.memory import memory_url_path
2223
from basic_memory.utils import validate_project_path
2324

2425

@@ -205,8 +206,13 @@ async def read_content(
205206
active_project, url, _ = await resolve_project_and_path(client, path, project, context)
206207

207208
# Validate path to prevent path traversal attacks
209+
# For memory:// URLs, validate the extracted path (not the raw URL which
210+
# has a scheme prefix that confuses path validation)
211+
raw_path = memory_url_path(path) if path.startswith("memory://") else path
208212
project_path = active_project.home
209-
if not validate_project_path(url, project_path):
213+
if not validate_project_path(raw_path, project_path) or not validate_project_path(
214+
url, project_path
215+
):
210216
logger.warning(
211217
"Attempted path traversal attack blocked",
212218
path=path,

src/basic_memory/mcp/tools/read_note.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from basic_memory.mcp.project_context import resolve_project_and_path
1111
from basic_memory.mcp.server import mcp
1212
from basic_memory.mcp.tools.search import search_notes
13+
from basic_memory.schemas.memory import memory_url_path
1314
from basic_memory.utils import validate_project_path
1415

1516

@@ -83,11 +84,13 @@ async def read_note(
8384
)
8485

8586
# Validate identifier to prevent path traversal attacks
86-
# We need to check both the raw identifier and the processed path
87+
# For memory:// URLs, validate the extracted path (not the raw URL which
88+
# has a scheme prefix that confuses path validation)
89+
raw_path = memory_url_path(identifier) if identifier.startswith("memory://") else identifier
8790
processed_path = entity_path
8891
project_path = active_project.home
8992

90-
if not validate_project_path(identifier, project_path) or not validate_project_path(
93+
if not validate_project_path(raw_path, project_path) or not validate_project_path(
9194
processed_path, project_path
9295
):
9396
logger.warning(

src/basic_memory/services/entity_service.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
)
4343
from basic_memory.services.link_resolver import LinkResolver
4444
from basic_memory.services.search_service import SearchService
45-
from basic_memory.utils import build_canonical_permalink, generate_permalink
45+
from basic_memory.utils import build_canonical_permalink
4646

4747

4848
class EntityService(BaseService[EntityModel]):
@@ -346,9 +346,12 @@ async def update_entity(self, entity: EntityModel, schema: EntitySchema) -> Enti
346346
# Merge new metadata with existing metadata
347347
existing_markdown.frontmatter.metadata.update(post.metadata)
348348

349-
# Ensure the permalink in the metadata is the resolved one
350-
if new_permalink != entity.permalink:
351-
existing_markdown.frontmatter.metadata["permalink"] = new_permalink
349+
# Always ensure the permalink in the metadata is the canonical one from the database.
350+
# The schema_to_markdown call above uses EntitySchema.permalink which computes a
351+
# non-prefixed permalink (e.g., "test/note"). The metadata merge on the previous line
352+
# would overwrite the project-prefixed permalink (e.g., "project/test/note") stored
353+
# in the existing file. Setting it unconditionally preserves the correct value.
354+
existing_markdown.frontmatter.metadata["permalink"] = new_permalink
352355

353356
# Create a new post with merged metadata
354357
merged_post = frontmatter.Post(post.content, **existing_markdown.frontmatter.metadata)

tests/api/v2/test_knowledge_router.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ async def test_create_entity(client: AsyncClient, file_service, v2_project_url):
171171
assert isinstance(entity.id, int)
172172
assert entity.api_version == "v2"
173173

174-
assert entity.permalink == "test/test-v2-entity"
174+
assert entity.permalink == "test-project/test/test-v2-entity"
175175
assert entity.file_path == "test/TestV2Entity.md"
176176
assert entity.entity_type == data["entity_type"]
177177

tests/importers/test_conversation_indexing.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ async def test_imported_conversations_have_correct_permalink_and_title(
8383
assert "---" in content, "File should have frontmatter markers"
8484
assert "title: My Test Conversation Title" in content, "File should have title in frontmatter"
8585
assert (
86-
f"permalink: {project_config.name}/conversations/20250115-My_Test_Conversation_Title"
86+
f"permalink: {project_config.name}/conversations/20250115-my-test-conversation-title"
8787
in content
8888
), (
8989
"File should have permalink in frontmatter"
@@ -104,7 +104,7 @@ async def test_imported_conversations_have_correct_permalink_and_title(
104104
)
105105
assert (
106106
entity.permalink
107-
== f"{project_config.name}/conversations/20250115-My_Test_Conversation_Title"
107+
== f"{project_config.name}/conversations/20250115-my-test-conversation-title"
108108
), (
109109
f"Permalink should be from frontmatter, got: {entity.permalink}"
110110
)
@@ -121,7 +121,7 @@ async def test_imported_conversations_have_correct_permalink_and_title(
121121
)
122122
assert (
123123
search_result.permalink
124-
== f"{project_config.name}/conversations/20250115-My_Test_Conversation_Title"
124+
== f"{project_config.name}/conversations/20250115-my-test-conversation-title"
125125
), (
126126
f"Search permalink should not be null, got: {search_result.permalink}"
127127
)

tests/mcp/test_tool_search.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,8 @@ class StubProject:
437437
id = 1
438438
external_id = "test-external-id"
439439

440-
async def fake_get_active_project(*args, **kwargs):
441-
return StubProject()
440+
async def fake_resolve_project_and_path(*args, **kwargs):
441+
return StubProject(), "semantic lookup", False
442442

443443
captured_payload: dict = {}
444444

@@ -450,7 +450,7 @@ async def search(self, payload, page, page_size):
450450
captured_payload.update(payload)
451451
return SearchResponse(results=[], current_page=page, page_size=page_size)
452452

453-
monkeypatch.setattr(search_mod, "get_active_project", fake_get_active_project)
453+
monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path)
454454
monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient)
455455

456456
result = await search_mod.search_notes.fn(

tests/mcp/test_tool_write_note.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ async def test_write_note_with_custom_entity_type(app, test_project):
544544
assert "# Created note" in result
545545
assert f"project: {test_project.name}" in result
546546
assert "file_path: guides/Test Guide.md" in result
547-
assert "permalink: guides/test-guide" in result
547+
assert f"permalink: {test_project.name}/guides/test-guide" in result
548548
assert "## Tags" in result
549549
assert "- guide, documentation" in result
550550
assert f"[Session: Using project '{test_project.name}']" in result

tests/mcp/tools/test_chatgpt_tools.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ async def test_search_successful_results(client, test_project):
3838
assert content["query"] == "test content"
3939

4040
# Verify individual result format
41-
assert any(r["id"] == "docs/test-document-1" for r in content["results"])
42-
assert any(r["id"] == "docs/test-document-2" for r in content["results"])
41+
assert any(r["id"] == f"{test_project.name}/docs/test-document-1" for r in content["results"])
42+
assert any(r["id"] == f"{test_project.name}/docs/test-document-2" for r in content["results"])
4343

4444

4545
@pytest.mark.asyncio

tests/services/test_context_service.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ async def test_find_connected_timeframe(
137137
@pytest.mark.asyncio
138138
async def test_build_context(context_service, test_graph):
139139
"""Test exact permalink lookup."""
140-
url = memory_url.validate_strings("memory://test/root")
140+
url = memory_url.validate_strings("memory://test-project/test/root")
141141
context_result = await context_service.build_context(url)
142142

143143
# Check metadata
@@ -156,7 +156,7 @@ async def test_build_context(context_service, test_graph):
156156
assert primary_result.id == test_graph["root"].id
157157
assert primary_result.type == "entity"
158158
assert primary_result.title == "Root"
159-
assert primary_result.permalink == "test/root"
159+
assert primary_result.permalink == "test-project/test/root"
160160
assert primary_result.file_path == "test/Root.md"
161161
assert primary_result.created_at is not None
162162

@@ -185,7 +185,7 @@ async def test_build_context_with_observations(context_service, test_graph):
185185
# Let's use those existing observations
186186

187187
# Build context
188-
url = memory_url.validate_strings("memory://test/root")
188+
url = memory_url.validate_strings("memory://test-project/test/root")
189189
context_result = await context_service.build_context(url, include_observations=True)
190190

191191
# Check the metadata
@@ -220,9 +220,9 @@ async def test_build_context_not_found(context_service):
220220
@pytest.mark.asyncio
221221
async def test_context_metadata(context_service, test_graph):
222222
"""Test metadata is correctly populated."""
223-
context = await context_service.build_context("memory://test/root", depth=2)
223+
context = await context_service.build_context("memory://test-project/test/root", depth=2)
224224
metadata = context.metadata
225-
assert metadata.uri == "test/root"
225+
assert metadata.uri == "test-project/test/root"
226226
assert metadata.depth == 2
227227
assert metadata.generated_at is not None
228228
assert metadata.primary_count > 0

0 commit comments

Comments
 (0)