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..f17112c --- /dev/null +++ b/tests/api/test_chat_requires_human.py @@ -0,0 +1,75 @@ +"""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( # noqa: S603 - fixed argv, no shell, no user input + [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 + for off in ('"0",', '"false",', '"no",'): + assert off in source + assert "if CHAT_REQUIRES_HUMAN and not CLOUDFLARE_SECRET_KEY:" in source