diff --git a/backend/core/path_security.py b/backend/core/path_security.py index f465b09f..c4aa8d33 100644 --- a/backend/core/path_security.py +++ b/backend/core/path_security.py @@ -17,6 +17,13 @@ f"{prefix}{number}" for prefix in ("COM", "LPT") for number in range(1, 10) ) +# Both separator families, so a stored sub-path splits into the same components +# on every host. Windows accepts ``/`` as a real separator, so splitting on +# ``os.sep`` alone left ``"job/out.mp4"`` as a single component there while the +# identical value split cleanly on POSIX. POSIX input never reaches this with a +# backslash — it is rejected as a foreign separator before the split. +_PATH_SEPARATORS = re.compile(r"[\\/]") + class UnsafePath(ValueError): """Raised when a path crosses its allowed filesystem boundary.""" @@ -69,7 +76,7 @@ def resolve_within(root: os.PathLike[str] | str, value: os.PathLike[str] | str) # containment proof explicit to static analysis, this rejects empty, # dot, parent, drive, and separator-bearing components before Path sees # any persisted/request-derived string. - parts = raw.split(os.sep) + parts = _PATH_SEPARATORS.split(raw) clean_parts: list[str] = [] for part in parts: clean = os.path.basename(part) diff --git a/tests/test_filesystem_boundaries.py b/tests/test_filesystem_boundaries.py index ce69e5fe..7ba50c8e 100644 --- a/tests/test_filesystem_boundaries.py +++ b/tests/test_filesystem_boundaries.py @@ -52,6 +52,41 @@ def test_resolve_within_accepts_relative_and_existing_absolute_paths(tmp_path): assert resolve_within(root, item) == item +def test_stored_subpaths_split_on_both_separator_families(): + """The component split is host-independent. + + Asserted on the splitter itself, not through ``resolve_within``: the + Python suite runs on Linux only, where ``os.sep`` splitting already + handled ``/``. A behavioural test would pass here whether or not the + Windows path is fixed, so it would not guard the regression. + """ + from core.path_security import _PATH_SEPARATORS + assert _PATH_SEPARATORS.split("sub/voice.wav") == ["sub", "voice.wav"] + assert _PATH_SEPARATORS.split(r"sub\voice.wav") == ["sub", "voice.wav"] + + +def test_resolve_within_reads_a_stored_subpath(tmp_path): + """A persisted sub-path resolves to the same file on Windows and POSIX. + + Windows accepts ``/`` as a real separator, so a row written by a Linux + host (or a Docker deployment) must resolve there exactly as it does on + POSIX instead of being rejected as one unsafe component. + """ + from core.path_security import resolve_within + root = tmp_path / "root" + (root / "sub").mkdir(parents=True) + assert resolve_within(root, "sub/voice.wav") == root / "sub" / "voice.wav" + + +def test_resolve_within_rejects_traversal_through_either_separator(tmp_path): + """Splitting on both separators must not open a traversal path.""" + from core.path_security import UnsafePath, resolve_within + root = tmp_path / "root" + (root / "sub").mkdir(parents=True) + with pytest.raises(UnsafePath): + resolve_within(root, "sub/../../secret.wav") + + def test_resolve_within_rejects_parent_and_absolute_escape(tmp_path): from core.path_security import UnsafePath, resolve_within root = tmp_path / "root" @@ -76,7 +111,10 @@ def test_resolve_within_rejects_symlink_escape(tmp_path): (root / "link").symlink_to(outside, target_is_directory=True) except OSError: pytest.skip("symlink creation is unavailable on this host") - with pytest.raises(UnsafePath): + # Match the reason, not just the type: when ``/`` was not treated as a + # separator on Windows this call failed at component validation instead, + # so the containment check below it was never exercised there. + with pytest.raises(UnsafePath, match="escapes its allowed root"): resolve_within(root, "link/secret.wav")