peter
4bae9113e4
Review round 1: the two existing safe_abs_path escape tests both pass even against the classic buggy `str(path).startswith(str(root))` containment check, so a regression to that bug would have shipped green. Add the one input that distinguishes the correct `root not in path.parents` guard: a sibling dir whose name prefixes the root (/root vs /rootx). Mutation-verified — swapping the guard to startswith now fails this test. Also corrected the requirements-dev header (installed by Dockerfile.test, not a nonexistent `dev` stage).
108 lines
4.2 KiB
Python
108 lines
4.2 KiB
Python
"""On-disk layout helpers: name sanitation, the Plex/flat path shapes, and — most importantly —
|
|
the safe_abs_path traversal guard that every file-serving endpoint depends on."""
|
|
from datetime import date
|
|
|
|
from app.downloads.storage import (
|
|
MediaMeta,
|
|
download_filename,
|
|
rel_path,
|
|
safe_abs_path,
|
|
sanitize,
|
|
)
|
|
from pathlib import Path
|
|
|
|
|
|
class TestSanitize:
|
|
def test_spaces_become_underscores_and_result_has_no_separators(self):
|
|
out = sanitize("My Great Video")
|
|
assert out == "My_Great_Video"
|
|
assert "/" not in out and "\\" not in out and " " not in out
|
|
|
|
def test_illegal_and_emoji_chars_are_removed(self):
|
|
out = sanitize('a/b:c*?"<>| 🔥')
|
|
for bad in '/\\:*?"<>|🔥':
|
|
assert bad not in out
|
|
|
|
def test_repeated_punctuation_collapses(self):
|
|
assert sanitize("wow!!!___---end") == "wow!_-end"
|
|
|
|
def test_length_is_capped(self):
|
|
assert len(sanitize("x" * 500, limit=10)) <= 10
|
|
|
|
def test_empty_or_all_junk_falls_back_to_untitled(self):
|
|
assert sanitize("") == "untitled"
|
|
assert sanitize("🔥🔥") == "untitled"
|
|
|
|
|
|
class TestRelPath:
|
|
meta = MediaMeta(
|
|
video_id="abc123",
|
|
title="Cool Clip",
|
|
uploader="Some Channel",
|
|
upload_date=date(2024, 5, 2),
|
|
)
|
|
|
|
def test_flat_layout_is_a_single_file(self):
|
|
assert rel_path(self.meta, "mp4", layout="flat") == "Cool_Clip_[abc123].mp4"
|
|
|
|
def test_plex_layout_nests_channel_and_season(self):
|
|
out = rel_path(self.meta, "mp4", layout="plex")
|
|
assert out == "Some_Channel/Season_2024/Some_Channel_-_2024-05-02_-_Cool_Clip_[abc123].mp4"
|
|
|
|
def test_missing_date_uses_year_zero_and_placeholder_date(self):
|
|
meta = MediaMeta(video_id="x", title="t", uploader="c", upload_date=None)
|
|
out = rel_path(meta, "mp4")
|
|
assert "Season_0/" in out and "0000-00-00" in out
|
|
|
|
|
|
class TestDownloadFilename:
|
|
def test_appends_the_container_extension(self):
|
|
assert download_filename("My Video", "mp4", Path("/x/f.webm")) == "My Video.mp4"
|
|
|
|
def test_does_not_double_an_extension_already_present(self):
|
|
assert download_filename("clip.mp4", "mp4", Path("/x/f.mp4")) == "clip.mp4"
|
|
|
|
def test_falls_back_to_the_file_suffix_when_no_container(self):
|
|
assert download_filename("song", None, Path("/x/f.m4a")) == "song.m4a"
|
|
|
|
|
|
class TestSafeAbsPath:
|
|
"""The path-traversal guard: only paths that resolve INSIDE download_root AND exist are returned."""
|
|
|
|
def test_returns_the_path_for_a_file_inside_the_root(self, tmp_path: Path):
|
|
(tmp_path / "sub").mkdir()
|
|
f = tmp_path / "sub" / "video.mp4"
|
|
f.write_text("data")
|
|
got = safe_abs_path(str(tmp_path), "sub/video.mp4")
|
|
assert got is not None and got == f.resolve()
|
|
|
|
def test_rejects_dot_dot_traversal_out_of_the_root(self, tmp_path: Path):
|
|
root = tmp_path / "root"
|
|
root.mkdir()
|
|
secret = tmp_path / "secret.txt"
|
|
secret.write_text("nope")
|
|
# ../secret.txt escapes root even though the file exists — must be refused.
|
|
assert safe_abs_path(str(root), "../secret.txt") is None
|
|
|
|
def test_rejects_an_absolute_path_escape(self, tmp_path: Path):
|
|
root = tmp_path / "root"
|
|
root.mkdir()
|
|
outside = tmp_path / "outside.txt"
|
|
outside.write_text("nope")
|
|
assert safe_abs_path(str(root), str(outside)) is None
|
|
|
|
def test_rejects_a_sibling_dir_whose_name_prefixes_the_root(self, tmp_path: Path):
|
|
# The one input that distinguishes the correct `root not in path.parents` guard from the
|
|
# classic buggy `str(path).startswith(str(root))`: /root vs a real sibling /rootx. A file in
|
|
# rootx is OUTSIDE root, but its path DOES start with the root string — a prefix check would
|
|
# wrongly serve it. This pins the guard against that regression.
|
|
root = tmp_path / "root"
|
|
root.mkdir()
|
|
sibling = tmp_path / "rootx"
|
|
sibling.mkdir()
|
|
(sibling / "steal.mp4").write_text("nope")
|
|
assert safe_abs_path(str(root), "../rootx/steal.mp4") is None
|
|
|
|
def test_returns_none_for_a_nonexistent_file_inside_the_root(self, tmp_path: Path):
|
|
assert safe_abs_path(str(tmp_path), "does/not/exist.mp4") is None
|