Skip to content

Commit c2a0a0c

Browse files
WOLIKIMCHENGWOLIKIMCHENG
andauthored
fix(workflows): validate persisted step result shapes (#4399)
Reject non-object step_results values and per-step records at the RunState load boundary. Preserve valid mappings and the legacy empty default when the field is omitted. Co-authored-by: WOLIKIMCHENG <kinsonnee@gmail.com>
1 parent e1046bd commit c2a0a0c

2 files changed

Lines changed: 77 additions & 2 deletions

File tree

‎src/specify_cli/workflows/engine.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,18 @@ def load(cls, run_id: str, project_root: Path) -> RunState:
849849
installed_workflow_id = state_data.get("installed_workflow_id")
850850
installed_registry_root = state_data.get("installed_registry_root")
851851

852+
step_results = state_data.get("step_results", {})
853+
if not isinstance(step_results, dict):
854+
raise ValueError(
855+
"Invalid run state: 'step_results' must be a JSON object"
856+
)
857+
for step_id, result in step_results.items():
858+
if not isinstance(result, dict):
859+
raise ValueError(
860+
"Invalid run state: step_results record "
861+
f"{step_id!r} must be a JSON object"
862+
)
863+
852864
state = cls(
853865
run_id=state_data["run_id"],
854866
workflow_id=workflow_id,
@@ -875,7 +887,7 @@ def load(cls, run_id: str, project_root: Path) -> RunState:
875887
)
876888
state.current_step_index = current_step_index
877889
state.current_step_id = state_data.get("current_step_id")
878-
state.step_results = state_data.get("step_results", {})
890+
state.step_results = step_results
879891
state.workflow_dir = state_data.get("workflow_dir")
880892
state.created_at = state_data.get("created_at", "")
881893
state.updated_at = state_data.get("updated_at", "")

‎tests/test_workflows.py‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7681,7 +7681,70 @@ def test_save_and_load(self, project_dir):
76817681
assert loaded.workflow_id == "test-workflow"
76827682
assert loaded.status == RunStatus.RUNNING
76837683
assert loaded.inputs == {"name": "login"}
7684-
assert "step-one" in loaded.step_results
7684+
assert loaded.step_results == state.step_results
7685+
7686+
@pytest.mark.parametrize("invalid_step_results", [None, [], "invalid", 1, True])
7687+
def test_load_rejects_non_object_step_results(
7688+
self, project_dir, invalid_step_results
7689+
):
7690+
"""Persisted step results must be a JSON object."""
7691+
from specify_cli.workflows.engine import RunState
7692+
7693+
state = RunState(
7694+
run_id="invalid-results",
7695+
workflow_id="test-workflow",
7696+
project_root=project_dir,
7697+
)
7698+
state.save()
7699+
state_path = state.runs_dir / "state.json"
7700+
state_data = json.loads(state_path.read_text(encoding="utf-8"))
7701+
state_data["step_results"] = invalid_step_results
7702+
state_path.write_text(json.dumps(state_data), encoding="utf-8")
7703+
7704+
with pytest.raises(ValueError, match="step_results.*JSON object"):
7705+
RunState.load("invalid-results", project_dir)
7706+
7707+
@pytest.mark.parametrize("invalid_result", [None, [], "invalid", 1, True])
7708+
def test_load_rejects_non_object_step_result_records(
7709+
self, project_dir, invalid_result
7710+
):
7711+
"""Each persisted step result must be a JSON object."""
7712+
from specify_cli.workflows.engine import RunState
7713+
7714+
state = RunState(
7715+
run_id="invalid-record",
7716+
workflow_id="test-workflow",
7717+
project_root=project_dir,
7718+
)
7719+
state.save()
7720+
state_path = state.runs_dir / "state.json"
7721+
state_data = json.loads(state_path.read_text(encoding="utf-8"))
7722+
state_data["step_results"] = {"step-one": invalid_result}
7723+
state_path.write_text(json.dumps(state_data), encoding="utf-8")
7724+
7725+
with pytest.raises(
7726+
ValueError,
7727+
match="step_results record 'step-one' must be a JSON object",
7728+
):
7729+
RunState.load("invalid-record", project_dir)
7730+
7731+
def test_load_defaults_missing_step_results_for_legacy_state(self, project_dir):
7732+
"""Legacy states without step results load with an empty mapping."""
7733+
from specify_cli.workflows.engine import RunState
7734+
7735+
state = RunState(
7736+
run_id="legacy-results",
7737+
workflow_id="test-workflow",
7738+
project_root=project_dir,
7739+
)
7740+
state.save()
7741+
state_path = state.runs_dir / "state.json"
7742+
state_data = json.loads(state_path.read_text(encoding="utf-8"))
7743+
state_data.pop("step_results")
7744+
state_path.write_text(json.dumps(state_data), encoding="utf-8")
7745+
7746+
loaded = RunState.load("legacy-results", project_dir)
7747+
assert loaded.step_results == {}
76857748

76867749
def test_load_not_found(self, project_dir):
76877750
from specify_cli.workflows.engine import RunState

0 commit comments

Comments
 (0)