Skip to content

Commit e59b4b9

Browse files
RSPEED-2885: sanitize model and MCP output in all response paths (#1563)
* test(responses): add failing tests for output and model sanitization Red tests for _sanitize_response_dict to cover: - mcp_list_tools/mcp_call items not filtered from output array - model field not stripping provider prefix (google-vertex/...) These tests document the expected behavior before the fix. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(responses): sanitize model and MCP output in all response paths Move output array filtering into _sanitize_response_dict() so both streaming and non-streaming paths strip server-deployed MCP items (mcp_list_tools, mcp_call, mcp_approval_request). Strip provider routing prefix from model field (e.g. google-vertex/.../gemini-2.5-flash becomes gemini-2.5-flash). Removes redundant ad-hoc output filtering from the streaming generator that was missing from the non-streaming path, causing the leak QE found. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * test(e2e): update response model assertions for sanitized model field E2E feature files now use {MODEL_SHORT} placeholder for response body assertions since the model field is stripped of provider prefix. Request bodies still send {PROVIDER}/{MODEL} unchanged. Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(responses): only strip model prefix when server-substituted Follow the same pattern as instructions: if the client specified a model, echo it back unchanged. Only strip the provider routing prefix when the server chose the model (client sent empty/no model). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * docs(responses): document model sanitization behavior * Revert "test(e2e): update response model assertions for sanitized model field" This reverts commit d49aa55. * test: assert model prefix is stripped when server auto-selects Update e2e auto-select scenarios to verify the response model field contains just the model name (no provider prefix) when the client omits the model parameter and the server substitutes it. * style: add docstring to _make_streaming_completed_chunk helper * test: align auto-select e2e assertions with string input scenario Use full response structure assertion (object, status, model, output) matching the pattern from 'Responses accepts string input' scenario, per reviewer feedback. --------- Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 parent 6dddd89 commit e59b4b9

5 files changed

Lines changed: 415 additions & 18 deletions

File tree

docs/responses.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ The following response attributes are inherited directly from the LLS OpenAPI sp
399399
| `completed_at` | integer | Completion time (Unix), if set |
400400
| `error` | object | Error details if failed or incompleted |
401401
| `id` | string | Unique response ID or moderation ID |
402-
| `model` | string | Model ID (provider/model) used |
402+
| `model` | string | Model used for generation. If the client specified `model` in the request, it is echoed unchanged; if the server selected the model, the provider routing prefix is stripped (see [Model Selection](#model-selection)) |
403403
| `object` | string | Always `"response"` |
404404
| `output` | array[object] | Structured output (messages, tool calls, etc.) |
405405
| `parallel_tool_calls` | boolean | Parallel tool calls allowed |
@@ -515,6 +515,8 @@ In OpenResponses the `model` field is required; in LCORE it is optional. If you
515515
3. **First available** — Otherwise, the first available LLM model is used.
516516
4. If no model can be selected (e.g. no default and no LLM models), the request fails with 404 (model not found).
517517

518+
**Model in response:** If the client specified a `model` in the request, it is echoed back unchanged in the response. If the server selected the model (because `model` was omitted from the request), the provider routing prefix is stripped and only the base model name is returned (e.g. `google-vertex/publishers/google/models/gemini-2.5-flash``gemini-2.5-flash`). This prevents leaking server infrastructure details and follows the same pattern as [System Prompt Resolution](#system-prompt-resolution).
519+
518520
### Output Representation
519521

520522
Responses expose both:

src/app/endpoints/responses.py

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -280,10 +280,12 @@ async def responses_endpoint_handler(
280280

281281
# LCORE-specific: Automatically select model if not provided in request
282282
# This extends the base LLS API which requires model to be specified.
283+
client_model = responses_request.model
283284
if not responses_request.model:
284285
responses_request.model = await select_model_for_responses(
285286
client, response_context.user_conversation
286287
)
288+
model_substituted = not client_model
287289
if not await check_model_configured(client, responses_request.model):
288290
_, model_id = extract_provider_and_model_from_model_id(responses_request.model)
289291
error_response = NotFoundResponse(resource="model", resource_id=model_id)
@@ -371,6 +373,7 @@ async def responses_endpoint_handler(
371373
inline_rag_context=inline_rag_context,
372374
filter_server_tools=filter_server_tools,
373375
instructions_substituted=instructions_substituted,
376+
model_substituted=model_substituted,
374377
background_tasks=background_tasks,
375378
rh_identity_context=rh_identity_context,
376379
)
@@ -386,6 +389,7 @@ async def handle_streaming_response(
386389
inline_rag_context: RAGContext,
387390
filter_server_tools: bool = False,
388391
instructions_substituted: bool = False,
392+
model_substituted: bool = False,
389393
background_tasks: Optional[BackgroundTasks] = None,
390394
rh_identity_context: tuple[str, str] = ("", ""),
391395
) -> StreamingResponse:
@@ -401,6 +405,7 @@ async def handle_streaming_response(
401405
inline_rag_context: Inline RAG context to be used for the response
402406
filter_server_tools: Whether to filter server-deployed MCP tool events from the stream
403407
instructions_substituted: Whether the server substituted the instructions
408+
model_substituted: Whether the server substituted the model
404409
background_tasks: FastAPI background task manager for telemetry events
405410
rh_identity_context: Tuple of (org_id, system_id) from RH identity
406411
Returns:
@@ -453,6 +458,7 @@ async def handle_streaming_response(
453458
inline_rag_context=inline_rag_context,
454459
filter_server_tools=filter_server_tools,
455460
instructions_substituted=instructions_substituted,
461+
model_substituted=model_substituted,
456462
)
457463
except RuntimeError as e: # library mode wraps 413 into runtime error
458464
if is_context_length_error(str(e)):
@@ -624,6 +630,7 @@ def _sanitize_response_dict(
624630
response_dict: dict[str, Any],
625631
configured_mcp_labels: set[str],
626632
instructions_substituted: bool = False,
633+
model_substituted: bool = False,
627634
) -> None:
628635
"""Sanitize a serialized response object in-place to remove internal details.
629636
@@ -637,7 +644,14 @@ def _sanitize_response_dict(
637644
they were used as-is, the value is left unchanged.
638645
- ``tools``: server-deployed MCP tool definitions are removed; client-
639646
provided tools (those whose ``server_label`` is not in
640-
``configured_mcp_labels``) are preserved
647+
``configured_mcp_labels``) are preserved.
648+
- ``output``: server-deployed MCP output items (``mcp_list_tools``,
649+
``mcp_call``, ``mcp_approval_request``) are stripped so clients only
650+
see item types they understand (``message``, ``function_call``, etc.).
651+
- ``model``: the provider routing prefix (everything before the last
652+
``/``) is stripped only when the server selected the model
653+
(``model_substituted=True``). When the client specified the model,
654+
it is echoed back unchanged.
641655
642656
Args:
643657
response_dict: Mutable dict produced by ``model_dump`` on a response
@@ -646,6 +660,8 @@ def _sanitize_response_dict(
646660
server-deployed MCP servers.
647661
instructions_substituted: Whether the server substituted the
648662
instructions (True) or the client provided them (False).
663+
model_substituted: Whether the server substituted the model
664+
(True) or the client provided it (False).
649665
"""
650666
if instructions_substituted:
651667
response_dict["instructions"] = SUBSTITUTED_INSTRUCTIONS_PLACEHOLDER
@@ -658,6 +674,18 @@ def _sanitize_response_dict(
658674
if tool.get("server_label") not in configured_mcp_labels
659675
]
660676

677+
if output := response_dict.get("output"):
678+
response_dict["output"] = [
679+
item
680+
for item in output
681+
if not _is_server_mcp_output_item(item, configured_mcp_labels)
682+
]
683+
684+
if model_substituted:
685+
model = response_dict.get("model")
686+
if model and "/" in model:
687+
response_dict["model"] = model.rsplit("/", 1)[-1]
688+
661689

662690
def _is_server_mcp_output_item(
663691
item: dict[str, Any], configured_mcp_labels: set[str]
@@ -775,6 +803,7 @@ async def response_generator(
775803
inline_rag_context: RAGContext,
776804
filter_server_tools: bool = False,
777805
instructions_substituted: bool = False,
806+
model_substituted: bool = False,
778807
) -> AsyncIterator[str]:
779808
"""Generate SSE-formatted streaming response with LCORE-enriched events.
780809
@@ -787,6 +816,7 @@ async def response_generator(
787816
inline_rag_context: Inline RAG context to be used for the response
788817
filter_server_tools: Whether to filter server-deployed MCP tool events from the stream
789818
instructions_substituted: Whether the server substituted the instructions
819+
model_substituted: Whether the server substituted the model
790820
Yields:
791821
SSE-formatted strings for streaming events, ending with [DONE]
792822
"""
@@ -824,6 +854,7 @@ async def response_generator(
824854
chunk_dict["response"],
825855
configured_mcp_labels,
826856
instructions_substituted,
857+
model_substituted,
827858
)
828859
tools = chunk_dict["response"].get("tools")
829860
if tools is not None:
@@ -833,16 +864,6 @@ async def response_generator(
833864
configuration.rag_id_mapping,
834865
)
835866
)
836-
# Remove server-deployed MCP items from the output array so
837-
# clients only see item types they understand (message, function_call, etc.)
838-
output = chunk_dict["response"].get("output")
839-
if output is not None:
840-
chunk_dict["response"]["output"] = [
841-
item
842-
for item in output
843-
if not _is_server_mcp_output_item(item, configured_mcp_labels)
844-
]
845-
846867
# Intermediate response - no quota consumption and text yet
847868
if event_type == "response.in_progress":
848869
chunk_dict["response"]["available_quotas"] = {}
@@ -988,6 +1009,7 @@ async def handle_non_streaming_response(
9881009
inline_rag_context: RAGContext,
9891010
filter_server_tools: bool = False,
9901011
instructions_substituted: bool = False,
1012+
model_substituted: bool = False,
9911013
background_tasks: Optional[BackgroundTasks] = None,
9921014
rh_identity_context: tuple[str, str] = ("", ""),
9931015
) -> ResponsesResponse:
@@ -1003,6 +1025,7 @@ async def handle_non_streaming_response(
10031025
inline_rag_context: Inline RAG context to be used for the response
10041026
filter_server_tools: Whether to filter server-deployed MCP tool output
10051027
instructions_substituted: Whether the server substituted the instructions
1028+
model_substituted: Whether the server substituted the model
10061029
background_tasks: FastAPI background task manager for telemetry events
10071030
rh_identity_context: Tuple of (org_id, system_id) from RH identity
10081031
Returns:
@@ -1168,7 +1191,10 @@ async def handle_non_streaming_response(
11681191
configured_mcp_labels = {s.name for s in configuration.mcp_servers}
11691192
response_dict = api_response.model_dump(exclude_none=True)
11701193
_sanitize_response_dict(
1171-
response_dict, configured_mcp_labels, instructions_substituted
1194+
response_dict,
1195+
configured_mcp_labels,
1196+
instructions_substituted,
1197+
model_substituted,
11721198
)
11731199
tools = response_dict.get("tools")
11741200
if tools is not None:

tests/e2e/features/responses.feature

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,20 @@ Feature: Responses endpoint API tests
150150
"""
151151
Then The status code of the response is 200
152152
And The body of the response contains hello
153+
And the body of the response has the following structure
154+
"""
155+
{
156+
"object": "response",
157+
"status": "completed",
158+
"model": "{MODEL}",
159+
"output": [
160+
{
161+
"type": "message",
162+
"role": "assistant"
163+
}
164+
]
165+
}
166+
"""
153167

154168
Scenario: Responses returns 404 for unknown model segment in provider slash model id
155169
Given The system is in default state

tests/e2e/features/responses_streaming.feature

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,20 @@ Feature: Responses endpoint streaming API tests
200200
"""
201201
Then The status code of the response is 200
202202
And The body of the response contains hello
203+
And the body of the response has the following structure
204+
"""
205+
{
206+
"object": "response",
207+
"status": "completed",
208+
"model": "{MODEL}",
209+
"output": [
210+
{
211+
"type": "message",
212+
"role": "assistant"
213+
}
214+
]
215+
}
216+
"""
203217

204218
Scenario: Streaming responses returns 404 for unknown model segment in provider slash model id
205219
When I use "responses" to ask question with authorization header

0 commit comments

Comments
 (0)