scorecard-webapp icon indicating copy to clipboard operation
scorecard-webapp copied to clipboard

Limit the triggers we accept

Open laurentsimon opened this issue 3 years ago • 2 comments

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

laurentsimon avatar Apr 22 '22 17:04 laurentsimon

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.

azeemshaikh38 avatar Apr 22 '22 17:04 azeemshaikh38

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.

We don't have to send the entire context, the trigger is present in the certificate.

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'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

laurentsimon avatar Apr 22 '22 17:04 laurentsimon