Skip to content

Commit 3774234

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 cc9c3f7 commit 3774234

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
@@ -232,11 +232,18 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
232232
| _ -> Lwt.return_unit
233233

234234
let process_github_notification (ctx : Context.t) headers body =
235+
let validate_signature secrets =
236+
let signing_key = secrets.gh_hook_token in
237+
Github.validate_signature ?signing_key ~headers body
238+
in
235239
try%lwt
236240
let secrets = Context.get_secrets_exn ctx in
237-
match Github.parse_exn ~secret:secrets.gh_hook_token headers body with
241+
match Github.parse_exn headers body with
238242
| exception exn -> Exn_lwt.fail ~exn "failed to parse payload"
239243
| payload ->
244+
match validate_signature secrets with
245+
| Error e -> action_error e
246+
| Ok () ->
240247
( match%lwt refresh_repo_config ctx payload with
241248
| Error e -> action_error e
242249
| 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)