From 755b9619303a046204321282900c94d72fb624ab Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 23:12:16 +0000 Subject: [PATCH 1/2] Fail closed when wizard JSON bodies are not objects POST/PUT handlers used request.json or {} then .get. A JSON array is truthy, so Flask raised AttributeError. Require an object body and JSON booleans for with_manim / yaml_generate / llm_scene_spec flags. Co-authored-by: jmjava --- milestones/README.md | 7 ++- milestones/wizard-json-object.md | 34 ++++++++++++++ milestones/wizard-state-segments.md | 2 +- src/docgen/wizard.py | 72 ++++++++++++++++++++++------- tests/test_wizard.py | 43 ++++++++++++++++- 5 files changed, 138 insertions(+), 20 deletions(-) create mode 100644 milestones/wizard-json-object.md diff --git a/milestones/README.md b/milestones/README.md index fd98ffa..4ccf43a 100644 --- a/milestones/README.md +++ b/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). diff --git a/milestones/wizard-json-object.md b/milestones/wizard-json-object.md new file mode 100644 index 0000000..825f7ad --- /dev/null +++ b/milestones/wizard-json-object.md @@ -0,0 +1,34 @@ +# Milestone: wizard POST bodies must be JSON objects + +**Status:** Active +**PR:** (pending) +**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 + +- [ ] List / scalar POST bodies return 400 on all wizard JSON endpoints +- [ ] String ``"false"`` / ``"true"`` for bool fields return 400 +- [ ] Existing object-body tests still pass +- [ ] `ruff check src/ tests/` +- [ ] `pytest tests/` +- [ ] `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`` diff --git a/milestones/wizard-state-segments.md b/milestones/wizard-state-segments.md index 2783630..5aef409 100644 --- a/milestones/wizard-state-segments.md +++ b/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) diff --git a/src/docgen/wizard.py b/src/docgen/wizard.py index a72e12a..e0965a7 100644 --- a/src/docgen/wizard.py +++ b/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: diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 34f9992..28c5912 100644 --- a/tests/test_wizard.py +++ b/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" 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" + + +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 From 199132e6c7db1016a4ef62cf06a6eb94e11c6f0f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 23:13:03 +0000 Subject: [PATCH 2/2] Record PR #129 and match list-body error text Co-authored-by: jmjava --- milestones/wizard-json-object.md | 14 +++++++------- tests/test_wizard.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/milestones/wizard-json-object.md b/milestones/wizard-json-object.md index 825f7ad..162a89a 100644 --- a/milestones/wizard-json-object.md +++ b/milestones/wizard-json-object.md @@ -1,7 +1,7 @@ # Milestone: wizard POST bodies must be JSON objects **Status:** Active -**PR:** (pending) +**PR:** [#129](https://github.com/jmjava/documentation-generator/pull/129) **Depends on:** `milestones/wizard-state-segments.md` (PR #128) ## Problem @@ -21,12 +21,12 @@ Boolean fields (``with_manim``, ``update_requirements``, ``also_manim``, ## Done when -- [ ] List / scalar POST bodies return 400 on all wizard JSON endpoints -- [ ] String ``"false"`` / ``"true"`` for bool fields return 400 -- [ ] Existing object-body tests still pass -- [ ] `ruff check src/ tests/` -- [ ] `pytest tests/` -- [ ] `docgen benchmark` (no clock change; meets baseline) +- [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 diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 28c5912..e73da8c 100644 --- a/tests/test_wizard.py +++ b/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"] == "request body 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): @@ -171,7 +171,7 @@ def test_api_post_rejects_list_json_bodies(tmp_path): 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" + 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):