diff --git a/libs/beacon/src/beacon/domains/warehouse/contribute.py b/libs/beacon/src/beacon/domains/warehouse/contribute.py index 8f45987..c1b485f 100644 --- a/libs/beacon/src/beacon/domains/warehouse/contribute.py +++ b/libs/beacon/src/beacon/domains/warehouse/contribute.py @@ -12,6 +12,7 @@ from beacon.domains.warehouse._tracked_paths import get_tracked_paths from beacon.domains.warehouse.preconditions import ensure_sync_ready +from beacon.utils.paths import normalize_relative_path @dataclass @@ -91,15 +92,16 @@ def contribute( tracked_paths = get_tracked_paths(warehouse_path, beacon_yaml) if paths is not None: + normalized_paths = [normalize_relative_path(p) for p in paths] tracked_set = set(tracked_paths) - untracked = [p for p in paths if p not in tracked_set] + untracked = [p for p in normalized_paths if p not in tracked_set] if untracked: raise ValueError( f"The following paths are not tracked by beacon.yaml and cannot be committed: " f"{', '.join(repr(p) for p in untracked)}" ) # Use the caller-supplied paths (preserving their order), scoped within tracked_paths - commit_paths = list(paths) + commit_paths = normalized_paths else: commit_paths = tracked_paths diff --git a/libs/beacon/src/beacon/utils/paths.py b/libs/beacon/src/beacon/utils/paths.py new file mode 100644 index 0000000..c96dcdd --- /dev/null +++ b/libs/beacon/src/beacon/utils/paths.py @@ -0,0 +1,28 @@ +"""Stateless path helpers with no domain knowledge.""" + +import os +from pathlib import Path + + +def normalize_relative_path(path: str) -> str: + """Normalize a user-supplied relative path for membership checking. + + * Converts to POSIX-style forward slashes. + * Removes leading './' segments. + * Collapses redundant separators (e.g. 'skills//foo' -> 'skills/foo'). + * Rejects absolute paths and parent-directory traversal. + + Raises: + ValueError: if the path is absolute or contains a '..' component. + """ + p = Path(path) + if p.is_absolute(): + raise ValueError(f"Absolute paths are not allowed: {path!r}") + parts = p.parts + if ".." in parts: + raise ValueError(f"Parent-directory traversal is not allowed: {path!r}") + # normpath removes './' and collapses redundant separators + normalized = os.path.normpath(path) + # Ensure POSIX-style forward slashes for cross-platform consistency + normalized = normalized.replace(os.sep, "/") + return normalized diff --git a/libs/beacon/tests/unit/utils/test_paths.py b/libs/beacon/tests/unit/utils/test_paths.py new file mode 100644 index 0000000..288ab64 --- /dev/null +++ b/libs/beacon/tests/unit/utils/test_paths.py @@ -0,0 +1,43 @@ +"""Unit tests for path normalization utilities.""" + +import pytest +from beacon.utils.paths import normalize_relative_path + + +class TestNormalizeRelativePath: + """Test cases for normalize_relative_path (PER-184).""" + + def test_plain_path_unchanged(self): + """A clean relative path passes through unchanged.""" + assert normalize_relative_path("contexts/a.md") == "contexts/a.md" + + def test_leading_dot_slash_stripped(self): + """Leading './' is removed.""" + assert normalize_relative_path("./contexts/a.md") == "contexts/a.md" + + def test_double_slash_collapsed(self): + """Redundant '//' separators are collapsed.""" + assert normalize_relative_path("contexts//a.md") == "contexts/a.md" + + def test_nested_dot_segments_collapsed(self): + """Interior '.' segments are collapsed.""" + assert normalize_relative_path("contexts/./a.md") == "contexts/a.md" + + def test_absolute_path_rejected(self): + """Absolute paths raise ValueError.""" + with pytest.raises(ValueError, match="[Aa]bsolute"): + normalize_relative_path("/absolute/path.md") + + def test_parent_traversal_rejected(self): + """Paths containing '..' raise ValueError.""" + with pytest.raises(ValueError, match="[Pp]arent-directory"): + normalize_relative_path("../outside.md") + + def test_parent_traversal_interior_rejected(self): + """A '..' component anywhere in the path is rejected.""" + with pytest.raises(ValueError, match="[Pp]arent-directory"): + normalize_relative_path("contexts/../../../etc/passwd") + + def test_dotdot_in_filename_not_rejected(self): + """A literal '..' inside a filename is not a traversal component.""" + assert normalize_relative_path("contexts/foo..bar.md") == "contexts/foo..bar.md" diff --git a/libs/beacon/tests/unit/warehouse/test_contribute.py b/libs/beacon/tests/unit/warehouse/test_contribute.py index 5a82d24..66494f0 100644 --- a/libs/beacon/tests/unit/warehouse/test_contribute.py +++ b/libs/beacon/tests/unit/warehouse/test_contribute.py @@ -448,6 +448,80 @@ def test_contribute_paths_sequential_groups_leave_others_dirty( assert "commit b" in log.stdout assert "commit a" in log.stdout + def test_contribute_paths_leading_dot_slash(self, contrib_project_multi): + """Leading './' is normalized away before membership check.""" + project, wh = contrib_project_multi + env = _git_env() + + (wh / "contexts" / "a.md").write_text("# a modified\n") + + result = contribute( + project, + message="commit with ./prefix", + push=False, + paths=("./contexts/a.md",), + ) + assert result.status == "committed" + + committed_files = subprocess.run( + ["git", "show", "--name-only", "--format=", "HEAD"], + cwd=wh, + env=env, + capture_output=True, + text=True, + check=True, + ) + assert "contexts/a.md" in committed_files.stdout + + def test_contribute_paths_double_slash(self, contrib_project_multi): + """Redundant '//' separators are collapsed.""" + project, wh = contrib_project_multi + env = _git_env() + + (wh / "contexts" / "a.md").write_text("# a modified\n") + + result = contribute( + project, + message="commit with double slash", + push=False, + paths=("contexts//a.md",), + ) + assert result.status == "committed" + + committed_files = subprocess.run( + ["git", "show", "--name-only", "--format=", "HEAD"], + cwd=wh, + env=env, + capture_output=True, + text=True, + check=True, + ) + assert "contexts/a.md" in committed_files.stdout + + def test_contribute_paths_absolute_rejected(self, contrib_project_multi): + """Absolute paths raise ValueError.""" + project, wh = contrib_project_multi + with pytest.raises(ValueError) as exc_info: + contribute( + project, + message="should fail", + push=False, + paths=("/absolute/path.md",), + ) + assert "absolute" in str(exc_info.value).lower() + + def test_contribute_paths_dotdot_rejected(self, contrib_project_multi): + """Paths with '..' raise ValueError.""" + project, wh = contrib_project_multi + with pytest.raises(ValueError) as exc_info: + contribute( + project, + message="should fail", + push=False, + paths=("../outside.md",), + ) + assert "parent-directory" in str(exc_info.value).lower() + class TestDirtyOutsideScope: """Tests for PER-159: out-of-scope dirty file count in contribute."""