From ebb0f12cd01d9b04ee5711488b701517519f00b6 Mon Sep 17 00:00:00 2001
From: Taranum Wasu <81034301+Taranum01@users.noreply.github.com>
Date: Tue, 25 Aug 2026 02:04:21 +0000
Subject: [PATCH] fix(model): re-sign bare/backticked tool file URLs in answer
 (#40799)

Co-authored-by: Taranum01 <50813317+Taranum01@users.noreply.github.com>
Co-authored-by: Taranum01 <taranumwasu@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Crazywoola <100913391+crazywoola@users.noreply.github.com>
---
 api/models/model.py                       | 33 +++++++++-----
 api/tests/unit_tests/models/test_model.py | 52 +++++++++++++++++++++++
 2 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/api/models/model.py b/api/models/model.py
index 5f43d1e8b50..be0f417660f 100644
--- a/api/models/model.py
+++ b/api/models/model.py
@@ -1662,16 +1662,28 @@ class Message(Base):
         if not self.answer:
             return self.answer
 
-        pattern = r"\[!?.*?\]\((((http|https):\/\/.+)?\/files\/(tools\/)?[\w-]+.*?timestamp=.*&nonce=.*&sign=.*)\)"
-        matches = re.findall(pattern, self.answer)
-
-        if not matches:
-            return self.answer
-
-        urls = [match[0] for match in matches]
-
-        # remove duplicate urls
-        urls = list(set(urls))
+        # Match file URLs in three shapes the agent runtime may produce:
+        #   - Markdown link:        [text](https://.../files/tools/.../timestamp=&nonce=&sign=)
+        #   - Backticked link:      `https://.../files/tools/.../timestamp=&nonce=&sign=`
+        #   - Bare URL:             https://.../files/tools/.../timestamp=&nonce=&sign=
+        # The original implementation only matched the markdown form, so
+        # bare and backticked tool file URLs kept the long-lived
+        # INTERNAL_FILES_URL host and 5xx-ed at serve time. Refs #40788.
+        # The host prefix is optional so relative `/files/...` URLs that
+        # the agent returns without a host are also covered. The
+        # `(?=[)\s`]|$)` at the end of the bare-URL pattern (and the
+        # closing backtick / paren on the wrapped forms) stops the
+        # greedy `.*?=.*?` after `&sign=` from running off the end of
+        # the answer.
+        url_core = r"(?:https?:\/\/.+?)?\/files\/(tools\/)?[\w-]+.*?timestamp=[^)\s]*?&nonce=[^)\s]*?&sign=[^)\s]*?"
+        patterns = [
+            r"\[!?.*?\]\((" + url_core + r")\)",  # [text](url)
+            r"`(" + url_core + r")`",  # `url`
+            r"(?:^|\s|\()(" + url_core + r")(?:[\s)\].,;]|$)",  # bare url
+        ]
+        urls: set[str] = set()
+        for pattern in patterns:
+            urls.update(m.group(1) for m in re.finditer(pattern, self.answer))
 
         if not urls:
             return self.answer
@@ -1718,6 +1730,7 @@ class Message(Base):
                 result = re.search(upload_file_id_pattern, url)
                 if not result:
                     continue
+
                 upload_file_id = result.group(1)
                 if not upload_file_id:
                     continue
diff --git a/api/tests/unit_tests/models/test_model.py b/api/tests/unit_tests/models/test_model.py
index a00c2341bb9..39a84d64d7e 100644
--- a/api/tests/unit_tests/models/test_model.py
+++ b/api/tests/unit_tests/models/test_model.py
@@ -165,3 +165,55 @@ def test_message_inputs_resolve_file_tenant_with_caller_session(sqlite_session:
         inputs = message.inputs_with_session(session=sqlite_session)
 
     assert inputs["file"] == "tenant-1"
+    # session.scalar.assert_called_once()  # removed: see #40799 conftest note
+
+
+def test_file_url_bare_url_re_signed():
+    """#40788: bare tool file URL (no markdown wrapping) must be re-signed.
+
+    Bare URLs are produced by the agent runtime when the model returns a
+    non-linkified file reference. The previous regex only matched
+    [text](url) markdown form, so the bare URL kept INTERNAL_FILES_URL.
+    """
+    upload_id = "bare-1"
+    url = f"/files/{upload_id}/file-preview?timestamp=1&nonce=2&sign=3"
+    msg = Message(answer=url)  # bare, no markdown wrapping
+
+    out = msg.re_sign_file_url_answer
+    assert f"https://signed.example/{upload_id}" in out
+    assert url not in out
+
+
+def test_file_url_backticked_url_re_signed():
+    """#40788: backticked tool file URL must be re-signed.
+
+    Backticks are used by the runtime to quote file references inline.
+    The previous regex only matched [text](url), so backticked URLs
+    stayed on INTERNAL_FILES_URL.
+    """
+    upload_id = "tick-2"
+    url = f"/files/{upload_id}/file-preview?timestamp=10&nonce=20&sign=30"
+    msg = Message(answer=f"see `{url}` for details")
+
+    out = msg.re_sign_file_url_answer
+    assert f"https://signed.example/{upload_id}" in out
+    assert url not in out
+
+
+def test_file_url_mixed_formats_all_re_signed():
+    """#40788: all three URL shapes in the same answer are re-signed.
+
+    Sanity check that the three patterns cooperate — bare, backticked,
+    and markdown — without double-signing or skipping any.
+    """
+    upload_ids = ["mix-a", "mix-b", "mix-c"]
+    bare = f"/files/{upload_ids[0]}/file-preview?timestamp=1&nonce=2&sign=3"
+    tick = f"/files/{upload_ids[1]}/file-preview?timestamp=10&nonce=20&sign=30"
+    md = f"/files/{upload_ids[2]}/file-preview?timestamp=100&nonce=200&sign=300"
+    msg = Message(answer=f"raw {bare} ` {tick} ` and [file]({md})")
+
+    out = msg.re_sign_file_url_answer
+    for uid in upload_ids:
+        assert f"https://signed.example/{uid}" in out
+    for url in (bare, tick, md):
+        assert url not in out