Build production-ready anonymous matchmaking backend (FastAPI, Redis, Postgres, Docker) - #43
Build production-ready anonymous matchmaking backend (FastAPI, Redis, Postgres, Docker)#43huzely wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR introduces a production-ready FastAPI backend for an anonymous matchmaking chat application, featuring JWT authentication, Redis-based matchmaking queues, trust/reporting system, ad delivery, premium features, WebSocket chat, admin dashboard APIs, and Docker containerization with database migrations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as FastAPI<br/>Match API
participant Redis as Redis<br/>Queues
participant DB as Database
participant WS as WebSocket<br/>Manager
Client->>API: POST /match/find<br/>(JWT token)
activate API
API->>DB: Check active ChatRoom
alt Existing Room
DB-->>API: Return room
API-->>Client: MatchResponse(room_id)
else No Active Room
API->>Redis: Check premium queue
API->>Redis: Check standard queue
alt Candidate Found
Redis-->>API: Pop candidate
API->>DB: Validate candidate<br/>(status, trust, gender, blocks)
alt Valid Match
DB-->>API: ✓ All checks pass
API->>DB: Create ChatRoom<br/>(set expiry)
DB-->>API: Return room
API-->>Client: MatchResponse(room_id)
else Invalid Candidate
API->>Redis: Try next candidate
end
else No Candidate
API->>Redis: Enqueue requester
API-->>Client: {"message": "queued"}
end
end
deactivate API
Client->>WS: WebSocket connect<br/>/ws/chat/{room_id}<br/>?token=JWT
activate WS
WS->>DB: Decode JWT & validate room/user
DB-->>WS: ✓ Room & user valid
WS->>WS: Register connection
WS-->>Client: Connection established
Client->>WS: Send message
activate WS
WS->>DB: Persist Message
DB-->>WS: ✓ Saved
WS->>WS: Broadcast to other participant
deactivate WS
Client->>API: POST /match/rooms/{id}/report
activate API
API->>DB: Record Report
API->>DB: Create BlockedPair
API->>DB: Apply trust penalty
DB-->>API: ✓ Saved
API-->>Client: {"message": "reported"}
deactivate API
Client->>API: POST /match/rooms/{id}/close
activate API
API->>DB: Mark room closed
alt User not Premium
API->>DB: Fetch next ad
API-->>Client: {close + ad payload}
else Premium User
API-->>Client: {close only}
end
deactivate API
deactivate WS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68b3e3f866
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for queue in [MatchmakingService.premium_queue_key, MatchmakingService.queue_key]: | ||
| size = await redis.llen(queue) | ||
| for _ in range(size): | ||
| raw_id = await redis.lpop(queue) |
There was a problem hiding this comment.
try_match pops each queued user with lpop before eligibility checks and never pushes skipped users back. That means anyone rejected for this requester (gender mismatch, trust threshold, blocked pair, etc.) is silently removed from matchmaking, so users can disappear from the queue and never get matched unless they manually enqueue again.
Useful? React with 👍 / 👎.
| await db.commit() | ||
| await db.refresh(room) | ||
| return room |
There was a problem hiding this comment.
When a match is found, the function returns immediately after creating the room without clearing the requester's existing queue entry. If a user calls /match/find while already queued and is matched against someone ahead of them, their stale queue item remains and can be matched again later, resulting in multiple active rooms for the same user.
Useful? React with 👍 / 👎.
| connectable = engine_from_config( | ||
| config.get_section(config.config_ini_section, {}), | ||
| prefix="sqlalchemy.", | ||
| poolclass=pool.NullPool, | ||
| ) |
There was a problem hiding this comment.
This Alembic environment still uses the synchronous engine_from_config path even though alembic.ini is configured with postgresql+asyncpg. With asyncpg URLs, alembic upgrade head requires the async migration pattern (async_engine_from_config with run_sync), so migrations cannot run successfully with the current setup.
Useful? React with 👍 / 👎.
| raise HTTPException(status_code=403, detail="Not your room") | ||
|
|
||
| target_id = room.user_b_id if user.id == room.user_a_id else room.user_a_id | ||
| db.add(Report(room_id=room_id, reporter_id=user.id, reported_id=target_id, reason=payload.reason)) |
There was a problem hiding this comment.
The report endpoint inserts a new report unconditionally and later applies trust penalties, but there is no check for an existing report by the same reporter against the same user in the same room. Repeating this call allows one client to stack penalties and push another account into cooldown/ban from a single chat session.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@alembic/env.py`:
- Around line 23-32: run_migrations_online currently builds a synchronous engine
with engine_from_config and uses connectable.connect(), which breaks with the
postgresql+asyncpg URL; replace the sync flow in run_migrations_online with the
async pattern: use async_engine_from_config(...) to create an AsyncEngine, open
it with "async with connectable.connect() as connection", and inside await
connection.run_sync(lambda sync_conn: context.configure(connection=sync_conn,
target_metadata=target_metadata)) followed by await connection.run_sync(lambda
_: context.run_migrations()) (or equivalent use of connection.run_sync for
context.begin_transaction()/run_migrations); also update imports to include
async_engine_from_config and change the existing noqa comment to "# noqa: F401"
to clarify intentional unused imports for model registration.
In `@alembic/versions/0001_initial.py`:
- Around line 105-115: The downgrade() in alembic/versions/0001_initial.py
currently drops tables and indexes but leaves the Postgres enum types (gender,
roomstatus, accountstatus) behind; update downgrade() to drop these enum types
after dropping dependent tables/indexes by executing DROP TYPE IF EXISTS for
each enum name (gender, roomstatus, accountstatus) or using the corresponding
sa.Enum(...).drop(...) pattern against the migration bind so a subsequent
upgrade can recreate them cleanly. Ensure you reference the enum names exactly
and perform the drops at the end of the downgrade() function.
In `@app/api/user.py`:
- Around line 37-47: The activate_premium endpoint currently flips user.premium
without verifying payment; change activate_premium to first check idempotency
(if user.premium is already True return 200/no-op), then call a payment
verification function (e.g., verify_payment(payment_reference) — implement or
inject a service) to validate payload.payment_reference and raise an HTTP error
if verification fails, and only on successful verification set user.premium =
True and persist inside the AsyncSession transaction (await db.commit()); also
create and insert an audit record (e.g., PremiumActivationLog with user id,
payment_reference, status, timestamp) or at minimum log the activation via the
logger, and if payment integration is not ready restrict the route to admins or
disable the endpoint until verify_payment is implemented.
In `@app/api/ws.py`:
- Around line 34-56: The WebSocket handler chat_socket currently holds an
AsyncSessionLocal for the entire connection and only catches
WebSocketDisconnect, which can leak manager state on other exceptions; change it
to ensure manager.disconnect(room_id, user_id) is always called by wrapping the
receive/send loop in try/finally (or try/except/finally) and move the DB work
into short-lived sessions per message (use async with AsyncSessionLocal() as db:
db.add(msg); await db.commit()) inside the loop; also catch JSON decoding errors
(e.g., ValueError/JSONDecodeError around websocket.receive_json()) so they don't
escape and leave stale connections, and ensure websocket is closed on fatal
errors as appropriate while still calling manager.disconnect in the finally
block.
In `@app/core/config.py`:
- Line 7: The config currently defines a hardcoded default secret_key
("change-me") in app/core/config.py which is insecure; remove the insecure
default so SECRET_KEY must be provided via environment or, if you keep a
dev-friendly default, add a startup validation in your application bootstrap
(e.g., in app/main.py or the FastAPI startup event) that checks settings.env and
settings.secret_key and raises an error if settings.env != "dev" and
settings.secret_key == "change-me" (or is unset), ensuring the app fails to
start in non-dev environments without a real secret.
In `@app/core/deps.py`:
- Around line 30-33: decode_token can return a non-numeric "sub" string which
will cause int(user_id) to raise ValueError and produce a 500; change the code
around decode_token / user_id to validate and safely parse the subject before
calling db.get(User, ...). Specifically, replace the direct int(user_id) call in
the auth flow (the user_id variable returned by decode_token) with a safe parse:
try to convert to int (or check isdigit() for your use case) and if conversion
fails raise an HTTPException (use status.HTTP_401_UNAUTHORIZED or the
appropriate auth status) so a tampered or non-numeric sub results in a 401
instead of an uncaught 500; ensure this validation occurs before calling
db.get(User, int(...)).
In `@app/services/matchmaking.py`:
- Around line 41-71: The current loop uses redis.lpop on
MatchmakingService.premium_queue_key and MatchmakingService.queue_key, which
removes every candidate even if they fail checks; fix by performing a
non-destructive scan: for each queue, read all IDs (e.g., LRANGE) or pop while
collecting skipped raw_ids, evaluate each candidate (using _gender_compatible,
_is_blocked, requester, db, trust checks) and only remove the matched candidate
from the queue; re-enqueue any skipped IDs back to the same queue in their
original order (or use LREM to remove only the matched candidate after selecting
it) so invalid candidates are not permanently lost.
In `@docker-compose.yml`:
- Around line 8-11: The docker-compose environment block currently hardcodes
SECRET_KEY ("super-secret-key"), which is insecure; update the compose to accept
the secret via an environment variable or Docker secret (e.g., reference an env
var like ${SECRET_KEY} or use a Docker secret named SECRET_KEY) and remove the
literal value, add documentation or a compose comment that the SECRET_KEY must
be overridden in production, and ensure any service startup (JWT code) reads the
same env var; look for the environment: SECRET_KEY entry in the compose file and
replace it with the env-var/secret reference and accompanying note.
In `@requirements.txt`:
- Line 7: The requirements pin uses python-jose[cryptography]==3.3.0 which has
critical vulnerabilities (CVE-2024-33663/CVE-2024-33664); update the dependency
in requirements.txt to python-jose[cryptography]>=3.4.0 to pull the patched
release, or if you prefer to switch libraries, replace python-jose with
PyJWT[crypto]==2.11.0 and update JWT usages: change imports and calls from jose
(used in app/core/security.py and app/api/ws.py) to jwt and adjust any
encode()/decode() call sites accordingly (APIs are compatible for signed JWTs),
then run tests and regenerate lock files.
🟠 Major comments (15)
Dockerfile-1-14 (1)
1-14:⚠️ Potential issue | 🟠 MajorAdd a non-root user to the container.
Running the container as root is a security risk. If the application is compromised, the attacker gains root privileges inside the container. Trivy correctly flags this (DS-0002).
🔒 Proposed fix
FROM python:3.11-slim WORKDIR /app ENV PYTHONDONTWRITEBYTECODE=1 ENV PYTHONUNBUFFERED=1 COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt COPY . . +RUN addgroup --system app && adduser --system --ingroup app app +USER app + EXPOSE 8000 CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"]requirements.txt-1-1 (1)
1-1:⚠️ Potential issue | 🟠 MajorUpgrade
fastapiandpython-multipartto patch known DoS and file-write vulnerabilities.All four vulnerabilities are confirmed:
python-multipart==0.0.17: CVE-2024-53981 (DoS via malformed boundary) and CVE-2026-24486 (arbitrary file write with non-defaultUPLOAD_KEEP_FILENAME=Trueconfig). Fixed in 0.0.22.starlette==0.41.3(transitive viafastapi==0.115.5): CVE-2025-54121 (DoS on large file uploads; fixed in 0.47.2) and CVE-2025-62727 (Range header CPU exhaustion; fixed in 0.49.1).Bump to latest patched versions:
fastapi>=0.129.0,python-multipart>=0.0.22, andstarlette>=0.49.1(automatically resolved by upgrading fastapi).app/websocket/manager.py-17-24 (1)
17-24:⚠️ Potential issue | 🟠 MajorUnhandled exceptions during broadcast will skip remaining recipients.
If any
ws.send_json(payload)call raises (e.g., the peer disconnected), the loop aborts and remaining users in the room never receive the message. Wrap individual sends in a try/except to isolate failures.🛡️ Proposed fix
async def send_to_other(self, room_id: int, sender_id: int, payload: dict): for uid, ws in self.room_connections.get(room_id, {}).items(): if uid != sender_id: - await ws.send_json(payload) + try: + await ws.send_json(payload) + except Exception: + pass # peer already disconnected async def send_to_room(self, room_id: int, payload: dict): for ws in self.room_connections.get(room_id, {}).values(): - await ws.send_json(payload) + try: + await ws.send_json(payload) + except Exception: + passapp/models/models.py-53-53 (1)
53-53:⚠️ Potential issue | 🟠 Major
datetime.utcnowproduces naive datetimes for timezone-aware columns and is deprecated since Python 3.12.All
created_atcolumns (and this pattern repeats on lines 63, 75, 86, 96, 107, 116, 126) usedefault=datetime.utcnow, which returns a naive datetime (no tzinfo). Storing naive datetimes inDateTime(timezone=True)columns means PostgreSQL will silently assume the server's local timezone, which may not be UTC.Use
datetime.now(timezone.utc)(via a lambda orfunctools.partial) instead:Proposed fix (apply to all `created_at` columns)
-from datetime import datetime, date +from datetime import datetime, date, timezone ... - created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=datetime.utcnow) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), default=lambda: datetime.now(timezone.utc) + )alembic.ini-3-3 (1)
3-3:⚠️ Potential issue | 🟠 MajorRemove hardcoded database credentials from alembic.ini and use environment variables instead.
alembic/env.pydoes not override thesqlalchemy.urlsetting fromalembic.ini—it reads directly from it (lines 17, 25). The hardcoded credentialspostgresql+asyncpg://postgres:postgres@db:5432/matchbotwill always be used for migrations, duplicating the value inapp/core/config.py. Whileconfig.pysupports loading from.envvia Pydantic, Alembic's configuration bypasses this entirely.Move credentials to environment variables and update both
alembic.iniandconfig.pyto read from${DATABASE_URL}or similar, ensuring the same source of truth for all database connections.app/services/ads.py-9-14 (1)
9-14:⚠️ Potential issue | 🟠 MajorMissing
.limit(1)— fetches all active ads into memory.
db.scalar(select(...))executes the full query and returns the first row in Python, but without.limit(1)the database will return every active ad row. As the ads table grows, this becomes an unnecessary full-table scan and memory hog.Proposed fix
- ad = await db.scalar(select(Ad).where(Ad.active.is_(True)).order_by(Ad.impressions.asc(), Ad.id.asc())) + ad = await db.scalar( + select(Ad) + .where(Ad.active.is_(True)) + .order_by(Ad.impressions.asc(), Ad.id.asc()) + .limit(1) + )app/services/trust.py-22-29 (1)
22-29:⚠️ Potential issue | 🟠 Major
db.commit()insidetry_regeneratebreaks atomicity when called fromrelease_user_restrictions.
release_user_restrictions(inapp/services/maintenance.py, Line 42) callstry_regenerateinside a loop that mutates multiple users, then issues its owndb.commit()at Line 43. Whentry_regeneratecommits mid-loop, it flushes all pending session changes — including partially processed users — destroying the all-or-nothing guarantee. If an error occurs after some iterations, you get a partial update with no way to roll back.Let the caller own the transaction boundary. Remove the commit from
try_regenerateand letapply_report_penaltyremain the only committing method (or push commits to callers there too for consistency).Proposed fix
`@staticmethod` async def try_regenerate(user: User, db: AsyncSession) -> None: if not user.last_report_at: return delta = datetime.now(timezone.utc) - user.last_report_at.replace(tzinfo=timezone.utc) if delta.days >= settings.trust_regen_days: user.trust_score = min(100, user.trust_score + settings.trust_regen_amount) - await db.commit()app/api/admin.py-33-33 (1)
33-33:⚠️ Potential issue | 🟠 MajorString literal
"active"instead ofRoomStatus.activeenum.Line 33 compares
ChatRoom.status == "active"while Line 26 correctly usesAccountStatus.active. Depending on the DB driver and enum configuration, the raw string may not match the stored enum value, silently returning 0 for active rooms.Proposed fix
- active_rooms = await db.scalar(select(func.count(ChatRoom.id)).where(ChatRoom.status == "active")) + active_rooms = await db.scalar(select(func.count(ChatRoom.id)).where(ChatRoom.status == RoomStatus.active))Add the missing import:
from app.models.models import ( AccountStatus, Ad, BlockedPair, ChatRoom, DailyVerificationToken, Report, + RoomStatus, User, )app/services/matchmaking.py-17-21 (1)
17-21:⚠️ Potential issue | 🟠 Major
enqueueonly deduplicates in the target queue — user can exist in both queues.
lremon Line 20 only cleans the queue being pushed to. If a user's premium status changes between calls, their old entry in the other queue persists.try_matchcould then pop the same user from both queues, or the user could be matched while already in a room.Proposed fix
`@staticmethod` async def enqueue(user: User) -> None: queue = MatchmakingService.premium_queue_key if user.premium and settings.premium_matchmaking_boost else MatchmakingService.queue_key - await redis.lrem(queue, 0, str(user.id)) + await redis.lrem(MatchmakingService.queue_key, 0, str(user.id)) + await redis.lrem(MatchmakingService.premium_queue_key, 0, str(user.id)) await redis.rpush(queue, str(user.id))app/services/matchmaking.py-29-39 (1)
29-39:⚠️ Potential issue | 🟠 MajorActive-room check doesn't verify
expires_at, inconsistent with caller.
try_matchreturns an active room without checking expiry (Lines 30–39), but the callerfind_matchinapp/api/match.pyLines 27–35 explicitly filtersChatRoom.expires_at > datetime.now(timezone.utc). An expired-but-not-yet-cleaned-up room will be returned bytry_match, blocking the user from getting a new match.Proposed fix
active_room = await db.scalar( select(ChatRoom).where( and_( ChatRoom.status == RoomStatus.active, + ChatRoom.expires_at > datetime.now(timezone.utc), or_(ChatRoom.user_a_id == requester.id, ChatRoom.user_b_id == requester.id), ) ) )app/api/admin.py-55-58 (1)
55-58:⚠️ Potential issue | 🟠 Major
GET /blocked-pairsreturns all rows with no pagination.This loads the entire
blocked_pairstable into memory. As the system scales, this becomes a performance and memory issue. Addlimit/offsetpagination similar to the reports endpoint.Proposed fix
`@router.get`("/blocked-pairs") -async def blocked_pairs(db: AsyncSession = Depends(get_db)): - rows = (await db.execute(select(BlockedPair))).scalars().all() +async def blocked_pairs(limit: int = 100, offset: int = 0, db: AsyncSession = Depends(get_db)): + rows = (await db.execute(select(BlockedPair).offset(offset).limit(limit))).scalars().all()app/api/match.py-20-44 (1)
20-44:⚠️ Potential issue | 🟠 Major
response_model=MatchResponse | dictcauses unreliable response serialization.FastAPI's union
response_modelhandling can produce unexpected validation errors or silently drop fields depending on which branch matches first. When the queueddictis returned, theMatchResponsevalidator runs first and may emit confusing validation warnings in logs.Return a single response schema or drop
response_modelto let FastAPI pass through the raw dict:Proposed fix (simplest)
-@router.post("/find", response_model=MatchResponse | dict) +@router.post("/find") async def find_match(user: User = Depends(ensure_verified_today), db: AsyncSession = Depends(get_db)):app/api/auth.py-15-29 (1)
15-29:⚠️ Potential issue | 🟠 MajorBlocking bcrypt calls on the async event loop.
hash_password(Line 22) uses passlib/bcrypt which is CPU-intensive and synchronous. Calling it directly in anasync defhandler blocks the event loop for the duration of the hash computation (~200-400ms per call), stalling all concurrent requests. The same applies toverify_passwordon Line 35.Run these in a thread pool:
Proposed fix
+import asyncio ... async def register(payload: RegisterRequest, db: AsyncSession = Depends(get_db)): existing = await db.scalar(select(User).where(User.username == payload.username)) if existing: raise HTTPException(status_code=400, detail="Username already exists") + hashed = await asyncio.to_thread(hash_password, payload.password) user = User( username=payload.username, - password_hash=hash_password(payload.password), + password_hash=hashed, gender=payload.gender, preferred_match_gender=payload.preferred_match_gender, )async def login(payload: LoginRequest, db: AsyncSession = Depends(get_db)): user = await db.scalar(select(User).where(User.username == payload.username)) - if not user or not verify_password(payload.password, user.password_hash): + if not user or not await asyncio.to_thread(verify_password, payload.password, user.password_hash): raise HTTPException(status_code=401, detail="Invalid credentials")app/api/admin.py-61-70 (1)
61-70:⚠️ Potential issue | 🟠 MajorPOST endpoints receive data as query parameters instead of request body.
token: stron Line 62 (and similarlytitle,payload,activeon Lines 80 and 89) are bare typed parameters, which FastAPI interprets as query parameters. For POST/PATCH endpoints, secrets and payloads should be in the request body. The daily token in particular is being passed in the URL/query string, which gets logged in access logs and browser history.Wrap these in Pydantic models or annotate with
Body(...):Proposed fix for daily-token (apply same pattern to ads endpoints)
+from pydantic import BaseModel + +class DailyTokenRequest(BaseModel): + token: str + `@router.post`("/daily-token") -async def set_daily_token(token: str, db: AsyncSession = Depends(get_db)): +async def set_daily_token(payload: DailyTokenRequest, db: AsyncSession = Depends(get_db)): today = date.today() existing = await db.scalar(select(DailyVerificationToken).where(DailyVerificationToken.effective_date == today)) if existing: - existing.token = token + existing.token = payload.token else: - db.add(DailyVerificationToken(token=token, effective_date=today)) + db.add(DailyVerificationToken(token=payload.token, effective_date=today))app/api/ws.py-16-20 (1)
16-20: 🛠️ Refactor suggestion | 🟠 MajorDuplicate JWT decode logic — reuse
decode_tokenfromapp/core/security.py.
get_room_and_usermanually callsjwt.decode(...)replicating whatdecode_tokenalready does. This creates a maintenance burden: any change to token validation (e.g., adding audience or issuer claims) must be duplicated here.Proposed fix
-from jose import JWTError, jwt +from app.core.security import decode_token ... async def get_room_and_user(token: str, room_id: int, db: AsyncSession): - try: - user_id = int(jwt.decode(token, settings.secret_key, algorithms=[settings.algorithm])["sub"]) - except (JWTError, KeyError, ValueError): + sub = decode_token(token) + if not sub: + return None, None + try: + user_id = int(sub) + except (ValueError, TypeError): return None, None
🟡 Minor comments (8)
docker-compose.yml-5-7 (1)
5-7:⚠️ Potential issue | 🟡 Minor
depends_onshould use health checks to avoid startup race conditions.Without
condition: service_healthy, the API container starts as soon asdbandrediscontainers are created — not when they are ready to accept connections. This causes connection errors on startup.Proposed fix
depends_on: - - db - - redis + db: + condition: service_healthy + redis: + condition: service_healthyAnd add healthchecks to the
dbandredisservices:db: image: postgres:16 # ... healthcheck: test: ["CMD-SHELL", "pg_isready -U postgres"] interval: 5s timeout: 5s retries: 5 redis: image: redis:7 # ... healthcheck: test: ["CMD", "redis-cli", "ping"] interval: 5s timeout: 5s retries: 5app/core/deps.py-45-57 (1)
45-57:⚠️ Potential issue | 🟡 Minor
date.today()uses server-local time — could mismatch UTC-based verification records.If verification tokens are created or verified_date values are stored relative to UTC, but
date.today()resolves to a different calendar day in the server's local timezone, users near midnight boundaries will fail verification or pass when they shouldn't.Proposed fix
- from datetime import date + from datetime import date, datetime, timezone from app.models.models import UserVerification verified = await db.scalar( select(UserVerification).where( UserVerification.user_id == user.id, - UserVerification.verified_date == date.today(), + UserVerification.verified_date == datetime.now(timezone.utc).date(), ) )app/core/deps.py-16-23 (1)
16-23:⚠️ Potential issue | 🟡 MinorRate-limit race between
INCRandEXPIRE; shared "unknown" bucket.Two concerns:
If the process dies between
INCR(which auto-creates the key) andEXPIRE, the key lives forever and the counter grows unboundedly. Using a singleINCR+EXPIREpipeline (or Lua script, orSET NX EXfor the first hit) eliminates the window.When
request.clientisNone, all such requests share theratelimit:unknownbucket. Behind a reverse proxy that doesn't setrequest.client, this means a single bucket for all traffic. Consider readingX-Forwarded-For(with caution) or rejecting requests with no identifiable client.Atomic INCR + EXPIRE using a pipeline
async def rate_limit(request: Request): ip = request.client.host if request.client else "unknown" key = f"ratelimit:{ip}" - count = await redis.incr(key) - if count == 1: - await redis.expire(key, 60) + async with redis.pipeline(transaction=True) as pipe: + pipe.incr(key) + pipe.expire(key, 60) + results = await pipe.execute() + count = results[0] if count > settings.rate_limit_per_minute: raise HTTPException(status_code=429, detail="Rate limit exceeded")app/services/trust.py-26-26 (1)
26-26:⚠️ Potential issue | 🟡 Minor
replace(tzinfo=...)silently overwrites any existing timezone — prefer conversion.
last_report_atis stored in aDateTime(timezone=True)column and written withdatetime.now(timezone.utc), soasyncpgwill return it as a timezone-aware datetime..replace(tzinfo=timezone.utc)doesn't convert — it discards the original offset and stamps UTC on top. While the value is currently always UTC, this will silently produce wrong results if a non-UTC offset ever appears.Safer alternative
- delta = datetime.now(timezone.utc) - user.last_report_at.replace(tzinfo=timezone.utc) + last_report = user.last_report_at + if last_report.tzinfo is None: + last_report = last_report.replace(tzinfo=timezone.utc) + delta = datetime.now(timezone.utc) - last_reportapp/api/match.py-53-72 (1)
53-72:⚠️ Potential issue | 🟡 MinorReports are restricted to active rooms only.
Line 56 rejects reports when
room.status != RoomStatus.active. Users cannot report abusive behavior after a room has been closed or expired. This seems overly restrictive since most users would report after the conversation ends.Consider allowing reports on recently-closed rooms (e.g., within a time window).
app/api/match.py-76-92 (1)
76-92:⚠️ Potential issue | 🟡 MinorRoom can be closed multiple times — no idempotency guard.
If
close_roomis called twice for the same room, it succeeds both times, overwritingclosed_atand potentially serving duplicate ads to non-premium users. Consider returning early or 409 if the room is already closed.Proposed fix
room = await db.get(ChatRoom, room_id) if not room: raise HTTPException(status_code=404, detail="Room not found") + if room.status == RoomStatus.closed: + return {"message": "Room already closed"} if user.id not in [room.user_a_id, room.user_b_id]: raise HTTPException(status_code=403, detail="Not your room")app/api/auth.py-17-28 (1)
17-28:⚠️ Potential issue | 🟡 MinorRace condition on username uniqueness yields 500 instead of 400.
Between
await db.scalar(...)andawait db.commit(), a concurrent request can insert the same username. The DB unique constraint prevents corruption, but the unhandledIntegrityErrorsurfaces as a 500. Catch it to return a clean 400.Proposed fix
+from sqlalchemy.exc import IntegrityError ... db.add(user) - await db.commit() + try: + await db.commit() + except IntegrityError: + await db.rollback() + raise HTTPException(status_code=400, detail="Username already exists") await db.refresh(user)app/api/user.py-21-34 (1)
21-34:⚠️ Potential issue | 🟡 Minor
date.today()uses server-local time — prefer UTC.
date.today()returns the date in the server's local timezone. If the server isn't in UTC, the verification date can be off by a day relative to other UTC-based logic in the codebase (e.g., token expiry, session durations). The same concern applies to the adminset_daily_tokenendpoint.Proposed fix
-from datetime import date +from datetime import date, datetime, timezone ... - today = date.today() + today = datetime.now(timezone.utc).date()
🧹 Nitpick comments (13)
requirements.txt (1)
8-8:passlibis effectively unmaintained — consider future migration.
passlib==1.7.4hasn't had a release since 2020 and has open compatibility issues with newerbcryptversions. No critical CVEs currently, but for a "production-ready" system, consider migrating tobcryptdirectly orargon2-cffiin a follow-up.app/db/session.py (1)
10-12: Technically incorrect return type for async generator.
get_dbusesyield, making it anAsyncGenerator, not a plainAsyncSession. FastAPI'sDepends()handles this correctly at runtime, but the type hint may confuse static analysis tools and IDEs.Proposed fix
+from collections.abc import AsyncGenerator + -async def get_db() -> AsyncSession: +async def get_db() -> AsyncGenerator[AsyncSession, None]: async with AsyncSessionLocal() as session: yield sessionalembic/env.py (1)
7-7: Replace blanket# noqawith specific rule code.Ruff (RUF100) flags the blanket
noqa. The import is intentional (registers models onBase.metadata), so use the specific suppression code.Proposed fix
-from app.models import models # noqa +from app.models import models # noqa: F401alembic/versions/0001_initial.py (3)
35-35:ix_users_idindex on the primary key is redundant.Postgres automatically creates a unique index for the primary key column. This explicit index is a no-op that adds clutter.
Proposed fix
- op.create_index("ix_users_id", "users", ["id"])And remove the corresponding drop in
downgrade:- op.drop_index("ix_users_id", table_name="users")
37-46: Consider adding indexes onchat_rooms.user_a_idandchat_rooms.user_b_id.Queries that look up active rooms for a given user (e.g., matchmaking duplicate-session prevention, user history) will scan the full table without indexes on these foreign key columns. Postgres does not auto-index FK columns.
87-93: Consider adding an index ondaily_verification_tokens.token.Token lookups during user verification will require a sequential scan without an index on this column. The
effective_datehas a unique constraint (which creates an index), but thetokencolumn — which is the value users submit — does not.app/websocket/manager.py (1)
27-27: In-memory singleton won't work with multiple worker processes.
manageris process-local, so running with multiple Uvicorn workers (or multiple replicas) means WebSocket connections in different processes can't communicate. This is fine for a single-worker deployment, but worth documenting as a known limitation. For multi-process support, a Redis pub/sub broadcast layer would be needed.app/models/models.py (1)
38-53: Consider adding an index onUser.statusandUser.trust_score.The matchmaking service filters users by
statusandtrust_score. As the user base grows, queries filtering on these columns without indexes will degrade. Consider addingindex=Truetostatusandtrust_scorecolumns, or a composite index.app/core/redis_client.py (1)
6-6: Variableredisshadows theredispackage name.Naming the client instance
redisshadows the imported module, which can cause confusion. A name likeredis_clientwould be clearer and avoid accidental misuse.Proposed rename
-redis = Redis.from_url(settings.redis_url, decode_responses=True) +redis_client = Redis.from_url(settings.redis_url, decode_responses=True)This would require updating all import sites (e.g.,
from app.core.redis_client import redis→import redis_client).app/services/trust.py (1)
11-20: Same commit-ownership concern applies here —apply_report_penaltycommits eagerly.If this method is ever composed into a larger transactional flow (e.g., reporting also creates a
Reportrow inmatch.py), thedb.commit()here will flush the partial state. For now it works because the API endpoint doesn't add further DB writes after calling this, but pushing commit responsibility to callers would make both trust methods consistently composable.app/api/ws.py (1)
24-28: Fragile timezone handling onexpires_at.
room.expires_at.replace(tzinfo=timezone.utc)silently overwrites any existing timezone info. Since the model declaresDateTime(timezone=True), the DB should return tz-aware datetimes. If the driver returns UTC-aware values,replaceis harmless but misleading; if it returns a different offset,replacesilently corrupts the value. Prefer an explicit conversion or assert the value is already tz-aware.Suggested approach
- if datetime.now(timezone.utc) > room.expires_at.replace(tzinfo=timezone.utc): + expires_at = room.expires_at if room.expires_at.tzinfo else room.expires_at.replace(tzinfo=timezone.utc) + if datetime.now(timezone.utc) > expires_at:app/api/match.py (1)
10-10: Unused import:Message.
Messageis imported but not referenced anywhere in this file.Proposed fix
-from app.models.models import AccountStatus, BlockedPair, ChatRoom, Message, Report, RoomStatus, User +from app.models.models import AccountStatus, BlockedPair, ChatRoom, Report, RoomStatus, Userapp/api/admin.py (1)
73-76:GET /adsalso lacks pagination.Same unbounded query concern as blocked-pairs. Consider adding
limit/offset.
| def run_migrations_online() -> None: | ||
| connectable = engine_from_config( | ||
| config.get_section(config.config_ini_section, {}), | ||
| prefix="sqlalchemy.", | ||
| poolclass=pool.NullPool, | ||
| ) | ||
| with connectable.connect() as connection: | ||
| context.configure(connection=connection, target_metadata=target_metadata) | ||
| with context.begin_transaction(): | ||
| context.run_migrations() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check alembic.ini for the database URL configuration
fd 'alembic.ini' --exec cat {}Repository: huzely/Decrypt
Length of output: 614
🏁 Script executed:
# Also verify the current state of alembic/env.py to confirm it matches the review
fd 'alembic/env.py' -x head -40 {}Repository: huzely/Decrypt
Length of output: 40
🏁 Script executed:
# Display the full content of alembic/env.py to verify current state
cat -n alembic/env.py | head -50Repository: huzely/Decrypt
Length of output: 1397
🏁 Script executed:
# Also check imports to confirm the current driver setup
grep -n "engine_from_config\|async_engine_from_config\|import asyncio" alembic/env.pyRepository: huzely/Decrypt
Length of output: 148
run_migrations_online() uses synchronous connection — incompatible with postgresql+asyncpg:// driver.
The alembic.ini configures sqlalchemy.url = postgresql+asyncpg://..., but alembic/env.py uses synchronous operations: engine_from_config() with connectable.connect(). This is incompatible; asyncpg requires async/await patterns and will raise a runtime error when migrations run in online mode.
Update to the async migration pattern using async_engine_from_config, async with connectable.connect(), and connection.run_sync():
Async-compatible online migration
from logging.config import fileConfig
from alembic import context
-from sqlalchemy import engine_from_config, pool
+from sqlalchemy import pool
+from sqlalchemy.ext.asyncio import async_engine_from_config
+import asyncio
from app.db.base import Base
-from app.models import models # noqa
+from app.models import models # noqa: F401
config = context.config
if config.config_file_name is not None:
fileConfig(config.config_file_name)
target_metadata = Base.metadata
def run_migrations_offline() -> None:
url = config.get_main_option("sqlalchemy.url")
context.configure(url=url, target_metadata=target_metadata, literal_binds=True)
with context.begin_transaction():
context.run_migrations()
-def run_migrations_online() -> None:
- connectable = engine_from_config(
+async def run_async_migrations() -> None:
+ connectable = async_engine_from_config(
config.get_section(config.config_ini_section, {}),
prefix="sqlalchemy.",
poolclass=pool.NullPool,
)
- with connectable.connect() as connection:
- context.configure(connection=connection, target_metadata=target_metadata)
- with context.begin_transaction():
- context.run_migrations()
+ async with connectable.connect() as connection:
+ await connection.run_sync(do_run_migrations)
+ await connectable.dispose()
+
+
+def do_run_migrations(connection) -> None:
+ context.configure(connection=connection, target_metadata=target_metadata)
+ with context.begin_transaction():
+ context.run_migrations()
+
+
+def run_migrations_online() -> None:
+ asyncio.run(run_async_migrations())
if context.is_offline_mode():
run_migrations_offline()
else:
run_migrations_online()Also clarify the noqa directive on line 7 to # noqa: F401 (unused import suppression is intentional for model registration).
🤖 Prompt for AI Agents
In `@alembic/env.py` around lines 23 - 32, run_migrations_online currently builds
a synchronous engine with engine_from_config and uses connectable.connect(),
which breaks with the postgresql+asyncpg URL; replace the sync flow in
run_migrations_online with the async pattern: use async_engine_from_config(...)
to create an AsyncEngine, open it with "async with connectable.connect() as
connection", and inside await connection.run_sync(lambda sync_conn:
context.configure(connection=sync_conn, target_metadata=target_metadata))
followed by await connection.run_sync(lambda _: context.run_migrations()) (or
equivalent use of connection.run_sync for
context.begin_transaction()/run_migrations); also update imports to include
async_engine_from_config and change the existing noqa comment to "# noqa: F401"
to clarify intentional unused imports for model registration.
| def downgrade() -> None: | ||
| op.drop_table("user_verifications") | ||
| op.drop_table("daily_verification_tokens") | ||
| op.drop_table("ads") | ||
| op.drop_table("blocked_pairs") | ||
| op.drop_table("reports") | ||
| op.drop_index("ix_messages_room_id", table_name="messages") | ||
| op.drop_table("messages") | ||
| op.drop_table("chat_rooms") | ||
| op.drop_index("ix_users_id", table_name="users") | ||
| op.drop_table("users") |
There was a problem hiding this comment.
downgrade() does not drop Postgres enum types — re-running upgrade after downgrade will fail.
Postgres enum types (gender, roomstatus, accountstatus) are created by sa.Enum(...) during upgrade but are not dropped in downgrade. After a downgrade, these types persist, and a subsequent upgrade will fail with ProgrammingError: type "gender" already exists.
Proposed fix — drop enums in downgrade
def downgrade() -> None:
op.drop_table("user_verifications")
op.drop_table("daily_verification_tokens")
op.drop_table("ads")
op.drop_table("blocked_pairs")
op.drop_table("reports")
op.drop_index("ix_messages_room_id", table_name="messages")
op.drop_table("messages")
op.drop_table("chat_rooms")
op.drop_index("ix_users_id", table_name="users")
op.drop_table("users")
+ sa.Enum(name="gender").drop(op.get_bind(), checkfirst=True)
+ sa.Enum(name="roomstatus").drop(op.get_bind(), checkfirst=True)
+ sa.Enum(name="accountstatus").drop(op.get_bind(), checkfirst=True)🤖 Prompt for AI Agents
In `@alembic/versions/0001_initial.py` around lines 105 - 115, The downgrade() in
alembic/versions/0001_initial.py currently drops tables and indexes but leaves
the Postgres enum types (gender, roomstatus, accountstatus) behind; update
downgrade() to drop these enum types after dropping dependent tables/indexes by
executing DROP TYPE IF EXISTS for each enum name (gender, roomstatus,
accountstatus) or using the corresponding sa.Enum(...).drop(...) pattern against
the migration bind so a subsequent upgrade can recreate them cleanly. Ensure you
reference the enum names exactly and perform the drops at the end of the
downgrade() function.
| @router.post("/premium/activate") | ||
| async def activate_premium( | ||
| payload: PremiumActivateRequest, | ||
| user: User = Depends(get_current_user), | ||
| db: AsyncSession = Depends(get_db), | ||
| ): | ||
| if not payload.payment_reference: | ||
| raise HTTPException(status_code=400, detail="Payment reference required") | ||
| user.premium = True | ||
| await db.commit() | ||
| return {"message": "Premium activated", "premium": True} |
There was a problem hiding this comment.
Premium activation performs no payment verification — any user can self-grant premium.
The endpoint accepts any non-empty payment_reference string and immediately sets user.premium = True. There is no call to a payment gateway, no validation of the reference, no idempotency check, and no audit log. Any authenticated user can activate premium for free.
At minimum, log the activation and gate it behind actual payment verification. If the payment integration isn't ready yet, this endpoint should be disabled or admin-only.
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 40-40: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 41-41: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In `@app/api/user.py` around lines 37 - 47, The activate_premium endpoint
currently flips user.premium without verifying payment; change activate_premium
to first check idempotency (if user.premium is already True return 200/no-op),
then call a payment verification function (e.g.,
verify_payment(payment_reference) — implement or inject a service) to validate
payload.payment_reference and raise an HTTP error if verification fails, and
only on successful verification set user.premium = True and persist inside the
AsyncSession transaction (await db.commit()); also create and insert an audit
record (e.g., PremiumActivationLog with user id, payment_reference, status,
timestamp) or at minimum log the activation via the logger, and if payment
integration is not ready restrict the route to admins or disable the endpoint
until verify_payment is implemented.
| @router.websocket("/ws/chat/{room_id}") | ||
| async def chat_socket(websocket: WebSocket, room_id: int, token: str): | ||
| async with AsyncSessionLocal() as db: | ||
| room, user_id = await get_room_and_user(token, room_id, db) | ||
| if not room: | ||
| await websocket.close(code=1008) | ||
| return | ||
|
|
||
| await manager.connect(room_id, user_id, websocket) | ||
| await manager.send_to_room(room_id, {"type": "system", "message": "Stranger connected"}) | ||
|
|
||
| try: | ||
| while True: | ||
| data = await websocket.receive_json() | ||
| content = str(data.get("message", "")).strip() | ||
| if not content: | ||
| continue | ||
| msg = Message(room_id=room_id, sender_id=user_id, content=content) | ||
| db.add(msg) | ||
| await db.commit() | ||
| await manager.send_to_other(room_id, user_id, {"type": "message", "from": "Stranger", "message": content}) | ||
| except WebSocketDisconnect: | ||
| manager.disconnect(room_id, user_id) |
There was a problem hiding this comment.
Unhandled exceptions in the message loop leave stale connections in the manager.
Only WebSocketDisconnect is caught. If the client sends malformed JSON (receive_json raises JSONDecodeError), or a DB error occurs during db.commit(), the exception propagates without calling manager.disconnect, leaking the entry. Additionally, the AsyncSession is held open for the entire WebSocket lifetime, tying up a connection-pool slot for what may be a very long-lived session.
Wrap the loop in a broader try/finally for cleanup, and consider creating short-lived sessions for each DB write instead of reusing one session across the full connection:
Proposed fix
`@router.websocket`("/ws/chat/{room_id}")
async def chat_socket(websocket: WebSocket, room_id: int, token: str):
async with AsyncSessionLocal() as db:
room, user_id = await get_room_and_user(token, room_id, db)
if not room:
await websocket.close(code=1008)
return
await manager.connect(room_id, user_id, websocket)
await manager.send_to_room(room_id, {"type": "system", "message": "Stranger connected"})
- try:
- while True:
- data = await websocket.receive_json()
- content = str(data.get("message", "")).strip()
- if not content:
- continue
- msg = Message(room_id=room_id, sender_id=user_id, content=content)
- db.add(msg)
- await db.commit()
- await manager.send_to_other(room_id, user_id, {"type": "message", "from": "Stranger", "message": content})
- except WebSocketDisconnect:
- manager.disconnect(room_id, user_id)
+ try:
+ while True:
+ data = await websocket.receive_json()
+ content = str(data.get("message", "")).strip()
+ if not content:
+ continue
+ if len(content) > 2000:
+ continue
+ async with AsyncSessionLocal() as db:
+ msg = Message(room_id=room_id, sender_id=user_id, content=content)
+ db.add(msg)
+ await db.commit()
+ await manager.send_to_other(room_id, user_id, {"type": "message", "from": "Stranger", "message": content})
+ except WebSocketDisconnect:
+ pass
+ except Exception:
+ await websocket.close(code=1011)
+ finally:
+ manager.disconnect(room_id, user_id)🤖 Prompt for AI Agents
In `@app/api/ws.py` around lines 34 - 56, The WebSocket handler chat_socket
currently holds an AsyncSessionLocal for the entire connection and only catches
WebSocketDisconnect, which can leak manager state on other exceptions; change it
to ensure manager.disconnect(room_id, user_id) is always called by wrapping the
receive/send loop in try/finally (or try/except/finally) and move the DB work
into short-lived sessions per message (use async with AsyncSessionLocal() as db:
db.add(msg); await db.commit()) inside the loop; also catch JSON decoding errors
(e.g., ValueError/JSONDecodeError around websocket.receive_json()) so they don't
escape and leave stale connections, and ensure websocket is closed on fatal
errors as appropriate while still calling manager.disconnect in the finally
block.
| class Settings(BaseSettings): | ||
| app_name: str = "Anonymous Matchmaking" | ||
| env: str = "dev" | ||
| secret_key: str = "change-me" |
There was a problem hiding this comment.
Hardcoded default secret_key is a security risk.
If the SECRET_KEY environment variable is unset, the app silently falls back to "change-me", making all JWTs trivially forgeable. Remove the default to force explicit configuration, or add a startup check that rejects the placeholder in non-dev environments.
Proposed fix
- secret_key: str = "change-me"
+ secret_key: str # no default — must be set via env varOr, if you want to keep a dev-friendly default, validate at startup:
# in app/main.py or a startup event
if settings.env != "dev" and settings.secret_key == "change-me":
raise RuntimeError("SECRET_KEY must be set in production")🧰 Tools
🪛 Ruff (0.15.0)
[error] 7-7: Possible hardcoded password assigned to: "secret_key"
(S105)
🤖 Prompt for AI Agents
In `@app/core/config.py` at line 7, The config currently defines a hardcoded
default secret_key ("change-me") in app/core/config.py which is insecure; remove
the insecure default so SECRET_KEY must be provided via environment or, if you
keep a dev-friendly default, add a startup validation in your application
bootstrap (e.g., in app/main.py or the FastAPI startup event) that checks
settings.env and settings.secret_key and raises an error if settings.env !=
"dev" and settings.secret_key == "change-me" (or is unset), ensuring the app
fails to start in non-dev environments without a real secret.
| user_id = decode_token(credentials.credentials) | ||
| if not user_id: | ||
| raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token") | ||
| user = await db.get(User, int(user_id)) |
There was a problem hiding this comment.
int(user_id) will raise an unhandled ValueError → HTTP 500 on a tampered token.
decode_token returns the "sub" claim as a str | None. A valid signature with a non-numeric sub (e.g., a username accidentally stored there, or a maliciously crafted claim) passes the if not user_id check, then int(user_id) blows up with an uncaught ValueError, surfacing as a 500 to the client.
Proposed fix
user_id = decode_token(credentials.credentials)
if not user_id:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token")
- user = await db.get(User, int(user_id))
+ try:
+ uid = int(user_id)
+ except (ValueError, TypeError):
+ raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token")
+ user = await db.get(User, uid)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user_id = decode_token(credentials.credentials) | |
| if not user_id: | |
| raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token") | |
| user = await db.get(User, int(user_id)) | |
| user_id = decode_token(credentials.credentials) | |
| if not user_id: | |
| raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token") | |
| try: | |
| uid = int(user_id) | |
| except (ValueError, TypeError): | |
| raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token") | |
| user = await db.get(User, uid) |
🤖 Prompt for AI Agents
In `@app/core/deps.py` around lines 30 - 33, decode_token can return a non-numeric
"sub" string which will cause int(user_id) to raise ValueError and produce a
500; change the code around decode_token / user_id to validate and safely parse
the subject before calling db.get(User, ...). Specifically, replace the direct
int(user_id) call in the auth flow (the user_id variable returned by
decode_token) with a safe parse: try to convert to int (or check isdigit() for
your use case) and if conversion fails raise an HTTPException (use
status.HTTP_401_UNAUTHORIZED or the appropriate auth status) so a tampered or
non-numeric sub results in a 401 instead of an uncaught 500; ensure this
validation occurs before calling db.get(User, int(...)).
| for queue in [MatchmakingService.premium_queue_key, MatchmakingService.queue_key]: | ||
| size = await redis.llen(queue) | ||
| for _ in range(size): | ||
| raw_id = await redis.lpop(queue) | ||
| if not raw_id: | ||
| continue | ||
| candidate_id = int(raw_id) | ||
| if candidate_id == requester.id: | ||
| continue | ||
| candidate = await db.get(User, candidate_id) | ||
| if not candidate or candidate.status != AccountStatus.active: | ||
| continue | ||
| if candidate.trust_score < settings.matchmaking_min_trust_score: | ||
| continue | ||
| if not MatchmakingService._gender_compatible(requester, candidate): | ||
| continue | ||
| if await MatchmakingService._is_blocked(db, requester.id, candidate.id): | ||
| continue | ||
|
|
||
| expires_minutes = settings.premium_session_minutes if requester.premium or candidate.premium else settings.base_session_minutes | ||
| room = ChatRoom( | ||
| user_a_id=requester.id, | ||
| user_b_id=candidate.id, | ||
| status=RoomStatus.active, | ||
| expires_at=datetime.now(timezone.utc) + timedelta(minutes=expires_minutes), | ||
| ) | ||
| db.add(room) | ||
| await db.commit() | ||
| await db.refresh(room) | ||
| return room | ||
| return None |
There was a problem hiding this comment.
Critical: lpop permanently removes skipped candidates from the queue.
The loop pops each candidate via lpop (Line 44), then filters through validity checks (Lines 48–58). Candidates that fail any check (self-match, inactive, low trust, gender mismatch, blocked) are consumed and never re-enqueued. Over time, the queues drain to empty even when valid users are waiting.
Skipped candidates must be pushed back into the queue, or use a non-destructive iteration strategy (e.g., LRANGE + LREM only the matched candidate).
Proposed fix — collect skipped candidates and re-enqueue
for queue in [MatchmakingService.premium_queue_key, MatchmakingService.queue_key]:
size = await redis.llen(queue)
+ skipped = []
for _ in range(size):
raw_id = await redis.lpop(queue)
if not raw_id:
continue
candidate_id = int(raw_id)
if candidate_id == requester.id:
+ skipped.append(raw_id)
continue
candidate = await db.get(User, candidate_id)
if not candidate or candidate.status != AccountStatus.active:
continue
if candidate.trust_score < settings.matchmaking_min_trust_score:
continue
if not MatchmakingService._gender_compatible(requester, candidate):
+ skipped.append(raw_id)
continue
if await MatchmakingService._is_blocked(db, requester.id, candidate.id):
+ skipped.append(raw_id)
continue
expires_minutes = settings.premium_session_minutes if requester.premium or candidate.premium else settings.base_session_minutes
room = ChatRoom(
user_a_id=requester.id,
user_b_id=candidate.id,
status=RoomStatus.active,
expires_at=datetime.now(timezone.utc) + timedelta(minutes=expires_minutes),
)
db.add(room)
await db.commit()
await db.refresh(room)
+ # Re-enqueue skipped candidates
+ for s in skipped:
+ await redis.rpush(queue, s)
return room
+ # No match found in this queue — re-enqueue all skipped
+ for s in skipped:
+ await redis.rpush(queue, s)
return None🤖 Prompt for AI Agents
In `@app/services/matchmaking.py` around lines 41 - 71, The current loop uses
redis.lpop on MatchmakingService.premium_queue_key and
MatchmakingService.queue_key, which removes every candidate even if they fail
checks; fix by performing a non-destructive scan: for each queue, read all IDs
(e.g., LRANGE) or pop while collecting skipped raw_ids, evaluate each candidate
(using _gender_compatible, _is_blocked, requester, db, trust checks) and only
remove the matched candidate from the queue; re-enqueue any skipped IDs back to
the same queue in their original order (or use LREM to remove only the matched
candidate after selecting it) so invalid candidates are not permanently lost.
| environment: | ||
| DATABASE_URL: postgresql+asyncpg://postgres:postgres@db:5432/matchbot | ||
| REDIS_URL: redis://redis:6379/0 | ||
| SECRET_KEY: super-secret-key |
There was a problem hiding this comment.
Hardcoded SECRET_KEY is a critical security risk.
super-secret-key is trivially guessable. If this compose file is used as-is (or as a template) in any non-local environment, all JWTs are forgeable. Use an environment variable or Docker secret, and document that the value must be overridden.
Proposed fix
environment:
DATABASE_URL: postgresql+asyncpg://postgres:postgres@db:5432/matchbot
REDIS_URL: redis://redis:6379/0
- SECRET_KEY: super-secret-key
+ SECRET_KEY: ${SECRET_KEY:?SECRET_KEY must be set}🧰 Tools
🪛 Checkov (3.2.334)
[medium] 9-10: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In `@docker-compose.yml` around lines 8 - 11, The docker-compose environment block
currently hardcodes SECRET_KEY ("super-secret-key"), which is insecure; update
the compose to accept the secret via an environment variable or Docker secret
(e.g., reference an env var like ${SECRET_KEY} or use a Docker secret named
SECRET_KEY) and remove the literal value, add documentation or a compose comment
that the SECRET_KEY must be overridden in production, and ensure any service
startup (JWT code) reads the same env var; look for the environment: SECRET_KEY
entry in the compose file and replace it with the env-var/secret reference and
accompanying note.
| asyncpg==0.30.0 | ||
| alembic==1.14.0 | ||
| redis==5.2.0 | ||
| python-jose[cryptography]==3.3.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
python-jose 3.3.0 GHSA-6c5p-j8vq-pqhj algorithm confusion vulnerability
💡 Result:
GHSA-6c5p-j8vq-pqhj (CVE-2024-33663) – python-jose “algorithm confusion” (affects 3.3.0)
- What it is: An algorithm-confusion / signature-verification flaw where python-jose (through 3.3.0) can incorrectly accept/verify JWTs when using certain key formats—specifically OpenSSH ECDSA keys (and other key formats)—in a way that can enable HMAC verification with an asymmetric public key under some misuse/edge conditions. This is noted as similar to CVE-2022-29217. [1][2][3]
- Affected versions: python-jose < 3.4.0 (i.e., 3.3.0 is vulnerable). [1][2][3]
- Fix / patched version: Upgrade to python-jose 3.4.0 or later. [1][2][3]
- Severity: Reported as Critical (CVSS 9.3, v4) on GitHub’s advisory; OSV also lists severity entries and confirms affected/fixed versions. [1][3]
Sources:
[1] GitHub Advisory Database (GHSA-6c5p-j8vq-pqhj)
[2] OSV / PyPA advisory entry (PYSEC-2024-232)
[3] OSV entry for GHSA-6c5p-j8vq-pqhj
🌐 Web query:
python-jose GHSA-cjwg-qfpm-7377 DoS compressed JWE
💡 Result:
GHSA-cjwg-qfpm-7377 (CVE-2024-33664): python-jose DoS via compressed JWE (“JWT bomb”)
- What it is:
python-josecan be forced into resource exhaustion (CPU/memory) during JWE decode/decrypt by supplying a crafted JWE with an extremely high compression ratio (“JWT bomb”). This can cause a Denial of Service. [1][2] - Affected versions:
python-jose< 3.4.0 (i.e., through 3.3.0). [1][2] - Fixed version: 3.4.0. [1][2]
- Severity (CVSS v3.1): 5.3 (Medium); network exploitable, no auth/user interaction required; impacts availability. [1]
- Action: Upgrade:
pip install -U python-jose==3.4.0(or later). [1][2] - Distro note (example): Debian’s tracker has listed the Debian-packaged
python-jose(e.g., bookworm’s 3.3.0+dfsg-4) as vulnerable (fix status may depend on your distro packaging). [3]
Sources: GitHub Advisory Database entry for GHSA-cjwg-qfpm-7377/CVE-2024-33664 [1], OSV PYSEC-2024-233 [2], Debian security tracker [3].
References:
[1] GHSA-cjwg-qfpm-7377
[2] https://osv.dev/PYSEC-2024-233
[3] https://security-tracker.debian.org/tracker/CVE-2024-29370
🌐 Web query:
python-jose unmaintained maintenance status 2024 2025
💡 Result:
python-jose does not appear to be “unmaintained” in 2024–2025 based on release and security-fix activity:
- New releases shipped in 2025: PyPI shows 3.4.0 (Feb 18, 2025) and 3.5.0 (May 28, 2025). [1]
- Security response / CVE fix landed: GitHub Advisory CVE-2024-33664 lists versions < 3.4.0 as affected and 3.4.0 as the patched version (advisory updated Feb 18, 2025). [2]
- GitHub release notes confirm active maintenance work: the 3.4.0 release explicitly includes “Fix for CVE-2024-33664” and other compatibility/housekeeping updates; 3.5.0 adds Python version support changes and dependency updates. [3]
- There was community concern about inactivity in late 2023 (“Is python-jose still supported?”), but subsequent 2025 releases indicate the project resumed maintenance. [4]
Bottom line: it may have looked unmaintained during the 2021–2023 gap, but as of Feb–May 2025 it shows clear signs of active maintenance (including security patches). [1][3]
Sources: [1] PyPI release history, [2] GitHub Advisory (GHSA-cjwg-qfpm-7377 / CVE-2024-33664), [3] GitHub Releases page, [4] GitHub issue #340.
🌐 Web query:
PyJWT replacement for python-jose JWT library comparison
💡 Result:
Can PyJWT replace python-jose?
Sometimes. It depends on which JOSE features you rely on.
Quick comparison (PyJWT vs python-jose)
| Capability | PyJWT | python-jose |
|---|---|---|
| Core focus | JWT (RFC 7519) encode/decode | Broader JOSE toolkit |
| JWS (signed JWTs) | Yes | Yes (HS*/RS*/ES* listed in docs) [2] |
| JWE (encrypted JWTs) | Not a focus (PyJWT is primarily JWT/JWS) | Yes (has JWE module) [1] |
| JWK / key management | Limited compared to full JOSE stacks | Yes (has JWK module) [1] |
| Cryptography backends | Uses cryptography-based ecosystem (PyJWT project) |
Explicit multi-backend design (cryptography, pycryptodome, native-python) [1] |
| Typical API | jwt.encode(payload, key, algorithm=...) / jwt.decode(...) |
Similar high-level jwt.encode(...) / jwt.decode(...) [1][3] |
When PyJWT is a good replacement
Use PyJWT if you only need signed JWTs (JWS)—e.g., HS256/RS256/ES256 style tokens—and you don’t need JWE encryption or broader JOSE/JWK workflows. PyJWT’s basic usage matches this well (encode/decode). [3]
(PyJWT is actively maintained; the GitHub repo shows a “Latest” release tag 2.11.0 (Jan 30, 2026).) [3]
When PyJWT is not a drop-in replacement
If your python-jose usage includes any of the following, PyJWT alone usually won’t cover it:
- JWE (token encryption)
- A more complete JOSE surface area (JWS+JWE+JWK with richer primitives)
Practical alternatives if you need “python-jose-like” breadth
joserfc(Authlib’s JOSE-focused library): explicitly supports JWS, JWE, JWK, JWT, and includes migration notes from both PyJWT andpython-jose. [5][4]- Authlib (
authlib.jose.jwt): mature JOSE/JWT implementation with encode/decode and claim validation patterns. [6]
Rule of thumb
- Signed JWTs only → PyJWT [3]
- Signed + encrypted tokens (JWE) or full JOSE/JWK needs →
joserfcor Authlib [5][6]
Sources: python-jose README/backends [1], python-jose JWS algorithms [2], PyJWT README/releases [3], joserfc overview + migration notes [5][4], Authlib JWT docs [6].
🏁 Script executed:
# Check how python-jose is used in the codebase
find . -type f -name "*.py" | head -20Repository: huzely/Decrypt
Length of output: 478
🏁 Script executed:
# Search for imports and usage of jose
rg "from jose|import jose|python-jose" --type py -A 2Repository: huzely/Decrypt
Length of output: 318
🏁 Script executed:
# Check if security.py exists and examine it
find . -type f -name "security.py" | xargs cat 2>/dev/null | head -50Repository: huzely/Decrypt
Length of output: 995
python-jose 3.3.0 has critical vulnerabilities — upgrade to 3.4.0+
Two security advisories affect python-jose==3.3.0:
- CVE-2024-33663 (GHSA-6c5p-j8vq-pqhj, CVSS 9.3): Algorithm-confusion flaw enabling HMAC verification bypass with asymmetric keys
- CVE-2024-33664 (GHSA-cjwg-qfpm-7377, CVSS 5.3): Denial-of-service via crafted compressed JWE
Both are patched in python-jose 3.4.0 (released Feb 18, 2025).
Recommended fix: Upgrade python-jose[cryptography] to 3.4.0 or later. This requires no code changes—encode() and decode() APIs are identical.
Alternatively, migrate to PyJWT[crypto]==2.11.0 if preferred; the codebase uses only signed JWTs (JWS), which PyJWT fully supports. This would require changing imports (jose → jwt) in app/core/security.py and app/api/ws.py.
🧰 Tools
🪛 OSV Scanner (2.3.2)
[CRITICAL] 7-7: python-jose 3.3.0: undefined
(PYSEC-2024-232)
[CRITICAL] 7-7: python-jose 3.3.0: undefined
(PYSEC-2024-233)
[CRITICAL] 7-7: python-jose 3.3.0: python-jose algorithm confusion with OpenSSH ECDSA keys
[CRITICAL] 7-7: python-jose 3.3.0: python-jose denial of service via compressed JWE content
🤖 Prompt for AI Agents
In `@requirements.txt` at line 7, The requirements pin uses
python-jose[cryptography]==3.3.0 which has critical vulnerabilities
(CVE-2024-33663/CVE-2024-33664); update the dependency in requirements.txt to
python-jose[cryptography]>=3.4.0 to pull the patched release, or if you prefer
to switch libraries, replace python-jose with PyJWT[crypto]==2.11.0 and update
JWT usages: change imports and calls from jose (used in app/core/security.py and
app/api/ws.py) to jwt and adjust any encode()/decode() call sites accordingly
(APIs are compatible for signed JWTs), then run tests and regenerate lock files.
1 participant
Motivation
Description
User,ChatRoom,Message,Report,BlockedPair,Ad,DailyVerificationToken, andUserVerificationinapp/models/models.pyand a baseline Alembic migration atalembic/versions/0001_initial.py.app/api/*.py), including daily verification (POST /user/verify) and admin daily-token management (POST /admin/daily-token).app/services/matchmaking.pyusing Redis lists with a premium priority queue, gender/trust/block checks, duplicate-session prevention and session expiry handling, plusapp/services/trust.pyfor penalties and regeneration andapp/services/ads.pyfor ad rotation and impression tracking./ws/chat/{room_id}) with JWT token validation, room membership & expiry enforcement, message persistence, and a simple connection manager inapp/websocket/manager.py.scripts/maintenance.pyandapp/services/maintenance.py) to auto-close expired rooms and release cooldowns/suspensions, plusapp/core/*for config, Redis client and security helpers, Docker artifacts (Dockerfile,docker-compose.yml),requirements.txt, and a comprehensiveREADME.mdwith run and deploy instructions.app/core/config.py(overridable via.env/.env.example), daily token is managed viaPOST /admin/daily-token, ad selection logic is inapp/services/ads.pyand ad CRUD is exposed in admin routes, and premium benefits are controlled via config andPOST /user/premium/activate.Testing
python -m compileall app scripts alembicwhich completed successfully to validate module-level syntax and imports.alembic upgrade headand exercise endpoints (or add automated integration tests).Codex Task
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores