diff --git a/bin/chat-fastapi.py b/bin/chat-fastapi.py index 9b53a9e..85321c6 100644 --- a/bin/chat-fastapi.py +++ b/bin/chat-fastapi.py @@ -15,9 +15,9 @@ from agent.registry import build_graph, set_graph from api.answer import router as answer_router +from util.caller_token import load_verifying_key from util.captcha_scope import is_captcha_exempt from util.embedding_environment import EmbeddingEnvironment -from util.human_token import load_verifying_key from util.logging import logging from util.secrets import SECRET_NAMES, get_secret, load_secrets_to_environ @@ -43,7 +43,7 @@ async def lifespan(_app: FastAPI) -> AsyncIterator[None]: started = time.monotonic() # Before the graph: a missing verifying key must stop the process, and # spending 52 seconds building a graph first only delays the failure. - _app.state.human_token_key = load_verifying_key() + _app.state.caller_token_key = load_verifying_key() # The release the served answers are built from, for FR-007 cache # invalidation. Read here rather than per request because the graph below is # built from these same bundles, so this value describes what is served even diff --git a/bin/make-human-token-keypair.py b/bin/make-caller-token-keypair.py similarity index 92% rename from bin/make-human-token-keypair.py rename to bin/make-caller-token-keypair.py index 2c2d264..d394b25 100755 --- a/bin/make-human-token-keypair.py +++ b/bin/make-caller-token-keypair.py @@ -29,8 +29,8 @@ def main() -> int: directory = Path(sys.argv[1]) directory.mkdir(parents=True, exist_ok=True) - public_path = directory / "human_token_public.pem" - private_path = directory / "human_token_private.pem" + public_path = directory / "caller_token_public.pem" + private_path = directory / "caller_token_private.pem" # Refuse rather than overwrite: silently replacing a private key would # invalidate every token in flight with no way back. @@ -61,7 +61,7 @@ def main() -> int: print(f"private {private_path} (0600, for whoever mints tokens -- D1)") print() print("Point the service at the public half:") - print(f" HUMAN_TOKEN_PUBLIC_KEY_PATH=/run/secrets/{public_path.name}") + print(f" CALLER_TOKEN_PUBLIC_KEY_PATH=/run/secrets/{public_path.name}") return 0 diff --git a/compose.yaml b/compose.yaml index 4fc671a..181953e 100644 --- a/compose.yaml +++ b/compose.yaml @@ -28,11 +28,11 @@ services: OAUTH_GOOGLE_CLIENT_SECRET: ${OAUTH_GOOGLE_CLIENT_SECRET} OPENAI_API_KEY: ${OPENAI_API_KEY} TAVILY_API_KEY: ${TAVILY_API_KEY} - HUMAN_TOKEN_PUBLIC_KEY_PATH: /run/secrets/HUMAN_TOKEN_PUBLIC_KEY + CALLER_TOKEN_PUBLIC_KEY_PATH: /run/secrets/CALLER_TOKEN_PUBLIC_KEY # Postgres access when not using Vault (for development) POSTGRES_PASSWORD: ${POSTGRES_PASSWORD} secrets: - - HUMAN_TOKEN_PUBLIC_KEY + - CALLER_TOKEN_PUBLIC_KEY - CHAINLIT_AUTH_SECRET - CLOUDFLARE_SECRET_KEY - OAUTH_AUTH0_CLIENT_SECRET @@ -72,11 +72,11 @@ services: CLOUDFLARE_SECRET_KEY: ${CLOUDFLARE_SECRET_KEY} OPENAI_API_KEY: ${OPENAI_API_KEY} TAVILY_API_KEY: ${TAVILY_API_KEY} - HUMAN_TOKEN_PUBLIC_KEY_PATH: /run/secrets/HUMAN_TOKEN_PUBLIC_KEY + CALLER_TOKEN_PUBLIC_KEY_PATH: /run/secrets/CALLER_TOKEN_PUBLIC_KEY # Postgres access when not using Vault (for development) POSTGRES_PASSWORD: ${POSTGRES_PASSWORD} secrets: - - HUMAN_TOKEN_PUBLIC_KEY + - CALLER_TOKEN_PUBLIC_KEY - CLOUDFLARE_SECRET_KEY - OPENAI_API_KEY - TAVILY_API_KEY @@ -158,9 +158,9 @@ services: secrets: # The public half of the answer endpoint's token-verifying keypair. # External like the rest, so no key material lives in the repository: - # docker secret create HUMAN_TOKEN_PUBLIC_KEY deploy/beta/human_token_public.pem + # docker secret create CALLER_TOKEN_PUBLIC_KEY deploy/beta/caller_token_public.pem # Generate the pair with ./bin/make-human-token-keypair.py deploy/beta - HUMAN_TOKEN_PUBLIC_KEY: + CALLER_TOKEN_PUBLIC_KEY: external: true CHAINLIT_AUTH_SECRET: external: true diff --git a/docker-compose.yml b/docker-compose.yml index 1cee603..488996f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,7 +22,7 @@ services: - OAUTH_GOOGLE_CLIENT_SECRET=${OAUTH_GOOGLE_CLIENT_SECRET} - CHAINLIT_AUTH_SECRET=${CHAINLIT_AUTH_SECRET} - CHAINLIT_URI=${CHAINLIT_URI} - - HUMAN_TOKEN_PUBLIC_KEY_PATH=${HUMAN_TOKEN_PUBLIC_KEY_PATH} + - CALLER_TOKEN_PUBLIC_KEY_PATH=${CALLER_TOKEN_PUBLIC_KEY_PATH} - CHAINLIT_URL=${CHAINLIT_URL} - CHAINLIT_ROOT_PATH=${CHAINLIT_ROOT_PATH} - TAVILY_API_KEY=${TAVILY_API_KEY} @@ -38,7 +38,7 @@ services: - ./records:/app/records - ./config.yml:/app/config.yml secrets: - - human_token_public.pem + - caller_token_public.pem chainlit-no-login: image: ${CHAINLIT_IMAGE} @@ -55,7 +55,7 @@ services: - CLOUDFLARE_SECRET_KEY=${CLOUDFLARE_SECRET_KEY} - CLOUDFLARE_SITE_KEY=${CLOUDFLARE_SITE_KEY} - CHAINLIT_URI=${CHAINLIT_URI_NO_LOGIN} - - HUMAN_TOKEN_PUBLIC_KEY_PATH=${HUMAN_TOKEN_PUBLIC_KEY_PATH} + - CALLER_TOKEN_PUBLIC_KEY_PATH=${CALLER_TOKEN_PUBLIC_KEY_PATH} - CHAINLIT_URI_LOGIN=${CHAINLIT_URI} - CHAINLIT_URL=${CHAINLIT_URL} - TAVILY_API_KEY=${TAVILY_API_KEY} @@ -68,7 +68,7 @@ services: - ./embeddings:/app/embeddings - ./config.yml:/app/config.yml secrets: - - human_token_public.pem + - caller_token_public.pem postgres: @@ -106,12 +106,12 @@ secrets: # exist, and it declares this as an external secret like every other. # This file-based form keeps THIS compose file working standalone. # - # Delivered at /run/secrets/human_token_public.pem, which is what - # HUMAN_TOKEN_PUBLIC_KEY_PATH points at. A secret rather than a bind mount on + # Delivered at /run/secrets/caller_token_public.pem, which is what + # CALLER_TOKEN_PUBLIC_KEY_PATH points at. A secret rather than a bind mount on # purpose: a bind mount whose source is missing makes Docker create a # root-owned directory at that path, which then needs sudo to clear and which # the key generator would refuse to overwrite. A missing secret just errors. # # Generate it with ./bin/make-human-token-keypair.py deploy/beta - human_token_public.pem: - file: ./deploy/beta/human_token_public.pem + caller_token_public.pem: + file: ./deploy/beta/caller_token_public.pem diff --git a/specs/010-search-page-answers/contracts/answer_endpoint.md b/specs/010-search-page-answers/contracts/answer_endpoint.md index 2de2cdc..34093ee 100644 --- a/specs/010-search-page-answers/contracts/answer_endpoint.md +++ b/specs/010-search-page-answers/contracts/answer_endpoint.md @@ -11,12 +11,36 @@ POST /chat/api/answer Content-Type: application/json { "question": "what does CDK5 phosphorylate in Alzheimer disease?", - "human_token": "" } + "caller_token": "" } ``` -`human_token` is D1 and **not yet decided** -- a signed cookie on the shared parent -domain, a short-lived minted token, or a server-side vouch. This repo verifies -evidence; it does not perform the check. +**`caller_token` is settled (D1, 2026-09-18), and it does not assert humanity.** +The field was `human_token` and the premise was wrong: there is no human gate on +the search path and there will not be one -- nobody solves a captcha to run a +search. What the token asserts is *caller identity*. + +The website mints it server-side, per request, and verification here is: + +| claim | required | checked against | +|---|---|---| +| signature | yes | the public key, EdDSA or RS256 -- never an HS* algorithm | +| `exp` | yes | now; they mint at +120s | +| `aud` | yes | `reactome-chatbot`, overridable with `CALLER_TOKEN_AUDIENCE` | +| `sub` | no, but expected | not validated; used as the rate-limit key | +| `iss` | no | not currently checked -- the signature already identifies the minter | + +`sub` is an opaque per-visit id, 128 random bits, not derived from anything about +the reader. The backstop limit keys on it, so repeat questions in one reading +session count as one caller and a freshly minted token does not buy a fresh +allowance. + +**A token with no `aud`, or one minted for a different audience, is refused.** +Worth stating because the earlier code refused *every* token carrying an `aud` +claim -- PyJWT rejects one when no audience is expected -- so this had to change +before the first real token could ever have been accepted. + +Abuse control is the website's: the panel is opt-in behind a click, so a crawled +search never reaches a model, and their proxy rate limits by address. ## Response: Server-Sent Events @@ -72,6 +96,12 @@ produces a `done` with a non-`answered` state. The website renders no panel. The search page must never be slower or broken because this service is down (FR-006, SC-004). +**Hanging up cancels the work.** Measured: when the caller closes the connection +mid-stream, `CancelledError` is raised inside the answer generator and nothing +further is produced -- 3 tokens read, 3 produced, then cancelled. So a reader who +navigates away does not cost a full model call, and there is nothing for the +proxy to cancel upstream beyond closing the connection. + **The stream is bounded.** The server gives up after 120 seconds and sends `done` with `state: failed`. A caller still needs its own timeout -- a dropped connection sends nothing -- but the server will not hold one open indefinitely. 120s is about diff --git a/specs/010-search-page-answers/plan.md b/specs/010-search-page-answers/plan.md index e530d77..8470226 100644 --- a/specs/010-search-page-answers/plan.md +++ b/specs/010-search-page-answers/plan.md @@ -89,7 +89,7 @@ specs/010-search-page-answers/ src/ ├── api/ # new: the endpoint, its models, SSE framing ├── agent/graph.py # gains a streaming surface -└── util/human_token.py # new: signature verification only +└── util/caller_token.py # new: signature verification only bin/chat-fastapi.py # mounts the router; middleware ordering matters tests/api/ # new diff --git a/specs/010-search-page-answers/quickstart.md b/specs/010-search-page-answers/quickstart.md index 2ba3cd0..b662aca 100644 --- a/specs/010-search-page-answers/quickstart.md +++ b/specs/010-search-page-answers/quickstart.md @@ -13,7 +13,7 @@ Measured 2026-09-17 across the 15 tracked sweep questions, so Phase 5 has a befo curl -N -X POST https://beta.reactome.org/chat/api/answer \ -H 'Content-Type: application/json' \ -d '{"question":"what does CDK5 phosphorylate in Alzheimer disease?", - "human_token":""}' + "caller_token":""}' ``` `-N` matters. Without it curl buffers and the stream looks like a single slow diff --git a/specs/010-search-page-answers/spec.md b/specs/010-search-page-answers/spec.md index 47926c9..61fc6b0 100644 --- a/specs/010-search-page-answers/spec.md +++ b/specs/010-search-page-answers/spec.md @@ -234,20 +234,31 @@ or dropping it for this path -- is open, and is not a blocker for the handover. ## Decisions -### D1 -- what proves a person is human? - -Turnstile already exists here, and a bug in it was fixed on 2026-09-17: a deployment -mounting `CLOUDFLARE_SECRET_KEY` as a Docker secret had the captcha silently -disabled, because the middleware read `os.environ` directly while the value came from -`get_secret`. - -What is missing is the handoff. The website needs something to present to this -service. Options: a signed cookie on the shared parent domain (what the chat uses -now); a short-lived token the website mints after its own Turnstile check; or the -website proxying the call and vouching server-side. - -**Open.** It is a security boundary and belongs with whoever owns the website's -session model, not with this repo alone. +### D1 -- what does the caller present? **Decided 2026-09-18** + +This was "what proves a person is human?", and the question had a false premise. +The website session checked before answering: there is **no human gate on the +search path**, and there is not going to be one -- nobody solves a captcha to run +a search. Their only hCaptcha belongs to the contact form, gating that form and +spent on submit. Every search is anonymous and ungated by design. + +So the token cannot honestly assert humanity, and asking it to would have meant +inventing a claim. **It asserts caller identity instead**: minted server-side by +the website, per request, EdDSA, with `iss`, `aud: reactome-chatbot`, `iat`, +`exp` at +120s, and `sub` -- an opaque per-visit id of 128 random bits, not +derived from anything about the reader, held in an HttpOnly cookie. No address, +no IP hash; neither side holds personal data. + +This service verifies signature, expiry and **audience**, and keys its backstop +limit on `sub`. The request field is `caller_token`, and the misleading +`caller_token` name is gone from the code as well as this document. + +Abuse control moved with the premise. The panel is opt-in behind a click (D5), so +a crawled search never reaches a model, and the website's proxy rate limits by +address. That is a real deterrent rather than a claim we would be inventing. + +Turnstile stays where it is, guarding the chat UI. It was never part of this +path, and the two must not be "unified". ### D2 -- which searches get an answer? diff --git a/specs/010-search-page-answers/tasks.md b/specs/010-search-page-answers/tasks.md index 9f4462a..835abd7 100644 --- a/specs/010-search-page-answers/tasks.md +++ b/specs/010-search-page-answers/tasks.md @@ -27,9 +27,9 @@ website repo entirely. Speed is Phase 5 and does not gate the handover. arrives, tokens stream, citations resolve to real stable IDs, and `done` carries a state. -- [x] T007 [P] [US1] Add src/util/human_token.py: verify signature and expiry only, stateless, no consumption tracking -- [x] T008 [P] [US1] Test human_token in tests/util/test_human_token.py: valid, expired, wrong key, tampered payload, absent — every failure refuses -- [x] T009 [US1] Refuse at startup in src/util/human_token.py when the verifying key is missing or unreadable, rather than accepting everything (Principle IV) +- [x] T007 [P] [US1] Add src/util/caller_token.py: verify signature and expiry only, stateless, no consumption tracking +- [x] T008 [P] [US1] Test caller_token in tests/util/test_caller_token.py: valid, expired, wrong key, tampered payload, absent — every failure refuses +- [x] T009 [US1] Refuse at startup in src/util/caller_token.py when the verifying key is missing or unreadable, rather than accepting everything (Principle IV) - [x] T010 [US1] Add the SSE endpoint in src/api/answer.py implementing contracts/answer_endpoint.md: start, token, citation, done - [x] T011 [US1] Emit citations from retrieved documents' `st_id` metadata, deduplicated — never by parsing anchors out of the model's prose - [x] T012 [US1] Mount the router in bin/chat-fastapi.py and let the captcha middleware pass /chat/api/ through, since the endpoint verifies its own caller @@ -44,6 +44,9 @@ state. - [x] T016 [US2] Test that no model call happens for a refused request in tests/api/test_answer_endpoint.py, by asserting on a patched graph rather than on timing (SC-002) - [x] T017 [P] [US2] Rate limit per token as a backstop in src/util/rate_limit.py; the budget is the website's, enforced before the call reaches here (FR-008). 30 per 10 minutes, keyed on `sub`/`jti` when D1 provides one and a token hash until then (PR #237) - [x] T017a [US2] Stop paying for a discarded web search: the endpoint took `enable_postprocess` at its default, so every answer ran a Tavily search that `astream_answer` has no event to return (PR #237) +- [x] T021 [US2] Enforce `aud` on the caller token (asked for by the website, D1); the code refused every token carrying one, since PyJWT rejects `aud` when no audience is expected (PR #240) +- [x] T022 Rename human_token -> caller_token everywhere; D1 established the token asserts caller identity, not humanity (PR #240) +- [x] T023 Answer the website's cancellation question: a client hang-up raises CancelledError inside the answer generator and produces nothing further, so they need not cancel upstream (PR #240) - [ ] T020a Decide what to do about non-reproducible retrieval: three runs of one question shared only 4 of 19 citations (Jaccard 0.26) because query expansion is itself a model call. Affects what FR-007 can cache - [x] T018 [US2] Return `state: failed` with no partial answer on any internal error, so the page renders no panel (FR-006) - [x] T011a [US1] Strip inline HTML anchors from the token stream in src/util/anchor_strip.py; the contract promises prose without them and the chat prompt emits them, split across ~20 fragments (PR #236) diff --git a/src/api/answer.py b/src/api/answer.py index 05bd408..3cfd753 100644 --- a/src/api/answer.py +++ b/src/api/answer.py @@ -30,7 +30,7 @@ from agent.registry import get_graph from util.anchor_strip import AnchorStripper -from util.human_token import TokenRejectedError, verify +from util.caller_token import TokenRejectedError, verify from util.logging import logging from util.rate_limit import identity_of, limiter_from_env @@ -57,7 +57,7 @@ class AnswerRequest(BaseModel): question: str = Field(min_length=1, max_length=2000) - human_token: str = "" + caller_token: str = "" def _sse(event: str, payload: dict[str, Any]) -> str: @@ -83,21 +83,21 @@ async def body() -> AsyncIterator[str]: @router.post("/answer") async def answer(request: Request, body: AnswerRequest) -> StreamingResponse: - verifying_key = getattr(request.app.state, "human_token_key", None) + verifying_key = getattr(request.app.state, "caller_token_key", None) if not verifying_key: # Should be unreachable: startup refuses without a key. If it happens, # refuse rather than answer. return _refusal("no verifying key on the app") try: - claims = verify(body.human_token, verifying_key) + claims = verify(body.caller_token, verifying_key) except TokenRejectedError as rejected: return _refusal(rejected.reason) # After verification, so an unsigned token cannot consume someone else's # budget by claiming their `sub`, and before the graph, so a caller over the # limit costs nothing. - if not _limiter.allow(identity_of(claims, body.human_token)): + if not _limiter.allow(identity_of(claims, body.caller_token)): return _refusal("rate limited") graph = get_graph() diff --git a/src/util/caller_token.py b/src/util/caller_token.py new file mode 100644 index 0000000..486e15e --- /dev/null +++ b/src/util/caller_token.py @@ -0,0 +1,129 @@ +"""Verify the token the website mints for each call. Signature, expiry, audience. + +Agreed with the website session on 2026-09-18, which corrected the premise this +started from. There is **no human gate on their search path** and there is not +going to be one -- nobody solves a captcha to run a search. Their only hCaptcha +belongs to the contact form, and its response is spent on submit. So a token here +cannot honestly assert that a human is present, and this module does not claim it +does. + +What it does assert is **caller identity**: this request came from the Reactome +website's server, for one visit. They mint server-side per request, EdDSA, with +`iss`, `aud`, `iat`, `exp` at +120s, and `sub` -- an opaque per-visit id that is +128 random bits, not derived from anything about the reader. Abuse control is +theirs: the panel is opt-in behind a click, so a crawled search never reaches a +model, and their proxy rate limits by address. + +**Asymmetric on purpose.** They hold the signing key; this holds only the public +half. A shared secret would let either side mint, and this is the side reachable +from a search page if the proxy is ever bypassed. + +**The audience is enforced, not optional.** They asked for it, and it is what +stops a token minted for this service being replayed at a different consumer +later -- or one minted for something else being presented here. + +**Stateless on purpose.** The query budget is theirs, enforced before the call +reaches here. What this service keeps is a backstop limit keyed on `sub`. +""" + +import os +from dataclasses import dataclass +from pathlib import Path + +import jwt + +ALGORITHMS = ["EdDSA", "RS256"] +"""What the website may sign with. Both are asymmetric; no HS* symmetric option is +offered, because accepting one would let a leaked verifying key mint tokens.""" + +KEY_PATH_ENV = "CALLER_TOKEN_PUBLIC_KEY_PATH" + + +@dataclass(frozen=True) +class TokenRejectedError(Exception): + """Why a token was refused. The reason is logged, never returned to the caller.""" + + reason: str + + +def load_verifying_key(path: str | None = None) -> str: + """The public key, or refuse to start. + + Principle IV. An endpoint that accepts everything because its key is missing is + the worst outcome available, and it would test clean -- every request would + succeed. + """ + configured = path or os.getenv(KEY_PATH_ENV) + if not configured: + raise RuntimeError( + f"{KEY_PATH_ENV} is not set. The answer endpoint verifies a signed " + "proof-of-human token and cannot run without a verifying key; starting " + "without one would accept every request." + ) + key_file = Path(configured) + try: + key = key_file.read_text().strip() + except OSError as exc: + raise RuntimeError( + f"Cannot read the verifying key at {key_file}: {exc}" + ) from exc + if not key: + raise RuntimeError(f"The verifying key at {key_file} is empty.") + return key + + +AUDIENCE_ENV = "CALLER_TOKEN_AUDIENCE" +DEFAULT_AUDIENCE = "reactome-chatbot" + + +def expected_audience() -> str: + """Who tokens must be minted for. Configurable, but never empty. + + An empty audience would not mean "accept anything" -- PyJWT rejects a token + that carries `aud` when none is expected -- so a blank setting would refuse + every real token instead of loosening the check. Falling back to the agreed + value keeps a misconfiguration from looking like a signing problem. + """ + return os.getenv(AUDIENCE_ENV, "").strip() or DEFAULT_AUDIENCE + + +def verify(token: str, verifying_key: str, *, audience: str | None = None) -> dict: + """Return the token's claims, or raise TokenRejectedError. + + Every failure path refuses. There is deliberately no branch that returns claims + for an unverified token, however malformed. + + `audience` defaults to the configured one and is required: `aud` is in the + required claims, so a token without it is refused by name rather than slipping + through. Passing an explicit audience is for tests. + """ + if not token: + raise TokenRejectedError("no token presented") + expected = audience or expected_audience() + try: + return dict( + jwt.decode( + token, + verifying_key, + algorithms=ALGORITHMS, + audience=expected, + options={"require": ["exp", "aud"]}, + ) + ) + except jwt.ExpiredSignatureError as exc: + raise TokenRejectedError("token expired") from exc + except jwt.InvalidAudienceError as exc: + raise TokenRejectedError("token minted for a different audience") from exc + except jwt.InvalidSignatureError as exc: + raise TokenRejectedError("signature does not verify") from exc + except jwt.MissingRequiredClaimError as exc: + # Named, not assumed: this used to report "no expiry" for whatever was + # missing, which would now misreport a token with no audience. A token + # with no expiry is a permanent credential, which is what the short + # lifetime exists to avoid; one with no audience cannot be checked + # against the consumer it was minted for. + raise TokenRejectedError( + f"token is missing a required claim: {exc.claim}" + ) from exc + except jwt.InvalidTokenError as exc: + raise TokenRejectedError(f"invalid token: {type(exc).__name__}") from exc diff --git a/src/util/human_token.py b/src/util/human_token.py deleted file mode 100644 index e8d8e4f..0000000 --- a/src/util/human_token.py +++ /dev/null @@ -1,97 +0,0 @@ -"""Verify that the caller proved someone is human. Signature and expiry, nothing else. - -Agreed with the website session on 2026-09-17. They run their own captcha, mint a -short-lived token, and proxy every request presenting it. This service verifies the -signature and refuses otherwise. - -**Asymmetric on purpose.** They hold the signing key; this holds only a public -verifying key. A shared secret would let either side mint, and this is the side -reachable from a search page if the proxy is ever bypassed -- so compromising it must -not produce valid tokens. - -**Stateless on purpose.** The query budget is theirs, enforced before the call -reaches here, because they proxy every request. Counting consumption here would mean -state and a second counter that can disagree with theirs. - -Worth knowing, because it looks like an inconsistency: this repo verifies Cloudflare -Turnstile for the chat UI and the website's search page uses hCaptcha. Under a shared -cookie those two would have to be reconciled. Under minting this service never sees a -captcha at all, so there is nothing to reconcile -- do not "unify" them. -""" - -import os -from dataclasses import dataclass -from pathlib import Path - -import jwt - -ALGORITHMS = ["EdDSA", "RS256"] -"""What the website may sign with. Both are asymmetric; no HS* symmetric option is -offered, because accepting one would let a leaked verifying key mint tokens.""" - -KEY_PATH_ENV = "HUMAN_TOKEN_PUBLIC_KEY_PATH" - - -@dataclass(frozen=True) -class TokenRejectedError(Exception): - """Why a token was refused. The reason is logged, never returned to the caller.""" - - reason: str - - -def load_verifying_key(path: str | None = None) -> str: - """The public key, or refuse to start. - - Principle IV. An endpoint that accepts everything because its key is missing is - the worst outcome available, and it would test clean -- every request would - succeed. - """ - configured = path or os.getenv(KEY_PATH_ENV) - if not configured: - raise RuntimeError( - f"{KEY_PATH_ENV} is not set. The answer endpoint verifies a signed " - "proof-of-human token and cannot run without a verifying key; starting " - "without one would accept every request." - ) - key_file = Path(configured) - try: - key = key_file.read_text().strip() - except OSError as exc: - raise RuntimeError( - f"Cannot read the verifying key at {key_file}: {exc}" - ) from exc - if not key: - raise RuntimeError(f"The verifying key at {key_file} is empty.") - return key - - -def verify(token: str, verifying_key: str, *, audience: str | None = None) -> dict: - """Return the token's claims, or raise TokenRejectedError. - - Every failure path refuses. There is deliberately no branch that returns claims - for an unverified token, however malformed. - """ - if not token: - raise TokenRejectedError("no token presented") - try: - return dict( - jwt.decode( - token, - verifying_key, - algorithms=ALGORITHMS, - audience=audience, - options={"require": ["exp"]}, - ) - ) - except jwt.ExpiredSignatureError as exc: - raise TokenRejectedError("token expired") from exc - except jwt.InvalidAudienceError as exc: - raise TokenRejectedError("token minted for a different audience") from exc - except jwt.InvalidSignatureError as exc: - raise TokenRejectedError("signature does not verify") from exc - except jwt.MissingRequiredClaimError as exc: - # A token with no expiry is a permanent credential, which is the thing the - # short lifetime exists to avoid. - raise TokenRejectedError("token has no expiry") from exc - except jwt.InvalidTokenError as exc: - raise TokenRejectedError(f"invalid token: {type(exc).__name__}") from exc diff --git a/src/util/secrets.py b/src/util/secrets.py index 49d196b..dee7eca 100644 --- a/src/util/secrets.py +++ b/src/util/secrets.py @@ -129,11 +129,11 @@ def load_secrets_to_environ(names: Iterable[str]) -> None: # this tuple did not list them. # Secrets the app consumes as a FILE at /run/secrets/, not by loading the # value into the environment. The answer endpoint's verifying key is one: it is -# read through HUMAN_TOKEN_PUBLIC_KEY_PATH, and putting PEM text in an +# read through CALLER_TOKEN_PUBLIC_KEY_PATH, and putting PEM text in an # environment variable would buy nothing. Listed so the compose tripwire can tell # "read as a file" apart from "mounted and silently never read", which is the # drift it exists to catch. -FILE_SECRET_NAMES = ("HUMAN_TOKEN_PUBLIC_KEY",) +FILE_SECRET_NAMES = ("CALLER_TOKEN_PUBLIC_KEY",) SECRET_NAMES = ( "CHAINLIT_AUTH_SECRET", diff --git a/tests/api/test_answer_endpoint.py b/tests/api/test_answer_endpoint.py index cab9858..028cb97 100644 --- a/tests/api/test_answer_endpoint.py +++ b/tests/api/test_answer_endpoint.py @@ -25,6 +25,7 @@ from agent.graph import AnswerEvent from api.answer import router +from util.caller_token import DEFAULT_AUDIENCE from util.rate_limit import SlidingWindowLimiter PREFIX = "/chat/guest/api" @@ -96,14 +97,20 @@ def stub(monkeypatch: pytest.MonkeyPatch) -> _StubGraph: def _client(public_pem: str) -> TestClient: app = FastAPI() app.include_router(router, prefix=PREFIX) - app.state.human_token_key = public_pem + app.state.caller_token_key = public_pem return TestClient(app) -def _token(private_pem: str, seconds: int = 300) -> str: - return jwt.encode( - {"exp": int(time.time()) + seconds}, private_pem, algorithm="EdDSA" - ) +def _token(private_pem: str, seconds: int = 300, **claims: object) -> str: + """A token shaped like the website's: iss, aud, exp, and a per-visit sub.""" + payload: dict[str, object] = { + "iss": "reactome-website", + "aud": DEFAULT_AUDIENCE, + "exp": int(time.time()) + seconds, + "sub": "visit-1", + } + payload.update(claims) + return jwt.encode(payload, private_pem, algorithm="EdDSA") def _events(text: str) -> list[tuple[str, str]]: @@ -123,7 +130,7 @@ def test_a_verified_request_streams_tokens_and_citations( f"{PREFIX}/answer", json={ "question": "what does CDK5 phosphorylate?", - "human_token": _token(private_pem), + "caller_token": _token(private_pem), }, ) assert response.status_code == 200 @@ -139,7 +146,7 @@ def test_no_token_means_no_model_call(keys: tuple[str, str], stub: _StubGraph) - """Search pages are crawled; every crawled search reaching the model is a bill.""" _, public_pem = keys response = _client(public_pem).post( - f"{PREFIX}/answer", json={"question": "anything", "human_token": ""} + f"{PREFIX}/answer", json={"question": "anything", "caller_token": ""} ) assert response.status_code == 200 events = _events(response.text) @@ -154,7 +161,7 @@ def test_an_expired_token_makes_no_model_call( private_pem, public_pem = keys response = _client(public_pem).post( f"{PREFIX}/answer", - json={"question": "anything", "human_token": _token(private_pem, seconds=-10)}, + json={"question": "anything", "caller_token": _token(private_pem, seconds=-10)}, ) assert '"state": "refused"' in response.text assert stub.calls == 0 @@ -168,7 +175,7 @@ def test_a_failure_mid_stream_still_ends_with_done( monkeypatch.setattr("api.answer.get_graph", lambda: _ExplodingGraph()) response = _client(public_pem).post( f"{PREFIX}/answer", - json={"question": "boom", "human_token": _token(private_pem)}, + json={"question": "boom", "caller_token": _token(private_pem)}, ) assert response.status_code == 200 kinds = [kind for kind, _ in _events(response.text)] @@ -181,7 +188,7 @@ def test_an_empty_question_is_rejected_by_validation( ) -> None: private_pem, public_pem = keys response = _client(public_pem).post( - f"{PREFIX}/answer", json={"question": "", "human_token": _token(private_pem)} + f"{PREFIX}/answer", json={"question": "", "caller_token": _token(private_pem)} ) assert response.status_code == 422 assert stub.calls == 0 @@ -227,7 +234,7 @@ def test_separate_requests_do_not_share_a_thread( for index in range(requests): response = client.post( f"{PREFIX}/answer", - json={"question": f"question {index}", "human_token": _token(private)}, + json={"question": f"question {index}", "caller_token": _token(private)}, ) assert response.status_code == 200 @@ -250,13 +257,13 @@ def test_start_carries_the_release_and_done_carries_seconds( private, public = keys app = FastAPI() app.include_router(router, prefix=PREFIX) - app.state.human_token_key = public + app.state.caller_token_key = public app.state.release = 97 client = TestClient(app) response = client.post( f"{PREFIX}/answer", - json={"question": "what is CDK5", "human_token": _token(private)}, + json={"question": "what is CDK5", "caller_token": _token(private)}, ) events = dict(_events(response.text)) @@ -315,7 +322,7 @@ def test_a_hanging_upstream_still_ends_the_stream( started = time.monotonic() response = client.post( f"{PREFIX}/answer", - json={"question": "what is CDK5", "human_token": _token(private)}, + json={"question": "what is CDK5", "caller_token": _token(private)}, ) elapsed = time.monotonic() - started @@ -342,7 +349,7 @@ def test_a_caller_over_the_limit_is_refused_before_the_model( states = [] for _ in range(4): response = client.post( - f"{PREFIX}/answer", json={"question": "what is CDK5", "human_token": token} + f"{PREFIX}/answer", json={"question": "what is CDK5", "caller_token": token} ) assert response.status_code == 200 states.append(json.loads(dict(_events(response.text))["done"])["state"]) @@ -361,18 +368,31 @@ def test_the_limit_is_per_caller( ) client = _client(public) - first = _token(private) - second = _token(private, seconds=301) # A different token, so a different key. + # Two visits, not two tokens. The limiter keys on `sub` -- an opaque + # per-visit id from the website -- so a second token for the SAME visit is + # the same caller, which is the point of keying on it rather than on the + # token. An earlier version of this test used two tokens differing only in + # expiry and expected them to count separately; once `sub` arrived that + # became wrong. + first = _token(private, sub="visit-1") + second = _token(private, sub="visit-2") def ask(token: str) -> str: response = client.post( - f"{PREFIX}/answer", json={"question": "what is CDK5", "human_token": token} + f"{PREFIX}/answer", json={"question": "what is CDK5", "caller_token": token} ) return str(json.loads(dict(_events(response.text))["done"])["state"]) assert ask(first) == "answered" assert ask(first) == "refused" - assert ask(second) == "answered" + assert ( + ask(second) == "answered" + ), "a different visit was refused someone else's budget" + + # A newly minted token for the first visit must not buy a fresh allowance -- + # they mint one per request, so keying on the token would make the limit + # meaningless. + assert ask(_token(private, sub="visit-1", seconds=301)) == "refused" def test_the_search_page_does_not_pay_for_a_web_search( @@ -400,7 +420,7 @@ async def astream_answer( client = _client(public) client.post( f"{PREFIX}/answer", - json={"question": "what is CDK5", "human_token": _token(private)}, + json={"question": "what is CDK5", "caller_token": _token(private)}, ) assert seen["enable_postprocess"] is False diff --git a/tests/util/test_human_token.py b/tests/util/test_caller_token.py similarity index 69% rename from tests/util/test_human_token.py rename to tests/util/test_caller_token.py index 2f697be..3fec656 100644 --- a/tests/util/test_human_token.py +++ b/tests/util/test_caller_token.py @@ -19,10 +19,13 @@ from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey -from util.human_token import ( +from util.caller_token import ( ALGORITHMS, + AUDIENCE_ENV, + DEFAULT_AUDIENCE, KEY_PATH_ENV, TokenRejectedError, + expected_audience, load_verifying_key, verify, ) @@ -48,8 +51,13 @@ def keys() -> tuple[str, str]: def _mint(private_pem: str, **claims: object) -> str: - payload: dict[str, object] = {"exp": int(time.time()) + 300} + """The shape the website mints: aud included unless a test overrides it.""" + payload: dict[str, object] = { + "exp": int(time.time()) + 300, + "aud": DEFAULT_AUDIENCE, + } payload.update(claims) + payload = {k: v for k, v in payload.items() if v is not None} return jwt.encode(payload, private_pem, algorithm="EdDSA") @@ -61,7 +69,7 @@ def test_a_validly_signed_token_is_accepted(keys: tuple[str, str]) -> None: def test_an_expired_token_is_refused(keys: tuple[str, str]) -> None: private_pem, public_pem = keys - token = jwt.encode({"exp": int(time.time()) - 1}, private_pem, algorithm="EdDSA") + token = _mint(private_pem, exp=int(time.time()) - 1) with pytest.raises(TokenRejectedError, match="expired"): verify(token, public_pem) @@ -69,11 +77,50 @@ def test_an_expired_token_is_refused(keys: tuple[str, str]) -> None: def test_a_token_with_no_expiry_is_refused(keys: tuple[str, str]) -> None: """A token without one is a permanent credential.""" private_pem, public_pem = keys - token = jwt.encode({"sub": "forever"}, private_pem, algorithm="EdDSA") - with pytest.raises(TokenRejectedError, match="no expiry"): + token = _mint(private_pem, sub="forever", exp=None) + with pytest.raises(TokenRejectedError, match="missing a required claim: exp"): verify(token, public_pem) +def test_a_token_for_another_audience_is_refused(keys: tuple[str, str]) -> None: + """Enforcing `aud` is what stops a token minted for this service being + replayed at a different consumer, and one minted elsewhere being presented + here. The website asked for it explicitly.""" + private_pem, public_pem = keys + token = _mint(private_pem, aud="some-other-service") + with pytest.raises(TokenRejectedError, match="different audience"): + verify(token, public_pem) + + +def test_a_token_with_no_audience_is_refused(keys: tuple[str, str]) -> None: + """Refused by name, rather than being waved through as "nothing to check".""" + private_pem, public_pem = keys + token = _mint(private_pem, aud=None) + with pytest.raises(TokenRejectedError, match="missing a required claim: aud"): + verify(token, public_pem) + + +def test_the_expected_audience_is_configurable_but_never_empty( + monkeypatch: pytest.MonkeyPatch, keys: tuple[str, str] +) -> None: + """An empty setting must not mean "accept anything". + + PyJWT rejects a token carrying `aud` when none is expected, so a blank value + would refuse every real token rather than loosening the check -- a + misconfiguration that would look like a signing problem. It falls back to the + agreed audience instead. + """ + private_pem, public_pem = keys + monkeypatch.setenv(AUDIENCE_ENV, " ") + assert expected_audience() == DEFAULT_AUDIENCE + verify(_mint(private_pem), public_pem) # still accepted + + monkeypatch.setenv(AUDIENCE_ENV, "someone-else") + assert expected_audience() == "someone-else" + with pytest.raises(TokenRejectedError, match="different audience"): + verify(_mint(private_pem), public_pem) + + def test_a_token_signed_by_someone_else_is_refused(keys: tuple[str, str]) -> None: _, public_pem = keys other = (