From 4bae9113e4861cd16fd9cf739e5cfee1c96e8a75 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 21 Jul 2026 03:36:42 +0200 Subject: [PATCH] test(backend): pin the traversal guard against a string-prefix bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- backend/requirements-dev.txt | 9 +++++---- backend/tests/test_storage.py | 12 ++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt index f07b152..bc9c558 100644 --- a/backend/requirements-dev.txt +++ b/backend/requirements-dev.txt @@ -1,6 +1,7 @@ -# Test + lint tooling — NEVER in the prod image (installed only in the Dockerfile's `dev` stage, -# which `siftlode publish` does not target). Pinned so the gate is reproducible: the same ruff the -# `siftlode check` backend lane runs, and pytest for the pure-logic suite. httpx is already an app -# dependency (requirements.txt), so its TestClient needs nothing extra here. +# Test + lint tooling — NEVER in the prod image. Installed only by backend/Dockerfile.test (the +# throwaway test image), which the prod build (root Dockerfile) never references. Pinned so the +# suite is reproducible: pytest for the pure-logic tests, and ruff at the SAME version the gate's +# backend lint lane currently runs on the host (see the run_lint note in siftlode.sh). httpx is +# already an app dependency (requirements.txt), so its TestClient needs nothing extra here. pytest==8.4.2 ruff==0.15.21 diff --git a/backend/tests/test_storage.py b/backend/tests/test_storage.py index 4e1d043..ab2c8e7 100644 --- a/backend/tests/test_storage.py +++ b/backend/tests/test_storage.py @@ -91,5 +91,17 @@ class TestSafeAbsPath: 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