-
Notifications
You must be signed in to change notification settings - Fork 27
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
Limit the triggers we accept #81
Comments
I disagree that such verification should be present in the API. We'd be sending over the entire GitHub context in the API request just for the sake of verification which should have been done in the action in the first place. Note that verifying in 2 different places (action and API) does not provide any advantage either since the logic which couldn't verify this correctly in the action will also fail to verify this in the API. |
We don't have to send the entire context, the trigger is present in the certificate.
We're going to make mistakes on the action side, so having this check server-side is beneficial, I think. The server-side logic can be updated at will, whereas the action needs users to bump the hash/version, so any bugs will take long to be fixed client-side |
We need to test what happens if we receive results for scorecard on a PR branch. We should refuse these requests on the server. (I think it's already taken care of because we check the default branch, but we should verify)
I think we should only accept schedule, push to default and workflow_dispatch events. Verifying these server-side may be useful
/cc @azeemsgoogle @rohankh532
The text was updated successfully, but these errors were encountered: