Skip to content

Commit 9afa36f

Browse files
authored
GitHub App: don't sync repositories that the app doesn't have access to (#12272)
When an app is installed into an organization, GH will send events related to members being added/removed from repositories, even if those repositories weren't granted access to the app.. so when attempting to sync the collaborators from those repositories, there was an error from GH since we don't have access to those. This is mainly a problem with public repos, as for private repos GH will always return a 404 if the app doesn't have access. This isn't breaking anything from our logic, just creating remote repositories we don't need and filling sentry with errors. ref https://read-the-docs.sentry.io/issues/6701666499/events/0ef3cf2bb9554e39bc2dd9b874aef879/events/?project=148442
1 parent 1976d06 commit 9afa36f

File tree

2 files changed

+95
-1
lines changed

2 files changed

+95
-1
lines changed

readthedocs/oauth/services/githubapp.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,14 @@ def update_or_create_repositories(self, repository_ids: list[int]):
249249
# in order for PyGithub to use the API to fetch the repository by ID (not by name).
250250
# it needs to be an integer, so just in case we cast it to an integer.
251251
repo = self.installation_client.get_repo(int(repository_id))
252+
253+
# GitHub will send some events from all repositories in the organization (like the members event),
254+
# even from those that don't have the app installed. For private repositories, the previous API
255+
# call will fail, but for public repositories we can still hit the API successfully, so we make
256+
# an additional check using the GitHub App API, which will raise a GithubException with a 404
257+
# status code if the app is not installed on the repository.
258+
if not repo.private:
259+
self.gh_app_client.get_repo_installation(owner=repo.owner.login, repo=repo.name)
252260
except GithubException as e:
253261
log.info(
254262
"Failed to fetch repository from GitHub",
@@ -278,7 +286,7 @@ def _create_or_update_repository_from_gh(
278286
target_id = self.installation.target_id
279287
target_type = self.installation.target_type
280288
# NOTE: All the repositories should be owned by the installation account.
281-
# he following condition should never happen, unless the previous assumption is wrong.
289+
# The following condition should never happen, unless the previous assumption is wrong.
282290
if gh_repo.owner.id != target_id or gh_repo.owner.type != target_type:
283291
log.exception(
284292
"Repository owner does not match the installation account",

readthedocs/rtd_tests/tests/test_oauth.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,10 @@ def test_create_repository(self, request):
438438
full_name="user/repo", id=4444, owner={"id": int(self.account.uid)}
439439
),
440440
)
441+
request.get(
442+
f"{self.api_url}/repos/user/repo/installation",
443+
json=self._get_installation_json(id=1111),
444+
)
441445
request.get(
442446
f"{self.api_url}/repos/user/repo/collaborators",
443447
json=[self._get_collaborator_json()],
@@ -468,6 +472,56 @@ def test_create_repository(self, request):
468472
assert relation.account == self.account
469473
assert relation.admin
470474

475+
@requests_mock.Mocker(kw="request")
476+
def test_create_private_repository(self, request):
477+
new_repo_id = 4444
478+
assert not RemoteRepository.objects.filter(
479+
remote_id=new_repo_id, vcs_provider=GitHubAppProvider.id
480+
).exists()
481+
482+
request.post(
483+
f"{self.api_url}/app/installations/1111/access_tokens",
484+
json=self._get_access_token_json(),
485+
)
486+
request.get(
487+
f"{self.api_url}/repositories/4444",
488+
json=self._get_repository_json(
489+
full_name="user/repo",
490+
id=4444,
491+
owner={"id": int(self.account.uid)},
492+
private=True,
493+
),
494+
)
495+
request.get(
496+
f"{self.api_url}/repos/user/repo/collaborators",
497+
json=[self._get_collaborator_json()],
498+
)
499+
500+
service = self.installation.service
501+
service.update_or_create_repositories([new_repo_id])
502+
503+
repo = RemoteRepository.objects.get(
504+
remote_id=new_repo_id, vcs_provider=GitHubAppProvider.id
505+
)
506+
assert repo.name == "repo"
507+
assert repo.full_name == "user/repo"
508+
assert repo.organization is None
509+
assert repo.description == "Some description"
510+
assert repo.avatar_url == "https://github.com/images/error/octocat_happy.gif"
511+
assert repo.ssh_url == "[email protected]:user/repo.git"
512+
assert repo.html_url == "https://github.com/user/repo"
513+
assert repo.clone_url == "https://github.com/user/repo.git"
514+
assert repo.private
515+
assert repo.default_branch == "main"
516+
assert repo.github_app_installation == self.installation
517+
518+
relations = repo.remote_repository_relations.all()
519+
assert relations.count() == 1
520+
relation = relations[0]
521+
assert relation.user == self.user
522+
assert relation.account == self.account
523+
assert relation.admin
524+
471525
@requests_mock.Mocker(kw="request")
472526
def test_update_repository(self, request):
473527
assert self.remote_repository.description == "Some description"
@@ -484,6 +538,38 @@ def test_update_repository(self, request):
484538
description="New description",
485539
),
486540
)
541+
request.get(
542+
f"{self.api_url}/repos/user/repo/installation",
543+
json=self._get_installation_json(id=1111),
544+
)
545+
request.get(
546+
f"{self.api_url}/repos/user/repo/collaborators",
547+
json=[self._get_collaborator_json()],
548+
)
549+
550+
service = self.installation.service
551+
service.update_or_create_repositories([int(self.remote_repository.remote_id)])
552+
553+
self.remote_repository.refresh_from_db()
554+
assert self.remote_repository.description == "New description"
555+
556+
@requests_mock.Mocker(kw="request")
557+
def test_update_private_repository(self, request):
558+
assert self.remote_repository.description == "Some description"
559+
request.post(
560+
f"{self.api_url}/app/installations/1111/access_tokens",
561+
json=self._get_access_token_json(),
562+
)
563+
request.get(
564+
f"{self.api_url}/repositories/{self.remote_repository.remote_id}",
565+
json=self._get_repository_json(
566+
full_name="user/repo",
567+
id=int(self.remote_repository.remote_id),
568+
owner={"id": int(self.account.uid)},
569+
description="New description",
570+
private=True,
571+
),
572+
)
487573
request.get(
488574
f"{self.api_url}/repos/user/repo/collaborators",
489575
json=[self._get_collaborator_json()],

0 commit comments

Comments
 (0)