Repository navigation
feat(git): Let callers map adapter errors to coded errors, like a refused host - #168
Merged
Merged
Conversation
Sentry builds its relay with BlacklistAdapter, which refuses a GitHub Enterprise or self-managed GitLab host that resolves to an internal address by raising its own RestrictedIPAddress from inside urllib3's connect. That error is neither coded nor a requests exception, so GitServer counted it as unhandled_exception and re-raised it, and Sentry had to catch, report and answer it itself. scm can't import Sentry's exception, so the caller names it in GitRelay(refused_host_errors=...), and the relay raises GitRemoteRefused in its place. GitServer then counts and answers it like its other refusals. The original is dropped with "from None" because its traceback runs through urllib3 frames that hold the Authorization header, and the new error says nothing of the address. The constructor checks the tuple because an except clause with anything else fails only when a request does, as a TypeError chained to that request's error.
A fixed list of refused-host exception types only covers that one case. A map_error callable lets the caller turn any error of its adapter into a coded error, while scm keeps owning the error classes and their status. The relay's own conversions of requests and urllib3 errors, and its own coded errors, still run first, so the mapper never sees an exception that holds the Authorization header and can't relabel the client going away. A mapped error is raised "from None" because a refusal raised as urllib3 connects carries frames that hold the header.
vaind
changed the title
vaind
marked this pull request as ready for review
October 27, 2026 22:53
…t-relay-refused-host # Conflicts: # src/scm/git/relay.py
giovanni-guidini
approved these changes
Oct 29, 2026
vaind
added a commit
that referenced
this pull request
Oct 30, 2026
… failures (#171) Porting Sentry and Seer to the Ed25519 grants from #170 found gaps that should be fixed before 3.0.0: - A bad key raised plain `ValueError`/`TypeError` from the same calls that parse untrusted grants, so callers couldn't tell their own misconfiguration apart from a bad or oversized grant. - There was no helper to make a key, so `bin/git-server` and every test suite copied the same `cryptography` calls. - Three permanent failures answered 502, which Seer's checkout (getsentry/seer#9229) retries for about a minute before failing. Follow-up to #170 (CW-2334). ## Approach - New `scm.errors.GitGrantKeyError(SCMError, ValueError)` for every bad key, on both sides. It isn't an `SCMCodedError`, because a bad key is the server's fault: `GitServer` raises it from its constructor rather than answering requests with a 403. An oversized grant still raises a plain `ValueError`. - New `require_private_key(Secret) -> Secret`, the signer's counterpart to `require_public_keys`, to check a key at startup. - New `generate_private_key() -> Secret`, which replaces the copied `cryptography` calls in `bin/git-server` and `make_key_pair`. - `GitServer` answers `git_remote_not_https`, `git_remote_refused` and `resource_certificate_invalid` with a 500 instead of a 502. The integration or host is misconfigured, so no retry helps. 502 still means the provider or the token failed; 503 and 504 still mean a retry can succeed. - `GetToken` and `GitServer` now document that `get_token` may raise a coded error, which `GitServer` answers by its code. Sentry relies on this to answer an unreachable control silo with a 503. Not included, as decided: a public `key_id()`, or reporting which key matched a grant. ## Release notes Craft uses this section for the changelog in place of the title. The last item covers earlier PRs whose titles don't show a breaking change. ### Changelog Entry - Raise `GitGrantKeyError` for a missing or unusable git grant key, so callers can tell their own misconfiguration apart from a bad grant - It's a `ValueError`, not an `SCMCodedError`. A public key that isn't a string used to raise `TypeError`. - `verify_grant([], token)` raises it. 2.x refused the grant with `GitInvalidGrant` (a 403 from `GitServer`). - Add `require_private_key` to check a signing key at startup, and `generate_private_key` to make one for tests - Answer a remote that isn't https, a refused remote host and an untrusted TLS certificate with a 500 instead of a 502, since no retry can fix them - Document that `get_token` may raise a coded error, which `GitServer` answers by its code, e.g. `ResourceServiceUnavailable` as a 503 - Upgrade notes for earlier 3.0.0 changes - `GitServer`'s `get_token` must return an `scm.Secret`, or every relayed request fails with `TypeError` ([#167](#167)). - Once Sentry maps `RestrictedIPAddress` to `GitRemoteRefused` with `GitRelay(map_error=...)`, its refusal of internal addresses changes from a 503 to a 500 ([#168](#168)). ## Tests Each bad-key case raises `GitGrantKeyError` on both sides, and no message quotes the key. That check also now works for multi-line keys: it used to join the lines, so it never matched RSA, EC or Ed448 keys. A generated key round-trips through `sign_grant`, `public_key_of` and `verify_grant`. New `GitServer` tests cover the three 500s and a coded and an uncoded error from `get_token`. The suite, ruff, mypy and prek pass, and I smoke-tested `bin/git-server`'s key loading. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
3 tasks done
vaind
pushed a commit
to getsentry/sentry
that referenced
this pull request
Oct 30, 2026
Bumps `sentry-scm` from 2.0.0 to [3.0.0](https://github.com/getsentry/scm-platform/releases/tag/3.0.0). The lockfile also picks up `cryptography` as a direct dependency of sentry-scm; the locked `cryptography` (50.0.0) and `pyjwt` (2.15.0) already meet 3.0.0's new floors (`>=48.0.1`, `>=2.14.0`). No code changes are needed on master. The 3.0.0 breaking changes are all in `scm.git` (Ed25519 grants, `GitServer(public_keys=...)`, `get_token` returning an `scm.Secret`, `GitRelay(map_error=...)`, `GitGrantKeyError`, and the 502 → 500 status changes), and nothing on master uses `scm.git` yet. The only user is the git endpoint in #128444, which is already written against 3.0.0 and only needs this bump to pass CI. The one other change that touches master's code, getsentry/scm-platform#169, is backward compatible for `RpcServer`'s callers. Landing this first lets #128444 drop its "bump sentry-scm" checklist item after a rebase. If you'd rather keep the bump inside #128444, close this one. This PR needs no deploy config or secrets. `SCM_GIT_GRANT_PUBLIC_KEYS` (Ed25519 public keys as PEM, matching Seer's signing key) comes in with #128444. Upstream: getsentry/scm-platform#168, getsentry/scm-platform#170, getsentry/scm-platform#171 Refs CW-2222 <!-- junior-request-attribution:start --> via **Ivan Dlugos**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A19a84388325b8be5736899297f85ece4 --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3A19a84388325b8be5736899297f85ece4) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A19a84388325b8be5736899297f85ece4/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sentry's git endpoint (getsentry/sentry#128444) relays git requests through
GitRelaywith Sentry'sBlacklistAdapter, so a GitHub Enterprise or self-managed GitLab host that resolves to an internal address is refused. The adapter refuses by raising Sentry's ownRestrictedIPAddress. scm doesn't know that exception, soGitServercounts the refusal asunhandled_exceptionand re-raises it, and Sentry's view has to catch, report and answer it itself.Approach
GitRelay(map_error=...)takes a function that gets an exception from the upstream request and returns either a coded error to raise in its place orNoneto let the original through unchanged. scm can't import Sentry's exception, so the caller decides which of its adapter's errors mean what. Without a mapper, nothing changes.ResourceGatewayTimeout,ResourceServiceUnavailableand the rest, and the relay's own coded errors (such as the client going away mid-body) pass straight through. So the mapper never sees an exception that holds theAuthorizationheader, and can't relabel those.from None. A refusal raised as urllib3 connects has a traceback that runs through urllib3 frames holding theAuthorizationheader.GitRemoteRefused(git_remote_refused) for a refused host. Its message doesn't name the host or address, like the relay's other errors.GitServeranswers it with a 502, next togit_remote_not_https, the other remote the relay won't connect to. A 503 there means "temporarily unavailable, the request can be retried" (resource_service_unavailable), and retrying a refused host never helps. Sentry answers this case with a 503 today; after the bump it becomes a 502.Usage, for an adapter that refuses with
HostRefused:Tests
A test adapter refuses the way
BlacklistAdapterdoes, from inside urllib3's connect.GitRemoteRefusedwith no cause, a suppressed context, and neither the token nor the address.GitServeranswers it with a 502 and countsgit_remote_refusedrather thanunhandled_exception.None, the original propagates as raised.client_closed_requestnever reach the mapper.Dropping
from None, the_STATUSentry or the pass-through of the relay's own errors, or calling the mapper before the requests handling, each fails a test.After release
featmakes this a minor release. Sentry's bump PR then passesmap_error=lambda e: GitRemoteRefused() if isinstance(e, RestrictedIPAddress) else Noneinmake_git_server()and drops the view's ownexcept RestrictedIPAddressbranch.