Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: fork から PR が来たとき esbuild-bundle-analyzer がコケるのを修正 #725

Merged
merged 3 commits into from
Jun 26, 2024

Conversation

exoego
Copy link
Contributor

@exoego exoego commented Jun 2, 2024

GitHub Action のセキュリティ上の制限として、fork からプルリクに対する pull_request イベントでは、GITHUB_TOKENpull-request scope への write 権限が付与されません。

https://docs.github.com/ja/actions/security-guides/automatic-token-authentication
image
...
pull_request_target イベントによってワークフローがトリガーされると、パブリック フォークからトリガーされた場合でも、GITHUB_TOKEN にはリポジトリの読み取り/書き込みアクセス許可が付与されます。 詳しくは、「ワークフローをトリガーするイベント」を参照してください。

esbuild-bundle-analyzer は pull-request にコメントを書き込むため、write 権限が必要です。
fork からのプルリクでも GITHUB_TOKEN に write 権限を与えるためには、pull_request_target という別のイベントを明示する必要があるようです。

注:
この PR は初めての pull_request_target の追加なので、マージされるまで発火しないようです。
このような変更をマージしたあと、fork から来た PR で pull_request_target が発火しているのは exoego/esbuild-bundle-analyzer#43 で確認できます

Copy link

codecov bot commented Jun 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.57%. Comparing base (4147899) to head (d57da56).
Report is 24 commits behind head on main.

Current head d57da56 differs from pull request most recent head e20e21a

Please upload reports for the commit e20e21a to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #725   +/-   ##
=======================================
  Coverage   96.57%   96.57%           
=======================================
  Files          17       17           
  Lines         321      321           
=======================================
  Hits          310      310           
  Misses         11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@exoego exoego marked this pull request as ready for review June 2, 2024 02:13
@momocus momocus requested a review from yonta June 6, 2024 07:00
@yonta yonta assigned yonta and exoego and unassigned yonta Jun 12, 2024
Copy link
Collaborator

@yonta yonta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

わーい、Fork向けに修正ありがとう!

OKです。
質問答えてくれたらマージします!

.github/workflows/esbuild-bundle-analyzer.yml Show resolved Hide resolved
@yonta yonta merged commit 0b20705 into momocus:main Jun 26, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants