From cc3c23256691ec7ea4a831f629ca8e607e03eccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Keilyn=20Rodr=C3=ADguez=20Perojo?= <73491750+keilynrp@users.noreply.github.com> Date: Wed, 30 Sep 2026 08:29:27 -0600 Subject: [PATCH] fix(security): sign JWTs with PyJWT, and drop python-jose and ecdsa (#422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit python-jose was the only reason ecdsa, rsa and pyasn1 were in the backend image, and ecdsa carried PYSEC-2026-1325 (CVE-2024-23342, the Minerva timing attack on ECDSA signing): no version fixes it, so it sat in the pip-audit baseline as a permanent exception, re-justified every quarter for a library UKIP never used. UKIP signs HS256 only, which PyJWT covers with no dependencies. - backend/auth.py, routers/auth_users.py, routers/ws.py: import PyJWT, and PyJWTError under the JWTError name the six call sites already catch, so every refusal (expired, wrong key, tampered, unsigned, another algorithm, garbage) lands where it did. The key-rotation loop is unchanged. - requirements.txt: PyJWT instead of python-jose[cryptography]. requirements.lock: PyJWT==2.15.1 in, python-jose, ecdsa, rsa and pyasn1 out. Verified the way the lock's header requires: built Dockerfile.backend and compared its pip freeze. Nothing is installed that the lock does not pin, PyJWT is in, the four are out, and the only change to the lock-only set is those four leaving it. - security.yml and SECURITY_GATES.md §7e: PYSEC-2026-1325 removed rather than ignored; pip-audit reports "No known vulnerabilities found, 6 ignored" without it. Nobody is signed out: test_jwt_library.py pins an access token and a refresh token (the latter signed with a retiring key) minted by python-jose 3.5.0 with production's claims, and both decode under PyJWT. It also checks every refusal raises the type the call sites catch, that only HS256 is accepted, and that nothing in backend/ imports jose. The JWT, SSO and session tests (55) pass with python-jose uninstalled. Closes #422. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/security.yml | 3 +- README.md | 8 +- backend/auth.py | 5 +- backend/routers/auth_users.py | 2 +- backend/routers/ws.py | 2 +- backend/tests/test_auth.py | 2 +- backend/tests/test_epic017_jwt.py | 3 +- backend/tests/test_ir_session_revocation.py | 2 +- backend/tests/test_jwt_library.py | 118 ++++++++++++++++++++ docs/generated/repo_metrics.json | 4 +- docs/operating/SECURITY_GATES.md | 10 +- requirements.lock | 5 +- requirements.txt | 4 +- 13 files changed, 146 insertions(+), 22 deletions(-) create mode 100644 backend/tests/test_jwt_library.py diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 5090856f..275c2be4 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -48,7 +48,7 @@ jobs: # --no-deps: requirements.lock is a flat freeze (no hashes); pip # resolver is disabled via --disable-pip so no network lookup needed. # - # Six entries, down from 33 at gate introduction. The old list + # Five entries, down from 33 at gate introduction. The old list # deferred most of its entries to "the upgrade sprint"; #202 was # that sprint, and the resulting versions reached or passed every # fix version the baseline recorded, so 31 flags were suppressing @@ -71,7 +71,6 @@ jobs: run: >- pip-audit -r requirements.lock --disable-pip --no-deps --ignore-vuln PYSEC-2026-311 - --ignore-vuln PYSEC-2026-1325 --ignore-vuln PYSEC-2026-3813 --ignore-vuln PYSEC-2026-3814 --ignore-vuln PYSEC-2026-3815 diff --git a/README.md b/README.md index 185d13b6..0223052b 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ ![Rust](https://img.shields.io/badge/Rust-gRPC_Engine-000000?logo=rust&logoColor=white) ![Docker](https://img.shields.io/badge/Docker-Compose-2496ED?logo=docker&logoColor=white) -![Tests](https://img.shields.io/badge/Tests-4408_collected-28A745?logo=pytest&logoColor=white) +![Tests](https://img.shields.io/badge/Tests-4418_collected-28A745?logo=pytest&logoColor=white) ![API Operations](https://img.shields.io/badge/API_Operations-427-blue) @@ -79,7 +79,7 @@ backend/ FastAPI API server domains/ 3 configurable schemas: default, science, healthcare scripts/ maintenance + backfills (nif_bayes, work_type, retrospective) + job worker/scheduler entrypoints - tests/ 333 test files, 4408 tests + tests/ 334 test files, 4418 tests frontend/ Next.js 16 App Router app/ 65 pages, 90+ components, 8 context providers @@ -147,7 +147,7 @@ UKIP manages major product and implementation decisions as architecture decision | Analytics | pandas, DuckDB, PyArrow, NumPy, SciPy | | Background Jobs | Durable PostgreSQL lease queue (broker-free, at-least-once) with worker/scheduler processes | -| Testing | pytest (4408 tests collected), Vitest (312 tests), Playwright | +| Testing | pytest (4418 tests collected), Vitest (312 tests), Playwright | | Deployment | Docker Compose, GHCR images, Dokploy-oriented production compose | | Monitoring | Sentry (opt-in), structured logging | @@ -284,7 +284,7 @@ cd frontend && npx tsc --noEmit ``` -**Current test stats:** 4408 backend tests collected across 333 test files. Frontend: 312 Vitest tests. +**Current test stats:** 4418 backend tests collected across 334 test files. Frontend: 312 Vitest tests. --- diff --git a/backend/auth.py b/backend/auth.py index 842c2084..6ef0b128 100644 --- a/backend/auth.py +++ b/backend/auth.py @@ -17,9 +17,10 @@ from datetime import datetime, timedelta, timezone import bcrypt as _bcrypt +import jwt from fastapi import Depends, HTTPException, Request, status from fastapi.security import OAuth2PasswordBearer -from jose import JWTError, jwt +from jwt import PyJWTError as JWTError from sqlalchemy.orm import Session from backend import models @@ -57,7 +58,7 @@ def _parse_secret_keys(raw): # raw: str | None def _decode_token(token: str) -> dict: """Decode a JWT, verifying against the primary key then each retiring key. - Raises jose.JWTError if no configured key validates the token. This is the + Raises jwt.PyJWTError if no configured key validates the token. This is the single decode path for ALL JWT verification sites. """ keys = [SECRET_KEY, *RETIRING_SECRET_KEYS] diff --git a/backend/routers/auth_users.py b/backend/routers/auth_users.py index 4fa76c21..8b9fedd1 100644 --- a/backend/routers/auth_users.py +++ b/backend/routers/auth_users.py @@ -18,7 +18,7 @@ from authlib.integrations.starlette_client import OAuth from fastapi import APIRouter, Depends, HTTPException, Path, Query, Request from fastapi.security import OAuth2PasswordRequestForm -from jose import JWTError +from jwt import PyJWTError as JWTError from pydantic import BaseModel, Field from sqlalchemy import func from sqlalchemy.orm import Session diff --git a/backend/routers/ws.py b/backend/routers/ws.py index 6a9b872e..544c37b2 100644 --- a/backend/routers/ws.py +++ b/backend/routers/ws.py @@ -63,7 +63,7 @@ def _resolve_user(token: str, db: Session) -> models.User | None: Validate a JWT or ukip_ API key against *db*. Returns the active User on success, None otherwise. """ - from jose import JWTError + from jwt import PyJWTError as JWTError from backend.auth import _decode_token diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index 93b79304..f1e8a0f0 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -4,8 +4,8 @@ import os from datetime import timedelta +import jwt import pytest -from jose import jwt # Ensure env vars are set before import (conftest.py handles this, # but be explicit for isolated runs) diff --git a/backend/tests/test_epic017_jwt.py b/backend/tests/test_epic017_jwt.py index e33b12e7..a3bba238 100644 --- a/backend/tests/test_epic017_jwt.py +++ b/backend/tests/test_epic017_jwt.py @@ -1,8 +1,9 @@ # backend/tests/test_epic017_jwt.py import importlib +import jwt import pytest -from jose import JWTError, jwt +from jwt import PyJWTError as JWTError pytestmark = pytest.mark.security diff --git a/backend/tests/test_ir_session_revocation.py b/backend/tests/test_ir_session_revocation.py index dc348686..9972e1d1 100644 --- a/backend/tests/test_ir_session_revocation.py +++ b/backend/tests/test_ir_session_revocation.py @@ -11,8 +11,8 @@ """ import os +import jwt import pytest -from jose import jwt from starlette.websockets import WebSocketDisconnect from backend import models diff --git a/backend/tests/test_jwt_library.py b/backend/tests/test_jwt_library.py new file mode 100644 index 00000000..18cffe48 --- /dev/null +++ b/backend/tests/test_jwt_library.py @@ -0,0 +1,118 @@ +"""JWTs are signed and verified with PyJWT, not python-jose (#422). + +python-jose was the only reason ecdsa, rsa and pyasn1 were in the image, and +ecdsa carried an advisory with no fix in any version (PYSEC-2026-1325). UKIP +signs with HS256 only, which PyJWT covers with no dependencies. + +The switch must not sign anyone out: tokens minted by python-jose before the +deploy have to keep working. The two below were minted by python-jose 3.5.0 on +2026-09-30 with the keys in this file, the claims create_access_token and +create_refresh_token produce, and an expiry in 2099. +""" +from __future__ import annotations + +import base64 +import json +import pathlib +from datetime import timedelta + +import jwt +import pytest + +from backend import auth + +PRIMARY = "compat-primary-key-for-tests-only-0123456789abcdef" +RETIRING = "compat-retiring-key-for-tests-only-0123456789abcdef" + +JOSE_ACCESS = ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + "eyJzdWIiOiJjb21wYXRfdXNlciIsInJvbGUiOiJlZGl0b3IiLCJzaWQiOiJjb21wYXQtc2lkLTAwMDEiLCJleHAiOjQwNzA5MDg4MDAsInR5cGUiOiJhY2Nlc3MifQ." + "f0T1N3da8Pa1_RPl7PBFljIG2-fLPZ1Ffg-5TrTdPc8" +) +JOSE_REFRESH_WITH_RETIRING_KEY = ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + "eyJzdWIiOiJjb21wYXRfdXNlciIsInJvbGUiOiJlZGl0b3IiLCJzaWQiOiJjb21wYXQtc2lkLTAwMDEiLCJleHAiOjQwNzA5MDg4MDAsInR5cGUiOiJyZWZyZXNoIn0." + "_oSvbLWM_GL-e8YQrXxRhrshbJegZnzCRwWvmsGIQVs" +) + + +@pytest.fixture() +def keys(monkeypatch): + monkeypatch.setattr(auth, "SECRET_KEY", PRIMARY) + monkeypatch.setattr(auth, "RETIRING_SECRET_KEYS", [RETIRING]) + + +def _b64(part: dict) -> str: + return base64.urlsafe_b64encode(json.dumps(part).encode()).rstrip(b"=").decode() + + +class TestTokensMintedBeforeTheSwitch: + def test_an_access_token_from_python_jose_still_decodes(self, keys): + claims = auth._decode_token(JOSE_ACCESS) + assert claims["sub"] == "compat_user" + assert claims["role"] == "editor" + assert claims["sid"] == "compat-sid-0001" + assert claims["type"] == "access" + + def test_a_refresh_token_signed_with_a_retiring_key_still_decodes(self, keys): + claims = auth._decode_token(JOSE_REFRESH_WITH_RETIRING_KEY) + assert claims["type"] == "refresh" + assert claims["sid"] == "compat-sid-0001" + + +class TestNewTokens: + def test_access_and_refresh_tokens_round_trip(self, keys): + access = auth.create_access_token("alice", "viewer", "sid-1") + refresh = auth.create_refresh_token("alice", "viewer", "sid-1") + assert auth._decode_token(access)["type"] == "access" + assert auth._decode_token(refresh)["type"] == "refresh" + assert jwt.get_unverified_header(access)["alg"] == "HS256" + + +class TestRefusals: + """Every refusal raises jwt.PyJWTError, the type every call site catches.""" + + def test_an_expired_token(self, keys): + token = auth.create_access_token("alice", "viewer", "sid-1", timedelta(seconds=-5)) + with pytest.raises(jwt.PyJWTError): + auth._decode_token(token) + + def test_a_token_signed_with_an_unknown_key(self, keys): + token = jwt.encode({"sub": "mallory", "exp": 4070908800}, "some-other-key-0123456789abcdef-long", algorithm="HS256") + with pytest.raises(jwt.PyJWTError): + auth._decode_token(token) + + def test_a_tampered_payload(self, keys): + header, _, signature = JOSE_ACCESS.split(".") + forged = _b64({"sub": "compat_user", "role": "super_admin", "sid": "compat-sid-0001", "exp": 4070908800, "type": "access"}) + with pytest.raises(jwt.PyJWTError): + auth._decode_token(f"{header}.{forged}.{signature}") + + def test_an_unsigned_token(self, keys): + # alg "none": no signature at all. + token = f"{_b64({'alg': 'none', 'typ': 'JWT'})}.{_b64({'sub': 'mallory', 'exp': 4070908800})}." + with pytest.raises(jwt.PyJWTError): + auth._decode_token(token) + + def test_another_algorithm_with_the_real_key(self, keys): + # Only HS256 is accepted, even when the signature is otherwise valid. + token = jwt.encode({"sub": "alice", "exp": 4070908800}, PRIMARY, algorithm="HS512") + with pytest.raises(jwt.PyJWTError): + auth._decode_token(token) + + def test_garbage(self, keys): + with pytest.raises(jwt.PyJWTError): + auth._decode_token("not.a.jwt") + + +def test_nothing_in_the_backend_imports_python_jose(): + backend = pathlib.Path(__file__).resolve().parents[1] + offenders = [ + str(path.relative_to(backend)) + for path in backend.rglob("*.py") + if any( + line.strip().startswith(("from jose", "import jose")) + for line in path.read_text(encoding="utf-8").splitlines() + ) + ] + assert offenders == [] diff --git a/docs/generated/repo_metrics.json b/docs/generated/repo_metrics.json index db7e541c..dfc35854 100644 --- a/docs/generated/repo_metrics.json +++ b/docs/generated/repo_metrics.json @@ -1,8 +1,8 @@ { "schema_version": 1, "metrics": { - "backend_tests_collected": 4408, - "backend_test_files": 333, + "backend_tests_collected": 4418, + "backend_test_files": 334, "frontend_vitest_tests": 312, "api_operations": 427, "python_runtime_shipped": "3.13", diff --git a/docs/operating/SECURITY_GATES.md b/docs/operating/SECURITY_GATES.md index ab6b9095..f6533690 100644 --- a/docs/operating/SECURITY_GATES.md +++ b/docs/operating/SECURITY_GATES.md @@ -312,7 +312,7 @@ per-advisory disposition. ### 7e. pip-audit baseline (`--ignore-vuln` flags in `.github/workflows/security.yml`) -**6 vulnerability IDs ignored** (down from 33 at gate introduction). Owner: +**5 vulnerability IDs ignored** (down from 33 at gate introduction). Owner: platform owner. SLA: reviewed quarterly, or sooner when an entry's review date comes first (last review 2026-09-30; next by 2026-12-31). @@ -344,12 +344,18 @@ re-verified by running the tool rather than assumed. | ID | Package (pinned) | Fix version if known | Review date | | --- | --- | --- | --- | | PYSEC-2026-311 | chromadb==1.5.2 | none published (1.5.9 still affected) | 2026-12-31 | -| PYSEC-2026-1325 | ecdsa==0.19.2 | none, by upstream policy | 2026-12-31 | | PYSEC-2026-3813 (CVE-2026-45830) | chromadb==1.5.2 | none published (1.5.9 still affected) | 2026-12-31 | | PYSEC-2026-3815 (CVE-2026-45831) | chromadb==1.5.2 | none published (1.5.9 still affected) | 2026-12-31 | | PYSEC-2026-3814 (CVE-2026-45833) | chromadb==1.5.2 | none published (1.5.9 still affected) | 2026-12-31 | | CVE-2026-49265 | oauthlib==3.3.1 | 4.0.0 (major) | 2026-10-31 | +**2026-09-30: PYSEC-2026-1325 (ecdsa) removed, not ignored.** `python-jose` +was replaced by PyJWT (#422), and `ecdsa`, `rsa` and `pyasn1`, which only +`python-jose` required, left `requirements.lock` and the image. With the +flag gone, pip-audit reports `No known vulnerabilities found, 6 ignored` +(the four chromadb and one oauthlib IDs, two of them matched twice). The +review note below is kept as the record of why the entry existed. + **2026-09-30 review (the baseline's due date).** Every entry re-checked with pip-audit 2.10.1, OSV.dev and PyPI: diff --git a/requirements.lock b/requirements.lock index 69632eed..2a9f3bba 100644 --- a/requirements.lock +++ b/requirements.lock @@ -37,7 +37,6 @@ distro==1.9.0 docutils==0.22.4 duckdb==1.5.5 durationpy==0.10 -ecdsa==0.19.2 et_xmlfile==2.0.0 Faker==40.8.0 fakeredis==2.38.0 @@ -101,13 +100,13 @@ posthog==5.4.0 protobuf==6.33.6 psycopg2-binary==2.9.12 pyarrow==25.0.0 -pyasn1==0.6.4 pybase64==1.4.3 pycparser==3.0 pydantic==2.13.4 pydantic_core==2.46.4 pydyf==0.12.1 Pygments==2.20.0 +PyJWT==2.15.1 PyMySQL==1.2.0 pyparsing==3.3.2 pyphen==0.17.2 @@ -117,7 +116,6 @@ pytest-asyncio==1.4.0 pytest==9.1.1 python-dateutil==2.9.0.post0 python-dotenv==1.2.2 -python-jose==3.5.0 python-Levenshtein==0.27.3 python-louvain==0.16 python-multipart==0.0.32 @@ -132,7 +130,6 @@ requests==2.34.2 rich==14.3.3 roman-numerals==4.1.0 rpds-py==0.30.0 -rsa==4.9.1 scipy==1.18.0 sentry-sdk==2.66.1 setuptools==83.0.0 diff --git a/requirements.txt b/requirements.txt index 6245c62f..869c3883 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,7 +23,9 @@ requests numpy scipy # ── Sprint 1: Security ────────────────────────────────────── -python-jose[cryptography] +# PyJWT, not python-jose: UKIP signs HS256 only, and python-jose pulled in +# ecdsa, rsa and pyasn1, one with an advisory no version fixes (#422). +PyJWT bcrypt cryptography # ── Sprint 6: Rate limiting ──────────────────────────────────