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.

Author: yasunariw

lib/action.ml

@@ -208,9 +208,12 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
   let process_github_notification (ctx : Context.t) headers body =
     try%lwt
       let secrets = Context.get_secrets_exn ctx in
-      match Github.parse_exn ~secret:secrets.gh_hook_token headers body with
+      match Github.parse_exn headers body with
       | exception exn -> Exn_lwt.fail ~exn "failed to parse payload"
       | payload ->
+      match Github.validate_signature ?signing_key:secrets.gh_hook_token ~headers body with
+      | Error e -> action_error e
+      | Ok () ->
         ( match%lwt refresh_repo_config ctx payload with
         | Error e -> action_error e
         | Ok () ->

lib/github.ml

@@ -64,16 +64,16 @@ let is_valid_signature ~secret headers_sig body =
   let (`Hex request_hash) = Hex.of_string request_hash in
   String.equal headers_sig (sprintf "sha1=%s" request_hash)
 
+let validate_signature ?signing_key ~headers body =
+  match signing_key with
+  | None -> Ok ()
+  | Some secret ->
+  match List.Assoc.find headers "x-hub-signature" ~equal:String.equal with
+  | None -> Error "unable to find header x-hub-signature"
+  | Some signature -> if is_valid_signature ~secret signature body then Ok () else Error "signatures don't match"
+
 (* Parse a payload. The type of the payload is detected from the headers. *)
-let parse_exn ~secret headers body =
-  begin
-    match secret with
-    | None -> ()
-    | Some secret ->
-    match List.Assoc.find headers "x-hub-signature" ~equal:String.equal with
-    | None -> Exn.fail "unable to find header x-hub-signature"
-    | Some req_sig -> if not @@ is_valid_signature ~secret req_sig body then failwith "request signature invalid"
-  end;
+let parse_exn headers body =
   match List.Assoc.find_exn headers "x-github-event" ~equal:String.equal with
   | exception exn -> Exn.fail ~exn "unable to read x-github-event"
   | "push" -> Push (commit_pushed_notification_of_string body)

test/test.ml

@@ -22,7 +22,7 @@ let get_mock_payloads () =
 let process ~(secrets : Config_t.secrets) ~config (kind, path, state_path) =
   let headers = [ "x-github-event", kind ] in
   let make_test_context event =
-    let repo = Github.repo_of_notification @@ Github.parse_exn ~secret:secrets.gh_token headers event in
+    let repo = Github.repo_of_notification @@ Github.parse_exn headers event in
     let ctx = Context.make () in
     ctx.secrets <- Some secrets;
     ignore (State.find_or_add_repo ctx.state repo.url);