From e68ef667a3d720711f0f7cd2753aef5540f4b821 Mon Sep 17 00:00:00 2001 From: lisry-1 <249596828+lisry-1@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:18:38 +0800 Subject: [PATCH 1/3] fix(redis): skip malformed history entries instead of raising on read get_messages deserialized every Redis list entry without per-entry error handling, so a single empty string or corrupt JSON entry would raise JSONDecodeError / ValueError and abort the whole read. CosmosHistoryProvider already skips such entries with a warning. Wrap each entry in try/except (ValueError, json.JSONDecodeError), log a warning, and continue, matching the Cosmos behavior. --- .../redis/agent_framework_redis/_history_provider.py | 8 +++++++- python/packages/redis/tests/test_providers.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/python/packages/redis/agent_framework_redis/_history_provider.py b/python/packages/redis/agent_framework_redis/_history_provider.py index 07827ac62aa..5bcbdf0b313 100644 --- a/python/packages/redis/agent_framework_redis/_history_provider.py +++ b/python/packages/redis/agent_framework_redis/_history_provider.py @@ -9,6 +9,7 @@ from __future__ import annotations import json +import logging import warnings from collections.abc import Awaitable, Sequence from inspect import isawaitable @@ -23,6 +24,8 @@ from ._feature_usage import FeatureIndex +logger = logging.getLogger(__name__) + _T = TypeVar("_T") @@ -271,7 +274,10 @@ async def get_messages( redis_messages = cast("list[str]", await _redis_result(cast("Any", self._redis_client).lrange(key, 0, -1))) messages: list[Message] = [] for serialized in redis_messages: - messages.append(Message.from_dict(self._deserialize_json(serialized))) + try: + messages.append(Message.from_dict(self._deserialize_json(serialized))) + except (ValueError, json.JSONDecodeError) as exc: + logger.warning("Skipping malformed Redis history entry: %s", exc) return messages async def save_messages( diff --git a/python/packages/redis/tests/test_providers.py b/python/packages/redis/tests/test_providers.py index c8aba3a2e55..bb81f542d10 100644 --- a/python/packages/redis/tests/test_providers.py +++ b/python/packages/redis/tests/test_providers.py @@ -681,6 +681,18 @@ async def test_empty_returns_empty(self, mock_redis_client: MagicMock): messages = await provider.get_messages("s1") assert messages == [] + async def test_skips_malformed_entries(self, mock_redis_client: MagicMock): + msg = Message(role="user", contents=["Hello"]) + mock_redis_client.lrange = AsyncMock(return_value=["", "not-json", json.dumps(msg.to_dict())]) + + with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url: + mock_from_url.return_value = mock_redis_client + provider = RedisHistoryProvider("mem", redis_url="redis://localhost:6379", application_id="test-app") + + messages = await provider.get_messages("s1") + assert len(messages) == 1 + assert messages[0].text == "Hello" + async def test_returns_messages_when_lrange_is_synchronous(self, mock_redis_client: MagicMock): """redis-py types several commands as returning a value or an awaitable; handle both.""" msg = Message(role="user", contents=["Hello"]) From 1a0c129f5aa15c03468e0dc72df10a9ef54d62d0 Mon Sep 17 00:00:00 2001 From: lisry-1 <249596828+lisry-1@users.noreply.github.com> Date: Sun, 1 Nov 2026 20:28:34 +0800 Subject: [PATCH 2/3] fix(redis): tolerate malformed message shapes in history read Valid JSON that deserializes to a non-dict (null, []) or a dict missing required fields ({}) previously raised AttributeError/TypeError from Message.from_dict, which was not caught and aborted the entire read. Guard the decoded payload with an isinstance-dict check and broaden the except clause to (ValueError, TypeError, AttributeError, JSONDecodeError). --- .../redis/agent_framework_redis/_history_provider.py | 7 +++++-- python/packages/redis/tests/test_providers.py | 11 ++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/python/packages/redis/agent_framework_redis/_history_provider.py b/python/packages/redis/agent_framework_redis/_history_provider.py index 5bcbdf0b313..c874e274e27 100644 --- a/python/packages/redis/agent_framework_redis/_history_provider.py +++ b/python/packages/redis/agent_framework_redis/_history_provider.py @@ -275,8 +275,11 @@ async def get_messages( messages: list[Message] = [] for serialized in redis_messages: try: - messages.append(Message.from_dict(self._deserialize_json(serialized))) - except (ValueError, json.JSONDecodeError) as exc: + payload = self._deserialize_json(serialized) + if not isinstance(payload, dict): + raise ValueError(f"Expected a JSON object, got {type(payload).__name__}") + messages.append(Message.from_dict(payload)) + except (ValueError, TypeError, AttributeError, json.JSONDecodeError) as exc: logger.warning("Skipping malformed Redis history entry: %s", exc) return messages diff --git a/python/packages/redis/tests/test_providers.py b/python/packages/redis/tests/test_providers.py index bb81f542d10..31e49339bfb 100644 --- a/python/packages/redis/tests/test_providers.py +++ b/python/packages/redis/tests/test_providers.py @@ -683,7 +683,16 @@ async def test_empty_returns_empty(self, mock_redis_client: MagicMock): async def test_skips_malformed_entries(self, mock_redis_client: MagicMock): msg = Message(role="user", contents=["Hello"]) - mock_redis_client.lrange = AsyncMock(return_value=["", "not-json", json.dumps(msg.to_dict())]) + mock_redis_client.lrange = AsyncMock( + return_value=[ + "", + "not-json", + "null", # valid JSON, but deserializes to None + "[]", # valid JSON, but deserializes to a list + "{}", # valid JSON, but missing the required "role" field + json.dumps(msg.to_dict()), + ] + ) with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url: mock_from_url.return_value = mock_redis_client From 9a1cd01869e3891d5fc966087e2587a653634675 Mon Sep 17 00:00:00 2001 From: lisry-1 <249596828+lisry-1@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:33:41 +0800 Subject: [PATCH 3/3] fix(redis): log exception class instead of message when skipping malformed history Message.from_dict embeds the payload's type value in mismatch errors, so logging str(exc) copies untrusted history data into application logs. Log the exception class name instead and add a regression asserting the sensitive value is absent from the warning. --- .../_history_provider.py | 7 +++++- python/packages/redis/tests/test_providers.py | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/python/packages/redis/agent_framework_redis/_history_provider.py b/python/packages/redis/agent_framework_redis/_history_provider.py index c874e274e27..06f34e7c9f5 100644 --- a/python/packages/redis/agent_framework_redis/_history_provider.py +++ b/python/packages/redis/agent_framework_redis/_history_provider.py @@ -280,7 +280,12 @@ async def get_messages( raise ValueError(f"Expected a JSON object, got {type(payload).__name__}") messages.append(Message.from_dict(payload)) except (ValueError, TypeError, AttributeError, json.JSONDecodeError) as exc: - logger.warning("Skipping malformed Redis history entry: %s", exc) + # Log the exception class, not its message: ``Message.from_dict`` + # embeds the payload's ``type`` value in mismatch errors, so logging + # ``str(exc)`` could copy untrusted history data into application logs. + logger.warning( + "Skipping malformed Redis history entry (%s)", type(exc).__name__ + ) return messages async def save_messages( diff --git a/python/packages/redis/tests/test_providers.py b/python/packages/redis/tests/test_providers.py index 31e49339bfb..ba4e5356033 100644 --- a/python/packages/redis/tests/test_providers.py +++ b/python/packages/redis/tests/test_providers.py @@ -702,6 +702,29 @@ async def test_skips_malformed_entries(self, mock_redis_client: MagicMock): assert len(messages) == 1 assert messages[0].text == "Hello" + async def test_malformed_entries_do_not_leak_payload_in_logs( + self, mock_redis_client: MagicMock, caplog: pytest.LogCaptureFixture + ): + # A payload whose ``type`` field carries a private value must not be echoed + # into the skip warning: ``Message.from_dict`` embeds ``type`` in mismatch + # errors, and logging ``str(exc)`` would copy it into application logs. + sensitive = "secret-internal-token" + mock_redis_client.lrange = AsyncMock( + return_value=[json.dumps({"type": sensitive, "role": "unknown"})] + ) + + with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url: + mock_from_url.return_value = mock_redis_client + provider = RedisHistoryProvider("mem", redis_url="redis://localhost:6379", application_id="test-app") + + with caplog.at_level("WARNING", logger="agent_framework_redis._history_provider"): + messages = await provider.get_messages("s1") + + assert messages == [] + warning_text = "\n".join(r.message for r in caplog.records) + assert "Skipping malformed Redis history entry" in warning_text + assert sensitive not in warning_text + async def test_returns_messages_when_lrange_is_synchronous(self, mock_redis_client: MagicMock): """redis-py types several commands as returning a value or an awaitable; handle both.""" msg = Message(role="user", contents=["Hello"])