From e537387c30b1aca1bea9281d3110e77be9e5ecb2 Mon Sep 17 00:00:00 2001 From: Will Simpson Date: Wed, 25 Mar 2026 10:12:29 -0700 Subject: [PATCH 1/3] fix: use getPublicKey to retrieve key algorithms --- internal/server/github/appkey/google_kms.go | 23 ++++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/internal/server/github/appkey/google_kms.go b/internal/server/github/appkey/google_kms.go index 601f67e..0bdd591 100644 --- a/internal/server/github/appkey/google_kms.go +++ b/internal/server/github/appkey/google_kms.go @@ -11,10 +11,10 @@ import ( var _ AppKey = (*GoogleKMS)(nil) type GoogleKMS struct { - clientID string + clientID string + resourceID string - client *cloudkms.KeyManagementClient - keyVersion *kmspb.CryptoKeyVersion + client *cloudkms.KeyManagementClient } func NewGoogleKMS(ctx context.Context, clientID string, resourceID string) (*GoogleKMS, error) { @@ -24,22 +24,25 @@ func NewGoogleKMS(ctx context.Context, clientID string, resourceID string) (*Goo return nil, fmt.Errorf("could not construct KMS client: %w", err) } - keyVersion, err := client.GetCryptoKeyVersion(ctx, &kmspb.GetCryptoKeyVersionRequest{ + // Use GetPublicKey instead of GetCryptoKeyVersion so that the service account + // only needs roles/cloudkms.signerVerifier (which includes viewPublicKey) + // rather than the broader roles/cloudkms.viewer (which includes cryptoKeyVersions.get). + publicKey, err := client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{ Name: resourceID, }) if err != nil { - return nil, fmt.Errorf("could not fetch KMS key metadata: %w", err) + return nil, fmt.Errorf("could not fetch KMS public key: %w", err) } - if keyVersion.Algorithm != kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_2048_SHA256 && - keyVersion.Algorithm != kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_3072_SHA256 && - keyVersion.Algorithm != kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_4096_SHA256 { + if publicKey.Algorithm != kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_2048_SHA256 && + publicKey.Algorithm != kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_3072_SHA256 && + publicKey.Algorithm != kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_4096_SHA256 { return nil, fmt.Errorf("KMS key must be of type RSA_SIGN_PKCS1_*_SHA256") } - return &GoogleKMS{clientID, client, keyVersion}, nil + return &GoogleKMS{clientID, resourceID, client}, nil } func (s *GoogleKMS) ClientID() string { @@ -51,7 +54,7 @@ func (s *GoogleKMS) SignRS256( digest [32]byte, ) ([]byte, error) { signRequest := &kmspb.AsymmetricSignRequest{ - Name: s.keyVersion.Name, + Name: s.resourceID, Digest: &kmspb.Digest{Digest: &kmspb.Digest_Sha256{Sha256: digest[:]}}, } From c36df0b454bfc56c85a41763f50481f4fc10e6a6 Mon Sep 17 00:00:00 2001 From: Will Simpson Date: Wed, 25 Mar 2026 10:45:16 -0700 Subject: [PATCH 2/3] chore: remove comment Future readers probably won't care about the comment and it lives in the commit history anyhow. --- internal/server/github/appkey/google_kms.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/server/github/appkey/google_kms.go b/internal/server/github/appkey/google_kms.go index 0bdd591..031a2a3 100644 --- a/internal/server/github/appkey/google_kms.go +++ b/internal/server/github/appkey/google_kms.go @@ -24,9 +24,6 @@ func NewGoogleKMS(ctx context.Context, clientID string, resourceID string) (*Goo return nil, fmt.Errorf("could not construct KMS client: %w", err) } - // Use GetPublicKey instead of GetCryptoKeyVersion so that the service account - // only needs roles/cloudkms.signerVerifier (which includes viewPublicKey) - // rather than the broader roles/cloudkms.viewer (which includes cryptoKeyVersions.get). publicKey, err := client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{ Name: resourceID, }) From 50e2d189d77dfbc9df8b4f809bab5f0cc0babdff Mon Sep 17 00:00:00 2001 From: Will Simpson Date: Wed, 25 Mar 2026 10:45:48 -0700 Subject: [PATCH 3/3] refactor: rename field to be more explicit --- internal/server/github/appkey/google_kms.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/server/github/appkey/google_kms.go b/internal/server/github/appkey/google_kms.go index 031a2a3..96ac506 100644 --- a/internal/server/github/appkey/google_kms.go +++ b/internal/server/github/appkey/google_kms.go @@ -11,13 +11,13 @@ import ( var _ AppKey = (*GoogleKMS)(nil) type GoogleKMS struct { - clientID string - resourceID string + clientID string + kmsResourceID string client *cloudkms.KeyManagementClient } -func NewGoogleKMS(ctx context.Context, clientID string, resourceID string) (*GoogleKMS, error) { +func NewGoogleKMS(ctx context.Context, clientID string, kmsResourceID string) (*GoogleKMS, error) { client, err := cloudkms.NewKeyManagementClient(ctx) if err != nil { @@ -25,7 +25,7 @@ func NewGoogleKMS(ctx context.Context, clientID string, resourceID string) (*Goo } publicKey, err := client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{ - Name: resourceID, + Name: kmsResourceID, }) if err != nil { @@ -39,7 +39,7 @@ func NewGoogleKMS(ctx context.Context, clientID string, resourceID string) (*Goo return nil, fmt.Errorf("KMS key must be of type RSA_SIGN_PKCS1_*_SHA256") } - return &GoogleKMS{clientID, resourceID, client}, nil + return &GoogleKMS{clientID, kmsResourceID, client}, nil } func (s *GoogleKMS) ClientID() string { @@ -51,7 +51,7 @@ func (s *GoogleKMS) SignRS256( digest [32]byte, ) ([]byte, error) { signRequest := &kmspb.AsymmetricSignRequest{ - Name: s.resourceID, + Name: s.kmsResourceID, Digest: &kmspb.Digest{Digest: &kmspb.Digest_Sha256{Sha256: digest[:]}}, }