Merge branch 'main' into v0.15.0-release-docs

phernandez · phernandez · commit 87504e99695b · 2025-10-05T11:02:44.000-05:00
diff --git a/Dockerfile b/Dockerfile
@@ -24,11 +24,12 @@ WORKDIR /app
 RUN uv sync --locked
 
 # Create necessary directories and set ownership
-RUN mkdir -p /app/data /app/.basic-memory && \
+RUN mkdir -p /app/data/basic-memory /app/.basic-memory && \
     chown -R appuser:${GID} /app
 
 # Set default data directory and add venv to PATH
-ENV BASIC_MEMORY_HOME=/app/data \
+ENV BASIC_MEMORY_HOME=/app/data/basic-memory \
+    BASIC_MEMORY_PROJECT_ROOT=/app/data \
     PATH="/app/.venv/bin:$PATH"
 
 # Switch to the non-root user
diff --git a/src/basic_memory/api/routers/project_router.py b/src/basic_memory/api/routers/project_router.py
@@ -194,6 +194,7 @@ async def add_project(
         Response confirming the project was added
     """
     try:  # pragma: no cover
+        # The service layer now handles cloud mode validation and path sanitization
         await project_service.add_project(
             project_data.name, project_data.path, set_default=project_data.set_default
         )
diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py
@@ -103,6 +103,12 @@ class BasicMemoryConfig(BaseSettings):
         description="Skip expensive initialization synchronization. Useful for cloud/stateless deployments where project reconciliation is not needed.",
     )
 
+    # Project path constraints
+    project_root: Optional[str] = Field(
+        default=None,
+        description="If set, all projects must be created underneath this directory. Paths will be sanitized and constrained to this root. If not set, projects can be created anywhere (default behavior).",
+    )
+
     # API connection configuration
     api_url: Optional[str] = Field(
         default=None,
@@ -232,6 +238,10 @@ def data_dir_path(self):
         return Path.home() / DATA_DIR_NAME
 
 
+# Module-level cache for configuration
+_CONFIG_CACHE: Optional[BasicMemoryConfig] = None
+
+
 class ConfigManager:
     """Manages Basic Memory configuration."""
 
@@ -253,12 +263,45 @@ def config(self) -> BasicMemoryConfig:
         return self.load_config()
 
     def load_config(self) -> BasicMemoryConfig:
-        """Load configuration from file or create default."""
+        """Load configuration from file or create default.
+
+        Environment variables take precedence over file config values,
+        following Pydantic Settings best practices.
+
+        Uses module-level cache for performance across ConfigManager instances.
+        """
+        global _CONFIG_CACHE
+
+        # Return cached config if available
+        if _CONFIG_CACHE is not None:
+            return _CONFIG_CACHE
 
         if self.config_file.exists():
             try:
-                data = json.loads(self.config_file.read_text(encoding="utf-8"))
-                return BasicMemoryConfig(**data)
+                file_data = json.loads(self.config_file.read_text(encoding="utf-8"))
+
+                # First, create config from environment variables (Pydantic will read them)
+                # Then overlay with file data for fields that aren't set via env vars
+                # This ensures env vars take precedence
+
+                # Get env-based config fields that are actually set
+                env_config = BasicMemoryConfig()
+                env_dict = env_config.model_dump()
+
+                # Merge: file data as base, but only use it for fields not set by env
+                # We detect env-set fields by comparing to default values
+                merged_data = file_data.copy()
+
+                # For fields that have env var overrides, use those instead of file values
+                # The env_prefix is "BASIC_MEMORY_" so we check those
+                for field_name in BasicMemoryConfig.model_fields.keys():
+                    env_var_name = f"BASIC_MEMORY_{field_name.upper()}"
+                    if env_var_name in os.environ:
+                        # Environment variable is set, use it
+                        merged_data[field_name] = env_dict[field_name]
+
+                _CONFIG_CACHE = BasicMemoryConfig(**merged_data)
+                return _CONFIG_CACHE
             except Exception as e:  # pragma: no cover
                 logger.exception(f"Failed to load config: {e}")
                 raise e
@@ -268,8 +311,11 @@ def load_config(self) -> BasicMemoryConfig:
             return config
 
     def save_config(self, config: BasicMemoryConfig) -> None:
-        """Save configuration to file."""
+        """Save configuration to file and invalidate cache."""
+        global _CONFIG_CACHE
         save_basic_memory_config(self.config_file, config)
+        # Invalidate cache so next load_config() reads fresh data
+        _CONFIG_CACHE = None
 
     @property
     def projects(self) -> Dict[str, str]:
diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py
@@ -99,14 +99,31 @@ async def add_project(self, name: str, path: str, set_default: bool = False) ->
         Raises:
             ValueError: If the project already exists
         """
-        # in cloud mode, don't allow arbitrary paths.
-        if config.cloud_mode:
-            basic_memory_home = os.getenv("BASIC_MEMORY_HOME")
-            assert basic_memory_home is not None
-            base_path = Path(basic_memory_home)
-
-            # Resolve to absolute path
-            resolved_path = Path(os.path.abspath(os.path.expanduser(base_path / path))).as_posix()
+        # If project_root is set, constrain all projects to that directory
+        project_root = self.config_manager.config.project_root
+        if project_root:
+            base_path = Path(project_root)
+
+            # Sanitize the input path
+            # Strip leading slashes, home directory references, and parent directory references
+            clean_path = path.lstrip("/").replace("~/", "").replace("~", "")
+
+            # Remove any parent directory traversal attempts
+            path_parts = []
+            for part in clean_path.split("/"):
+                if part and part != "." and part != "..":
+                    path_parts.append(part)
+            clean_path = "/".join(path_parts) if path_parts else ""
+
+            # Construct path relative to project_root
+            resolved_path = (base_path / clean_path).resolve().as_posix()
+
+            # Verify the resolved path is actually under project_root
+            if not resolved_path.startswith(base_path.resolve().as_posix()):
+                raise ValueError(
+                    f"BASIC_MEMORY_PROJECT_ROOT is set to {project_root}. "
+                    f"All projects must be created under this directory. Invalid path: {path}"
+                )
         else:
             resolved_path = Path(os.path.abspath(os.path.expanduser(path))).as_posix()
 
diff --git a/test-int/test_disable_permalinks_integration.py b/test-int/test_disable_permalinks_integration.py
@@ -1,12 +1,9 @@
 """Integration tests for the disable_permalinks configuration."""
 
 import pytest
-from pathlib import Path
-from textwrap import dedent
 
 from basic_memory.config import BasicMemoryConfig
 from basic_memory.markdown import EntityParser, MarkdownProcessor
-from basic_memory.models import Project
 from basic_memory.repository import (
     EntityRepository,
     ObservationRepository,
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -78,6 +78,11 @@ def app_config(config_home, tmp_path, monkeypatch) -> BasicMemoryConfig:
 def config_manager(
     app_config: BasicMemoryConfig, project_config: ProjectConfig, config_home: Path, monkeypatch
 ) -> ConfigManager:
+    # Invalidate config cache to ensure clean state for each test
+    from basic_memory import config as config_module
+
+    config_module._CONFIG_CACHE = None
+
     # Create a new ConfigManager that uses the test home directory
     config_manager = ConfigManager()
     # Update its paths to use the test directory
diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py
@@ -714,3 +714,156 @@ async def test_synchronize_projects_handles_case_sensitivity_bug(
                     db_project = await project_service.repository.get_by_name(name)
                     if db_project:
                         await project_service.repository.delete(db_project.id)
+
+
+@pytest.mark.skipif(os.name == "nt", reason="Project root constraints only tested on POSIX systems")
+@pytest.mark.asyncio
+async def test_add_project_with_project_root_sanitizes_paths(
+    project_service: ProjectService, config_manager: ConfigManager, tmp_path, monkeypatch
+):
+    """Test that BASIC_MEMORY_PROJECT_ROOT sanitizes and validates project paths."""
+    # Set up project root environment
+    project_root_path = tmp_path / "app" / "data"
+    project_root_path.mkdir(parents=True, exist_ok=True)
+
+    monkeypatch.setenv("BASIC_MEMORY_PROJECT_ROOT", str(project_root_path))
+
+    # Invalidate config cache so it picks up the new env var
+    from basic_memory import config as config_module
+
+    config_module._CONFIG_CACHE = None
+
+    test_cases = [
+        # (input_path, expected_result_path, should_succeed)
+        ("test", str(project_root_path / "test"), True),  # Simple relative path
+        ("~/Documents/test", str(project_root_path / "Documents" / "test"), True),  # Home directory
+        (
+            "/tmp/test",
+            str(project_root_path / "tmp" / "test"),
+            True,
+        ),  # Absolute path (sanitized to relative)
+        (
+            "../../../etc/passwd",
+            str(project_root_path),
+            True,
+        ),  # Path traversal (all ../ removed, results in project_root)
+        ("folder/subfolder", str(project_root_path / "folder" / "subfolder"), True),  # Nested path
+        (
+            "~/folder/../test",
+            str(project_root_path / "test"),
+            True,
+        ),  # Mixed patterns (sanitized to just 'test')
+    ]
+
+    for i, (input_path, expected_path, should_succeed) in enumerate(test_cases):
+        test_project_name = f"project-root-test-{i}"
+
+        try:
+            # Add the project
+            await project_service.add_project(test_project_name, input_path)
+
+            if should_succeed:
+                # Verify the path was sanitized correctly
+                assert test_project_name in project_service.projects
+                actual_path = project_service.projects[test_project_name]
+
+                # The path should be under project_root
+                assert actual_path.startswith(str(project_root_path)), (
+                    f"Path {actual_path} should start with {project_root_path} for input {input_path}"
+                )
+
+                # Clean up
+                await project_service.remove_project(test_project_name)
+            else:
+                pytest.fail(f"Expected ValueError for input path: {input_path}")
+
+        except ValueError as e:
+            if should_succeed:
+                pytest.fail(f"Unexpected ValueError for input path {input_path}: {e}")
+            # Expected failure - continue to next test case
+
+
+@pytest.mark.skipif(os.name == "nt", reason="Project root constraints only tested on POSIX systems")
+@pytest.mark.asyncio
+async def test_add_project_with_project_root_rejects_escape_attempts(
+    project_service: ProjectService, config_manager: ConfigManager, tmp_path, monkeypatch
+):
+    """Test that BASIC_MEMORY_PROJECT_ROOT rejects paths that try to escape the project root."""
+    # Set up project root environment
+    project_root_path = tmp_path / "app" / "data"
+    project_root_path.mkdir(parents=True, exist_ok=True)
+
+    # Create a directory outside project_root to verify it's not accessible
+    outside_dir = tmp_path / "outside"
+    outside_dir.mkdir(parents=True, exist_ok=True)
+
+    monkeypatch.setenv("BASIC_MEMORY_PROJECT_ROOT", str(project_root_path))
+
+    # Invalidate config cache so it picks up the new env var
+    from basic_memory import config as config_module
+
+    config_module._CONFIG_CACHE = None
+
+    # All of these should succeed by being sanitized to paths under project_root
+    # The sanitization removes dangerous patterns, so they don't escape
+    safe_after_sanitization = [
+        "../../../etc/passwd",
+        "../../.env",
+        "../../../home/user/.ssh/id_rsa",
+    ]
+
+    for i, attack_path in enumerate(safe_after_sanitization):
+        test_project_name = f"project-root-attack-test-{i}"
+
+        try:
+            # Add the project
+            await project_service.add_project(test_project_name, attack_path)
+
+            # Verify it was sanitized to be under project_root
+            actual_path = project_service.projects[test_project_name]
+            assert actual_path.startswith(str(project_root_path)), (
+                f"Sanitized path {actual_path} should be under {project_root_path}"
+            )
+
+            # Clean up
+            await project_service.remove_project(test_project_name)
+
+        except ValueError:
+            # If it raises ValueError, that's also acceptable for security
+            pass
+
+
+@pytest.mark.skipif(os.name == "nt", reason="Project root constraints only tested on POSIX systems")
+@pytest.mark.asyncio
+async def test_add_project_without_project_root_allows_arbitrary_paths(
+    project_service: ProjectService, config_manager: ConfigManager, tmp_path, monkeypatch
+):
+    """Test that without BASIC_MEMORY_PROJECT_ROOT set, arbitrary paths are allowed."""
+    # Ensure project_root is not set
+    if "BASIC_MEMORY_PROJECT_ROOT" in os.environ:
+        monkeypatch.delenv("BASIC_MEMORY_PROJECT_ROOT")
+
+    # Force reload config without project_root
+    from basic_memory.services import project_service as ps_module
+
+    monkeypatch.setattr(ps_module, "config", config_manager.load_config())
+
+    # Create a test directory
+    test_dir = tmp_path / "arbitrary-location"
+    test_dir.mkdir(parents=True, exist_ok=True)
+
+    test_project_name = "no-project-root-test"
+
+    try:
+        # Without project_root, we should be able to use arbitrary absolute paths
+        await project_service.add_project(test_project_name, str(test_dir))
+
+        # Verify the path was accepted as-is
+        assert test_project_name in project_service.projects
+        actual_path = project_service.projects[test_project_name]
+        assert actual_path == str(test_dir)
+
+    finally:
+        # Clean up
+        if test_project_name in project_service.projects:
+            await project_service.remove_project(test_project_name)

Original file line number	Diff line number	Diff line change
`@@ -194,6 +194,7 @@ async def add_project(`
`194`	`194`	`Response confirming the project was added`
`195`	`195`	`"""`
`196`	`196`	`try: # pragma: no cover`
	`197`	`+ # The service layer now handles cloud mode validation and path sanitization`
`197`	`198`	`await project_service.add_project(`
`198`	`199`	`project_data.name, project_data.path, set_default=project_data.set_default`
`199`	`200`	`)`