Проблема именно с merge-коммитами, потому что в GitHub UI можно редактировать конфликты прямо в браузере, добавляя extra code. При rebase + fast-forward конфликты разрешаются локально, и такой фокус не пройдёт.
В статье я допустил неточную формулировку про скролл. Суть проблемы не в этом. Дело в том, что GitHub не показывает вообще строку в PR-diff. Это не про длинные строки, а про то, что изменения, добавленные в самом merge-коммите (поверх результата слияния), не отображаются в PR. GitHub показывает diff: feature branch vs main. GitHub не показывает: что было добавлено в merge-коммите после слияния.
Спасибо за уточнение, я обновил формулировку в статье.
Вот полный пример: https://github.com/fimskiy/evil-merge-demo В PR виден только "superuser", но в итоговом файле в main есть оба токена — "hacked" и "superuser". Можешь сам посмотреть merge-коммит и убедиться.
Да, это усложняет атаку в GitHub, но полностью проблему не убирает. При достаточных правах правила можно обойти. И это не защищает от уже существующих изменений в истории.
Rebase + fast-forward полностью закрывает эту атаку — merge-коммитов нет, вектора нет. Чистая история как бонус. Если команда может себе это позволить, это лучшее решение. Проблема в том, что большинство крупных проектов используют merge-коммиты — GitHub создаёт их по умолчанию, и далеко не все меняют эту настройку. Для них и нужен инструментальный контроль.
Именно так. Классический сценарий: ревьюер одобрил PR, попросил только разрешить конфликты — и после этого повторного ревью не требуется. Атакующий в момент разрешения конфликта добавляет произвольный код в merge-коммит, и этот код уходит в main без чьего-либо взгляда.
▎ Что интересно — решение, которое GitHub упомянул как возможное направление (“may make this functionality more strict”), как раз и означало бы: после изменений в merge-коммите аппрув сбрасывается и требуется повторное ревью. Пока этого нет, единственная защита — инструментальная проверка самого merge-коммита.
При разрешении конфликта все изменения видны в diff PR’а — ревьюер может их проверить. Evil merge — это изменения, которых нет ни в одной из веток-родителей и которые не являются конфликтом. Они добавляются напрямую в merge-коммит после того, как git уже сделал слияние. GitHub в таком случае показывает в PR только diff относительно base branch — и эти изменения в нём не видны совсем.
Проблема именно с merge-коммитами, потому что в GitHub UI можно редактировать конфликты прямо в браузере, добавляя extra code. При rebase + fast-forward конфликты разрешаются локально, и такой фокус не пройдёт.
В статье я допустил неточную формулировку про скролл. Суть проблемы не в этом. Дело в том, что GitHub не показывает вообще строку в PR-diff. Это не про длинные строки, а про то, что изменения, добавленные в самом merge-коммите (поверх результата слияния), не отображаются в PR.
GitHub показывает diff: feature branch vs main.
GitHub не показывает: что было добавлено в merge-коммите после слияния.
Спасибо за уточнение, я обновил формулировку в статье.
Вот полный пример: https://github.com/fimskiy/evil-merge-demo
В PR виден только "superuser", но в итоговом файле в main есть оба токена — "hacked" и "superuser". Можешь сам посмотреть merge-коммит и убедиться.
Да, это усложняет атаку в GitHub, но полностью проблему не убирает. При достаточных правах правила можно обойти. И это не защищает от уже существующих изменений в истории.
Rebase + fast-forward полностью закрывает эту атаку — merge-коммитов нет, вектора нет. Чистая история как бонус. Если команда может себе это позволить, это лучшее решение. Проблема в том, что большинство крупных проектов используют merge-коммиты — GitHub создаёт их по умолчанию, и далеко не все меняют эту настройку. Для них и нужен инструментальный контроль.
Именно так. Классический сценарий: ревьюер одобрил PR, попросил только разрешить конфликты — и после этого повторного ревью не требуется. Атакующий в момент разрешения конфликта добавляет произвольный код в merge-коммит, и этот код уходит в main без чьего-либо взгляда.
▎ Что интересно — решение, которое GitHub упомянул как возможное направление (“may make this functionality more strict”), как раз и означало бы: после изменений в merge-коммите аппрув сбрасывается и требуется повторное ревью. Пока этого нет,
единственная защита — инструментальная проверка самого merge-коммита.
При разрешении конфликта все изменения видны в diff PR’а — ревьюер может их проверить. Evil merge — это изменения, которых нет ни в одной из веток-родителей и которые не являются конфликтом. Они добавляются напрямую в merge-коммит после того, как git уже сделал слияние. GitHub в таком случае показывает в PR только diff относительно base branch — и эти изменения в нём не видны совсем.
Выглядит, как фантастика, но легко воспроизводится