[FIX] use authenticated GitHub API to check maintainers in other branches

src/oca_github_bot/manifest.py

@@ -6,8 +6,6 @@
 import os
 import re
 
-import requests
-
 from . import config
 from .github import git_get_current_branch, github_user_can_push
 from .process import check_call, check_output
@@ -258,34 +256,55 @@ def user_can_push(gh, org, repo, username, addons_dir, target_branch):
     if result:
         return True
 
-    other_branches = config.MAINTAINER_CHECK_ODOO_RELEASES
+    other_branches = list(config.MAINTAINER_CHECK_ODOO_RELEASES)
     if target_branch in other_branches:
         other_branches.remove(target_branch)
 
     return is_maintainer_other_branches(
-        org, repo, username, modified_addons, other_branches
+        gh_repo, username, modified_addons, other_branches
     )
 
 
-def is_maintainer_other_branches(org, repo, username, modified_addons, other_branches):
+def _get_manifest_from_api(gh_repo, addon, branch, manifest_file):
+    """Read an addon manifest from a GitHub branch using the authenticated API.
+
+    Returns the parsed manifest dict, or None if the file does not exist or
+    cannot be read.
+    """
+    path = f"{addon}/{manifest_file}"
+    try:
+        file_contents = gh_repo.file_contents(path, ref=branch)
+    except Exception as e:
+        _logger.debug("Could not read %s@%s via GitHub API: %s", path, branch, e)
+        return None
+    if file_contents is None:
+        return None
+    try:
+        return parse_manifest(file_contents.content)
+    except Exception as e:
+        _logger.warning(
+            "Failed to parse manifest %s@%s from GitHub API: %s", path, branch, e
+        )
+        return None
+
+
+def is_maintainer_other_branches(gh_repo, username, modified_addons, other_branches):
+    """Check if username is maintainer of modified_addons in any configured branch.
+
+    The authenticated GitHub contents API is used to read manifests. This
+    avoids rate-limiting and transient network errors that could spuriously deny
+    maintainer privileges during migrations.
+    """
     for addon in modified_addons:
         is_maintainer = False
         for branch in other_branches:
             manifest_file = (
                 "__openerp__.py" if float(branch) < 10.0 else "__manifest__.py"
             )
-            url = (
-                f"https://github.com/{org}/{repo}/raw/{branch}/{addon}/{manifest_file}"
-            )
-            _logger.debug("Looking for maintainers in %s", url)
-            r = requests.get(
-                url, allow_redirects=True, headers={"Cache-Control": "no-cache"}
-            )
-            if r.ok:
-                manifest = parse_manifest(r.content)
-                if username in manifest.get("maintainers", []):
-                    is_maintainer = True
-                    break
+            manifest = _get_manifest_from_api(gh_repo, addon, branch, manifest_file)
+            if manifest and username in manifest.get("maintainers", []):
+                is_maintainer = True
+                break
 
         if not is_maintainer:
             return False

tests/test_manifest.py

@@ -1,6 +1,7 @@
 # Copyright (c) ACSONE SA/NV 2018
 # Distributed under the MIT License (http://opensource.org/licenses/MIT).
 
+import logging
 import subprocess
 
 import pytest
@@ -242,10 +243,51 @@ def test_is_maintainer(tmp_path):
     assert not is_maintainer("u1", [tmp_path / "not_an_addon"])
 
 
-def test_is_maintainer_other_branches():
+def test_is_maintainer_other_branches(mocker):
+    gh_repo_mock = mocker.MagicMock()
+
+    def _file_contents(path, ref=None):
+        # Mimic the real OCA/mis-builder 12.0 manifest maintainers.
+        if ref == "12.0" and path == "mis_builder/__manifest__.py":
+            content = b"{'name': 'mis_builder', 'maintainers': ['sbidoul']}"
+        else:
+            content = b"{'name': 'other'}"
+        file_contents = mocker.MagicMock()
+        file_contents.content = content
+        return file_contents
+
+    gh_repo_mock.file_contents.side_effect = _file_contents
+
     assert is_maintainer_other_branches(
-        "OCA", "mis-builder", "sbidoul", {"mis_builder"}, ["12.0"]
+        gh_repo_mock, "sbidoul", {"mis_builder"}, ["12.0"]
+    )
+    assert not is_maintainer_other_branches(
+        gh_repo_mock, "fpdoo", {"mis_builder"}, ["12.0"]
+    )
+
+
+def test_is_maintainer_other_branches_with_gh(mocker):
+    """The manifest is read through the authenticated GitHub contents API."""
+    gh_repo_mock = mocker.MagicMock()
+    file_contents_mock = mocker.MagicMock()
+    file_contents_mock.content = b"{'name': 'addon1', 'maintainers': ['u1']}"
+    gh_repo_mock.file_contents.return_value = file_contents_mock
+
+    assert is_maintainer_other_branches(gh_repo_mock, "u1", {"addon1"}, ["15.0"])
+    gh_repo_mock.file_contents.assert_called_once_with(
+        "addon1/__manifest__.py", ref="15.0"
     )
+
+
+def test_is_maintainer_other_branches_api_errors(mocker, caplog):
+    """A missing or unparseable manifest is ignored, not raised."""
+    gh_repo_mock = mocker.MagicMock()
+    file_contents_mock = mocker.MagicMock()
+    file_contents_mock.content = b"garbage{"
+    gh_repo_mock.file_contents.side_effect = [None, file_contents_mock]
+    caplog.set_level(logging.WARNING, logger="oca_github_bot.manifest")
+
     assert not is_maintainer_other_branches(
-        "OCA", "mis-builder", "fpdoo", {"mis_builder"}, ["12.0"]
+        gh_repo_mock, "u1", {"addon1"}, ["15.0", "14.0"]
     )
+    assert "Failed to parse manifest addon1/__manifest__.py@14.0" in caplog.text