doc: code review Соннета — app-autotest v1.2.39 (10 находок)

polygon-docs/sonnet-app-autotest-review-v1.2.39.md

@@ -0,0 +1,132 @@
+# Code Review: app-autotest v1.2.39 — Соннет
+
+Дата: 2026-08-03
+
+---
+
+## 🔴 Критические
+
+**1. XSS в instances.js — `o.operation` в innerHTML без `_esc()`**
+
+instances.js (функция `toggleInstance`):
+```js
+opsEl.innerHTML = ops.map(o =>
+  `<button ... onclick="runOp('${o.operation}',${o.svcOperationId})">${o.operation}</button>`
+).join('');
+```
+`o.operation` вставляется **три раза без `_esc()`**: в onclick-атрибут (`'${...}'`), в текст кнопки (`>${...}<`). Если API вернёт `operation = "'; alert(1)//"` — onclick-атрибут ломается. На практике операции — это "modify"/"delete" из Nubes API, но принцип нарушен. `_esc()` определён специально для этого.
+
+---
+
+## 🟡 Важные
+
+**2. Двойной вызов `detect_endpoint()` при каждом HTTP-запросе**
+
+auth.py — `get_client()` вызывает `detect_endpoint(token)`, и `get_stand()` вызывает `detect_endpoint(token)` независимо. В api_test.py при `POST /api/test`:
+```python
+client = get_client()   # detect_endpoint() #1
+...
+get_client_id(), get_stand()   # detect_endpoint() #2
+```
+Итого **2 лишних HTTP-запроса к Nubes API** (dev + test) при каждом запросе в облачном режиме. Оба блока `if endpoint in STANDS` срабатывают при стандартной конфигурации.
+
+**3. `mode` и `polygon_stand` не валидируются при `set_mode`**
+
+main.py:
+```python
+new_mode = request.form.get("mode", "polygon")
+new_stand = request.form.get("polygon_stand") or request.cookies.get("polygon_stand", "test")
+```
+Любое значение пишется в cookie. Если `polygon_stand = "evil/../../../etc"`, то в `get_client()` формируется URL:
+```
+polygon_url.replace("/api/v1/svc", "/evil/../../../etc/api/v1/svc")
+```
+Путь нормализуется HTTP-клиентом/сервером. Допустимые значения известны: `mode` ∈ {"polygon","cloud"}, `polygon_stand` ∈ {"dev","test","prod"} — надо добавить whitelist.
+
+**4. В polygon-режиме у всех пользователей одинаковый `client_id`**
+
+auth.py:
+```python
+def get_token():
+    if get_mode() == "polygon":
+        return current_app.config["NUBES_API_TOKEN"]  # env-токен
+```
+`get_client_id()` тоже парсит env-токен → у всех пользователей в эмуляции одинаковый ClientID. Вся изоляция данных в БД (`runs`, `scenario_runs`, `scenario_definitions`) — по `(client_id, stand)`. Если два пользователя переключатся в polygon-режим — они видят историю и сценарии **друг друга**. Если приложение использует только один человек — не проблема. Если несколько — критично.
+
+**5. `api_history` — соединение не в `finally`**
+
+api_test.py:
+```python
+try:
+    conn = get_conn()
+    cur = conn.cursor()
+    cur.execute(...)
+    ...
+    cur.close()
+    put_conn(conn)        # явный return соединения
+    return jsonify(result)
+except Exception as e:
+    try: cur.close()      # NameError если исключение до cur = ...
+    except: pass
+    try: put_conn(conn)   # NameError если исключение до conn = ...
+    except: pass
+    return jsonify({"error": str(e)}), 500
+```
+Все остальные функции (`api_scenario_run_status`, `api_scenario_status`) используют `finally: put_conn(conn)` — корректный паттерн. Здесь — нет. При ошибке до `cur.close()` соединение возвращается, но `cur` не закрывается. Не утечка (psycopg2 закроет при возврате conn в пул), но несоответствие стилю.
+
+**6. `stand_name()` — подстрочный поиск**
+
+http_client.py:
+```python
+for name in ("dev", "test"):
+    if name in (endpoint or ""):
+        return name
+```
+Если URL содержит "dev" как часть другого слова (например, "development", "devnull"), вернёт неверное значение. URL-то сейчас конкретные, но хрупко.
+
+---
+
+## 🟢 Рекомендации
+
+**7. Двойной отступ в scenario.py**
+
+scenario.py:
+```python
+for i, step in enumerate(steps):
+        step_num = i + 1  # 8 пробелов вместо 4
+```
+Весь цикл имеет нестандартный отступ (8 пробелов). Работает корректно (Python), но выглядит как след удалённого `try:` или `with:` блока, который был внутри for.
+
+**8. `get_real_client()` — мёртвый алиас**
+
+auth.py: функция идентична `get_client()`, есть только для обратной совместимости. В main.py и других файлах есть вызовы `get_real_client()`. Стоит постепенно заменить на `get_client()` и убрать алиас.
+
+**9. Нет `UNIQUE` на `runs.op_uid`**
+
+init_db.py: ручной UPSERT в save_run.py (UPDATE → INSERT) предполагает уникальность `op_uid`. Индекса нет. При маловероятной гонке двух параллельных `save_run` с одним `op_uid` — оба сделают UPDATE (rowcount=0) → оба сделают INSERT → дубль. Добавить `CREATE UNIQUE INDEX IF NOT EXISTS idx_runs_op_uid ON runs (op_uid) WHERE op_uid IS NOT NULL`.
+
+**10. Ротация лога: `flock(LOCK_UN)` перед flush буфера**
+
+api_test.py:
+```python
+f.write(rest)
+fcntl.flock(f, fcntl.LOCK_UN)  # лок снят, но буфер ещё не сброшен
+```
+Python-буфер сбрасывается при закрытии файла (`with`-блок), но лок уже снят. Другой воркер может прочитать неполные данные. Добавить `f.flush()` перед `LOCK_UN`.
+
+---
+
+## Итого по приоритетам
+
+| # | Файл | Проблема | Серьёзность |
+|---|------|----------|-------------|
+| 1 | `static/js/instances.js` | `o.operation` в innerHTML без `_esc()` | 🔴 |
+| 2 | `api/auth.py` | Двойной `detect_endpoint()` на запрос | 🟡 |
+| 3 | `routes/main.py` | `mode`/`polygon_stand` без whitelist-валидации | 🟡 |
+| 4 | `api/auth.py` | Shared `client_id` в polygon-режиме | 🟡 |
+| 5 | `routes/api_test.py` | `api_history` без `finally` | 🟡 |
+| 6 | `api/http_client.py` | `stand_name()` substring match | 🟡 |
+| 7 | `operations/scenario.py` | Двойной отступ в for-цикле | 🟢 |
+| 8 | `api/auth.py` | `get_real_client()` мёртвый алиас | 🟢 |
+| 9 | `db/init_db.py` | Нет UNIQUE индекса на `runs.op_uid` | 🟢 |
+| 10 | `routes/api_test.py` | `flock(LOCK_UN)` перед flush в ротации | 🟢 |