diff --git a/polygon-docs/sonnet-app-autotest-review-v1.2.39.md b/polygon-docs/sonnet-app-autotest-review-v1.2.39.md new file mode 100644 index 0000000..5ca2384 --- /dev/null +++ b/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 => + `` +).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 в ротации | 🟢 |