From b5cb4d12c132503e71a5537a1686eefabca50f79 Mon Sep 17 00:00:00 2001 From: Alpamys Date: Mon, 11 May 2026 19:25:22 +0500 Subject: [PATCH] fix(v0.48.0): lstat symlink check on original path, not realpath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI revealed that on macOS the 3 symlink-rejection tests pass on Windows (where tmp_path is non-symlink-resolved) but fail on macOS because `/var/folders/...` is itself a symlink to `/private/var/folders/...`, and the test's symlink `link.jsonl -> real.jsonl` was being checked AFTER `os.path.realpath` had already followed both symlinks. Fix: `os.lstat(original_path)` BEFORE `os.path.realpath` in three sites — `validate_datasets`, `write_mix_recipe`, `load_mix_recipe`, and the `soup runs curriculum-curve` history reader. Matches the v0.46.0 Part A `_reject_symlink_target` pattern. `FileNotFoundError` from `lstat` is treated as "no existing path" (write target case); other `OSError`s propagate as `ValueError`. Cached `st` result reused for the `overwrite` gate in `write_mix_recipe` so we don't re-stat after realpath. Co-Authored-By: Claude Opus 4.7 (1M context) --- soup_cli/commands/runs.py | 33 ++++++++------ soup_cli/utils/data_mix.py | 93 ++++++++++++++++++++------------------ 2 files changed, 68 insertions(+), 58 deletions(-) diff --git a/soup_cli/commands/runs.py b/soup_cli/commands/runs.py index 4a9c376..68c6f34 100644 --- a/soup_cli/commands/runs.py +++ b/soup_cli/commands/runs.py @@ -588,25 +588,28 @@ def curriculum_curve( candidate = os.path.join(out_dir, "curriculum_history.jsonl") else: candidate = history_path + # Symlink check on the ORIGINAL path BEFORE realpath (matches v0.46.0 + # `_reject_symlink_target` pattern — realpath follows the symlink). + try: + _st = os.lstat(candidate) + except FileNotFoundError: + _st = None + except OSError as exc: + console.print( + f"[red]history path is not stat-able:[/] " + f"{markup_escape(os.path.basename(candidate))}" + ) + raise typer.Exit(2) from exc + if _st is not None and _stat.S_ISLNK(_st.st_mode): + console.print( + f"[red]history path is a symlink (rejected for safety):[/] " + f"{markup_escape(os.path.basename(candidate))}" + ) + raise typer.Exit(2) real = os.path.realpath(candidate) if not is_under_cwd(real): console.print("[red]history path is outside cwd[/]") raise typer.Exit(2) - if os.path.lexists(real): - try: - _st = os.lstat(real) - except OSError as exc: - console.print( - f"[red]history path is not stat-able:[/] " - f"{markup_escape(os.path.basename(real))}" - ) - raise typer.Exit(2) from exc - if _stat.S_ISLNK(_st.st_mode): - console.print( - f"[red]history path is a symlink (rejected for safety):[/] " - f"{markup_escape(os.path.basename(real))}" - ) - raise typer.Exit(2) if not os.path.isfile(real): console.print( f"[yellow]curriculum_history.jsonl not found:[/] " diff --git a/soup_cli/utils/data_mix.py b/soup_cli/utils/data_mix.py index 089be0c..5b4a42b 100644 --- a/soup_cli/utils/data_mix.py +++ b/soup_cli/utils/data_mix.py @@ -210,25 +210,27 @@ def validate_datasets(raw: Sequence[str]) -> Tuple[str, ...]: seen: List[str] = [] for item in raw: path = _check_str_path("dataset", item) + # Symlink check on the ORIGINAL path BEFORE realpath (which would + # follow the symlink, defeating the check). Matches v0.46.0 Part A + # `_reject_symlink_target` pattern. + try: + st = os.lstat(path) + except FileNotFoundError: + st = None + except OSError as exc: + raise ValueError( + f"dataset path is not stat-able: {os.path.basename(path)!r}" + ) from exc + if st is not None and stat.S_ISLNK(st.st_mode): + raise ValueError( + f"dataset path is a symlink (rejected for safety): " + f"{os.path.basename(path)!r}" + ) real = os.path.realpath(path) if not is_under_cwd(real): raise ValueError( f"dataset path is outside cwd: {os.path.basename(real)!r}" ) - # Reject symlink at the actual path (TOCTOU defence mirroring - # v0.33.0 #22 / v0.43.0 Part C / v0.46.0 Part A policy). - if os.path.lexists(real): - try: - st = os.lstat(real) - except OSError as exc: - raise ValueError( - f"dataset path is not stat-able: {os.path.basename(real)!r}" - ) from exc - if stat.S_ISLNK(st.st_mode): - raise ValueError( - f"dataset path is a symlink (rejected for safety): " - f"{os.path.basename(real)!r}" - ) if real in seen: raise ValueError( f"duplicate dataset path: {os.path.basename(real)!r}" @@ -575,28 +577,31 @@ def write_mix_recipe( from soup_cli.utils.paths import is_under_cwd _check_str_path("output_path", output_path) + # Symlink check on the ORIGINAL path BEFORE realpath. + try: + st = os.lstat(output_path) + except FileNotFoundError: + st = None + except OSError as exc: + raise ValueError( + f"output_path is not stat-able: " + f"{os.path.basename(output_path)!r}" + ) from exc + if st is not None and stat.S_ISLNK(st.st_mode): + raise ValueError( + f"output_path is a symlink (rejected for safety): " + f"{os.path.basename(output_path)!r}" + ) real = os.path.realpath(output_path) if not is_under_cwd(real): raise ValueError( f"output_path is outside cwd: {os.path.basename(real)!r}" ) - if os.path.lexists(real): - try: - st = os.lstat(real) - except OSError as exc: - raise ValueError( - f"output_path is not stat-able: {os.path.basename(real)!r}" - ) from exc - if stat.S_ISLNK(st.st_mode): - raise ValueError( - f"output_path is a symlink (rejected for safety): " - f"{os.path.basename(real)!r}" - ) - if not overwrite: - raise ValueError( - f"output_path already exists (use overwrite=True): " - f"{os.path.basename(real)!r}" - ) + if st is not None and not overwrite: + raise ValueError( + f"output_path already exists (use overwrite=True): " + f"{os.path.basename(real)!r}" + ) text = render_mix_recipe_yaml(report) if len(text.encode("utf-8")) > _MAX_RECIPE_BYTES: @@ -628,23 +633,25 @@ def load_mix_recipe(path: str) -> Mapping[str, object]: from soup_cli.utils.paths import is_under_cwd _check_str_path("path", path) + # Symlink check on the ORIGINAL path BEFORE realpath. + try: + st = os.lstat(path) + except FileNotFoundError: + st = None + except OSError as exc: + raise ValueError( + f"recipe path is not stat-able: {os.path.basename(path)!r}" + ) from exc + if st is not None and stat.S_ISLNK(st.st_mode): + raise ValueError( + f"recipe path is a symlink (rejected for safety): " + f"{os.path.basename(path)!r}" + ) real = os.path.realpath(path) if not is_under_cwd(real): raise ValueError( f"recipe path is outside cwd: {os.path.basename(real)!r}" ) - if os.path.lexists(real): - try: - st = os.lstat(real) - except OSError as exc: - raise ValueError( - f"recipe path is not stat-able: {os.path.basename(real)!r}" - ) from exc - if stat.S_ISLNK(st.st_mode): - raise ValueError( - f"recipe path is a symlink (rejected for safety): " - f"{os.path.basename(real)!r}" - ) if not os.path.isfile(real): raise FileNotFoundError( f"recipe not found: {os.path.basename(real)!r}"