DRU-1145 -- Ask GitHub whether a reviewer can write when the delivery hides it

backend/druks/core/apis/github.py

@@ -550,6 +550,15 @@ async def post_check_run(
             body["output"] = {**output, "annotations": list(annotations[start : start + limit])}
             await checks.async_update(owner, repo_name, check_run_id, data=body)
 
+    async def get_permission(self, repo: str, login: str) -> str:
+        """``login``'s base permission on ``repo``: admin, write, read, or none. GitHub
+        reports maintain as write and triage as read."""
+        owner, name = repo.split("/", 1)
+        response = await self._github.rest.repos.async_get_collaborator_permission_level(
+            owner, name, login
+        )
+        return response.parsed_data.permission
+
     async def get_file_content(
         self,
         repo: str,

backend/druks/core/webhooks/github.py

@@ -12,9 +12,11 @@
 
 _REVIEW_ACTION = {"APPROVED": "approve", "CHANGES_REQUESTED": "request_changes"}
 
-# GitHub's standing for the commenter on that repo. These three write to it;
-# everyone else is a passer-by, and on a public repo that is the whole internet.
+# GitHub's standing for the commenter on that repo. These three write to it. GitHub
+# reports a private org member as a contributor, so anyone else's permission is read.
 _WRITERS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
+# The repo permissions that write. GitHub reports maintain as write.
+_WRITE_PERMISSIONS = frozenset({"admin", "write"})
 
 
 class GitHubEvents(Webhook):
@@ -71,7 +73,7 @@ async def on_pull_request_review_submitted(self) -> Response:
                     "branch": pull_request["head"]["ref"],
                     "action": action,
                     "reviewer": sender["login"],
-                    "author_can_write": review["author_association"] in _WRITERS,
+                    "author_can_write": await self._can_write(sender, review["author_association"]),
                     "body": review["body"] or "",  # body is nullable on an approve
                     "is_latest_commit": review["commit_id"] == pull_request["head"]["sha"],
                 },
@@ -84,7 +86,9 @@ async def on_issue_comment_created(self) -> Response:
         if "pull_request" in issue:
             await self._publish_pull_request_comment(issue["number"])
         else:
-            await publish("ticket.commented", payload={**self._ticket(issue), **self._comment()})
+            await publish(
+                "ticket.commented", payload={**self._ticket(issue), **await self._comment()}
+            )
         return _accepted()
 
     async def on_pull_request_review_comment_created(self) -> Response:
@@ -105,23 +109,33 @@ async def _publish_pull_request_comment(
             repo=_repo_name(self.data),
             pr_number=pr_number,
             payload={
-                **self._comment(),
+                **await self._comment(),
                 "actor_id": str(self.data["sender"]["id"]),
                 "review_thread_id": review_thread_id,
             },
         )
 
-    def _comment(self) -> dict[str, Any]:
+    async def _comment(self) -> dict[str, Any]:
         """The keys every comment event carries, for the comment in this delivery."""
         sender, comment = self.data["sender"], self.data["comment"]
         return {
             "comment_id": comment["id"],
             "body": comment["body"],
             "author": sender["login"],
             "author_is_bot": sender["type"] == "Bot",
-            "author_can_write": comment["author_association"] in _WRITERS,
+            "author_can_write": await self._can_write(sender, comment["author_association"]),
         }
 
+    async def _can_write(self, user: dict[str, Any], association: str) -> bool:
+        """Whether ``user`` can write to the delivery's repo."""
+        if association in _WRITERS:
+            return True
+        if user["type"] == "Bot":
+            return False
+        repo = _repo_name(self.data)
+        github = await Github.get_client(repo)
+        return await github.get_permission(repo, user["login"]) in _WRITE_PERMISSIONS
+
     async def on_issues_labeled(self) -> Response:
         """Publish a label that a person or this App put on an issue. Another App's
         label acts for no one here."""
@@ -210,7 +224,9 @@ async def _publish_pull_request(self, fact: str) -> Response:
                 "body": pull_request["body"] or "",
                 "draft": pull_request["draft"],
                 "author": pull_request["user"]["login"],
-                "author_can_write": pull_request["author_association"] in _WRITERS,
+                "author_can_write": await self._can_write(
+                    pull_request["user"], pull_request["author_association"]
+                ),
             },
         )
         return _accepted()

backend/tests/test_github_channel.py

@@ -69,12 +69,16 @@ async def create_reaction(client, repo, comment_id, content):
     async def create_review_comment_reaction(client, repo, comment_id, content):
         posts.append(("create_review_comment_reaction", repo, comment_id, content))
 
+    async def get_permission(client, repo, login):
+        return "read"
+
     monkeypatch.setattr(GitHubClient, "create_comment", create_comment)
     monkeypatch.setattr(GitHubClient, "reply_to_review_comment", reply_to_review_comment)
     monkeypatch.setattr(GitHubClient, "create_reaction", create_reaction)
     monkeypatch.setattr(
         GitHubClient, "create_review_comment_reaction", create_review_comment_reaction
     )
+    monkeypatch.setattr(GitHubClient, "get_permission", get_permission)
     return posts
 
 
@@ -139,7 +143,7 @@ async def receive(tmp_path, data, *, inline=False):
         await events.on_issue_comment_created()
 
 
-async def test_the_webhook_publishes_each_comment_once(tmp_path, monkeypatch):
+async def test_the_webhook_publishes_each_comment_once(card, github, tmp_path, monkeypatch):
     published = []
 
     async def publish(name, **kwargs):

backend/tests/test_webhooks_github.py

@@ -2,6 +2,7 @@
 from typing import Any, cast
 
 import pytest
+from druks.core.apis.github import GitHubClient
 from druks.core.webhooks import github as github_webhooks
 from druks.core.webhooks.github import GitHubEvents
 from druks.testing import make_settings
@@ -48,7 +49,30 @@ async def _emit(event_type, **kwargs):
     return published
 
 
-def _issue(*, action, is_pull_request=False, sender_type="User"):
+class _Permissions(GitHubClient):
+    """GitHub's permission of each login on the repo. Anyone else reads."""
+
+    def __init__(self, **permissions: str):
+        super().__init__("token", app_id="1", slug="druks")
+        self.permissions = permissions
+        self.asked: list[str] = []
+
+    async def get_permission(self, repo, login):
+        self.asked.append(login)
+        return self.permissions.get(login, "read")
+
+
+def _github(monkeypatch, **permissions: str) -> _Permissions:
+    github = _Permissions(**permissions)
+
+    async def get_client(owner_or_repo):
+        return github
+
+    monkeypatch.setattr(github_webhooks.Github, "get_client", get_client)
+    return github
+
+
+def _issue(*, action, is_pull_request=False, sender_type="User", association="NONE"):
     issue = {
         "id": 4242,
         "number": 7,
@@ -64,7 +88,7 @@ def _issue(*, action, is_pull_request=False, sender_type="User"):
         "repository": {"full_name": "acme/widget"},
         "issue": issue,
         "sender": {"type": sender_type, "login": "passerby", "id": 99},
-        "comment": {"id": 501, "body": "Still broken", "author_association": "NONE"},
+        "comment": {"id": 501, "body": "Still broken", "author_association": association},
     }
 
 
@@ -109,6 +133,7 @@ async def test_an_issue_action_publishes_a_tracker_fact(
 
 async def test_an_issue_comment_publishes_ticket_commented(tmp_path, monkeypatch):
     published = _capture(monkeypatch)
+    _github(monkeypatch)
 
     await _events(tmp_path, payload=_issue(action="created")).on_issue_comment_created()
 
@@ -117,6 +142,7 @@ async def test_an_issue_comment_publishes_ticket_commented(tmp_path, monkeypatch
 
 async def test_a_pull_request_comment_publishes_pr_commented(tmp_path, monkeypatch):
     published = _capture(monkeypatch)
+    _github(monkeypatch)
     payload = _issue(action="created", is_pull_request=True)
 
     await _events(tmp_path, payload=payload).on_issue_comment_created()
@@ -214,3 +240,48 @@ async def test_a_review_says_whether_it_is_of_the_latest_commit(
     [(event_type, published_event)] = published
     assert event_type == "pr.review_submitted"
     assert published_event["payload"]["is_latest_commit"] is is_latest_commit
+
+
+@pytest.mark.parametrize(("permission", "can_write"), [("write", True), ("read", False)])
+async def test_a_review_from_a_contributor_asks_github_whether_they_write(
+    tmp_path, monkeypatch, permission, can_write
+):
+    # GitHub reports a private org member as a contributor.
+    published = _capture(monkeypatch)
+    _github(monkeypatch, alice=permission)
+
+    payload = {
+        **_pull_request(action="submitted"),
+        "sender": {"login": "alice", "type": "User"},
+        "review": {
+            "state": "approved",
+            "author_association": "CONTRIBUTOR",
+            "body": None,
+            "commit_id": "0f3a2c1d",
+        },
+    }
+
+    await _events(tmp_path, payload=payload).on_pull_request_review_submitted()
+
+    assert published[0][1]["payload"]["author_can_write"] is can_write
+
+
+async def test_a_comment_from_a_private_member_who_writes_can_write(tmp_path, monkeypatch):
+    published = _capture(monkeypatch)
+    _github(monkeypatch, passerby="admin")
+    payload = _issue(action="created", is_pull_request=True, association="CONTRIBUTOR")
+
+    await _events(tmp_path, payload=payload).on_issue_comment_created()
+
+    assert published[0][1]["payload"]["author_can_write"] is True
+
+
+@pytest.mark.parametrize(("sender_type", "association"), [("User", "MEMBER"), ("Bot", "NONE")])
+async def test_a_writer_or_a_bot_is_not_looked_up(tmp_path, monkeypatch, sender_type, association):
+    _capture(monkeypatch)
+    github = _github(monkeypatch)
+    payload = _issue(action="created", sender_type=sender_type, association=association)
+
+    await _events(tmp_path, payload=payload).on_issue_comment_created()
+
+    assert github.asked == []