From f418b49de3095485cdbc720144a965aa8a7b34b9 Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Tue, 3 Nov 2026 17:59:00 -0500 Subject: [PATCH] fix(github): Percent-encode branch names and refs in URL paths `#` and `%` are legal in git ref names but end or corrupt a URL path, so calls naming a branch like `feature/#123-fix` hit `feature/` instead. Co-Authored-By: Claude Opus 5.5 --- src/scm/providers/github/provider.py | 27 +++++++++++++++--------- tests/unit/provider/test_github.py | 31 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/scm/providers/github/provider.py b/src/scm/providers/github/provider.py index b359174..0d6b1f7 100644 --- a/src/scm/providers/github/provider.py +++ b/src/scm/providers/github/provider.py @@ -3,6 +3,7 @@ from datetime import date, datetime from email.utils import format_datetime, parsedate_to_datetime from typing import Any, Literal, cast +from urllib.parse import quote import msgspec import requests @@ -432,6 +433,12 @@ def _extract_response_meta(response: requests.Response) -> ResponseMeta: return meta +def _quote_ref(ref: str) -> str: + """Percent-encode a branch name, ref or SHA for a URL path; ``#``, ``%`` and ``?`` are legal in + git refs but would otherwise end or corrupt the path. ``/`` stays literal, as GitHub expects.""" + return quote(ref, safe="/") + + class GitHubProvider: def __init__( self, @@ -945,7 +952,7 @@ def get_branch( request_options: RequestOptions | None = None, ) -> ActionResult[GitRef]: response = self.get( - f"/repos/{self.repository['name']}/branches/{branch}", + f"/repos/{self.repository['name']}/branches/{_quote_ref(branch)}", request_options=request_options, ) return map_action(response, lambda r: GitRef(ref=r["name"], sha=r["commit"]["sha"])) @@ -963,7 +970,7 @@ def create_branch(self, branch: BranchName, sha: SHA) -> ActionResult[GitRef]: def update_branch(self, branch: BranchName, sha: SHA, force: bool = False) -> ActionResult[GitRef]: response = self.patch( - f"/repos/{self.repository['name']}/git/refs/heads/{branch}", + f"/repos/{self.repository['name']}/git/refs/heads/{_quote_ref(branch)}", data={"sha": sha, "force": force}, ) return map_action( @@ -972,7 +979,7 @@ def update_branch(self, branch: BranchName, sha: SHA, force: bool = False) -> Ac ) def delete_branch(self, branch: BranchName) -> None: - self.delete(f"/repos/{self.repository['name']}/git/refs/heads/{branch}") + self.delete(f"/repos/{self.repository['name']}/git/refs/heads/{_quote_ref(branch)}") def _assert_branch_head(self, branch: BranchName, expected_head_sha: SHA) -> None: head_sha = self.get_branch(branch)["data"]["sha"] @@ -987,7 +994,7 @@ def get_git_ref( request_options: RequestOptions | None = None, ) -> ActionResult[GitRef]: response = self.get( - f"/repos/{self.repository['name']}/git/ref/{ref}", + f"/repos/{self.repository['name']}/git/ref/{_quote_ref(ref)}", request_options=request_options, ) return map_action(response, lambda r: GitRef(ref=r["ref"], sha=r["object"]["sha"])) @@ -1163,7 +1170,7 @@ def get_commit( request_options: RequestOptions | None = None, ) -> ActionResult[CommitWithChanges]: response = self.get( - f"/repos/{self.repository['name']}/commits/{sha}", + f"/repos/{self.repository['name']}/commits/{_quote_ref(sha)}", request_options=request_options, ) return map_action(response, map_commit_with_changes) @@ -1175,7 +1182,7 @@ def get_commit_changes( request_options: RequestOptions | None = None, ) -> PaginatedActionResult[list[CommitFile]]: response = self.get( - f"/repos/{self.repository['name']}/commits/{sha}", + f"/repos/{self.repository['name']}/commits/{_quote_ref(sha)}", pagination=pagination, request_options=request_options, ) @@ -1243,7 +1250,7 @@ def compare_commits( ) -> PaginatedActionResult[CommitComparison]: """Compare two commits. ``include_behind`` is a no-op: GitHub always returns ``behind_by``.""" response = self.get( - f"/repos/{self.repository['name']}/compare/{start_sha}...{end_sha}", + f"/repos/{self.repository['name']}/compare/{_quote_ref(start_sha)}...{_quote_ref(end_sha)}", pagination=pagination, request_options=request_options, ) @@ -1897,7 +1904,7 @@ def list_check_runs_for_ref( params["filter"] = timestamp_filter response = self.get( - f"/repos/{self.repository['name']}/commits/{ref}/check-runs", + f"/repos/{self.repository['name']}/commits/{_quote_ref(ref)}/check-runs", params=params, pagination=pagination, request_options=request_options, @@ -1951,7 +1958,7 @@ def get_archive_link( request_options: RequestOptions | None = None, ) -> ActionResult[ArchiveLink]: response = self.get( - f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{ref}", + f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{_quote_ref(ref)}", request_options=request_options, allow_redirects=False, ) @@ -1972,7 +1979,7 @@ def download_archive( request_options: RequestOptions | None = None, ) -> requests.Response: return self.get( - f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{ref}", + f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{_quote_ref(ref)}", request_options=request_options, ) diff --git a/tests/unit/provider/test_github.py b/tests/unit/provider/test_github.py index 738722f..1c8e29c 100644 --- a/tests/unit/provider/test_github.py +++ b/tests/unit/provider/test_github.py @@ -1004,6 +1004,14 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]: "raw": BRANCH_RAW, "expected_data": expected_git_ref_from_branch(BRANCH_RAW), }, + { + "name": "get_branch", + "operation": "get", + "kwargs": {"branch": "feature/#12-50%-fix"}, + "path": "/repos/test-org/test-repo/branches/feature/%2312-50%25-fix", + "raw": BRANCH_RAW, + "expected_data": expected_git_ref_from_branch(BRANCH_RAW), + }, { "name": "create_branch", "operation": "post", @@ -1022,6 +1030,15 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]: "raw": GIT_REF_RAW, "expected_data": expected_git_ref(GIT_REF_RAW), }, + { + "name": "update_branch", + "operation": "patch", + "kwargs": {"branch": "feature/#12-50%-fix", "sha": "abc123", "force": True}, + "path": "/repos/test-org/test-repo/git/refs/heads/feature/%2312-50%25-fix", + "data": {"sha": "abc123", "force": True}, + "raw": GIT_REF_RAW, + "expected_data": expected_git_ref(GIT_REF_RAW), + }, { "name": "get_git_ref", "operation": "get", @@ -1030,6 +1047,14 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]: "raw": GIT_REF_RAW, "expected_data": expected_git_ref_full(GIT_REF_RAW), }, + { + "name": "get_git_ref", + "operation": "get", + "kwargs": {"ref": "heads/feature/#12"}, + "path": "/repos/test-org/test-repo/git/ref/heads/feature/%2312", + "raw": GIT_REF_RAW, + "expected_data": expected_git_ref_full(GIT_REF_RAW), + }, { "name": "create_git_blob", "operation": "post", @@ -1377,6 +1402,12 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]: "kwargs": {"branch": "feature"}, "path": "/repos/test-org/test-repo/git/refs/heads/feature", }, + { + "name": "delete_branch", + "operation": "delete", + "kwargs": {"branch": "feature/#12-50%-fix"}, + "path": "/repos/test-org/test-repo/git/refs/heads/feature/%2312-50%25-fix", + }, { "name": "delete_issue_comment", "operation": "delete",