fix(sdk): mask model output in the durable MessageEvent

openhands-sdk/openhands/sdk/agent/response_dispatch.py

@@ -281,7 +281,7 @@ def _emit_message_event(
         """Create and emit a MessageEvent, running critic if configured."""
         msg_event = MessageEvent(
             source="agent",
-            llm_message=message,
+            llm_message=self._mask_secrets(message, conversation),
             llm_response_id=llm_response.id,
         )
         if self.critic is not None and self.critic.mode == "finish_and_message":
@@ -293,6 +293,28 @@ def _emit_message_event(
         on_event(msg_event)
         return msg_event
 
+    @staticmethod
+    def _mask_secrets(message: Message, conversation: LocalConversation) -> Message:
+        """Return ``message`` with registered secret values masked in its text.
+
+        ``thinking_blocks`` and ``responses_reasoning_item`` are left alone:
+        they are signed provider payloads, and rewriting them invalidates the
+        signature replayed on the next request.
+        """
+        mask = conversation.state.secret_registry.mask_secrets_in_output
+        reasoning = message.reasoning_content
+        return message.model_copy(
+            update={
+                "content": [
+                    part.model_copy(update={"text": mask(part.text)})
+                    if isinstance(part, TextContent)
+                    else part
+                    for part in message.content
+                ],
+                "reasoning_content": mask(reasoning) if reasoning else reasoning,
+            }
+        )
+
     def _send_corrective_nudge(self, on_event: ConversationCallbackType) -> None:
         """Inject corrective feedback when no tool call and no content.
 

tests/sdk/agent/test_response_dispatch.py

@@ -1,5 +1,6 @@
 """Unit tests for LLM response classification and dispatch."""
 
+from collections.abc import Callable
 from unittest.mock import MagicMock
 
 import pytest
@@ -200,6 +201,8 @@ def _run_single_step(
     *,
     seed_events: list[Event] | None = None,
     record_emitted_events_in_state: bool = False,
+    secrets: dict[str, str] | None = None,
+    prepare: Callable[[LocalConversation], None] | None = None,
 ) -> tuple[list[Event], LocalConversation]:
     """Run one agent step with a canned LLM response."""
     from pydantic import PrivateAttr
@@ -220,9 +223,13 @@ def completion(  # type: ignore[override]
     agent = Agent(llm=llm, tools=[])
     conversation = Conversation(agent=agent)
     conversation._ensure_agent_ready()
+    if secrets is not None:
+        conversation.update_secrets(secrets)
     if seed_events is not None:
         for event in seed_events:
             conversation.state.append_event(event)
+    if prepare is not None:
+        prepare(conversation)
 
     events: list[Event] = []
 
@@ -397,3 +404,102 @@ def test_batch_action_events_are_emitted_consecutively():
     assert [a.tool_call_id for a in action_events] == ["tc-1", "tc-2"]
     # Only the first event of the batch carries the turn's thought.
     assert action_events[0].thought and not action_events[1].thought
+
+
+def _agent_message_text(events: list[Event]) -> str:
+    """Text of the single agent MessageEvent emitted by a step."""
+    msg_event = next(
+        e for e in events if isinstance(e, MessageEvent) and e.source == "agent"
+    )
+    part = msg_event.llm_message.content[0]
+    assert isinstance(part, TextContent)
+    return part.text
+
+
+def test_content_response_masks_registered_secret():
+    """The model's own words are masked in the durable MessageEvent.
+
+    Only tool output was masked before, so a secret the model echoed back was
+    persisted verbatim into the copy that is stored, replayed to the LLM,
+    rendered, exported and POSTed to webhooks.
+    """
+    msg = Message(
+        role="assistant", content=[TextContent(text="the value is supersecret now")]
+    )
+    events, _ = _run_single_step(
+        _make_llm_response(msg), secrets={"TOKEN": "supersecret"}
+    )
+
+    text = _agent_message_text(events)
+    assert "supersecret" not in text
+    assert text == "the value is <secret-hidden> now"
+
+
+def test_content_response_masks_secret_joined_from_stream_chunks():
+    """A secret straddling two deltas is masked in the durable message.
+
+    The ``Agent`` path accumulates deltas before it builds the ``Message``, so
+    neither chunk matches on its own — only the reassembled text does, and that
+    is what the durable copy is built from.
+    """
+    chunks = ["the value is super", "secret now"]
+    msg = Message(role="assistant", content=[TextContent(text="".join(chunks))])
+    events, _ = _run_single_step(
+        _make_llm_response(msg), secrets={"TOKEN": "supersecret"}
+    )
+
+    assert _agent_message_text(events) == "the value is <secret-hidden> now"
+
+
+def test_content_response_without_secrets_is_left_alone():
+    """With nothing registered, the message text is passed through unchanged."""
+    msg = Message(role="assistant", content=[TextContent(text="nothing to hide")])
+    events, _ = _run_single_step(_make_llm_response(msg))
+
+    assert _agent_message_text(events) == "nothing to hide"
+
+
+def test_reasoning_only_response_masks_registered_secret():
+    """The nudge path routes through the same chokepoint.
+
+    A reasoning-only message carries no ``content``, so masking that alone
+    would be a no-op here — ``reasoning_content`` is the text that leaks.
+    """
+    msg = Message(role="assistant", reasoning_content="the value is supersecret now")
+    events, _ = _run_single_step(
+        _make_llm_response(msg), secrets={"TOKEN": "supersecret"}
+    )
+
+    msg_event = next(
+        e for e in events if isinstance(e, MessageEvent) and e.source == "agent"
+    )
+    assert msg_event.llm_message.reasoning_content == "the value is <secret-hidden> now"
+
+
+def test_content_response_masks_value_dropped_from_secret_sources():
+    """Masking follows the tracked values, not the registry's source list.
+
+    ``EventService.activate_credential_binding`` pops a name from
+    ``secret_sources`` while the resolved value stays tracked for masking, so
+    keying off ``secret_sources`` would leak a live credential here.
+    """
+
+    def drop_source(convo: LocalConversation) -> None:
+        registry = convo.state.secret_registry
+        registry.get_secret_value("TOKEN")  # resolve -> tracked for masking
+        with convo.state:
+            convo.state.secret_registry = registry.model_copy(
+                update={"secret_sources": {}}
+            )
+
+    msg = Message(
+        role="assistant", content=[TextContent(text="the value is supersecret now")]
+    )
+    events, convo = _run_single_step(
+        _make_llm_response(msg),
+        secrets={"TOKEN": "supersecret"},
+        prepare=drop_source,
+    )
+
+    assert not convo.state.secret_registry.secret_sources
+    assert _agent_message_text(events) == "the value is <secret-hidden> now"