fix(sec-core): fix telemetry error mapping and non-finite float sanitization

Author: RemindD

src/agent-sec-core/agent-sec-cli/src/agent_sec_cli/telemetry/sanitizer.py

@@ -1,7 +1,7 @@
 """Map SecurityEvent details into telemetry business fields."""
 
-import copy
 import json
+import math
 from datetime import datetime, timezone
 from typing import Any
 
@@ -12,9 +12,8 @@ def now_iso() -> str:
 
 
 def to_json_safe(value: Any) -> Any:
-    """Return a deep-copied JSON-safe representation of *value*."""
-    copied = copy.deepcopy(value)
-    return _make_json_safe(copied)
+    """Return a JSON-safe representation of *value*."""
+    return _make_json_safe(value)
 
 
 def details_dict(value: Any) -> dict[str, Any]:
@@ -46,28 +45,18 @@ def request_value(details: dict[str, Any]) -> Any:
     return to_json_safe(details.get("request"))
 
 
-def error_value(details: dict[str, Any], result: dict[str, Any]) -> Any:
-    """Return the best available error value from event details/result data."""
-    if "error" in details:
-        return to_json_safe(details.get("error"))
-    if "error" in result:
-        return to_json_safe(result.get("error"))
-    summary = result.get("summary")
-    if isinstance(summary, dict) and "error" in summary:
-        return to_json_safe(summary.get("error"))
-    return None
+def error_value(details: dict[str, Any]) -> Any:
+    """Return the explicit error value from event details."""
+    if "error" not in details:
+        return None
+    return to_json_safe(details.get("error"))
 
 
-def error_type_value(details: dict[str, Any], result: dict[str, Any]) -> Any:
-    """Return the best available error type from event details/result data."""
-    if "error_type" in details:
-        return to_json_safe(details.get("error_type"))
-    if "error_type" in result:
-        return to_json_safe(result.get("error_type"))
-    summary = result.get("summary")
-    if isinstance(summary, dict) and "error_type" in summary:
-        return to_json_safe(summary.get("error_type"))
-    return None
+def error_type_value(details: dict[str, Any]) -> Any:
+    """Return the explicit error type from event details."""
+    if "error_type" not in details:
+        return None
+    return to_json_safe(details.get("error_type"))
 
 
 def result_value(result: dict[str, Any], key: str) -> Any:
@@ -77,10 +66,22 @@ def result_value(result: dict[str, Any], key: str) -> Any:
     return to_json_safe(result.get(key))
 
 
+def _is_json_scalar(value: Any) -> bool:
+    """Return whether *value* can be represented as a JSON scalar."""
+    return value is None or isinstance(value, (str, bool, int, float))
+
+
+def _normalize_json_scalar(value: Any) -> Any:
+    """Return the strict JSON representation of a scalar value."""
+    if isinstance(value, float) and not math.isfinite(value):
+        return None
+    return value
+
+
 def _make_json_safe(value: Any) -> Any:
     """Convert arbitrary Python values into JSON-serializable values."""
-    if value is None or isinstance(value, (str, int, float, bool)):
-        return value
+    if _is_json_scalar(value):
+        return _normalize_json_scalar(value)
     if isinstance(value, dict):
         return {str(key): _make_json_safe(item) for key, item in value.items()}
     if isinstance(value, (list, tuple)):
@@ -93,7 +94,7 @@ def _make_json_safe(value: Any) -> Any:
         return _make_json_safe(model_dump())
 
     try:
-        json.dumps(value)
-    except TypeError:
+        json.dumps(value, allow_nan=False)
+    except (TypeError, ValueError):
         return str(value)
     return value

src/agent-sec-core/agent-sec-cli/src/agent_sec_cli/telemetry/schema.py

@@ -45,8 +45,8 @@ def _build_seccore_record(event: SecurityEvent) -> dict[str, Any]:
             "seccore.call_id": value_or_none(event.call_id),
             "seccore.tool_call_id": value_or_none(event.tool_call_id),
             "seccore.request": request_value(details),
-            "seccore.error": error_value(details, result),
-            "seccore.error_type": error_type_value(details, result),
+            "seccore.error": error_value(details),
+            "seccore.error_type": error_type_value(details),
             "seccore.verdict": result_value(result, "verdict"),
             "seccore.summary": result_value(result, "summary"),
             "seccore.elapsed_ms": result_value(result, "elapsed_ms"),
@@ -70,8 +70,8 @@ def _build_baseline_record(event: SecurityEvent) -> dict[str, Any]:
             "baseline.result": value_or_none(event.result),
             "baseline.timestamp": _timestamp(event),
             "baseline.request": request_value(details),
-            "baseline.error": error_value(details, result),
-            "baseline.error_type": error_type_value(details, result),
+            "baseline.error": error_value(details),
+            "baseline.error_type": error_type_value(details),
             "baseline.passed": result_value(result, "passed"),
             "baseline.fixed": result_value(result, "fixed"),
             "baseline.failed": result_value(result, "failed"),

src/agent-sec-core/tests/unit-test/telemetry/test_schema.py

@@ -189,7 +189,7 @@ def test_error_fields_map_from_exception_details() -> None:
     assert record["seccore.request"] == {"skill": "/path"}
 
 
-def test_error_fields_map_from_result_summary_when_present() -> None:
+def test_error_fields_do_not_fallback_to_result_summary() -> None:
     event = _event(
         event_type="pii_scan",
         category="pii_scan",
@@ -204,8 +204,34 @@ def test_error_fields_map_from_result_summary_when_present() -> None:
 
     record = build_telemetry_security_event(event)
 
-    assert record["seccore.error"] == "bad input"
-    assert record["seccore.error_type"] == "TypeError"
+    assert record["seccore.error"] is None
+    assert record["seccore.error_type"] is None
+    assert record["seccore.summary"] == {
+        "error": "bad input",
+        "error_type": "TypeError",
+    }
+
+
+def test_error_fields_preserve_explicit_details_null_over_result_values() -> None:
+    event = _event(
+        event_type="pii_scan",
+        category="pii_scan",
+        result="failed",
+        details={
+            "error": None,
+            "error_type": None,
+            "result": {
+                "error": "ignored",
+                "error_type": "IgnoredError",
+                "summary": {"error": "bad input", "error_type": "TypeError"},
+            },
+        },
+    )
+
+    record = build_telemetry_security_event(event)
+
+    assert record["seccore.error"] is None
+    assert record["seccore.error_type"] is None
 
 
 def test_missing_fields_use_null_except_generated_event_id_timestamp_and_details() -> (
@@ -228,7 +254,7 @@ def test_missing_fields_use_null_except_generated_event_id_timestamp_and_details
     assert record["seccore.details"] == {}
 
 
-def test_mapping_deep_copies_and_converts_values_to_json_safe() -> None:
+def test_mapping_does_not_mutate_input_and_converts_values_to_json_safe() -> None:
     details = {
         "request": {"items": ("a", "b")},
         "result": {"summary": {"values": {"z", "a"}}},
@@ -242,3 +268,27 @@ def test_mapping_deep_copies_and_converts_values_to_json_safe() -> None:
     assert record["seccore.request"] == {"items": ["a", "b"]}
     assert record["seccore.summary"] == {"values": ["a", "z"]}
     json.dumps(record)
+
+
+def test_mapping_converts_non_finite_floats_to_null_for_strict_json() -> None:
+    event = _event(
+        details={
+            "request": {
+                "nan": float("nan"),
+                "values": [float("inf"), float("-inf"), 1.25],
+            },
+            "error": float("nan"),
+            "result": {
+                "elapsed_ms": float("inf"),
+                "summary": {"score": float("nan")},
+            },
+        }
+    )
+
+    record = build_telemetry_security_event(event)
+
+    assert record["seccore.request"] == {"nan": None, "values": [None, None, 1.25]}
+    assert record["seccore.error"] is None
+    assert record["seccore.elapsed_ms"] is None
+    assert record["seccore.summary"] == {"score": None}
+    json.dumps(record, allow_nan=False)