fix: audit — json try/except, ZIP-bomb drhider, classify reset only processing
Deploy contracts-flask / validate (push) Successful in 0s
Deploy contracts-flask / validate (push) Successful in 0s
This commit is contained in:
@@ -0,0 +1,64 @@
|
|||||||
|
# Аудит кода contracts-flask — 2026-06-30
|
||||||
|
|
||||||
|
## Контекст
|
||||||
|
|
||||||
|
После переименования `services/` → `compare/` и восстановления сервиса проведён аудит кодовой базы.
|
||||||
|
Фокус: **что реально упадёт, сломается или работает нестабильно**. Без вылизывания.
|
||||||
|
Сервис непубличный, дев-окружение. Auth пока не нужен.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## ⚠️ Исправлено в этом цикле
|
||||||
|
|
||||||
|
### #1. POST-хендлеры без try вокруг Content-Length и json.loads
|
||||||
|
**Файл:** `deploy/convert_server.py`
|
||||||
|
**Проблема:** `_handle_api_sync`, `_handle_api_groups`, `_handle_apply_groups`, промпт-хендлеры — все делают `json.loads(self.rfile.read(length))` без try. Битый Content-Length → ValueError, пустой body → JSONDecodeError. Оба не ловятся → оборванное соединение.
|
||||||
|
**Фикс:** обёрнуто в try/except с возвратом `{"ok": false, "error": "..."}`.
|
||||||
|
|
||||||
|
### #11. ZIP-бомба в DrHider
|
||||||
|
**Файл:** `deploy/drhider/drhider.py`
|
||||||
|
**Проблема:** проверяется только заявленный `info.file_size` (можно подделать), нет лимита фактически распакованных байт, нет проверки ratio сжатия.
|
||||||
|
**Фикс:** скопирована защита из `compare/unzip.py` — накопительный счётчик распакованных байт + ratio-проверка (100×).
|
||||||
|
|
||||||
|
### #6. classify сбрасывает status у уже классифицированных документов
|
||||||
|
**Файл:** `deploy/compare/classify.py`
|
||||||
|
**Проблема:** `reset_classify_status(batch_id)` в начале `classify_batch()` сбрасывает на 'pending' ВСЕ документы батча, включая уже классифицированные. При повторном запуске — жжёт токены LLM на переклассификацию.
|
||||||
|
**Фикс:** `reset_classify_status` теперь сбрасывает только `'processing'` (crash recovery), оставляя `'classified'` и `'garbage'` нетронутыми.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 🟡 Известные проблемы (не исправлены — низкий приоритет)
|
||||||
|
|
||||||
|
### SSE: двойная запись в мёртвый сокет
|
||||||
|
`convert_server.py` — при отвале клиента `run_pipeline` бросает `BrokenPipeError`, except ловит и **снова** зовёт `_sse` → второе исключение. Грязные трейсы, но не крашит сервер.
|
||||||
|
|
||||||
|
### classify: «успешный» JSON от LLM, который не dict
|
||||||
|
`classify.py` — `json.loads` может вернуть список/строку вместо dict → `AttributeError` → документ молча помечается failed. Случается редко.
|
||||||
|
|
||||||
|
### .doc через /upload не работает
|
||||||
|
`parse.py` зовёт `_parse_docx` (python-docx) для `.doc`, а тот читает только OOXML/.docx. Формат разрешён в `upload.py`, но не парсится. Надо либо убрать `.doc` из whitelist, либо звать `/convert-doc`.
|
||||||
|
|
||||||
|
### Гонка счётчиков в ThreadPoolExecutor
|
||||||
|
`classify.py` мутирует `garbage`, `json_total`, `type_counts` из 4 потоков через `nonlocal` без локов. Счётчики могут привирать на 1-2 единицы.
|
||||||
|
|
||||||
|
### _safe_json_parse — 4 уровня эвристик
|
||||||
|
Реаниматор битого JSON. Лечит симптом «LLM обрезал ответ». Дописывание кавычек/скобок вслепую может дать валидный но мусорный JSON → тихая неверная классификация.
|
||||||
|
|
||||||
|
### Весь файл в base64 в БД
|
||||||
|
`upload.py` хранит `original_bytes` как base64 (+33%) в Postgres. `SELECT *` таскает мегабайты. База пухнет.
|
||||||
|
|
||||||
|
### LLM URL/key захардкожены в 3 местах
|
||||||
|
`classify.py`, `drhider_server.py`, `llm_client.py`. Рассинхрон — тот же класс проблемы, что с `DB_NAME`.
|
||||||
|
|
||||||
|
### DrHider: PDF→txt + потеря форматирования DOCX
|
||||||
|
PDF на выходе становится `.txt`. Замена в DOCX сваливает текст в первый run → форматирование плывёт.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## ✅ Что НЕ является проблемой (проверено)
|
||||||
|
|
||||||
|
- SQL-инъекций нет — весь `db/` на параметризованных запросах
|
||||||
|
- Path traversal в static-раздаче прикрыт `realpath`+`startswith`
|
||||||
|
- Path traversal в ZIP-распаковке прикрыт (`unzip.py`, `drhider.py`)
|
||||||
|
- Nginx защищает от больших тел (client_max_body_size)
|
||||||
|
- DrHider двойной encode fix сделан
|
||||||
Binary file not shown.
|
After Width: | Height: | Size: 70 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 33 KiB |
+33
-12
@@ -218,8 +218,7 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
def _handle_api_sync(self):
|
def _handle_api_sync(self):
|
||||||
"""Удалить ВСЕ документы, НЕ входящие в keep_ids. БД = зеркало таблицы."""
|
"""Удалить ВСЕ документы, НЕ входящие в keep_ids. БД = зеркало таблицы."""
|
||||||
from db.connection import execute, query
|
from db.connection import execute, query
|
||||||
length = int(self.headers.get("Content-Length", 0))
|
body = self._read_json_body()
|
||||||
body = json.loads(self.rfile.read(length)) if length > 0 else {}
|
|
||||||
keep_ids = set(body.get("keep_ids", []))
|
keep_ids = set(body.get("keep_ids", []))
|
||||||
# Prevent DoS: too many IDs
|
# Prevent DoS: too many IDs
|
||||||
if len(keep_ids) > 1000:
|
if len(keep_ids) > 1000:
|
||||||
@@ -283,8 +282,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
Отдельный процесс — свои коннекты, своя память.
|
Отдельный процесс — свои коннекты, своя память.
|
||||||
HTTP-сервер продолжает отвечать на batch-progress.
|
HTTP-сервер продолжает отвечать на batch-progress.
|
||||||
"""
|
"""
|
||||||
length = int(self.headers.get("Content-Length", 0))
|
body = self._read_json_body()
|
||||||
body = json.loads(self.rfile.read(length))
|
if not body:
|
||||||
|
return
|
||||||
batch_id = body.get("batch_id")
|
batch_id = body.get("batch_id")
|
||||||
if not batch_id:
|
if not batch_id:
|
||||||
self._json({"ok": False, "error": "batch_id required"}, 400)
|
self._json({"ok": False, "error": "batch_id required"}, 400)
|
||||||
@@ -326,8 +326,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
# ── POST /apply-groups ───────────────────────────────────────────────
|
# ── POST /apply-groups ───────────────────────────────────────────────
|
||||||
|
|
||||||
def _handle_apply_groups(self):
|
def _handle_apply_groups(self):
|
||||||
length = int(self.headers.get("Content-Length", 0))
|
body = self._read_json_body()
|
||||||
body = json.loads(self.rfile.read(length))
|
if not body:
|
||||||
|
return
|
||||||
batch_id = body.get("batch_id")
|
batch_id = body.get("batch_id")
|
||||||
groups = body.get("groups", [])
|
groups = body.get("groups", [])
|
||||||
if not batch_id:
|
if not batch_id:
|
||||||
@@ -392,8 +393,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
|
|
||||||
def _handle_api_prompts_save(self):
|
def _handle_api_prompts_save(self):
|
||||||
from db import prompts as db_p
|
from db import prompts as db_p
|
||||||
length = int(self.headers.get("Content-Length", 0))
|
body = self._read_json_body()
|
||||||
body = json.loads(self.rfile.read(length))
|
if not body:
|
||||||
|
return
|
||||||
role = body.get("role", "")
|
role = body.get("role", "")
|
||||||
name = body.get("name", "v" + __import__("datetime").datetime.now().isoformat()[:16])
|
name = body.get("name", "v" + __import__("datetime").datetime.now().isoformat()[:16])
|
||||||
prompt_body = body.get("body", "")
|
prompt_body = body.get("body", "")
|
||||||
@@ -407,8 +409,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
|
|
||||||
def _handle_api_prompts_activate(self):
|
def _handle_api_prompts_activate(self):
|
||||||
from db import prompts as db_p
|
from db import prompts as db_p
|
||||||
length = int(self.headers.get("Content-Length", 0))
|
body = self._read_json_body()
|
||||||
body = json.loads(self.rfile.read(length))
|
if not body:
|
||||||
|
return
|
||||||
pid = body.get("id", "")
|
pid = body.get("id", "")
|
||||||
if not pid:
|
if not pid:
|
||||||
self._json({"ok": False, "error": "id required"}, 400)
|
self._json({"ok": False, "error": "id required"}, 400)
|
||||||
@@ -418,8 +421,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
|
|
||||||
def _handle_api_prompts_delete(self):
|
def _handle_api_prompts_delete(self):
|
||||||
from db import prompts as db_p
|
from db import prompts as db_p
|
||||||
length = int(self.headers.get("Content-Length", 0))
|
body = self._read_json_body()
|
||||||
body = json.loads(self.rfile.read(length))
|
if not body:
|
||||||
|
return
|
||||||
pid = body.get("id", "")
|
pid = body.get("id", "")
|
||||||
if not pid:
|
if not pid:
|
||||||
self._json({"ok": False, "error": "id required"}, 400)
|
self._json({"ok": False, "error": "id required"}, 400)
|
||||||
@@ -532,6 +536,23 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
|
|
||||||
# ── Helpers ───────────────────────────────────────────────────────────
|
# ── Helpers ───────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
def _read_json_body(self):
|
||||||
|
"""Безопасное чтение JSON из тела POST-запроса.
|
||||||
|
Возвращает dict или None (если ошибка — уже отправлен 400)."""
|
||||||
|
try:
|
||||||
|
length = int(self.headers.get("Content-Length", 0))
|
||||||
|
except (ValueError, TypeError):
|
||||||
|
self._json({"ok": False, "error": "invalid Content-Length"}, 400)
|
||||||
|
return None
|
||||||
|
if length == 0:
|
||||||
|
return {}
|
||||||
|
try:
|
||||||
|
raw = self.rfile.read(length)
|
||||||
|
return json.loads(raw)
|
||||||
|
except (json.JSONDecodeError, Exception) as e:
|
||||||
|
self._json({"ok": False, "error": f"invalid JSON: {e}"}, 400)
|
||||||
|
return None
|
||||||
|
|
||||||
def _json(self, data, status=200):
|
def _json(self, data, status=200):
|
||||||
self.send_response(status)
|
self.send_response(status)
|
||||||
self._send_cors()
|
self._send_cors()
|
||||||
|
|||||||
@@ -79,9 +79,10 @@ def list_pending(batch_id):
|
|||||||
|
|
||||||
|
|
||||||
def reset_classify_status(batch_id):
|
def reset_classify_status(batch_id):
|
||||||
"""Сбросить classify_status на 'pending' для всех документов батча (включая 'processing' — crash recovery)."""
|
"""Сбросить classify_status на 'pending' только для 'processing' (crash recovery).
|
||||||
|
Уже классифицированные ('classified', 'garbage', 'failed') НЕ трогаем."""
|
||||||
return execute(
|
return execute(
|
||||||
"UPDATE documents SET classify_status='pending', error_message=NULL WHERE batch_id=%s",
|
"UPDATE documents SET classify_status='pending', error_message=NULL WHERE batch_id=%s AND classify_status='processing'",
|
||||||
(batch_id,),
|
(batch_id,),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -251,25 +251,29 @@ class TwoPassObfuscator:
|
|||||||
self._sorted_keys.clear()
|
self._sorted_keys.clear()
|
||||||
|
|
||||||
def _expand_zips(self, files: List[Tuple[str, bytes, str]]) -> List[Tuple[str, bytes, str]]:
|
def _expand_zips(self, files: List[Tuple[str, bytes, str]]) -> List[Tuple[str, bytes, str]]:
|
||||||
"""Распаковать ZIP-файлы, заменив их содержимым. Остальные файлы — как есть."""
|
"""Распаковать ZIP-файлы, заменив их содержимым. Остальные файлы — как есть.
|
||||||
|
Защита от ZIP-бомб: ratio + накопительный размер (как в compare/unzip.py)."""
|
||||||
result = []
|
result = []
|
||||||
for fname, content, ctype in files:
|
for fname, content, ctype in files:
|
||||||
if fname.lower().endswith('.zip'):
|
if fname.lower().endswith('.zip'):
|
||||||
try:
|
try:
|
||||||
with zipfile.ZipFile(io.BytesIO(content)) as zf:
|
with zipfile.ZipFile(io.BytesIO(content)) as zf:
|
||||||
total_size = sum(info.file_size for info in zf.infolist())
|
|
||||||
if total_size > 500 * 1024 * 1024: # 500 MB
|
|
||||||
log.warning("ZIP too large, skipping expansion: %s", fname)
|
|
||||||
result.append((fname, content, ctype))
|
|
||||||
continue
|
|
||||||
if len(zf.infolist()) > 500:
|
if len(zf.infolist()) > 500:
|
||||||
log.warning("ZIP too many files, skipping: %s", fname)
|
log.warning("ZIP too many files, skipping: %s", fname)
|
||||||
result.append((fname, content, ctype))
|
result.append((fname, content, ctype))
|
||||||
continue
|
continue
|
||||||
|
total_uncompressed = 0
|
||||||
for info in zf.infolist():
|
for info in zf.infolist():
|
||||||
if info.is_dir():
|
if info.is_dir():
|
||||||
continue
|
continue
|
||||||
# cp437 → utf8 (как в services/unzip.py)
|
# ZIP bomb: ratio check
|
||||||
|
if info.compress_size > 0:
|
||||||
|
ratio = info.file_size / info.compress_size
|
||||||
|
if ratio > 100:
|
||||||
|
log.warning("ZIP bomb ratio %.0f:1, skipping: %s", ratio, fname)
|
||||||
|
result.append((fname, content, ctype))
|
||||||
|
break
|
||||||
|
# cp437 → utf8 (как в compare/unzip.py)
|
||||||
name = info.filename
|
name = info.filename
|
||||||
try:
|
try:
|
||||||
name = name.encode("cp437").decode("utf-8", errors="replace")
|
name = name.encode("cp437").decode("utf-8", errors="replace")
|
||||||
@@ -280,6 +284,10 @@ class TwoPassObfuscator:
|
|||||||
if not name or name.endswith("/") or ".." in name or "/" in name or "\\" in name:
|
if not name or name.endswith("/") or ".." in name or "/" in name or "\\" in name:
|
||||||
continue
|
continue
|
||||||
inner_data = zf.read(info)
|
inner_data = zf.read(info)
|
||||||
|
total_uncompressed += len(inner_data)
|
||||||
|
if total_uncompressed > 500 * 1024 * 1024: # 500 MB
|
||||||
|
log.warning("ZIP uncompressed limit exceeded, stopping: %s", fname)
|
||||||
|
break
|
||||||
result.append((name, inner_data, ""))
|
result.append((name, inner_data, ""))
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
log.warning("Failed to expand ZIP %s: %s", fname, e)
|
log.warning("Failed to expand ZIP %s: %s", fname, e)
|
||||||
|
|||||||
Reference in New Issue
Block a user