Skip to content

Commit 1374080

Browse files
phernandezclaude
andcommitted
fix(api): honor ignore rules in sync-file and fail fast in disk recovery
Address BM Bossbot review findings on the #581 recovery path: - The sync-file endpoint went straight from boundary/markdown checks to sync_one_markdown_file, bypassing the ignore filtering that scan and watch flows apply before indexing. Hidden files, .bmignore matches, and project .gitignore matches could be indexed and made searchable (also indirectly via edit_note disk recovery). The canonical path is now checked against the same should_ignore_path() rules and rejected with 400. Regression tests cover hidden, .gitignore-ignored, and .bmignore-ignored files. - edit_note disk recovery treated every sync-file ToolError as a harmless non-recovery case, masking auth/server/transport failures as not-found and letting append/prepend continue into auto-create. Only expected 400/404 candidate rejections are suppressed now; anything else propagates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 31ab176 commit 1374080

4 files changed

Lines changed: 112 additions & 6 deletions

File tree

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from loguru import logger
1818

1919
import logfire
20+
from basic_memory.ignore_utils import load_gitignore_patterns, should_ignore_path
2021
from basic_memory.deps import (
2122
EntityServiceV2ExternalDep,
2223
SearchServiceV2ExternalDep,
@@ -300,8 +301,8 @@ async def sync_file(
300301
301302
Raises:
302303
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
304+
non-normalized segments, matches the project ignore rules, or is
305+
not markdown, 404 if the file does not exist on disk
305306
"""
306307
with logfire.span(
307308
"api.request.knowledge.sync_file",
@@ -336,6 +337,20 @@ async def sync_file(
336337
raise HTTPException(
337338
status_code=404, detail=f"File not found on disk: '{data.file_path}'"
338339
)
340+
# Trigger: the canonical path matches the .bmignore / project .gitignore rules
341+
# Why: scan and watch flows filter ignored files before they ever reach the
342+
# indexer; indexing one here would bypass the ignored-file contract and
343+
# make hidden or gitignored content searchable
344+
# Outcome: the same should_ignore_path() rules apply to single-file sync
345+
ignore_patterns = load_gitignore_patterns(project_config.home)
346+
if should_ignore_path(
347+
project_config.home / file_path, project_config.home, ignore_patterns
348+
):
349+
raise HTTPException(
350+
status_code=400,
351+
detail=f"File path '{data.file_path}' matches Basic Memory ignore rules "
352+
"(.bmignore or project .gitignore) and cannot be indexed",
353+
)
339354
if not sync_service.file_service.is_markdown(file_path):
340355
raise HTTPException(
341356
status_code=400,

src/basic_memory/mcp/tools/edit_note.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from typing import TYPE_CHECKING, Annotated, Optional, Literal
44

55
import logfire
6+
from httpx import HTTPStatusError
67
from loguru import logger
78
from fastmcp import Context
89
from mcp.server.fastmcp.exceptions import ToolError
@@ -73,9 +74,20 @@ async def _resolve_after_disk_recovery(
7374
try:
7475
await knowledge_client.sync_file(candidate)
7576
except ToolError as sync_error:
76-
# Trigger: server rejected the candidate path (missing file, traversal, non-markdown)
77-
# Why: only identifiers that plausibly map to a real file qualify for recovery
78-
# Outcome: caller falls through to its existing not-found behavior
77+
# Trigger: the sync-file request failed
78+
# Why: 400/404 are the expected "nothing to recover" rejections (missing file,
79+
# traversal, ignored path, non-markdown); anything else — auth, server,
80+
# transport-level failures — is a real error that must not be masked as
81+
# a not-found miss
82+
# Outcome: expected rejections fall through to the caller's existing
83+
# not-found behavior; unexpected failures propagate
84+
cause = sync_error.__cause__
85+
candidate_rejected = isinstance(cause, HTTPStatusError) and cause.response.status_code in (
86+
400,
87+
404,
88+
)
89+
if not candidate_rejected:
90+
raise
7991
logger.debug(f"edit_note disk recovery skipped for '{candidate}': {sync_error}")
8092
return None
8193

tests/api/v2/test_knowledge_router.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import pytest
88
from httpx import AsyncClient
99

10+
from basic_memory.ignore_utils import get_bmignore_path
1011
from basic_memory.models import Entity as EntityModel, Project
1112
from basic_memory.repository.entity_repository import EntityRepository
1213
from basic_memory.repository.project_repository import ProjectRepository
@@ -1120,6 +1121,62 @@ async def test_sync_file_path_through_file_returns_404(
11201121
assert "File not found on disk" in response.json()["detail"]
11211122

11221123

1124+
@pytest.mark.asyncio
1125+
async def test_sync_file_rejects_hidden_file(
1126+
client: AsyncClient, v2_project_url, test_project: Project
1127+
):
1128+
"""sync-file refuses hidden files, matching the default '.*' ignore pattern."""
1129+
hidden_path = Path(test_project.path) / ".secrets.md"
1130+
hidden_path.write_text("# Hidden\n\nShould never be indexed.\n", encoding="utf-8")
1131+
1132+
response = await client.post(
1133+
f"{v2_project_url}/knowledge/sync-file",
1134+
json={"file_path": ".secrets.md"},
1135+
)
1136+
assert response.status_code == 400
1137+
assert "ignore rules" in response.json()["detail"]
1138+
1139+
1140+
@pytest.mark.asyncio
1141+
async def test_sync_file_rejects_gitignored_file(
1142+
client: AsyncClient, v2_project_url, test_project: Project
1143+
):
1144+
"""sync-file honors the project .gitignore, matching scan/watch filtering."""
1145+
project_path = Path(test_project.path)
1146+
(project_path / ".gitignore").write_text("private/\n", encoding="utf-8")
1147+
note_path = project_path / "private" / "secret.md"
1148+
note_path.parent.mkdir(parents=True, exist_ok=True)
1149+
note_path.write_text("# Secret\n\nGitignored content.\n", encoding="utf-8")
1150+
1151+
response = await client.post(
1152+
f"{v2_project_url}/knowledge/sync-file",
1153+
json={"file_path": "private/secret.md"},
1154+
)
1155+
assert response.status_code == 400
1156+
assert "ignore rules" in response.json()["detail"]
1157+
1158+
1159+
@pytest.mark.asyncio
1160+
async def test_sync_file_rejects_bmignored_file(
1161+
client: AsyncClient, v2_project_url, test_project: Project
1162+
):
1163+
"""sync-file honors user .bmignore patterns, matching scan/watch filtering."""
1164+
bmignore_path = get_bmignore_path()
1165+
bmignore_path.parent.mkdir(parents=True, exist_ok=True)
1166+
bmignore_path.write_text("drafts-wip\n", encoding="utf-8")
1167+
1168+
note_path = Path(test_project.path) / "drafts-wip" / "scratch.md"
1169+
note_path.parent.mkdir(parents=True, exist_ok=True)
1170+
note_path.write_text("# Scratch\n\nBmignored content.\n", encoding="utf-8")
1171+
1172+
response = await client.post(
1173+
f"{v2_project_url}/knowledge/sync-file",
1174+
json={"file_path": "drafts-wip/scratch.md"},
1175+
)
1176+
assert response.status_code == 400
1177+
assert "ignore rules" in response.json()["detail"]
1178+
1179+
11231180
@pytest.mark.asyncio
11241181
async def test_sync_file_rejects_non_markdown(
11251182
client: AsyncClient, v2_project_url, test_project: Project

tests/mcp/test_tool_edit_note.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
from pathlib import Path
44
from unittest.mock import patch
55

6+
import httpx
67
import pytest
8+
from mcp.server.fastmcp.exceptions import ToolError
79

8-
from basic_memory.mcp.tools.edit_note import edit_note
10+
from basic_memory.mcp.clients import KnowledgeClient
11+
from basic_memory.mcp.tools.edit_note import _resolve_after_disk_recovery, edit_note
912
from basic_memory.mcp.tools.read_note import read_note
1013
from basic_memory.mcp.tools.write_note import write_note
1114

@@ -1329,6 +1332,25 @@ async def test_edit_note_append_recovers_file_on_disk_instead_of_autocreate(clie
13291332
assert "Appended line." in final_content
13301333

13311334

1335+
@pytest.mark.asyncio
1336+
async def test_resolve_after_disk_recovery_propagates_unexpected_errors():
1337+
"""Server-side failures during disk recovery must not be masked as a not-found miss.
1338+
1339+
Only 400/404 sync-file rejections mean "nothing to recover"; a 500 (or auth
1340+
failure) would otherwise be swallowed and edit_note would continue into
1341+
auto-create with a misleading not-found error.
1342+
"""
1343+
1344+
def server_error(request: httpx.Request) -> httpx.Response:
1345+
return httpx.Response(500, json={"detail": "boom"})
1346+
1347+
transport = httpx.MockTransport(server_error)
1348+
async with httpx.AsyncClient(transport=transport, base_url="http://test") as http_client:
1349+
knowledge_client = KnowledgeClient(http_client, "project-external-id")
1350+
with pytest.raises(ToolError, match="boom"):
1351+
await _resolve_after_disk_recovery(knowledge_client, "notes/unlucky-note")
1352+
1353+
13321354
@pytest.mark.asyncio
13331355
async def test_edit_note_append_traversal_identifier_is_blocked(client, test_project):
13341356
"""A traversal identifier must be rejected by both disk recovery and auto-create."""

0 commit comments

Comments
 (0)