fix(paths): treat / as a separator on Windows so stored sub-paths resolve

resolve_within split candidate paths on os.sep alone. Windows accepts /
as a real separator but os.sep is \ there, so a persisted sub-path such
as "job_123/out.mp4" stayed a single component, failed the
basename-equality check, and raised UnsafePath — while the identical
value split cleanly and resolved on POSIX. A data directory written on
Linux or by the Docker deployment and then opened by the Windows desktop
app hit exactly that.

Split on both separator families instead, which is what the comment
above the split already states the code intends. This is not a
loosening: every component still goes through the same basename / "." /
".." / empty rejection, and the commonpath containment check and symlink
resolution below are unchanged. POSIX behaviour is unchanged too — a
backslash is already rejected there as a foreign separator before the
split runs.

This also restores real coverage of the symlink-escape guard on Windows.
test_resolve_within_rejects_symlink_escape asserts through
"link/secret.wav", which previously raised at component validation
before reaching the containment check it exists to cover, so it passed
for the wrong reason. It now matches on the reason.
This commit is contained in:
Eman-Yousaf
2026-08-14 17:39:09 +05:00
parent 420bc73e78
commit 579f2e0a2e
2 changed files with 47 additions and 2 deletions
+8 -1
View File
@@ -17,6 +17,13 @@ _WINDOWS_RESERVED_NAMES = frozenset({"CON", "PRN", "AUX", "NUL"}) | frozenset(
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)
+39 -1
View File
@@ -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")