Skip to content

Commit 31ab176

Browse files
phernandezclaude
andcommitted
fix(api): canonicalize sync-file paths before indexing
Review fixes for the #581 recovery endpoint: - The sync-file endpoint indexed the caller-supplied path verbatim. On case-insensitive filesystems (macOS/Windows) a wrong-cased path like 'notes/Disk-Note.md' passes the existence check for 'notes/disk-note.md', misses the DB row keyed by the on-disk path, and inserts a duplicate entity under the wrong-cased path. The endpoint now rejects non-normalized segments ('./', '//') with 400 and canonicalizes the path by matching real directory entries (exact name first, then a unique case-insensitive match), so the index lookup and sync always use the on-disk casing. Behavior is identical on case-sensitive and case-insensitive filesystems. - Document the automatic single-file disk recovery in the edit_note docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 787896e commit 31ab176

3 files changed

Lines changed: 149 additions & 9 deletions

File tree

src/basic_memory/api/v2/routers/knowledge_router.py

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
- Simplified caching strategies
1111
"""
1212

13+
import os
14+
import pathlib
15+
1316
from fastapi import APIRouter, HTTPException, Response, Path
1417
from loguru import logger
1518

@@ -242,6 +245,40 @@ async def resolve_identifier(
242245
## Single-file sync endpoint
243246

244247

248+
def _canonical_file_path(home: pathlib.Path, segments: list[str]) -> str | None:
249+
"""Resolve the actual on-disk casing of a file path under the project home.
250+
251+
Trigger: case-insensitive filesystems (macOS/Windows) pass existence checks for
252+
wrong-cased paths like 'notes/Disk-Note.md' when the file is 'notes/disk-note.md'.
253+
Why: indexing the caller-supplied casing misses the existing DB row keyed by the
254+
on-disk path and inserts a duplicate entity under the wrong-cased path.
255+
Outcome: each segment is matched against real directory entries — exact name first
256+
(so distinct case-variant files on case-sensitive filesystems stay distinct),
257+
then a unique case-insensitive match. Returns None when any segment cannot be
258+
matched to exactly one entry, including missing files.
259+
"""
260+
current = home
261+
canonical_segments: list[str] = []
262+
for segment in segments:
263+
try:
264+
with os.scandir(current) as entries_iter:
265+
entries = [entry.name for entry in entries_iter]
266+
except OSError:
267+
# A parent segment resolved to a non-directory (or vanished): no canonical
268+
# path exists for the remaining segments.
269+
return None
270+
if segment in entries:
271+
matched = segment
272+
else:
273+
matches = [entry for entry in entries if entry.lower() == segment.lower()]
274+
if len(matches) != 1:
275+
return None
276+
matched = matches[0]
277+
canonical_segments.append(matched)
278+
current = current / matched
279+
return "/".join(canonical_segments)
280+
281+
245282
@router.post("/sync-file", response_model=EntityResponseV2)
246283
async def sync_file(
247284
data: SyncFileRequest,
@@ -262,8 +299,9 @@ async def sync_file(
262299
The indexed entity
263300
264301
Raises:
265-
HTTPException: 400 if the path escapes the project root or is not
266-
markdown, 404 if the file does not exist on disk
302+
HTTPException: 400 if the path escapes the project root, contains
303+
non-normalized segments, or is not markdown, 404 if the file does
304+
not exist on disk
267305
"""
268306
with logfire.span(
269307
"api.request.knowledge.sync_file",
@@ -279,11 +317,26 @@ async def sync_file(
279317
detail=f"File path '{data.file_path}' is not allowed - "
280318
"paths must stay within project boundaries",
281319
)
282-
if not (project_config.home / data.file_path).is_file():
320+
321+
# Trigger: segments like './' or '//' survive the traversal check above
322+
# Why: a non-normalized path would index under a non-canonical DB key
323+
# Outcome: reject fail-fast instead of guessing the canonical form
324+
segments = data.file_path.replace("\\", "/").split("/")
325+
if any(segment in ("", ".") for segment in segments):
326+
raise HTTPException(
327+
status_code=400,
328+
detail=f"File path '{data.file_path}' is not normalized - "
329+
"segments like './' or '//' are not allowed",
330+
)
331+
332+
# Canonicalize to the actual on-disk casing so the DB lookup below hits the
333+
# row keyed by the real path instead of inserting a wrong-cased duplicate.
334+
file_path = _canonical_file_path(project_config.home, segments)
335+
if file_path is None or not (project_config.home / file_path).is_file():
283336
raise HTTPException(
284337
status_code=404, detail=f"File not found on disk: '{data.file_path}'"
285338
)
286-
if not sync_service.file_service.is_markdown(data.file_path):
339+
if not sync_service.file_service.is_markdown(file_path):
287340
raise HTTPException(
288341
status_code=400,
289342
detail=f"Only markdown files can be indexed: '{data.file_path}'",
@@ -292,15 +345,14 @@ async def sync_file(
292345
# Trigger: the file may already have a DB row (e.g. modified on disk after indexing)
293346
# Why: the indexer needs to know whether to insert or update the entity
294347
# Outcome: new is computed from the database instead of assumed by the caller
295-
existing = await sync_service.entity_repository.get_by_file_path(data.file_path)
348+
existing = await sync_service.entity_repository.get_by_file_path(file_path)
296349
synced = await sync_service.sync_one_markdown_file(
297-
data.file_path, new=existing is None, index_search=True
350+
file_path, new=existing is None, index_search=True
298351
)
299352

300353
result = EntityResponseV2.model_validate(synced.entity)
301354
logger.info(
302-
f"API v2 response: sync_file file_path='{data.file_path}' "
303-
f"external_id={result.external_id}"
355+
f"API v2 response: sync_file file_path='{file_path}' external_id={result.external_id}"
304356
)
305357
return result
306358

src/basic_memory/mcp/tools/edit_note.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,9 @@ async def edit_note(
375375
376376
Note:
377377
Edit operations require exact identifier matches. If unsure, use read_note() or
378-
search_notes() first to find the correct identifier. The tool provides detailed
378+
search_notes() first to find the correct identifier. When the identifier looks
379+
like a file path and the file exists on disk but is not indexed yet, edit_note
380+
indexes that file automatically and retries the edit. The tool provides detailed
379381
error messages with suggestions if operations fail.
380382
"""
381383
# Resolve effective default: allow MCP clients to send null for optional int field

tests/api/v2/test_knowledge_router.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,92 @@ async def test_sync_file_rejects_path_traversal(client: AsyncClient, v2_project_
10341034
assert "project boundaries" in response.json()["detail"]
10351035

10361036

1037+
@pytest.mark.asyncio
1038+
async def test_sync_file_wrong_cased_path_does_not_create_duplicate(
1039+
client: AsyncClient, v2_project_url, test_project: Project, entity_repository
1040+
):
1041+
"""A wrong-cased path resolves to the canonical on-disk file without duplicating it.
1042+
1043+
On case-insensitive filesystems (macOS/Windows) a wrong-cased path passes existence
1044+
checks; without canonicalization the indexer would insert a second entity keyed by
1045+
the wrong-cased path. The endpoint matches real directory entries, so the request
1046+
behaves identically on case-sensitive and case-insensitive filesystems.
1047+
"""
1048+
note_path = Path(test_project.path) / "notes" / "disk-note.md"
1049+
note_path.parent.mkdir(parents=True, exist_ok=True)
1050+
note_path.write_text("# Disk Note\n\nWritten directly to disk.\n", encoding="utf-8")
1051+
1052+
first = await client.post(
1053+
f"{v2_project_url}/knowledge/sync-file",
1054+
json={"file_path": "notes/disk-note.md"},
1055+
)
1056+
assert first.status_code == 200
1057+
canonical = EntityResponseV2.model_validate(first.json())
1058+
assert canonical.file_path == "notes/disk-note.md"
1059+
1060+
second = await client.post(
1061+
f"{v2_project_url}/knowledge/sync-file",
1062+
json={"file_path": "notes/Disk-Note.md"},
1063+
)
1064+
assert second.status_code == 200
1065+
synced = EntityResponseV2.model_validate(second.json())
1066+
assert synced.file_path == "notes/disk-note.md"
1067+
assert synced.external_id == canonical.external_id
1068+
1069+
entities = await entity_repository.find_all()
1070+
assert [entity.file_path for entity in entities] == ["notes/disk-note.md"]
1071+
1072+
1073+
@pytest.mark.asyncio
1074+
async def test_sync_file_rejects_non_normalized_segments(
1075+
client: AsyncClient, v2_project_url, test_project: Project
1076+
):
1077+
"""sync-file rejects './' and '//' style segments instead of indexing them verbatim."""
1078+
note_path = Path(test_project.path) / "notes" / "disk-note.md"
1079+
note_path.parent.mkdir(parents=True, exist_ok=True)
1080+
note_path.write_text("# Disk Note\n", encoding="utf-8")
1081+
1082+
for non_normalized in ("./notes/disk-note.md", "notes//disk-note.md"):
1083+
response = await client.post(
1084+
f"{v2_project_url}/knowledge/sync-file",
1085+
json={"file_path": non_normalized},
1086+
)
1087+
assert response.status_code == 400
1088+
assert "not normalized" in response.json()["detail"]
1089+
1090+
1091+
@pytest.mark.asyncio
1092+
async def test_sync_file_directory_returns_404(
1093+
client: AsyncClient, v2_project_url, test_project: Project
1094+
):
1095+
"""sync-file refuses a path that canonicalizes to a directory instead of a file."""
1096+
(Path(test_project.path) / "just-a-directory").mkdir(parents=True, exist_ok=True)
1097+
1098+
response = await client.post(
1099+
f"{v2_project_url}/knowledge/sync-file",
1100+
json={"file_path": "just-a-directory"},
1101+
)
1102+
assert response.status_code == 404
1103+
assert "File not found on disk" in response.json()["detail"]
1104+
1105+
1106+
@pytest.mark.asyncio
1107+
async def test_sync_file_path_through_file_returns_404(
1108+
client: AsyncClient, v2_project_url, test_project: Project
1109+
):
1110+
"""sync-file fails fast when a parent segment resolves to a file, not a directory."""
1111+
note_path = Path(test_project.path) / "notes" / "disk-note.md"
1112+
note_path.parent.mkdir(parents=True, exist_ok=True)
1113+
note_path.write_text("# Disk Note\n", encoding="utf-8")
1114+
1115+
response = await client.post(
1116+
f"{v2_project_url}/knowledge/sync-file",
1117+
json={"file_path": "notes/disk-note.md/child.md"},
1118+
)
1119+
assert response.status_code == 404
1120+
assert "File not found on disk" in response.json()["detail"]
1121+
1122+
10371123
@pytest.mark.asyncio
10381124
async def test_sync_file_rejects_non_markdown(
10391125
client: AsyncClient, v2_project_url, test_project: Project

0 commit comments

Comments
 (0)