diff --git a/docs/specs/004-python-function-calling-loop.md b/docs/specs/004-python-function-calling-loop.md index bdd6599c69a..8381d6b65c6 100644 --- a/docs/specs/004-python-function-calling-loop.md +++ b/docs/specs/004-python-function-calling-loop.md @@ -738,6 +738,7 @@ that manually replay messages own the equivalent rule: do not resend an approval | Foundry encrypted reasoning opt-in | Foundry clients omit `reasoning.encrypted_content` by default and preserve an explicit caller opt-in. | `packages/foundry/tests/foundry/test_foundry_chat_client.py::test_get_response_does_not_request_encrypted_reasoning_by_default`, `test_get_response_preserves_explicit_encrypted_reasoning_opt_in`, `packages/foundry/tests/foundry/test_foundry_agent.py::test_foundry_agent_basic_call_does_not_request_unsupported_encrypted_reasoning`, `test_foundry_agent_preserves_caller_requested_encrypted_reasoning`, `packages/foundry_hosting/tests/test_responses_int.py::TestReasoningHostedMcpReplay::test_second_turn_replays_mcp_call_with_encrypted_reasoning` | | Opaque reasoning signature replay | Provider-specific opaque reasoning metadata is captured and restored on reconstructed calls. | `packages/gemini/tests/test_gemini_client.py::test_function_call_part_captures_thought_signature_as_reasoning_content`, `test_reconstructed_function_call_replays_thought_signature_from_reasoning_content` | | Chat Completions approval wrappers | Framework approval wrappers are not sent as chat messages. | `packages/openai/tests/openai/test_openai_chat_completion_client.py` approval serialization tests | +| DevUI conversation-bound approval ownership | A pending function approval is bound to the conversation that originated it. A response from another conversation is rejected without consuming the pending approval, and identical request IDs in different conversations remain independent. | `packages/devui/tests/devui/test_approval_validation.py::test_approval_request_must_be_bound_to_originating_conversation`, `test_same_approval_request_id_is_scoped_per_conversation` | | AG-UI approval result event | Approved result emits once with content and persists in snapshot. | `packages/ag-ui/tests/ag_ui/test_approval_result_event.py::test_approved_call_emits_one_live_result_under_original_identity`, `packages/ag-ui/tests/ag_ui/test_endpoint.py::test_endpoint_agent_approval_resume_persists_replayable_tool_results`, `test_endpoint_agent_approval_replayed_resume_entry_reprojects_retained_result` | | AG-UI rejection/mixed decision | Transport emits only the events defined for approved and rejected calls without duplicates. | `packages/ag-ui/tests/ag_ui/test_approval_result_event.py::test_rejected_call_does_not_execute_or_emit_live_result`, `test_mixed_batch_preserves_approved_result_identity_and_order`, `packages/ag-ui/tests/ag_ui/test_endpoint.py::test_endpoint_agent_approval_rejection_releases_already_approved_sibling` | | AG-UI approval-time follow-up | The full grouped user-input pause remains in message history and emits no synthetic `TOOL_CALL_RESULT`. | `packages/ag-ui/tests/ag_ui/test_approval_result_event.py::test_approval_follow_up_group_remains_in_history_without_live_tool_result` | diff --git a/python/packages/devui/agent_framework_devui/_executor.py b/python/packages/devui/agent_framework_devui/_executor.py index ec389201917..19c71146b00 100644 --- a/python/packages/devui/agent_framework_devui/_executor.py +++ b/python/packages/devui/agent_framework_devui/_executor.py @@ -71,9 +71,10 @@ def __init__( self.checkpoint_manager = CheckpointConversationManager(self.conversation_store) - # Tracks pending approval requests: request_id -> server-side function_call. + # Tracks pending approval requests by (conversation_id, request_id), with + # plain request_id keys retained for legacy/unscoped approvals. # Prevents forged responses from executing arbitrary tools (CWE-863). - self._pending_approvals: dict[str, dict[str, Any]] = {} + self._pending_approvals: dict[str | tuple[str, str], dict[str, Any]] = {} def _setup_instrumentation_provider(self) -> None: """Set up our own TracerProvider so we can add processors.""" @@ -130,17 +131,21 @@ def _get_request_conversation_id(self, request: AgentFrameworkRequest) -> str | return None - def _track_approval_request(self, event: dict[str, Any]) -> None: + def _track_approval_request(self, event: dict[str, Any], conversation_id: str | None = None) -> None: """Record a server-issued approval request so we can validate the response later.""" request_id = event.get("request_id") fc = event.get("function_call", {}) if isinstance(request_id, str) and request_id: occurrence_id = fc.get("occurrence_id") - self._pending_approvals[request_id] = { + approval_key: str | tuple[str, str] = ( + (conversation_id, request_id) if conversation_id is not None else request_id + ) + self._pending_approvals[approval_key] = { "id": occurrence_id if isinstance(occurrence_id, str) and occurrence_id else request_id, "call_id": fc.get("id", ""), "name": fc.get("name", ""), "arguments": fc.get("arguments", {}), + "conversation_id": conversation_id, } logger.debug("Tracked approval request: %s for function: %s", request_id, fc.get("name", "unknown")) @@ -258,7 +263,10 @@ async def execute_streaming(self, request: AgentFrameworkRequest) -> AsyncGenera isinstance(event, dict) and cast(dict[str, Any], event).get("type") == "response.function_approval.requested" ): - self._track_approval_request(cast(dict[str, Any], event)) + self._track_approval_request( + cast(dict[str, Any], event), + self._get_request_conversation_id(request), + ) events.append(event) if _get_event_type(event) == "response.failed": continue @@ -366,12 +374,15 @@ async def _execute_agent( yield AgentStartedEvent() + # Get conversation identity before converting input so approval + # responses can be bound to the conversation that originated them. + conversation_id = self._get_request_conversation_id(request) + # Convert input to proper Message or string - user_message = self._convert_input_to_chat_message(request.input) + user_message = self._convert_input_to_chat_message(request.input, conversation_id) # Get session from conversation parameter (OpenAI standard!) session = None - conversation_id = self._get_request_conversation_id(request) if conversation_id: session = self.conversation_store.get_session(conversation_id) if session: @@ -622,7 +633,7 @@ async def _execute_workflow( logger.error(f"Error in workflow execution: {e}") yield {"type": "error", "message": f"Workflow execution error: {e!s}"} - def _convert_input_to_chat_message(self, input_data: Any) -> Any: + def _convert_input_to_chat_message(self, input_data: Any, conversation_id: str | None = None) -> Any: """Convert OpenAI Responses API input to Agent Framework Message or string. Handles various input formats including text, images, files, and multimodal content. @@ -630,6 +641,7 @@ def _convert_input_to_chat_message(self, input_data: Any) -> Any: Args: input_data: OpenAI ResponseInputParam (List[ResponseInputItemParam]) + conversation_id: Conversation associated with the input, used to validate approval responses. Returns: Message for multimodal content, or string for simple text @@ -648,12 +660,14 @@ def _convert_input_to_chat_message(self, input_data: Any) -> Any: # Handle OpenAI ResponseInputParam (List[ResponseInputItemParam]) if isinstance(input_data, list): input_items: Any = cast(Any, input_data) - return self._convert_openai_input_to_chat_message(input_items, Message) + return self._convert_openai_input_to_chat_message(input_items, Message, conversation_id) # Fallback for other formats return self._extract_user_message_fallback(input_data) - def _convert_openai_input_to_chat_message(self, input_items: list[Any], Message: Any) -> Any: + def _convert_openai_input_to_chat_message( + self, input_items: list[Any], Message: Any, conversation_id: str | None = None + ) -> Any: """Convert OpenAI ResponseInputParam to Agent Framework messages. Processes text, images, files, and other content types from OpenAI format @@ -662,6 +676,7 @@ def _convert_openai_input_to_chat_message(self, input_items: list[Any], Message: Args: input_items: List of OpenAI ResponseInputItemParam objects (dicts or objects) Message: Message class for creating chat messages + conversation_id: Conversation associated with the input, used to validate approval responses. Returns: One Message, or a list of Messages when input contains multiple message items @@ -803,11 +818,42 @@ def _convert_openai_input_to_chat_message(self, input_items: list[Any], Message: # Only accept responses that match a request we issued. # Always use the server-stored function_call data. - stored_fc = ( - None - if request_id in approval_request_ids - else self._pending_approvals.get(request_id) + approval_key: str | tuple[str, str] = ( + (conversation_id, request_id) if conversation_id is not None else request_id ) + legacy_key = request_id + + if request_id in approval_request_ids: + stored_fc = None + elif ( + approval_key != legacy_key + and approval_key in self._pending_approvals + and legacy_key in self._pending_approvals + ): + logger.warning( + "Rejected function_approval_response for request_id %s: " + "ambiguous scoped and unscoped approvals exist.", + request_id, + ) + stored_fc = None + continue + else: + stored_fc = self._pending_approvals.get(approval_key) + if stored_fc is None and approval_key != legacy_key: + stored_fc = self._pending_approvals.get(legacy_key) + if ( + stored_fc is not None + and stored_fc.get("conversation_id") is not None + and stored_fc.get("conversation_id") != conversation_id + ): + logger.warning( + "Rejected function_approval_response for request_id %s: " + "approval belongs to a different conversation.", + request_id, + ) + stored_fc = None + continue + if stored_fc is None: logger.warning( "Rejected function_approval_response with unknown " @@ -870,7 +916,13 @@ def _convert_openai_input_to_chat_message(self, input_items: list[Any], Message: raise InputConversionError("OpenAI input did not contain any supported message content") for request_id in approval_request_ids: - self._pending_approvals.pop(request_id, None) + approval_key: str | tuple[str, str] = ( + (conversation_id, request_id) if conversation_id is not None else request_id + ) + if approval_key in self._pending_approvals: + self._pending_approvals.pop(approval_key, None) + else: + self._pending_approvals.pop(request_id, None) logger.info("Created %d Message object(s) from OpenAI input", len(messages)) diff --git a/python/packages/devui/tests/devui/test_approval_validation.py b/python/packages/devui/tests/devui/test_approval_validation.py index f8d764fd5b4..6f2f93aea25 100644 --- a/python/packages/devui/tests/devui/test_approval_validation.py +++ b/python/packages/devui/tests/devui/test_approval_validation.py @@ -22,6 +22,7 @@ from agent_framework_devui._discovery import EntityDiscovery from agent_framework_devui._executor import AgentFrameworkExecutor from agent_framework_devui._mapper import MessageMapper +from agent_framework_devui.models._openai_custom import AgentFrameworkRequest @pytest.fixture @@ -250,3 +251,168 @@ def test_multiple_approvals_independent(executor: AgentFrameworkExecutor) -> Non # req_b should still be pending assert "req_b" in executor._pending_approvals assert "req_a" not in executor._pending_approvals + + +def test_approval_request_must_be_bound_to_originating_conversation( + executor: AgentFrameworkExecutor, +) -> None: + """CWE-863: approval from one conversation must not be usable by another.""" + # Simulate an approval request issued while handling conversation A. + executor._track_approval_request( + { + "type": "response.function_approval.requested", + "request_id": "req_cross_conversation", + "function_call": { + "id": "call_sensitive", + "occurrence_id": "af-call-sensitive", + "name": "sensitive_tool", + "arguments": {"resource": "conversation-a-secret"}, + }, + }, + conversation_id="conversation-a", + ) + + # Conversation B submits an approval for conversation A's request. + conversation_b = AgentFrameworkRequest( + model="test-agent", + input=_make_approval_response_input( + request_id="req_cross_conversation", + approved=True, + ), + conversation="conversation-b", + ) + + # A pending approval must be scoped to the conversation that caused it. + # A response from a different conversation must be rejected. + with pytest.raises(ValueError, match="did not contain any supported message content"): + executor._convert_input_to_chat_message( + conversation_b.input, + conversation_id="conversation-b", + ) + + # A rejected response from another conversation must not consume the approval. + assert ("conversation-a", "req_cross_conversation") in executor._pending_approvals + + # The originating conversation can still consume its approval. + conversation_a_input = _make_approval_response_input( + request_id="req_cross_conversation", + approved=True, + ) + result = executor._convert_input_to_chat_message( + conversation_a_input, + conversation_id="conversation-a", + ) + + approval_contents = [c for c in result.contents if c.type == "function_approval_response"] + assert len(approval_contents) == 1 + assert approval_contents[0].function_call.name == "sensitive_tool" + assert ("conversation-a", "req_cross_conversation") not in executor._pending_approvals + + +def test_same_approval_request_id_is_scoped_per_conversation( + executor: AgentFrameworkExecutor, +) -> None: + """Same request_id in different conversations must not collide.""" + request_id = "req_shared" + + executor._track_approval_request( + { + "type": "response.function_approval.requested", + "request_id": request_id, + "function_call": { + "id": "call_a", + "occurrence_id": "af-call-a", + "name": "tool_a", + "arguments": {"conversation": "a"}, + }, + }, + conversation_id="conversation-a", + ) + executor._track_approval_request( + { + "type": "response.function_approval.requested", + "request_id": request_id, + "function_call": { + "id": "call_b", + "occurrence_id": "af-call-b", + "name": "tool_b", + "arguments": {"conversation": "b"}, + }, + }, + conversation_id="conversation-b", + ) + + assert ("conversation-a", request_id) in executor._pending_approvals + assert ("conversation-b", request_id) in executor._pending_approvals + + result_a = executor._convert_input_to_chat_message( + _make_approval_response_input(request_id=request_id, approved=True), + conversation_id="conversation-a", + ) + + approval_a = [c for c in result_a.contents if c.type == "function_approval_response"] + assert len(approval_a) == 1 + assert approval_a[0].function_call.name == "tool_a" + assert ("conversation-a", request_id) not in executor._pending_approvals + assert ("conversation-b", request_id) in executor._pending_approvals + + result_b = executor._convert_input_to_chat_message( + _make_approval_response_input(request_id=request_id, approved=True), + conversation_id="conversation-b", + ) + + approval_b = [c for c in result_b.contents if c.type == "function_approval_response"] + assert len(approval_b) == 1 + assert approval_b[0].function_call.name == "tool_b" + assert ("conversation-b", request_id) not in executor._pending_approvals + + +def test_scoped_and_legacy_approval_with_same_request_id_is_rejected( + executor: AgentFrameworkExecutor, +) -> None: + """Conflicting scoped and legacy approvals must fail closed.""" + request_id = "req_mixed_scope" + + # Legacy/unscoped pending approval. + executor._track_approval_request( + { + "type": "response.function_approval.requested", + "request_id": request_id, + "function_call": { + "id": "call_legacy", + "occurrence_id": "af-call-legacy", + "name": "legacy_tool", + "arguments": {"scope": "legacy"}, + }, + }, + ) + + # Conversation-scoped pending approval with the same request ID. + executor._track_approval_request( + { + "type": "response.function_approval.requested", + "request_id": request_id, + "function_call": { + "id": "call_scoped", + "occurrence_id": "af-call-scoped", + "name": "scoped_tool", + "arguments": {"scope": "conversation-a"}, + }, + }, + conversation_id="conversation-a", + ) + + assert request_id in executor._pending_approvals + assert ("conversation-a", request_id) in executor._pending_approvals + + # The mixed scoped/unscoped state is ambiguous. Fail closed rather than + # selecting either approval and potentially allowing a replay. + with pytest.raises(ValueError, match="did not contain any supported message content"): + executor._convert_input_to_chat_message( + _make_approval_response_input(request_id=request_id, approved=True), + conversation_id="conversation-a", + ) + + # Rejection must not consume either pending approval. + assert request_id in executor._pending_approvals + assert ("conversation-a", request_id) in executor._pending_approvals