feat(git): Add key_id to name a git grant key by its kid

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

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
 
 

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):