diff --git a/Cargo.lock b/Cargo.lock index 5963b0d..ed8ed65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,7 +40,7 @@ checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" [[package]] name = "amplifier-core" -version = "1.5.1" +version = "1.5.2" dependencies = [ "chrono", "log", @@ -76,7 +76,7 @@ dependencies = [ [[package]] name = "amplifier-core-py" -version = "1.5.1" +version = "1.5.2" dependencies = [ "amplifier-core", "log", diff --git a/bindings/python/Cargo.toml b/bindings/python/Cargo.toml index 724c794..0132271 100644 --- a/bindings/python/Cargo.toml +++ b/bindings/python/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "amplifier-core-py" -version = "1.5.1" +version = "1.5.2" edition = "2021" description = "PyO3 bridge for amplifier-core Rust kernel" license = "MIT" diff --git a/bindings/python/src/bridges.rs b/bindings/python/src/bridges.rs index 33658d4..5195d78 100644 --- a/bindings/python/src/bridges.rs +++ b/bindings/python/src/bridges.rs @@ -19,7 +19,7 @@ use amplifier_core::errors::{AmplifierError, HookError, SessionError}; use amplifier_core::models::{HookAction, HookResult}; use amplifier_core::traits::HookHandler; -use crate::helpers::{is_approval_granted, try_model_dump}; +use crate::helpers::{is_approval_granted, json_dumps_safe, try_model_dump}; // --------------------------------------------------------------------------- // PyHookHandlerBridge — wraps a Python callable as a Rust HookHandler @@ -130,13 +130,8 @@ impl HookHandler for PyHookHandlerBridge { if bound.is_none() { return Ok("{}".to_string()); } - let json_mod = py.import("json")?; let serializable = try_model_dump(bound); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract() - .unwrap_or_else(|_| "{}".to_string()); - Ok(json_str) + Ok(json_dumps_safe(py, &serializable).unwrap_or_else(|_| "{}".to_string())) }) .ok_or_else(|| HookError::HandlerFailed { message: "Failed to attach to Python runtime for result parsing".to_string(), diff --git a/bindings/python/src/coordinator/mod.rs b/bindings/python/src/coordinator/mod.rs index 1cf65ad..ead0986 100644 --- a/bindings/python/src/coordinator/mod.rs +++ b/bindings/python/src/coordinator/mod.rs @@ -14,7 +14,7 @@ use pyo3::types::{PyDict, PyList}; use serde_json::Value; use crate::cancellation::PyCancellationToken; -use crate::helpers::{try_model_dump, wrap_future_as_coroutine}; +use crate::helpers::{json_dumps_safe, try_model_dump, wrap_future_as_coroutine}; use crate::hooks::PyHookRegistry; mod capabilities; @@ -114,11 +114,8 @@ impl PyCoordinator { }; let cfg = sess.getattr("config")?; let rc: HashMap = { - let json_mod = py.import("json")?; let serializable = try_model_dump(&cfg); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; serde_json::from_str(&json_str).unwrap_or_else(|e| { log::warn!("Failed to parse session config as JSON object (using empty config): {e}"); HashMap::new() diff --git a/bindings/python/src/helpers.rs b/bindings/python/src/helpers.rs index c98e97c..92d8189 100644 --- a/bindings/python/src/helpers.rs +++ b/bindings/python/src/helpers.rs @@ -3,6 +3,7 @@ // --------------------------------------------------------------------------- use pyo3::prelude::*; +use pyo3::types::PyDict; /// Parse an approval system's decision string into a boolean. /// @@ -44,3 +45,20 @@ pub(crate) fn try_model_dump<'py>(obj: &Bound<'py, PyAny>) -> Bound<'py, PyAny> } } } + +/// Serialize a Python object to a JSON string with `default=str` fallback. +/// +/// Like `json.dumps(obj)` but passes Python's built-in `str` as the `default=` +/// callable so non-JSON-native types (e.g. `decimal.Decimal`, `datetime`) +/// become their string representation instead of raising `TypeError`. +/// +/// Use this everywhere we call `json.dumps()` at the Python/Rust FFI boundary. +pub(crate) fn json_dumps_safe<'py>(py: Python<'py>, obj: &Bound<'py, PyAny>) -> PyResult { + let json_mod = py.import("json")?; + let str_fn = py.import("builtins")?.getattr("str")?; + let kwargs = PyDict::new(py); + kwargs.set_item("default", &str_fn)?; + json_mod + .call_method("dumps", (obj,), Some(&kwargs))? + .extract() +} diff --git a/bindings/python/src/hooks.rs b/bindings/python/src/hooks.rs index 2aa125b..ba0a7d3 100644 --- a/bindings/python/src/hooks.rs +++ b/bindings/python/src/hooks.rs @@ -11,7 +11,7 @@ use pyo3::types::{PyDict, PyList}; use serde_json::Value; use crate::bridges::PyHookHandlerBridge; -use crate::helpers::{try_model_dump, wrap_future_as_coroutine}; +use crate::helpers::{json_dumps_safe, try_model_dump, wrap_future_as_coroutine}; // --------------------------------------------------------------------------- // PyUnregisterFn — callable returned by PyHookRegistry.register() @@ -129,11 +129,8 @@ impl PyHookRegistry { ) -> PyResult> { let inner = self.inner.clone(); // Convert Python data to serde_json::Value - let json_mod = py.import("json")?; let serializable = try_model_dump(&data); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let value: Value = serde_json::from_str(&json_str) .map_err(|e| PyErr::new::(format!("Invalid JSON: {e}")))?; @@ -186,8 +183,7 @@ impl PyHookRegistry { fn set_default_fields(&self, kwargs: Option<&Bound<'_, PyDict>>) -> PyResult<()> { let value = match kwargs { Some(dict) => { - let json_mod = dict.py().import("json")?; - let json_str: String = json_mod.call_method1("dumps", (dict,))?.extract()?; + let json_str = json_dumps_safe(dict.py(), dict.as_any())?; serde_json::from_str(&json_str) .map_err(|e| PyErr::new::(format!("Invalid JSON: {e}")))? } @@ -238,11 +234,8 @@ impl PyHookRegistry { timeout: f64, ) -> PyResult> { let inner = self.inner.clone(); - let json_mod = py.import("json")?; let serializable = try_model_dump(&data); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let value: Value = serde_json::from_str(&json_str) .map_err(|e| PyErr::new::(format!("Invalid JSON: {e}")))?; let timeout_dur = std::time::Duration::from_secs_f64(timeout); diff --git a/bindings/python/src/session.rs b/bindings/python/src/session.rs index c1267b7..9f1a63a 100644 --- a/bindings/python/src/session.rs +++ b/bindings/python/src/session.rs @@ -9,7 +9,7 @@ use pyo3::prelude::*; use pyo3::types::PyDict; use serde_json::Value; -use crate::helpers::wrap_future_as_coroutine; +use crate::helpers::{json_dumps_safe, wrap_future_as_coroutine}; use crate::hooks::PyHookRegistry; // --------------------------------------------------------------------------- @@ -109,8 +109,7 @@ impl PySession { } // ---- Build Rust kernel Session ---- - let json_mod = py.import("json")?; - let json_str: String = json_mod.call_method1("dumps", (config,))?.extract()?; + let json_str: String = json_dumps_safe(py, config.as_any())?; let value: Value = serde_json::from_str(&json_str) .map_err(|e| PyErr::new::(format!("Invalid config JSON: {e}")))?; let session_config = amplifier_core::SessionConfig::from_value(value) diff --git a/bindings/python/src/tests/mod.rs b/bindings/python/src/tests/mod.rs index 6441511..cc8560a 100644 --- a/bindings/python/src/tests/mod.rs +++ b/bindings/python/src/tests/mod.rs @@ -116,3 +116,62 @@ fn load_wasm_from_path_rejects_rust_transport_with_specific_message() { "load_wasm_from_path cannot load Rust modules. Use the gRPC sidecar pattern instead." ); } + +/// Verify `json_dumps_safe` exists in helpers with the correct signature. +/// +/// This test fails to compile until `json_dumps_safe` is added to helpers.rs. +/// Signature: `fn(Python<'_>, &Bound<'_, PyAny>) -> PyResult`. +#[test] +fn json_dumps_safe_signature_compiles() { + let _: fn(Python<'_>, &Bound<'_, PyAny>) -> PyResult = crate::helpers::json_dumps_safe; +} + +/// Structural guard: no raw `json.dumps()` calls outside `helpers.rs`. +/// +/// All `json.dumps()` at the Python/Rust FFI boundary must go through +/// `json_dumps_safe()` (which passes `default=str`) to prevent TypeError +/// crashes on non-JSON-native types like `Decimal` or `datetime`. +/// +/// If this test fails, you added a `json.dumps()` call in a binding file. +/// Replace: `json_mod.call_method1("dumps", (&obj,))` +/// With: `json_dumps_safe(py, &obj)` (from `crate::helpers`) +#[test] +fn no_raw_json_dumps_outside_helpers() { + use std::fs; + use std::path::Path; + + fn check_dir(dir: &Path, violations: &mut Vec) { + if let Ok(entries) = fs::read_dir(dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + // Skip tests/ — no production FFI code there + if path.file_name().map_or(false, |n| n == "tests") { + continue; + } + check_dir(&path, violations); + } else if path.extension().map_or(false, |e| e == "rs") + && path.file_name().map_or(false, |n| n != "helpers.rs") + { + if let Ok(content) = fs::read_to_string(&path) { + if content.contains(r#"call_method1("dumps""#) + || content.contains(r#"call_method("dumps""#) + { + violations.push(path.display().to_string()); + } + } + } + } + } + } + + let src_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut violations = Vec::new(); + check_dir(&src_dir, &mut violations); + + assert!( + violations.is_empty(), + "Raw json.dumps() found outside helpers.rs — use json_dumps_safe() instead:\n{}", + violations.join("\n") + ); +} diff --git a/bindings/python/src/wasm.rs b/bindings/python/src/wasm.rs index 6bc591a..79cf647 100644 --- a/bindings/python/src/wasm.rs +++ b/bindings/python/src/wasm.rs @@ -14,7 +14,7 @@ use pyo3::types::PyDict; use serde_json::Value; use crate::coordinator::PyCoordinator; -use crate::helpers::{try_model_dump, wrap_future_as_coroutine}; +use crate::helpers::{json_dumps_safe, try_model_dump, wrap_future_as_coroutine}; // --------------------------------------------------------------------------- // PyWasmTool — thin Python wrapper around a Rust Arc @@ -75,11 +75,8 @@ impl PyWasmTool { let inner = self.inner.clone(); // Convert Python input to serde_json::Value - let json_mod = py.import("json")?; let serializable = try_model_dump(&input); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let value: Value = serde_json::from_str(&json_str) .map_err(|e| PyErr::new::(format!("Invalid JSON input: {e}")))?; @@ -199,11 +196,8 @@ impl PyWasmProvider { let inner = self.inner.clone(); // Convert Python request to serde_json::Value - let json_mod = py.import("json")?; let serializable = try_model_dump(&request); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let chat_request: amplifier_core::messages::ChatRequest = serde_json::from_str(&json_str) .map_err(|e| { PyErr::new::(format!("Invalid ChatRequest JSON: {e}")) @@ -242,9 +236,7 @@ impl PyWasmProvider { fn parse_tool_calls(&self, py: Python<'_>, response: Bound<'_, PyAny>) -> PyResult> { let json_mod = py.import("json")?; let serializable = try_model_dump(&response); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let chat_response: amplifier_core::messages::ChatResponse = serde_json::from_str(&json_str) .map_err(|e| { PyErr::new::(format!("Invalid ChatResponse JSON: {e}")) @@ -300,11 +292,8 @@ impl PyWasmHook { ) -> PyResult> { let inner = self.inner.clone(); - let json_mod = py.import("json")?; let serializable = try_model_dump(&data); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let value: Value = serde_json::from_str(&json_str).map_err(|e| { PyErr::new::(format!("Invalid JSON for hook data: {e}")) })?; @@ -372,11 +361,8 @@ impl PyWasmContext { ) -> PyResult> { let inner = self.inner.clone(); - let json_mod = py.import("json")?; let serializable = try_model_dump(&message); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let value: Value = serde_json::from_str(&json_str) .map_err(|e| PyErr::new::(format!("Invalid JSON for message: {e}")))?; @@ -475,8 +461,7 @@ impl PyWasmContext { ) -> PyResult> { let inner = self.inner.clone(); - let json_mod = py.import("json")?; - let json_str: String = json_mod.call_method1("dumps", (&messages,))?.extract()?; + let json_str: String = json_dumps_safe(py, messages.as_any())?; let values: Vec = serde_json::from_str(&json_str).map_err(|e| { PyErr::new::(format!("Invalid JSON for messages: {e}")) })?; @@ -707,11 +692,8 @@ impl PyWasmApproval { ) -> PyResult> { let inner = self.inner.clone(); - let json_mod = py.import("json")?; let serializable = try_model_dump(&request); - let json_str: String = json_mod - .call_method1("dumps", (&serializable,))? - .extract()?; + let json_str: String = json_dumps_safe(py, &serializable)?; let approval_request: amplifier_core::models::ApprovalRequest = serde_json::from_str(&json_str).map_err(|e| { PyErr::new::(format!("Invalid ApprovalRequest JSON: {e}")) diff --git a/bindings/python/tests/test_emit_json_sanitize.py b/bindings/python/tests/test_emit_json_sanitize.py new file mode 100644 index 0000000..1517333 --- /dev/null +++ b/bindings/python/tests/test_emit_json_sanitize.py @@ -0,0 +1,113 @@ +"""Tests for non-JSON-native type safety at the emit() FFI boundary. + +Verifies that emit(), emit_and_collect(), and hook result serialization +never crash on non-JSON-native Python types (e.g. Decimal, datetime). +The fix: json.dumps(..., default=str) at all FFI call sites. +""" + +import pytest + +from amplifier_core._engine import RustHookRegistry + + +@pytest.mark.asyncio +async def test_emit_with_decimal_does_not_crash(): + """emit() must not raise when the event payload contains a Decimal. + + Without the fix, json.dumps({"cost": Decimal("1.23")}) raises TypeError + at the FFI boundary, crashing the caller before the handler is ever invoked. + """ + from decimal import Decimal + + registry = RustHookRegistry() + received = [] + + def handler(event, data): + received.append(data) + return None + + registry.register("test:event", handler, 0, name="test-hook") + await registry.emit("test:event", {"cost": Decimal("1.23")}) + assert len(received) == 1 + + +@pytest.mark.asyncio +async def test_emit_decimal_serializes_as_string(): + """emit() must serialize Decimal values as their str() representation. + + str(Decimal("1.23")) == "1.23", which matches the @field_serializer + output on the Pydantic model path — no inconsistency. + """ + from decimal import Decimal + + registry = RustHookRegistry() + received = [] + + def handler(event, data): + received.append(data) + return None + + registry.register("test:event", handler, 0, name="test-hook") + await registry.emit("test:event", {"cost": Decimal("1.23")}) + assert received[0]["cost"] == "1.23" + + +@pytest.mark.asyncio +async def test_emit_datetime_does_not_crash(): + """emit() must not raise when the event payload contains a datetime. + + datetime objects are not JSON-native. str(datetime(2024,1,1,12,0,0)) + produces "2024-01-01 12:00:00". + """ + from datetime import datetime + + registry = RustHookRegistry() + received = [] + + def handler(event, data): + received.append(data) + return None + + registry.register("test:event", handler, 0, name="test-hook") + await registry.emit("test:event", {"ts": datetime(2024, 1, 1, 12, 0, 0)}) + assert len(received) == 1 + assert received[0]["ts"] == "2024-01-01 12:00:00" + + +@pytest.mark.asyncio +async def test_emit_and_collect_with_decimal_does_not_crash(): + """emit_and_collect() must not raise on Decimal in the event payload. + + This exercises a separate json.dumps() call site from emit() — + both FFI entry points must be fixed. + """ + from decimal import Decimal + + registry = RustHookRegistry() + + def handler(event, data): + return {"action": "continue", "data": {}} + + registry.register("test:event", handler, 0, name="test-hook") + results = await registry.emit_and_collect("test:event", {"cost": Decimal("1.23")}) + assert isinstance(results, list) + + +@pytest.mark.asyncio +async def test_hook_result_with_decimal_in_data_does_not_crash(): + """A hook handler returning a dict with Decimal must not crash. + + This exercises the bridges.rs result-serialization path (Step 3 inside + PyHookHandlerBridge::handle()), a separate call site from the emit() + input-serialization path. + """ + from decimal import Decimal + + registry = RustHookRegistry() + + def handler(event, data): + return {"action": "continue", "data": {"cost": Decimal("2.50")}} + + registry.register("test:event", handler, 0, name="test-hook") + # Must not raise — handler returns Decimal in result dict + await registry.emit("test:event", {"input": "test"}) diff --git a/bindings/python/tests/test_loader_grpc.py b/bindings/python/tests/test_loader_grpc.py index 71a91a7..e482680 100644 --- a/bindings/python/tests/test_loader_grpc.py +++ b/bindings/python/tests/test_loader_grpc.py @@ -60,6 +60,28 @@ def test_grpc_tool_bridge_serialize_input(): assert json.loads(data) == {"query": "hello world"} +def test_grpc_tool_bridge_serialize_input_with_decimal(): + """_serialize_input must not raise on Decimal values (default=str fallback). + + Without default=str, json.dumps({"cost": Decimal("1.23")}) raises TypeError. + str(Decimal("1.23")) == "1.23". + """ + from decimal import Decimal + + from amplifier_core.loader_grpc import GrpcToolBridge + + bridge = GrpcToolBridge( + name="test", + description="test", + parameters_json="{}", + endpoint="localhost:50052", + channel=None, + ) + data, content_type = bridge._serialize_input({"cost": Decimal("1.23")}) + assert content_type == "application/json" + assert json.loads(data)["cost"] == "1.23" + + def test_grpc_tool_bridge_deserialize_output(): """GrpcToolBridge._deserialize_output decodes JSON bytes to dict.""" from amplifier_core.loader_grpc import GrpcToolBridge diff --git a/crates/amplifier-core/Cargo.toml b/crates/amplifier-core/Cargo.toml index b4afc68..6fbff0f 100644 --- a/crates/amplifier-core/Cargo.toml +++ b/crates/amplifier-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "amplifier-core" -version = "1.5.1" +version = "1.5.2" edition = "2021" description = "Pure Rust kernel for the Amplifier modular AI agent system" license = "MIT" diff --git a/pyproject.toml b/pyproject.toml index 979eb51..4943aeb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "amplifier-core" -version = "1.5.1" +version = "1.5.2" description = "Rust kernel with Python bindings for the Amplifier modular AI agent framework" license = "MIT" readme = "README.md" diff --git a/python/amplifier_core/__init__.py b/python/amplifier_core/__init__.py index d55d06f..f27c52a 100644 --- a/python/amplifier_core/__init__.py +++ b/python/amplifier_core/__init__.py @@ -6,7 +6,7 @@ AmplifierSession`) still give the pure-Python implementations. """ -__version__ = "1.5.1" +__version__ = "1.5.2" # --- Rust-backed primary types (THE SWITCHOVER) --- # These four were previously imported from their Python submodules. diff --git a/python/amplifier_core/loader_grpc.py b/python/amplifier_core/loader_grpc.py index 0e2e48c..41b0c3c 100644 --- a/python/amplifier_core/loader_grpc.py +++ b/python/amplifier_core/loader_grpc.py @@ -84,7 +84,7 @@ def _serialize_input(self, input_dict: dict[str, Any]) -> tuple[bytes, str]: Returns: Tuple of (payload_bytes, content_type_string) """ - data = json.dumps(input_dict).encode("utf-8") + data = json.dumps(input_dict, default=str).encode("utf-8") return data, "application/json" def _deserialize_output(self, output_bytes: bytes, content_type: str) -> Any: diff --git a/scripts/bump_version.py b/scripts/bump_version.py index 3249896..ede634f 100644 --- a/scripts/bump_version.py +++ b/scripts/bump_version.py @@ -33,7 +33,7 @@ ] SEMVER_RE = re.compile(r"^\d+\.\d+\.\d+$") -VERSION_LINE_RE = re.compile(r'^((?:__)?version\s*=\s*")([^"]+)(")', re.MULTILINE) +VERSION_LINE_RE = re.compile(r'^((?:__)?version(?:__)?\s*=\s*")([^"]+)(")', re.MULTILINE) def die(msg: str) -> NoReturn: