feat(git): Let callers map adapter errors to coded errors, like a refused host

src/scm/errors.py

@@ -6,6 +6,7 @@
     "git_operation_not_granted",
     "git_provider_not_supported",
     "git_remote_not_https",
+    "git_remote_refused",
     "git_repository_not_granted",
     "git_route_not_supported",
     "git_token_rejected",
@@ -53,6 +54,7 @@
     "git_operation_not_granted": "The git grant does not allow this operation.",
     "git_provider_not_supported": "The repository's provider has no git remote.",
     "git_remote_not_https": "The repository's git remote is not an https URL.",
+    "git_remote_refused": "The repository's git remote is at an address Sentry does not connect to.",
     "git_repository_not_granted": "The git grant does not cover this repository.",
     "git_route_not_supported": "The request is not a git fetch or push.",
     "git_token_rejected": "The provider rejected Sentry's credentials for the repository.",
@@ -169,6 +171,10 @@ class GitRemoteNotHttps(GitSCMCodedError):
     code = "git_remote_not_https"
 
 
+class GitRemoteRefused(GitSCMCodedError):
+    code = "git_remote_refused"
+
+
 class GitRepositoryNotGranted(GitSCMCodedError):
     code = "git_repository_not_granted"
 

src/scm/git/relay.py

@@ -18,6 +18,7 @@
     ResourceGatewayTimeout,
     ResourceServiceUnavailable,
     RpcMalformedRequestHeaders,
+    SCMCodedError,
 )
 from scm.git.types import GitOperation, Grant, Readable
 from scm.rpc.types import StreamResponse
@@ -107,12 +108,18 @@ class GitRelay:
     ``adapter`` builds the adapter that sends each upstream request. Pass one that refuses internal addresses: a
     GitHub Enterprise or self-managed GitLab host is whatever the customer configured, and the relay connects
     wherever it resolves. Sentry's ``BlacklistAdapter`` does this for its API clients, and the class itself can be
-    passed. An error the adapter raises for a refused host reaches the caller unchanged, unless it is a requests or
-    urllib3 exception, which the relay replaces like any other so that it cannot carry the token. One that reaches
-    the caller does carry it: an adapter refusing the host as it connects raises through urllib3's frames, which hold
-    the Authorization header, so report such an error without its frames' local variables. The adapter must
-    not retry, because a retry cannot resend a body that has already been streamed. Without one, the relay uses a plain
-    ``HTTPAdapter``, which checks nothing.
+    passed. The adapter must not retry, because a retry cannot resend a body that has already been streamed. Without
+    one, the relay uses a plain ``HTTPAdapter``, which checks nothing.
+
+    The relay replaces a requests or urllib3 exception from the upstream request with a coded error, so that it
+    cannot carry the token. ``map_error`` gets any other exception, such as the adapter's own for a refused host,
+    and returns either a coded error, which the relay raises in its place without the original, or None to let the
+    original reach the caller unchanged. An error that reaches the caller this way does carry the token: an adapter
+    refusing the host as it connects raises through urllib3's frames, which hold the Authorization header, so report
+    such an error without its frames' local variables. GitServer answers a coded error like its other refusals. For
+    an adapter that refuses a host with ``HostRefused``::
+
+        GitRelay(adapter=..., map_error=lambda e: GitRemoteRefused() if isinstance(e, HostRefused) else None)
 
     The relay takes no settings from the environment. Through a proxy from ``HTTPS_PROXY``, the adapter would
     check the proxy's address rather than the provider's. ``ca_bundle`` names a CA file to trust instead of the
@@ -123,11 +130,13 @@ def __init__(
         self,
         *,
         adapter: Callable[[], HTTPAdapter] = HTTPAdapter,
+        map_error: Callable[[Exception], SCMCodedError | None] | None = None,
         ca_bundle: str | None = None,
         timeout: tuple[float, float] = DEFAULT_TIMEOUT,
         chunk_size: int = DEFAULT_CHUNK_SIZE,
     ) -> None:
         self.adapter = adapter
+        self.map_error = map_error
         self.ca_bundle = ca_bundle
         self.timeout = timeout
         self.chunk_size = chunk_size
@@ -219,6 +228,16 @@ def forward(
             # Any other failure of the request, e.g. a malformed response. A requests exception, like those above,
             # holds the request.
             raise ResourceBadGateway() from None
+        # The relay's own, such as the client's connection failing while its body is sent.
+        except SCMCodedError:
+            raise
+        except Exception as error:
+            mapped = self.map_error(error) if self.map_error is not None else None
+            if mapped is None:
+                raise
+            # "from None" drops the original. Raised as the adapter connects, e.g. for a refused host, its traceback
+            # runs through urllib3's frames, which hold the Authorization header.
+            raise mapped from None
         finally:
             # Once a response arrives, _stream closes the session when the response ends.
             if upstream is None:

src/scm/git/server.py

@@ -33,6 +33,7 @@
     "missing_repository_path": 500,
     # The provider cannot be reached safely, or failed.
     "git_remote_not_https": 502,
+    "git_remote_refused": 502,
     "git_token_rejected": 502,
     "git_token_unavailable": 502,
     "resource_bad_gateway": 502,
@@ -59,8 +60,8 @@ class GitServer:
     when it failed, as tags: ``repository_id`` as requested, then ``organization_id`` and ``referrer`` once the
     grant is verified, ``operation``, ``provider`` and ``remote_host``. A tag is missing when the request failed
     before it was known. A client closing its request is not reported, since there is nothing to fix. Any other
-    exception, such as an adapter refusing a host or the token supplier failing, reaches the caller unchanged, so
-    the caller can answer it the way it answers the same error elsewhere.
+    exception, such as the token supplier failing, or an error of the relay's adapter that its ``map_error`` leaves
+    alone, reaches the caller unchanged, so the caller can answer it the way it answers the same error elsewhere.
 
     Every count is tagged with the grant's ``referrer`` once the grant is verified, so Sentry can tell a coding
     job's clone from Seer's own fetch or push.

tests/unit/git_upstream.py

@@ -8,6 +8,7 @@
 import datetime
 import functools
 import ipaddress
+import socket
 import ssl
 import sys
 import tempfile
@@ -23,6 +24,8 @@
 from cryptography.x509.oid import NameOID
 from requests.adapters import HTTPAdapter
 from requests.structures import CaseInsensitiveDict
+from urllib3 import HTTPSConnectionPool
+from urllib3.connection import HTTPSConnection
 
 
 @dataclass
@@ -138,6 +141,27 @@ def close(self) -> None:
         super().close()
 
 
+class HostRefused(Exception):
+    """Stands in for Sentry's ``RestrictedIPAddress``, which is neither a requests nor a urllib3 exception."""
+
+
+class _RefusingConnection(HTTPSConnection):
+    def _new_conn(self) -> socket.socket:
+        raise HostRefused(f"{self.host} is an internal address")
+
+
+class _RefusingPool(HTTPSConnectionPool):
+    ConnectionCls = _RefusingConnection
+
+
+class RefusingAdapter(HTTPAdapter):
+    """Refuses every host as it connects, from inside urllib3 the way Sentry's ``BlacklistAdapter`` does."""
+
+    def init_poolmanager(self, *args, **kwargs) -> None:
+        super().init_poolmanager(*args, **kwargs)
+        self.poolmanager.pool_classes_by_scheme = {"https": _RefusingPool}
+
+
 def echo(handler: UpstreamHandler) -> None:
     body = b"".join(handler.body_chunks())
     handler.record(body)

tests/unit/test_git_relay.py

@@ -3,7 +3,7 @@
 import socket
 import threading
 import time
-from collections.abc import Iterator
+from collections.abc import Callable, Iterator
 from unittest.mock import MagicMock
 
 import pytest
@@ -14,6 +14,7 @@
 from scm.errors import (
     GitOperationNotGranted,
     GitRemoteNotHttps,
+    GitRemoteRefused,
     GitRepositoryNotGranted,
     GitRouteNotSupported,
     ResourceBadGateway,
@@ -27,7 +28,15 @@
 from scm.types import GitRemote
 from tests.unit.error_reports import assert_not_in_frames, reported_local
 from tests.unit.git_builders import make_grant
-from tests.unit.git_upstream import ClosingAdapter, UpstreamHandler, basic_auth, ca_bundle, running_upstream
+from tests.unit.git_upstream import (
+    ClosingAdapter,
+    HostRefused,
+    RefusingAdapter,
+    UpstreamHandler,
+    basic_auth,
+    ca_bundle,
+    running_upstream,
+)
 
 TOKEN = Secret("ghs_secrettoken")
 # Long enough that a relay that buffers fails the test instead of hanging it.
@@ -414,16 +423,70 @@ def handler(h: UpstreamHandler) -> None:
 class TestAdapter:
     def test_uses_the_adapter_it_is_given(self, upstream):
         # Sentry passes an adapter that refuses internal addresses; the relay must send through it.
-        class HostRefused(Exception):
-            pass
+        with pytest.raises(HostRefused, match="internal address"):
+            fetch_refs(GitRelay(adapter=RefusingAdapter, ca_bundle=ca_bundle()), upstream)
+        assert upstream.connections == 0
+
+
+def refusal_mapper(seen: list[Exception]) -> Callable[[Exception], SCMCodedError | None]:
+    """A map_error that records each error it gets and maps HostRefused, the way Sentry maps its refusal."""
+
+    def map_error(error: Exception) -> SCMCodedError | None:
+        seen.append(error)
+        return GitRemoteRefused() if isinstance(error, HostRefused) else None
+
+    return map_error
+
 
-        class RefusingAdapter(HTTPAdapter):
+class TestMapError:
+    def test_mapped_error_is_raised_in_place_of_the_original(self, upstream):
+        seen: list[Exception] = []
+        relay = GitRelay(adapter=RefusingAdapter, map_error=refusal_mapper(seen), ca_bundle=ca_bundle())
+        with pytest.raises(GitRemoteRefused) as exc_info:
+            fetch_refs(relay, upstream)
+        assert [type(error) for error in seen] == [HostRefused]
+        # The refusal passes through urllib3's frames, which hold the Authorization header.
+        assert_token_not_in(exc_info.value)
+        # The refusal names the address; the client has no use for it.
+        assert "127.0.0.1" not in exc_info.value.detail
+
+    @pytest.mark.parametrize("map_error", [None, lambda error: None], ids=["no_mapper", "unmapped"])
+    def test_unmapped_error_reaches_the_caller_unchanged(self, upstream, map_error):
+        with pytest.raises(HostRefused, match="internal address"):
+            fetch_refs(GitRelay(adapter=RefusingAdapter, map_error=map_error, ca_bundle=ca_bundle()), upstream)
+
+    @pytest.mark.parametrize(
+        ("make_error", "expected"),
+        [
+            pytest.param(
+                lambda request: requests.ConnectionError("refused", request=request),
+                ResourceServiceUnavailable,
+                id="requests",
+            ),
+            pytest.param(
+                lambda request: urllib3.exceptions.HTTPError("bad response"), ResourceBadGateway, id="urllib3"
+            ),
+        ],
+    )
+    def test_requests_and_urllib3_errors_never_reach_the_mapper(self, upstream, make_error, expected):
+        # A requests exception holds the request and its Authorization header.
+        class FailingAdapter(HTTPAdapter):
             def send(self, request, *args, **kwargs):
-                raise HostRefused(request.url)
+                raise make_error(request)
 
-        with pytest.raises(HostRefused):
-            fetch_refs(GitRelay(adapter=RefusingAdapter, ca_bundle=ca_bundle()), upstream)
-        assert upstream.requests == []
+        seen: list[Exception] = []
+        relay = GitRelay(adapter=FailingAdapter, map_error=refusal_mapper(seen), ca_bundle=ca_bundle())
+        with pytest.raises(expected):
+            fetch_refs(relay, upstream)
+        assert seen == []
+
+    def test_the_relays_own_errors_never_reach_the_mapper(self, upstream):
+        seen: list[Exception] = []
+        relay = GitRelay(map_error=refusal_mapper(seen), ca_bundle=ca_bundle())
+        with pytest.raises(SCMCodedError) as exc_info:
+            upload_pack(relay, upstream, io.BytesIO(b"00"), headers={"Content-Length": "4"})
+        assert exc_info.value.code == "client_closed_request"
+        assert seen == []
 
 
 class TestEnvironment:

tests/unit/test_git_server.py

@@ -7,7 +7,7 @@
 
 import pytest
 
-from scm.errors import ClientClosedRequest, RateLimitExceeded
+from scm.errors import ClientClosedRequest, GitRemoteRefused, RateLimitExceeded
 from scm.git.grant import sign_grant
 from scm.git.relay import GitRelay, RelayResponse, ResponseBody
 from scm.git.server import _STATUS, GitServer
@@ -17,7 +17,7 @@
 from scm.types import GitRemote, Provider, Repository, RepositoryId
 from tests.unit.error_reports import assert_not_in_frames, reported_local
 from tests.unit.git_builders import make_grant, make_provider, make_repository
-from tests.unit.git_upstream import ClosingAdapter, basic_auth, running_upstream
+from tests.unit.git_upstream import ClosingAdapter, HostRefused, RefusingAdapter, basic_auth, running_upstream
 
 SECRET = Secret("test-secret-0000000000000000000000")
 TOKEN = Secret("ghs_secrettoken")
@@ -400,6 +400,22 @@ def test_plain_http_remote(self):
             ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_remote_not_https"})
         ]
 
+    def test_adapter_refusing_the_host(self):
+        # e.g. Sentry's BlacklistAdapter refusing a GitHub Enterprise host that resolves to an internal address.
+        recorder = Recorder(
+            repository=make_repository(provider_name="github_enterprise", web_base_url="https://127.0.0.1:1")
+        )
+        relay = GitRelay(
+            adapter=RefusingAdapter,
+            map_error=lambda error: GitRemoteRefused() if isinstance(error, HostRefused) else None,
+        )
+        response = request(make_server(recorder, relay), signed_grant())
+        assert_refused(response, 502, "at an address Sentry does not connect to")
+        assert recorder.counts == [
+            ("sentry.scm.git.request.failed", {"referrer": REFERRER, "code": "git_remote_refused"})
+        ]
+        assert recorder.error_codes == ["git_remote_refused"]
+
     def test_provider_rejecting_the_token(self):
         # Passed on, a 401 would make git ask the sandboxed client for credentials.
         adapters: list[ClosingAdapter] = []
@@ -474,10 +490,8 @@ def test_no_refusal_is_a_401(self):
         assert 401 not in _STATUS.values()
 
     def test_other_errors_reach_the_caller(self):
-        # Sentry's view answers these the way it answers them elsewhere, e.g. a refused host as ApiHostError.
-        class HostRefused(Exception):
-            pass
-
+        # Sentry's view answers these the way it answers them elsewhere, e.g. an adapter's error that the relay's
+        # map_error leaves alone.
         relay = MagicMock(spec=GitRelay)
         relay.forward.side_effect = HostRefused()
         recorder = Recorder()