search

SciTools-incubator / scitools-cla-checker

A Heroku service that checks whether a PR is covered by a completed CLA
BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

Verify PR state before modification #10

Closed QuLogic closed 6 years ago

QuLogic commented 6 years ago

If you look at SciTools/cartopy#123, @pelson has marked it as needing a CLA. Lack of Met Office employees in the contributors.json aside, this is because the code checks the webhook data and not actual PR status.

It's not a major security bug, but it does allow someone (me in this case) to trigger unnecessary label changes.

pelson commented 6 years ago

Lack of employees resolved in: https://github.com/SciTools/scitools.org.uk/pull/160

I've resolved this issue by ensuring the request body is validated in the digest. The only people who may now send a webhook event are github, and we trust that github sends a valid payload. The code does currently check that the payload tells us the PR is open.

Thanks @QuLogic.

pelson commented 6 years ago

Incidentally, I updated that PR from the CLI (once the Met Office employees were added to the contributors.json file) with:

python -m scitools_cla_checker.update_pr scitools/cartopy 123

TOKEN environment variable needed, as described in the README