Skip to content

Commit ff52364

Browse files
authored
chore(deps): refresh the lockfile, drop python-jose, move to Python 3.13 (#67)
pip-audit reported 12 vulnerable packages in the lockfile, which was about six months stale. The pyproject floors already allowed every fix, so this is `uv lock --upgrade` plus the one dependency that needed replacing. pip-audit now reports no known vulnerabilities. python-jose has been unmaintained since 2021 and FastAPI's own security tutorial moved to PyJWT. It also dragged in ecdsa, whose PYSEC-2026-1325 has no fix available at all -- the only advisory the upgrade could not close. Swapping it out removes ecdsa, pyasn1, rsa and six along with it. encode/decode are drop-in; JWTError becomes PyJWTError. Python 3.12 has been security-only since April 2025. 3.13 is a drop-in: the whole locked dependency set installs with no source builds and the suite passes unchanged. Stopping at 3.13 because aiodocker declares support no further. The five per-module mypy overrides were hiding four errors between them, so they are gone and the four are fixed rather than suppressed: - verify_github_webhook_signature declared `signature: str` while the caller passes `str | None`; the function's own first line handles None, so the annotation was simply wrong - two exception handlers declared their narrow exception type, while Starlette types every handler as taking Exception. They take Exception and narrow, with a thin adapter for slowapi's handler - AsyncSession.execute is typed as returning Result, but an UPDATE returns CursorResult, which is where rowcount lives -- a cast, not a suppression Build and CI hygiene, from the same audit: - uv pinned instead of :latest, and `uv sync --frozen` so an image cannot resolve a different dependency set than the one CI tested - CI declares `permissions: contents: read` and a concurrency group; a push to a PR branch matched both triggers and ran every job twice - uv cache enabled; the frontend jobs already cached npm - `alembic upgrade head` runs in CI. Tests build the schema with create_all, so a broken revision chain could only surface as a crash loop on deploy - test and CI secrets lengthened past the RFC 7518 HMAC floor, which PyJWT warns about and production already enforces
1 parent ae2f8d2 commit ff52364

13 files changed

Lines changed: 1131 additions & 1268 deletions

File tree

.github/workflows/ci.yml

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ on:
66
pull_request:
77
branches: [main]
88

9+
# Read-only by default; no job here writes to the repository.
10+
permissions:
11+
contents: read
12+
13+
# A push to a PR branch matches both triggers, so every job used to run twice;
14+
# superseded runs are now cancelled instead of finishing pointlessly.
15+
concurrency:
16+
group: "${{ github.workflow }}-${{ github.ref }}"
17+
cancel-in-progress: true
18+
919
jobs:
1020
# The runner entrypoint executes untrusted PR content and was the only code
1121
# in the repo with no linter over it. A shell injection lived there
@@ -24,6 +34,8 @@ jobs:
2434
steps:
2535
- uses: actions/checkout@v4
2636
- uses: astral-sh/setup-uv@v4
37+
with:
38+
enable-cache: true
2739
- run: uv sync --frozen
2840
- run: uv run ruff check src/ tests/
2941
- run: uv run ruff format --check src/ tests/
@@ -51,13 +63,21 @@ jobs:
5163
steps:
5264
- uses: actions/checkout@v4
5365
- uses: astral-sh/setup-uv@v4
66+
with:
67+
enable-cache: true
5468
- run: uv sync --frozen
69+
# Tests build the schema with metadata create_all, so a broken revision
70+
# chain would otherwise only surface as a crash loop on deploy --
71+
# production runs `alembic upgrade head` in the container CMD.
72+
- run: uv run alembic upgrade head
73+
env:
74+
DATABASE_URL: postgresql+asyncpg://helprs:helprs@localhost:5432/helprs_test
5575
- run: uv run pytest --cov=helprs --cov-report=term-missing --cov-fail-under=70
5676
env:
5777
# Test-only values. The ephemeral CI database is destroyed after each run
5878
# and nothing it encrypts ever leaves the job. Do not reuse anywhere else.
5979
DATABASE_URL: postgresql+asyncpg://helprs:helprs@localhost:5432/helprs_test
60-
SECRET_KEY: ci-test-secret-key
80+
SECRET_KEY: ci-test-secret-key-at-least-32-bytes
6181
FERNET_KEY: "-fB7lL74GHGbXnRClRQTaBP9_flqSUHFC9_c2n3Tvbo="
6282
GITHUB_APP_ID: "000000"
6383
ENVIRONMENT: test

apps/api/.python-version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.12
1+
3.13

apps/api/pyproject.toml

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name = "helprs"
33
version = "0.1.0"
44
description = "helPRs API — Socratic comprehension sessions for pull requests"
55
readme = "README.md"
6-
requires-python = ">=3.12"
6+
requires-python = ">=3.13"
77

88
dependencies = [
99
"fastapi[standard]>=0.115.0",
@@ -12,7 +12,7 @@ dependencies = [
1212
"pydantic-settings>=2.7.0",
1313
"asyncpg>=0.30.0",
1414
"sqladmin>=0.20.0",
15-
"python-jose[cryptography]>=3.3.0",
15+
"pyjwt[crypto]>=2.10.0",
1616
"cryptography>=44.0.0",
1717
"httpx>=0.28.0",
1818
"structlog>=24.4.0",
@@ -44,7 +44,7 @@ build-backend = "hatchling.build"
4444
packages = ["src/helprs"]
4545

4646
[tool.ruff]
47-
target-version = "py312"
47+
target-version = "py313"
4848
line-length = 120
4949

5050
[tool.ruff.lint]
@@ -59,7 +59,7 @@ select = ["E", "F", "I", "N", "UP", "B", "A", "SIM", "TCH", "RUF100"]
5959
"alembic/env.py" = ["E402"]
6060

6161
[tool.mypy]
62-
python_version = "3.12"
62+
python_version = "3.13"
6363
warn_unused_configs = true
6464
ignore_missing_imports = true
6565
plugins = ["pydantic.mypy"]
@@ -68,23 +68,6 @@ plugins = ["pydantic.mypy"]
6868
init_forbid_extra = true
6969
init_typed = true
7070

71-
# Per-module overrides for third-party lib typing issues (aiodocker, SQLAlchemy, Starlette)
72-
[[tool.mypy.overrides]]
73-
module = ["helprs.main"]
74-
disable_error_code = ["arg-type", "call-overload", "attr-defined"]
75-
76-
[[tool.mypy.overrides]]
77-
module = ["helprs.core.middleware"]
78-
disable_error_code = ["arg-type"]
79-
80-
[[tool.mypy.overrides]]
81-
module = ["helprs.modules.webhook.repository", "helprs.modules.webhook.verification"]
82-
disable_error_code = ["attr-defined", "arg-type"]
83-
84-
[[tool.mypy.overrides]]
85-
module = ["helprs.modules.installation.service"]
86-
disable_error_code = ["arg-type"]
87-
8871
[tool.pytest.ini_options]
8972
asyncio_mode = "auto"
9073
testpaths = ["tests"]

apps/api/src/helprs/core/database.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def create_session_factory(engine) -> async_sessionmaker[AsyncSession]:
117117

118118
async def get_db_session(
119119
session_factory: async_sessionmaker[AsyncSession],
120-
) -> AsyncGenerator[AsyncSession, None]:
120+
) -> AsyncGenerator[AsyncSession]:
121121
"""Async context manager dependency for database sessions."""
122122
async with session_factory() as session:
123123
try:

apps/api/src/helprs/core/dependencies.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from typing import Annotated
66

77
from fastapi import Depends, Request
8-
from jose import JWTError
8+
from jwt import PyJWTError
99
from sqlalchemy.ext.asyncio import AsyncSession
1010

1111
from helprs.core.config import Settings, get_settings
@@ -18,7 +18,7 @@
1818
GetSettings = Annotated[Settings, Depends(get_settings)]
1919

2020

21-
async def get_db(request: Request) -> AsyncGenerator[AsyncSession, None]:
21+
async def get_db(request: Request) -> AsyncGenerator[AsyncSession]:
2222
"""Database session dependency wired to app.state.session_factory."""
2323
session_factory = request.app.state.session_factory
2424
async with session_factory() as session:
@@ -43,7 +43,7 @@ async def authenticate_token(session: AsyncSession, settings: Settings, token: s
4343
"""
4444
try:
4545
payload = decode_access_token(token, settings.SECRET_KEY.get_secret_value())
46-
except JWTError as e:
46+
except PyJWTError as e:
4747
raise UnauthorizedError("Invalid or expired token") from e
4848

4949
if payload.get("type") == "refresh":

apps/api/src/helprs/core/exceptions.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,15 @@ def __init__(self, message: str = "External service error", detail: Any = None):
6363
super().__init__("external_service_error", message, 502, detail)
6464

6565

66-
async def domain_exception_handler(request: Request, exc: DomainError) -> JSONResponse:
67-
"""Global exception handler for DomainError subclasses."""
66+
async def domain_exception_handler(request: Request, exc: Exception) -> JSONResponse:
67+
"""Global exception handler for DomainError subclasses.
68+
69+
Typed as ``Exception`` because that is the signature Starlette's
70+
``add_exception_handler`` declares; it dispatches on the registered class,
71+
so the narrowing below never actually fails.
72+
"""
73+
if not isinstance(exc, DomainError):
74+
raise exc
6875
return JSONResponse(
6976
status_code=exc.http_status,
7077
content={"error": exc.error_code, "message": exc.message, "detail": exc.detail},

apps/api/src/helprs/core/middleware.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,18 @@ async def dispatch(self, request: Request, call_next) -> Response:
8888
return response
8989

9090

91+
def _handle_rate_limit_exceeded(request: Request, exc: Exception) -> Response:
92+
"""Adapter around slowapi's handler.
93+
94+
slowapi declares its handler as taking ``RateLimitExceeded``, while
95+
Starlette types every handler as taking ``Exception``. Dispatch is by
96+
registered class, so the narrowing never fails in practice.
97+
"""
98+
if not isinstance(exc, RateLimitExceeded):
99+
raise exc
100+
return _rate_limit_exceeded_handler(request, exc)
101+
102+
91103
def setup_middleware(app: FastAPI, settings: Settings) -> None:
92104
"""Register all middleware on the app.
93105
@@ -107,7 +119,7 @@ def setup_middleware(app: FastAPI, settings: Settings) -> None:
107119
"""
108120
# Innermost: rate limiting.
109121
app.state.limiter = limiter
110-
app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
122+
app.add_exception_handler(RateLimitExceeded, _handle_rate_limit_exceeded)
111123
app.add_middleware(SlowAPIMiddleware)
112124

113125
# Then request logging, so rate-limited requests get logged too.

apps/api/src/helprs/core/security.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
import time
66
from datetime import UTC, datetime, timedelta
77

8+
import jwt
89
from cryptography.fernet import Fernet
9-
from jose import jwt
1010

1111

1212
def fernet_encrypt(plaintext: str, fernet_key: str) -> str:
@@ -36,12 +36,13 @@ def create_app_jwt(app_id: str, private_key: str) -> str:
3636
return jwt.encode(payload, private_key, algorithm="RS256")
3737

3838

39-
def verify_github_webhook_signature(payload: bytes, signature: str, secret: str) -> bool:
39+
def verify_github_webhook_signature(payload: bytes, signature: str | None, secret: str) -> bool:
4040
"""Verify GitHub webhook HMAC SHA-256 signature.
4141
4242
Args:
4343
payload: Raw request body bytes.
44-
signature: The X-Hub-Signature-256 header value (e.g., "sha256=...").
44+
signature: The X-Hub-Signature-256 header value (e.g., "sha256=..."),
45+
or None when the header is absent, which is a rejection.
4546
secret: The webhook secret configured in GitHub.
4647
"""
4748
if not signature or not signature.startswith("sha256="):
@@ -69,5 +70,5 @@ def create_access_token(
6970

7071

7172
def decode_access_token(token: str, secret_key: str) -> dict:
72-
"""Decode and verify a JWT access token. Raises JWTError on failure."""
73+
"""Decode and verify a JWT access token. Raises PyJWTError on failure."""
7374
return jwt.decode(token, secret_key, algorithms=["HS256"])

apps/api/src/helprs/modules/webhook/repository.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@
2222
"""
2323

2424
from datetime import UTC, datetime, timedelta
25+
from typing import cast
2526
from uuid import UUID
2627

2728
import structlog
28-
from sqlalchemy import and_, func, or_, select, update
29+
from sqlalchemy import CursorResult, and_, func, or_, select, update
2930
from sqlalchemy.exc import IntegrityError
3031
from sqlalchemy.ext.asyncio import AsyncSession
3132

@@ -146,7 +147,11 @@ async def mark_processing(
146147
.values(status="processing", updated_at=func.now())
147148
.execution_options(synchronize_session=False)
148149
)
149-
result = await session.execute(stmt)
150+
# AsyncSession.execute is typed as returning Result; an UPDATE returns a
151+
# CursorResult, which is where rowcount lives. The claim is decided by
152+
# whether this process won the conditional UPDATE, so rowcount is the
153+
# signal, not a convenience.
154+
result = cast("CursorResult", await session.execute(stmt))
150155
await session.commit()
151156
return (result.rowcount or 0) == 1
152157

apps/api/tests/conftest.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
# Set test environment variables BEFORE any app imports
44
os.environ.setdefault("DATABASE_URL", "postgresql+asyncpg://helprs:helprs@localhost:5432/helprs_test")
5-
os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing")
5+
# >= 32 bytes: PyJWT warns below the RFC 7518 minimum for HMAC-SHA256, and
6+
# production already enforces the same floor via validate_production_secrets.
7+
os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing-32b+")
68
os.environ.setdefault("GITHUB_APP_ID", "000000")
79
os.environ.setdefault("ENVIRONMENT", "test")
810
os.environ.setdefault("GITHUB_WEBHOOK_SECRET", "test-webhook-secret")

0 commit comments

Comments
 (0)