From 645b87f9dbe5ea5ae636631e2e2443749181597b Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Mon, 27 Jul 2026 09:32:42 +0200 Subject: [PATCH 01/13] Hide empty Chainlit sources --- openrag/app_front.py | 101 +++++++++++++++++++--------- tests/unit/test_app_front_secret.py | 101 +++++++++++++++++++++++----- 2 files changed, 155 insertions(+), 47 deletions(-) diff --git a/openrag/app_front.py b/openrag/app_front.py index fd5be7434..ae6476284 100644 --- a/openrag/app_front.py +++ b/openrag/app_front.py @@ -465,26 +465,52 @@ async def __fetch_page_content(chunk_url, headers=None): async def _format_sources(metadata_sources, only_txt=False, api_key=None): - external_url = get_external_url() # used to override the base URL when the front-end requests a file resource if not metadata_sources: - return None, None + return [], [] d = {} headers = get_headers(api_key) + external_url = get_external_url() # used to override the base URL when the front-end requests a file resource for i, s in enumerate(metadata_sources): + if not isinstance(s, dict): + continue + if s.get("source_type") == "web": - title = s.get("title") or s.get("url", f"Web source {i + 1}") + title = s.get("title", "") url = s.get("url", "") snippet = s.get("snippet", "") - content = f"**[{title}]({url})**\n\n{snippet}" - source_name = title + title = title.strip() if isinstance(title, str) else "" + url = url.strip() if isinstance(url, str) else "" + snippet = snippet.strip() if isinstance(snippet, str) else "" + parsed_url = urlparse(url) + if parsed_url.scheme not in {"http", "https"} or not parsed_url.netloc: + continue + + source_label = title or url + source_name = source_label if source_name in d: - source_name = f"{title} ({i})" + source_name = f"{source_name} ({i})" + content = f"**[{source_label}]({url})**" + if snippet: + content += f"\n\n{snippet}" d[source_name] = cl.Text(content=content, name=source_name, display="side") continue - filename = Path(s["filename"]) - file_url = s["file_url"] + filename_value = s.get("filename") + file_url = s.get("file_url") + page = s.get("page") + if ( + not isinstance(filename_value, str) + or not filename_value.strip() + or not isinstance(file_url, str) + or not file_url.strip() + or page is None + or not str(page).strip() + ): + continue + + filename = Path(filename_value.strip()) + file_url = file_url.strip() file_url = file_url.replace(INTERNAL_BASE_URL, external_url) # put the correct base url # Avoid leaking the credential in the URL (browser history, proxy logs, # Referer headers). In OIDC mode the browser already sends the @@ -495,32 +521,45 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): # authenticate the fetch. if api_key and (AUTH_MODE != "oidc" or _current_openrag_auth_provider() == "credentials"): file_url = f"{file_url}?token={api_key}" - page = s["page"] source_name = f"{filename}" + ( f" (page: {page})" if filename.suffix in [".pdf", ".pptx", ".docx", ".doc"] else "" ) - if only_txt: - chunk_content = await __fetch_page_content(chunk_url=s["chunk_url"], headers=headers) - elem = cl.Text(content=chunk_content, name=source_name, display="side") - else: - match filename.suffix.lower(): - case ".pdf": - elem = cl.Pdf( - name=source_name, - url=file_url, - page=int(s["page"]), - display="side", - ) - case suffix if suffix in [".png", ".jpg", ".jpeg"]: - elem = cl.Image(name=source_name, url=file_url, display="side") - case ".mp4": - elem = cl.Video(name=source_name, url=file_url, display="side") - case ".mp3": - elem = cl.Audio(name=source_name, url=file_url, display="side") - case _: - chunk_content = await __fetch_page_content(chunk_url=s["chunk_url"], headers=headers) - elem = cl.Text(content=chunk_content, name=source_name, display="side") + try: + if only_txt: + chunk_url = s.get("chunk_url") + if not isinstance(chunk_url, str) or not chunk_url.strip(): + continue + chunk_content = await __fetch_page_content(chunk_url=chunk_url, headers=headers) + if not isinstance(chunk_content, str) or not chunk_content.strip(): + continue + elem = cl.Text(content=chunk_content, name=source_name, display="side") + else: + match filename.suffix.lower(): + case ".pdf": + elem = cl.Pdf( + name=source_name, + url=file_url, + page=int(page), + display="side", + ) + case suffix if suffix in [".png", ".jpg", ".jpeg"]: + elem = cl.Image(name=source_name, url=file_url, display="side") + case ".mp4": + elem = cl.Video(name=source_name, url=file_url, display="side") + case ".mp3": + elem = cl.Audio(name=source_name, url=file_url, display="side") + case _: + chunk_url = s.get("chunk_url") + if not isinstance(chunk_url, str) or not chunk_url.strip(): + continue + chunk_content = await __fetch_page_content(chunk_url=chunk_url, headers=headers) + if not isinstance(chunk_content, str) or not chunk_content.strip(): + continue + elem = cl.Text(content=chunk_content, name=source_name, display="side") + except (httpx.HTTPError, TypeError, ValueError): + logger.warning("Skipping an unavailable source", source_index=i) + continue d[source_name] = elem @@ -582,7 +621,7 @@ async def on_message(message: cl.Message): # Show sources elements, source_names = await _format_sources(sources, api_key=api_key, only_txt=False) msg.elements = elements if elements else [] - if source_names: + if elements and source_names: s = "\n\n" + "-" * 50 + f"\n\n{t('sources_label')}: \n" + "\n".join(source_names) await msg.stream_token(s) await msg.update() diff --git a/tests/unit/test_app_front_secret.py b/tests/unit/test_app_front_secret.py index b0a9a5e37..03de973f1 100644 --- a/tests/unit/test_app_front_secret.py +++ b/tests/unit/test_app_front_secret.py @@ -29,6 +29,16 @@ def _load_app_front(monkeypatch, *, auth_mode: str, module_name: str): return module +def _stub_chainlit_elements(module): + module.cl = SimpleNamespace( + Pdf=lambda **kwargs: SimpleNamespace(**kwargs), + Text=lambda **kwargs: SimpleNamespace(**kwargs), + Image=lambda **kwargs: SimpleNamespace(**kwargs), + Video=lambda **kwargs: SimpleNamespace(**kwargs), + Audio=lambda **kwargs: SimpleNamespace(**kwargs), + ) + + def test_no_hardcoded_default_secret_assignment_in_source(): """The fall-through to a literal default secret must be gone. @@ -398,6 +408,77 @@ async def fake_load_model_ids(_client, api_key): assert module._OPENRAG_TOKEN_STORE[auth_handle][0] == "handoff-token" +@pytest.mark.parametrize( + "sources", + [ + None, + [], + [{}], + [None], + [{"source_type": "web", "title": "", "url": "", "snippet": ""}], + [{"filename": "", "file_url": "", "page": ""}], + ], +) +@pytest.mark.asyncio +async def test_chainlit_hides_sources_when_none_are_displayable(monkeypatch, sources): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_empty_sources_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + elements, source_names = await module._format_sources(sources) + + assert elements == [] + assert source_names == [] + + +@pytest.mark.asyncio +async def test_chainlit_keeps_valid_web_sources(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_web_source_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + elements, source_names = await module._format_sources( + [ + { + "source_type": "web", + "title": "Example reference", + "url": "https://example.test/reference", + "snippet": "Supporting evidence", + } + ] + ) + + assert source_names == ["Example reference"] + assert elements[0].name == "Example reference" + assert elements[0].content == ("**[Example reference](https://example.test/reference)**\n\nSupporting evidence") + + +@pytest.mark.asyncio +async def test_chainlit_skips_unavailable_text_sources(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_unavailable_source_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + async def unavailable_chunk(*_args, **_kwargs): + raise httpx.ConnectError("source unavailable") + + monkeypatch.setattr(module, "__fetch_page_content", unavailable_chunk) + + elements, source_names = await module._format_sources( + [ + { + "filename": "notes.txt", + "file_url": "http://internal:8080/static/source-id", + "page": "1", + "chunk_url": "http://internal:8080/chunks/source-id", + } + ] + ) + + assert elements == [] + assert source_names == [] + + @pytest.mark.asyncio async def test_oidc_token_handoff_keeps_bearer_on_static_source_urls(monkeypatch): module = _load_app_front(monkeypatch, auth_mode="oidc", module_name="app_front_source_token_test") @@ -410,14 +491,8 @@ def get(self, key): return SimpleNamespace(metadata={"provider": "credentials"}) return None - module.cl = SimpleNamespace( - user_session=UserSession(), - Pdf=lambda **kwargs: SimpleNamespace(**kwargs), - Text=lambda **kwargs: SimpleNamespace(**kwargs), - Image=lambda **kwargs: SimpleNamespace(**kwargs), - Video=lambda **kwargs: SimpleNamespace(**kwargs), - Audio=lambda **kwargs: SimpleNamespace(**kwargs), - ) + _stub_chainlit_elements(module) + module.cl.user_session = UserSession() elements, _ = await module._format_sources( [ @@ -445,14 +520,8 @@ def get(self, key): return SimpleNamespace(metadata={"provider": "oidc"}) return None - module.cl = SimpleNamespace( - user_session=UserSession(), - Pdf=lambda **kwargs: SimpleNamespace(**kwargs), - Text=lambda **kwargs: SimpleNamespace(**kwargs), - Image=lambda **kwargs: SimpleNamespace(**kwargs), - Video=lambda **kwargs: SimpleNamespace(**kwargs), - Audio=lambda **kwargs: SimpleNamespace(**kwargs), - ) + _stub_chainlit_elements(module) + module.cl.user_session = UserSession() elements, _ = await module._format_sources( [ From ec3e0369781141abeec8cafc9f83c8bb3caf1e97 Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:27:18 +0200 Subject: [PATCH 02/13] Harden Chainlit source rendering --- openrag/app_front.py | 22 ++++++--- tests/unit/test_app_front_secret.py | 69 +++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/openrag/app_front.py b/openrag/app_front.py index ae6476284..1cd78805f 100644 --- a/openrag/app_front.py +++ b/openrag/app_front.py @@ -1,10 +1,11 @@ import json import os import secrets +import string import time from functools import lru_cache from pathlib import Path -from urllib.parse import urlparse +from urllib.parse import quote, urlparse import chainlit as cl import httpx @@ -42,12 +43,19 @@ OPENRAG_CHAT_PROFILES_METADATA_KEY = "openrag_chat_profiles" OPENRAG_SESSION_COOKIE_NAME = "openrag_session" _OPENRAG_TOKEN_STORE: dict[str, tuple[str, float]] = {} +_MARKDOWN_ESCAPE_TABLE = str.maketrans({char: f"\\{char}" for char in string.punctuation}) +_MARKDOWN_URL_SAFE_CHARS = ":/?#[]@!$&'+,;=%" class MissingOpenRAGCredentialError(RuntimeError): pass +def _escape_markdown_text(value: str) -> str: + """Render untrusted source metadata as literal Markdown text.""" + return value.translate(_MARKDOWN_ESCAPE_TABLE) + + def get_user_language() -> str: """Return the active language: env override if set, otherwise browser's Accept-Language.""" if DEFAULT_LANGUAGE: @@ -485,14 +493,18 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): parsed_url = urlparse(url) if parsed_url.scheme not in {"http", "https"} or not parsed_url.netloc: continue + try: + markdown_url = quote(str(httpx.URL(url)), safe=_MARKDOWN_URL_SAFE_CHARS) + except httpx.InvalidURL: + continue source_label = title or url source_name = source_label if source_name in d: source_name = f"{source_name} ({i})" - content = f"**[{source_label}]({url})**" + content = f"**[{_escape_markdown_text(source_label)}]({markdown_url})**" if snippet: - content += f"\n\n{snippet}" + content += f"\n\n{_escape_markdown_text(snippet)}" d[source_name] = cl.Text(content=content, name=source_name, display="side") continue @@ -557,13 +569,13 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): if not isinstance(chunk_content, str) or not chunk_content.strip(): continue elem = cl.Text(content=chunk_content, name=source_name, display="side") - except (httpx.HTTPError, TypeError, ValueError): + except (httpx.HTTPError, TypeError, ValueError, AttributeError): logger.warning("Skipping an unavailable source", source_index=i) continue d[source_name] = elem - source_names = list(d.keys()) + source_names = [_escape_markdown_text(name) for name in d] elements = list(d.values()) return elements, source_names diff --git a/tests/unit/test_app_front_secret.py b/tests/unit/test_app_front_secret.py index 03de973f1..3156c92f0 100644 --- a/tests/unit/test_app_front_secret.py +++ b/tests/unit/test_app_front_secret.py @@ -453,6 +453,35 @@ async def test_chainlit_keeps_valid_web_sources(monkeypatch): assert elements[0].content == ("**[Example reference](https://example.test/reference)**\n\nSupporting evidence") +@pytest.mark.asyncio +async def test_chainlit_escapes_untrusted_web_source_markdown(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_web_source_escaping_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + title = "Reference ](https://spoof.test) **trusted**" + snippet = "Evidence [click here](https://spoof.test) or *ignore this*." + url = "https://example.test/reference_(draft)" + elements, source_names = await module._format_sources( + [ + { + "source_type": "web", + "title": title, + "url": url, + "snippet": snippet, + } + ] + ) + + assert source_names == [r"Reference \]\(https\:\/\/spoof\.test\) \*\*trusted\*\*"] + assert elements[0].name == title + assert elements[0].content == ( + r"**[Reference \]\(https\:\/\/spoof\.test\) \*\*trusted\*\*]" + "(https://example.test/reference_%28draft%29)**\n\n" + r"Evidence \[click here\]\(https\:\/\/spoof\.test\) or \*ignore this\*\." + ) + + @pytest.mark.asyncio async def test_chainlit_skips_unavailable_text_sources(monkeypatch): module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_unavailable_source_test") @@ -479,6 +508,46 @@ async def unavailable_chunk(*_args, **_kwargs): assert source_names == [] +@pytest.mark.asyncio +async def test_chainlit_skips_text_source_with_non_object_json(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_malformed_source_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + class FakeResponse: + def raise_for_status(self): + return None + + def json(self): + return [{"page_content": "unexpected list response"}] + + class FakeAsyncClient: + async def __aenter__(self): + return self + + async def __aexit__(self, *_args): + return None + + async def get(self, *_args, **_kwargs): + return FakeResponse() + + monkeypatch.setattr(module.httpx, "AsyncClient", FakeAsyncClient) + + elements, source_names = await module._format_sources( + [ + { + "filename": "notes.txt", + "file_url": "http://internal:8080/static/source-id", + "page": "1", + "chunk_url": "http://internal:8080/chunks/source-id", + } + ] + ) + + assert elements == [] + assert source_names == [] + + @pytest.mark.asyncio async def test_oidc_token_handoff_keeps_bearer_on_static_source_urls(monkeypatch): module = _load_app_front(monkeypatch, auth_mode="oidc", module_name="app_front_source_token_test") From f8ce6abb07f388c689eb16d2dbfc1f978755606c Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:13:14 +0200 Subject: [PATCH 03/13] Fix Chainlit source validation edge cases --- openrag/api/routers/user/source_links.py | 14 +++--- openrag/app_front.py | 18 ++++---- .../api/routers/user/test_source_links.py | 22 ++++++++++ tests/unit/test_app_front_secret.py | 43 +++++++++++++++++++ 4 files changed, 82 insertions(+), 15 deletions(-) diff --git a/openrag/api/routers/user/source_links.py b/openrag/api/routers/user/source_links.py index 7ace374d9..cb17289f5 100644 --- a/openrag/api/routers/user/source_links.py +++ b/openrag/api/routers/user/source_links.py @@ -33,9 +33,11 @@ def build_document_source_link( encoded_url = None if filename: encoded_url = quote(static_url_builder(doc_metadata["_id"]), safe=":/") - return { - "source_type": "document", - **({"file_url": encoded_url} if encoded_url else {}), - "chunk_url": chunk_url_builder(doc_metadata["_id"]), - **doc_metadata, - } + link = dict(doc_metadata) + link["source_type"] = "document" + link["chunk_url"] = chunk_url_builder(doc_metadata["_id"]) + if encoded_url: + link["file_url"] = encoded_url + else: + link.pop("file_url", None) + return link diff --git a/openrag/app_front.py b/openrag/app_front.py index 1cd78805f..aa6072033 100644 --- a/openrag/app_front.py +++ b/openrag/app_front.py @@ -473,7 +473,7 @@ async def __fetch_page_content(chunk_url, headers=None): async def _format_sources(metadata_sources, only_txt=False, api_key=None): - if not metadata_sources: + if not isinstance(metadata_sources, list) or not metadata_sources: return [], [] d = {} @@ -490,12 +490,12 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): title = title.strip() if isinstance(title, str) else "" url = url.strip() if isinstance(url, str) else "" snippet = snippet.strip() if isinstance(snippet, str) else "" - parsed_url = urlparse(url) - if parsed_url.scheme not in {"http", "https"} or not parsed_url.netloc: - continue try: + parsed_url = urlparse(url) + if parsed_url.scheme not in {"http", "https"} or not parsed_url.netloc: + continue markdown_url = quote(str(httpx.URL(url)), safe=_MARKDOWN_URL_SAFE_CHARS) - except httpx.InvalidURL: + except (ValueError, httpx.InvalidURL): continue source_label = title or url @@ -516,12 +516,11 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): or not filename_value.strip() or not isinstance(file_url, str) or not file_url.strip() - or page is None - or not str(page).strip() ): continue filename = Path(filename_value.strip()) + suffix = filename.suffix.lower() file_url = file_url.strip() file_url = file_url.replace(INTERNAL_BASE_URL, external_url) # put the correct base url # Avoid leaking the credential in the URL (browser history, proxy logs, @@ -533,8 +532,9 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): # authenticate the fetch. if api_key and (AUTH_MODE != "oidc" or _current_openrag_auth_provider() == "credentials"): file_url = f"{file_url}?token={api_key}" + page_label = str(page).strip() if page is not None else "" source_name = f"{filename}" + ( - f" (page: {page})" if filename.suffix in [".pdf", ".pptx", ".docx", ".doc"] else "" + f" (page: {page_label})" if suffix in [".pdf", ".pptx", ".docx", ".doc"] and page_label else "" ) try: @@ -547,7 +547,7 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): continue elem = cl.Text(content=chunk_content, name=source_name, display="side") else: - match filename.suffix.lower(): + match suffix: case ".pdf": elem = cl.Pdf( name=source_name, diff --git a/tests/unit/api/routers/user/test_source_links.py b/tests/unit/api/routers/user/test_source_links.py index ed20af42f..9127837c1 100644 --- a/tests/unit/api/routers/user/test_source_links.py +++ b/tests/unit/api/routers/user/test_source_links.py @@ -61,3 +61,25 @@ def test_metadata_is_passed_through(): link = _build({"_id": "x", "source": "doc.pdf", "author": "alice"}) assert link["author"] == "alice" assert link["chunk_url"] == "https://host/extract/x" + + +def test_metadata_cannot_override_authoritative_source_fields(): + link = _build( + { + "_id": "42", + "source": "diagram.png", + "file_url": "https://attacker.example/file", + "chunk_url": "https://attacker.example/chunk", + "source_type": "web", + } + ) + + assert link["source_type"] == "document" + assert link["file_url"] == "https://host/static/42" + assert link["chunk_url"] == "https://host/extract/42" + + +def test_metadata_file_url_is_removed_when_source_is_missing(): + link = _build({"_id": "42", "file_url": "https://attacker.example/file"}) + + assert "file_url" not in link diff --git a/tests/unit/test_app_front_secret.py b/tests/unit/test_app_front_secret.py index 3156c92f0..b61438c7f 100644 --- a/tests/unit/test_app_front_secret.py +++ b/tests/unit/test_app_front_secret.py @@ -413,10 +413,15 @@ async def fake_load_model_ids(_client, api_key): [ None, [], + 42, + "not-a-source-list", + {"filename": "notes.txt"}, [{}], [None], [{"source_type": "web", "title": "", "url": "", "snippet": ""}], + [{"source_type": "web", "title": "Invalid URL", "url": "http://[invalid"}], [{"filename": "", "file_url": "", "page": ""}], + [{"filename": "document.pdf", "file_url": "https://openrag.example/static/source-id"}], ], ) @pytest.mark.asyncio @@ -431,6 +436,44 @@ async def test_chainlit_hides_sources_when_none_are_displayable(monkeypatch, sou assert source_names == [] +@pytest.mark.asyncio +async def test_chainlit_keeps_page_less_non_pdf_sources(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_page_less_sources_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + async def available_chunk(*_args, **_kwargs): + return "Page-less text content" + + monkeypatch.setattr(module, "__fetch_page_content", available_chunk) + + elements, source_names = await module._format_sources( + [ + { + "filename": "diagram.png", + "file_url": "https://openrag.example/static/image-id", + }, + { + "filename": "demo.mp4", + "file_url": "https://openrag.example/static/video-id", + }, + { + "filename": "recording.mp3", + "file_url": "https://openrag.example/static/audio-id", + }, + { + "filename": "notes.txt", + "file_url": "https://openrag.example/static/text-id", + "chunk_url": "https://openrag.example/chunks/text-id", + }, + ] + ) + + assert source_names == [r"diagram\.png", r"demo\.mp4", r"recording\.mp3", r"notes\.txt"] + assert [element.name for element in elements] == ["diagram.png", "demo.mp4", "recording.mp3", "notes.txt"] + assert elements[-1].content == "Page-less text content" + + @pytest.mark.asyncio async def test_chainlit_keeps_valid_web_sources(monkeypatch): module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_web_source_test") From 43060c69ee419c155b0f7a655bdc35c62892b20b Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:59:44 +0200 Subject: [PATCH 04/13] fix(chainlit): keep source citations clickable --- openrag/app_front.py | 25 +++++++++---- tests/unit/test_app_front_secret.py | 56 +++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/openrag/app_front.py b/openrag/app_front.py index aa6072033..fc7d7f0c9 100644 --- a/openrag/app_front.py +++ b/openrag/app_front.py @@ -45,6 +45,7 @@ _OPENRAG_TOKEN_STORE: dict[str, tuple[str, float]] = {} _MARKDOWN_ESCAPE_TABLE = str.maketrans({char: f"\\{char}" for char in string.punctuation}) _MARKDOWN_URL_SAFE_CHARS = ":/?#[]@!$&'+,;=%" +_MARKDOWN_UNSAFE_SOURCE_NAME_CHARS = str.maketrans(dict.fromkeys("[]()*_`~#>|\\", " ")) class MissingOpenRAGCredentialError(RuntimeError): @@ -56,6 +57,17 @@ def _escape_markdown_text(value: str) -> str: return value.translate(_MARKDOWN_ESCAPE_TABLE) +def _safe_source_name(value: str, existing: dict) -> str: + """Build a Markdown-inert name that Chainlit can match to its element.""" + base = " ".join(value.translate(_MARKDOWN_UNSAFE_SOURCE_NAME_CHARS).split()) or "source" + candidate = base + suffix = 2 + while candidate in existing: + candidate = f"{base} {suffix}" + suffix += 1 + return candidate + + def get_user_language() -> str: """Return the active language: env override if set, otherwise browser's Accept-Language.""" if DEFAULT_LANGUAGE: @@ -499,9 +511,7 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): continue source_label = title or url - source_name = source_label - if source_name in d: - source_name = f"{source_name} ({i})" + source_name = _safe_source_name(source_label, d) content = f"**[{_escape_markdown_text(source_label)}]({markdown_url})**" if snippet: content += f"\n\n{_escape_markdown_text(snippet)}" @@ -533,9 +543,10 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): if api_key and (AUTH_MODE != "oidc" or _current_openrag_auth_provider() == "credentials"): file_url = f"{file_url}?token={api_key}" page_label = str(page).strip() if page is not None else "" - source_name = f"{filename}" + ( + source_label = f"{filename}" + ( f" (page: {page_label})" if suffix in [".pdf", ".pptx", ".docx", ".doc"] and page_label else "" ) + source_name = _safe_source_name(source_label, d) try: if only_txt: @@ -552,7 +563,7 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): elem = cl.Pdf( name=source_name, url=file_url, - page=int(page), + page=int(page) if page_label else None, display="side", ) case suffix if suffix in [".png", ".jpg", ".jpeg"]: @@ -569,13 +580,13 @@ async def _format_sources(metadata_sources, only_txt=False, api_key=None): if not isinstance(chunk_content, str) or not chunk_content.strip(): continue elem = cl.Text(content=chunk_content, name=source_name, display="side") - except (httpx.HTTPError, TypeError, ValueError, AttributeError): + except (httpx.HTTPError, httpx.InvalidURL, TypeError, ValueError, AttributeError): logger.warning("Skipping an unavailable source", source_index=i) continue d[source_name] = elem - source_names = [_escape_markdown_text(name) for name in d] + source_names = list(d) elements = list(d.values()) return elements, source_names diff --git a/tests/unit/test_app_front_secret.py b/tests/unit/test_app_front_secret.py index b61438c7f..c17df0ea7 100644 --- a/tests/unit/test_app_front_secret.py +++ b/tests/unit/test_app_front_secret.py @@ -421,7 +421,6 @@ async def fake_load_model_ids(_client, api_key): [{"source_type": "web", "title": "", "url": "", "snippet": ""}], [{"source_type": "web", "title": "Invalid URL", "url": "http://[invalid"}], [{"filename": "", "file_url": "", "page": ""}], - [{"filename": "document.pdf", "file_url": "https://openrag.example/static/source-id"}], ], ) @pytest.mark.asyncio @@ -469,11 +468,32 @@ async def available_chunk(*_args, **_kwargs): ] ) - assert source_names == [r"diagram\.png", r"demo\.mp4", r"recording\.mp3", r"notes\.txt"] + assert source_names == ["diagram.png", "demo.mp4", "recording.mp3", "notes.txt"] assert [element.name for element in elements] == ["diagram.png", "demo.mp4", "recording.mp3", "notes.txt"] + assert source_names == [element.name for element in elements] assert elements[-1].content == "Page-less text content" +@pytest.mark.asyncio +async def test_chainlit_keeps_page_less_pdf_sources(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_page_less_pdf_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + elements, source_names = await module._format_sources( + [ + { + "filename": "report_[draft].pdf", + "file_url": "https://openrag.example/static/pdf-id", + } + ] + ) + + assert source_names == ["report draft .pdf"] + assert source_names == [element.name for element in elements] + assert elements[0].page is None + + @pytest.mark.asyncio async def test_chainlit_keeps_valid_web_sources(monkeypatch): module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_web_source_test") @@ -516,8 +536,8 @@ async def test_chainlit_escapes_untrusted_web_source_markdown(monkeypatch): ] ) - assert source_names == [r"Reference \]\(https\:\/\/spoof\.test\) \*\*trusted\*\*"] - assert elements[0].name == title + assert source_names == ["Reference https://spoof.test trusted"] + assert elements[0].name == source_names[0] assert elements[0].content == ( r"**[Reference \]\(https\:\/\/spoof\.test\) \*\*trusted\*\*]" "(https://example.test/reference_%28draft%29)**\n\n" @@ -526,13 +546,37 @@ async def test_chainlit_escapes_untrusted_web_source_markdown(monkeypatch): @pytest.mark.asyncio -async def test_chainlit_skips_unavailable_text_sources(monkeypatch): +async def test_chainlit_keeps_source_names_unique_after_sanitizing(monkeypatch): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_source_name_collision_test") + monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") + _stub_chainlit_elements(module) + + elements, source_names = await module._format_sources( + [ + {"source_type": "web", "title": "Reference [draft]", "url": "https://example.test/one"}, + {"source_type": "web", "title": "Reference (draft)", "url": "https://example.test/two"}, + ] + ) + + assert source_names == ["Reference draft", "Reference draft 2"] + assert source_names == [element.name for element in elements] + + +@pytest.mark.parametrize( + "source_error", + [ + pytest.param(httpx.ConnectError("source unavailable"), id="connection-error"), + pytest.param(httpx.InvalidURL("invalid source URL"), id="invalid-url"), + ], +) +@pytest.mark.asyncio +async def test_chainlit_skips_unavailable_text_sources(monkeypatch, source_error): module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_unavailable_source_test") monkeypatch.setattr(module, "get_external_url", lambda: "https://openrag.example") _stub_chainlit_elements(module) async def unavailable_chunk(*_args, **_kwargs): - raise httpx.ConnectError("source unavailable") + raise source_error monkeypatch.setattr(module, "__fetch_page_content", unavailable_chunk) From bae5a27f0faf3313a822edd6512ac985be9b0fc2 Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:49:13 +0200 Subject: [PATCH 05/13] fix(chainlit): preserve safe source name punctuation --- openrag/app_front.py | 4 ++- tests/unit/test_app_front_secret.py | 40 ++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/openrag/app_front.py b/openrag/app_front.py index fc7d7f0c9..69398e898 100644 --- a/openrag/app_front.py +++ b/openrag/app_front.py @@ -45,7 +45,9 @@ _OPENRAG_TOKEN_STORE: dict[str, tuple[str, float]] = {} _MARKDOWN_ESCAPE_TABLE = str.maketrans({char: f"\\{char}" for char in string.punctuation}) _MARKDOWN_URL_SAFE_CHARS = ":/?#[]@!$&'+,;=%" -_MARKDOWN_UNSAFE_SOURCE_NAME_CHARS = str.maketrans(dict.fromkeys("[]()*_`~#>|\\", " ")) +# Chainlit inserts source names into Markdown link labels. Strip only characters +# that can break or restyle that label so ordinary filenames stay recognizable. +_MARKDOWN_UNSAFE_SOURCE_NAME_CHARS = str.maketrans(dict.fromkeys("[]*`\\>", " ")) class MissingOpenRAGCredentialError(RuntimeError): diff --git a/tests/unit/test_app_front_secret.py b/tests/unit/test_app_front_secret.py index c17df0ea7..f0ab9515a 100644 --- a/tests/unit/test_app_front_secret.py +++ b/tests/unit/test_app_front_secret.py @@ -489,7 +489,7 @@ async def test_chainlit_keeps_page_less_pdf_sources(monkeypatch): ] ) - assert source_names == ["report draft .pdf"] + assert source_names == ["report_ draft .pdf"] assert source_names == [element.name for element in elements] assert elements[0].page is None @@ -536,7 +536,7 @@ async def test_chainlit_escapes_untrusted_web_source_markdown(monkeypatch): ] ) - assert source_names == ["Reference https://spoof.test trusted"] + assert source_names == ["Reference (https://spoof.test) trusted"] assert elements[0].name == source_names[0] assert elements[0].content == ( r"**[Reference \]\(https\:\/\/spoof\.test\) \*\*trusted\*\*]" @@ -554,7 +554,7 @@ async def test_chainlit_keeps_source_names_unique_after_sanitizing(monkeypatch): elements, source_names = await module._format_sources( [ {"source_type": "web", "title": "Reference [draft]", "url": "https://example.test/one"}, - {"source_type": "web", "title": "Reference (draft)", "url": "https://example.test/two"}, + {"source_type": "web", "title": "Reference *draft*", "url": "https://example.test/two"}, ] ) @@ -562,6 +562,40 @@ async def test_chainlit_keeps_source_names_unique_after_sanitizing(monkeypatch): assert source_names == [element.name for element in elements] +@pytest.mark.parametrize( + ("source_name", "expected"), + [ + ("rapport_annuel_2026.pdf", "rapport_annuel_2026.pdf"), + ("report.pdf (page: 3)", "report.pdf (page: 3)"), + ("C++_style_guide.pdf", "C++_style_guide.pdf"), + ], +) +def test_chainlit_preserves_safe_source_name_punctuation(monkeypatch, source_name, expected): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_source_name_punctuation_test") + + assert module._safe_source_name(source_name, {}) == expected + + +@pytest.mark.parametrize( + "hostile_name", + [ + "x](https://evil.test)", + "nested[a](b)c", + "trailing\\", + "**bold**", + "back`tick`", + ">quote", + ], +) +def test_chainlit_source_name_cannot_break_markdown_link(monkeypatch, hostile_name): + module = _load_app_front(monkeypatch, auth_mode="token", module_name="app_front_source_name_safety_test") + + safe_name = module._safe_source_name(hostile_name, {}) + + assert not set(safe_name) & set("[]*`\\>") + assert safe_name.strip() == safe_name + + @pytest.mark.parametrize( "source_error", [ From 3d89102f572454d21e9d04666efba3a8f69b1c43 Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:01:27 +0200 Subject: [PATCH 06/13] fix(chainlit): neutralize active source markdown --- openrag/app_front.py | 5 +++++ tests/unit/test_app_front_secret.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/openrag/app_front.py b/openrag/app_front.py index 69398e898..f6a2a26d2 100644 --- a/openrag/app_front.py +++ b/openrag/app_front.py @@ -1,5 +1,6 @@ import json import os +import re import secrets import string import time @@ -48,6 +49,8 @@ # Chainlit inserts source names into Markdown link labels. Strip only characters # that can break or restyle that label so ordinary filenames stay recognizable. _MARKDOWN_UNSAFE_SOURCE_NAME_CHARS = str.maketrans(dict.fromkeys("[]*`\\>", " ")) +_MARKDOWN_UNDERSCORE_EMPHASIS = re.compile(r"(?_+)(?P