diff --git a/src/basic_memory/file_utils.py b/src/basic_memory/file_utils.py index 9c7ab7b1..3de80b29 100644 --- a/src/basic_memory/file_utils.py +++ b/src/basic_memory/file_utils.py @@ -447,6 +447,11 @@ def sanitize_for_filename(text: str, replacement: str = "-") -> str: # compress multiple, repeated replacements text = re.sub(f"{re.escape(replacement)}+", replacement, text) + # Strip trailing periods — they cause "hi-everyone..md" double-dot filenames + # when ".md" is appended, which triggers path traversal false positives. + # Trailing periods are also invalid on Windows filesystems. + text = text.strip(".") + return text.strip(replacement) diff --git a/src/basic_memory/utils.py b/src/basic_memory/utils.py index ba36a87e..34cc5357 100644 --- a/src/basic_memory/utils.py +++ b/src/basic_memory/utils.py @@ -503,12 +503,23 @@ def valid_project_path_value(path: str): if not path: return True - # Check for obvious path traversal patterns first - if ".." in path or "~" in path: + # Check for tilde (home directory expansion) + if "~" in path: return False - # Check for Windows-style path traversal (even on Unix systems) - if "\\.." in path or path.startswith("\\"): + # Check for ".." as a path segment (path traversal), not as a substring. + # Filenames like "hi-everyone..md" are legitimate and must not be blocked. + # Also block segments like ".. " and ".. ." because Windows normalizes + # trailing dots and spaces away, making them equivalent to "..". + segments = path.replace("\\", "/").split("/") + if any( + seg == ".." or (len(seg) > 2 and seg[:2] == ".." and all(c in ". " for c in seg[2:])) + for seg in segments + ): + return False + + # Check for Windows-style leading backslash + if path.startswith("\\"): return False # Block absolute paths (Unix-style starting with / or Windows-style with drive letters) diff --git a/tests/utils/test_file_utils.py b/tests/utils/test_file_utils.py index a2025687..d731bad3 100644 --- a/tests/utils/test_file_utils.py +++ b/tests/utils/test_file_utils.py @@ -198,6 +198,19 @@ def test_sanitize_for_filename_removes_invalid_characters(): assert char not in sanitized_text +def test_sanitize_for_filename_strips_trailing_periods(): + """Trailing periods cause double-dot filenames like 'hi-everyone..md'. + + This was a production bug where title "Hi everyone." produced file path + "hi-everyone..md" which failed path traversal validation. + """ + assert sanitize_for_filename("Hi everyone.") == "Hi everyone" + assert sanitize_for_filename("test...") == "test" + assert sanitize_for_filename(".hidden") == "hidden" + assert sanitize_for_filename("...dots...") == "dots" + assert sanitize_for_filename("normal title") == "normal title" + + @pytest.mark.parametrize( "input_directory,expected", [ diff --git a/tests/utils/test_validate_project_path.py b/tests/utils/test_validate_project_path.py index f5e87a6b..7c6ed767 100644 --- a/tests/utils/test_validate_project_path.py +++ b/tests/utils/test_validate_project_path.py @@ -191,6 +191,54 @@ def test_absolute_paths(self, tmp_path): assert not result, f"Absolute path '{path}' should be blocked" +class TestValidateProjectPathDoubleDotInFilename: + """Test that filenames containing '..' as part of the name are allowed.""" + + def test_double_dot_in_filename_allowed(self, tmp_path): + """Filenames like 'hi-everyone..md' should NOT be blocked. + + This was a production bug: a title ending with a period (e.g. "Hi everyone.") + produced a file path like "hi-everyone..md" which the old substring check + ('..' in path) incorrectly flagged as path traversal. + """ + project_path = tmp_path / "project" + project_path.mkdir() + + safe_paths_with_dots = [ + "hi-everyone..md", + "notes/hi-everyone..md", + "version-2..0.md", + "file...name.md", + "docs/report..final.txt", + ] + + for path in safe_paths_with_dots: + assert validate_project_path(path, project_path), ( + f"Path '{path}' with '..' in filename should be allowed" + ) + + def test_actual_traversal_still_blocked(self, tmp_path): + """Ensure '..' as a path segment is still blocked.""" + project_path = tmp_path / "project" + project_path.mkdir() + + attack_paths = [ + "../file.md", + "notes/../../etc/passwd", + "foo/../../../bar", + "..\\Windows\\System32", + # Windows normalizes trailing dots/spaces to ".." + ".. /file.md", + ".. ./file.md", + "notes/.. /etc/passwd", + ] + + for path in attack_paths: + assert not validate_project_path(path, project_path), ( + f"Traversal path '{path}' should still be blocked" + ) + + class TestValidateProjectPathEdgeCases: """Test edge cases and error conditions."""