From b6430dbfc6fd1f50c9bf3dfdbf1cb1cf4bf48787 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Nov 2026 19:26:35 +0100 Subject: [PATCH] feat(git): Add key_id to name a git grant key by its kid Consumers that log which key signed a grant, like Seer during a key rotation, had to parse the JWT header with PyJWT to read the kid. The kid is derived from the key, so the library can hand it out directly and keep the derivation in one place. A signer gets its own id with key_id(public_key_of(private_key)) rather than a second, private-key form: the composition is one line and keeps the API to one function. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/scm/git/grant.py | 27 +++++++++++----- tests/unit/test_git_grant.py | 61 +++++++++++++++++++++++++++++++----- 3 files changed, 74 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 36cbaf84..64ba9186 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The platform exposes four subsystems: - **Actions** — outbound SCM operations initiated by Sentry code. The `SourceCodeManager` class provides 70+ methods covering comments, reactions, pull requests, branches, git objects, reviews, and check runs. With more actions planned to be added as we port more use cases. - **Actions RPC** - outbound SCM commands exposed over the network. The `SourceCodeManager` is fully available over the network. This enables usage of the SCM for services outside the monolith. -- **Git relay** - `git` fetches and pushes for services that hold no provider credentials. Seer signs a grant naming the organization, operation and repositories with `scm.git.grant.sign_grant` and an Ed25519 private key only Seer holds. `scm.git.server.GitServer` verifies the grant against the matching public keys, so whoever verifies grants cannot mint one, and checks each git smart-HTTP request against it before relaying the request to the provider with a token, streaming both bodies. A missing or unusable key on either side raises `scm.errors.GitGrantKeyError`, the server's misconfiguration rather than a refused grant. `require_private_key` and `require_public_keys` check the keys at startup, and `generate_private_key` makes a key for tests and local tools. +- **Git relay** - `git` fetches and pushes for services that hold no provider credentials. Seer signs a grant naming the organization, operation and repositories with `scm.git.grant.sign_grant` and an Ed25519 private key only Seer holds. `scm.git.server.GitServer` verifies the grant against the matching public keys, so whoever verifies grants cannot mint one, and checks each git smart-HTTP request against it before relaying the request to the provider with a token, streaming both bodies. A missing or unusable key on either side raises `scm.errors.GitGrantKeyError`, the server's misconfiguration rather than a refused grant. `require_private_key` and `require_public_keys` check the keys at startup, and `generate_private_key` makes a key for tests and local tools. `key_id` gives a public key's `kid`, the id each grant names its signing key by, so a log can name the key that signed a grant without quoting it. - **Event Stream** — inbound webhook processing. SCM service-providers push events which are deserialized into typed, provider-neutral dataclasses (`CheckRunEvent`, `CommentEvent`, `PullRequestEvent`) which are then dispatched to registered listener functions. ### Why diff --git a/src/scm/git/grant.py b/src/scm/git/grant.py index 400ee2a9..7763458b 100644 --- a/src/scm/git/grant.py +++ b/src/scm/git/grant.py @@ -10,6 +10,8 @@ Keys are PEM strings, as for objectstore's tokens: the private key as PKCS#8 (``-----BEGIN PRIVATE KEY-----``), public keys as SubjectPublicKeyInfo (``-----BEGIN PUBLIC KEY-----``). ``openssl genpkey -algorithm ed25519`` or ``generate_private_key`` makes a private key, and ``openssl pkey -pubout`` or ``public_key_of`` gives its public key. +``key_id`` gives a public key's ``kid``, so a signer can log which key signed a grant and an operator can tell the keys +in the verifier's list apart. A key that is missing or unusable raises ``GitGrantKeyError`` from every function that takes it, whichever side it is on. ``require_private_key`` and ``require_public_keys`` check the keys on their own, so a signer or a verifier @@ -79,6 +81,16 @@ def public_key_of(private_key: Secret) -> str: return public_key.public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo).decode() +def key_id(public_key: str) -> str: + """Return the ``kid`` that grants signed by ``public_key``'s private key carry, for an Ed25519 public key in PEM. + + The id is derived from the key, so it names a key in logs without quoting it. A signer, which holds only its + private key, gets its own with ``key_id(public_key_of(private_key))``. Raises GitGrantKeyError, as + ``verify_grant`` does, if ``public_key`` is not an Ed25519 public key in PEM. + """ + return _key_id(_load_public_key(public_key, "The git grant public key")) + + def generate_private_key() -> Secret: """Return a new Ed25519 private key to sign grants with, in PEM (PKCS#8). @@ -195,28 +207,27 @@ def _public_keys_by_id(public_keys: Sequence[str]) -> dict[str, Ed25519PublicKey raise GitGrantKeyError("There are no git grant public keys, so no grant could verify.") keys: dict[str, Ed25519PublicKey] = {} for index, pem in enumerate(public_keys): - key = _load_public_key(pem, index) + key = _load_public_key(pem, f"public_keys[{index}]") keys[_key_id(key)] = key return keys -def _load_public_key(pem: str, index: int) -> Ed25519PublicKey: - # Each message names the key by its place in the list rather than quoting it. +def _load_public_key(pem: str, name: str) -> Ed25519PublicKey: + # Each message names the key, e.g. by its place in the list, rather than quoting it. if not isinstance(pem, str): - raise GitGrantKeyError(f"public_keys[{index}] must be a PEM string, not {type(pem).__name__}.") + raise GitGrantKeyError(f"{name} must be a PEM string, not {type(pem).__name__}.") # load_pem_public_key refuses a private key too, but saying what it is tells the operator to move it away from # where verifiers read their keys. if "PRIVATE KEY" in pem: raise GitGrantKeyError( - f"public_keys[{index}] is a private key. Verifiers take public keys only; the private key stays with the " - "signer." + f"{name} is a private key. Verifiers take public keys only; the private key stays with the signer." ) try: key = load_pem_public_key(pem.encode()) except (ValueError, UnsupportedAlgorithm) as e: - raise GitGrantKeyError(f"public_keys[{index}] is not a public key in PEM (-----BEGIN PUBLIC KEY-----).") from e + raise GitGrantKeyError(f"{name} is not a public key in PEM (-----BEGIN PUBLIC KEY-----).") from e if not isinstance(key, Ed25519PublicKey): - raise GitGrantKeyError(f"public_keys[{index}] must be an Ed25519 key, not {type(key).__name__}.") + raise GitGrantKeyError(f"{name} must be an Ed25519 key, not {type(key).__name__}.") return key diff --git a/tests/unit/test_git_grant.py b/tests/unit/test_git_grant.py index f939281e..a8d07ff8 100644 --- a/tests/unit/test_git_grant.py +++ b/tests/unit/test_git_grant.py @@ -24,6 +24,7 @@ from scm.git.grant import ( GRANT_AUDIENCE, generate_private_key, + key_id, public_key_of, require_private_key, require_public_keys, @@ -43,7 +44,8 @@ OTHER_KEY = make_key_pair() -def key_id(key: KeyPair) -> str: +def expected_key_id(key: KeyPair) -> str: + """The kid derived here from the raw key, so the tests check key_id and sign_grant against it.""" raw = load_pem_public_key(key.public.encode()).public_bytes(Encoding.Raw, PublicFormat.Raw) return hashlib.sha256(raw).hexdigest()[:16] @@ -56,7 +58,7 @@ def claims_of(grant: Grant) -> dict: def sign_claims(claims: dict, key: Any = KEY.key, algorithm: str = "EdDSA", kid: str | None = None) -> str: """Sign arbitrary claims the way sign_grant does, to reach the checks that run after the signature.""" - return jwt.encode(claims, key, algorithm=algorithm, headers={"kid": kid or key_id(KEY)}) + return jwt.encode(claims, key, algorithm=algorithm, headers={"kid": kid or expected_key_id(KEY)}) def b64url(data: bytes) -> str: @@ -121,7 +123,7 @@ def test_push_grant(self): def test_token_is_a_jwt_with_the_grants_claims(self): token = sign_grant(KEY.private, make_grant(), now=NOW) - assert jwt.get_unverified_header(token) == {"alg": "EdDSA", "typ": "JWT", "kid": key_id(KEY)} + assert jwt.get_unverified_header(token) == {"alg": "EdDSA", "typ": "JWT", "kid": expected_key_id(KEY)} assert jwt.decode(token, options={"verify_signature": False}) == { "organization_id": 1, "referrer": "seer.coding_job", @@ -136,8 +138,8 @@ def test_token_is_a_jwt_with_the_grants_claims(self): def test_key_id_is_the_same_for_every_grant_a_key_signs(self): # The signer derives it from its private key and the verifier from the public key, with no setting to match. tokens = [sign_grant(KEY.private, make_grant(organization_id=i), now=NOW) for i in (1, 2)] - assert {jwt.get_unverified_header(token)["kid"] for token in tokens} == {key_id(KEY)} - assert key_id(KEY) != key_id(OTHER_KEY) + assert {jwt.get_unverified_header(token)["kid"] for token in tokens} == {expected_key_id(KEY)} + assert expected_key_id(KEY) != expected_key_id(OTHER_KEY) def test_public_key_with_surrounding_whitespace(self): # As a setting read from a file or an environment variable may have. @@ -243,7 +245,7 @@ def test_hs256_keyed_with_the_public_key(self): # The public key is no secret, so a verifier that let a token pick HS256 and used the key it holds as the HMAC # key would accept a grant from anyone. PyJWT refuses to sign with a PEM as an HMAC key, so it is made here. token = handmade_token( - {"alg": "HS256", "typ": "JWT", "kid": key_id(KEY)}, + {"alg": "HS256", "typ": "JWT", "kid": expected_key_id(KEY)}, claims_of(make_grant()), lambda signing_input: hmac.digest(KEY.public.encode(), signing_input, "sha256"), ) @@ -255,7 +257,7 @@ def test_hs256(self): assert_invalid(token, "alg value is not allowed") def test_unsigned_token(self): - token = jwt.encode(claims_of(make_grant()), None, algorithm="none", headers={"kid": key_id(KEY)}) + token = jwt.encode(claims_of(make_grant()), None, algorithm="none", headers={"kid": expected_key_id(KEY)}) assert_invalid(token, "alg value is not allowed") def test_other_asymmetric_algorithm(self): @@ -421,6 +423,51 @@ def test_other_key_type(self, load, key): assert_quotes_no_key(exc_info.value, pem) +class TestKeyId: + def test_kid_of_the_grants_the_key_signs(self): + token = sign_grant(KEY.private, make_grant(), now=NOW) + assert key_id(KEY.public) == jwt.get_unverified_header(token)["kid"] == expected_key_id(KEY) + + def test_signer_gets_its_own_from_its_private_key(self): + private_key = generate_private_key() + token = sign_grant(private_key, make_grant(), now=NOW) + assert key_id(public_key_of(private_key)) == jwt.get_unverified_header(token)["kid"] + + def test_differs_between_keys(self): + assert key_id(KEY.public) != key_id(OTHER_KEY.public) + + def test_public_key_with_surrounding_whitespace(self): + assert key_id(f"\n {KEY.public}\n\n") == key_id(KEY.public) + + def test_private_key(self): + pem = KEY.private.reveal() + with pytest.raises(GitGrantKeyError, match=r"^The git grant public key is a private key\.") as exc_info: + key_id(pem) + assert_quotes_no_key(exc_info.value, pem) + + def test_private_key_as_a_secret(self): + with pytest.raises(GitGrantKeyError, match=r"^The git grant public key must be a PEM string, not Secret\."): + key_id(KEY.private) # type: ignore[arg-type] + + @pytest.mark.parametrize( + "pem", + ["", "not a key", OTHER_KEY.public[:60], pem_body(OTHER_KEY.public)], + ids=["empty", "garbage", "truncated", "no-delimiters"], + ) + def test_not_a_public_key(self, pem): + with pytest.raises(GitGrantKeyError, match=r"^The git grant public key is not a public key in PEM") as exc_info: + key_id(pem) + assert_quotes_no_key(exc_info.value, pem) + + @pytest.mark.parametrize("key", other_key_types(), ids=["rsa", "ec", "ed448"]) + def test_other_key_type(self, key): + pem = public_pem(key) + expected = f"^The git grant public key must be an Ed25519 key, not {type(key.public_key()).__name__}" + with pytest.raises(GitGrantKeyError, match=expected) as exc_info: + key_id(pem) + assert_quotes_no_key(exc_info.value, pem) + + class TestRotatedKeys: @pytest.mark.parametrize("signer", [OLD_KEY, NEW_KEY], ids=["old", "new"]) def test_both_keys_verify_during_rotation(self, signer):