mirror of https://github.com/razor-ai/soup.git
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:
parent
e44e0bd663
commit
670968e2d5
|
|
@ -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?"):
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue