191 lines
6.4 KiB
Markdown
191 lines
6.4 KiB
Markdown
# Todolist - Architecture and Design Issues
|
|
|
|
This document tracks design and architecture issues discovered during documentation review.
|
|
|
|
---
|
|
|
|
## Issues
|
|
|
|
### 1. In-Memory Rate Limiting Not Persistent
|
|
|
|
**Title:** In-memory rate limiting resets on process restart
|
|
|
|
**Severity:** medium
|
|
|
|
**Location:** [src/server/middleware/auth.py](src/server/middleware/auth.py#L54-L68)
|
|
|
|
**Description:** Rate limiting state is stored in memory dictionaries (`_rate`, `_origin_rate`) which reset when the process restarts, allowing attackers to bypass lockouts.
|
|
|
|
**Suggested action:** Implement Redis-backed rate limiting for production deployments; add documentation warning about single-process limitation.
|
|
|
|
---
|
|
|
|
### 2. Failed Login Attempts Not Persisted
|
|
|
|
**Title:** Failed login attempts stored in-memory only
|
|
|
|
**Severity:** medium
|
|
|
|
**Location:** [src/server/services/auth_service.py](src/server/services/auth_service.py#L62-L74)
|
|
|
|
**Description:** The `_failed` dictionary tracking failed login attempts resets on process restart, allowing brute-force bypass via service restart.
|
|
|
|
**Suggested action:** Store failed attempts in database or Redis; add configurable lockout policy.
|
|
|
|
---
|
|
|
|
### 3. Duplicate Health Endpoints
|
|
|
|
**Title:** Health endpoints defined in two locations
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/server/api/health.py](src/server/api/health.py) and [src/server/controllers/health_controller.py](src/server/controllers/health_controller.py)
|
|
|
|
**Description:** Health check functionality is split between `api/health.py` (detailed checks) and `controllers/health_controller.py` (basic check). Both are registered, potentially causing confusion.
|
|
|
|
**Suggested action:** Consolidate health endpoints into a single module; remove duplicate controller.
|
|
|
|
---
|
|
|
|
### 4. Deprecation Warnings in Production Code
|
|
|
|
**Title:** Deprecated file-based scan method still in use
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/core/SerieScanner.py](src/core/SerieScanner.py#L129-L145)
|
|
|
|
**Description:** The `scan()` method emits deprecation warnings but is still callable. CLI may still use this method.
|
|
|
|
**Suggested action:** Complete migration to `scan_async()` with database storage; remove deprecated method after CLI update.
|
|
|
|
---
|
|
|
|
### 5. SQLite Concurrency Limitations
|
|
|
|
**Title:** SQLite not suitable for high concurrency
|
|
|
|
**Severity:** medium
|
|
|
|
**Location:** [src/config/settings.py](src/config/settings.py#L53-L55)
|
|
|
|
**Description:** Default database is SQLite (`sqlite:///./data/aniworld.db`) which has limited concurrent write support. May cause issues under load.
|
|
|
|
**Suggested action:** Document PostgreSQL migration path; add connection pooling configuration for production.
|
|
|
|
---
|
|
|
|
### 6. Master Password Hash in Config File
|
|
|
|
**Title:** Password hash stored in plaintext config file
|
|
|
|
**Severity:** medium
|
|
|
|
**Location:** [data/config.json](data/config.json) (other.master_password_hash)
|
|
|
|
**Description:** The bcrypt password hash is stored in `config.json` which may be world-readable depending on deployment.
|
|
|
|
**Suggested action:** Ensure config file has restricted permissions (600); consider environment variable for hash in production.
|
|
|
|
---
|
|
|
|
### 7. Module-Level Singleton Pattern
|
|
|
|
**Title:** Singleton services using module-level globals
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/server/services/download_service.py](src/server/services/download_service.py), [src/server/utils/dependencies.py](src/server/utils/dependencies.py)
|
|
|
|
**Description:** Services use module-level `_instance` variables for singletons, making testing harder and preventing multi-instance hosting.
|
|
|
|
**Suggested action:** Migrate to FastAPI app.state for service storage; document testing patterns for singleton cleanup.
|
|
|
|
---
|
|
|
|
### 8. Hardcoded Provider
|
|
|
|
**Title:** Default provider hardcoded
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/config/settings.py](src/config/settings.py#L66-L68)
|
|
|
|
**Description:** The `default_provider` setting defaults to `"aniworld.to"` but provider switching is not fully implemented in the API.
|
|
|
|
**Suggested action:** Implement provider selection endpoint; document available providers.
|
|
|
|
---
|
|
|
|
### 9. Inconsistent Error Response Format
|
|
|
|
**Title:** Some endpoints return different error formats
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/server/api/download.py](src/server/api/download.py), [src/server/api/anime.py](src/server/api/anime.py)
|
|
|
|
**Description:** Most endpoints use the standard error response format from `error_handler.py`, but some handlers return raw `{"detail": "..."}` responses.
|
|
|
|
**Suggested action:** Audit all endpoints for consistent error response structure; use custom exception classes uniformly.
|
|
|
|
---
|
|
|
|
### 10. Missing Input Validation on WebSocket
|
|
|
|
**Title:** WebSocket messages lack comprehensive validation
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/server/api/websocket.py](src/server/api/websocket.py#L120-L145)
|
|
|
|
**Description:** Client messages are parsed with basic Pydantic validation, but room names and action types are not strictly validated against an allow-list.
|
|
|
|
**Suggested action:** Add explicit room name validation; rate-limit WebSocket message frequency.
|
|
|
|
---
|
|
|
|
### 11. No Token Revocation Storage
|
|
|
|
**Title:** JWT token revocation is a no-op
|
|
|
|
**Severity:** medium
|
|
|
|
**Location:** [src/server/services/auth_service.py](src/server/services/auth_service.py)
|
|
|
|
**Description:** The `revoke_token()` method exists but does not persist revocations. Logged-out tokens remain valid until expiry.
|
|
|
|
**Suggested action:** Implement token blacklist in database or Redis; check blacklist in `create_session_model()`.
|
|
|
|
---
|
|
|
|
### 12. Anime Directory Validation
|
|
|
|
**Title:** Anime directory path not validated on startup
|
|
|
|
**Severity:** low
|
|
|
|
**Location:** [src/server/fastapi_app.py](src/server/fastapi_app.py#L107-L125)
|
|
|
|
**Description:** The configured anime directory is used without validation that it exists and is readable. Errors only appear when scanning.
|
|
|
|
**Suggested action:** Add directory validation in lifespan startup; return clear error if path invalid.
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
| Severity | Count |
|
|
| --------- | ------ |
|
|
| High | 0 |
|
|
| Medium | 5 |
|
|
| Low | 7 |
|
|
| **Total** | **12** |
|
|
|
|
---
|
|
|
|
## Changelog Note
|
|
|
|
**2025-12-13**: Initial documentation review completed. Created comprehensive API.md with all REST and WebSocket endpoints documented with source references. Updated ARCHITECTURE.md with system overview, layer descriptions, design patterns, and data flow diagrams. Created README.md with quick start guide. Identified 12 design/architecture issues requiring attention.
|