Merge branch 'yasu/support-multiple-repos'

* yasu/support-multiple-repos:
  cfg: make specifying repos mandatory
  fmt
  cfg: unify repos and allowed_repos into singlee field
  add comments
  state: add lock
  docs: add info on multi-repo setup
  test: recreate context per test case
  cfg: add option to restrict handling of GH payloads to certain repo urls
  cfg: allow repo-specific configuration of gh_token and gh_hook_token
  github: move webhook signature checking logic out of parse_exn
  ctx: allocate new hash table for every Context.make ()
  state: store runtime repo state in a hash table indexed by repo url
  cfg: make ctx store a hash tbl mapping urls to cfgs
  add atd wrapper for hash tables

Author: yasunariw

README.md

@@ -55,7 +55,7 @@ Note: The `slack_access_token` must be configured in your secrets file for link
 
 ### Documentation
 
-The bot expects two configuration files to be present.
+Commit a configuration file to the root of each repository you want to support, and add a secrets file on the bot server itself. Read on for instructions to set up each file:
 
 * [Repository configuration](./documentation/config_docs.md)
 * [Secrets](./documentation/secret_docs.md)

documentation/secret_docs.md

@@ -8,26 +8,58 @@ A secrets file stores sensitive information. Unlike the repository configuration
 
 ```json
 {
-    "gh_token": "",
-    "slack_access_token": ""
+    "repos": [
+      {
+        "url": "https://github.com/ahrefs/monorobot",
+        "gh_token": "XXX"
+      }
+    ],
+    "slack_access_token": "XXX"
 }
 ```
 
 | value | description | optional | default |
 |-|-|-|-|
-| `gh_token` | specify to grant the bot access to private repositories; omit for public repositories | Yes | - |
-| `gh_hook_token` | specify to ensure the bot only receives GitHub notifications from pre-approved repositories | Yes | - |
+| `repos` | specify each target repository's url and its secrets | No | - |
 | `slack_access_token` | slack bot access token to enable message posting to the workspace | Yes | try to use webhooks defined in `slack_hooks` instead |
 | `slack_hooks` | list of channel names and their corresponding webhook endpoint | Yes | try to use token defined in `slack_access_token` instead |
 | `slack_signing_secret` | specify to verify incoming slack requests | Yes | - |
 
 Note that either `slack_access_token` or `slack_hooks` must be defined. If both are present, the bot will send notifications using webhooks.
 
-## `gh_token`
+## `repos`
+
+Specifies which repositories to accept events from, along with any repository-specific overrides to secrets.
+
+```json
+[
+  {
+    "url": "https://github.com/ahrefs/runner",
+    "gh_token": "XXX"
+  },
+  {
+    "url": "https://git.ahrefs.com/ahrefs/coyote",
+    "gh_token": "XXX",
+    "gh_hook_token": "XXX"
+  }
+]
+```
+
+| value | description | optional | default |
+|-|-|-|-|
+| `url` | the repository url. | No | - |
+| `gh_token` | specify to grant the bot access to private repositories; omit for public repositories | Yes | - |
+| `gh_hook_token` | specify to ensure the bot only receives GitHub notifications from pre-approved repositories | Yes | - |
+
+### `repos`
+
+Repository URLs should be fully qualified (include the protocol), with no trailing backslash.
+
+### `gh_token`
 
 Some operations, such as fetching a config file from a private repository, or the commit corresponding to a commit comment event, require a personal access token. Refer [here](https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/creating-a-personal-access-token) for detailed instructions on token generation.
 
-## `gh_hook_token`
+### `gh_hook_token`
 
 Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/securing-your-webhooks) for more information on securing webhooks with a token.
 

lib/action.ml

@@ -93,13 +93,14 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     if List.is_empty matched_channel_names then default else matched_channel_names
 
   let partition_status (ctx : Context.t) (n : status_notification) =
-    let cfg = Context.get_config_exn ctx in
+    let repo = n.repository in
+    let cfg = Context.find_repo_config_exn ctx repo.url in
     let pipeline = n.context in
     let current_status = n.state in
     let rules = cfg.status_rules.rules in
     let action_on_match (branches : branch list) =
       let default = Option.to_list cfg.prefix_rules.default_channel in
-      let () = Context.refresh_pipeline_status ~pipeline ~branches ~status:current_status ctx in
+      let%lwt () = State.set_repo_pipeline_status ctx.state repo.url ~pipeline ~branches ~status:current_status in
       match List.is_empty branches with
       | true -> Lwt.return []
       | false ->
@@ -111,18 +112,18 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
       | false -> Lwt.return default
       | true ->
         let sha = n.commit.sha in
-        let repo = n.repository in
         ( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with
         | Error e -> action_error e
         | Ok commit -> Lwt.return @@ partition_commit cfg commit.files
         )
     in
-    if Context.is_pipeline_allowed ctx ~pipeline then begin
+    if Context.is_pipeline_allowed ctx repo.url ~pipeline then begin
+      let%lwt repo_state = State.find_or_add_repo ctx.state repo.url in
       match Rule.Status.match_rules ~rules n with
       | Some Ignore | None -> Lwt.return []
       | Some Allow -> action_on_match n.branches
       | Some Allow_once ->
-      match Map.find ctx.state.pipeline_statuses pipeline with
+      match Map.find repo_state.pipeline_statuses pipeline with
       | Some branch_statuses ->
         let has_same_status_state_as_prev (branch : branch) =
           match Map.find branch_statuses branch.name with
@@ -136,7 +137,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     else Lwt.return []
 
   let partition_commit_comment (ctx : Context.t) n =
-    let cfg = Context.get_config_exn ctx in
+    let cfg = Context.find_repo_config_exn ctx n.repository.url in
     match n.comment.commit_id with
     | None -> action_error "unable to find commit id for this commit comment event"
     | Some sha ->
@@ -155,7 +156,8 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
       )
 
   let generate_notifications (ctx : Context.t) req =
-    let cfg = Context.get_config_exn ctx in
+    let repo = Github.repo_of_notification req in
+    let cfg = Context.find_repo_config_exn ctx repo.url in
     match req with
     | Github.Push n ->
       partition_push cfg n |> List.map ~f:(fun (channel, n) -> generate_push_notification n channel) |> Lwt.return
@@ -184,20 +186,20 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     in
     Lwt_list.iter_s notify notifications
 
-  (** [refresh_config_of_context ctx n] updates the current context if the configuration
-      hasn't been loaded yet, or if the incoming request [n] is a push
+  (** [refresh_repo_config ctx n] fetches the latest repo config if it's
+      uninitialized, or if the incoming request [n] is a push
       notification containing commits that touched the config file. *)
-  let refresh_config_of_context (ctx : Context.t) notification =
+  let refresh_repo_config (ctx : Context.t) notification =
+    let repo = Github.repo_of_notification notification in
     let fetch_config () =
-      let repo = Github.repo_of_notification notification in
       match%lwt Github_api.get_config ~ctx ~repo with
       | Ok config ->
-        ctx.config <- Some config;
-        Context.print_config ctx;
+        Context.set_repo_config ctx repo.url config;
+        Context.print_config ctx repo.url;
         Lwt.return @@ Ok ()
       | Error e -> action_error e
     in
-    match ctx.config with
+    match Context.find_repo_config ctx repo.url with
     | None -> fetch_config ()
     | Some _ ->
     match notification with
@@ -208,8 +210,8 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
       if config_was_modified then fetch_config () else Lwt.return @@ Ok ()
     | _ -> Lwt.return @@ Ok ()
 
-  let do_github_tasks ctx (req : Github.t) =
-    let cfg = Context.get_config_exn ctx in
+  let do_github_tasks ctx (repo : repository) (req : Github.t) =
+    let cfg = Context.find_repo_config_exn ctx repo.url in
     let project_owners (pull_request : pull_request) repository number =
       match Github.get_project_owners pull_request cfg.project_owners with
       | Some reviewers ->
@@ -230,16 +232,32 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     | _ -> Lwt.return_unit
 
   let process_github_notification (ctx : Context.t) headers body =
+    let validate_signature secrets payload =
+      let repo = Github.repo_of_notification payload in
+      let signing_key = Context.gh_hook_token_of_secrets secrets repo.url in
+      Github.validate_signature ?signing_key ~headers body
+    in
+    let repo_is_supported secrets payload =
+      let repo = Github.repo_of_notification payload in
+      List.exists secrets.repos ~f:(fun r -> String.equal r.url repo.url)
+    in
     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%lwt refresh_config_of_context ctx payload with
+      match validate_signature secrets payload with
+      | Error e -> action_error e
+      | Ok () ->
+      match repo_is_supported secrets payload with
+      | false -> action_error "unsupported repository"
+      | true ->
+        ( match%lwt refresh_repo_config ctx payload with
         | Error e -> action_error e
         | Ok () ->
           let%lwt notifications = generate_notifications ctx payload in
-          let%lwt () = Lwt.join [ send_notifications ctx notifications; do_github_tasks ctx payload ] in
+          let repo = Github.repo_of_notification payload in
+          let%lwt () = Lwt.join [ send_notifications ctx notifications; do_github_tasks ctx repo payload ] in
           ( match ctx.state_filepath with
           | None -> Lwt.return_unit
           | Some path ->
@@ -264,7 +282,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     let fetch_bot_user_id () =
       match%lwt Slack_api.send_auth_test ~ctx () with
       | Ok { user_id; _ } ->
-        ctx.state.bot_user_id <- Some user_id;
+        State.set_bot_user_id ctx.state user_id;
         let%lwt () =
           Option.value_map ctx.state_filepath ~default:Lwt.return_unit ~f:(fun path ->
             match%lwt State.save ctx.state path with
@@ -301,7 +319,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
         )
     in
     let%lwt bot_user_id =
-      match ctx.state.bot_user_id with
+      match State.get_bot_user_id ctx.state with
       | Some id -> Lwt.return_some id
       | None -> fetch_bot_user_id ()
     in

lib/api_remote.ml

@@ -23,7 +23,8 @@ module Github : Api.Github = struct
   let get_config ~(ctx : Context.t) ~repo =
     let secrets = Context.get_secrets_exn ctx in
     let url = contents_url ~repo ~path:ctx.config_filename in
-    let headers = build_headers ?token:secrets.gh_token () in
+    let token = Context.gh_token_of_secrets secrets repo.url in
+    let headers = build_headers ?token () in
     match%lwt http_request ~headers `GET url with
     | Error e -> Lwt.return @@ fmt_error "error while querying remote: %s\nfailed to get config from file %s" e url
     | Ok res ->
@@ -44,35 +45,38 @@ module Github : Api.Github = struct
         @@ fmt_error "unexpected encoding '%s' in Github response\nfailed to get config from file %s" encoding url
       )
 
-  let get_resource (ctx : Context.t) url =
-    let secrets = Context.get_secrets_exn ctx in
-    let headers = build_headers ?token:secrets.gh_token () in
+  let get_resource ~secrets ~repo_url url =
+    let token = Context.gh_token_of_secrets secrets repo_url in
+    let headers = build_headers ?token () in
     match%lwt http_request ~headers `GET url with
     | Ok res -> Lwt.return @@ Ok res
     | Error e -> Lwt.return @@ fmt_error "error while querying remote: %s\nfailed to get resource from %s" e url
 
-  let post_resource (ctx : Context.t) body url =
-    let secrets = Context.get_secrets_exn ctx in
-    let headers = build_headers ?token:secrets.gh_token () in
+  let post_resource ~secrets ~repo_url body url =
+    let token = Context.gh_token_of_secrets secrets repo_url in
+    let headers = build_headers ?token () in
     match%lwt http_request ~headers ~body:(`Raw ("application/json; charset=utf-8", body)) `POST url with
     | Ok res -> Lwt.return @@ Ok res
     | Error e -> Lwt.return @@ fmt_error "POST to %s failed : %s" url e
 
-  let get_api_commit ~(ctx : Context.t) ~repo ~sha =
-    let%lwt res = commits_url ~repo ~sha |> get_resource ctx in
+  let get_api_commit ~(ctx : Context.t) ~(repo : Github_t.repository) ~sha =
+    let%lwt res = commits_url ~repo ~sha |> get_resource ~secrets:(Context.get_secrets_exn ctx) ~repo_url:repo.url in
     Lwt.return @@ Result.map res ~f:Github_j.api_commit_of_string
 
-  let get_pull_request ~(ctx : Context.t) ~repo ~number =
-    let%lwt res = pulls_url ~repo ~number |> get_resource ctx in
+  let get_pull_request ~(ctx : Context.t) ~(repo : Github_t.repository) ~number =
+    let%lwt res = pulls_url ~repo ~number |> get_resource ~secrets:(Context.get_secrets_exn ctx) ~repo_url:repo.url in
     Lwt.return @@ Result.map res ~f:Github_j.pull_request_of_string
 
-  let get_issue ~(ctx : Context.t) ~repo ~number =
-    let%lwt res = issues_url ~repo ~number |> get_resource ctx in
+  let get_issue ~(ctx : Context.t) ~(repo : Github_t.repository) ~number =
+    let%lwt res = issues_url ~repo ~number |> get_resource ~secrets:(Context.get_secrets_exn ctx) ~repo_url:repo.url in
     Lwt.return @@ Result.map res ~f:Github_j.issue_of_string
 
   let request_reviewers ~(ctx : Context.t) ~repo ~number ~reviewers =
     let body = Github_j.string_of_request_reviewers_req reviewers in
-    let%lwt res = pulls_url ~repo ~number ^ "/requested_reviewers" |> post_resource ctx body in
+    let%lwt res =
+      pulls_url ~repo ~number ^ "/requested_reviewers"
+      |> post_resource ~secrets:(Context.get_secrets_exn ctx) ~repo_url:repo.url body
+    in
     Lwt.return @@ Result.map res ~f:(fun _ -> ())
 end
 

lib/common.atd

@@ -1,3 +1,7 @@
 type 'v map_as_object =
   (string * 'v) list <json repr="object">
     wrap <ocaml module="Common.StringMap" t="'v Common.StringMap.t">
+
+type 'v table_as_object =
+  (string * 'v) list <json repr="object">
+    wrap <ocaml module="Common.Stringtbl" t="'v Common.Stringtbl.t">

lib/common.ml

@@ -1,5 +1,4 @@
 open Base
-open Devkit
 
 module StringMap = struct
   type 'a t = 'a Map.M(String).t
@@ -11,13 +10,27 @@ module StringMap = struct
   let unwrap = to_list
 end
 
+module Stringtbl = struct
+  include Hashtbl
+
+  type 'a t = 'a Hashtbl.M(String).t
+
+  let empty () = Hashtbl.create (module String)
+  let to_list (l : 'a t) : (string * 'a) list = Hashtbl.to_alist l
+  let of_list (m : (string * 'a) list) : 'a t = Hashtbl.of_alist_exn (module String) m
+  let wrap = of_list
+  let unwrap = to_list
+end
+
 module Re2 = struct
   include Re2
 
   let wrap s = create_exn s
   let unwrap = Re2.to_string
 end
 
+open Devkit
+
 let fmt_error fmt = Printf.ksprintf (fun s -> Error s) fmt
 
 let first_line s =

lib/config.atd

@@ -44,13 +44,20 @@ type webhook = {
   channel : string; (* name of the Slack channel to post the message *)
 }
 
-(* This is the structure of the secrets file which stores sensitive information, and
-   shouldn't be checked into version control.  *)
-type secrets = {
+type repo_config = {
+  (* Repository url. Fully qualified (include protocol), without trailing slash. e.g. https://github.com/ahrefs/monorobot *)
+  url : string;
   (* GitHub personal access token, if repo access requires it *)
   ?gh_token : string nullable;
   (* GitHub webhook token to secure the webhook *)
   ?gh_hook_token : string nullable;
+}
+
+(* This is the structure of the secrets file which stores sensitive information, and
+   shouldn't be checked into version control.  *)
+type secrets = {
+  (* repo-specific secrets; overrides global values if defined for a given repo *)
+  repos : repo_config list;
   (* list of Slack webhook & channel name pairs *)
   ~slack_hooks <ocaml default="[]"> : webhook list;
   (* Slack bot token (`xoxb-XXXX`), giving the bot capabilities to interact with the workspace *)