mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
feat: Add destination_folder parameter to move_note tool
Allows callers to move a note into a folder while preserving its original filename — no need for a separate read_note round-trip to extract the basename. - destination_folder is mutually exclusive with destination_path - Rejected for directory moves (is_directory=True) - Uses Path().name and PureWindowsPath().as_posix() for cross-platform compat - Validates resolved path against path traversal attacks - Includes formatting fixes in promo.py and test_analytics.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
"""Move note tool for Basic Memory MCP server."""
|
||||
|
||||
from pathlib import Path, PureWindowsPath
|
||||
from textwrap import dedent
|
||||
from typing import Optional, Literal
|
||||
|
||||
@@ -345,7 +346,8 @@ delete_note("{identifier}")
|
||||
)
|
||||
async def move_note(
|
||||
identifier: str,
|
||||
destination_path: str,
|
||||
destination_path: str = "",
|
||||
destination_folder: Optional[str] = None,
|
||||
is_directory: bool = False,
|
||||
project: Optional[str] = None,
|
||||
workspace: Optional[str] = None,
|
||||
@@ -365,6 +367,9 @@ async def move_note(
|
||||
Use search_notes() or list_directory() first to find the correct path if uncertain.
|
||||
destination_path: For files: new path relative to project root (e.g., "work/meetings/note.md")
|
||||
For directories: new directory path (e.g., "archive/docs")
|
||||
Mutually exclusive with destination_folder.
|
||||
destination_folder: Move the note into this folder, preserving the original filename.
|
||||
Mutually exclusive with destination_path. Only for single-file moves.
|
||||
is_directory: If True, moves an entire directory and all its contents.
|
||||
When True, identifier and destination_path should be directory paths
|
||||
(without file extensions). Defaults to False.
|
||||
@@ -385,6 +390,9 @@ async def move_note(
|
||||
# Move by exact permalink
|
||||
move_note("my-note-permalink", "archive/old-notes/my-note.md")
|
||||
|
||||
# Move note to archive folder (filename preserved automatically)
|
||||
move_note("my-note", destination_folder="archive")
|
||||
|
||||
# Move with complex path structure
|
||||
move_note("experiments/ml-results", "archive/2025/ml-experiments.md")
|
||||
|
||||
@@ -415,6 +423,57 @@ async def move_note(
|
||||
- Re-indexes the entity for search
|
||||
- Maintains all observations and relations
|
||||
"""
|
||||
# --- Parameter Validation ---
|
||||
# Trigger: both destination_path and destination_folder provided
|
||||
# Why: they are mutually exclusive — one specifies full path, the other just the folder
|
||||
# Outcome: early error before any entity resolution or API calls
|
||||
if destination_folder and destination_path:
|
||||
error_msg = (
|
||||
"Cannot specify both destination_path and destination_folder. Use one or the other."
|
||||
)
|
||||
if output_format == "json":
|
||||
return {
|
||||
"moved": False,
|
||||
"title": None,
|
||||
"permalink": None,
|
||||
"file_path": None,
|
||||
"source": identifier,
|
||||
"destination": None,
|
||||
"error": "MUTUALLY_EXCLUSIVE_PARAMS",
|
||||
}
|
||||
return f"# Move Failed - Invalid Parameters\n\n{error_msg}"
|
||||
|
||||
if not destination_folder and not destination_path:
|
||||
error_msg = "Either destination_path or destination_folder must be provided."
|
||||
if output_format == "json":
|
||||
return {
|
||||
"moved": False,
|
||||
"title": None,
|
||||
"permalink": None,
|
||||
"file_path": None,
|
||||
"source": identifier,
|
||||
"destination": None,
|
||||
"error": "MISSING_DESTINATION",
|
||||
}
|
||||
return f"# Move Failed - Missing Destination\n\n{error_msg}"
|
||||
|
||||
# Trigger: destination_folder used with is_directory=True
|
||||
# Why: destination_folder preserves a single file's name — meaningless for directory moves
|
||||
if destination_folder and is_directory:
|
||||
error_msg = (
|
||||
"destination_folder is only supported for single-file moves, not directory moves."
|
||||
)
|
||||
if output_format == "json":
|
||||
return {
|
||||
"moved": False,
|
||||
"title": None,
|
||||
"permalink": None,
|
||||
"file_path": None,
|
||||
"source": identifier,
|
||||
"destination": None,
|
||||
"error": "DESTINATION_FOLDER_NOT_FOR_DIRECTORIES",
|
||||
}
|
||||
return f"# Move Failed - Invalid Parameters\n\n{error_msg}"
|
||||
async with get_project_client(project, workspace, context) as (client, active_project):
|
||||
logger.debug(
|
||||
f"Moving {'directory' if is_directory else 'note'}: {identifier} to {destination_path} in project: {active_project.name}"
|
||||
@@ -589,6 +648,66 @@ move_note("path/to/file.md", "{destination_path}/file.md")
|
||||
# If we can't fetch source metadata, continue with extension defaults.
|
||||
logger.debug(f"Could not fetch source entity for extension check: {e}")
|
||||
|
||||
# --- Resolve destination_folder into destination_path ---
|
||||
# Trigger: caller passed destination_folder instead of destination_path
|
||||
# Why: extract the original filename from the resolved entity so callers
|
||||
# don't need a separate read_note round-trip
|
||||
# Outcome: destination_path is set to folder/original-filename.ext
|
||||
if destination_folder is not None:
|
||||
if source_entity is None:
|
||||
error_msg = (
|
||||
f"Could not resolve source entity '{identifier}' to extract filename "
|
||||
f"for destination_folder. Use destination_path with an explicit filename instead."
|
||||
)
|
||||
if output_format == "json":
|
||||
return {
|
||||
"moved": False,
|
||||
"title": None,
|
||||
"permalink": None,
|
||||
"file_path": None,
|
||||
"source": identifier,
|
||||
"destination": None,
|
||||
"error": "ENTITY_RESOLUTION_FAILED",
|
||||
}
|
||||
return f"# Move Failed - Entity Resolution Failed\n\n{error_msg}"
|
||||
|
||||
source_filename = Path(source_entity.file_path).name
|
||||
# Normalize backslashes to forward slashes for Windows compatibility,
|
||||
# then strip leading/trailing separators
|
||||
folder = PureWindowsPath(destination_folder).as_posix().strip("/")
|
||||
destination_path = f"{folder}/{source_filename}" if folder else source_filename
|
||||
|
||||
# Validate resolved path to prevent path traversal via destination_folder
|
||||
if not validate_project_path(destination_path, project_path):
|
||||
logger.warning(
|
||||
"Attempted path traversal attack blocked via destination_folder",
|
||||
destination_folder=destination_folder,
|
||||
project=active_project.name,
|
||||
)
|
||||
if output_format == "json":
|
||||
return {
|
||||
"moved": False,
|
||||
"title": None,
|
||||
"permalink": None,
|
||||
"file_path": None,
|
||||
"source": identifier,
|
||||
"destination": destination_path,
|
||||
"error": "SECURITY_VALIDATION_ERROR",
|
||||
}
|
||||
return f"""# Move Failed - Security Validation Error
|
||||
|
||||
The destination folder '{destination_folder}' is not allowed - paths must stay within project boundaries.
|
||||
|
||||
## Valid folder examples:
|
||||
- `notes`
|
||||
- `projects/2025`
|
||||
- `archive/old-notes`
|
||||
|
||||
## Try again with a safe folder:
|
||||
```
|
||||
move_note("{identifier}", destination_folder="notes")
|
||||
```"""
|
||||
|
||||
# Validate that destination path includes a file extension
|
||||
if "." not in destination_path or not destination_path.split(".")[-1]:
|
||||
logger.warning(f"Move failed - no file extension provided: {destination_path}")
|
||||
|
||||
@@ -639,3 +639,80 @@ async def test_move_note_normal_moves_still_work(mcp_server, app, test_project):
|
||||
|
||||
content = read_result.content[0].text
|
||||
assert "This should move normally" in content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_with_destination_folder(mcp_server, app, test_project):
|
||||
"""Test moving a note using destination_folder to preserve the original filename."""
|
||||
|
||||
async with Client(mcp_server) as client:
|
||||
# Create a note to move
|
||||
await client.call_tool(
|
||||
"write_note",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"title": "Folder Move Integration",
|
||||
"directory": "source",
|
||||
"content": "# Folder Move Integration\n\nTesting destination_folder parameter.",
|
||||
"tags": "test,folder-move",
|
||||
},
|
||||
)
|
||||
|
||||
# Move using destination_folder (filename preserved automatically)
|
||||
move_result = await client.call_tool(
|
||||
"move_note",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"identifier": "Folder Move Integration",
|
||||
"destination_folder": "archive/2025",
|
||||
},
|
||||
)
|
||||
|
||||
# Should return successful move message
|
||||
assert len(move_result.content) == 1
|
||||
move_text = move_result.content[0].text
|
||||
assert "✅ Note moved successfully" in move_text
|
||||
assert "Folder Move Integration" in move_text
|
||||
|
||||
# Verify the note can be read from its new location (original filename preserved)
|
||||
read_result = await client.call_tool(
|
||||
"read_note",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"identifier": "archive/2025/folder-move-integration",
|
||||
},
|
||||
)
|
||||
|
||||
content = read_result.content[0].text
|
||||
assert "Testing destination_folder parameter" in content
|
||||
|
||||
# Verify the original location no longer works
|
||||
read_original = await client.call_tool(
|
||||
"read_note",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"identifier": "source/folder-move-integration",
|
||||
},
|
||||
)
|
||||
assert "Note Not Found" in read_original.content[0].text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_mutually_exclusive(mcp_server, app, test_project):
|
||||
"""Test that providing both destination_path and destination_folder returns an error."""
|
||||
|
||||
async with Client(mcp_server) as client:
|
||||
move_result = await client.call_tool(
|
||||
"move_note",
|
||||
{
|
||||
"project": test_project.name,
|
||||
"identifier": "some-note",
|
||||
"destination_path": "target/note.md",
|
||||
"destination_folder": "target",
|
||||
},
|
||||
)
|
||||
|
||||
assert len(move_result.content) == 1
|
||||
error_text = move_result.content[0].text
|
||||
assert "# Move Failed - Invalid Parameters" in error_text
|
||||
assert "Cannot specify both" in error_text
|
||||
|
||||
@@ -911,3 +911,223 @@ class TestMoveNoteSecurityValidation:
|
||||
assert isinstance(result, str)
|
||||
# Should NOT contain security error message
|
||||
assert "Security Validation Error" not in result
|
||||
|
||||
|
||||
class TestMoveNoteDestinationFolder:
|
||||
"""Test the destination_folder parameter for move_note."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_with_destination_folder(self, client, test_project):
|
||||
"""Test moving a note using destination_folder preserves the original filename."""
|
||||
await write_note.fn(
|
||||
project=test_project.name,
|
||||
title="Folder Move Test",
|
||||
directory="source",
|
||||
content="# Folder Move Test\nContent for folder move.",
|
||||
)
|
||||
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="source/folder-move-test",
|
||||
destination_folder="archive",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "✅ Note moved successfully" in result
|
||||
|
||||
# Verify note exists at archive/Folder Move Test.md (original filename preserved)
|
||||
content = await read_note.fn("archive/folder-move-test", project=test_project.name)
|
||||
assert "# Folder Move Test" in content
|
||||
assert "Content for folder move" in content
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_with_nested_destination_folder(self, client, test_project):
|
||||
"""Test moving a note into a nested folder structure."""
|
||||
await write_note.fn(
|
||||
project=test_project.name,
|
||||
title="Nested Folder Test",
|
||||
directory="source",
|
||||
content="# Nested Folder Test\nNested folder content.",
|
||||
)
|
||||
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="source/nested-folder-test",
|
||||
destination_folder="archive/2025/q1",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "✅ Note moved successfully" in result
|
||||
|
||||
content = await read_note.fn(
|
||||
"archive/2025/q1/nested-folder-test", project=test_project.name
|
||||
)
|
||||
assert "# Nested Folder Test" in content
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_strips_slashes(self, client, test_project):
|
||||
"""Test that leading/trailing slashes are stripped from destination_folder."""
|
||||
await write_note.fn(
|
||||
project=test_project.name,
|
||||
title="Slash Strip Test",
|
||||
directory="source",
|
||||
content="# Slash Strip Test\nSlash stripping content.",
|
||||
)
|
||||
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="source/slash-strip-test",
|
||||
destination_folder="/archive/notes/",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "✅ Note moved successfully" in result
|
||||
|
||||
content = await read_note.fn("archive/notes/slash-strip-test", project=test_project.name)
|
||||
assert "# Slash Strip Test" in content
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_both_params_error(self, client, test_project):
|
||||
"""Test that providing both destination_path and destination_folder is an error."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="some-note",
|
||||
destination_path="target/note.md",
|
||||
destination_folder="target",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Move Failed - Invalid Parameters" in result
|
||||
assert "Cannot specify both" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_both_params_error_json(self, client, test_project):
|
||||
"""Test JSON output when both destination_path and destination_folder are provided."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="some-note",
|
||||
destination_path="target/note.md",
|
||||
destination_folder="target",
|
||||
output_format="json",
|
||||
)
|
||||
|
||||
assert isinstance(result, dict)
|
||||
assert result["moved"] is False
|
||||
assert result["error"] == "MUTUALLY_EXCLUSIVE_PARAMS"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_neither_param_error(self, client, test_project):
|
||||
"""Test that providing neither destination_path nor destination_folder is an error."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="some-note",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Move Failed - Missing Destination" in result
|
||||
assert "Either destination_path or destination_folder must be provided" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_neither_param_error_json(self, client, test_project):
|
||||
"""Test JSON output when neither param is provided."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="some-note",
|
||||
output_format="json",
|
||||
)
|
||||
|
||||
assert isinstance(result, dict)
|
||||
assert result["moved"] is False
|
||||
assert result["error"] == "MISSING_DESTINATION"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_with_is_directory_error(self, client, test_project):
|
||||
"""Test that destination_folder is rejected for directory moves."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="some-dir",
|
||||
destination_folder="archive",
|
||||
is_directory=True,
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Move Failed - Invalid Parameters" in result
|
||||
assert "only supported for single-file moves" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_with_is_directory_error_json(
|
||||
self, client, test_project
|
||||
):
|
||||
"""Test JSON output when destination_folder is used with is_directory."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="some-dir",
|
||||
destination_folder="archive",
|
||||
is_directory=True,
|
||||
output_format="json",
|
||||
)
|
||||
|
||||
assert isinstance(result, dict)
|
||||
assert result["moved"] is False
|
||||
assert result["error"] == "DESTINATION_FOLDER_NOT_FOR_DIRECTORIES"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_nonexistent_note(self, client, test_project):
|
||||
"""Test destination_folder with a note that doesn't exist."""
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="nonexistent/note",
|
||||
destination_folder="archive",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Move Failed" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_json_output(self, client, test_project):
|
||||
"""Test JSON output for successful destination_folder move."""
|
||||
await write_note.fn(
|
||||
project=test_project.name,
|
||||
title="JSON Folder Test",
|
||||
directory="source",
|
||||
content="# JSON Folder Test\nJSON folder content.",
|
||||
)
|
||||
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="source/json-folder-test",
|
||||
destination_folder="archive",
|
||||
output_format="json",
|
||||
)
|
||||
|
||||
assert isinstance(result, dict)
|
||||
assert result["moved"] is True
|
||||
assert result["source"] == "source/json-folder-test"
|
||||
assert "archive" in result["destination"]
|
||||
assert result["file_path"] is not None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_destination_folder_path_traversal(self, client, test_project):
|
||||
"""Test that path traversal via destination_folder is blocked."""
|
||||
await write_note.fn(
|
||||
project=test_project.name,
|
||||
title="Traversal Test",
|
||||
directory="source",
|
||||
content="# Traversal Test\nSecurity test content.",
|
||||
)
|
||||
|
||||
attack_folders = [
|
||||
"../../../etc",
|
||||
"../../.ssh",
|
||||
"notes/../../../root",
|
||||
]
|
||||
|
||||
for attack_folder in attack_folders:
|
||||
result = await move_note.fn(
|
||||
project=test_project.name,
|
||||
identifier="source/traversal-test",
|
||||
destination_folder=attack_folder,
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "Security Validation Error" in result
|
||||
|
||||
Reference in New Issue
Block a user