diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index 6bafa0b4..1e297c1a 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -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}") diff --git a/test-int/mcp/test_move_note_integration.py b/test-int/mcp/test_move_note_integration.py index 6088a202..b2af3851 100644 --- a/test-int/mcp/test_move_note_integration.py +++ b/test-int/mcp/test_move_note_integration.py @@ -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 diff --git a/tests/mcp/test_tool_move_note.py b/tests/mcp/test_tool_move_note.py index 78d86544..27e0009d 100644 --- a/tests/mcp/test_tool_move_note.py +++ b/tests/mcp/test_tool_move_note.py @@ -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