From 5390634d6eea15f0b155ac4108d2aaa79b447b99 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 18 Sep 2026 07:15:35 +0000 Subject: [PATCH 1/2] Require a human check on the chat, and stop absence meaning "off" Adam asked for a human check when going to the regular chat, not only the search-page endpoint. The middleware already existed; what did not was any way to tell whether a given deployment was actually using it. `is_captcha_exempt` treated "no CLOUDFLARE_SECRET_KEY" as "no captcha to enforce" and let everything through. So "there is captcha middleware" and "the chat is gated" were different statements, and the gap was silent: beta has been serving an ungated chat, correctly per that rule and invisibly. Now the decision is written down rather than inferred. CHAT_REQUIRES_HUMAN defaults to on, and a deployment that requires a human but has no key refuses to start, with a message saying which of the two things to do. An ungated chat is still available -- beta may want one -- but only by setting CHAT_REQUIRES_HUMAN=0, which someone has to type. Refusing to start rather than serving unprotected is the trade the answer endpoint's verifying key already makes. Losing the feature is the correct failure. Sequencing, because this could otherwise take beta down: the deploy script now checks the env file for a key or an explicit opt-out *before* it stops the running container, the same way it checks the caller-token key. Exercised against beta's current env, where it correctly refuses, and against five variants including an empty value and a mistyped opt-out, both of which fail closed. The keys themselves are Adam's to place. ~/install-beta-captcha-keys.sh prompts for them, hides the secret, keeps 0600 and a backup, and prints neither -- a transcript is a worse home for a credential than the host, the same reasoning that kept the signing key off this machine. Co-Authored-By: Claude Opus 5 --- bin/chat-fastapi.py | 21 ++++++++ src/util/captcha_scope.py | 12 ++++- tests/api/test_chat_requires_human.py | 74 +++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/api/test_chat_requires_human.py diff --git a/bin/chat-fastapi.py b/bin/chat-fastapi.py index 85321c6..50bfa1b 100644 --- a/bin/chat-fastapi.py +++ b/bin/chat-fastapi.py @@ -69,6 +69,27 @@ async def lifespan(_app: FastAPI) -> AsyncIterator[None]: CHAINLIT_URL = os.getenv("CHAINLIT_URL") CLOUDFLARE_SECRET_KEY = get_secret("CLOUDFLARE_SECRET_KEY") + +# A human check on the chat is required unless a deployment says otherwise, and +# saying otherwise is explicit. Absence used to mean "no captcha to enforce", so +# a deployment that simply had no key served an ungated chat and reported +# nothing -- satisfied on paper, off in practice. +# +# Refusing to start rather than serving ungated is the same trade the answer +# endpoint's verifying key already makes: losing the feature is the correct +# failure, serving it unprotected is not. +CHAT_REQUIRES_HUMAN = os.getenv("CHAT_REQUIRES_HUMAN", "1").strip() not in { + "0", + "false", + "no", +} +if CHAT_REQUIRES_HUMAN and not CLOUDFLARE_SECRET_KEY: + raise RuntimeError( + "CHAT_REQUIRES_HUMAN is on and no CLOUDFLARE_SECRET_KEY is configured, " + "so the chat would be served with no human check at all. Provide the " + "Turnstile keys, or set CHAT_REQUIRES_HUMAN=0 to say deliberately that " + "this deployment does not want one." + ) CLOUDFLARE_SITE_KEY = os.getenv("CLOUDFLARE_SITE_KEY") ERROR_PAGE_TEMPLATE = Template( diff --git a/src/util/captcha_scope.py b/src/util/captcha_scope.py index f4d54f7..9e1349e 100644 --- a/src/util/captcha_scope.py +++ b/src/util/captcha_scope.py @@ -49,8 +49,16 @@ def is_captcha_exempt( return True if any(path.startswith(prefix) for prefix in extra_prefixes): return True - # No captcha configured means no captcha to enforce. Deliberate: beta runs - # without one because the Turnstile site key is bound to reactome.org. + # A missing key is NOT a reason to skip the check. It used to be, and that + # is the failure this exists to prevent: "there is captcha middleware" and + # "the chat is gated" were different statements, and nothing said which one + # was true of a given deployment. + # + # `captcha_configured` is still taken, because a deployment that has + # deliberately opted out (CHAT_REQUIRES_HUMAN=0) passes False and expects to + # be let through. What changed is where that decision is made: in + # configuration someone wrote, rather than inferred from a value being + # absent. if not captcha_configured: return True # Anything outside the Chainlit app is not ours to guard. diff --git a/tests/api/test_chat_requires_human.py b/tests/api/test_chat_requires_human.py new file mode 100644 index 0000000..86c4031 --- /dev/null +++ b/tests/api/test_chat_requires_human.py @@ -0,0 +1,74 @@ +"""The chat must not be served without a human check by accident. + +Adam asked for a human check on the regular chat, not only the search-page +endpoint. The obstacle was not the check -- the middleware existed -- but that a +deployment with no Turnstile key skipped it and said nothing, so "there is +captcha middleware" and "the chat is gated" were different statements with +nothing to tell them apart. + +This pins the decision being explicit in both directions: no key means refuse to +start, and a deployment that genuinely wants no check must say so. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).parent.parent.parent +SNIPPET = """ +import os, sys +sys.path.insert(0, "src") +required = os.getenv("CHAT_REQUIRES_HUMAN", "1").strip() not in {"0", "false", "no"} +secret = os.getenv("CLOUDFLARE_SECRET_KEY") or "" +if required and not secret: + raise SystemExit("REFUSED") +print("STARTED") +""" + + +def _run(env: dict[str, str]) -> str: + """The startup decision, in isolation. + + Importing bin/chat-fastapi.py itself would build a graph and need an OpenAI + key, so the guard's logic is exercised rather than the module. The test for + the two staying in step is below. + """ + result = subprocess.run( + [sys.executable, "-c", SNIPPET], + capture_output=True, + text=True, + cwd=REPO, + env={"PATH": "/usr/bin:/bin", **env}, + ) + return (result.stdout + result.stderr).strip() + + +def test_no_key_and_no_opt_out_refuses_to_start() -> None: + assert "REFUSED" in _run({}) + + +def test_a_key_starts_the_chat() -> None: + assert "STARTED" in _run({"CLOUDFLARE_SECRET_KEY": "0x-secret"}) + + +@pytest.mark.parametrize("value", ["0", "false", "no"]) +def test_an_explicit_opt_out_starts_without_a_key(value: str) -> None: + """Beta ran ungated deliberately. That stays possible -- it just has to be + written down rather than inferred from an absent value.""" + assert "STARTED" in _run({"CHAT_REQUIRES_HUMAN": value}) + + +@pytest.mark.parametrize("value", ["1", "true", "yes", "", "anything-else"]) +def test_anything_but_an_explicit_off_still_requires_a_key(value: str) -> None: + """A typo in the opt-out must not silently disable the check.""" + assert "REFUSED" in _run({"CHAT_REQUIRES_HUMAN": value}) + + +def test_the_guard_in_the_app_matches_the_one_tested_here() -> None: + """The snippet above is a copy, so pin that it has not drifted.""" + source = (REPO / "bin" / "chat-fastapi.py").read_text() + assert 'os.getenv("CHAT_REQUIRES_HUMAN", "1").strip() not in {' in source + assert '"0",' in source and '"false",' in source and '"no",' in source + assert "if CHAT_REQUIRES_HUMAN and not CLOUDFLARE_SECRET_KEY:" in source From d20ad485cb24e20021e7fdc30f9be00409fb8be0 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 18 Sep 2026 07:16:19 +0000 Subject: [PATCH 2/2] Satisfy the linter on the new test Committed the previous change with ruff failing: I printed its exit code and then ran git anyway, which is the second time today that shape has caught me. Co-Authored-By: Claude Opus 5 --- tests/api/test_chat_requires_human.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/api/test_chat_requires_human.py b/tests/api/test_chat_requires_human.py index 86c4031..f17112c 100644 --- a/tests/api/test_chat_requires_human.py +++ b/tests/api/test_chat_requires_human.py @@ -35,7 +35,7 @@ def _run(env: dict[str, str]) -> str: key, so the guard's logic is exercised rather than the module. The test for the two staying in step is below. """ - result = subprocess.run( + result = subprocess.run( # noqa: S603 - fixed argv, no shell, no user input [sys.executable, "-c", SNIPPET], capture_output=True, text=True, @@ -70,5 +70,6 @@ def test_the_guard_in_the_app_matches_the_one_tested_here() -> None: """The snippet above is a copy, so pin that it has not drifted.""" source = (REPO / "bin" / "chat-fastapi.py").read_text() assert 'os.getenv("CHAT_REQUIRES_HUMAN", "1").strip() not in {' in source - assert '"0",' in source and '"false",' in source and '"no",' in source + for off in ('"0",', '"false",', '"no",'): + assert off in source assert "if CHAT_REQUIRES_HUMAN and not CLOUDFLARE_SECRET_KEY:" in source