Merge pull request #129 from jmjava/cursor/wizard-json-object-2ccd

Fail closed when wizard JSON bodies are not objects

Author: cursor[bot]

milestones/README.md

@@ -5,10 +5,13 @@ repositories that install `docgen` and maintain their own demo bundle. The
 library no longer ships an in-repo dogfood; consumers are the integration test
 of record.
 
-**Active:** **[wizard-state-segments.md](wizard-state-segments.md)** —
-wizard `.docgen-state.json` `segments` must be a mapping of objects.
+**Active:** **[wizard-json-object.md](wizard-json-object.md)** —
+wizard POST/PUT bodies must be JSON objects; bool fields must be booleans.
 
 **Shipped:**
+- **[wizard-state-segments.md](wizard-state-segments.md)** —
+  wizard `.docgen-state.json` `segments` must be a mapping of objects
+  (#128).
 - **[timing-inner-lists.md](timing-inner-lists.md)** —
   `timing.json` `words` / `segments` must be JSON arrays of objects
   (#127).

milestones/wizard-json-object.md

@@ -0,0 +1,34 @@
+# Milestone: wizard POST bodies must be JSON objects
+
+**Status:** Active  
+**PR:** [#129](https://github.com/jmjava/documentation-generator/pull/129)  
+**Depends on:** `milestones/wizard-state-segments.md` (PR #128)
+
+## Problem
+
+PR #128 typed ``POST /api/state``. Other wizard POST/PUT handlers still
+did ``request.json or {}`` then ``.get``. A JSON **array** is truthy, so
+the default never applied and Flask raised ``AttributeError``.
+
+``bool(data.get("with_manim", False))`` treated the string ``"false"`` as
+true.
+
+## Goal
+
+Every wizard JSON body must be an object (missing body still ``{}``).
+Boolean fields (``with_manim``, ``update_requirements``, ``also_manim``,
+``yaml_generate``, ``llm_scene_spec``) must be JSON booleans when present.
+
+## Done when
+
+- [x] List / scalar POST bodies return 400 on all wizard JSON endpoints
+- [x] String ``"false"`` / ``"true"`` for bool fields return 400
+- [x] Existing object-body tests still pass
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/` (745 passed, 1 skipped)
+- [x] `docgen benchmark` (no clock change; meets baseline)
+
+## Out of scope
+
+- Invalid JSON still becomes ``{}`` via ``get_json(silent=True)``
+- Wizard ``except Exception`` around ``narration_topic_label``

milestones/wizard-state-segments.md

@@ -1,6 +1,6 @@
 # Milestone: wizard state segments must be objects
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#128](https://github.com/jmjava/documentation-generator/pull/128)  
 **Depends on:** `milestones/timing-inner-lists.md` (PR #127)
 

src/docgen/wizard.py

@@ -37,6 +37,32 @@ def require_state_segments(data: dict[str, Any], *, label: str) -> dict[str, Any
     return segs
 
 
+def request_json_object() -> dict[str, Any]:
+    """Return the JSON request body as an object. A missing body is ``{}``.
+
+    A JSON array, string, number, bool, or ``null`` raises :class:`WizardError`
+    so handlers cannot ``.get`` on a list.
+    """
+    raw = request.get_json(silent=True)
+    if raw is None:
+        return {}
+    if not isinstance(raw, dict):
+        raise WizardError(
+            f"request body must be a JSON object, not {_json_kind(raw)}"
+        )
+    return raw
+
+
+def require_json_bool(data: dict[str, Any], key: str, *, default: bool) -> bool:
+    """Return ``data[key]`` when present; reject non-bool JSON (including ``\"false\"``)."""
+    if key not in data or data[key] is None:
+        return default
+    val = data[key]
+    if not isinstance(val, bool):
+        raise WizardError(f"{key} must be a JSON boolean, not {_json_kind(val)}")
+    return val
+
+
 def session_payload(config: Any | None) -> dict[str, Any]:
     """GUI session: frozen shell vs pip CLI, and whether a bundle is attached."""
     from docgen.resources import is_frozen
@@ -477,7 +503,10 @@ def api_session():
 
     @app.route("/api/open-bundle", methods=["POST"])
     def api_open_bundle():
-        data = request.get_json(silent=True) or {}
+        try:
+            data = request_json_object()
+        except WizardError as exc:
+            return jsonify({"error": str(exc)}), 400
         try:
             cfg = open_bundle_config(str(data.get("path") or ""))
         except ValueError as exc:
@@ -514,10 +543,13 @@ def api_tool_update():
         if blocked is not None:
             return blocked
         cfg = _cfg()
-        data = request.json or {}
+        try:
+            data = request_json_object()
+            with_manim = require_json_bool(data, "with_manim", default=False)
+            update_req = require_json_bool(data, "update_requirements", default=True)
+        except WizardError as exc:
+            return jsonify({"error": str(exc)}), 400
         ref = str(data.get("ref") or "main")
-        with_manim = bool(data.get("with_manim", False))
-        update_req = bool(data.get("update_requirements", True))
         bundle = cfg.base_dir if cfg else None
         try:
             result = update_docgen_install(
@@ -577,7 +609,10 @@ def api_generate_narration():
         if blocked is not None:
             return blocked
         cfg = _cfg()
-        data = request.json or {}
+        try:
+            data = request_json_object()
+        except WizardError as exc:
+            return jsonify({"error": str(exc)}), 400
         source_paths: list[str] = list(data.get("source_paths") or [])
         guidance: str = data.get("guidance", "")
         segment_name: str = data.get("segment_name", "untitled")
@@ -686,12 +721,8 @@ def api_get_state():
     def api_set_state():
         cfg = _cfg()
         base = cfg.base_dir if cfg else Path.cwd()
-        raw = request.get_json(silent=True)
-        if raw is None:
-            raw = {}
-        if not isinstance(raw, dict):
-            return jsonify({"error": "state must be a JSON object"}), 400
         try:
+            raw = request_json_object()
             segs = require_state_segments(raw, label="state")
         except WizardError as exc:
             return jsonify({"error": str(exc)}), 400
@@ -792,12 +823,15 @@ def api_put_focus(segment_id: str):
         cfg = _cfg()
         if not cfg:
             return jsonify({"error": "no config"}), 400
-        data = request.json or {}
+        try:
+            data = request_json_object()
+            also_manim = require_json_bool(data, "also_manim", default=True)
+            do_yaml = require_json_bool(data, "yaml_generate", default=True)
+        except WizardError as exc:
+            return jsonify({"error": str(exc)}), 400
         paths = data.get("paths")
         if not isinstance(paths, list):
             return jsonify({"error": "paths must be a list of repo-root-relative strings"}), 400
-        also_manim = data.get("also_manim", True)
-        do_yaml = data.get("yaml_generate", True)
 
         root = cfg.repo_root.resolve()
         clean: list[str] = []
@@ -879,7 +913,10 @@ def api_put_narration(segment_id: str):
         cfg = _cfg()
         if not cfg:
             return jsonify({"error": "no config"}), 400
-        data = request.json or {}
+        try:
+            data = request_json_object()
+        except WizardError as exc:
+            return jsonify({"error": str(exc)}), 400
         text = data.get("text", "")
         seg_name = cfg.resolve_segment_name(segment_id)
         found = _find_asset(cfg.narration_dir, seg_name, segment_id, ".md")
@@ -1056,8 +1093,11 @@ def api_run_from(step: str, segment_id: str):
         cfg = _cfg()
         if not cfg:
             return jsonify({"error": "no config"}), 400
-        data = request.json or {}
-        llm_scene = bool(data.get("llm_scene_spec", False))
+        try:
+            data = request_json_object()
+            llm_scene = require_json_bool(data, "llm_scene_spec", default=False)
+        except WizardError as exc:
+            return jsonify({"error": str(exc)}), 400
         from docgen.asset_graph import cascade_steps
 
         try:

tests/test_wizard.py

@@ -133,7 +133,7 @@ def test_api_state_post_rejects_list_body(tmp_path):
     client, _cfg = _wizard_client(tmp_path)
     res = client.post("/api/state", json=["not", "an", "object"])
     assert res.status_code == 400
-    assert res.get_json()["error"] == "state must be a JSON object"
+    assert res.get_json()["error"] == "request body must be a JSON object, not list"
 
 
 def test_api_state_post_rejects_list_segments(tmp_path):
@@ -158,6 +158,47 @@ def test_api_state_roundtrip_object_segments(tmp_path):
     assert (cfg.base_dir / ".docgen-state.json").is_file()
 
 
+def test_api_post_rejects_list_json_bodies(tmp_path):
+    client, _cfg = _wizard_client(tmp_path)
+    endpoints = (
+        ("POST", "/api/open-bundle"),
+        ("POST", "/api/tool/update"),
+        ("POST", "/api/generate-narration"),
+        ("POST", "/api/run-from/tts/01"),
+        ("PUT", "/api/narration/01"),
+        ("PUT", "/api/segments/01/focus"),
+    )
+    for method, path in endpoints:
+        res = client.open(path, method=method, json=["not", "an", "object"])
+        assert res.status_code == 400, path
+        assert res.get_json()["error"] == "request body must be a JSON object, not list"
+
+
+def test_api_tool_update_rejects_string_with_manim(tmp_path):
+    client, _cfg = _wizard_client(tmp_path)
+    res = client.post("/api/tool/update", json={"with_manim": "false"})
+    assert res.status_code == 400
+    assert "with_manim must be a JSON boolean" in res.get_json()["error"]
+
+
+def test_api_run_from_rejects_string_llm_scene_spec(tmp_path):
+    client, _cfg = _wizard_client(tmp_path)
+    res = client.post("/api/run-from/tts/01", json={"llm_scene_spec": "true"})
+    assert res.status_code == 400
+    assert "llm_scene_spec must be a JSON boolean" in res.get_json()["error"]
+
+
+def test_api_put_focus_rejects_string_yaml_generate(tmp_path):
+    client, _cfg = _wizard_client(tmp_path)
+    res = client.put(
+        "/api/segments/01/focus",
+        json={"paths": ["README.md"], "yaml_generate": "true"},
+    )
+    assert res.status_code == 400
+    assert "yaml_generate must be a JSON boolean" in res.get_json()["error"]
+
+
+
 def test_api_file_rejects_prefix_escape(tmp_path):
     from docgen.config import Config
     from docgen.wizard import create_app