Один кошелёк, три написания
Библиотека, через которую сервис ходит к платёжному провайдеру, умеет три денежные операции из консоли: вывести крипту, обменять валюту и собрать нашу комиссию. Перед выплатой команда смотрит, не было ли уже выплаты по этому кошельку, и если была — просит набрать фразу «pay twice». Защита честная, пока терминал один.
Два терминала
Проверка стоит в начале запуска, а approve уходит через несколько минут, после диалога. Два оператора на одном кошельке оба видят «выплат не было», оба создают ордер, оба одобряют. Фразу не спросил никто.
Закрыл это замком на весь запуск: неблокирующий flock на файл рядом с состоянием кошелька. Второй запуск получает отказ сразу, ещё до того как что-то создал; процесс умер — ядро замок сняло, чистить нечего. Имя файла строится из идентификатора кошелька, и я привёл его к нижнему регистру: один uuid, набранный заглавными, — тот же кошелёк. Три линзы ревью, мутации, зелёный прогон. Я считал вопрос закрытым.
Что показал замер
Одна из линз заметила: регистр приведён, а дефисы нет. Уязвимо, если провайдер примет uuid без дефисов. Проверить это можно только у самого провайдера, и я спросил его баланс одного кошелька тремя способами: как положено, без дефисов и заглавными.
Все три ответили одинаково — два баланса.
То есть для провайдера это один кошелёк, а для моего замка — два разных файла. Оператор, набравший id без дефисов, получал пустое состояние, отдельный замок и выплату без всякой фразы. Защита, которую я только что закончил, обходилась одним удалённым символом, и обходилась не хитро, а случайно: скопировал id из таблицы, где дефисов нет, — и готово.
Ошибка не в регулярке. Ошибка в том, чей ключ. Я нормализовал идентификатор так, как считал правильным сам, а повтор определяется тем, как его нормализует тот, у кого лежат деньги. Правка поэтому не «убрать дефисы из ключа» — тогда поехали бы имена уже лежащих файлов состояния, — а отказ до первого запроса, если кошелёк не записан каноническим uuid.
Та же ошибка ещё два раза
Исправление снова ушло на ревью, одной линзой. Она нашла не обход замка, а тот же класс в месте, куда я не смотрел.
Комиссия с депозитов считается по списку ордеров, который вводит оператор, и внутри одного запуска команда не даёт ввести один ордер дважды. Сверка шла по тому, что набрано, а в список клалось то, что ответил провайдер. Введи ордер один раз с дефисами, второй — без, провайдер оба раза вернёт одну и ту же запись, и база комиссии удвоится: сорок вместо двадцати. Следующий запуск это поймал бы — там сверка идёт по ответам с обеих сторон, — но деньги уйдут в этом. Линза воспроизвела это на моке, я подтвердил чтением и закрыл сверкой по ответу.
Третий раз был раньше, на ревью большого куска того же дня, и про статус, а не про написание. После потерянного ответа на создание ордера команда при следующем запуске ищет его по своему ключу. Нашла в статусе «новый» — отказала и записала в журнал «новый». Следующий запуск видел уже не «неподтверждённый заряд», а обычную запись и шёл дальше. У комиссии за конвертацию сверять повтор не по чему, так что создавался второй ордер рядом с висящим первым. Это нашли две линзы независимо: одна экспериментом, другая мутацией — выключила ветку и увидела, что тесты остались зелёными.
Все три случая одного устройства: признак, по которому я решал «это уже было», не совпадал с тем, по которому это «уже было» у провайдера. Регистр против дефисов, ввод против ответа, «записали» против «живой статус».
Что поймала машина, а не я
За сутки в ревью участвовало тринадцать агентов: три линзы и два опровергающих на замок, четыре линзы на план по ответам провайдера, три на код, одна на исправление с uuid. Опровергающие нужны, чтобы правдоподобная находка не шла в работу без проверки, и три находки они действительно сняли.
Одна поймала меня на неправде в собственном changelog. Я написал, что старый разбор тел запросов обрывался на первой ] и поэтому не видел поле documents. Линза прогнала старый разбор и показала: documents он видел. Скобка стояла сразу после него, и невидимым оказалось бы только следующее поле — тот самый reference, который я как раз добавлял. Ошибка в формулировке, но это ровно та ошибка, которую потом кто-нибудь процитирует как факт.
Две другие поймали провайдера, а не меня. В спецификации написано, что заголовок тенанта урезает каталог типов ордеров до тех, у которых заведены ставки; представитель провайдера написал то же самое. На песочнице каталог с заголовком и без совпал — 39 типов и 39. И формулу комиссии «общая = тарифная + газ» подтвердить нечем: все 30 крипто-валют песочницы ответили с нулевым газом. Поэтому проверка котировки принимает оба прочтения, а заголовок уходит только туда, где без него запрос падает.
Что можно унести
Ключ защиты от повтора — это нормализация того, у кого лежат деньги, а не твоя. Прежде чем написать «уже было», стоит спросить себя: а если тот же объект придёт другим написанием, другим регистром, другим статусом — узнает ли его проверка? Ответ дешевле всего получить у самого провайдера: три запроса на один баланс стоили минуту и перевернули вывод, в котором я был уверен.
Модуль мы в тот же день закрыли. Когда я в очередной раз предложил список, что брать дальше, получил: «:)))) Я уже сегодня говорил к чему стремимся. Предложи как уже закончить с модулем то?» Справедливо. Последний обход закрыли, 660 тестов, из красных только известное ограничение песочницы, тег стоит.