From afcb915de351c9621fad4a452da98816cb98d59d Mon Sep 17 00:00:00 2001 From: sxl-lucky7 <60656474+sxl-lucky7@users.noreply.github.com> Date: Sun, 25 Oct 2026 11:02:02 +0800 Subject: [PATCH] fix(skills): reject non-finite JSON export values before writing (#7456) --- README.md | 4 + skills/public/data-analysis/SKILL.md | 5 ++ .../public/data-analysis/scripts/analyze.py | 7 +- .../skills/test_data_analysis_json_export.py | 85 +++++++++++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c9a5a4b560..ac61ac1fe2 100644 --- a/README.md +++ b/README.md @@ -1709,6 +1709,10 @@ before exporting. Duplicate names cause an error before the output file is opene preserving any existing file. The CLI names the repeated columns in a short stderr error and exits nonzero, so callers can correct their aliases without reading a traceback. +JSON exports likewise reject non-finite numbers (`NaN` and infinities), including +values nested in lists or structs, before opening the destination. Filter them +or replace them with `NULL` in SQL; the CLI reports an error and preserves any +existing output rather than writing invalid JSON. The **data-analysis** skill preserves each CSV file or Excel sheet when names collide across inputs. [Prepare its Python environment](skills/public/data-analysis/SKILL.md#environment-setup) explicitly with DuckDB and openpyxl; missing packages report setup instructions instead of installing themselves during analysis. Use the SQL names printed by `inspect` (such as `report`, `report_1`) for queries and summaries. Its cache follows ordered input basenames and contents, so renaming or reordering inputs rebuilds their table map instead of silently selecting a different source. Quoted original names also resolve without rewriting query text; apostrophes in upload paths and double quotes in column names are preserved through loading, inspection, queries, and summaries. diff --git a/skills/public/data-analysis/SKILL.md b/skills/public/data-analysis/SKILL.md index 777ea1fb24..f0daa233ee 100644 --- a/skills/public/data-analysis/SKILL.md +++ b/skills/public/data-analysis/SKILL.md @@ -101,6 +101,11 @@ Duplicate names cause an error and leave any existing output file untouched. The error lists the repeated names so you can alias them directly. The CLI prints a short `Error: ...` message to stderr and exits nonzero for this rejection. +JSON exports also reject non-finite numbers (`NaN`, `Infinity`, and `-Infinity`), +including values inside lists or structs, before opening the output file. Filter +or replace them in SQL, for example `CASE WHEN isfinite(ratio) THEN ratio ELSE +NULL END AS ratio`. Finite numbers, nulls, and text such as `'NaN'` remain valid. + ### Parameters | Parameter | Required | Description | diff --git a/skills/public/data-analysis/scripts/analyze.py b/skills/public/data-analysis/scripts/analyze.py index 82bd478af7..67e08d1756 100644 --- a/skills/public/data-analysis/scripts/analyze.py +++ b/skills/public/data-analysis/scripts/analyze.py @@ -383,8 +383,13 @@ def _export_results(columns: list[str], rows: list[tuple], output_file: str) -> val = val.hex() record[col] = val records.append(record) + # Reject non-JSON numbers, including nested values, before touching the + # destination so a failed export cannot overwrite an existing result. + serialized = json.dumps( + records, indent=2, ensure_ascii=False, default=str, allow_nan=False + ) with open(output_file, "w", encoding="utf-8") as f: - json.dump(records, f, indent=2, ensure_ascii=False, default=str) + f.write(serialized) elif ext == ".md": with open(output_file, "w", encoding="utf-8") as f: diff --git a/tests/skills/test_data_analysis_json_export.py b/tests/skills/test_data_analysis_json_export.py index 5f0aaf0861..30fa5183aa 100644 --- a/tests/skills/test_data_analysis_json_export.py +++ b/tests/skills/test_data_analysis_json_export.py @@ -128,6 +128,91 @@ def test_json_export_preserves_unique_and_case_distinct_keys(analysis, tmp_path) ] +@pytest.mark.parametrize( + "expression", + [ + "'NaN'::DOUBLE", + "'Infinity'::DOUBLE", + "'-Infinity'::DOUBLE", + "[1.0, 'NaN'::DOUBLE]", + "{'nested': 'Infinity'::DOUBLE}", + ], +) +@pytest.mark.parametrize("existing", [False, True]) +def test_json_export_rejects_nonfinite_values_before_writing( + analysis, tmp_path, expression, existing +): + output = tmp_path / "result.json" + original = b'[{"previous": "keep me"}]\n' + if existing: + output.write_bytes(original) + + with duckdb.connect() as con: + with pytest.raises(ValueError, match="Out of range float values"): + analysis["action_query"]( + con, f"SELECT {expression} AS value", {}, str(output) + ) + + if existing: + assert output.read_bytes() == original + else: + assert not output.exists() + + +def test_json_export_keeps_finite_numbers_nulls_and_nonfinite_text(analysis, tmp_path): + output = tmp_path / "result.json" + with duckdb.connect() as con: + analysis["action_query"]( + con, + "SELECT 1.25::DOUBLE AS amount, NULL AS missing, " + "['NaN', 'Infinity'] AS labels", + {}, + str(output), + ) + assert json.loads(output.read_text(encoding="utf-8")) == [ + {"amount": 1.25, "missing": None, "labels": ["NaN", "Infinity"]} + ] + + +@pytest.mark.parametrize("existing", [False, True]) +def test_cli_rejects_nonfinite_json_without_overwriting( + cached_source, tmp_path, existing +): + source, temp_root = cached_source + output = tmp_path / "result.json" + original = b'[{"previous": "keep me"}]\n' + if existing: + output.write_bytes(original) + completed = subprocess.run( + [ + sys.executable, + str(SCRIPT), + "--files", + str(source), + "--action", + "query", + "--sql", + "SELECT 1.0 / 0.0 AS ratio FROM input", + "--output-file", + str(output), + ], + env={**os.environ, "TMPDIR": str(temp_root)}, + capture_output=True, + text=True, + encoding="utf-8", + timeout=30, + ) + + assert completed.returncode == 1 + assert "Error: Out of range float values" in completed.stderr + assert "Traceback" not in completed.stderr + assert "Results exported" not in completed.stdout + if existing: + assert output.read_bytes() == original + else: + assert not output.exists() + + @pytest.mark.parametrize("extension", ["csv", "md"]) def test_non_json_exports_keep_duplicate_columns(analysis, tmp_path, extension): output = tmp_path / f"result.{extension}"