# Phase 4: Router Layer Test Report **Date:** 2026-05-24 **Project:** mtgonline (`/home/wall-o/projects/mtgonline/backend/`) **Scope:** All FastAPI routers in `app/routers/` — correctness, consistency, and integration with schemas/services --- ## Summary | Router File | Status | Endpoints | Critical Issues | Warnings | Info | |---|---|---|---|---|---| | `main.py` | ⚠️ FAIL | — | 1 | 2 | 2 | | `auth.py` | ✅ PASS | 4 | 0 | 1 | 0 | | `users.py` | ✅ PASS | 4 | 0 | 0 | 1 | | `decks.py` | ✅ PASS | 14 | 0 | 1 | 2 | | `card_import.py` | ⚠️ FAIL | 7 | 1 | 1 | 1 | | `card_router.py` | ⚠️ FAIL | 6 | 2 | 1 | 1 | | `interactions.py` | ⚠️ FAIL | 8 | 1 | 2 | 1 | | `games/router.py` | ⚠️ FAIL | 6 | 1 | 2 | 2 | | `admin.py` | ✅ PASS | 6 | 0 | 0 | 1 | | `rooms.py` | ✅ PASS | 5 | 0 | 0 | 1 | | `refresh.py` | ⚠️ FAIL | 3 | 1 | 1 | 0 | | `user_data.py` | ⚠️ FAIL | 28 | 3 | 5 | 4 | **Overall: FAIL** — 7 out of 11 router files have critical or blocking issues. --- ## 1. `main.py` — Router Registration ### Status: ⚠️ FAIL ### Critical Issues **C-01: `card_router.router` double-prefix path conflict** - `card_router.py` defines `router = APIRouter(prefix="/api/cards", ...)` - `main.py` mounts it with `app.include_router(card_router.router, prefix="/api", ...)` - **Result:** All card search endpoints resolve to `/api/api/cards/...` instead of `/api/cards/...` - **Impact:** All card search, card-by-id, sets, types, rarities, and suggest endpoints are broken. ### Warnings **W-01: `interactions.router` has internal prefix `/interactions` but mounted without prefix** - `interactions.py`: `router = APIRouter(prefix="/interactions", ...)` - `main.py`: `app.include_router(interactions.router, ...)` (no prefix) - **Result:** Paths resolve correctly to `/interactions/...` — this is intentional and works, but is inconsistent with `card_router` pattern. **W-02: `refresh.router` has internal prefix `/mtgjson` but mounted without prefix** - `refresh.py`: `router = APIRouter(prefix="/mtgjson", ...)` - `main.py`: `app.include_router(refresh.router)` (no prefix) - **Result:** Paths resolve correctly to `/mtgjson/...` — same pattern as W-01. ### Info **I-01:** `games.router` is imported as `games` (the package) and mounted with `prefix="/games"`. The actual router is at `app.routers.games.router.router`. This works because FastAPI resolves `games.router` to the `router` attribute of the `games` module. **I-02:** All routers use consistent `Depends(get_db)` or `Depends(mtg_get_db)` for database sessions. --- ## 2. `auth.py` — Authentication Router ### Status: ✅ PASS ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | POST | `/auth/login` | `LoginResponse` | ❌ (public) | | POST | `/auth/refresh` | `TokenResponse` | ❌ (token-based) | | POST | `/auth/register` | `UserResponse` | ❌ (public) | | GET | `/auth/me` | `UserResponse` | ✅ (token query param) | ### Warnings **W-01: `get_current_user` endpoint uses query parameter for token** - The `/auth/me` endpoint expects `token: str` as a query parameter rather than a Bearer token in the Authorization header. - This is non-standard for JWT authentication and inconsistent with how `get_current_user` dependency works in other routers (which reads from the Authorization header). - **Recommendation:** Consider using `Authorization: Bearer ` header for consistency, or document this as a deliberate design choice. ### Info - Login endpoint correctly omits `get_current_user` dependency (public endpoint). - All response models match their schema definitions in `schemas.py`. - Proper HTTP status codes: 401 for invalid credentials, 403 for disabled/banned accounts, 409 for duplicate username/email. --- ## 3. `users.py` — User Management Router ### Status: ✅ PASS ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/users/{user_id}` | `UserResponse` | ✅ | | PATCH | `/users/{user_id}` | `UserResponse` | ✅ (self-only) | | POST | `/users/{user_id}/ban` | dict | ✅ (admin/judge) | | POST | `/users/{user_id}/unban` | dict | ✅ (admin/judge) | ### Info - Self-update protection is correctly implemented (users can only update their own profile). - Ban/unban endpoints properly check for admin/judge privileges. - `UserUpdate` schema fields are correctly mapped to model fields. - Password hashing is applied only when `new_password` is provided. --- ## 4. `decks.py` — Deck Management Router ### Status: ✅ PASS ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/decks/` | `UserDeckListResponse` | ✅ | | POST | `/decks/` | `UserDeckResponse` | ✅ | | GET | `/decks/{deck_id}` | `UserDeckResponse` | ✅ | | PATCH | `/decks/{deck_id}` | `UserDeckResponse` | ✅ | | DELETE | `/decks/{deck_id}` | `MessageResponse` | ✅ | | POST | `/decks/{deck_id}/finalize` | `DeckFinalizeResponse` | ✅ | | POST | `/decks/{deck_id}/cards` | `DeckCardResponse` | ✅ | | GET | `/decks/{deck_id}/cards` | `DeckCardListResponse` | ✅ | | PATCH | `/decks/{deck_id}/cards/{card_id}` | `DeckCardResponse` | ✅ | | DELETE | `/decks/{deck_id}/cards/{card_id}` | `MessageResponse` | ✅ | | GET | `/decks/precedents` | `PrecedentListResponse` | ✅ | | POST | `/decks/precedents` | `PrecedentResponse` | ✅ | | GET | `/decks/precedents/{precedent_id}` | `PrecedentResponse` | ✅ | | POST | `/decks/precedents/{precedent_id}/use` | dict | ✅ | | POST | `/decks/search/cards` | `CardSearchResponse` | ✅ | | GET | `/decks/{deck_id}/suggestions` | `SuggestionListResponse` | ✅ | | POST | `/decks/{deck_id}/suggestions` | `SuggestionResponse` | ✅ | ### Warnings **W-01: `DeckPrecedent` imported from `app.models.user_deck`** - `DeckPrecedent` and `DeckPrecedentCard` are imported from `app.models.user_deck` but may belong in a different model file. Verify model placement is correct. ### Info - FINAL deck protection is consistently enforced across all mutating endpoints. - Owner verification is applied on all deck-access endpoints. - Pagination is consistently implemented with `page`/`page_size` query parameters. - Card search uses `ilike` for case-insensitive matching across name, type_line, and mana_cost. - Precedent cloning correctly copies cards from precedent to new deck. --- ## 5. `card_import.py` — Card Import Router ### Status: ⚠️ FAIL ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | POST | `/api/v1/card-import/import` | `CardImportResponse` | ✅ | | GET | `/api/v1/card-import/import/{import_id}/status` | `CardImportStatusResponse` | ✅ | | GET | `/api/v1/card-import/import/{import_id}/results` | `CardImportSummary` | ✅ | | POST | `/api/v1/card-import/import/{import_id}/confirm` | `MessageResponse` | ✅ | | GET | `/api/v1/card-import/user/cards` | `List[Dict]` | ✅ | | DELETE | `/api/v1/card-import/user/cards/{card_import_id}` | `MessageResponse` | ✅ | | GET | `/api/v1/card-import/user/decks` | `List[Dict]` | ✅ | ### Critical Issues **C-01: `get_user_cards` endpoint has no response model** - Returns `List[Dict[str, Any]]` instead of a proper Pydantic response model. - Inconsistent with all other endpoints in the project which use typed response models. - **Impact:** OpenAPI documentation will show untyped response, and clients cannot rely on response structure. - **Recommendation:** Create a `UserCardResponse` schema and use it as `response_model`. ### Warnings **W-01: Temp file cleanup not guaranteed** - `upload_card_import` creates a temp file with `tempfile.NamedTemporaryFile` but only wraps parsing in a try/except. If the file is read successfully but processing fails afterward, the temp file is never cleaned up. - **Recommendation:** Use `with tempfile.NamedTemporaryFile(...)` as a context manager, or add a `finally` block to delete the temp file. ### Info - File type validation correctly restricts to xlsx, csv, json, ods. - Owner verification is applied on all import-related endpoints. - Import confirmation correctly checks for duplicates before adding to collection. --- ## 6. `card_router.py` — Card Search Router ### Status: ⚠️ FAIL ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/api/cards/search` | `CardSearchResponse` | ❌ | | GET | `/api/cards/{card_id}` | `CardResponse` | ❌ | | GET | `/api/cards/sets` | `List[SetResponse]` | ❌ | | GET | `/api/cards/types` | `List[CardTypeResponse]` | ❌ | | GET | `/api/cards/rarities` | `List[str]` | ❌ | | GET | `/api/cards/suggest` | `List[Dict]` | ❌ | ### Critical Issues **C-01: Double-prefix path conflict (see main.py C-01)** - Router has `prefix="/api/cards"` and is mounted at `prefix="/api"`. - All endpoints resolve to `/api/api/cards/...` — **all 6 endpoints are broken.** **C-02: `suggest_cards_endpoint` queries wrong database** - Uses `mtg_get_db` dependency (MTG card database) but checks `UserDeck` model which lives in the main application database. - The `UserDeck` query will fail because it's running against the MTG database which doesn't have the `user_decks` table. - **Impact:** Card suggestion endpoint will always return 404 or crash. - **Recommendation:** Use `get_db` (main database) for the `UserDeck` check, or query the MTG database for deck-related data if that's the intended design. ### Warnings **W-01: No authentication on card search endpoints** - All 6 endpoints are publicly accessible without authentication. - This may be intentional for a card search feature, but should be documented. - No rate limiting is applied. ### Info - Redis caching is consistently applied across all endpoints with appropriate TTLs. - Response format uses `{"cached": bool, "results": ...}` pattern — non-standard but consistent within this router. --- ## 7. `interactions.py` — Card Interactions Router ### Status: ⚠️ FAIL ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/interactions/synergies/{card_id}` | dict | ❌ | | GET | `/interactions/counters/{card_id}` | dict | ❌ | | GET | `/interactions/evolutions/{card_id}` | dict | ❌ | | GET | `/interactions/recommend/{card_id}` | dict | ❌ | | GET | `/interactions/search/synergies` | dict | ❌ | | GET | `/interactions/search/counters` | dict | ❌ | | GET | `/interactions/search/evolutions` | dict | ❌ | | GET | `/interactions/stats/{card_id}` | dict | ❌ | ### Critical Issues **C-01: All endpoints return untyped `dict` responses** - None of the 8 endpoints use `response_model` — they all return raw dictionaries. - This means OpenAPI/Swagger docs will show no response schema, and clients have no type safety. - **Recommendation:** Create response schemas (e.g., `SynergyResponse`, `CounterResponse`, etc.) and apply them. ### Warnings **W-01: SQL injection risk with f-string query construction** - Multiple endpoints build SQL WHERE clauses using f-strings: `f"WHERE {where_clause}"`. - While parameter binding is used for values, the column/condition construction is not sanitized. - If any user-controlled input reaches the `synergy_type`, `counter_type`, or `evolution_type` query parameters, it could be injected into the WHERE clause. - **Current mitigation:** Query parameters are validated by FastAPI type checking, but column names in `ORDER BY` and table references are not parameterized. - **Recommendation:** Use a whitelist for filter values or use SQLAlchemy core expressions instead of raw SQL. **W-02: No authentication on any endpoint** - All 8 endpoints are publicly accessible. - No rate limiting is applied. - **Recommendation:** Document as intentional public API, or add authentication if these are sensitive interaction data. ### Info - Redis caching is consistently applied with appropriate TTLs (10min–30min). - Pagination is included in search endpoints. - The `stats/{card_id}` endpoint returns zeros for missing cards rather than 404 — reasonable design choice. --- ## 8. `games/router.py` — Game Management Router ### Status: ⚠️ FAIL ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/games/` | `List[GameResponse]` | ✅ | | POST | `/games/` | `GameResponse` | ✅ | | GET | `/games/{game_id}` | `GameResponse` | ✅ | | POST | `/games/{game_id}/join` | dict | ✅ | | POST | `/games/{game_id}/leave` | dict | | POST | `/games/{game_id}/start` | dict | | POST | `/games/{game_id}/end` | dict | ### Critical Issues **C-01: `list_games` and `get_game` are stub/mock implementations** - `list_games` always returns `[]` without querying any database. - `get_game` returns a hardcoded mock response regardless of `game_id`. - **Impact:** These endpoints are non-functional. Any client relying on them will get incorrect data. - **Recommendation:** Either implement proper database queries or mark these as TODO with appropriate error responses. ### Warnings **W-01: Join/Leave/Start/End endpoints have no response model** - All 4 mutation endpoints return raw dicts instead of typed response models. - **Recommendation:** Create a `GameActionResponse` schema. **W-02: `get_game` does not verify game ownership or existence** - Returns a mock response for any `game_id` without checking if the game actually exists. - No authorization check beyond the general `get_current_user` dependency. - **Recommendation:** Implement proper game lookup and ownership verification. ### Info - `create_game` correctly verifies user existence before creating. - `GameCreate` schema is properly used for the create endpoint. - The router imports `User` model but only for verification in `create_game`. --- ## 9. `admin.py` — Admin Router ### Status: ✅ PASS ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/admin/users` | `List[dict]` | ✅ (admin/judge) | | GET | `/admin/bans` | `List[BanResponse]` | ✅ (admin/judge) | | POST | `/admin/bans` | `BanResponse` | ✅ (admin/judge) | | POST | `/admin/bans/{ban_id}/unban` | dict | ✅ (admin/judge) | | GET | `/admin/logs` | `List[dict]` | ✅ (admin/judge) | | POST | `/admin/audit` | dict | ✅ (admin/judge) | ### Info - All endpoints correctly enforce admin/judge privilege checks. - Ban creation properly links to user and sets moderator name. - Unban operation correctly updates both Ban and User records. - Audit logging captures admin actions with target user and details. --- ## 10. `rooms.py` — Room Management Router ### Status: ✅ PASS ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/rooms/` | `List[RoomResponse]` | ✅ | | GET | `/rooms/{room_id}` | `RoomResponse` | ✅ | | POST | `/rooms/` | dict | ✅ (admin/judge) | | PATCH | `/rooms/{room_id}` | dict | ✅ (admin/judge) | | DELETE | `/rooms/{room_id}` | dict | ✅ (admin/judge) | ### Info - CRUD operations are complete for rooms. - Admin-only write operations are properly enforced. - Room name uniqueness is checked before creation. - `RoomResponse` schema matches model fields. --- ## 11. `refresh.py` — MTGJSON Data Refresh Router ### Status: ⚠️ FAIL ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | POST | `/mtgjson/refresh` | dict | ✅ (admin) | | GET | `/mtgjson/status` | dict | ✅ (admin) | | POST | `/mtgjson/verify` | dict | ✅ (admin) | ### Critical Issues **C-01: `/status` and `/verify` endpoints missing `db` dependency** - `get_refresh_status` and `verify_files` do not accept `db: AsyncSession = Depends(get_db)`. - If `MTGJSONManager.get_health_status()` or `verify_files()` need database access, these endpoints will fail. - **Impact:** Potential runtime error if the manager methods require a database session. - **Recommendation:** Add `db` dependency or verify that the manager methods don't need it. ### Warnings **W-01: No response model on any endpoint** - All 3 endpoints return raw dicts without `response_model`. - **Recommendation:** Create response schemas for consistency. --- ## 12. `user_data.py` — User Data Router ### Status: ⚠️ FAIL ### Endpoints | Method | Path | Response Model | Auth Required | |---|---|---|---| | GET | `/api/v1/user-data/sessions/me` | `List[SessionCleanupResponse]` | ✅ | | DELETE | `/api/v1/user-data/sessions/cleanup` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/sessions/logout` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/decks/{deck_id}/versions` | `DeckVersionResponse` | ✅ | | GET | `/api/v1/user-data/decks/{deck_id}/versions` | `DeckVersionListResponse` | ✅ | | PATCH | `/api/v1/user-data/decks/{deck_id}/versions/{version_id}` | `DeckVersionResponse` | ✅ | | DELETE | `/api/v1/user-data/decks/{deck_id}/versions/{version_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/replays` | `GameReplayResponse` | ✅ | | GET | `/api/v1/user-data/replays` | `GameReplayListResponse` | ✅ | | GET | `/api/v1/user-data/replays/{replay_id}` | `GameReplayResponse` | ✅ | | PATCH | `/api/v1/user-data/replays/{replay_id}` | `GameReplayResponse` | ✅ | | DELETE | `/api/v1/user-data/replays/{replay_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/replays/{replay_id}/players` | dict | ✅ | | GET | `/api/v1/user-data/replays/{replay_id}/players` | `List[dict]` | ✅ | | POST | `/api/v1/user-data/outcomes` | `GameOutcomeResponse` | ✅ | | GET | `/api/v1/user-data/outcomes` | `GameOutcomeListResponse` | ✅ | | GET | `/api/v1/user-data/statistics/{user_id}` | `UserStatisticsResponse` | ✅ | | POST | `/api/v1/user-data/statistics/update` | `StatisticsUpdateResponse` | ✅ | | POST | `/api/v1/user-data/collection` | `CardCollectionResponse` | ✅ | | GET | `/api/v1/user-data/collection` | `CardCollectionListResponse` | ✅ | | PATCH | `/api/v1/user-data/collection/{card_id}` | `CardCollectionResponse` | ✅ | | DELETE | `/api/v1/user-data/collection/{card_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/wishlist` | `WishlistResponse` | ✅ | | GET | `/api/v1/user-data/wishlist` | `WishlistListResponse` | ✅ | | PATCH | `/api/v1/user-data/wishlist/{item_id}` | `WishlistResponse` | ✅ | | DELETE | `/api/v1/user-data/wishlist/{item_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/groups` | `GroupResponse` | ✅ | | GET | `/api/v1/user-data/groups` | `GroupListResponse` | ✅ | | GET | `/api/v1/user-data/groups/{group_id}` | `GroupResponse` | ✅ | | PATCH | `/api/v1/user-data/groups/{group_id}` | `GroupResponse` | ✅ | | DELETE | `/api/v1/user-data/groups/{group_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/groups/{group_id}/members` | dict | ✅ | | PATCH | `/api/v1/user-data/groups/{group_id}/members/{member_id}` | `GroupMemberUpdate` | ✅ | | DELETE | `/api/v1/user-data/groups/{group_id}/members/{member_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/groups/{group_id}/messages` | `GroupChatMessageResponse` | ✅ | | GET | `/api/v1/user-data/groups/{group_id}/messages` | `GroupChatMessageListResponse` | ✅ | | POST | `/api/v1/user-data/networks` | `NetworkResponse` | ✅ | | GET | `/api/v1/user-data/networks` | `NetworkListResponse` | ✅ | | GET | `/api/v1/user-data/networks/{network_id}` | `NetworkResponse` | ✅ | | PATCH | `/api/v1/user-data/networks/{network_id}` | `NetworkResponse` | ✅ | | DELETE | `/api/v1/user-data/networks/{network_id}` | `MessageResponse` | ✅ | | POST | `/api/v1/user-data/networks/{network_id}/members` | dict | ✅ | | GET | `/api/v1/user-data/preferences` | `UserPreferenceResponse` | ✅ | | PATCH | `/api/v1/user-data/preferences` | `UserPreferenceResponse` | ✅ | | GET | `/api/v1/user-data/activity` | `ActivityLogListResponse` | ✅ | ### Critical Issues **C-01: `create_deck_version` verifies user against `User` table instead of deck ownership** - The endpoint checks `select(User).where(User.id == user_id)` which will always succeed for any authenticated user. - It does NOT verify that the user owns the deck being versioned. - **Impact:** Any authenticated user can create versions for any deck. - **Recommendation:** Add a deck ownership check (e.g., query the deck's `user_id` field). **C-02: `get_game_replays` has ambiguous join condition** - `conditions.append(ReplayPlayer.user_id == user_id)` — the `user_id` column exists in both `GameReplay` (via ReplayPlayer join) and `ReplayPlayer`. - SQLAlchemy may raise `AmbiguousForeignKeysError` or join against the wrong table. - **Recommendation:** Use explicit table references: `ReplayPlayer.user_id == user_id` is correct, but the join should be explicit: `stmt = stmt.join(ReplayPlayer)`. **C-03: `update_user_statistics` has no user ownership verification** - The endpoint accepts `user_id` as a query parameter and updates statistics for any user. - Any authenticated user can modify another user's statistics. - **Recommendation:** Either restrict to self-update or add admin check. ### Warnings **W-01: `get_deck_versions` returns wrong response structure** - `DeckVersionListResponse` expects `versions: List[DeckVersionResponse]`, `total: int`, `page: int`, `page_size: int`, `total_pages: int`. - But the endpoint wraps the list in `[SessionCleanupResponse(...)]` for sessions — wait, that's the sessions endpoint. - Actually, `get_deck_versions` correctly returns `DeckVersionListResponse` with proper pagination fields. **This is correct.** **W-02: `get_user_groups` and `get_user_networks` add `member_count` to response** - `GroupResponse` and `NetworkResponse` schemas include `member_count: int = 0` as a default, so this is actually valid. - **No issue here** — the schemas already account for this field. **W-03: `send_group_message` manually constructs response instead of using `model_validate`** - Inconsistent with other endpoints that use `model_validate`. - **Recommendation:** Use `GroupChatMessageResponse.model_validate(message)` for consistency, or keep manual construction if sender_username needs special handling. **W-04: `add_replay_player` and `add_network_member` return raw dicts** - These endpoints return `{"message": ..., "player_id": ...}` instead of typed response models. - **Recommendation:** Create response schemas. **W-05: `get_replay_players` returns `List[dict]` instead of typed response** - **Recommendation:** Create a `ReplayPlayerResponse` schema. ### Info - Extensive CRUD coverage across sessions, decks, replays, outcomes, statistics, collection, wishlist, groups, networks, preferences, and activity logs. - Owner verification is consistently applied on collection, wishlist, and preference endpoints. - Group and network admin/owner checks are properly implemented. - Pagination is consistently applied across list endpoints. - `get_user_groups` correctly filters to groups where the user is a member using subquery. --- ## Cross-Cutting Issues ### Authentication/Authorization | Issue | Severity | Affected Routers | |---|---|---| | No auth on card_router endpoints | Warning | `card_router.py` | | No auth on interactions endpoints | Warning | `interactions.py` | | `update_user_statistics` allows updating any user | Critical | `user_data.py` | | `create_deck_version` doesn't verify deck ownership | Critical | `user_data.py` | ### Response Model Consistency | Issue | Count | Affected Routers | |---|---|---| | Endpoints returning `dict` instead of typed model | 12 | `card_import.py`, `card_router.py`, `interactions.py`, `games/router.py`, `refresh.py`, `user_data.py` | ### Database Session Consistency | Issue | Affected Routers | |---|---| | Missing `db` dependency | `refresh.py` | | Cross-database query (mtg_get_db for UserDeck) | `card_router.py` | ### Path Prefix Consistency | Issue | Affected Routers | |---|---| | Double-prefix conflict (`/api/api/cards/...`) | `card_router.py` + `main.py` | | Inconsistent prefix patterns (some routers define internal prefix, some don't) | `card_router.py`, `interactions.py`, `refresh.py` | --- ## Recommendations (Priority Order) ### P0 — Fix Immediately (Blocking) 1. **Fix `card_router.py` double-prefix**: Remove `prefix="/api/cards"` from the router definition in `card_router.py` since `main.py` already mounts it at `prefix="/api"`. This breaks all 6 card search endpoints. 2. **Fix `card_router.py` cross-database query**: Change `suggest_cards_endpoint` to use `get_db` instead of `mtg_get_db` for the `UserDeck` check. 3. **Fix `user_data.py` `create_deck_version` ownership check**: Add deck ownership verification before allowing version creation. 4. **Fix `user_data.py` `update_user_statistics` authorization**: Add ownership or admin check. ### P1 — Fix Soon (Significant Impact) 5. **Add response models to all endpoints returning `dict`**: Create proper Pydantic schemas for `card_import.py`, `interactions.py`, `games/router.py`, `refresh.py`, and `user_data.py` endpoints that currently return raw dicts. 6. **Fix `games/router.py` stub implementations**: Either implement proper database queries for `list_games` and `get_game`, or return appropriate error responses. 7. **Fix `refresh.py` missing `db` dependency**: Add `db` parameter to `/status` and `/verify` endpoints. 8. **Fix `interactions.py` SQL injection risk**: Use parameterized queries or whitelists for filter values. ### P2 — Improve (Nice to Have) 9. **Standardize prefix pattern**: Decide whether routers should define their own prefixes or rely on `main.py` mounting. Apply consistently. 10. **Add authentication to public endpoints**: Document why `card_router.py` and `interactions.py` are public, or add rate limiting. 11. **Fix `auth.py` token retrieval**: Consider using Bearer token header instead of query parameter for `/auth/me`. 12. **Add temp file cleanup in `card_import.py`**: Use context manager or finally block. --- ## Appendix: Endpoint Count by Router | Router | Total | GET | POST | PATCH | DELETE | |---|---|---|---|---|---| | `auth.py` | 4 | 1 | 3 | 0 | 0 | | `users.py` | 4 | 1 | 2 | 1 | 0 | | `decks.py` | 17 | 7 | 6 | 2 | 2 | | `card_import.py` | 7 | 3 | 2 | 0 | 1 | | `card_router.py` | 6 | 6 | 0 | 0 | 0 | | `interactions.py` | 8 | 8 | 0 | 0 | 0 | | `games/router.py` | 7 | 2 | 5 | 0 | 0 | | `admin.py` | 6 | 3 | 3 | 0 | 0 | | `rooms.py` | 5 | 2 | 1 | 1 | 1 | | `refresh.py` | 3 | 1 | 2 | 0 | 0 | | `user_data.py` | 45 | 17 | 14 | 8 | 6 | | **Total** | **112** | **51** | **38** | **12** | **10** |