Skip to content

Commit 25daf01

Browse files
varunsh-codervarunsh-coder
committed
clean up major-tag replacement and expand unit tests
- remove debug fmt.Println in GetMajorTagFromSHA - delete TestSome which made a live GitHub API call - log skipped replacements instead of silently continuing - wrap GetMajorTagFromSHA error with %w in resolveVersion - split maintainedActions fixtures into per-test input/output files (*_majorTag.yml and *_latest.yml) so each test row has its own pair - add tests covering GetLatestRelease, GetMajorTagFromSHA, GetMajorTagIfExists, resolveVersion, LoadMaintainedActions, and ReplaceActions edge cases; package coverage now 96.4%
1 parent 9390f0c commit 25daf01

24 files changed

Lines changed: 543 additions & 33 deletions

‎remediation/workflow/maintainedactions/getlatestrelease.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ func GetMajorTagFromSHA(ownerRepo, sha string) (string, error) {
9292
return "", err
9393
}
9494

95-
fmt.Println("len refs:", len(refs))
96-
9795
for _, ref := range refs {
9896
var refSHA string
9997
if ref.GetObject().GetType() == "commit" {

‎remediation/workflow/maintainedactions/getlatestrelease_test.go‎

Lines changed: 389 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,389 @@
1+
package maintainedactions
2+
3+
import (
4+
"io/ioutil"
5+
"net/http"
6+
"path"
7+
"testing"
8+
9+
"github.com/jarcoal/httpmock"
10+
)
11+
12+
func TestGetMajorVersion(t *testing.T) {
13+
cases := map[string]string{
14+
"v5": "v5",
15+
"v5.5.5": "v5",
16+
"5.5.5": "5",
17+
"5": "5",
18+
"v": "v",
19+
"": "",
20+
}
21+
for in, want := range cases {
22+
if got := getMajorVersion(in); got != want {
23+
t.Errorf("getMajorVersion(%q) = %q, want %q", in, got, want)
24+
}
25+
}
26+
}
27+
28+
func TestGetLatestRelease_InvalidRepo(t *testing.T) {
29+
if _, err := GetLatestRelease("no-slash"); err == nil {
30+
t.Fatal("expected error for invalid owner/repo")
31+
}
32+
}
33+
34+
func TestGetLatestRelease_NoPATFails(t *testing.T) {
35+
httpmock.Activate()
36+
defer httpmock.DeactivateAndReset()
37+
t.Setenv("PAT", "")
38+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/releases/latest",
39+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
40+
if _, err := GetLatestRelease("owner/repo"); err == nil {
41+
t.Fatal("expected error when first call fails and no PAT is set")
42+
}
43+
}
44+
45+
func TestGetLatestRelease_PATRetrySucceeds(t *testing.T) {
46+
httpmock.Activate()
47+
defer httpmock.DeactivateAndReset()
48+
t.Setenv("PAT", "fake-token")
49+
calls := 0
50+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/releases/latest",
51+
func(req *http.Request) (*http.Response, error) {
52+
calls++
53+
if calls == 1 {
54+
return httpmock.NewStringResponse(500, `{"message":"boom"}`), nil
55+
}
56+
return httpmock.NewStringResponse(200, `{"tag_name":"v3.2.1"}`), nil
57+
})
58+
v, err := GetLatestRelease("owner/repo")
59+
if err != nil {
60+
t.Fatalf("unexpected error: %v", err)
61+
}
62+
if v != "v3" {
63+
t.Errorf("got %q, want v3", v)
64+
}
65+
}
66+
67+
func TestGetLatestRelease_PATRetryFails(t *testing.T) {
68+
httpmock.Activate()
69+
defer httpmock.DeactivateAndReset()
70+
t.Setenv("PAT", "fake-token")
71+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/releases/latest",
72+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
73+
if _, err := GetLatestRelease("owner/repo"); err == nil {
74+
t.Fatal("expected error when both attempts fail")
75+
}
76+
}
77+
78+
// GetMajorTagFromSHA
79+
80+
func TestGetMajorTagFromSHA_InvalidRepo(t *testing.T) {
81+
if _, err := GetMajorTagFromSHA("no-slash", "abc"); err == nil {
82+
t.Fatal("expected error for invalid owner/repo")
83+
}
84+
}
85+
86+
func TestGetMajorTagFromSHA_ListError(t *testing.T) {
87+
httpmock.Activate()
88+
defer httpmock.DeactivateAndReset()
89+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/matching-refs/tags/v",
90+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
91+
if _, err := GetMajorTagFromSHA("owner/repo", "anything"); err == nil {
92+
t.Fatal("expected error from ListMatchingRefs failure")
93+
}
94+
}
95+
96+
func TestGetMajorTagFromSHA_CommitMatch(t *testing.T) {
97+
httpmock.Activate()
98+
defer httpmock.DeactivateAndReset()
99+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/matching-refs/tags/v",
100+
httpmock.NewStringResponder(200, `[
101+
{"ref":"refs/tags/v2.0.0","object":{"sha":"aaaa","type":"commit"}},
102+
{"ref":"refs/tags/v5.1.0","object":{"sha":"bbbb","type":"commit"}}
103+
]`))
104+
v, err := GetMajorTagFromSHA("owner/repo", "bbbb")
105+
if err != nil {
106+
t.Fatalf("unexpected error: %v", err)
107+
}
108+
if v != "v5" {
109+
t.Errorf("got %q, want v5", v)
110+
}
111+
}
112+
113+
func TestGetMajorTagFromSHA_AnnotatedTagMatch(t *testing.T) {
114+
httpmock.Activate()
115+
defer httpmock.DeactivateAndReset()
116+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/matching-refs/tags/v",
117+
httpmock.NewStringResponder(200, `[
118+
{"ref":"refs/tags/v3.0.0","object":{"sha":"tagsha","type":"tag"}}
119+
]`))
120+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/commits/refs/tags/v3.0.0",
121+
httpmock.NewStringResponder(200, `commitsha`))
122+
v, err := GetMajorTagFromSHA("owner/repo", "commitsha")
123+
if err != nil {
124+
t.Fatalf("unexpected error: %v", err)
125+
}
126+
if v != "v3" {
127+
t.Errorf("got %q, want v3", v)
128+
}
129+
}
130+
131+
func TestGetMajorTagFromSHA_AnnotatedDerefError(t *testing.T) {
132+
httpmock.Activate()
133+
defer httpmock.DeactivateAndReset()
134+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/matching-refs/tags/v",
135+
httpmock.NewStringResponder(200, `[
136+
{"ref":"refs/tags/v3.0.0","object":{"sha":"tagsha","type":"tag"}},
137+
{"ref":"refs/tags/v4.0.0","object":{"sha":"match","type":"commit"}}
138+
]`))
139+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/commits/refs/tags/v3.0.0",
140+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
141+
v, err := GetMajorTagFromSHA("owner/repo", "match")
142+
if err != nil {
143+
t.Fatalf("unexpected error: %v", err)
144+
}
145+
if v != "v4" {
146+
t.Errorf("got %q, want v4 (deref error should be skipped, not fatal)", v)
147+
}
148+
}
149+
150+
func TestGetMajorTagFromSHA_NoMatch(t *testing.T) {
151+
httpmock.Activate()
152+
defer httpmock.DeactivateAndReset()
153+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/matching-refs/tags/v",
154+
httpmock.NewStringResponder(200, `[
155+
{"ref":"refs/tags/v2.0.0","object":{"sha":"aaaa","type":"commit"}}
156+
]`))
157+
v, err := GetMajorTagFromSHA("owner/repo", "nomatch")
158+
if err != nil {
159+
t.Fatalf("unexpected error: %v", err)
160+
}
161+
if v != "" {
162+
t.Errorf("got %q, want empty string", v)
163+
}
164+
}
165+
166+
func TestGetMajorTagFromSHA_WithPAT(t *testing.T) {
167+
httpmock.Activate()
168+
defer httpmock.DeactivateAndReset()
169+
t.Setenv("PAT", "fake-token")
170+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/matching-refs/tags/v",
171+
httpmock.NewStringResponder(200, `[
172+
{"ref":"refs/tags/v1.0.0","object":{"sha":"match","type":"commit"}}
173+
]`))
174+
v, err := GetMajorTagFromSHA("owner/repo", "match")
175+
if err != nil {
176+
t.Fatalf("unexpected error: %v", err)
177+
}
178+
if v != "v1" {
179+
t.Errorf("got %q, want v1", v)
180+
}
181+
}
182+
183+
// GetMajorTagIfExists
184+
185+
func TestGetMajorTagIfExists_InvalidRepo(t *testing.T) {
186+
if _, _, err := GetMajorTagIfExists("no-slash", "v1"); err == nil {
187+
t.Fatal("expected error for invalid owner/repo")
188+
}
189+
}
190+
191+
func TestGetMajorTagIfExists_NonErrorNoPAT(t *testing.T) {
192+
httpmock.Activate()
193+
defer httpmock.DeactivateAndReset()
194+
t.Setenv("PAT", "")
195+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/ref/tags/v5",
196+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
197+
tag, exists, err := GetMajorTagIfExists("owner/repo", "v5")
198+
if err != nil || exists || tag != "" {
199+
t.Errorf("got tag=%q exists=%v err=%v, want empty/false/nil", tag, exists, err)
200+
}
201+
}
202+
203+
func TestGetMajorTagIfExists_PATRetrySucceeds(t *testing.T) {
204+
httpmock.Activate()
205+
defer httpmock.DeactivateAndReset()
206+
t.Setenv("PAT", "fake-token")
207+
calls := 0
208+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/ref/tags/v5",
209+
func(req *http.Request) (*http.Response, error) {
210+
calls++
211+
if calls == 1 {
212+
return httpmock.NewStringResponse(500, `{"message":"boom"}`), nil
213+
}
214+
return httpmock.NewStringResponse(200,
215+
`{"ref":"refs/tags/v5","object":{"sha":"x","type":"commit"}}`), nil
216+
})
217+
tag, exists, err := GetMajorTagIfExists("owner/repo", "v5")
218+
if err != nil || !exists || tag != "v5" {
219+
t.Errorf("got tag=%q exists=%v err=%v, want v5/true/nil", tag, exists, err)
220+
}
221+
}
222+
223+
func TestGetMajorTagIfExists_PATRetry404(t *testing.T) {
224+
httpmock.Activate()
225+
defer httpmock.DeactivateAndReset()
226+
t.Setenv("PAT", "fake-token")
227+
calls := 0
228+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/ref/tags/v5",
229+
func(req *http.Request) (*http.Response, error) {
230+
calls++
231+
if calls == 1 {
232+
return httpmock.NewStringResponse(500, `{"message":"boom"}`), nil
233+
}
234+
return httpmock.NewStringResponse(404, `{"message":"Not Found"}`), nil
235+
})
236+
tag, exists, err := GetMajorTagIfExists("owner/repo", "v5")
237+
if err != nil || exists || tag != "" {
238+
t.Errorf("got tag=%q exists=%v err=%v, want empty/false/nil", tag, exists, err)
239+
}
240+
}
241+
242+
func TestGetMajorTagIfExists_PATRetryFails(t *testing.T) {
243+
httpmock.Activate()
244+
defer httpmock.DeactivateAndReset()
245+
t.Setenv("PAT", "fake-token")
246+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/owner/repo/git/ref/tags/v5",
247+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
248+
_, exists, err := GetMajorTagIfExists("owner/repo", "v5")
249+
if err == nil {
250+
t.Fatal("expected wrapped error when both attempts fail with non-404")
251+
}
252+
if exists {
253+
t.Error("expected exists=false")
254+
}
255+
}
256+
257+
// resolveVersion
258+
259+
func TestResolveVersion_NoRef(t *testing.T) {
260+
if _, err := resolveVersion("actions/checkout", "actions/checkout", "new/action", true); err == nil {
261+
t.Fatal("expected error when originalUses has no @ref")
262+
}
263+
}
264+
265+
func TestResolveVersion_SHAResolved(t *testing.T) {
266+
httpmock.Activate()
267+
defer httpmock.DeactivateAndReset()
268+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/orig/repo/git/matching-refs/tags/v",
269+
httpmock.NewStringResponder(200, `[
270+
{"ref":"refs/tags/v5.2.0","object":{"sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","type":"commit"}}
271+
]`))
272+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/new/repo/git/ref/tags/v5",
273+
httpmock.NewStringResponder(200,
274+
`{"ref":"refs/tags/v5","object":{"sha":"x","type":"commit"}}`))
275+
uses := "orig/repo@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
276+
v, err := resolveVersion(uses, "orig/repo", "new/repo", true)
277+
if err != nil {
278+
t.Fatalf("unexpected error: %v", err)
279+
}
280+
if v != "v5" {
281+
t.Errorf("got %q, want v5", v)
282+
}
283+
}
284+
285+
func TestResolveVersion_SHALookupFails(t *testing.T) {
286+
httpmock.Activate()
287+
defer httpmock.DeactivateAndReset()
288+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/orig/repo/git/matching-refs/tags/v",
289+
httpmock.NewStringResponder(500, `{"message":"boom"}`))
290+
uses := "orig/repo@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
291+
if _, err := resolveVersion(uses, "orig/repo", "new/repo", true); err == nil {
292+
t.Fatal("expected error when SHA lookup fails")
293+
}
294+
}
295+
296+
func TestResolveVersion_SHANoMatch(t *testing.T) {
297+
httpmock.Activate()
298+
defer httpmock.DeactivateAndReset()
299+
httpmock.RegisterResponder("GET", "https://api.github.com/repos/orig/repo/git/matching-refs/tags/v",
300+
httpmock.NewStringResponder(200, `[]`))
301+
uses := "orig/repo@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
302+
if _, err := resolveVersion(uses, "orig/repo", "new/repo", true); err == nil {
303+
t.Fatal("expected error when SHA has no matching tag")
304+
}
305+
}
306+
307+
// LoadMaintainedActions
308+
309+
func TestLoadMaintainedActions_ReadError(t *testing.T) {
310+
if _, err := LoadMaintainedActions("/nonexistent/path/to/file.json"); err == nil {
311+
t.Fatal("expected error for missing file")
312+
}
313+
}
314+
315+
func TestLoadMaintainedActions_InvalidJSON(t *testing.T) {
316+
dir := t.TempDir()
317+
f := path.Join(dir, "bad.json")
318+
if err := ioutil.WriteFile(f, []byte("{not valid json"), 0644); err != nil {
319+
t.Fatalf("setup: %v", err)
320+
}
321+
if _, err := LoadMaintainedActions(f); err == nil {
322+
t.Fatal("expected error for malformed JSON")
323+
}
324+
}
325+
326+
// ReplaceActions
327+
328+
func TestReplaceActions_InvalidYAML(t *testing.T) {
329+
// A mapping key cannot also be a sequence at the same indent — yaml.Unmarshal errors.
330+
bad := "foo: bar\n- item"
331+
if _, _, err := ReplaceActions(bad, map[string]string{}, false); err == nil {
332+
t.Fatal("expected error for invalid YAML")
333+
}
334+
}
335+
336+
func TestReplaceActions_ReusableWorkflowSkipped(t *testing.T) {
337+
// A job that calls a reusable workflow (has top-level `uses:`) should be skipped.
338+
// No HTTP calls should be made, and no replacements should occur.
339+
httpmock.Activate()
340+
defer httpmock.DeactivateAndReset()
341+
input := `name: reusable
342+
on: push
343+
jobs:
344+
call:
345+
uses: ./.github/workflows/other.yml
346+
`
347+
actionMap := map[string]string{"amannn/action-semantic-pull-request": "step-security/action-semantic-pull-request"}
348+
got, updated, err := ReplaceActions(input, actionMap, false)
349+
if err != nil {
350+
t.Fatalf("unexpected error: %v", err)
351+
}
352+
if updated {
353+
t.Error("expected updated=false for reusable workflow")
354+
}
355+
if got != input {
356+
t.Errorf("expected input unchanged, got %q", got)
357+
}
358+
}
359+
360+
func TestReplaceActions_CompositeResolverFailureSkipped(t *testing.T) {
361+
// Composite action where the resolver fails on the single mapped step should
362+
// skip that step (log + continue) and return unchanged input.
363+
httpmock.Activate()
364+
defer httpmock.DeactivateAndReset()
365+
// Fork has no matching major version.
366+
httpmock.RegisterResponder("GET",
367+
"https://api.github.com/repos/step-security/action-semantic-pull-request/git/ref/tags/v3",
368+
httpmock.NewStringResponder(404, `{"message":"Not Found"}`))
369+
370+
input := `name: composite
371+
runs:
372+
using: composite
373+
steps:
374+
- uses: amannn/action-semantic-pull-request@v3
375+
`
376+
actionMap := map[string]string{
377+
"amannn/action-semantic-pull-request": "step-security/action-semantic-pull-request",
378+
}
379+
got, updated, err := ReplaceActions(input, actionMap, true)
380+
if err != nil {
381+
t.Fatalf("unexpected error: %v", err)
382+
}
383+
if updated {
384+
t.Error("expected updated=false when composite resolver fails")
385+
}
386+
if got != input {
387+
t.Errorf("expected input unchanged, got %q", got)
388+
}
389+
}

0 commit comments

Comments
 (0)