From 86d0f6cf234fa409e859798ec30b6d85a406097e Mon Sep 17 00:00:00 2001 From: PedroHMCosme Date: Tue, 14 Jul 2026 19:18:52 -0300 Subject: [PATCH] fix(gateway): make WriteEngine + converter raise-safe against malformed write commands --- ingestor/converters.py | 2 ++ ingestor/engines/write.py | 6 ++++- tests/unit/engines/test_write.py | 40 ++++++++++++++++++++++++++++++++ tests/unit/test_converters.py | 7 ++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/ingestor/converters.py b/ingestor/converters.py index f785136..d8b5951 100644 --- a/ingestor/converters.py +++ b/ingestor/converters.py @@ -25,6 +25,8 @@ def to_typed_value(raw, data_type: str): data_type: 'boolean' | 'int' | 'float' | 'double' | 'string'. Raises ConversionError on any invalid input (never a bare TypeError). """ + if not isinstance(data_type, str): + raise ConversionError(f'data_type must be a string, got {type(data_type).__name__}') dtype = data_type.strip().lower() try: if dtype in ('bool', 'boolean'): diff --git a/ingestor/engines/write.py b/ingestor/engines/write.py index ec1242d..7bf8f68 100644 --- a/ingestor/engines/write.py +++ b/ingestor/engines/write.py @@ -88,6 +88,10 @@ class WriteEngine(SientiaMonitoring): async def write(self, cmd: dict) -> WriteResult: await self._rate_limit() + missing = [k for k in ('value', 'data_type', 'server_id', 'addr') if k not in cmd] + if missing: + return await self._emit_error(cmd, 'bad_command', f'missing fields: {missing}', 'error') + try: typed = to_typed_value(cmd['value'], cmd['data_type']) except ConversionError as e: @@ -156,7 +160,7 @@ class WriteEngine(SientiaMonitoring): return result async def _emit_success(self, cmd: dict, start: float): - tags = {'pod_id': self.pod_id, 'server_name': cmd['server_id']} + tags = {'pod_id': self.pod_id, 'server_name': cmd.get('server_id', 'unknown')} await self.emit_metric( metric_object=metrics.OPC_WRITE_TOTAL, tags={**tags, 'model_id': cmd.get('model_id', 'unknown'), 'result': 'success'}, diff --git a/tests/unit/engines/test_write.py b/tests/unit/engines/test_write.py index e92d058..d888ee7 100644 --- a/tests/unit/engines/test_write.py +++ b/tests/unit/engines/test_write.py @@ -161,6 +161,46 @@ async def test_write_bad_type_returns_error_without_raising(): ) +@pytest.mark.asyncio +async def test_write_none_data_type_returns_error_without_raising(): + eng = make_engine() + + result = await eng.write(base_cmd(data_type=None)) + + assert result.ok is False + assert result.status == 'error' + eng.sink.update_write_status.assert_awaited_once() + eng.pool.get.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_write_missing_value_key_returns_error_without_raising(): + eng = make_engine() + cmd = base_cmd() + del cmd['value'] + + result = await eng.write(cmd) + + assert result.ok is False + assert result.status == 'error' + eng.sink.update_write_status.assert_awaited_once() + eng.pool.get.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_write_missing_data_type_key_returns_error_without_raising(): + eng = make_engine() + cmd = base_cmd() + del cmd['data_type'] + + result = await eng.write(cmd) + + assert result.ok is False + assert result.status == 'error' + eng.sink.update_write_status.assert_awaited_once() + eng.pool.get.assert_not_awaited() + + @pytest.mark.asyncio async def test_write_value_raises_maps_to_error_result_not_raised(): eng = make_engine() diff --git a/tests/unit/test_converters.py b/tests/unit/test_converters.py index b5a4ff3..6fcfaa7 100644 --- a/tests/unit/test_converters.py +++ b/tests/unit/test_converters.py @@ -22,3 +22,10 @@ def test_float_and_int(): def test_invalid_raises_conversion_error(raw, dtype): with pytest.raises(ConversionError): to_typed_value(raw, dtype) + + +@pytest.mark.parametrize('dtype', [None, 123]) +def test_non_string_data_type_raises_conversion_error(dtype): + # contract: never a bare AttributeError/TypeError, always ConversionError + with pytest.raises(ConversionError): + to_typed_value('5', dtype)