145 lines
11 KiB
Markdown
145 lines
11 KiB
Markdown
Read [](file:///home/naeel/nubes/contracts/contractor/apply_events.cfm)
|
||
|
||
Read [](file:///home/naeel/nubes/contracts/contractor/db.cfc#1-1), lines 1 to 150
|
||
|
||
Read [](file:///home/naeel/nubes/contracts/contractor/deploy/llm_prompt.py)
|
||
|
||
Read [](file:///home/naeel/nubes/contracts/contractor/deploy/convert_server.py#1-1), lines 1 to 150
|
||
|
||
Read [](file:///home/naeel/nubes/contracts/contractor/deploy/convert_server.py#150-150), lines 150 to 250
|
||
|
||
Проанализировал все файлы. Вот подробный аудит.
|
||
|
||
---
|
||
|
||
## Аудит Event Sourcing — сверка договоров
|
||
|
||
---
|
||
|
||
### Находки Gemini — подтверждение/опровержение
|
||
|
||
**1. Race condition `MAX(seq)` — ✅ ПОДТВЕРЖДЕНО, критично**
|
||
|
||
apply_events.cfm: `SELECT COALESCE(MAX(seq), 0)` → `seq++` в цикле — это read-modify-write без блокировки. Два параллельных вызова (разные supplements) получат одинаковый `MAX(seq)` и сгенерируют одинаковые seq. В DDL есть `UNIQUE(contract_id, seq)` — транзакция упадёт с ошибкой дублирующегося ключа.
|
||
|
||
**Рекомендация:** Заменить на `nextval` через sequence (`CREATE SEQUENCE spec_events_seq`) или PostgreSQL `INSERT ... RETURNING` с `seq = (SELECT COALESCE(MAX(seq),0)+1 FROM spec_events WHERE contract_id=... FOR UPDATE)` — с `FOR UPDATE` для пессимистической блокировки.
|
||
|
||
---
|
||
|
||
**2. `full_replace` с пустым `ops` стирает спецификацию — ✅ ПОДТВЕРЖДЕНО, критично**
|
||
|
||
apply_events.cfm: Если LLM вернул `mode=full_replace` с пустым `ops[]` (например, не распознал таблицу), код сначала DELETE всех строк из `spec_current`, потом цикл по ops не выполняется. Спецификация обнулена без восстановления.
|
||
|
||
**Рекомендация:** Добавить guard: если `mode == full_replace` и `len(ops) == 0` — отклонить с ошибкой, не трогать `spec_current`.
|
||
|
||
---
|
||
|
||
**3. LLM возвращает строку вместо числа → `cf_sql_float` падает — ✅ ПОДТВЕРЖДЕНО**
|
||
|
||
apply_events.cfm: Переменные `pr`, `qt`, `sm` берутся напрямую из `op.new_row`. Промпт в llm_prompt.py говорит «ЧИСЛА, не строки», но LLM может вернуть `"1 000,00"` или `"null"` как строку. `cf_sql_float` с нечисловой строкой кидает исключение внутри транзакции — вся транзакция откатывается.
|
||
|
||
**Рекомендация:** Перед передачей в cfqueryparam привести к числу через `val()` или попытаться `javacast("double", pr)` с catch.
|
||
|
||
---
|
||
|
||
**4. N+1 SELECT в цикле UPDATE — ✅ ПОДТВЕРЖДЕНО**
|
||
|
||
apply_events.cfm: Для каждого UPDATE-op выполняется отдельный `SELECT price, qty, sum, date_start FROM spec_current WHERE name_hash=?`. Если ДС обновляет 50 строк — 50 SELECT-запросов внутри одной транзакции.
|
||
|
||
**Рекомендация:** Перед циклом собрать все `target_hash` UPDATE-ops, сделать один `SELECT ... WHERE name_hash = ANY(ARRAY[...])`, сложить результат в struct по hash.
|
||
|
||
---
|
||
|
||
**5. Построчный INSERT в `full_replace` — ✅ ПОДТВЕРЖДЕНО**
|
||
|
||
apply_events.cfm: В `full_replace`-блоке цикл по `curRows` выполняет по одному `INSERT INTO spec_events` на каждую строку. После — снова цикл по `ops`, каждый ADD — ещё 2 INSERT. Для 100-строчной спецификации: 100+100×2 = 300 отдельных INSERT внутри одной транзакции.
|
||
|
||
**Рекомендация:** Использовать `INSERT INTO spec_events ... SELECT unnest(...)` или сформировать multi-row VALUES через ColdFusion loop перед запросом.
|
||
|
||
---
|
||
|
||
**6. `name_hash` зависит от LLM-форматирования — ✅ ПОДТВЕРЖДЕНО, фундаментальная проблема**
|
||
|
||
apply_events.cfm: `md5(lower(trim(name)) || coalesce(date_start, ''))` вычисляется на стороне PostgreSQL из данных, которые LLM только что вернул. В llm_prompt.py: в `_build_diff` LLM видит `[hash: {hash}]` и должен вернуть этот же hash в `target_hash`. Но LLM может сократить название, изменить регистр или пробелы → `md5` будет другим → UPDATE-op не найдёт строку → `cfthrow "UPDATE target not found"`.
|
||
|
||
**Рекомендация:** Это структурная проблема. Варианты:
|
||
- Передавать LLM только `hash` как непрозрачный идентификатор (уже так и делается), но добавить fuzzy-matching на стороне apply: если `target_hash` не найден — искать по близкому имени и создавать UNRESOLVED вместо throw.
|
||
- Добавить отдельный endpoint для ручного разрешения UNRESOLVED.
|
||
|
||
---
|
||
|
||
**7. `last_event_id` UUID → `cf_sql_varchar` — ✅ ПОДТВЕРЖДЕНО**
|
||
|
||
apply_events.cfm: `<cfqueryparam value="#evt.id#" cfsqltype="cf_sql_varchar">` для колонки `last_event_id UUID`. Lucee отправит строку, PostgreSQL неявно приведёт к UUID — это работает, но неправильно. При включённом строгом режиме или нестандартном JDBC-драйвере может дать ошибку. Стоит использовать `cfsqltype="cf_sql_char"` с `maxlength="36"` или более явный тип.
|
||
|
||
---
|
||
|
||
### Упущенные проблемы
|
||
|
||
**8. SQL-инъекция в `_lucee_query` — КРИТИЧНО**
|
||
|
||
convert_server.py: Метод `_lucee_query` принимает готовую SQL-строку, которую формируют через f-string интерполяцию. Например:
|
||
```python
|
||
f"... WHERE s.contract_id='{cid}' ..."
|
||
```
|
||
`cid` приходит из GET-параметра `?contract_id=...` без какой-либо санитизации. Если атакующий передаст `cid = "'; DROP TABLE spec_current; --"` — запрос выполнится.
|
||
|
||
**Рекомендация:** Передавать `contract_id` как параметр через `params` API Lucee, а не конкатенацией в строку. Либо хотя бы проверять `cid` по regex UUID: `re.fullmatch(r'[0-9a-f-]{36}', cid)`.
|
||
|
||
---
|
||
|
||
**9. `elements_json` double-decode — баг надёжности**
|
||
|
||
convert_server.py:
|
||
```python
|
||
ej = docs[0]["elements_json"]
|
||
if isinstance(ej, dict) and "Value" in ej:
|
||
ej = ej["Value"]
|
||
elements = json.loads(ej)
|
||
```
|
||
Если Lucee вернул `elements_json` уже как распарсенный dict (без `"Value"` обёртки), то `json.loads(dict)` упадёт с `TypeError`. Условие `isinstance(ej, dict)` обрабатывает только Lucee-обёртку, но если `ej` уже список — код падает.
|
||
|
||
**Рекомендация:** `if isinstance(ej, (dict, list)): elements = ej if isinstance(ej, list) else json.loads(ej["Value"])`.
|
||
|
||
---
|
||
|
||
**10. `full_replace` DELETE не в правильном порядке в аудит-логе**
|
||
|
||
apply_events.cfm: DELETE-события для old строк записываются с `seq++` начиная с 0, затем ADD-события из `ops` продолжают seq. Это корректно. НО: если replay этих событий — сначала DELETE всего, потом ADD новых. Проблема: `UNIQUE(contract_id, seq)` не гарантирует порядок применения при replay, если seq не строго монотонный из-за прошлых partial-операций. Нет механизма replay/восстановления из событий.
|
||
|
||
**Рекомендация:** Либо добавить флаг `is_replayable` в архитектурный документ (явно задокументировать что replay не поддерживается), либо добавить `applied_at TIMESTAMPTZ` и `parent_seq` для правильного replay.
|
||
|
||
---
|
||
|
||
**11. LLM-ключ в открытом виде в коде**
|
||
|
||
convert_server.py: `LLM_KEY = "sk-ucI5YvOticoOQ9Kuj5K9mQ"` — hardcoded в файле. При любом git push ключ попадёт в историю.
|
||
|
||
**Рекомендация:** Перенести в переменную окружения `os.environ.get("LLM_KEY")`.
|
||
|
||
---
|
||
|
||
**12. Нет идемпотентности — повторный вызов дублирует данные**
|
||
|
||
Нет проверки что `supplement_id` уже обработан. Если `/process-v2` вызвать дважды для одного контракта — все operations применятся повторно: ADD-строки задублируются (или упадут на `UNIQUE name_hash`), seq увеличится.
|
||
|
||
**Рекомендация:** Добавить `status` колонку в `supplements` (`processed` / `pending`) и проверять в начале цикла. Либо проверять `SELECT COUNT(*) FROM spec_events WHERE supplement_id = ?` перед обработкой.
|
||
|
||
---
|
||
|
||
### Итоговая таблица приоритетов
|
||
|
||
| # | Проблема | Критичность |
|
||
|---|----------|-------------|
|
||
| 8 | SQL-инъекция в `_lucee_query` | 🔴 Критично |
|
||
| 1 | Race condition MAX(seq) | 🔴 Критично |
|
||
| 2 | full_replace + empty ops → потеря данных | 🔴 Критично |
|
||
| 12 | Нет идемпотентности | 🟠 Высокая |
|
||
| 6 | name_hash нестабилен | 🟠 Высокая |
|
||
| 3 | LLM строка вместо числа → crash | 🟠 Высокая |
|
||
| 11 | LLM-ключ в коде | 🟡 Средняя |
|
||
| 9 | elements_json double-decode | 🟡 Средняя |
|
||
| 4 | N+1 SELECT в UPDATE | 🟡 Средняя |
|
||
| 5 | Построчный INSERT в full_replace | 🟢 Низкая |
|
||
| 7 | UUID как cf_sql_varchar | 🟢 Низкая |
|
||
| 10 | Нет механизма replay | 🟢 Низкая | |