Skip to content

Commit f1c10e0

Browse files
bdehamerbdehamer
authored
Merge pull request cli#13430 from bdehamer/fix-release-verify-digest-algorithm
Derive digest algorithm from ref length in release verify commands
2 parents 751ae57 + d9eb062 commit f1c10e0

6 files changed

Lines changed: 153 additions & 6 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: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func Test_verifyAssetRun_SuccessNoTagArg(t *testing.T) {
166166
require.NoError(t, err)
167167
}
168168

169-
func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) {
169+
func Test_verifyAssetRun_FailedNoAttestations_SHA1(t *testing.T) {
170170
ios, _, _, _ := iostreams.Test()
171171
tagName := "v1"
172172

@@ -180,6 +180,55 @@ func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) {
180180

181181
releaseAssetPath := test.NormalizeRelativePath("../../attestation/test/data/github_release_artifact.zip")
182182

183+
var capturedParams api.FetchParams
184+
attClient := &api.MockClient{
185+
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
186+
capturedParams = params
187+
return api.OnGetByDigestFailure(params)
188+
},
189+
}
190+
191+
cfg := &VerifyAssetConfig{
192+
Opts: &VerifyAssetOptions{
193+
AssetFilePath: releaseAssetPath,
194+
TagName: tagName,
195+
BaseRepo: baseRepo,
196+
Exporter: nil,
197+
},
198+
IO: ios,
199+
HttpClient: &http.Client{Transport: fakeHTTP},
200+
AttClient: attClient,
201+
AttVerifier: nil,
202+
}
203+
204+
err = verifyAssetRun(cfg)
205+
require.ErrorContains(t, err, "no attestations found for tag v1")
206+
require.ErrorContains(t, err, "sha1:"+fakeSHA)
207+
require.Equal(t, "sha1:"+fakeSHA, capturedParams.Digest)
208+
}
209+
210+
func Test_verifyAssetRun_FailedNoAttestations_SHA256(t *testing.T) {
211+
ios, _, _, _ := iostreams.Test()
212+
tagName := "v1"
213+
214+
fakeHTTP := &httpmock.Registry{}
215+
defer fakeHTTP.Verify(t)
216+
fakeSHA := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
217+
shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA)
218+
219+
baseRepo, err := ghrepo.FromFullName("owner/repo")
220+
require.NoError(t, err)
221+
222+
releaseAssetPath := test.NormalizeRelativePath("../../attestation/test/data/github_release_artifact.zip")
223+
224+
var capturedParams api.FetchParams
225+
attClient := &api.MockClient{
226+
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
227+
capturedParams = params
228+
return api.OnGetByDigestFailure(params)
229+
},
230+
}
231+
183232
cfg := &VerifyAssetConfig{
184233
Opts: &VerifyAssetOptions{
185234
AssetFilePath: releaseAssetPath,
@@ -189,12 +238,14 @@ func Test_verifyAssetRun_FailedNoAttestations(t *testing.T) {
189238
},
190239
IO: ios,
191240
HttpClient: &http.Client{Transport: fakeHTTP},
192-
AttClient: api.NewFailTestClient(),
241+
AttClient: attClient,
193242
AttVerifier: nil,
194243
}
195244

196245
err = verifyAssetRun(cfg)
197246
require.ErrorContains(t, err, "no attestations found for tag v1")
247+
require.ErrorContains(t, err, "sha256:"+fakeSHA)
248+
require.Equal(t, "sha256:"+fakeSHA, capturedParams.Digest)
198249
}
199250

200251
func Test_verifyAssetRun_FailedTagNotInAttestation(t *testing.T) {

‎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: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func Test_verifyRun_Success(t *testing.T) {
103103
require.NoError(t, err)
104104
}
105105

106-
func Test_verifyRun_FailedNoAttestations(t *testing.T) {
106+
func Test_verifyRun_FailedNoAttestations_SHA1(t *testing.T) {
107107
ios, _, _, _ := iostreams.Test()
108108
tagName := "v1"
109109

@@ -115,6 +115,52 @@ func Test_verifyRun_FailedNoAttestations(t *testing.T) {
115115
baseRepo, err := ghrepo.FromFullName("owner/repo")
116116
require.NoError(t, err)
117117

118+
var capturedParams api.FetchParams
119+
attClient := &api.MockClient{
120+
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
121+
capturedParams = params
122+
return api.OnGetByDigestFailure(params)
123+
},
124+
}
125+
126+
cfg := &VerifyConfig{
127+
Opts: &VerifyOptions{
128+
TagName: tagName,
129+
BaseRepo: baseRepo,
130+
Exporter: nil,
131+
},
132+
IO: ios,
133+
HttpClient: &http.Client{Transport: fakeHTTP},
134+
AttClient: attClient,
135+
AttVerifier: nil,
136+
}
137+
138+
err = verifyRun(cfg)
139+
require.ErrorContains(t, err, "no attestations for tag v1")
140+
require.ErrorContains(t, err, "sha1:"+fakeSHA)
141+
require.Equal(t, "sha1:"+fakeSHA, capturedParams.Digest)
142+
}
143+
144+
func Test_verifyRun_FailedNoAttestations_SHA256(t *testing.T) {
145+
ios, _, _, _ := iostreams.Test()
146+
tagName := "v1"
147+
148+
fakeHTTP := &httpmock.Registry{}
149+
defer fakeHTTP.Verify(t)
150+
fakeSHA := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
151+
shared.StubFetchRefSHA(t, fakeHTTP, "owner", "repo", tagName, fakeSHA)
152+
153+
baseRepo, err := ghrepo.FromFullName("owner/repo")
154+
require.NoError(t, err)
155+
156+
var capturedParams api.FetchParams
157+
attClient := &api.MockClient{
158+
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
159+
capturedParams = params
160+
return api.OnGetByDigestFailure(params)
161+
},
162+
}
163+
118164
cfg := &VerifyConfig{
119165
Opts: &VerifyOptions{
120166
TagName: tagName,
@@ -123,12 +169,14 @@ func Test_verifyRun_FailedNoAttestations(t *testing.T) {
123169
},
124170
IO: ios,
125171
HttpClient: &http.Client{Transport: fakeHTTP},
126-
AttClient: api.NewFailTestClient(),
172+
AttClient: attClient,
127173
AttVerifier: nil,
128174
}
129175

130176
err = verifyRun(cfg)
131177
require.ErrorContains(t, err, "no attestations for tag v1")
178+
require.ErrorContains(t, err, "sha256:"+fakeSHA)
179+
require.Equal(t, "sha256:"+fakeSHA, capturedParams.Digest)
132180
}
133181

134182
func Test_verifyRun_FailedTagNotInAttestation(t *testing.T) {

0 commit comments

Comments
 (0)