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 from __future__ import annotations
import os
from pathlib import Path from pathlib import Path
import typer import typer
@ -19,6 +20,30 @@ from soup_cli.autopilot.generate_config import build_soup_config, write_yaml
console = Console() 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( def autopilot_cmd(
model: str = typer.Option(..., "--model", "-m", help="Base model (HF model id)"), model: str = typer.Option(..., "--model", "-m", help="Base model (HF model id)"),
data: str = typer.Option(..., "--data", "-d", help="Dataset path (JSONL)"), data: str = typer.Option(..., "--data", "-d", help="Dataset path (JSONL)"),
@ -51,12 +76,11 @@ def autopilot_cmd(
raise typer.Exit(1) raise typer.Exit(1)
# Path traversal protection — data must stay under cwd # Path traversal protection — data must stay under cwd
try: data_path = Path(data)
data_path = Path(data).resolve() if not _is_under_cwd(data_path):
data_path.relative_to(Path.cwd().resolve())
except ValueError:
console.print("[red]Data path must be under the current working directory.[/]") console.print("[red]Data path must be under the current working directory.[/]")
raise typer.Exit(1) raise typer.Exit(1)
data_path = Path(os.path.realpath(str(data_path)))
if not data_path.exists(): if not data_path.exists():
console.print(f"[red]Data file not found: {data_path}[/]") console.print(f"[red]Data file not found: {data_path}[/]")
@ -133,12 +157,11 @@ def autopilot_cmd(
return return
# Path traversal protection for output # Path traversal protection for output
try: output_raw = Path(output)
output_path = Path(output).resolve() if not _is_under_cwd(output_raw):
output_path.relative_to(Path.cwd().resolve())
except ValueError:
console.print("[red]Output path must be under the current working directory.[/]") console.print("[red]Output path must be under the current working directory.[/]")
raise typer.Exit(1) raise typer.Exit(1)
output_path = Path(os.path.realpath(str(output_raw)))
if output_path.exists() and not yes: if output_path.exists() and not yes:
if not typer.confirm(f"{output_path} exists. Overwrite?"): if not typer.confirm(f"{output_path} exists. Overwrite?"):

View File

@ -335,7 +335,11 @@ class TestAutopilotCLI:
"--output", "soup.yaml", "--output", "soup.yaml",
"--yes", "--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() assert (tmp_path / "soup.yaml").exists()
def test_rejects_path_traversal_data(self, tmp_path, monkeypatch): def test_rejects_path_traversal_data(self, tmp_path, monkeypatch):