From 6d4c53b0e316ff6edfb6ff11aa70019c2f9048ff Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sun, 8 May 2022 11:37:53 +0200 Subject: [PATCH 1/7] Add `webhook-github` endpoint, redirect and deprecate `push-github` --- src/lib/Hydra/Controller/API.pm | 50 +++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/src/lib/Hydra/Controller/API.pm b/src/lib/Hydra/Controller/API.pm index 6f10ef575..29d65b744 100644 --- a/src/lib/Hydra/Controller/API.pm +++ b/src/lib/Hydra/Controller/API.pm @@ -267,24 +267,56 @@ sub push : Chained('api') PathPart('push') Args(0) { ); } -sub push_github : Chained('api') PathPart('push-github') Args(0) { +sub webhook_github : Chained('api') PathPart('webhook-github') Args(0) { my ($self, $c) = @_; $c->{stash}->{json}->{jobsetsTriggered} = []; my $in = $c->request->{data}; + + # every GitHub webhook payload has the `repository` key and a `X-GitHub-Event` header + my $event = $c->req->header('X-GitHub-Event') or die; my $owner = $in->{repository}->{owner}->{name} or die; - my $repo = $in->{repository}->{name} or die; - print STDERR "got push from GitHub repository $owner/$repo\n"; + my $repo = $in->{repository}->{name} or die; + + print STDERR "got event '$event' from GitHub repository $owner/$repo\n"; + + # `jobsetsOfInputs type value` finds the jobsets that have an input of type `type` and of value LIKE `value` + my $jobsetsOfInputs = sub { + my ($type, $value) = @_; + $c->model('DB::Jobsets')->search( + { 'jobsetinputs.type' => "githubpulls", 'project.enabled' => 1, 'me.enabled' => 1 }, + { join => ['project', {'jobsetinputs' => 'jobsetinputalts'}], + where => \ [ ' LOWER( jobsetinputalts.value ) LIKE LOWER ( ? ) ', [ 'value', $value] ] + }) + }; + + # Different SQL queries according the kind of `$event` we are dealing with + my $actions = { + create => sub {$jobsetsOfInputs->('github_refs', "$owner $repo %")}, + delete => sub {$jobsetsOfInputs->('github_refs', "$owner $repo %")}, + pull_request => sub {$jobsetsOfInputs->('githubpulls', "$owner $repo")}, + push => sub { + $c->model('DB::Jobsets')->search( + { 'project.enabled' => 1, 'me.enabled' => 1 }, + { join => 'project', where => \ [ + 'me.flake like ? or exists (select 1 from JobsetInputAlts where project = me.project and jobset = me.name and value like ?)', + [ 'flake', "%github%$owner/$repo%"], + [ 'value', "%github.com%$owner/$repo%" ] + ] }) + } + }; + + triggerJobset($self, $c, $_, 0) foreach ($actions->{$event} // sub { + die "Cannot handle GitHub event [$event]"; + })->(); - triggerJobset($self, $c, $_, 0) foreach $c->model('DB::Jobsets')->search( - { 'project.enabled' => 1, 'me.enabled' => 1 }, - { join => 'project' - , where => \ [ 'me.flake like ? or exists (select 1 from JobsetInputAlts where project = me.project and jobset = me.name and value like ?)', [ 'flake', "%github%$owner/$repo%"], [ 'value', "%github.com%$owner/$repo%" ] ] - }); $c->response->body(""); } - +sub push_github : Chained('api') PathPart('push-github') Args(0) { + print STDERR 'The endpoint [/api/push_github] is deprecated in favor of [/api/webhook_github]'; + webhook_github @_ +} 1; From 2af4e36cdcfdccacf6ac7138d5ab7ccaca8458d9 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sun, 8 May 2022 11:40:01 +0200 Subject: [PATCH 2/7] Support for secure GitHub webhook payloads (Fixes NixOS/hydra#333) --- src/lib/Hydra/Controller/API.pm | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lib/Hydra/Controller/API.pm b/src/lib/Hydra/Controller/API.pm index 29d65b744..625ab6255 100644 --- a/src/lib/Hydra/Controller/API.pm +++ b/src/lib/Hydra/Controller/API.pm @@ -9,9 +9,13 @@ use Hydra::Helper::CatalystUtils; use Hydra::Controller::Project; use JSON::MaybeXS; use DateTime; -use Digest::SHA qw(sha256_hex); +use Digest::SHA qw(sha256 sha256_hex); +use Digest::HMAC qw(hmac_hex); +use String::Compare::ConstantTime; +use File::Slurper qw(read_text); use Text::Diff; use IPC::Run qw(run); +use List::Util 'first'; sub api : Chained('/') PathPart('api') CaptureArgs(0) { @@ -281,6 +285,25 @@ sub webhook_github : Chained('api') PathPart('webhook-github') Args(0) { print STDERR "got event '$event' from GitHub repository $owner/$repo\n"; + { # Verify X-Hub-Signature-256 if secret was defined in config + my $cfg = $c->config->{github_webhook}; + my @config = defined $cfg ? ref $cfg eq "ARRAY" ? @$cfg : ($cfg) : (); + my $rule = first { $owner =~ /^$_->{owner}$/ && $repo =~ /^$_->{repo}$/ } @config; + + if (defined $rule) { + my $sig = $c->req->header('X-Hub-Signature-256'); + die "X-Hub-Signature-256 is missing, but a secret was defined for GitHub repository $owner/$repo" + unless defined $sig; + my $body = read_text($c->req->body) or die; + my $secret = $rule->{secret} or die; + my $digest = hmac_hex($body, $secret, \&sha256); + die "Request body digest (${digest}) did not match X-Hub-Signature-256 (${sig})" + unless String::Compare::ConstantTime::equals($sig, "sha256=$digest"); + } else { + print STDERR "no secret given for webhook comming from GitHub repository $owner/$repo"; + } + } + # `jobsetsOfInputs type value` finds the jobsets that have an input of type `type` and of value LIKE `value` my $jobsetsOfInputs = sub { my ($type, $value) = @_; From aca2dacd16beb7be52c0dc6b8365c518b7772156 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sat, 7 May 2022 23:20:08 +0200 Subject: [PATCH 3/7] Whitelist `webhook-github` path --- src/lib/Hydra/Controller/Root.pm | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm index c6843d296..5d0c29e7a 100644 --- a/src/lib/Hydra/Controller/Root.pm +++ b/src/lib/Hydra/Controller/Root.pm @@ -32,6 +32,7 @@ sub noLoginNeeded { return $whitelisted || $c->request->path eq "api/push-github" || + $c->request->path eq "api/webhook-github" || $c->request->path eq "google-login" || $c->request->path eq "github-redirect" || $c->request->path eq "github-login" || @@ -77,7 +78,9 @@ sub begin :Private { $_->supportedInputTypes($c->stash->{inputTypes}) foreach @{$c->hydra_plugins}; # XSRF protection: require POST requests to have the same origin. - if ($c->req->method eq "POST" && $c->req->path ne "api/push-github") { + if ($c->req->method eq "POST" && ( + $c->req->path ne "api/push-github" && $c->req->path ne "api/webhook-github" + )) { my $referer = $c->req->header('Referer'); $referer //= $c->req->header('Origin'); my $base = $c->req->base; From cf068096d5a7992b36081426e0236aa9f0da89a9 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sat, 7 May 2022 23:20:51 +0200 Subject: [PATCH 4/7] more documentation for GitHub webhooks --- doc/manual/src/webhooks.md | 39 ++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/doc/manual/src/webhooks.md b/doc/manual/src/webhooks.md index 2b26cd612..82d278a93 100644 --- a/doc/manual/src/webhooks.md +++ b/doc/manual/src/webhooks.md @@ -1,13 +1,40 @@ # Webhooks -Hydra can be notified by github's webhook to trigger a new evaluation when a -jobset has a github repo in its input. -To set up a github webhook go to `https://github.com///settings` and in the `Webhooks` tab +## GitHub +Hydra can be notified by GitHub's webhook to trigger a new evaluation when a +jobset has a GitHub repo in its input. + +GitHub's webhook can be triggered on [various events](https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads). Hydra recognizes the following events. + + - [`push`](https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#push): triggers a new evaluation for every jobset that have the GitHub repository as a "Git Checkout" input. + - [`create`](https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#create) and [`delete`](https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#deleta): triggers a new evaluation for every jobset that have the GitHub repository as a "github_refs" input. + - [`pull_request`](https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#pull_request): triggers a new evaluation for every jobset that have the GitHub repository as a "githubpulls" input. + +### Guide + +To set up a GitHub webhook go to `https://github.com///settings` and in the `Webhooks` tab click on `Add webhook`. -- In `Payload URL` fill in `https:///api/push-github`. +- In `Payload URL` fill in `https:///api/webhook-github`. - In `Content type` switch to `application/json`. -- The `Secret` field can stay empty. -- For `Which events would you like to trigger this webhook?` keep the default option for events on `Just the push event.`. +- The `Secret` field can stay empty (see below to configure a secret). +- For `Which events would you like to trigger this webhook?` either keep the default option, or select the ones you are interested in (see above for the supported events). Then add the hook with `Add webhook`. + +### Securing GitHub's webhooks +Secrets for webhooks can be configured by adding `github_webhook` keys in your Hydra configuration. +Each `github_webhook` provides a secret (`secret`, a string) for a certain range of repository name (`repo`, a regex) and repository owner (`owner`, a regex). + +For instance below we declare one secret, `foo`, for the repositories whose owner is `someone` or `someother` and is named `somerepo`. + +**IMPORTANT**: note that secrets should **never** be included directly in your `hydra.conf`, otherwise they will be exposed in plain text in the store. Instead, use includes [as described here](./configuration.html#including-files). + +```xml + + owner = (someone|someother) + repo = somerepo + secret = foo + +``` + From 23019a75db3e1e22f291707ae2a5226a77662331 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sun, 8 May 2022 09:13:25 +0200 Subject: [PATCH 5/7] Add X-GitHub-Event headers in API tests --- t/Hydra/Controller/API/checks.t | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/Hydra/Controller/API/checks.t b/t/Hydra/Controller/API/checks.t index 2b97b4895..301410b5f 100644 --- a/t/Hydra/Controller/API/checks.t +++ b/t/Hydra/Controller/API/checks.t @@ -169,6 +169,7 @@ subtest "/api/push-github" => sub { my $req = POST '/api/push-github', "Content-Type" => "application/json", + "X-GitHub-Event" => "push", "Content" => encode_json({ repository => { owner => { @@ -195,6 +196,7 @@ subtest "/api/push-github" => sub { my $req = POST '/api/push-github', "Content-Type" => "application/json", + "X-GitHub-Event" => "push", "Content" => encode_json({ repository => { owner => { From cb0ee7ec76368f6242adb071dc9b077e16490414 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sun, 8 May 2022 10:30:07 +0200 Subject: [PATCH 6/7] GitHub username: use `login` key in addition to `name` key Both seems to exists (https://docs.github.com/en/graphql/reference/objects#user), but in lots of cases, `name` is acutally missing from payloads, e.g. https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#webhook-payload-example-32 --- src/lib/Hydra/Controller/API.pm | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/lib/Hydra/Controller/API.pm b/src/lib/Hydra/Controller/API.pm index 625ab6255..dbf532caa 100644 --- a/src/lib/Hydra/Controller/API.pm +++ b/src/lib/Hydra/Controller/API.pm @@ -279,9 +279,10 @@ sub webhook_github : Chained('api') PathPart('webhook-github') Args(0) { my $in = $c->request->{data}; # every GitHub webhook payload has the `repository` key and a `X-GitHub-Event` header - my $event = $c->req->header('X-GitHub-Event') or die; - my $owner = $in->{repository}->{owner}->{name} or die; - my $repo = $in->{repository}->{name} or die; + my $event = $c->req->header('X-GitHub-Event') or die; + my $owner = ( $in->{repository}->{owner}->{name} + // $in->{repository}->{owner}->{login}) or die; + my $repo = $in->{repository}->{name} or die; print STDERR "got event '$event' from GitHub repository $owner/$repo\n"; @@ -303,12 +304,12 @@ sub webhook_github : Chained('api') PathPart('webhook-github') Args(0) { print STDERR "no secret given for webhook comming from GitHub repository $owner/$repo"; } } - + # `jobsetsOfInputs type value` finds the jobsets that have an input of type `type` and of value LIKE `value` my $jobsetsOfInputs = sub { my ($type, $value) = @_; $c->model('DB::Jobsets')->search( - { 'jobsetinputs.type' => "githubpulls", 'project.enabled' => 1, 'me.enabled' => 1 }, + { 'jobsetinputs.type' => $type, 'project.enabled' => 1, 'me.enabled' => 1 }, { join => ['project', {'jobsetinputs' => 'jobsetinputalts'}], where => \ [ ' LOWER( jobsetinputalts.value ) LIKE LOWER ( ? ) ', [ 'value', $value] ] }) From c54866f2a07367f25f9006f54eb4c325a0be2540 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Sat, 18 Jun 2022 07:47:53 +0200 Subject: [PATCH 7/7] Convert tabs to spaces --- src/lib/Hydra/Controller/API.pm | 70 ++++++++++++++++---------------- src/lib/Hydra/Controller/Root.pm | 4 +- t/Hydra/Controller/API/checks.t | 4 +- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/lib/Hydra/Controller/API.pm b/src/lib/Hydra/Controller/API.pm index dbf532caa..4771de99c 100644 --- a/src/lib/Hydra/Controller/API.pm +++ b/src/lib/Hydra/Controller/API.pm @@ -281,58 +281,58 @@ sub webhook_github : Chained('api') PathPart('webhook-github') Args(0) { # every GitHub webhook payload has the `repository` key and a `X-GitHub-Event` header my $event = $c->req->header('X-GitHub-Event') or die; my $owner = ( $in->{repository}->{owner}->{name} - // $in->{repository}->{owner}->{login}) or die; + // $in->{repository}->{owner}->{login}) or die; my $repo = $in->{repository}->{name} or die; print STDERR "got event '$event' from GitHub repository $owner/$repo\n"; { # Verify X-Hub-Signature-256 if secret was defined in config - my $cfg = $c->config->{github_webhook}; - my @config = defined $cfg ? ref $cfg eq "ARRAY" ? @$cfg : ($cfg) : (); - my $rule = first { $owner =~ /^$_->{owner}$/ && $repo =~ /^$_->{repo}$/ } @config; - - if (defined $rule) { - my $sig = $c->req->header('X-Hub-Signature-256'); - die "X-Hub-Signature-256 is missing, but a secret was defined for GitHub repository $owner/$repo" - unless defined $sig; - my $body = read_text($c->req->body) or die; - my $secret = $rule->{secret} or die; - my $digest = hmac_hex($body, $secret, \&sha256); - die "Request body digest (${digest}) did not match X-Hub-Signature-256 (${sig})" - unless String::Compare::ConstantTime::equals($sig, "sha256=$digest"); - } else { - print STDERR "no secret given for webhook comming from GitHub repository $owner/$repo"; - } + my $cfg = $c->config->{github_webhook}; + my @config = defined $cfg ? ref $cfg eq "ARRAY" ? @$cfg : ($cfg) : (); + my $rule = first { $owner =~ /^$_->{owner}$/ && $repo =~ /^$_->{repo}$/ } @config; + + if (defined $rule) { + my $sig = $c->req->header('X-Hub-Signature-256'); + die "X-Hub-Signature-256 is missing, but a secret was defined for GitHub repository $owner/$repo" + unless defined $sig; + my $body = read_text($c->req->body) or die; + my $secret = $rule->{secret} or die; + my $digest = hmac_hex($body, $secret, \&sha256); + die "Request body digest (${digest}) did not match X-Hub-Signature-256 (${sig})" + unless String::Compare::ConstantTime::equals($sig, "sha256=$digest"); + } else { + print STDERR "no secret given for webhook comming from GitHub repository $owner/$repo"; + } } - + # `jobsetsOfInputs type value` finds the jobsets that have an input of type `type` and of value LIKE `value` my $jobsetsOfInputs = sub { - my ($type, $value) = @_; - $c->model('DB::Jobsets')->search( - { 'jobsetinputs.type' => $type, 'project.enabled' => 1, 'me.enabled' => 1 }, - { join => ['project', {'jobsetinputs' => 'jobsetinputalts'}], - where => \ [ ' LOWER( jobsetinputalts.value ) LIKE LOWER ( ? ) ', [ 'value', $value] ] - }) + my ($type, $value) = @_; + $c->model('DB::Jobsets')->search( + { 'jobsetinputs.type' => $type, 'project.enabled' => 1, 'me.enabled' => 1 }, + { join => ['project', {'jobsetinputs' => 'jobsetinputalts'}], + where => \ [ ' LOWER( jobsetinputalts.value ) LIKE LOWER ( ? ) ', [ 'value', $value] ] + }) }; # Different SQL queries according the kind of `$event` we are dealing with my $actions = { create => sub {$jobsetsOfInputs->('github_refs', "$owner $repo %")}, delete => sub {$jobsetsOfInputs->('github_refs', "$owner $repo %")}, - pull_request => sub {$jobsetsOfInputs->('githubpulls', "$owner $repo")}, - push => sub { - $c->model('DB::Jobsets')->search( - { 'project.enabled' => 1, 'me.enabled' => 1 }, - { join => 'project', where => \ [ - 'me.flake like ? or exists (select 1 from JobsetInputAlts where project = me.project and jobset = me.name and value like ?)', - [ 'flake', "%github%$owner/$repo%"], - [ 'value', "%github.com%$owner/$repo%" ] - ] }) - } + pull_request => sub {$jobsetsOfInputs->('githubpulls', "$owner $repo")}, + push => sub { + $c->model('DB::Jobsets')->search( + { 'project.enabled' => 1, 'me.enabled' => 1 }, + { join => 'project', where => \ [ + 'me.flake like ? or exists (select 1 from JobsetInputAlts where project = me.project and jobset = me.name and value like ?)', + [ 'flake', "%github%$owner/$repo%"], + [ 'value', "%github.com%$owner/$repo%" ] + ] }) + } }; triggerJobset($self, $c, $_, 0) foreach ($actions->{$event} // sub { - die "Cannot handle GitHub event [$event]"; + die "Cannot handle GitHub event [$event]"; })->(); $c->response->body(""); diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm index 5d0c29e7a..c6879ddde 100644 --- a/src/lib/Hydra/Controller/Root.pm +++ b/src/lib/Hydra/Controller/Root.pm @@ -79,8 +79,8 @@ sub begin :Private { # XSRF protection: require POST requests to have the same origin. if ($c->req->method eq "POST" && ( - $c->req->path ne "api/push-github" && $c->req->path ne "api/webhook-github" - )) { + $c->req->path ne "api/push-github" && $c->req->path ne "api/webhook-github" + )) { my $referer = $c->req->header('Referer'); $referer //= $c->req->header('Origin'); my $base = $c->req->base; diff --git a/t/Hydra/Controller/API/checks.t b/t/Hydra/Controller/API/checks.t index 301410b5f..712c63e81 100644 --- a/t/Hydra/Controller/API/checks.t +++ b/t/Hydra/Controller/API/checks.t @@ -169,7 +169,7 @@ subtest "/api/push-github" => sub { my $req = POST '/api/push-github', "Content-Type" => "application/json", - "X-GitHub-Event" => "push", + "X-GitHub-Event" => "push", "Content" => encode_json({ repository => { owner => { @@ -196,7 +196,7 @@ subtest "/api/push-github" => sub { my $req = POST '/api/push-github', "Content-Type" => "application/json", - "X-GitHub-Event" => "push", + "X-GitHub-Event" => "push", "Content" => encode_json({ repository => { owner => {