From 5c437a887595846af0ac053d0b03bd0a5783295d Mon Sep 17 00:00:00 2001 From: Brian DeHamer Date: Thu, 14 May 2026 10:31:10 -0700 Subject: [PATCH 1/3] Derive digest algorithm from ref length in release verify commands The 'gh release verify' and 'gh release verify-asset' commands hard-coded a 'sha1:' prefix when constructing the digest identifier for a release tag's commit SHA. Once GitHub repositories using SHA-256 commit digests are supported, that ref will be a 64-character SHA-256 hash and labeling it as 'sha1:' is both misleading in user output and incorrect for the attestation lookup. Add a shared 'DigestAlgForRef' helper that returns 'sha256' for 64-char digests and 'sha1' otherwise (preserving existing behavior for SHA-1 repositories), and use it at both call sites. Add test coverage for the helper and for the SHA-256 error path in both commands. Fixes #13429 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/release/shared/fetch.go | 13 +++++++ pkg/cmd/release/shared/fetch_test.go | 35 +++++++++++++++++++ pkg/cmd/release/verify-asset/verify_asset.go | 2 +- .../release/verify-asset/verify_asset_test.go | 32 +++++++++++++++++ pkg/cmd/release/verify/verify.go | 2 +- pkg/cmd/release/verify/verify_test.go | 29 +++++++++++++++ 6 files changed, 111 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/release/shared/fetch.go b/pkg/cmd/release/shared/fetch.go index e1f155d046b..f2e370ecf96 100644 --- a/pkg/cmd/release/shared/fetch.go +++ b/pkg/cmd/release/shared/fetch.go @@ -170,6 +170,19 @@ func FetchRefSHA(ctx context.Context, httpClient *http.Client, repo ghrepo.Inter return ref.Object.SHA, nil } +// DigestAlgForRef returns the digest algorithm name corresponding to the given +// git ref SHA. SHA-1 git object IDs are 40 hex characters and SHA-256 git +// object IDs are 64 hex characters. Unknown lengths default to "sha1" to +// preserve backwards-compatible behavior. +func DigestAlgForRef(digest string) string { + switch len(digest) { + case 64: + return "sha256" + default: + return "sha1" + } +} + // FetchRelease finds a published repository release by its tagName, or a draft release by its pending tag name. func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Interface, tagName string) (*Release, error) { cc, cancel := context.WithCancel(ctx) diff --git a/pkg/cmd/release/shared/fetch_test.go b/pkg/cmd/release/shared/fetch_test.go index 9b4e5df8083..0720b876f6f 100644 --- a/pkg/cmd/release/shared/fetch_test.go +++ b/pkg/cmd/release/shared/fetch_test.go @@ -92,3 +92,38 @@ func TestFetchRefSHA(t *testing.T) { }) } } + +func TestDigestAlgForRef(t *testing.T) { + tests := []struct { + name string + digest string + expected string + }{ + { + name: "sha1 (40 hex chars)", + digest: "1234567890abcdef1234567890abcdef12345678", + expected: "sha1", + }, + { + name: "sha256 (64 hex chars)", + digest: "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + expected: "sha256", + }, + { + name: "empty string defaults to sha1", + digest: "", + expected: "sha1", + }, + { + name: "unexpected length defaults to sha1", + digest: "abc", + expected: "sha1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, DigestAlgForRef(tt.digest)) + }) + } +} diff --git a/pkg/cmd/release/verify-asset/verify_asset.go b/pkg/cmd/release/verify-asset/verify_asset.go index acd8a134e8e..2cebce53bc8 100644 --- a/pkg/cmd/release/verify-asset/verify_asset.go +++ b/pkg/cmd/release/verify-asset/verify_asset.go @@ -142,7 +142,7 @@ func verifyAssetRun(config *VerifyAssetConfig) error { return err } - releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, "sha1") + releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, shared.DigestAlgForRef(ref)) // Find attestations for the release tag SHA attestations, err := config.AttClient.GetByDigest(api.FetchParams{ diff --git a/pkg/cmd/release/verify-asset/verify_asset_test.go b/pkg/cmd/release/verify-asset/verify_asset_test.go index 530f478ed16..8e3654119ce 100644 --- a/pkg/cmd/release/verify-asset/verify_asset_test.go +++ b/pkg/cmd/release/verify-asset/verify_asset_test.go @@ -197,6 +197,38 @@ func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) { require.ErrorContains(t, err, "no attestations found for tag v1") } +func Test_verifyAssetRun_FailedNoAttestations_SHA256(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tagName := "v1" + + fakeHTTP := &httpmock.Registry{} + defer fakeHTTP.Verify(t) + fakeSHA := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" + shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA) + + baseRepo, err := ghrepo.FromFullName("owner/repo") + require.NoError(t, err) + + releaseAssetPath := test.NormalizeRelativePath("../../attestation/test/data/github_release_artifact.zip") + + cfg := &VerifyAssetConfig{ + Opts: &VerifyAssetOptions{ + AssetFilePath: releaseAssetPath, + TagName: tagName, + BaseRepo: baseRepo, + Exporter: nil, + }, + IO: ios, + HttpClient: &http.Client{Transport: fakeHTTP}, + AttClient: api.NewFailTestClient(), + AttVerifier: nil, + } + + err = verifyAssetRun(cfg) + require.ErrorContains(t, err, "no attestations found for tag v1") + require.ErrorContains(t, err, "sha256:"+fakeSHA) +} + func Test_verifyAssetRun_FailedTagNotInAttestation(t *testing.T) { ios, _, _, _ := iostreams.Test() diff --git a/pkg/cmd/release/verify/verify.go b/pkg/cmd/release/verify/verify.go index 65516764ebe..39e27bbc50b 100644 --- a/pkg/cmd/release/verify/verify.go +++ b/pkg/cmd/release/verify/verify.go @@ -130,7 +130,7 @@ func verifyRun(config *VerifyConfig) error { return err } - releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, "sha1") + releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, shared.DigestAlgForRef(ref)) // Find all the attestations for the release tag SHA attestations, err := config.AttClient.GetByDigest(api.FetchParams{ diff --git a/pkg/cmd/release/verify/verify_test.go b/pkg/cmd/release/verify/verify_test.go index 40009fc7d5a..df6070cc689 100644 --- a/pkg/cmd/release/verify/verify_test.go +++ b/pkg/cmd/release/verify/verify_test.go @@ -131,6 +131,35 @@ func Test_verifyRun_FailedNoAttestations(t *testing.T) { require.ErrorContains(t, err, "no attestations for tag v1") } +func Test_verifyRun_FailedNoAttestations_SHA256(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tagName := "v1" + + fakeHTTP := &httpmock.Registry{} + defer fakeHTTP.Verify(t) + fakeSHA := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" + shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA) + + baseRepo, err := ghrepo.FromFullName("owner/repo") + require.NoError(t, err) + + cfg := &VerifyConfig{ + Opts: &VerifyOptions{ + TagName: tagName, + BaseRepo: baseRepo, + Exporter: nil, + }, + IO: ios, + HttpClient: &http.Client{Transport: fakeHTTP}, + AttClient: api.NewFailTestClient(), + AttVerifier: nil, + } + + err = verifyRun(cfg) + require.ErrorContains(t, err, "no attestations for tag v1") + require.ErrorContains(t, err, "sha256:"+fakeSHA) +} + func Test_verifyRun_FailedTagNotInAttestation(t *testing.T) { ios, _, _, _ := iostreams.Test() From 93e0a2811b7bf8d110c2558c123da773d2046fbd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 15 May 2026 14:03:42 +0000 Subject: [PATCH 2/3] chore(deps): bump google.golang.org/grpc from 1.81.0 to 1.81.1 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.81.0 to 1.81.1. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](https://github.com/grpc/grpc-go/compare/v1.81.0...v1.81.1) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.81.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 715bc101295..395aba966c3 100644 --- a/go.mod +++ b/go.mod @@ -62,7 +62,7 @@ require ( golang.org/x/sys v0.44.0 golang.org/x/term v0.43.0 golang.org/x/text v0.37.0 - google.golang.org/grpc v1.81.0 + google.golang.org/grpc v1.81.1 google.golang.org/protobuf v1.36.11 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 69272e7792e..90f6a825319 100644 --- a/go.sum +++ b/go.sum @@ -636,8 +636,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20260316180232-0b37fe3546d5 h1: google.golang.org/genproto/googleapis/api v0.0.0-20260316180232-0b37fe3546d5/go.mod h1:EIQZ5bFCfRQDV4MhRle7+OgjNtZ6P1PiZBgAKuxXu/Y= google.golang.org/genproto/googleapis/rpc v0.0.0-20260316180232-0b37fe3546d5 h1:aJmi6DVGGIStN9Mobk/tZOOQUBbj0BPjZjjnOdoZKts= google.golang.org/genproto/googleapis/rpc v0.0.0-20260316180232-0b37fe3546d5/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= -google.golang.org/grpc v1.81.0 h1:W3G9N3KQf3BU+YuCtGKJk0CmxQNbAISICD/9AORxLIw= -google.golang.org/grpc v1.81.0/go.mod h1:xGH9GfzOyMTGIOXBJmXt+BX/V0kcdQbdcuwQ/zNw42I= +google.golang.org/grpc v1.81.1 h1:VnnIIZ88UzOOKLukQi+ImGz8O1Wdp8nAGGnvOfEIWQQ= +google.golang.org/grpc v1.81.1/go.mod h1:xGH9GfzOyMTGIOXBJmXt+BX/V0kcdQbdcuwQ/zNw42I= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From d9eb0627dceeb49b2943fa992414eb185787d02e Mon Sep 17 00:00:00 2001 From: Brian DeHamer Date: Fri, 15 May 2026 07:57:58 -0700 Subject: [PATCH 3/3] Assert digest prefix in release verify no-attestation tests Address PR review feedback: - Rename SHA1 tests to make the algorithm explicit - Assert the sha1:/sha256: prefix appears in the error - Use a capturing MockClient so we verify the actual digest sent to GetByDigest, not just the wrapped error message Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../release/verify-asset/verify_asset_test.go | 25 ++++++++++++++++--- pkg/cmd/release/verify/verify_test.go | 25 ++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/release/verify-asset/verify_asset_test.go b/pkg/cmd/release/verify-asset/verify_asset_test.go index 8e3654119ce..dc881ec00a9 100644 --- a/pkg/cmd/release/verify-asset/verify_asset_test.go +++ b/pkg/cmd/release/verify-asset/verify_asset_test.go @@ -166,7 +166,7 @@ func Test_verifyAssetRun_SuccessNoTagArg(t *testing.T) { require.NoError(t, err) } -func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) { +func Test_verifyAssetRun_FailedNoAttestations_SHA1(t *testing.T) { ios, _, _, _ := iostreams.Test() tagName := "v1" @@ -180,6 +180,14 @@ func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) { releaseAssetPath := test.NormalizeRelativePath("../../attestation/test/data/github_release_artifact.zip") + var capturedParams api.FetchParams + attClient := &api.MockClient{ + OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) { + capturedParams = params + return api.OnGetByDigestFailure(params) + }, + } + cfg := &VerifyAssetConfig{ Opts: &VerifyAssetOptions{ AssetFilePath: releaseAssetPath, @@ -189,12 +197,14 @@ func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) { }, IO: ios, HttpClient: &http.Client{Transport: fakeHTTP}, - AttClient: api.NewFailTestClient(), + AttClient: attClient, AttVerifier: nil, } err = verifyAssetRun(cfg) require.ErrorContains(t, err, "no attestations found for tag v1") + require.ErrorContains(t, err, "sha1:"+fakeSHA) + require.Equal(t, "sha1:"+fakeSHA, capturedParams.Digest) } func Test_verifyAssetRun_FailedNoAttestations_SHA256(t *testing.T) { @@ -211,6 +221,14 @@ func Test_verifyAssetRun_FailedNoAttestations_SHA256(t *testing.T) { releaseAssetPath := test.NormalizeRelativePath("../../attestation/test/data/github_release_artifact.zip") + var capturedParams api.FetchParams + attClient := &api.MockClient{ + OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) { + capturedParams = params + return api.OnGetByDigestFailure(params) + }, + } + cfg := &VerifyAssetConfig{ Opts: &VerifyAssetOptions{ AssetFilePath: releaseAssetPath, @@ -220,13 +238,14 @@ func Test_verifyAssetRun_FailedNoAttestations_SHA256(t *testing.T) { }, IO: ios, HttpClient: &http.Client{Transport: fakeHTTP}, - AttClient: api.NewFailTestClient(), + AttClient: attClient, AttVerifier: nil, } err = verifyAssetRun(cfg) require.ErrorContains(t, err, "no attestations found for tag v1") require.ErrorContains(t, err, "sha256:"+fakeSHA) + require.Equal(t, "sha256:"+fakeSHA, capturedParams.Digest) } func Test_verifyAssetRun_FailedTagNotInAttestation(t *testing.T) { diff --git a/pkg/cmd/release/verify/verify_test.go b/pkg/cmd/release/verify/verify_test.go index df6070cc689..ccb3b35a6ba 100644 --- a/pkg/cmd/release/verify/verify_test.go +++ b/pkg/cmd/release/verify/verify_test.go @@ -103,7 +103,7 @@ func Test_verifyRun_Success(t *testing.T) { require.NoError(t, err) } -func Test_verifyRun_FailedNoAttestations(t *testing.T) { +func Test_verifyRun_FailedNoAttestations_SHA1(t *testing.T) { ios, _, _, _ := iostreams.Test() tagName := "v1" @@ -115,6 +115,14 @@ func Test_verifyRun_FailedNoAttestations(t *testing.T) { baseRepo, err := ghrepo.FromFullName("owner/repo") require.NoError(t, err) + var capturedParams api.FetchParams + attClient := &api.MockClient{ + OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) { + capturedParams = params + return api.OnGetByDigestFailure(params) + }, + } + cfg := &VerifyConfig{ Opts: &VerifyOptions{ TagName: tagName, @@ -123,12 +131,14 @@ func Test_verifyRun_FailedNoAttestations(t *testing.T) { }, IO: ios, HttpClient: &http.Client{Transport: fakeHTTP}, - AttClient: api.NewFailTestClient(), + AttClient: attClient, AttVerifier: nil, } err = verifyRun(cfg) require.ErrorContains(t, err, "no attestations for tag v1") + require.ErrorContains(t, err, "sha1:"+fakeSHA) + require.Equal(t, "sha1:"+fakeSHA, capturedParams.Digest) } func Test_verifyRun_FailedNoAttestations_SHA256(t *testing.T) { @@ -143,6 +153,14 @@ func Test_verifyRun_FailedNoAttestations_SHA256(t *testing.T) { baseRepo, err := ghrepo.FromFullName("owner/repo") require.NoError(t, err) + var capturedParams api.FetchParams + attClient := &api.MockClient{ + OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) { + capturedParams = params + return api.OnGetByDigestFailure(params) + }, + } + cfg := &VerifyConfig{ Opts: &VerifyOptions{ TagName: tagName, @@ -151,13 +169,14 @@ func Test_verifyRun_FailedNoAttestations_SHA256(t *testing.T) { }, IO: ios, HttpClient: &http.Client{Transport: fakeHTTP}, - AttClient: api.NewFailTestClient(), + AttClient: attClient, AttVerifier: nil, } err = verifyRun(cfg) require.ErrorContains(t, err, "no attestations for tag v1") require.ErrorContains(t, err, "sha256:"+fakeSHA) + require.Equal(t, "sha256:"+fakeSHA, capturedParams.Digest) } func Test_verifyRun_FailedTagNotInAttestation(t *testing.T) {