mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(core): allow double-dot filenames while still blocking path traversal (#673)
Signed-off-by: Joe P <joe@basicmemory.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
[
|
||||
|
||||
@@ -191,6 +191,54 @@ class TestValidateProjectPathAttacks:
|
||||
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."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user