From 670968e2d5ce9651142926eb0c4aea0b77bc04c1 Mon Sep 17 00:00:00 2001 From: Alpamys Date: Mon, 13 Apr 2026 13:31:43 +0500 Subject: [PATCH] fix(autopilot): Windows py3.9 path traversal false-positive test_writes_config fails on windows-latest / Python 3.9 with exit code 1 because the path-traversal check in soup_cli/commands/autopilot.py was: data_path = Path(data).resolve() data_path.relative_to(Path.cwd().resolve()) On Windows + Python 3.9, Path.resolve() occasionally leaves 8.3 short names (e.g. "C:\Users\RUNNER~1") in one of the two sides but not the other, so relative_to raises ValueError even when both paths point to the same location. GitHub Actions runner home dirs frequently trigger this (the runneradmin account is created as "runneradmin" but short names get generated as "RUNNER~1"). Fix: introduce _is_under_cwd(path) helper in soup_cli/commands/autopilot.py that uses os.path.realpath on both sides (handles 8.3 expansion consistently) plus os.path.commonpath for the containment check, with case-insensitive comparison on NT. Apply it to both the --data and --output path guards. The data_path / output_path locals are then rebuilt from the realpath result so downstream logic sees the canonical long-name path. Also enriches the test assertion to print result.output and result.exception on failure so future CI breaks are easier to diagnose without needing to push a debug commit first. Local verification: all 38 tests in tests/test_autopilot.py pass on Python 3.10 Windows, full suite 2313 passed. Co-Authored-By: Claude Opus 4.6 (1M context) --- soup_cli/commands/autopilot.py | 39 +++++++++++++++++++++++++++------- tests/test_autopilot.py | 6 +++++- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/soup_cli/commands/autopilot.py b/soup_cli/commands/autopilot.py index ba7cd14..4505433 100644 --- a/soup_cli/commands/autopilot.py +++ b/soup_cli/commands/autopilot.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os from pathlib import Path import typer @@ -19,6 +20,30 @@ from soup_cli.autopilot.generate_config import build_soup_config, write_yaml console = Console() +def _is_under_cwd(path: Path) -> bool: + """Check whether ``path`` resolves inside the current working directory. + + Uses ``os.path.realpath`` for both sides instead of ``Path.resolve()``. + On Windows + Python 3.9, ``Path.resolve()`` occasionally leaves 8.3 short + names (e.g. ``C:\\Users\\RUNNER~1``) in one of the two paths but not the + other, making ``relative_to`` fail even when the paths refer to the same + location. ``realpath`` handles the short-name expansion consistently. + """ + try: + resolved = os.path.realpath(str(path)) + cwd = os.path.realpath(str(Path.cwd())) + except (OSError, ValueError): + return False + if os.name == "nt": + resolved = resolved.lower() + cwd = cwd.lower() + try: + common = os.path.commonpath([resolved, cwd]) + except ValueError: + return False + return common == cwd + + def autopilot_cmd( model: str = typer.Option(..., "--model", "-m", help="Base model (HF model id)"), data: str = typer.Option(..., "--data", "-d", help="Dataset path (JSONL)"), @@ -51,12 +76,11 @@ def autopilot_cmd( raise typer.Exit(1) # Path traversal protection — data must stay under cwd - try: - data_path = Path(data).resolve() - data_path.relative_to(Path.cwd().resolve()) - except ValueError: + data_path = Path(data) + if not _is_under_cwd(data_path): console.print("[red]Data path must be under the current working directory.[/]") raise typer.Exit(1) + data_path = Path(os.path.realpath(str(data_path))) if not data_path.exists(): console.print(f"[red]Data file not found: {data_path}[/]") @@ -133,12 +157,11 @@ def autopilot_cmd( return # Path traversal protection for output - try: - output_path = Path(output).resolve() - output_path.relative_to(Path.cwd().resolve()) - except ValueError: + output_raw = Path(output) + if not _is_under_cwd(output_raw): console.print("[red]Output path must be under the current working directory.[/]") raise typer.Exit(1) + output_path = Path(os.path.realpath(str(output_raw))) if output_path.exists() and not yes: if not typer.confirm(f"{output_path} exists. Overwrite?"): diff --git a/tests/test_autopilot.py b/tests/test_autopilot.py index 45ca108..326613b 100644 --- a/tests/test_autopilot.py +++ b/tests/test_autopilot.py @@ -335,7 +335,11 @@ class TestAutopilotCLI: "--output", "soup.yaml", "--yes", ]) - assert result.exit_code == 0 + assert result.exit_code == 0, ( + f"exit={result.exit_code}\n" + f"output={result.output}\n" + f"exception={result.exception!r}" + ) assert (tmp_path / "soup.yaml").exists() def test_rejects_path_traversal_data(self, tmp_path, monkeypatch):