From f8ee4495784957f5607fbbe06b7f9dd98b717e9c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 23:25:27 +0000 Subject: [PATCH] Fail closed when wizard JSON bodies do not parse get_json(silent=True) returned None for both a missing body and garbage JSON, so {not-json looked like {}. Parse the raw body; invalid JSON and JSON null raise WizardError. Empty bodies stay {}. Co-authored-by: jmjava --- milestones/README.md | 7 +++++-- milestones/wizard-json-parse.md | 32 +++++++++++++++++++++++++++++++ milestones/wizard-source-paths.md | 2 +- src/docgen/wizard.py | 12 +++++++++--- tests/test_wizard.py | 29 ++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 milestones/wizard-json-parse.md diff --git a/milestones/README.md b/milestones/README.md index 409806b..36a9737 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-source-paths.md](wizard-source-paths.md)** — -wizard `source_paths` must be a string array; prose fields must be strings. +**Active:** **[wizard-json-parse.md](wizard-json-parse.md)** — +wizard JSON bodies must parse; garbage JSON must not look like `{}`. **Shipped:** +- **[wizard-source-paths.md](wizard-source-paths.md)** — + wizard `source_paths` must be a string array; prose fields must be strings + (#131). - **[bootstrap-timing-helpers.md](bootstrap-timing-helpers.md)** — Manim `_load_timing` helpers must fail closed on corrupt `timing.json` (#130). diff --git a/milestones/wizard-json-parse.md b/milestones/wizard-json-parse.md new file mode 100644 index 0000000..d7ceb08 --- /dev/null +++ b/milestones/wizard-json-parse.md @@ -0,0 +1,32 @@ +# Milestone: wizard JSON bodies must parse + +**Status:** Active +**PR:** (pending) +**Depends on:** `milestones/wizard-source-paths.md` (PR #131) + +## Problem + +``request_json_object`` used Flask ``get_json(silent=True)``. Missing bodies +and **invalid JSON** both returned ``None``, which became ``{}``. A POST of +``{not-json`` looked like an empty object and could write ``.docgen-state.json`` +or run generate-narration with no fields. + +JSON ``null`` was also treated as a missing body. + +## Goal + +Empty / whitespace-only bodies stay ``{}``. Invalid JSON raises +``WizardError``. JSON ``null`` is rejected as a non-object (same as a list). + +## Done when + +- [ ] Invalid JSON POST returns 400 and does not write state +- [ ] JSON ``null`` returns 400 +- [ ] Empty body still succeeds as ``{}`` +- [ ] `ruff check src/ tests/` +- [ ] `pytest tests/` +- [ ] `docgen benchmark` (no clock change; meets baseline) + +## Out of scope + +- Wizard ``except Exception`` around ``narration_topic_label`` diff --git a/milestones/wizard-source-paths.md b/milestones/wizard-source-paths.md index e3fed91..1305da2 100644 --- a/milestones/wizard-source-paths.md +++ b/milestones/wizard-source-paths.md @@ -1,6 +1,6 @@ # Milestone: wizard source_paths and string fields must be typed -**Status:** Active +**Status:** Shipped **PR:** [#131](https://github.com/jmjava/documentation-generator/pull/131) **Depends on:** `milestones/wizard-json-object.md` (PR #129), `milestones/bootstrap-timing-helpers.md` (PR #130) diff --git a/src/docgen/wizard.py b/src/docgen/wizard.py index cd0e85d..f1d05b9 100644 --- a/src/docgen/wizard.py +++ b/src/docgen/wizard.py @@ -41,11 +41,17 @@ 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. + so handlers cannot ``.get`` on a list. Garbage JSON raises instead of + looking like an empty object (``get_json(silent=True)`` used to return + ``None`` for both missing and invalid bodies). """ - raw = request.get_json(silent=True) - if raw is None: + raw_bytes = request.get_data(cache=True) + if not raw_bytes or not raw_bytes.strip(): return {} + try: + raw = json.loads(raw_bytes) + except json.JSONDecodeError as exc: + raise WizardError(f"request body is not valid JSON ({exc})") from exc if not isinstance(raw, dict): raise WizardError( f"request body must be a JSON object, not {_json_kind(raw)}" diff --git a/tests/test_wizard.py b/tests/test_wizard.py index d8e1f9f..d6a999d 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -136,6 +136,35 @@ def test_api_state_post_rejects_list_body(tmp_path): assert res.get_json()["error"] == "request body must be a JSON object, not list" +def test_api_state_post_rejects_invalid_json(tmp_path): + client, cfg = _wizard_client(tmp_path) + res = client.post( + "/api/state", + data="{not-json", + content_type="application/json", + ) + assert res.status_code == 400 + assert "not valid JSON" in res.get_json()["error"] + assert not (cfg.base_dir / ".docgen-state.json").exists() + + +def test_api_state_post_rejects_json_null(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.post("/api/state", data="null", content_type="application/json") + assert res.status_code == 400 + assert res.get_json()["error"] == "request body must be a JSON object, not null" + + +def test_api_state_post_empty_body_is_empty_object(tmp_path): + client, cfg = _wizard_client(tmp_path) + res = client.post("/api/state") + assert res.status_code == 200 + got = client.get("/api/state") + assert got.status_code == 200 + assert got.get_json()["segments"] == {} + assert (cfg.base_dir / ".docgen-state.json").is_file() + + def test_api_state_post_rejects_list_segments(tmp_path): client, cfg = _wizard_client(tmp_path) res = client.post("/api/state", json={"segments": ["01"]})