Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ on:
pull_request:
branches: [main]

# Read-only by default; no job here writes to the repository.
permissions:
contents: read

# A push to a PR branch matches both triggers, so every job used to run twice;
# superseded runs are now cancelled instead of finishing pointlessly.
concurrency:
group: "${{ github.workflow }}-${{ github.ref }}"
cancel-in-progress: true

jobs:
# The runner entrypoint executes untrusted PR content and was the only code
# in the repo with no linter over it. A shell injection lived there
Expand All @@ -24,6 +34,8 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: astral-sh/setup-uv@v4
with:
enable-cache: true
- run: uv sync --frozen
- run: uv run ruff check src/ tests/
- run: uv run ruff format --check src/ tests/
Expand Down Expand Up @@ -51,13 +63,21 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: astral-sh/setup-uv@v4
with:
enable-cache: true
- run: uv sync --frozen
# Tests build the schema with metadata create_all, so a broken revision
# chain would otherwise only surface as a crash loop on deploy --
# production runs `alembic upgrade head` in the container CMD.
- run: uv run alembic upgrade head
env:
DATABASE_URL: postgresql+asyncpg://helprs:helprs@localhost:5432/helprs_test
- run: uv run pytest --cov=helprs --cov-report=term-missing --cov-fail-under=70
env:
# Test-only values. The ephemeral CI database is destroyed after each run
# and nothing it encrypts ever leaves the job. Do not reuse anywhere else.
DATABASE_URL: postgresql+asyncpg://helprs:helprs@localhost:5432/helprs_test
SECRET_KEY: ci-test-secret-key
SECRET_KEY: ci-test-secret-key-at-least-32-bytes
FERNET_KEY: "-fB7lL74GHGbXnRClRQTaBP9_flqSUHFC9_c2n3Tvbo="
GITHUB_APP_ID: "000000"
ENVIRONMENT: test
Expand Down
2 changes: 1 addition & 1 deletion apps/api/.python-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.12
3.13
25 changes: 4 additions & 21 deletions apps/api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "helprs"
version = "0.1.0"
description = "helPRs API — Socratic comprehension sessions for pull requests"
readme = "README.md"
requires-python = ">=3.12"
requires-python = ">=3.13"

dependencies = [
"fastapi[standard]>=0.115.0",
Expand All @@ -12,7 +12,7 @@ dependencies = [
"pydantic-settings>=2.7.0",
"asyncpg>=0.30.0",
"sqladmin>=0.20.0",
"python-jose[cryptography]>=3.3.0",
"pyjwt[crypto]>=2.10.0",
"cryptography>=44.0.0",
"httpx>=0.28.0",
"structlog>=24.4.0",
Expand Down Expand Up @@ -44,7 +44,7 @@ build-backend = "hatchling.build"
packages = ["src/helprs"]

[tool.ruff]
target-version = "py312"
target-version = "py313"
line-length = 120

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

[tool.mypy]
python_version = "3.12"
python_version = "3.13"
warn_unused_configs = true
ignore_missing_imports = true
plugins = ["pydantic.mypy"]
Expand All @@ -68,23 +68,6 @@ plugins = ["pydantic.mypy"]
init_forbid_extra = true
init_typed = true

# Per-module overrides for third-party lib typing issues (aiodocker, SQLAlchemy, Starlette)
[[tool.mypy.overrides]]
module = ["helprs.main"]
disable_error_code = ["arg-type", "call-overload", "attr-defined"]

[[tool.mypy.overrides]]
module = ["helprs.core.middleware"]
disable_error_code = ["arg-type"]

[[tool.mypy.overrides]]
module = ["helprs.modules.webhook.repository", "helprs.modules.webhook.verification"]
disable_error_code = ["attr-defined", "arg-type"]

[[tool.mypy.overrides]]
module = ["helprs.modules.installation.service"]
disable_error_code = ["arg-type"]

[tool.pytest.ini_options]
asyncio_mode = "auto"
testpaths = ["tests"]
2 changes: 1 addition & 1 deletion apps/api/src/helprs/core/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def create_session_factory(engine) -> async_sessionmaker[AsyncSession]:

async def get_db_session(
session_factory: async_sessionmaker[AsyncSession],
) -> AsyncGenerator[AsyncSession, None]:
) -> AsyncGenerator[AsyncSession]:
"""Async context manager dependency for database sessions."""
async with session_factory() as session:
try:
Expand Down
6 changes: 3 additions & 3 deletions apps/api/src/helprs/core/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import Annotated

from fastapi import Depends, Request
from jose import JWTError
from jwt import PyJWTError
from sqlalchemy.ext.asyncio import AsyncSession

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


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

if payload.get("type") == "refresh":
Expand Down
11 changes: 9 additions & 2 deletions apps/api/src/helprs/core/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,15 @@ def __init__(self, message: str = "External service error", detail: Any = None):
super().__init__("external_service_error", message, 502, detail)


async def domain_exception_handler(request: Request, exc: DomainError) -> JSONResponse:
"""Global exception handler for DomainError subclasses."""
async def domain_exception_handler(request: Request, exc: Exception) -> JSONResponse:
"""Global exception handler for DomainError subclasses.

Typed as ``Exception`` because that is the signature Starlette's
``add_exception_handler`` declares; it dispatches on the registered class,
so the narrowing below never actually fails.
"""
if not isinstance(exc, DomainError):
raise exc
return JSONResponse(
status_code=exc.http_status,
content={"error": exc.error_code, "message": exc.message, "detail": exc.detail},
Expand Down
14 changes: 13 additions & 1 deletion apps/api/src/helprs/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ async def dispatch(self, request: Request, call_next) -> Response:
return response


def _handle_rate_limit_exceeded(request: Request, exc: Exception) -> Response:
"""Adapter around slowapi's handler.

slowapi declares its handler as taking ``RateLimitExceeded``, while
Starlette types every handler as taking ``Exception``. Dispatch is by
registered class, so the narrowing never fails in practice.
"""
if not isinstance(exc, RateLimitExceeded):
raise exc
return _rate_limit_exceeded_handler(request, exc)


def setup_middleware(app: FastAPI, settings: Settings) -> None:
"""Register all middleware on the app.

Expand All @@ -107,7 +119,7 @@ def setup_middleware(app: FastAPI, settings: Settings) -> None:
"""
# Innermost: rate limiting.
app.state.limiter = limiter
app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
app.add_exception_handler(RateLimitExceeded, _handle_rate_limit_exceeded)
app.add_middleware(SlowAPIMiddleware)

# Then request logging, so rate-limited requests get logged too.
Expand Down
9 changes: 5 additions & 4 deletions apps/api/src/helprs/core/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import time
from datetime import UTC, datetime, timedelta

import jwt
from cryptography.fernet import Fernet
from jose import jwt


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


def verify_github_webhook_signature(payload: bytes, signature: str, secret: str) -> bool:
def verify_github_webhook_signature(payload: bytes, signature: str | None, secret: str) -> bool:
"""Verify GitHub webhook HMAC SHA-256 signature.

Args:
payload: Raw request body bytes.
signature: The X-Hub-Signature-256 header value (e.g., "sha256=...").
signature: The X-Hub-Signature-256 header value (e.g., "sha256=..."),
or None when the header is absent, which is a rejection.
secret: The webhook secret configured in GitHub.
"""
if not signature or not signature.startswith("sha256="):
Expand Down Expand Up @@ -69,5 +70,5 @@ def create_access_token(


def decode_access_token(token: str, secret_key: str) -> dict:
"""Decode and verify a JWT access token. Raises JWTError on failure."""
"""Decode and verify a JWT access token. Raises PyJWTError on failure."""
return jwt.decode(token, secret_key, algorithms=["HS256"])
9 changes: 7 additions & 2 deletions apps/api/src/helprs/modules/webhook/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
"""

from datetime import UTC, datetime, timedelta
from typing import cast
from uuid import UUID

import structlog
from sqlalchemy import and_, func, or_, select, update
from sqlalchemy import CursorResult, and_, func, or_, select, update
from sqlalchemy.exc import IntegrityError
from sqlalchemy.ext.asyncio import AsyncSession

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

Expand Down
4 changes: 3 additions & 1 deletion apps/api/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

# Set test environment variables BEFORE any app imports
os.environ.setdefault("DATABASE_URL", "postgresql+asyncpg://helprs:helprs@localhost:5432/helprs_test")
os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing")
# >= 32 bytes: PyJWT warns below the RFC 7518 minimum for HMAC-SHA256, and
# production already enforces the same floor via validate_production_secrets.
os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing-32b+")
os.environ.setdefault("GITHUB_APP_ID", "000000")
os.environ.setdefault("ENVIRONMENT", "test")
os.environ.setdefault("GITHUB_WEBHOOK_SECRET", "test-webhook-secret")
Expand Down
10 changes: 5 additions & 5 deletions apps/api/tests/core/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import pytest
from cryptography.fernet import Fernet, InvalidToken
from jose import JWTError
from jwt import PyJWTError

from helprs.core.security import (
create_access_token,
Expand All @@ -16,7 +16,7 @@
)

FERNET_KEY = Fernet.generate_key().decode()
SECRET_KEY = "test-jwt-secret"
SECRET_KEY = "test-jwt-secret-at-least-32-bytes"


# --- Fernet encrypt/decrypt ---
Expand Down Expand Up @@ -74,11 +74,11 @@ def test_jwt_expired_token_rejected():
from datetime import timedelta

token = create_access_token({"sub": "user-1"}, SECRET_KEY, expires_delta=timedelta(seconds=-1))
with pytest.raises(JWTError):
with pytest.raises(PyJWTError):
decode_access_token(token, SECRET_KEY)


def test_jwt_wrong_secret_rejected():
token = create_access_token({"sub": "user-1"}, SECRET_KEY)
with pytest.raises(JWTError):
decode_access_token(token, "wrong-secret")
with pytest.raises(PyJWTError):
decode_access_token(token, "a-different-secret-of-adequate-length")
Loading
Loading