From b0435a313180fde8b615bf89cd01b3fec4defb35 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 23:21:36 +0000 Subject: [PATCH 1/2] Fail closed when wizard source_paths is not a string array list(source_paths) treated a JSON string as characters and looked like missing files. Require a string array; type-check guidance/text/mode and focus path items so list values cannot AttributeError on .strip(). Co-authored-by: jmjava --- milestones/README.md | 7 +++- milestones/bootstrap-timing-helpers.md | 2 +- milestones/wizard-source-paths.md | 40 ++++++++++++++++++++ src/docgen/wizard.py | 52 +++++++++++++++++++++----- tests/test_wizard.py | 45 ++++++++++++++++++++++ 5 files changed, 134 insertions(+), 12 deletions(-) create mode 100644 milestones/wizard-source-paths.md diff --git a/milestones/README.md b/milestones/README.md index 867a9d3..409806b 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:** **[bootstrap-timing-helpers.md](bootstrap-timing-helpers.md)** — -Manim `_load_timing` helpers must fail closed on corrupt `timing.json`. +**Active:** **[wizard-source-paths.md](wizard-source-paths.md)** — +wizard `source_paths` must be a string array; prose fields must be strings. **Shipped:** +- **[bootstrap-timing-helpers.md](bootstrap-timing-helpers.md)** — + Manim `_load_timing` helpers must fail closed on corrupt `timing.json` + (#130). - **[wizard-json-object.md](wizard-json-object.md)** — wizard POST/PUT bodies must be JSON objects; bool fields must be booleans (#129). diff --git a/milestones/bootstrap-timing-helpers.md b/milestones/bootstrap-timing-helpers.md index d931d56..831decf 100644 --- a/milestones/bootstrap-timing-helpers.md +++ b/milestones/bootstrap-timing-helpers.md @@ -1,6 +1,6 @@ # Milestone: Manim bootstrap timing loaders must type timing.json -**Status:** Active +**Status:** Shipped **PR:** [#130](https://github.com/jmjava/documentation-generator/pull/130) **Depends on:** `milestones/timing-inner-lists.md` (PR #127), `milestones/wizard-json-object.md` (PR #129) diff --git a/milestones/wizard-source-paths.md b/milestones/wizard-source-paths.md new file mode 100644 index 0000000..50a3325 --- /dev/null +++ b/milestones/wizard-source-paths.md @@ -0,0 +1,40 @@ +# Milestone: wizard source_paths and string fields must be typed + +**Status:** Active +**PR:** (pending) +**Depends on:** `milestones/wizard-json-object.md` (PR #129), +`milestones/bootstrap-timing-helpers.md` (PR #130) + +## Problem + +PR #129 required wizard bodies to be JSON objects. ``source_paths`` still +used ``list(data.get("source_paths") or [])``. A string is iterable, so +``"README.md"`` became ``['R','E','A',…]`` and looked like missing files +instead of a type error. A list ``guidance`` / ``text`` later called +``.strip()`` and raised ``AttributeError``. + +Focus ``paths`` items were ``str()``-coerced (an integer ``1`` became +``"1"``). + +## Goal + +``source_paths`` (when present) must be a JSON array of strings. Missing +/ null still means ``[]`` (hint-path fallback). ``guidance``, +``segment_name``, ``revision_notes``, ``current_narration``, ``mode``, +``topic_label``, ``segment_id``, and PUT ``text`` must be JSON strings +when present. Focus ``paths`` items must be strings. + +## Done when + +- [ ] String ``source_paths`` returns 400 (not char-split) +- [ ] Non-string list items return 400 +- [ ] List ``guidance`` / ``text`` return 400 +- [ ] Focus path integers return 400 +- [ ] `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/src/docgen/wizard.py b/src/docgen/wizard.py index e0965a7..cd0e85d 100644 --- a/src/docgen/wizard.py +++ b/src/docgen/wizard.py @@ -63,6 +63,35 @@ def require_json_bool(data: dict[str, Any], key: str, *, default: bool) -> bool: return val +def require_json_string( + data: dict[str, Any], key: str, *, default: str | None = "" +) -> str | None: + """Return ``data[key]`` when present; reject non-string JSON.""" + if key not in data or data[key] is None: + return default + val = data[key] + if not isinstance(val, str): + raise WizardError(f"{key} must be a JSON string, not {_json_kind(val)}") + return val + + +def require_json_str_list(data: dict[str, Any], key: str) -> list[str]: + """Return ``data[key]`` as a string list. Missing / null is ``[]``.""" + if key not in data or data[key] is None: + return [] + val = data[key] + if not isinstance(val, list): + raise WizardError(f"{key} must be a JSON array, not {_json_kind(val)}") + out: list[str] = [] + for i, item in enumerate(val): + if not isinstance(item, str): + raise WizardError( + f"{key}[{i}] must be a JSON string, not {_json_kind(item)}" + ) + out.append(item) + return out + + 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 @@ -611,16 +640,16 @@ def api_generate_narration(): cfg = _cfg() try: data = request_json_object() + source_paths = require_json_str_list(data, "source_paths") + guidance = require_json_string(data, "guidance", default="") or "" + segment_name = require_json_string(data, "segment_name", default="untitled") or "untitled" + revision_notes = require_json_string(data, "revision_notes", default="") or "" + current_narration = require_json_string(data, "current_narration", default="") or "" + mode = require_json_string(data, "mode", default="generate") or "generate" + topic_label = require_json_string(data, "topic_label", default=None) + seg_id_hint = require_json_string(data, "segment_id", default=None) 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") - revision_notes: str = data.get("revision_notes", "") - current_narration: str = data.get("current_narration", "") or "" - mode: str = data.get("mode", "generate") or "generate" - topic_label: str | None = data.get("topic_label") or None - seg_id_hint: str | None = data.get("segment_id") if topic_label is None and cfg is not None and seg_id_hint: try: topic_label = cfg.narration_topic_label(seg_id_hint) @@ -832,6 +861,11 @@ def api_put_focus(segment_id: str): paths = data.get("paths") if not isinstance(paths, list): return jsonify({"error": "paths must be a list of repo-root-relative strings"}), 400 + for i, raw in enumerate(paths): + if not isinstance(raw, str): + return jsonify({ + "error": f"paths[{i}] must be a JSON string, not {_json_kind(raw)}" + }), 400 root = cfg.repo_root.resolve() clean: list[str] = [] @@ -915,9 +949,9 @@ def api_put_narration(segment_id: str): return jsonify({"error": "no config"}), 400 try: data = request_json_object() + text = require_json_string(data, "text", default="") or "" 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") target = found or (cfg.narration_dir / f"{seg_name}.md") diff --git a/tests/test_wizard.py b/tests/test_wizard.py index e73da8c..d8e1f9f 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -198,6 +198,51 @@ def test_api_put_focus_rejects_string_yaml_generate(tmp_path): assert "yaml_generate must be a JSON boolean" in res.get_json()["error"] +def test_generate_narration_rejects_string_source_paths(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.post( + "/api/generate-narration", + json={"source_paths": "README.md", "guidance": "x"}, + ) + assert res.status_code == 400 + assert "source_paths must be a JSON array" in res.get_json()["error"] + + +def test_generate_narration_rejects_non_string_source_path_items(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.post( + "/api/generate-narration", + json={"source_paths": [1], "guidance": "x"}, + ) + assert res.status_code == 400 + assert "source_paths[0] must be a JSON string" in res.get_json()["error"] + + +def test_generate_narration_rejects_list_guidance(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.post( + "/api/generate-narration", + json={"source_paths": [], "guidance": ["do", "this"]}, + ) + assert res.status_code == 400 + assert "guidance must be a JSON string" in res.get_json()["error"] + + +def test_put_narration_rejects_list_text(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.put("/api/narration/01", json={"text": ["line"]}) + assert res.status_code == 400 + assert "text must be a JSON string" in res.get_json()["error"] + + +def test_put_focus_rejects_non_string_path_items(tmp_path): + client, _cfg = _wizard_client(tmp_path) + res = client.put("/api/segments/01/focus", json={"paths": [1]}) + assert res.status_code == 400 + assert "paths[0] must be a JSON string" in res.get_json()["error"] + + + def test_api_file_rejects_prefix_escape(tmp_path): from docgen.config import Config From cfaa34f378e06be14e0058d954de808b45e23f0c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 23:22:13 +0000 Subject: [PATCH 2/2] Record PR #131 and local gate results for wizard-source-paths Co-authored-by: jmjava --- milestones/wizard-source-paths.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/milestones/wizard-source-paths.md b/milestones/wizard-source-paths.md index 50a3325..e3fed91 100644 --- a/milestones/wizard-source-paths.md +++ b/milestones/wizard-source-paths.md @@ -1,7 +1,7 @@ # Milestone: wizard source_paths and string fields must be typed **Status:** Active -**PR:** (pending) +**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) @@ -26,13 +26,13 @@ when present. Focus ``paths`` items must be strings. ## Done when -- [ ] String ``source_paths`` returns 400 (not char-split) -- [ ] Non-string list items return 400 -- [ ] List ``guidance`` / ``text`` return 400 -- [ ] Focus path integers return 400 -- [ ] `ruff check src/ tests/` -- [ ] `pytest tests/` -- [ ] `docgen benchmark` (no clock change; meets baseline) +- [x] String ``source_paths`` returns 400 (not char-split) +- [x] Non-string list items return 400 +- [x] List ``guidance`` / ``text`` return 400 +- [x] Focus path integers return 400 +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (755 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) ## Out of scope