fix(app): sign multimodal tool file URL in MessageFile record#39244
Open
sergioperezcheco wants to merge 2 commits into
Open
fix(app): sign multimodal tool file URL in MessageFile record#39244sergioperezcheco wants to merge 2 commits into
sergioperezcheco wants to merge 2 commits into
Conversation
_handle_multimodal_image_content stored an unsigned relative URL (/files/tools/<id>) in the MessageFile record. The /files/tools/<id> endpoint (controllers/files/tool_files.py) validates timestamp, nonce, and sign query params via ToolFileQuery, so these URLs 400 on access. Build the URL with sign_tool_file(), matching how ToolFileMessageTransformer already signs tool file links. The extension is now resolved once for both the URL-fetch and base64 branches via resolve_extension(). Signed-off-by: sergioperezcheco <checo520@outlook.com>
sergioperezcheco
requested review from
QuantumGhost and
laipz8200
as code owners
July 20, 2026 00:42
Contributor
Author
|
Heads up — every CI job on this PR timed out at the 10m0s mark, and the |
Previous run failed across all jobs with 'self-hosted runner lost communication with the server'. No code changes.
dosubot
Bot
added
the
size:S
Contributor
Pyrefly Type Coverage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When an LLM returns image content (URL or base64), _handle_multimodal_image_content in base_app_runner.py stored an unsigned relative URL (/files/tools/) in the MessageFile record. The /files/tools/. endpoint validates timestamp, nonce, and sign query parameters via ToolFileQuery, so accessing these stored URLs fails with a 400 pydantic ValidationError (all three fields reported as missing).
This builds the URL through sign_tool_file(), the same helper ToolFileMessageTransformer already uses for its tool file links, so the stored MessageFile URL carries the required signature. The file extension is now resolved once after the file is saved (covering both the URL-fetch and base64 branches) via resolve_extension(filename, mimetype) instead of only being computed inside the base64 branch.
Added a regression assertion in test_handle_multimodal_image_content_with_url that the stored URL contains timestamp/nonce/sign and the tool file id. The existing multimodal test suite (7 tests) and base_app_runner tests (18 tests) pass, and ruff lint/format are clean.
This is the base_app_runner.py location called out in #39222; it complements #39225, which signs the ToolFileMessageTransformer.get_tool_file_url path.
Fixes #39222