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) <noreply@anthropic.com>
This commit is contained in:
Alpamys 2026-04-13 13:31:43 +05:00
parent e44e0bd663
commit 670968e2d5
2 changed files with 36 additions and 9 deletions

View File

@ -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?"):

View File

@ -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):