diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index d17513cc0b..c78fd62352 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -851,7 +851,32 @@ def load(cls, run_id: str, project_root: Path) -> RunState: installed_origin_tracked=has_installed_workflow_id, ) state.status = RunStatus(state_data["status"]) - state.current_step_index = state_data.get("current_step_index", 0) + + # ``resume()`` slices ``definition.steps[state.current_step_index :]`` + # with no guard of its own -- unlike ``workflow_id`` / + # ``installed_workflow_id`` / ``installed_registry_root`` / ``inputs`` + # above, this field was never shape-checked here. A non-int value (a + # hand-edited or externally-written state.json, e.g. a string or + # float) reaches that slice and raises a raw, unhelpful + # ``TypeError: slice indices must be integers or None or have an + # __index__ method`` from deep inside ``resume()`` instead of the + # clean "Invalid run state: ..." this loader already gives every + # other malformed field. A negative value slices from the end instead + # of failing, silently resuming from the wrong step. Reject both here, + # consistent with the sibling checks. ``bool`` is an ``int`` subclass, + # so it is excluded explicitly (mirrors the ``max_iterations`` / + # ``continue_on_error`` bool guards elsewhere in this module). + current_step_index = state_data.get("current_step_index", 0) + if ( + isinstance(current_step_index, bool) + or not isinstance(current_step_index, int) + or current_step_index < 0 + ): + raise ValueError( + "Invalid run state: 'current_step_index' must be a " + f"non-negative integer, got {current_step_index!r}" + ) + state.current_step_index = current_step_index state.current_step_id = state_data.get("current_step_id") state.step_results = state_data.get("step_results", {}) state.workflow_dir = state_data.get("workflow_dir") diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d599f3c6a4..8a72d075cc 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -7376,6 +7376,49 @@ def test_load_rejects_stored_run_id_mismatch(self, project_dir): ): RunState.load("requested-run", project_dir) + @pytest.mark.parametrize( + "bad_current_step_index", + ["not-a-number", 1.5, -1, [0], {"index": 0}, True], + ) + def test_load_rejects_invalid_current_step_index( + self, project_dir, bad_current_step_index + ): + """A malformed ``current_step_index`` must fail at load(), not resume(). + + ``resume()`` slices ``definition.steps[state.current_step_index :]`` + with no guard of its own. Every other field this loader restores + (``workflow_id``, ``installed_workflow_id``, ``installed_registry_root``, + ``inputs``) is shape-checked here and raises the same clean "Invalid + run state: ..." ``ValueError`` on a malformed value; ``current_step_index`` + was the one field silently passed through. A non-int value reaches the + slice and raises a raw, unhelpful ``TypeError`` from deep inside + ``resume()`` instead. ``True`` is included because ``bool`` is an + ``int`` subclass and would otherwise slip past a bare ``isinstance(..., + int)`` check. + """ + from specify_cli.workflows.engine import RunState + + run_dir = ( + project_dir / ".specify" / "workflows" / "runs" / "bad-index-run" + ) + run_dir.mkdir(parents=True) + (run_dir / "state.json").write_text( + json.dumps( + { + "run_id": "bad-index-run", + "workflow_id": "test-workflow", + "status": "paused", + "current_step_index": bad_current_step_index, + } + ), + encoding="utf-8", + ) + + with pytest.raises( + ValueError, match="'current_step_index' must be a non-negative integer" + ): + RunState.load("bad-index-run", project_dir) + @pytest.mark.parametrize( ("installed_workflow_id", "installed_registry_root"), [