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

Author: yajo

src/oca_github_bot/manifest.py

@@ -258,34 +258,95 @@ 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
+        org, repo, username, modified_addons, other_branches, gh=gh
     )
 
 
-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 _get_manifest_from_raw(org, repo, addon, branch, manifest_file):
+    """Fallback to read an addon manifest from github raw URLs.
+
+    Returns the parsed manifest dict, or None if the file does not exist or
+    cannot be read.
+    """
+    url = f"https://github.com/{org}/{repo}/raw/{branch}/{addon}/{manifest_file}"
+    _logger.debug("Looking for maintainers in %s", url)
+    try:
+        r = requests.get(
+            url, allow_redirects=True, headers={"Cache-Control": "no-cache"}
+        )
+    except requests.RequestException as e:
+        _logger.warning("Failed to fetch %s: %s", url, e)
+        return None
+    if not r.ok:
+        return None
+    try:
+        return parse_manifest(r.content)
+    except Exception as e:
+        _logger.warning("Failed to parse manifest from %s: %s", url, e)
+        return None
+
+
+def is_maintainer_other_branches(
+    org, repo, username, modified_addons, other_branches, gh=None
+):
+    """Check if username is maintainer of modified_addons in any configured branch.
+
+    When an authenticated GitHub session is available, the GitHub contents API
+    is used instead of unauthenticated raw.githubusercontent.com requests. This
+    avoids rate-limiting and transient network errors that could spuriously deny
+    maintainer privileges during migrations.
+    """
+    gh_repo = None
+    if gh is not None:
+        try:
+            gh_repo = gh.repository(org, repo)
+        except Exception as e:
+            _logger.warning("Could not get repository %s/%s from API: %s", org, repo, e)
+
     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 = None
+            if gh_repo is not None:
+                manifest = _get_manifest_from_api(gh_repo, addon, branch, manifest_file)
+            if manifest is None:
+                manifest = _get_manifest_from_raw(
+                    org, 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

@@ -249,3 +249,40 @@ def test_is_maintainer_other_branches():
     assert not is_maintainer_other_branches(
         "OCA", "mis-builder", "fpdoo", {"mis_builder"}, ["12.0"]
     )
+
+
+def test_is_maintainer_other_branches_with_gh(mocker):
+    """When an authenticated GitHub session is provided, the API is used.
+
+    The raw URL fallback must not be hit if the API returns a valid manifest.
+    """
+    gh_mock = mocker.MagicMock()
+    file_contents_mock = mocker.MagicMock()
+    file_contents_mock.content = b"{'name': 'addon1', 'maintainers': ['u1']}"
+    gh_repo_mock = mocker.MagicMock()
+    gh_repo_mock.file_contents.return_value = file_contents_mock
+    gh_mock.repository.return_value = gh_repo_mock
+
+    assert is_maintainer_other_branches(
+        "OCA", "repo", "u1", {"addon1"}, ["15.0"], gh=gh_mock
+    )
+    gh_repo_mock.file_contents.assert_called_once_with(
+        "addon1/__manifest__.py", ref="15.0"
+    )
+
+
+def test_is_maintainer_other_branches_api_fallback_to_raw(mocker):
+    """If the GitHub API returns no manifest, the raw URL fallback is used."""
+    gh_mock = mocker.MagicMock()
+    gh_repo_mock = mocker.MagicMock()
+    gh_repo_mock.file_contents.return_value = None
+    gh_mock.repository.return_value = gh_repo_mock
+    raw_mock = mocker.patch(
+        "oca_github_bot.manifest._get_manifest_from_raw",
+        return_value={"name": "addon1", "maintainers": ["u1"]},
+    )
+
+    assert is_maintainer_other_branches(
+        "OCA", "repo", "u1", {"addon1"}, ["15.0"], gh=gh_mock
+    )
+    raw_mock.assert_called_once_with("OCA", "repo", "addon1", "15.0", "__manifest__.py")