diff --git a/CHANGELOG.md b/CHANGELOG.md index ec20c2e..9dc80e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and versions are tracked in the repo-root `VERSION` file. ### Fixed +- Reject non-finite numbers in JSON and NDJSON output so emitted records remain + standards-compliant and failed NDJSON writes do not leave partial records. - Preserve explicit application identities losslessly while using collision-resistant, path-safe runtime namespace components. - Give `BatteriesIncludedConfigLoader.cli_name` a documented identity role by diff --git a/docs/json-contracts.md b/docs/json-contracts.md index 625f98c..a69db7c 100644 --- a/docs/json-contracts.md +++ b/docs/json-contracts.md @@ -53,6 +53,11 @@ contains the numeric `exit_code` and captured command stdout. A command's human output is represented as a JSON string, so it cannot introduce prose or ANSI escapes as a second stdout record. +All JSON and NDJSON emitters use strict JSON serialization and reject +non-finite numeric values (`NaN`, positive infinity, and negative infinity). +An invalid NDJSON record is fully serialized before it is written, so it does +not leave a partial line in the output stream. + `run_id` is the lifecycle run identifier when startup reached a runtime context, otherwise it is `null`. Unexpected failures intentionally expose only the generic message `Unexpected internal error.`; diagnostics stay in logs. diff --git a/lib/python/base_cli/inspection.py b/lib/python/base_cli/inspection.py index 4bbd229..7a1485d 100644 --- a/lib/python/base_cli/inspection.py +++ b/lib/python/base_cli/inspection.py @@ -1,9 +1,10 @@ from __future__ import annotations -import json from collections.abc import Mapping from typing import Any, Literal +from .json_contracts import dumps_strict_json + InspectionStatus = Literal["ok", "warn", "error"] @@ -35,7 +36,7 @@ def render_inspection_json( ) -> str: """Serialize the stable inspection envelope with Python's JSON encoder.""" return ( - json.dumps( + dumps_strict_json( inspection_envelope(command=command, status=status, data=data, error=error), ensure_ascii=False, indent=2, diff --git a/lib/python/base_cli/json_contracts.py b/lib/python/base_cli/json_contracts.py index 804c59f..1632015 100644 --- a/lib/python/base_cli/json_contracts.py +++ b/lib/python/base_cli/json_contracts.py @@ -43,6 +43,7 @@ "error_envelope", "success_envelope", "dumps_envelope", + "dumps_strict_json", "redact_json_value", ] @@ -91,7 +92,7 @@ def dumps_envelope(envelope: Mapping[str, Any]) -> str: """Serialize an envelope as one compact, newline-terminated JSON record.""" return ( - json.dumps( + dumps_strict_json( redact_json_value(dict(envelope)), ensure_ascii=False, separators=(",", ":"), @@ -100,6 +101,13 @@ def dumps_envelope(envelope: Mapping[str, Any]) -> str: ) +def dumps_strict_json(value: Any, **kwargs: Any) -> str: + """Serialize JSON while rejecting non-finite numeric values.""" + + kwargs["allow_nan"] = False + return json.dumps(value, **kwargs) + + def redact_json_value(value: Any, *, _key: str | None = None) -> Any: """Recursively redact secret-looking JSON keys and text values.""" @@ -140,7 +148,7 @@ def format(self, record: LogRecord) -> str: payload["details"] = { "exception_type": record.exc_info[0].__name__, } - return json.dumps(payload, ensure_ascii=False, separators=(",", ":")) + return dumps_strict_json(payload, ensure_ascii=False, separators=(",", ":")) def _timestamp(value: float) -> str: diff --git a/lib/python/base_cli/output.py b/lib/python/base_cli/output.py index a8d7fba..6411b25 100644 --- a/lib/python/base_cli/output.py +++ b/lib/python/base_cli/output.py @@ -15,6 +15,7 @@ from ._dependencies import require_yaml from .integrations import try_render_rich_table +from .json_contracts import dumps_strict_json PUBLIC_OUTPUT_FORMATS = ("text", "csv", "tsv", "yaml", "json", "ndjson") NDJSON_SCHEMA = "base-cli.record" @@ -62,7 +63,10 @@ def write(self, record: StructuredRecord) -> None: "schema": self.schema, "record": dict(record), } - self.stream.write(json.dumps(payload, separators=(",", ":"))) + # Serialize the complete record before touching the sink: with strict + # JSON, a nested NaN/Infinity must not leave a partial NDJSON line. + encoded = json.dumps(payload, separators=(",", ":"), allow_nan=False) + self.stream.write(encoded) self.stream.write("\n") self.stream.flush() @@ -135,9 +139,11 @@ def render_records( resolved = resolve_output_format(requested_format, stream=target) if resolved in ("csv", "tsv"): + record_list = [dict(record) for record in records] + _validate_delimited_records(record_list, columns) delimiter = "," if resolved == "csv" else "\t" writer = csv.writer(target, delimiter=delimiter, lineterminator="\n") - for record in records: + for record in record_list: writer.writerow([_delimited_value(record.get(key)) for _header, key in columns]) return resolved @@ -149,7 +155,7 @@ def render_records( record_list = [dict(record) for record in records] if resolved == "json": - target.write(json.dumps(record_list, separators=(",", ":"))) + target.write(dumps_strict_json(record_list, separators=(",", ":"))) target.write("\n") return resolved @@ -195,7 +201,7 @@ def render_document( if resolved == "text": return resolved if resolved == "json": - target.write(json.dumps(dict(document), indent=2)) + target.write(dumps_strict_json(dict(document), indent=2)) target.write("\n") return resolved if resolved == "yaml": @@ -249,10 +255,23 @@ def _cell_value(value: Any) -> str: if isinstance(value, bool): return "true" if value else "false" if isinstance(value, (Mapping, list, tuple)): - return json.dumps(value, separators=(",", ":")) + return dumps_strict_json(value, separators=(",", ":")) return str(value) +def _validate_delimited_records( + records: Sequence[Mapping[str, Any]], + columns: Sequence[tuple[str, str]], +) -> None: + """Validate nested cell values before a delimited stream is touched.""" + + for record in records: + for _header, key in columns: + value = record.get(key) + if isinstance(value, (Mapping, list, tuple)): + dumps_strict_json(value, separators=(",", ":")) + + def _delimited_value(value: Any) -> str: """Return a safe scalar for redirected CSV/TSV output. diff --git a/tests/test_json_contracts.py b/tests/test_json_contracts.py index 388475a..8e2e07d 100644 --- a/tests/test_json_contracts.py +++ b/tests/test_json_contracts.py @@ -52,6 +52,12 @@ def test_envelopes_have_stable_fields_and_recursive_redaction(self) -> None: self.assertEqual(failure["message"], "authorization=[REDACTED]") self.assertEqual(json.loads(base_cli.dumps_envelope(failure)), failure) + def test_json_contract_emitters_reject_nested_non_finite_values(self) -> None: + invalid = {"nested": [{"value": float("inf")}]} + envelope = base_cli.success_envelope(run_id=None, details=invalid) + with self.assertRaises(ValueError): + base_cli.dumps_envelope(envelope) + def test_inline_secret_redaction_keeps_delimiters_inside_values(self) -> None: for value in ("abc,def", "abc;def"): with self.subTest(value=value): diff --git a/tests/test_output.py b/tests/test_output.py index a4630a6..15bf6a3 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -65,6 +65,32 @@ def isatty(self) -> bool: class OutputTest(unittest.TestCase): + def test_json_emitters_reject_non_finite_values_without_partial_output(self) -> None: + invalid_record = {"nested": [{"value": float("nan")}]} + emitters = ( + lambda stream: render_records((invalid_record,), requested_format="json", columns=(), stream=stream), + lambda stream: render_document(invalid_record, requested_format="json", stream=stream), + lambda stream: NdjsonWriter(stream).write(invalid_record), + ) + + for emit in emitters: + with self.subTest(emit=emit): + stream = io.StringIO() + with self.assertRaises(ValueError): + emit(stream) + self.assertEqual(stream.getvalue(), "") + + def test_delimited_emitters_validate_nested_values_before_writing(self) -> None: + records = ({"name": "valid"}, {"name": {"value": float("nan")}}) + for requested_format in ("csv", "tsv"): + with self.subTest(format=requested_format): + stream = io.StringIO() + with self.assertRaises(ValueError): + render_records( + records, requested_format=requested_format, columns=(("NAME", "name"),), stream=stream + ) + self.assertEqual(stream.getvalue(), "") + def test_tsv_consumes_one_pass_iterable_without_materializing(self) -> None: consumed = False