Skip to content

Commit 2413a88

Browse files
committed
github: move webhook signature checking logic out of parse_exn
With repo-specific secrets, we'll need the repo name in order to obtain the webhook token used for signature validation. So parsing the request body for a GH payload needs to happen before, not after, the signature check.
1 parent d20f24e commit 2413a88

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

lib/action.ml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,18 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
211211
| _ -> Lwt.return @@ Ok ()
212212

213213
let process_github_notification (ctx : Context.t) headers body =
214+
let validate_signature secrets =
215+
let signing_key = secrets.gh_hook_token in
216+
Github.validate_signature ?signing_key ~headers body
217+
in
214218
try%lwt
215219
let secrets = Context.get_secrets_exn ctx in
216-
match Github.parse_exn ~secret:secrets.gh_hook_token headers body with
220+
match Github.parse_exn headers body with
217221
| exception exn -> Exn_lwt.fail ~exn "failed to parse payload"
218222
| payload ->
223+
match validate_signature secrets with
224+
| Error e -> action_error e
225+
| Ok () ->
219226
( match%lwt refresh_repo_config ctx payload with
220227
| Error e -> action_error e
221228
| Ok () ->

lib/github.ml

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,16 @@ let is_valid_signature ~secret headers_sig body =
6464
let (`Hex request_hash) = Hex.of_string request_hash in
6565
String.equal headers_sig (sprintf "sha1=%s" request_hash)
6666

67+
let validate_signature ?signing_key ~headers body =
68+
match signing_key with
69+
| None -> Ok ()
70+
| Some secret ->
71+
match List.Assoc.find headers "x-hub-signature" ~equal:String.equal with
72+
| None -> Error "unable to find header x-hub-signature"
73+
| Some signature -> if is_valid_signature ~secret signature body then Ok () else Error "signatures don't match"
74+
6775
(* Parse a payload. The type of the payload is detected from the headers. *)
68-
let parse_exn ~secret headers body =
69-
begin
70-
match secret with
71-
| None -> ()
72-
| Some secret ->
73-
match List.Assoc.find headers "x-hub-signature" ~equal:String.equal with
74-
| None -> Exn.fail "unable to find header x-hub-signature"
75-
| Some req_sig -> if not @@ is_valid_signature ~secret req_sig body then failwith "request signature invalid"
76-
end;
76+
let parse_exn headers body =
7777
match List.Assoc.find_exn headers "x-github-event" ~equal:String.equal with
7878
| exception exn -> Exn.fail ~exn "unable to read x-github-event"
7979
| "push" -> Push (commit_pushed_notification_of_string body)

0 commit comments

Comments
 (0)