feat(memory): implement path security validation
This commit is contained in:
@@ -0,0 +1,122 @@
|
||||
"""MemoryTool20250818 - Anthropic's native memory tool.
|
||||
|
||||
Enables Claude to create, read, update, and delete files in a persistent
|
||||
/memories directory across conversations.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any, Literal
|
||||
|
||||
from nanobot.agent.tools.anthropic.base import BaseAnthropicTool, CLIResult
|
||||
|
||||
|
||||
class MemoryTool20250818(BaseAnthropicTool):
|
||||
"""Anthropic's native memory_20250818 tool.
|
||||
|
||||
Client-side tool for persistent memory storage across conversations.
|
||||
All operations are restricted to the /memories directory.
|
||||
|
||||
Commands:
|
||||
- view: Show directory contents or file contents with line numbers
|
||||
- create: Create a new file with content
|
||||
- str_replace: Replace unique text occurrence in a file
|
||||
- insert: Insert text at a specific line number
|
||||
- delete: Delete a file or directory
|
||||
- rename: Rename or move a file/directory
|
||||
"""
|
||||
|
||||
api_type: Literal["memory_20250818"] = "memory_20250818"
|
||||
name: Literal["memory"] = "memory"
|
||||
beta_flag: str = "context-management-2025-06-27"
|
||||
|
||||
def __init__(self, workspace: Path):
|
||||
"""Initialize Memory tool.
|
||||
|
||||
Args:
|
||||
workspace: Root workspace directory
|
||||
"""
|
||||
self.workspace = workspace
|
||||
self.memories_dir = workspace / "memories"
|
||||
self.memories_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
def _validate_memory_path(self, path: str) -> Path:
|
||||
"""Validate and resolve path to prevent directory traversal.
|
||||
|
||||
Args:
|
||||
path: Path string starting with /memories
|
||||
|
||||
Returns:
|
||||
Validated absolute Path within memories directory
|
||||
|
||||
Raises:
|
||||
ValueError: If path is invalid or escapes /memories directory
|
||||
"""
|
||||
# Reject paths not starting with /memories
|
||||
if not path.startswith("/memories"):
|
||||
raise ValueError(f"Path must start with /memories, got: {path}")
|
||||
|
||||
# Resolve to absolute path within workspace
|
||||
# lstrip("/") removes leading slash: "/memories/file.txt" -> "memories/file.txt"
|
||||
relative_path = path.lstrip("/")
|
||||
full_path = (self.workspace / relative_path).resolve()
|
||||
|
||||
# Verify resolved path is within memories directory
|
||||
memories_dir_resolved = self.memories_dir.resolve()
|
||||
try:
|
||||
full_path.relative_to(memories_dir_resolved)
|
||||
except ValueError:
|
||||
raise ValueError(f"Path escapes /memories directory: {path}")
|
||||
|
||||
return full_path
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
command: Literal["view", "create", "str_replace", "insert", "delete", "rename"],
|
||||
path: str | None = None,
|
||||
old_path: str | None = None,
|
||||
new_path: str | None = None,
|
||||
file_text: str | None = None,
|
||||
old_str: str | None = None,
|
||||
new_str: str | None = None,
|
||||
insert_line: int | None = None,
|
||||
insert_text: str | None = None,
|
||||
view_range: list[int] | None = None,
|
||||
**kwargs: Any,
|
||||
) -> CLIResult:
|
||||
"""Execute memory command.
|
||||
|
||||
Args:
|
||||
command: Command to execute
|
||||
path: File/directory path (for view/create/str_replace/insert/delete)
|
||||
old_path: Source path (for rename)
|
||||
new_path: Destination path (for rename)
|
||||
file_text: File content (for create)
|
||||
old_str: Text to find (for str_replace)
|
||||
new_str: Replacement text (for str_replace)
|
||||
insert_line: Line number to insert at (for insert)
|
||||
insert_text: Text to insert (for insert)
|
||||
view_range: [start_line, end_line] for view
|
||||
**kwargs: Additional arguments (ignored)
|
||||
|
||||
Returns:
|
||||
CLIResult with command output or error
|
||||
"""
|
||||
# Placeholder - will implement in later tasks
|
||||
return CLIResult(
|
||||
exit_code=1,
|
||||
output="",
|
||||
error="Not implemented yet"
|
||||
)
|
||||
|
||||
def to_params(self) -> dict[str, Any]:
|
||||
"""Convert to Anthropic API tool parameter format.
|
||||
|
||||
Returns:
|
||||
Tool definition for Anthropic API
|
||||
"""
|
||||
return {
|
||||
"type": self.api_type,
|
||||
"name": self.name,
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
"""Security tests for MemoryTool20250818."""
|
||||
|
||||
import pytest
|
||||
from pathlib import Path
|
||||
from nanobot.agent.tools.anthropic import MemoryTool20250818
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_workspace(tmp_path):
|
||||
"""Create temporary workspace."""
|
||||
return tmp_path
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def memory_tool(temp_workspace):
|
||||
"""Create MemoryTool instance."""
|
||||
return MemoryTool20250818(workspace=temp_workspace)
|
||||
|
||||
|
||||
class TestPathSecurity:
|
||||
"""Test path validation security."""
|
||||
|
||||
def test_validate_path_valid_root(self, memory_tool):
|
||||
"""Test that /memories is valid."""
|
||||
result = memory_tool._validate_memory_path("/memories")
|
||||
assert result == memory_tool.memories_dir
|
||||
|
||||
def test_validate_path_valid_file(self, memory_tool):
|
||||
"""Test that /memories/notes.txt is valid."""
|
||||
result = memory_tool._validate_memory_path("/memories/notes.txt")
|
||||
assert result == memory_tool.memories_dir / "notes.txt"
|
||||
|
||||
def test_validate_path_valid_nested(self, memory_tool):
|
||||
"""Test that /memories/project/status.xml is valid."""
|
||||
result = memory_tool._validate_memory_path("/memories/project/status.xml")
|
||||
assert result == memory_tool.memories_dir / "project" / "status.xml"
|
||||
|
||||
def test_validate_path_rejects_parent_traversal(self, memory_tool):
|
||||
"""Test that ../ is rejected."""
|
||||
with pytest.raises(ValueError, match="escapes /memories directory"):
|
||||
memory_tool._validate_memory_path("/memories/../config.json")
|
||||
|
||||
def test_validate_path_rejects_double_parent_traversal(self, memory_tool):
|
||||
"""Test that ../../ is rejected."""
|
||||
with pytest.raises(ValueError, match="escapes /memories directory"):
|
||||
memory_tool._validate_memory_path("/memories/../../etc/passwd")
|
||||
|
||||
def test_validate_path_rejects_absolute_path(self, memory_tool):
|
||||
"""Test that absolute paths are rejected."""
|
||||
with pytest.raises(ValueError, match="must start with /memories"):
|
||||
memory_tool._validate_memory_path("/etc/passwd")
|
||||
|
||||
def test_validate_path_rejects_workspace_path(self, memory_tool):
|
||||
"""Test that /workspace paths are rejected."""
|
||||
with pytest.raises(ValueError, match="must start with /memories"):
|
||||
memory_tool._validate_memory_path("/workspace/data.txt")
|
||||
|
||||
def test_validate_path_rejects_relative_path(self, memory_tool):
|
||||
"""Test that relative paths are rejected."""
|
||||
with pytest.raises(ValueError, match="must start with /memories"):
|
||||
memory_tool._validate_memory_path("notes.txt")
|
||||
|
||||
def test_validate_path_url_encoded_is_safe(self, memory_tool):
|
||||
"""Test that URL-encoded paths are safe (not decoded by pathlib)."""
|
||||
# Python's pathlib treats %2e%2e as literal characters, not as ..
|
||||
# So this is actually safe - it creates a subdirectory named "%2e%2e"
|
||||
attack_path = "/memories/%2e%2e/config.json"
|
||||
result = memory_tool._validate_memory_path(attack_path)
|
||||
# This should resolve to memories/%2e%2e/config.json (literal characters)
|
||||
assert result == memory_tool.memories_dir / "%2e%2e" / "config.json"
|
||||
Reference in New Issue
Block a user