release_train: --logs is a relative folder inside the current directory (#214)

Closes SonarCloud's path-injection alert on the merged release_train.py (pythonsecurity:S8707):
an absolute path, .., or anything outside the working directory is refused before the folder is
made.
This commit is contained in:
FredApps authored and GitHub committed 2026-11-12 21:52:12 -05:00
1 parent fd88e26a87
commit 3eaa8798ac
2 files changed
+31 -3

No files matched your search

+12 -1
View File
@@ -50,6 +50,17 @@ def check(repos: list[str], extra: list[str]) -> None:
raise SystemExit(f"{opt!r} is not an option a train passes to cut_release.py")
def logs_dir(name: str) -> Path:
"""The log folder: a plain relative path inside the current directory, nothing outside it."""
base = Path.cwd().resolve()
if not re.fullmatch(r"[A-Za-z0-9._/-]+", name) or Path(name).is_absolute():
raise SystemExit(f"{name!r}: --logs is a relative folder name inside the current directory")
path = (base / name).resolve()
if path != base and base not in path.parents:
raise SystemExit(f"{name!r}: --logs must stay inside {base}")
return path
def run_app(repo: str, extra: list[str], logs: Path) -> tuple[str, int, str]:
log = logs / f"{repo.split('/')[-1]}.log"
with log.open("w", encoding="utf-8") as out:
@@ -80,7 +91,7 @@ def main(argv: list[str] | None = None) -> int:
p.error("name the apps, or pass --all")
extra += ["--publish"] * a.publish + ["--dry-run"] * a.dry_run
check(repos, extra)
logs = Path(a.logs)
logs = logs_dir(a.logs)
logs.mkdir(parents=True, exist_ok=True)
start = time.time()
+19 -2
View File
@@ -1,5 +1,6 @@
"""scripts/release_train.py: which apps a train releases, and what it passes on."""
import importlib.util
import os
import tempfile
import unittest
from pathlib import Path
@@ -19,8 +20,13 @@ class TrainTests(unittest.TestCase):
calls.append((repo, extra))
return repo, rc, "done"
with mock.patch.object(release_train, "run_app", side_effect=fake):
code = release_train.main(["--logs", tempfile.mkdtemp()] + argv)
cwd = os.getcwd()
os.chdir(tempfile.mkdtemp())
try:
with mock.patch.object(release_train, "run_app", side_effect=fake):
code = release_train.main(["--logs", "logs"] + argv)
finally:
os.chdir(cwd)
return code, calls
def test_all_but_the_skipped_apps_with_the_flags_passed_on(self):
@@ -39,6 +45,17 @@ class TrainTests(unittest.TestCase):
with self.assertRaises(SystemExit, msg=f"{repos} {extra}"):
release_train.check(repos, extra)
def test_the_log_folder_stays_inside_the_current_directory(self):
cwd = os.getcwd()
os.chdir(tempfile.mkdtemp())
try:
self.assertEqual(release_train.logs_dir("logs/today"), Path.cwd().resolve() / "logs" / "today")
for bad in ["/tmp/x", "../outside", "logs/../../x", "C:/Windows", "a b"]:
with self.assertRaises(SystemExit, msg=bad):
release_train.logs_dir(bad)
finally:
os.chdir(cwd)
def test_one_stopped_app_fails_the_train(self):
code, _ = self.run_train(["storytold/pdfcraft"], rc=1)
self.assertEqual(code, 1)