fix: enforce strict entity resolution in destructive MCP tools (#650)

Signed-off-by: phernandez <paul@basicmachines.co>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Paul Hernandez
2026-03-08 15:56:07 -05:00
committed by GitHub
parent 76ac880f2d
commit 9809b469c6
11 changed files with 309 additions and 17 deletions
+1 -1
View File
@@ -318,7 +318,7 @@ delete_note("path/to/file.md")
note_file_path = None
try:
# Resolve identifier to entity ID
entity_id = await knowledge_client.resolve_entity(identifier)
entity_id = await knowledge_client.resolve_entity(identifier, strict=True)
if output_format == "json":
entity = await knowledge_client.get_entity(entity_id)
note_title = entity.title
+1 -1
View File
@@ -293,7 +293,7 @@ async def edit_note(
# Try to resolve the entity; for append/prepend, create it if not found
try:
entity_id = await knowledge_client.resolve_entity(identifier)
entity_id = await knowledge_client.resolve_entity(identifier, strict=True)
except Exception as resolve_error:
# Trigger: entity does not exist yet
# Why: append/prepend can meaningfully create a new note from the content,
+21 -2
View File
@@ -6,6 +6,7 @@ from typing import Optional, Literal
from loguru import logger
from fastmcp import Context
from mcp.server.fastmcp.exceptions import ToolError
from basic_memory.mcp.server import mcp
from basic_memory.mcp.project_context import get_project_client
@@ -637,7 +638,7 @@ move_note("path/to/file.md", "{destination_path}/file.md")
"""Resolve and cache the source entity ID for the duration of this move."""
nonlocal resolved_entity_id
if resolved_entity_id is None:
resolved_entity_id = await knowledge_client.resolve_entity(identifier)
resolved_entity_id = await knowledge_client.resolve_entity(identifier, strict=True)
return resolved_entity_id
try:
@@ -645,8 +646,26 @@ move_note("path/to/file.md", "{destination_path}/file.md")
source_entity = await knowledge_client.get_entity(resolved_entity_id)
if "." in source_entity.file_path:
source_ext = source_entity.file_path.split(".")[-1]
except ToolError as e:
# Trigger: strict=True resolve_entity raised because the entity was not found.
# Why: fail fast with a formatted error instead of silently falling through
# to extension defaults and failing later with a confusing message.
# Outcome: move_note returns a user-facing not-found error immediately.
logger.error(f"Move failed for '{identifier}' to '{destination_path}': {e}")
if output_format == "json":
return {
"moved": False,
"title": None,
"permalink": None,
"file_path": None,
"source": identifier,
"destination": destination_path,
"error": str(e),
}
return _format_move_error_response(str(e), identifier, destination_path)
except Exception as e:
# If we can't fetch source metadata, continue with extension defaults.
# If we can't fetch source metadata (e.g. get_entity or file_path parsing fails),
# continue with extension defaults — the entity was at least resolved.
logger.debug(f"Could not fetch source entity for extension check: {e}")
# --- Resolve destination_folder into destination_path ---
+5 -3
View File
@@ -140,10 +140,12 @@ def validate_timeframe(timeframe: str) -> str:
if parsed > now:
raise ValueError("Timeframe cannot be in the future") # pragma: no cover
# Could format the duration back to our standard format
days = (now - parsed).days
# Round to nearest day to handle DST transitions where an hour shift
# can cause e.g. "7d" to compute as 6 days + 23 hours
total_seconds = (now - parsed).total_seconds()
days = round(total_seconds / 86400)
# Could enforce reasonable limits
# Enforce reasonable limits
if days > 365:
raise ValueError("Timeframe should be <= 1 year")