self approve を防ぐ
自分で Pull Request にコミットを push して approve する self approve を防ぐ方法を考察します。
概要
本記事では PR をレビューなしに不正にマージする方法とそれの対策について考察しました。
まずマトリックスを組んで不正パターンをもれなく考慮しました。
幾つかのパターンは Branch Rulesets や codeowner などの基本的な対策で防ぐ事ができます。
しかし、基本的な対策だけでは難しい 3 つのパターンが考えられました。
- codeowner 権限を持つ Machine User を悪用して approve
- 自分以外が作成した PR に自分で commit を追加して自分で approve
- 自分以外が作成した PR に Machine User や bot で commit を追加して自分で approve
パターン 2 に関しては GitHub Actions で防ぐことが出来るので、それを紹介します。
1, 3 のように Machine User や bot の悪用を防ぐのは根本的な対策が難しいですが、幾つかの対策を組み合わせることである程度不正を防ぐことができるので、それらについて紹介します。
導入
業務で GitHub を使って Pull Request ベースで team 開発している場合、 Pull Request には他のメンバーの approve を必須とするのが珍しくありません。
review によってコードの品質が高まりますし、他のメンバーへの変更・ナレッジの共有になりますし、誰か一人が悪さをしようとしても approve なしではできなくなります。
しかし Branch Rulesets で approve を必須にしていてもこれをすり抜けることができてしまうケースがあります。
本記事ではこれらの対策を考察します。
マトリックスで不正パターンをもれなく考慮する
Entitiy:
- ユーザー A
- A 以外の ユーザー
- Machine User
- GitHub Actions token
- GitHub App
Action:
- PR 作成
- ユーザー A
- A 以外の ユーザー
- Machine User
- GitHub Actions token: Org の設定で禁止できる
- GitHub App
- commit
- ユーザー A
- A 以外の ユーザー: commit signing を必須にすればなりすましを防げる
- Machine User: commit signing を必須にすれば署名鍵なしでは作成できなくなる
- GitHub Actions token
- GitHub App
- approve
- ユーザー A
- A 以外の ユーザー: 不正じゃない
- Machine User: codeowner にしなければ防げるが、したい場合もある
- GitHub Actions token: codeowner の approve を必須にすれば防げる
- GitHub App: codeowner の approve を必須にすれば防げる
ユーザー A が不正に PR をマージする方法を考えます。
PR 作成 | commit | approve | 結果 |
---|---|---|---|
A | * | Machine User | x Macine User を codeowner にしなければ防げるが、したい場合もある |
A 以外 | A | A | action で committer 以外の approve を強制すれば防げる |
A 以外 | Machine User, GitHub Actions token, GitHub App | A | 防ぐのが難しい |
前提となる対策
まず前提として Branch Rulesets で以下の設定を有効にするとよいでしょう。
-
Require a pull request before merging
: レビューなしで直接 push するのを防ぎますDismiss stale pull request approvals when new commits are pushed
-
Require review from Code Owners
: codeowner のレビューを強制します。 bot は codeowner になれないので、 bot で approve してマージするのを防げます -
Require approval of the most recent reviewable push
: 最新の状態に対するレビューなしでマージするのを防ぎます
-
Require status checks to pass
: CI がパスしてないとマージできないようにします
また、 Organization の設定で Allow GitHub Actions to create and approve pull requests
は有効にしましょう。
また、 GitHub Actions の workflow の改竄を防ぐ方法などについても記事を書いているので参考にしてください。
ちなみに Require approval of the most recent reviewable push
が有効になっていると最新コミットに対して最新コミットの作成者以外の approve が必要になるため、これで十分なのではと思うかもしれません。
しかし、 bot を使ってコミットを追加して approve してしまえばマージできるので対策としては不十分でしょう。
特に bot を使ってコードの自動修正等をやっている場合、簡単に制約を回避できてしまいます。
CI で self approve されているかチェック
self approve か否かの判定のために PR の commit の committer と PR を approve した人 を取得します。
PR の commiter や bot, machine user 以外の人が approve していなかったら self approve とみなします。
また複数人が approve している場合、それらが PR の committer によるものであっても許容するものとします。
GitHub Actions で validation をする job を実行し、その job を required check に追加することで self approve を防ぎます。
当初は Action で self approve を dismiss することも考えましたがやめました。
required check に追加した job を失敗させさえすれば dismiss する必要はありません。
複数人が approve している場合に self approve を許容したいことを考えると dismiss するのは都合が悪く、そういった場合を除外して dismiss するとなると dismiss する条件が複雑で分かりにくくなるのでやらないほうがよいと判断しました。
dismiss しないなら write 権限も不要になります。
deny-self-approve-action
self approve を防ぐための GitHub Action deny-self-approve-action
を作りました。
これは内部的に deny-self-approve
という CLI を実行しています。
以下ではこの action を用いた対策を 2 つ紹介します。
これらは併用するというより片方だけ採用すれば良いでしょう。
- pull_request_review event で self approve を validation
- merge_group event で self approve を validation
対策1. pull_request_review event で self approve を validation
pull_request_review event で workflow を trigger し、 self approve 以外の approve がなければ CI を失敗させます。
そしてこの job を required check に追加します。
name: Require approval
on:
pull_request_review:
types:
- submitted
jobs:
require-approval:
timeout-minutes: 10
runs-on: ubuntu-24.04
permissions:
pull-requests: read # To get a pull request
contents: read # To list commits in pull requests
steps:
- uses: suzuki-shunsuke/deny-self-approve-action@c82acbe810c484d272b5190963b285b06a141f00 # v0.1.1
ただし pull_request_review
workflow は改竄して無効化できてしまいます。
これに対しては以下のような対策が考えられます。
- Push rulesets を使って push 出来る人を制限する
- pull_request_target event で上記の workflow の変更を検知する
pull_request_review
event の難点は無駄に実行されることです。
理想を言えば approve されたときだけ実行されてほしいのですが、 reivew にコメントがついたりしたときも実行されてしまいます。
これは諦めるしかありません。
対策2. merge_group event で self approve を dismiss
merge_group event で workflow を trigger し、 self approve 以外の approve がなければ CI を失敗させます。
name: Require approval
on: merge_group
jobs:
require-approval:
timeout-minutes: 10
runs-on: ubuntu-24.04
permissions:
pull-requests: read
contents: read
steps:
- uses: suzuki-shunsuke/deny-self-approve-action@c82acbe810c484d272b5190963b285b06a141f00 # v0.1.1
require-approval
を required check に追加した場合、 pull_request または pull_request_target event でも同名の job を実行する必要があります。
そうしないと PR を Merge Queue に追加できません。
そこで以下のような job を追加します。
name: Test
on: pull_request
jobs:
require-approval:
timeout-minutes: 10
runs-on: ubuntu-24.04
permissions: {}
if: "false" # The job is always skipped.
steps:
- run: ":"
この job は常に skip するようにします。 job を skip しても required checks は pass します。
こうすれば self approve で Merge Queue に追加されてもマージされるのを防ぐことができます。
ただし、 merge_group でも pull_request_review のときと同様に改竄対策は必要でしょう。
GitHub User と link しない Commit
Git の設定で GitHub User の email を設定していないと commit が GitHub User に link しません。
GitHub User に link しない commit は誰がコミットしたのかわからないので self approve かどうかの判定ができません。
deny-self-approve は GitHub User に link しない commit があると失敗します。
その場合 email を正しく設定してください。
Required Workflow によって GitHub Organization 全体で self approve を防ぐ
Github Organization の Branch Ruleset で Required Workflow を設定し、 Org 全体で self approve を防ぐことができます。
Required Workflow を使えばリポジトリごとに個別に Workflow や Ruleset を設定する必要がないですし、 Workflow を改竄するのも難しくなります。
bot や Machine User による不正な commit の対策
deny-self-approve によって自分以外が作成した PR に自分でコミットを追加して自分で approve するのを防ぐことができます。
一方、自分以外が作成した PR に bot や Machine User で任意のコミットを生成し自分で approve するのは防げません。
これについては正直現状完全な対策が思いつきません。
ここでは現状で思いつく対策を幾つか書き連ねます。
commit signing を必須にすると Machine User でコミットをするハードルが上がるため、 Machine User の access token を悪用して commit を生成することをある程度防げます。
他には以下のような対策が考えられます。
- 公式の Renovate App のように信頼できる App 以外の bot や Machine User のコミットを含む PR には複数人の approve がなければ CI を失敗させる
- 信頼できる App 以外の bot や Machine User の commit を validation する。特定のファイルしか変更できなくしたり GitHub Actions token での commit を禁止させたり、特定の branch にしか push できなくする (CI を失敗させる)
- Branch Rulesets で特定の bot しか push できないようにする
1 は結構厳しくて開発者体験を損なう可能性が高いと思います。
2 に関しては例えば GitHub Actions token による commit を禁止するのは良さそうです。
特定のリポジトリには特定の App 以外の bot は commit できないようにするのも良いでしょう。
特定のファイルしか変更できなくするのも悪くはないですが、 App を使って自動で広範囲にフォーマットをかけるようなユースケースを考えるとちょっと難しいかもしれません。
Private Key の管理が雑な App や Access Token の扱いが雑な Machine User のコミットは禁止し、
きちんと管理された App からのみコミットを許可するのが良いでしょう (まぁきちんと管理する
のが難しいのですが)。
Access Token や App の Private Key の管理
Access Token や App の Private Key の管理には AWS SecretsManager のようなサービスで管理して OIDC でアクセス制御するのが良いでしょう。
また、 Mercari の Token Server のような形で App の Private Key を中央集権的に管理し Private Key の流出を防ぎつつアクセスや権限制御をかけるのも良いです。
Octo STS のようなサービスもあります。
ただし、 Mercari の Token Server のような仕組みを自前で構築するのはけっこう大変ですし、 Octo STS にも課題はあります。
これらを整備したからといって対策が完璧というわけではないですし、中々対策は一筋縄ではいきません。
さいごに
自分で Pull Request にコミットを push して approve する self approve を防ぐ方法を紹介しました。
今回紹介した通り、自分以外が作成した PR に自分でコミットを追加して自分で approve するのは workflow で防ぐことができます。
一方、自分以外が作成した PR に bot や Machine User で任意のコミットを生成し自分で approve するのを防ぐのは容易ではありません。
完璧な対策は難しいですが、様々な対策を掛け合わせることで不正を防げる確率を高めるしかありません。
より再現性の高い画一的な対策を検討していきたいと思います。
Discussion