Skip to content

Commit 5c437a8

Browse files
bdehamerCopilot
andcommitted
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 cli#13429 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b99d73d commit 5c437a8

6 files changed

Lines changed: 111 additions & 2 deletions

File tree

pkg/cmd/release/shared/fetch.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,19 @@ func FetchRefSHA(ctx context.Context, httpClient *http.Client, repo ghrepo.Inter
170170
return ref.Object.SHA, nil
171171
}
172172

173+
// DigestAlgForRef returns the digest algorithm name corresponding to the given
174+
// git ref SHA. SHA-1 git object IDs are 40 hex characters and SHA-256 git
175+
// object IDs are 64 hex characters. Unknown lengths default to "sha1" to
176+
// preserve backwards-compatible behavior.
177+
func DigestAlgForRef(digest string) string {
178+
switch len(digest) {
179+
case 64:
180+
return "sha256"
181+
default:
182+
return "sha1"
183+
}
184+
}
185+
173186
// FetchRelease finds a published repository release by its tagName, or a draft release by its pending tag name.
174187
func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Interface, tagName string) (*Release, error) {
175188
cc, cancel := context.WithCancel(ctx)

pkg/cmd/release/shared/fetch_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,38 @@ func TestFetchRefSHA(t *testing.T) {
9292
})
9393
}
9494
}
95+
96+
func TestDigestAlgForRef(t *testing.T) {
97+
tests := []struct {
98+
name string
99+
digest string
100+
expected string
101+
}{
102+
{
103+
name: "sha1 (40 hex chars)",
104+
digest: "1234567890abcdef1234567890abcdef12345678",
105+
expected: "sha1",
106+
},
107+
{
108+
name: "sha256 (64 hex chars)",
109+
digest: "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
110+
expected: "sha256",
111+
},
112+
{
113+
name: "empty string defaults to sha1",
114+
digest: "",
115+
expected: "sha1",
116+
},
117+
{
118+
name: "unexpected length defaults to sha1",
119+
digest: "abc",
120+
expected: "sha1",
121+
},
122+
}
123+
124+
for _, tt := range tests {
125+
t.Run(tt.name, func(t *testing.T) {
126+
assert.Equal(t, tt.expected, DigestAlgForRef(tt.digest))
127+
})
128+
}
129+
}

pkg/cmd/release/verify-asset/verify_asset.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func verifyAssetRun(config *VerifyAssetConfig) error {
142142
return err
143143
}
144144

145-
releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, "sha1")
145+
releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, shared.DigestAlgForRef(ref))
146146

147147
// Find attestations for the release tag SHA
148148
attestations, err := config.AttClient.GetByDigest(api.FetchParams{

pkg/cmd/release/verify-asset/verify_asset_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,38 @@ func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) {
197197
require.ErrorContains(t, err, "no attestations found for tag v1")
198198
}
199199

200+
func Test_verifyAssetRun_FailedNoAttestations_SHA256(t *testing.T) {
201+
ios, _, _, _ := iostreams.Test()
202+
tagName := "v1"
203+
204+
fakeHTTP := &httpmock.Registry{}
205+
defer fakeHTTP.Verify(t)
206+
fakeSHA := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
207+
shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA)
208+
209+
baseRepo, err := ghrepo.FromFullName("owner/repo")
210+
require.NoError(t, err)
211+
212+
releaseAssetPath := test.NormalizeRelativePath("../../attestation/test/data/github_release_artifact.zip")
213+
214+
cfg := &VerifyAssetConfig{
215+
Opts: &VerifyAssetOptions{
216+
AssetFilePath: releaseAssetPath,
217+
TagName: tagName,
218+
BaseRepo: baseRepo,
219+
Exporter: nil,
220+
},
221+
IO: ios,
222+
HttpClient: &http.Client{Transport: fakeHTTP},
223+
AttClient: api.NewFailTestClient(),
224+
AttVerifier: nil,
225+
}
226+
227+
err = verifyAssetRun(cfg)
228+
require.ErrorContains(t, err, "no attestations found for tag v1")
229+
require.ErrorContains(t, err, "sha256:"+fakeSHA)
230+
}
231+
200232
func Test_verifyAssetRun_FailedTagNotInAttestation(t *testing.T) {
201233
ios, _, _, _ := iostreams.Test()
202234

pkg/cmd/release/verify/verify.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func verifyRun(config *VerifyConfig) error {
130130
return err
131131
}
132132

133-
releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, "sha1")
133+
releaseRefDigest := artifact.NewDigestedArtifactForRelease(ref, shared.DigestAlgForRef(ref))
134134

135135
// Find all the attestations for the release tag SHA
136136
attestations, err := config.AttClient.GetByDigest(api.FetchParams{

pkg/cmd/release/verify/verify_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,35 @@ func Test_verifyRun_FailedNoAttestations(t *testing.T) {
131131
require.ErrorContains(t, err, "no attestations for tag v1")
132132
}
133133

134+
func Test_verifyRun_FailedNoAttestations_SHA256(t *testing.T) {
135+
ios, _, _, _ := iostreams.Test()
136+
tagName := "v1"
137+
138+
fakeHTTP := &httpmock.Registry{}
139+
defer fakeHTTP.Verify(t)
140+
fakeSHA := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
141+
shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA)
142+
143+
baseRepo, err := ghrepo.FromFullName("owner/repo")
144+
require.NoError(t, err)
145+
146+
cfg := &VerifyConfig{
147+
Opts: &VerifyOptions{
148+
TagName: tagName,
149+
BaseRepo: baseRepo,
150+
Exporter: nil,
151+
},
152+
IO: ios,
153+
HttpClient: &http.Client{Transport: fakeHTTP},
154+
AttClient: api.NewFailTestClient(),
155+
AttVerifier: nil,
156+
}
157+
158+
err = verifyRun(cfg)
159+
require.ErrorContains(t, err, "no attestations for tag v1")
160+
require.ErrorContains(t, err, "sha256:"+fakeSHA)
161+
}
162+
134163
func Test_verifyRun_FailedTagNotInAttestation(t *testing.T) {
135164
ios, _, _, _ := iostreams.Test()
136165

0 commit comments

Comments
 (0)