Repository navigation
feat(git)!: Sign git grants with Ed25519 - #170
Merged
Merged
Conversation
Grants only go one way: Seer signs them and Sentry's git endpoint verifies them. With HS256 both sides held the same secret, so anything that can read Sentry's settings could mint a grant for any repository. Security asked why a one-way token uses a shared secret, and Ed25519 was agreed on 2026-10-29. Seer now holds the only private key, and Sentry holds public keys, which are not secret. sign_grant takes an Ed25519 private key in PEM, still wrapped in Secret. verify_grant and GitServer take a list of public keys in PEM and refuse an empty list, a private key, or a key of another type up front. EdDSA is the only accepted algorithm and PyJWT gets parsed key objects, so an HS256 token keyed with a public key, or one with alg none, is refused. The kid is derived from the public key, so Seer needs no kid setting. While rewriting the header check: a grant whose kid is not a string is now refused as malformed (403). Before, PyJWT's InvalidTokenError escaped verify_grant and failed the request with a 500. Breaking: GitServer's secrets parameter is now public_keys, and sign_grant and verify_grant take keys instead of secrets. Fixes CW-2334 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 29, 2026
![semgrep-code-getsentry[bot]](https://avatars.githubusercontent.com/in/1153895?s=60&v=4){kind=small}
…ders PyJWT reads a grant's header before it checks the signature, so anyone who can reach the git endpoint chooses it. Before 2.14.0, a header nested past json's recursion limit (about 27 KB) raised RecursionError, which escaped verify_grant and failed the request with a 500 instead of refusing the grant (GHSA-8wjv-2p76-3863). Sentry already requires pyjwt>=2.13.0 and locks 2.15.0. Seer locks 2.13.0 and moves up when it adopts 3.0.0. The HS256 test now signs with a 32-byte key, so it needs no filter for PyJWT's short-key warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vaind
marked this pull request as ready for review
October 29, 2026 18:19
…ith it A bad --signing-key file ended bin/git-server with a raw traceback. A missing file, a non-PEM or DER file, an encrypted key and a public key each raised from cryptography, and only a key of the wrong type got a one-line error. The CLI also re-implemented the PEM loading and Ed25519 check that sign_grant already does, with messages of its own. public_key_of(private_key) derives the public key through sign_grant's loader. The CLI now reports each bad file in one line with the library's messages. Whoever holds the signing key can also get the public key to list in Sentry without openssl. The test builder keeps serializing keys with cryptography directly. That way the tests compare public_key_of with an independent result rather than with its own output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cmanallen
approved these changes
Oct 29, 2026
This was referenced 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.
Git grants only go one way: Seer signs them, and Sentry's git endpoint verifies them. With HS256 and a shared secret, anything that can read Sentry's settings could mint a grant for any connected repository. Security asked why a one-way token uses a shared secret, and Ed25519 was confirmed in Slack. Seer now holds the only private key and Sentry holds only public keys, as Sentry already does for objectstore tokens and Relay.
Nothing signs or verifies grants in production yet, so HS256 is replaced outright, with no period where both are accepted.
Fixes CW-2334
Approach
EdDSAand gives PyJWT parsed Ed25519 public keys. That refuses HS256 tokens keyed with the public key,alg: none, and Ed448.kidis the first 16 hex characters of the SHA-256 of the raw public key, so neither side needs a kid setting. It isn't an RFC 7638 thumbprint because the kid only picks from the verifier's own list. A thumbprint would add 43 characters to a length-capped grant.GitServeris built, without quoting key material. That covers an empty list, a key that doesn't parse, a key of the wrong type, and a private key among the public ones.public_key_of(private_key)returns the public key to list in Sentry.bin/git-serverloads--signing-keythrough it, so a bad key file gets the library's one-line message.cryptographybecomes a runtime dependency. Seer and Sentry already satisfy>=48.0.1.pyjwtfloor rises from 2.9.0 to 2.14.0. Older versions raiseRecursionErroron a deeply nested header, which anyone can send, and the request fails with a 500 (GHSA-8wjv-2p76-3863). Sentry already locks 2.15.0. Seer locks 2.13.0 and moves up when it adopts 3.0.0.kidisn't a string. Both cases are now refused with a 403.Breaking changes (3.0.0)
sign_grant(Secret(SCM_GIT_GRANT_SIGNING_KEY), grant), with one Ed25519 private key in PEM.GitServer(public_keys=SCM_GIT_GRANT_PUBLIC_KEYS, ...)instead ofsecrets=[...], with a list of PEM public keys not wrapped inSecret.bin/git-server:--secretis replaced by--signing-key, or by a key pair generated at startup.To make a key, run
openssl genpkey -algorithm ed25519 -out grant.pem, thenopenssl pkey -in grant.pem -puboutfor its public key. Not released yet. The!in the title makes craft cut a major version on the next release.Tests
The tests generate their keys, so no private key is committed. They cover:
I also ran
bin/git-serverend to end with a realgit ls-remote.🤖 Generated with Claude Code