fix: reject non-finite JSON contract values - #361
codeforester wants to merge 2 commits into
Conversation
| return "true" if value else "false" | ||
| if isinstance(value, (Mapping, list, tuple)): | ||
| return json.dumps(value, separators=(",", ":")) | ||
| return json.dumps(value, separators=(",", ":"), allow_nan=False) |
There was a problem hiding this comment.
Correctness (reproduced): adding allow_nan=False to _cell_value() extends strict-JSON rejection to the CSV/TSV/table rendering path too, and render_records() writes CSV/TSV rows one at a time — so a later record's non-finite value now raises after earlier rows are already flushed to the stream. This reintroduces, for CSV/TSV, exactly the partial-output problem this PR's changelog/docs claim to fix for NDJSON. Pre-diff, the same value just embedded a literal NaN string in the cell and never raised.
| redact_json_value(dict(envelope)), | ||
| ensure_ascii=False, | ||
| separators=(",", ":"), | ||
| allow_nan=False, |
There was a problem hiding this comment.
Reuse (root cause of a gap in inspection.py): allow_nan=False was hand-added at six separate call sites across this module and output.py instead of routing through one shared strict-JSON helper. A future emitter added without remembering to pass it by hand can reintroduce silent non-finite leakage — this already happened: render_inspection_json() in lib/python/base_cli/inspection.py was not updated and still uses plain json.dumps() with no allow_nan=False, despite docs/json-contracts.md's new claim that all JSON/NDJSON emitters reject non-finite values. Worth centralizing into one choke point.
1 participant
Summary
Closes #342
Validation
uv run --extra dev --extra typer --extra quality python -m pytest tests/test_output.py tests/test_json_contracts.py -quv run --extra dev --extra typer --extra quality ruff check lib/python/base_cli/output.py lib/python/base_cli/json_contracts.py tests/test_output.py tests/test_json_contracts.pygit diff --check