diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 835183a2cb..55e089f083 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -733,22 +733,21 @@ def load(cls, run_id: str, project_root: Path) -> RunState: the lookup path. Without this guard, a caller passing a value like ``../escape`` (e.g. via ``specify workflow resume`` CLI argument) would interpolate path-traversal segments into - ``runs_dir`` below, letting ``state_path.exists()`` probe arbitrary - paths and ``json.load`` read attacker-planted JSON from outside - the project's ``runs/`` directory. ``__init__`` already runs this - check on the stored ``state_data["run_id"]``, but that fires - *after* the file lookup — too late to prevent the disclosure. + ``runs_dir`` below, letting ``open()`` read attacker-planted JSON + from outside the project's ``runs/`` directory. ``__init__`` already + runs this check on the stored ``state_data["run_id"]``, but that + fires *after* the file lookup — too late to prevent the disclosure. Mirrors the precedent in ``agents._ensure_within_directory``. """ cls._validate_run_id(run_id) runs_dir = project_root / ".specify" / "workflows" / "runs" / run_id state_path = runs_dir / "state.json" - if not state_path.exists(): + try: + with open(state_path, encoding="utf-8") as f: + state_data = json.load(f) + except FileNotFoundError: msg = f"Run state not found: {state_path}" - raise FileNotFoundError(msg) - - with open(state_path, encoding="utf-8") as f: - state_data = json.load(f) + raise FileNotFoundError(msg) from None if not isinstance(state_data, dict): raise ValueError("Invalid run state: expected a JSON object") missing_fields = [ diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 95baf22d3c..8f3e7eab4e 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -7086,6 +7086,27 @@ def test_load_not_found(self, project_dir): with pytest.raises(FileNotFoundError): RunState.load("nonexistent", project_dir) + def test_load_not_found_custom_message(self, project_dir): + """Regression: RunState.load() must raise FileNotFoundError with + the custom 'Run state not found:' message, not a raw OSError.""" + from specify_cli.workflows.engine import RunState + + with pytest.raises(FileNotFoundError, match=r"Run state not found:.*nonexistent"): + RunState.load("nonexistent", project_dir) + + def test_load_not_found_no_exists_probe(self, project_dir): + """Regression: RunState.load() must not call exists() before open(), + so it cannot be tricked by a TOCTOU race where the file disappears + between the check and the read.""" + from unittest.mock import patch + from specify_cli.workflows.engine import RunState + + # Verify that a nonexistent state.json raises FileNotFoundError + # with the custom message even when we can't check exists() first. + with patch("builtins.open", side_effect=FileNotFoundError): + with pytest.raises(FileNotFoundError, match="Run state not found:"): + RunState.load("nonexistent", project_dir) + def test_load_rejects_stored_run_id_mismatch(self, project_dir): """The state payload cannot redirect later writes to another run.""" from specify_cli.workflows.engine import RunState