11 KiB
Проанализировал все файлы. Вот подробный аудит.
Аудит 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 интерполяцию. Например:
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:
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 | 🟢 Низкая |