Skip to content

Commit b74855c

Browse files
phernandezclaude
andcommitted
fix: prevent nested project paths to avoid data conflicts
Implements validation to reject project creation when paths would be nested (one project inside another). This prevents file ownership conflicts, data isolation issues, and phantom project problems. Changes: - Add _check_nested_paths() helper to detect nested paths - Validate against all existing projects in add_project() - Works in both local and cloud modes - Add comprehensive unit and integration tests - Fix test isolation issues using tempfile.TemporaryDirectory() - Handle macOS path resolution (/var -> /private/var symlinks) Fixes basicmachines-co/basic-memory-cloud#107 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 07e304c commit b74855c

8 files changed

Lines changed: 1355 additions & 991 deletions

src/basic_memory/services/project_service.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,37 @@ async def get_project(self, name: str) -> Optional[Project]:
8888
name
8989
)
9090

91+
def _check_nested_paths(self, path1: str, path2: str) -> bool:
92+
"""Check if two paths are nested (one is a prefix of the other).
93+
94+
Args:
95+
path1: First path to compare
96+
path2: Second path to compare
97+
98+
Returns:
99+
True if one path is nested within the other, False otherwise
100+
"""
101+
# Normalize paths to ensure proper comparison
102+
p1 = Path(path1).resolve()
103+
p2 = Path(path2).resolve()
104+
105+
# Check if either path is a parent of the other
106+
try:
107+
# Check if p2 is under p1
108+
p2.relative_to(p1)
109+
return True
110+
except ValueError:
111+
pass
112+
113+
try:
114+
# Check if p1 is under p2
115+
p1.relative_to(p2)
116+
return True
117+
except ValueError:
118+
pass
119+
120+
return False
121+
91122
async def add_project(self, name: str, path: str, set_default: bool = False) -> None:
92123
"""Add a new project to the configuration and database.
93124
@@ -142,6 +173,40 @@ async def add_project(self, name: str, path: str, set_default: bool = False) ->
142173
else:
143174
resolved_path = Path(os.path.abspath(os.path.expanduser(path))).as_posix()
144175

176+
# Check for nested paths with existing projects
177+
existing_projects = await self.list_projects()
178+
for existing in existing_projects:
179+
if self._check_nested_paths(resolved_path, existing.path):
180+
# Determine which path is nested within which
181+
p_new = Path(resolved_path).resolve()
182+
p_existing = Path(existing.path).resolve()
183+
184+
try:
185+
p_new.relative_to(p_existing)
186+
# New path is nested under existing project
187+
raise ValueError(
188+
f"Cannot create project at '{resolved_path}': "
189+
f"path is nested within existing project '{existing.name}' at '{existing.path}'. "
190+
f"Projects cannot share directory trees."
191+
)
192+
except ValueError as e:
193+
# Re-raise if it's the error we just raised, otherwise try the other direction
194+
if "Cannot create project" in str(e):
195+
raise
196+
197+
try:
198+
p_existing.relative_to(p_new)
199+
# Existing project is nested under new path
200+
raise ValueError(
201+
f"Cannot create project at '{resolved_path}': "
202+
f"existing project '{existing.name}' at '{existing.path}' is nested within this path. "
203+
f"Projects cannot share directory trees."
204+
)
205+
except ValueError as e:
206+
# Re-raise if it's the error we just raised
207+
if "Cannot create project" in str(e):
208+
raise
209+
145210
# First add to config file (this will validate the project doesn't exist)
146211
project_config = self.config_manager.add_project(name, resolved_path)
147212

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
"""Integration tests for project CLI commands."""
22

3+
import tempfile
4+
from pathlib import Path
5+
36
from typer.testing import CliRunner
47

58
from basic_memory.cli.main import app
@@ -52,61 +55,67 @@ def test_project_info_json(app_config, test_project, config_manager):
5255
assert "system" in data
5356

5457

55-
def test_project_add_and_remove(app_config, tmp_path, config_manager):
58+
def test_project_add_and_remove(app_config, config_manager):
5659
"""Test adding and removing a project."""
5760
runner = CliRunner()
58-
new_project_path = tmp_path / "new-project"
59-
new_project_path.mkdir()
6061

61-
# Add project
62-
result = runner.invoke(app, ["project", "add", "new-project", str(new_project_path)])
62+
# Use a separate temporary directory to avoid nested path conflicts
63+
with tempfile.TemporaryDirectory() as temp_dir:
64+
new_project_path = Path(temp_dir) / "new-project"
65+
new_project_path.mkdir()
6366

64-
if result.exit_code != 0:
65-
print(f"STDOUT: {result.stdout}")
66-
print(f"STDERR: {result.stderr}")
67-
assert result.exit_code == 0
68-
assert (
69-
"Project 'new-project' added successfully" in result.stdout
70-
or "added" in result.stdout.lower()
71-
)
67+
# Add project
68+
result = runner.invoke(app, ["project", "add", "new-project", str(new_project_path)])
7269

73-
# Verify it shows up in list
74-
result = runner.invoke(app, ["project", "list"])
75-
assert result.exit_code == 0
76-
assert "new-project" in result.stdout
70+
if result.exit_code != 0:
71+
print(f"STDOUT: {result.stdout}")
72+
print(f"STDERR: {result.stderr}")
73+
assert result.exit_code == 0
74+
assert (
75+
"Project 'new-project' added successfully" in result.stdout
76+
or "added" in result.stdout.lower()
77+
)
7778

78-
# Remove project
79-
result = runner.invoke(app, ["project", "remove", "new-project"])
80-
assert result.exit_code == 0
81-
assert "removed" in result.stdout.lower() or "deleted" in result.stdout.lower()
79+
# Verify it shows up in list
80+
result = runner.invoke(app, ["project", "list"])
81+
assert result.exit_code == 0
82+
assert "new-project" in result.stdout
83+
84+
# Remove project
85+
result = runner.invoke(app, ["project", "remove", "new-project"])
86+
assert result.exit_code == 0
87+
assert "removed" in result.stdout.lower() or "deleted" in result.stdout.lower()
8288

8389

84-
def test_project_set_default(app_config, tmp_path, config_manager):
90+
def test_project_set_default(app_config, config_manager):
8591
"""Test setting default project."""
8692
runner = CliRunner()
87-
new_project_path = tmp_path / "another-project"
88-
new_project_path.mkdir()
8993

90-
# Add a second project
91-
result = runner.invoke(app, ["project", "add", "another-project", str(new_project_path)])
92-
if result.exit_code != 0:
93-
print(f"STDOUT: {result.stdout}")
94-
print(f"STDERR: {result.stderr}")
95-
assert result.exit_code == 0
96-
97-
# Set as default
98-
result = runner.invoke(app, ["project", "default", "another-project"])
99-
if result.exit_code != 0:
100-
print(f"STDOUT: {result.stdout}")
101-
print(f"STDERR: {result.stderr}")
102-
assert result.exit_code == 0
103-
assert "default" in result.stdout.lower()
104-
105-
# Verify in list
106-
result = runner.invoke(app, ["project", "list"])
107-
assert result.exit_code == 0
108-
# The new project should have the checkmark now
109-
lines = result.stdout.split("\n")
110-
for line in lines:
111-
if "another-project" in line:
112-
assert "✓" in line
94+
# Use a separate temporary directory to avoid nested path conflicts
95+
with tempfile.TemporaryDirectory() as temp_dir:
96+
new_project_path = Path(temp_dir) / "another-project"
97+
new_project_path.mkdir()
98+
99+
# Add a second project
100+
result = runner.invoke(app, ["project", "add", "another-project", str(new_project_path)])
101+
if result.exit_code != 0:
102+
print(f"STDOUT: {result.stdout}")
103+
print(f"STDERR: {result.stderr}")
104+
assert result.exit_code == 0
105+
106+
# Set as default
107+
result = runner.invoke(app, ["project", "default", "another-project"])
108+
if result.exit_code != 0:
109+
print(f"STDOUT: {result.stdout}")
110+
print(f"STDERR: {result.stderr}")
111+
assert result.exit_code == 0
112+
assert "default" in result.stdout.lower()
113+
114+
# Verify in list
115+
result = runner.invoke(app, ["project", "list"])
116+
assert result.exit_code == 0
117+
# The new project should have the checkmark now
118+
lines = result.stdout.split("\n")
119+
for line in lines:
120+
if "another-project" in line:
121+
assert "✓" in line

test-int/mcp/test_project_management_integration.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,3 +512,42 @@ async def test_case_preservation_in_project_list(mcp_server, app, test_project):
512512
# Clean up - delete test projects
513513
for project_name in test_projects:
514514
await client.call_tool("delete_project", {"project_name": project_name})
515+
516+
517+
@pytest.mark.asyncio
518+
async def test_nested_project_paths_rejected(mcp_server, app, test_project):
519+
"""Test that creating nested project paths is rejected with clear error message."""
520+
521+
async with Client(mcp_server) as client:
522+
# Create a parent project
523+
parent_name = "parent-project"
524+
parent_path = "/tmp/nested-test/parent"
525+
526+
await client.call_tool(
527+
"create_memory_project",
528+
{
529+
"project_name": parent_name,
530+
"project_path": parent_path,
531+
},
532+
)
533+
534+
# Try to create a child project nested under the parent
535+
child_name = "child-project"
536+
child_path = "/tmp/nested-test/parent/child"
537+
538+
with pytest.raises(Exception) as exc_info:
539+
await client.call_tool(
540+
"create_memory_project",
541+
{
542+
"project_name": child_name,
543+
"project_path": child_path,
544+
},
545+
)
546+
547+
# Verify error message mentions nested paths
548+
error_message = str(exc_info.value)
549+
assert "nested" in error_message.lower()
550+
assert parent_name in error_message or parent_path in error_message
551+
552+
# Clean up parent project
553+
await client.call_tool("delete_project", {"project_name": parent_name})

0 commit comments

Comments
 (0)