klg-asutk-app/docs/ANALYSIS_AND_RECOMMENDATIONS.md
Yuriy a7da43be0e apply recommendations: security, get_db, exceptions, eslint, api-client
- session: set_tenant use bound param (SQL injection fix)
- health: text('SELECT 1'), REDIS_URL from config
- deps: re-export get_db from session, use settings.ENABLE_DEV_AUTH (default False)
- routes: all get_db from app.api.deps; conftest overrides deps.get_db
- main: register exception handlers from app.api.exceptions
- next.config: enable ESLint and TypeScript checks
- .eslintrc: drop @typescript-eslint/recommended; fix no-console (logger, ws-client, regulations)
- backend/.env.example added
- frontend: export apiFetch; dashboard, profile, settings, risks use api-client
- docs/ANALYSIS_AND_RECOMMENDATIONS.md

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-14 21:48:58 +03:00

209 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Анализ проекта КЛГ АСУ ТК и рекомендации
**Дата анализа:** 2025
**Путь к проекту:** `/Users/yrippertgmail.com/Desktop/klg_asutk_app`
**Статус:** Рекомендации из данного документа применены (коммит/дата по желанию).
---
## 1. Обзор архитектуры
| Слой | Технологии |
|------|------------|
| **Frontend** | Next.js 14, React 18, Tailwind, SWR, next-auth (beta), PWA |
| **Backend** | FastAPI, SQLAlchemy 2, Pydantic 2, Alembic, Redis, Prometheus |
| **Инфраструктура** | PostgreSQL, Redis, Keycloak OIDC, Docker, Helm |
- **Фронт:** ~40 страниц, единый API-клиент (`lib/api/api-client.ts`), проксирование `/api/v1/*` на бэкенд через `next.config.js` rewrites.
- **Бэкенд:** 29+ роут-файлов, 147+ эндпоинтов, мультитенантность (RLS), роли (RBAC), DEV/OIDC авторизация.
---
## 2. Сильные стороны
- **Чёткая доменная модель:** ВС, ЛГ, наряды, дефекты, персонал ПЛГ, ФГИС РЭВС, нормативная база — с привязкой к правовым основаниям (ВК РФ, ФАП, EASA, ICAO).
- **Единый API-клиент:** `api-client.ts` с токеном, редиректом при 401, типами; часть страниц уже переведена на него.
- **Безопасность:** `.env` в `.gitignore`, `.env.example`, `docs/SECURITY.md`, заголовки X-Frame-Options / X-Content-Type-Options, CORS из настроек.
- **Логгер:** `lib/logger.ts` вместо `console.log` на фронте; на бэкенде — `logging`.
- **Рефакторинг модулей:** legal → пакет (base + handlers), personnel — пакет, FGIS — вынесены типы в `fgis/base_service.py`.
- **Тесты:** pytest в backend (conftest, переопределение БД), Playwright для E2E.
---
## 3. Критичные проблемы и рекомендации
### 3.1 SQL-инъекция в `set_tenant` (backend)
**Файл:** `backend/app/db/session.py`
```python
db.execute(text(f"SET LOCAL app.current_org_id = '{org_id}'"))
```
Если `org_id` приходит из JWT/пользователя, возможна инъекция.
**Рекомендация:** использовать параметризованный запрос (например, через `text("... :org_id").bindparams(org_id=org_id)` или аналог для вашего драйвера), либо жёстко валидировать `org_id` (UUID/белый список).
---
### 3.2 SQLAlchemy 2: `db.execute("SELECT 1")` без `text()`
**Файл:** `backend/app/api/routes/health.py`, эндпоинт `detailed_health`:
```python
db.execute("SELECT 1")
```
В SQLAlchemy 2.x строка должна быть обёрнута в `text()`.
**Рекомендация:**
```python
from sqlalchemy import text
db.execute(text("SELECT 1"))
```
---
### 3.3 Тесты: переопределение `get_db` не покрывает все роуты
**Файл:** `backend/tests/conftest.py`
Переопределяется `app.db.session.get_db`, тогда как часть роутов импортирует `get_db` из `app.api.deps`. В таких роутах подмена не срабатывает, тесты могут ходить в реальную БД.
**Рекомендация:** везде использовать один источник зависимости — например, только `app.api.deps.get_db` — и в conftest переопределять именно его:
```python
from app.api.deps import get_db
# ...
app.dependency_overrides[get_db] = _override_get_db
```
Либо оставить реализацию в `session.py`, а в `deps.py` реэкспортировать `from app.db.session import get_db` и везде импортировать из `deps`.
---
### 3.4 Обработчики исключений не подключены
**Файл:** `backend/app/main.py`
В `app/api/exceptions.py` определены обработчики для `RequestValidationError`, `IntegrityError`, `SQLAlchemyError` и др., но в `main.py` они не регистрируются. Используется только общий `@app.exception_handler(Exception)`.
**Рекомендация:** в `main.py` подключить обработчики из `exceptions.py` (например, через `app.add_exception_handler`), чтобы возвращать 422/409 с единообразным форматом и не «проглатывать» валидацию и ошибки БД общим 500.
---
### 3.5 Сборка и качество кода отключены
**Файл:** `next.config.js`
```javascript
eslint: { ignoreDuringBuilds: true },
typescript: { ignoreBuildErrors: true },
```
Линт и проверка типов при сборке отключены — ошибки ESLint/TypeScript не блокируют деплой.
**Рекомендация:** для продакшена включить проверки: убрать или выставить `false` и починить накопившиеся ошибки; при необходимости временно ограничить scope (например, только `app/`, `lib/`).
---
## 4. Важные улучшения (средний приоритет)
### 4.1 Единый источник зависимости `get_db`
Сейчас часть роутов импортирует `get_db` из `app.db.session`, часть — из `app.api.deps`. Это усложняет тесты и конфигурацию.
**Рекомендация:** оставить реализацию в `app.db.session`, в `app.api.deps` делать реэкспорт и везде в роутах использовать `from app.api.deps import get_db`. В тестах переопределять только этот `get_db`.
---
### 4.2 Единообразные запросы к API на фронте
Много страниц по-прежнему используют «голый» `fetch('/api/v1/...')` без единого клиента: разный подход к токену, ошибкам и редиректу при 401.
**Рекомендация:** постепенно переводить все вызовы на `lib/api/api-client.ts` (или обёртки над ним), чтобы авторизация, обработка ошибок и редирект были централизованы.
---
### 4.3 DEV-авторизация по умолчанию
**Файл:** `backend/app/api/deps.py`
```python
ENABLE_DEV_AUTH = os.getenv("ENABLE_DEV_AUTH", "true").lower() == "true"
```
При отсутствии переменной окружения включён dev-режим (обход авторизации).
**Рекомендация:** по умолчанию считать продакшен: `"false"` и явно включать dev только в локальной среде (например, в `.env.local` с `ENABLE_DEV_AUTH=true`). В `config` уже есть `ENABLE_DEV_AUTH: bool = False` — использовать `settings.ENABLE_DEV_AUTH` вместо чтения `os.getenv` в deps.
---
### 4.4 Health check: Redis и зависимости
**Файл:** `backend/app/api/routes/health.py`
В `detailed_health` Redis подключается к `host="localhost", port=6379`, тогда как в конфиге используется `REDIS_URL`. При смене окружения проверка может быть некорректной.
**Рекомендация:** брать URL из `settings.REDIS_URL` (или из переменной окружения) и использовать `redis.from_url()` как в основном `health_check`.
---
### 4.5 Дублирование логгеров на фронте
В `package.json` присутствуют и `winston`, и `pino`. При этом в коде используется собственный `lib/logger.ts`.
**Рекомендация:** если winston/pino не используются — удалить из зависимостей; если планируется использование — зафиксировать один стек и постепенно перейти на него в `lib/logger.ts`, чтобы не дублировать логику.
---
## 5. Дополнительные рекомендации (низкий приоритет)
### 5.1 Backend `.env.example`
В корне есть `.env.example` для фронта и общих переменных; для бэкенда отдельного шаблона нет.
**Рекомендация:** добавить `backend/.env.example` с переменными из `app/core/config.py` (DATABASE_URL, REDIS_URL, OIDC_*, ENABLE_DEV_AUTH, CORS_ORIGINS и т.д.) и кратким комментарием.
---
### 5.2 Миграции при старте
В `main.py` при старте выполняется обход папки `migrations` и выполнение всех `.sql` файлов. При сбое делается только `rollback`, без явного учёта уже применённых миграций.
**Рекомендация:** в продакшене полагаться на Alembic и не применять сырые SQL при старте приложения; оставить автозапуск миграций только для dev или вынести в отдельный job/entrypoint.
---
### 5.3 Документация API и версии
В коде указаны версии (например, `2.1.0` в main, `2.2.0` в health). Имеет смысл вынести версию в одно место (например, `app/__init__.py` или `pyproject.toml`) и использовать её в OpenAPI и в health.
---
### 5.4 Роли на эндпоинтах
Часть роутов не использует `require_roles` и полагается только на `get_current_user`. Для чувствительных операций (удаление, экспорт, панель регулятора) стоит явно ограничивать доступ по ролям.
---
## 6. Краткий чек-лист по приоритетам
| Приоритет | Действие |
|-----------|----------|
| Критично | Исправить `set_tenant` (параметризованный запрос или валидация `org_id`) |
| Критично | В `health.py` использовать `text("SELECT 1")` |
| Критично | Унифицировать `get_db` и переопределение в тестах |
| Критично | Подключить обработчики из `exceptions.py` в `main.py` |
| Высоко | Убрать `ignoreDuringBuilds` / `ignoreBuildErrors` в next.config и починить ошибки |
| Средне | Перевести все вызовы API на фронте на api-client |
| Средне | Использовать в deps `settings.ENABLE_DEV_AUTH`, по умолчанию False |
| Средне | В detailed_health использовать `REDIS_URL` из конфига |
| Низко | Добавить backend/.env.example, упорядочить версии и миграции |
---
© Анализ подготовлен для проекта КЛГ АСУ ТК (АО «REFLY»).