From 1b8c939a8547d5ab7aa9295761a6f874f619e182 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Sat, 3 Oct 2026 00:21:52 +0000 Subject: [PATCH] Close four disclosure and injection paths found in review (area 1a) From the code review of src/api and src/handoff: - A search question handed off to the chat was rendered unescaped, and the chat renders HTML. A shared Continue-in-chat link put live markup into the chat of whoever opened it. Escaped; checked in a browser both ways (with the escape removed, the markup rendered as a live element). - httpx logs every request URL at INFO, and Analysis Service URLs carry the reader's analysis token. httpx and httpcore now log at WARNING. - The gene-list result seeded into the model's history included the Pathway Browser link, which embeds the analysis token. The model now gets the reply without it. - A thread seeded with a reader's analysis no longer runs the web search, whose query is rephrased from that history: the reader agreed to show the model provider, not a search engine. Co-Authored-By: Claude Opus 5.5 --- bin/chat-chainlit.py | 11 +++++++++-- src/analysis/gene_list.py | 11 ++++++++++- src/handoff/seed.py | 7 ++++++- src/util/logging.py | 8 ++++++++ tests/analysis/test_gene_list.py | 11 +++++++++++ tests/handoff/test_handoff_seed.py | 24 ++++++++++++++++++++++++ tests/util/test_logging_tokens.py | 22 ++++++++++++++++++++++ 7 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 tests/util/test_logging_tokens.py diff --git a/bin/chat-chainlit.py b/bin/chat-chainlit.py index bd020e9..4eb7775 100644 --- a/bin/chat-chainlit.py +++ b/bin/chat-chainlit.py @@ -259,6 +259,8 @@ async def continue_from_handoff(handoff_id: str) -> None: await cl.Message(content=seed.UNAVAILABLE).send() return + if isinstance(handoff, AnalysisHandoff): + cl.user_session.set("analysis_seeded", True) logger.info( "handoff claimed", extra={ @@ -434,7 +436,7 @@ async def run_gene_list_analysis(text: str, identifiers: list[str]) -> None: seeded = await get_graph().seed_history( profile, thread_id=current_thread_id(), - messages=[HumanMessage(content=text), AIMessage(content=reply.text)], + messages=[HumanMessage(content=text), AIMessage(content=reply.for_model)], ) except Exception: # The reader has their result; only follow-ups lose it. @@ -482,7 +484,12 @@ async def answer_with_model(content: str, message_id: str) -> None: ) openai_cb = OpenAICallbackHandler() - enable_postprocess: bool = is_feature_enabled(config, "postprocessing") + # Not on a thread seeded with a reader's analysis: the rephrased question + # sent to the web search is built from the history, which then holds + # data they agreed to show the model provider -- not a search engine. + enable_postprocess: bool = is_feature_enabled( + config, "postprocessing" + ) and not cl.user_session.get("analysis_seeded", False) result: OutputState = await get_graph().ainvoke( content, chat_profile.lower(), diff --git a/src/analysis/gene_list.py b/src/analysis/gene_list.py index e2db6be..164c3aa 100644 --- a/src/analysis/gene_list.py +++ b/src/analysis/gene_list.py @@ -476,6 +476,10 @@ class Overrepresentation: text: str #: False when nothing matched -- nothing for a follow-up to be about. has_pathways: bool + #: What the model may see: `text` without the Pathway Browser link. The + #: link embeds the analysis token, and anyone holding the token can + #: fetch the result; it goes to the reader, never to the model. + for_model: str = "" def _fdr(value: Any) -> str: @@ -564,7 +568,12 @@ def describe_overrepresentation( f"Not found in Reactome: {shown}" + (f" and {more} more" if more > 0 else ""), ] - return Overrepresentation(text="\n".join(lines), has_pathways=bool(pathways)) + text = "\n".join(lines) + return Overrepresentation( + text=text, + has_pathways=bool(pathways), + for_model="\n".join(line for line in lines if browser_url not in line), + ) FAILED = ( diff --git a/src/handoff/seed.py b/src/handoff/seed.py index f65cb58..7f3081c 100644 --- a/src/handoff/seed.py +++ b/src/handoff/seed.py @@ -28,6 +28,7 @@ from analysis.disclosure import for_tier from analysis.summarise import prompt_input from handoff.store import DEFAULT_TTL_SECONDS, AnalysisHandoff, Handoff, SearchHandoff +from util.markdown import escape #: What the reader asked for on the website, stated as what happened. HUMAN_TURN = ( @@ -98,7 +99,11 @@ def shown_to_reader(handoff: Handoff) -> str: """What the reader sees when the tab opens. Their own text, verbatim.""" if isinstance(handoff, SearchHandoff): return ( - f"Continuing from your search: **{handoff.question}**\n\n" + # Escaped: the question is whatever the search request carried, + # and the chat renders HTML. Unescaped, a shared handoff link put + # live markup -- script in a srcdoc iframe -- into the chat of + # whoever opened it (review, area 1a). + f"Continuing from your search: **{escape(handoff.question)}**\n\n" f"{handoff.summary}\n\n" "---\nAsk a follow-up question." ) diff --git a/src/util/logging.py b/src/util/logging.py index 8a7bf13..e800bef 100644 --- a/src/util/logging.py +++ b/src/util/logging.py @@ -17,6 +17,14 @@ "level": DEFAULT_LOG_LEVEL, # Change to WARNING, ERROR, or CRITICAL }, }, + "loggers": { + # httpx logs every request URL at INFO. Analysis Service URLs carry + # the reader's analysis token -- a bearer capability for their full + # result -- in the path, so at INFO every summary and handoff wrote + # tokens into the logs (review, area 1a). + "httpx": {"level": "WARNING"}, + "httpcore": {"level": "WARNING"}, + }, "root": { "handlers": ["console"], "level": DEFAULT_LOG_LEVEL, # Set the default log level for all loggers diff --git a/tests/analysis/test_gene_list.py b/tests/analysis/test_gene_list.py index cb600f4..f46dd19 100644 --- a/tests/analysis/test_gene_list.py +++ b/tests/analysis/test_gene_list.py @@ -604,3 +604,14 @@ def test_reading_a_whole_message_is_fast(invited: bool) -> None: started = time.perf_counter() read_message(text, invited=invited) assert time.perf_counter() - started < 2.0 + + +def test_the_model_never_gets_the_link_that_carries_the_token() -> None: + # The Pathway Browser link embeds the analysis token; anyone holding it can + # fetch the result. It is for the reader, never the model (review, 1a). + token = MEASURED["summary"]["token"] # type: ignore[index] + url = f"https://beta.reactome.org/PathwayBrowser/#/DTAB=AN&ANALYSIS={token}" + reply = describe_overrepresentation(SUBMITTED, MEASURED, url, ["NOTAGENE1"]) + assert url in reply.text + assert token not in reply.for_model + assert "Regulation of TP53 Expression" in reply.for_model diff --git a/tests/handoff/test_handoff_seed.py b/tests/handoff/test_handoff_seed.py index 7e1c663..6aa0920 100644 --- a/tests/handoff/test_handoff_seed.py +++ b/tests/handoff/test_handoff_seed.py @@ -181,3 +181,27 @@ def test_an_empty_answer_is_never_kept() -> None: ) ) assert kept is None + + +def test_a_search_question_is_shown_as_text_not_markup() -> None: + # Review, area 1a: the question is whatever the search request carried, + # the chat renders HTML, and a handoff link can be shared. Unescaped, it + # put live markup into the chat of whoever opened the link. + from handoff.store import SearchHandoff + + hostile = ' CDK5' + shown = seed.shown_to_reader( + SearchHandoff( + kind="search", + question=hostile, + summary="CDK5 phosphorylates tau.", + citations=(), + created_at=time.time(), + ) + ) + # Every "<" is escaped, so markdown yields text and no HTML node; the + # browser check is in ~/chat-uitest/handoff_markup.py. + import re + + assert re.search(r"(? None: + for name in ("httpx", "httpcore"): + assert not logging.getLogger(name).isEnabledFor(logging.INFO) + + +def test_our_own_info_logs_still_are() -> None: + # The fix must lower only the request logger, not logging generally. + root = logging.getLogger() + assert root.isEnabledFor(logging.getLevelName(util.logging.DEFAULT_LOG_LEVEL))