fix(responses): delimit untrusted web_search/file_search tool output before feeding it back to the model - #6337
Conversation
…put before feeding it back to the model web_search and file_search (and knowledge_search) results were placed into the model's next-turn context verbatim, with no boundary between trusted instructions and untrusted, externally-sourced content such as scraped web pages or indexed documents. An attacker who controls a page that gets searched or indexed could inject text that the model treats as an instruction rather than as data (indirect prompt injection). This wraps the text portions of results from these three tools in explicit delimiters with a short instruction that the enclosed content is untrusted data to analyze, not instructions to follow. MCP tool output and other tool types are unaffected, matching the scope of the reported issue. Image content parts pass through unwrapped. Fixes ogx-ai#6263 Signed-off-by: Praveen Mittal <pkmittal28@gmail.com>
…imiting Found during an edge-case review of the delimiting fix itself: 1. The wrapper did not escape occurrences of its own delimiter tags inside untrusted content. A page containing the literal string "</untrusted_tool_output>" could close the delimited block early and make attacker-injected text that follows look like it sits outside the untrusted region, defeating the mitigation with itself. Both the open and close tags are now escaped inside untrusted content before wrapping. 2. A successful web_search/file_search/knowledge_search call that legitimately returns empty content (zero results) was reported to the model as "Tool execution failed", because the pre-existing truthiness check treated an empty string or list the same as no result at all. Changed to an explicit "is not None" check so empty results are distinguished from a genuinely missing result. Neither is specific to ogx-ai#6263, but both live in the same function this branch already touches and surfaced during an edge-case pass over it. Signed-off-by: Praveen Mittal <pkmittal28@gmail.com>
The escape helper only matched the exact-case delimiter tag text, so content containing a case-varied copy (e.g. "</UNTRUSTED_TOOL_OUTPUT>") was left unescaped -- a trivial, well-known evasion technique. Switched to a case-insensitive regex match. Whitespace-padded variants (e.g. "< /untrusted_tool_output >") and Unicode-homoglyph tricks are not caught by this and are noted as a documented, known limitation rather than a blocker -- they require the model itself to recognize a structurally distorted tag as a real delimiter, a materially harder attack than the exact-text-modulo-case copy this closes. A per-request random delimiter token would close that gap and is a reasonable follow-up. Signed-off-by: Praveen Mittal <pkmittal28@gmail.com>
mattf
left a comment
There was a problem hiding this comment.
thanks for helping us make progress in this game of cat and mouse.
nit: use narrower types than Any
|
@nathan-weinberg ptal |
|
@mattf, Thank you The two failing checks (azure/responses, gpt/responses) are expected — this PR changes the exact request content sent to the model for web_search/file_search tool results (that's the fix), which invalidates the recorded fixtures keyed by request hash for those two suites. Since this is a fork PR, the auto-recording workflow only ran Could you trigger |
|
Recording workflow completed Providers: gpt, azure Recordings have been generated and will be committed automatically by the companion workflow. Fork PR: Recordings will be committed if you have "Allow edits from maintainers" enabled. |
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-Authored-By: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
✅ Recordings committed successfully Recordings from the integration tests have been committed to this PR. |
Fixes #6263
What
web_search,file_search, andknowledge_searchresults were placed into the model's next-turn context verbatim, with no boundary between trusted instructions and untrusted, externally-sourced content (scraped web pages, indexed documents). An attacker who controls a page that gets searched or indexed could inject text the model treats as an instruction rather than as data (indirect prompt injection).Fix
Wraps the text portions of results from these three tools in explicit
<untrusted_tool_output>delimiters with a short instruction that the enclosed content is untrusted data to analyze, never instructions to follow. MCP tool output and other tool types are unaffected, matching the scope of the reported issue. Image content parts pass through unwrapped.Two additional issues surfaced during an edge-case pass over this same code path and are fixed here too, since they live in the exact function this PR already touches:
</untrusted_tool_output>could close the delimited block early and make injected text that follows look like it sits outside the untrusted region — defeating the wrapping with itself. Both tags are now escaped inside untrusted content before wrapping, case-insensitively (case variation like</UNTRUSTED_TOOL_OUTPUT>is a trivial, well-known evasion of a naive case-sensitive match)."Tool execution failed", because the pre-existing check used truthiness (if result and result_content:) rather than distinguishing "no result at all" from "a result with empty content." Changed to an explicitis not Nonecheck.Known limitation (documented, not a blocker)
Whitespace-padded tag variants (e.g.
< /untrusted_tool_output >) and Unicode-homoglyph tricks are not caught by the current escaping — closing that fully would need a structurally different defense (e.g. a per-request random delimiter token instead of a static string). Noted explicitly in the_escape_delimiter_collisionsdocstring as a reasonable follow-up rather than silently left unstated.Tests
New
tests/unit/providers/inline/responses/builtin/responses/test_tool_executor.py(13 tests, this module previously had zero coverage):web_search/file_search/knowledge_search, both string and mixed text+image list content shapes.result=None) still correctly reports failure.One existing test (
test_openai_responses_tools.py::test_create_openai_response_with_string_input_with_tools) asserted tool output survives verbatim (content == "Dublin"); updated to"Dublin" in contentsince that's the new, correct contract given the delimiting.Every new/changed assertion was confirmed to fail against the pre-fix code (via
git stash) before the corresponding fix landed.Verification
uv run pytest tests/unit/providers/inline/responses/builtin/ tests/unit/providers/responses/builtin/: 373 passed, no regressionsuv run ruff check/ruff format --check: cleanuv run mypy: no issuesuv run pre-commit run --files <changed files>: all hooks passed (license header, FIPS check, SQL-injection lint, logging conventions, codegen-drift checks all N/A since no API/provider schema was touched)