[WIP] Accept Github webhooks payload #34
Conversation
It turns out that `:repository/:name` in the webhook payload doesn't include the user or organization prefix. Rather than try to reconstruct it myself, I'll just include the project id in the callback URL.
We might define 12 fixtures; we probably won't define 9999999.
| # POST /webhooks/receive | ||
| def receive | ||
| push = JSON.parse(params[:payload]) | ||
| p = Project.where(id: params[:project_id]).take |
There was a problem hiding this comment.
How come you need this .take call at the end if you’re querying by id? Surely id is a primary key so only one result is ever going to be returned?
There was a problem hiding this comment.
Yep yep, it is unique, but .where returns an association so we need the .take to pull that (single) result.
I'm using .where().take here because it returns nil if the project id isn't valid, rather than throwing a RecordNotFound exception like Project.find(project_id). By default RecordNotFound renders a 404 with an HTML page, which felt wrong for an API response.
This does feel kind of awkward, though... I think there's a way to control what gets rendered on exceptions, on a per-controller basis, so that would be cleaner. I'll look into it 🔍
There was a problem hiding this comment.
Nah, that’s fine and that actually makes a ton of sense. I just didn’t realise that you needed a single result object here instead of an AR association/relation. I wouldn’t worry about looking into it further 👍
|
💖 This is awesome! On the security side, I think it might be worth (at least in the short term) implementing some kind of |
|
Hmmm, checking a secret token might let us "verify" that requests are coming from the right source, but that doesn't buy us much. An attacker who knows your DCIY's url could maybe DDoS you by running expensive builds over and over again, but the bigger issue is the code that's actually run as part of the build itself. All someone has to do is submit a pull request that adds a spec: it "wipes your hard drive clean"... and you're sunk! You can't even really whitelist PRs, as there's nothing stopping someone from adding bad code to the branch after the fact when you're not looking. (Er, no idea what I was thinking way back at the start with whitelisting commands.) You'd mentioned using Vagrant and/or Docker to sandbox builds -- I think that's ultimately going to have to be the way we go. |
Fair enough. Yeah, sandboxing is something I’d really like to make possible eventually anyway, so if that means we can just bank on that then 👍 At the end of the day, if you’re really worried about the security implications here, people should just A) not run it on a machine they care about or have sensitive information on, and/or B) kill the server process when they don’t need it running. |
This is the first step in implementing #5. Add a webhook to your repository with a URL in the following format:
... and DCIY will automatically build branches when they're pushed. Pretty neat! It's starting to feel like a proper CI, now.
This is the very core functionality for that ticket, though; it's pretty unrefined. What's left to do:
projects/:id/builds.