From 6b7ad72dc9208304fc117325675a46d10a0fffe0 Mon Sep 17 00:00:00 2001 From: Rodos Date: Tue, 3 Nov 2026 17:56:07 +1100 Subject: [PATCH] Skip hooks for repositories without admin access instead of logging a 404 error (#532) Reading hooks needs admin access, and GitHub returns 404 without it. When the repository listing says the token isn't an admin, skip hooks without making the request. The 404 skip message now names the repository and the permission it needs, and the README note about hooks blocking starred backups is updated. --- README.rst | 6 +-- github_backup/github_backup.py | 16 +++++- tests/test_hooks.py | 96 ++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 tests/test_hooks.py diff --git a/README.rst b/README.rst index ccc8cfb..d16ba22 100644 --- a/README.rst +++ b/README.rst @@ -407,11 +407,9 @@ If the incremental argument is used, per-resource checkpoints are only advanced It's therefore recommended to only use the incremental argument if the output/result is being actively monitored, or complimented with periodic full non-incremental runs, to avoid unexpected missing data in a regular backup runs. -**Starred public repo hooks blocking** +**Hooks need admin access** -Since the ``--all`` argument includes ``--hooks``, if you use ``--all`` and ``--all-starred`` together to clone a users starred public repositories, the backup will likely error and block the backup continuing. - -This is due to needing the correct permission for ``--hooks`` on public repos. +Reading a repository's hooks requires admin access to it, and fine-grained tokens also need the Webhooks read permission. Since the ``--all`` argument includes ``--hooks``, repositories you don't administer, such as starred repositories with ``--all-starred``, are skipped for hooks and the rest of the backup continues. "bare" is actually "mirror" diff --git a/github_backup/github_backup.py b/github_backup/github_backup.py index a8297c9..9fbfacf 100644 --- a/github_backup/github_backup.py +++ b/github_backup/github_backup.py @@ -3155,6 +3155,15 @@ def backup_hooks(args, repo_cwd, repository, repos_template): if not auth: logger.info("Skipping hooks since no authentication provided") return + # Reading hooks needs admin access, and GitHub returns 404 without it. + # Skip the request when the repository listing says we aren't an admin. + if repository.get("permissions", {}).get("admin") is False: + logger.info( + "Skipping hooks for {0} since admin access is required".format( + repository["full_name"] + ) + ) + return hook_cwd = os.path.join(repo_cwd, "hooks") output_file = "{0}/hooks.json".format(hook_cwd) template = "{0}/{1}/hooks".format(repos_template, repository["full_name"]) @@ -3162,7 +3171,12 @@ def backup_hooks(args, repo_cwd, repository, repos_template): _backup_data(args, "hooks", template, output_file, hook_cwd) except HTTPError as e: if e.code == 404: - logger.info("Unable to read hooks, skipping") + logger.info( + "Unable to read hooks for {0}, skipping (requires admin access, " + "or Webhooks read permission for fine-grained tokens)".format( + repository["full_name"] + ) + ) else: raise e diff --git a/tests/test_hooks.py b/tests/test_hooks.py new file mode 100644 index 0000000..4869a7f --- /dev/null +++ b/tests/test_hooks.py @@ -0,0 +1,96 @@ +"""Tests for hooks backup permissions (GitHub issue #532). + +Reading ``/repos/{owner}/{repo}/hooks`` needs admin access to the repository. +Without it GitHub responds with HTTP 404 rather than 403, which used to log an +API error for every repository the user doesn't administer. +""" + +import logging +from urllib.error import HTTPError + +import pytest + +from github_backup import github_backup + +REPOS_TEMPLATE = "https://api.github.com/repos" + + +@pytest.fixture +def hook_args(create_args): + return create_args(include_hooks=True, token_classic="ghp_token") + + +def _repository(**fields): + return {"full_name": "owner/repo", **fields} + + +def test_hooks_skipped_without_request_when_not_admin( + hook_args, tmp_path, monkeypatch, caplog +): + def fail_retrieve_data(*args, **kwargs): + raise AssertionError("hooks should not be requested without admin access") + + monkeypatch.setattr(github_backup, "retrieve_data", fail_retrieve_data) + + with caplog.at_level(logging.INFO, logger=github_backup.logger.name): + github_backup.backup_hooks( + hook_args, + tmp_path, + _repository(permissions={"admin": False, "push": True, "pull": True}), + REPOS_TEMPLATE, + ) + + assert "Skipping hooks for owner/repo since admin access is required" in caplog.text + assert not (tmp_path / "hooks").exists() + + +@pytest.mark.parametrize( + "repository", + [ + _repository(permissions={"admin": True, "push": True, "pull": True}), + _repository(), + ], + ids=["admin", "no-permissions-field"], +) +def test_hooks_requested_when_admin_or_permissions_unknown( + hook_args, tmp_path, monkeypatch, repository +): + requested = [] + + def fake_retrieve_data(passed_args, template, *args, **kwargs): + requested.append(template) + return [{"id": 1, "name": "web"}] + + monkeypatch.setattr(github_backup, "retrieve_data", fake_retrieve_data) + + github_backup.backup_hooks(hook_args, tmp_path, repository, REPOS_TEMPLATE) + + assert requested == ["https://api.github.com/repos/owner/repo/hooks"] + assert (tmp_path / "hooks" / "hooks.json").exists() + + +def test_hooks_404_is_skipped_with_reason(hook_args, tmp_path, monkeypatch, caplog): + def fake_retrieve_data(passed_args, template, *args, **kwargs): + raise HTTPError(template, 404, "Not Found", hdrs=None, fp=None) + + monkeypatch.setattr(github_backup, "retrieve_data", fake_retrieve_data) + + with caplog.at_level(logging.INFO, logger=github_backup.logger.name): + github_backup.backup_hooks( + hook_args, + tmp_path, + _repository(permissions={"admin": True}), + REPOS_TEMPLATE, + ) + + assert "Unable to read hooks for owner/repo, skipping" in caplog.text + + +def test_hooks_non_404_error_propagates(hook_args, tmp_path, monkeypatch): + def fake_retrieve_data(passed_args, template, *args, **kwargs): + raise HTTPError(template, 500, "Server Error", hdrs=None, fp=None) + + monkeypatch.setattr(github_backup, "retrieve_data", fake_retrieve_data) + + with pytest.raises(HTTPError): + github_backup.backup_hooks(hook_args, tmp_path, _repository(), REPOS_TEMPLATE)