Clarify warnings for tags ahead of locked commits (#205) · github/gh-actions-lock@d549b67 · GitHub
Skip to content

Commit d549b67

Browse files
nodeselectorCopilot
nodeselector
and
Copilot
authored
Clarify warnings for tags ahead of locked commits (#205)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 817f1d4 commit d549b67

4 files changed

Lines changed: 70 additions & 26 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 2 deletions

‎cmd/gh-actions-lock/format/terminal.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,12 @@ func renderTermFindingDetail(out *ui.UI, f checks.Finding, dep string) {
182182
}
183183
}
184184

185-
// findingLabel is categoryLabel with terminal-only titles for refs that
186-
// don't exist upstream, which share the not-pinned and unreachable-pin
187-
// categories in JSON. Only a ref the lockfile recorded can no longer exist.
185+
// findingLabel distinguishes ref states that share a category in JSON.
186+
// Only a ref the lockfile recorded can no longer exist.
188187
func findingLabel(f checks.Finding) string {
189188
switch {
189+
case f.TagMoved && f.Category == checks.RefMoved:
190+
return "Tag now points past locked commit"
190191
case f.RefRefused && f.Category == checks.UnreachablePin:
191192
return "Ref name refused"
192193
case (f.RefMissing || f.RefGone) && f.Category == checks.UnreachablePin && f.FreshRef:
@@ -445,7 +446,12 @@ func renderWarnings(out *ui.UI, report *checks.Report, willRemediate bool) {
445446
if label == "" {
446447
label = f.WorkflowPath
447448
}
448-
out.TermWarn("%s: %s", label, cmp.Or(f.TermDetail, f.Detail))
449+
if f.TagMoved {
450+
out.TermWarn("%s %s", findingLabel(f), label)
451+
out.TermDetail("↳ %s", cmp.Or(f.TermDetail, f.Detail))
452+
} else {
453+
out.TermWarn("%s: %s", label, cmp.Or(f.TermDetail, f.Detail))
454+
}
449455
if f.Remediation != "" {
450456
out.TermDetail("↳ %s", f.Remediation)
451457
}

‎cmd/gh-actions-lock/parity_test.go‎

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,23 @@ jobs:
334334
steps:
335335
- uses: actions/checkout@v4.2.1
336336
`, "actions/checkout@v4.2.1=sha1-"+parityTestSHA)
337+
beforeWorkflow, err := os.ReadFile(path)
338+
require.NoError(t, err)
339+
lockPath := filepath.Join(".github", "workflows", "actions.lock")
340+
beforeLock, err := os.ReadFile(lockPath)
341+
require.NoError(t, err)
337342
p, err := runParity(t, reg, append(tt.args, path)...)
343+
afterWorkflow, readErr := os.ReadFile(path)
344+
require.NoError(t, readErr)
345+
assert.Equal(t, beforeWorkflow, afterWorkflow, "the workflow must stay unchanged")
338346
lock := readTempLockfilePins(t)
339347
assert.Contains(t, lock, "sha1-"+parityTestSHA)
340348
assert.NotContains(t, lock, parityTestMoved, "the lock keeps its commit")
349+
if tt.args[0] == "--verify" || tt.block {
350+
afterLock, readErr := os.ReadFile(lockPath)
351+
require.NoError(t, readErr)
352+
assert.Equal(t, beforeLock, afterLock, "verification and blocked runs must not write the lockfile")
353+
}
341354
if tt.want == "" {
342355
require.NoError(t, err)
343356
assert.True(t, p.Valid)
@@ -382,26 +395,46 @@ jobs:
382395
assert.Contains(t, stderr, "could not verify locked actions/checkout@9ddf35b71a48: GitHub returned 502 Bad Gateway")
383396
assert.NotContains(t, stderr, "api.github.com", "the request URL stays out of the terminal")
384397
})
385-
t.Run("moved exact tag warns", func(t *testing.T) {
386-
reg := &httpmock.Registry{}
387-
defer reg.Verify(t)
388-
reg.Register(httpmock.GraphQL(`commit: object\(oid`), httpmock.JSONResponse(map[string]any{
389-
"data": map[string]any{"a0": map[string]any{
390-
"nameWithOwner": "actions/checkout",
391-
"commit": map[string]any{"oid": parityTestSHA},
392-
"tag": map[string]any{"oid": parityTestMoved},
393-
"tagRef": map[string]any{"oid": parityTestMoved},
394-
}},
395-
}))
396-
reg.Register(httpmock.REST("GET", `repos/actions/checkout/compare/`),
397-
httpmock.JSONResponse(map[string]any{"status": "ahead", "merge_base_commit": map[string]any{"sha": parityTestSHA}}))
398-
path := writeTempWorkflow(t, workflow, "actions/checkout@v4.2.1=sha1-"+parityTestSHA)
399-
_, stderr, err := runCommandWithHTTP(t, reg, "--verify", path)
400-
require.NoError(t, err, stderr)
401-
assert.Contains(t, stderr, "actions/checkout@v4.2.1: tag v4.2.1 now points at bbbbbbbbbbbb; locked commit 9ddf35b71a48 is still in its history")
402-
assert.Contains(t, stderr, "All 1 workflow valid")
403-
assert.NotContains(t, stderr, "✗")
404-
})
398+
for _, tt := range []struct {
399+
name string
400+
status string
401+
base string
402+
block bool
403+
}{
404+
{"moved exact tag warns", "ahead", parityTestSHA, false},
405+
{"unreachable pin blocks", "diverged", "cccccccccccccccccccccccccccccccccccccccc", true},
406+
} {
407+
t.Run(tt.name, func(t *testing.T) {
408+
reg := &httpmock.Registry{}
409+
defer reg.Verify(t)
410+
reg.Register(httpmock.GraphQL(`commit: object\(oid`), httpmock.JSONResponse(map[string]any{
411+
"data": map[string]any{"a0": map[string]any{
412+
"nameWithOwner": "actions/checkout",
413+
"commit": map[string]any{"oid": parityTestSHA},
414+
"tag": map[string]any{"oid": parityTestMoved},
415+
"tagRef": map[string]any{"oid": parityTestMoved},
416+
}},
417+
}))
418+
reg.Register(httpmock.REST("GET", `repos/actions/checkout/compare/`),
419+
httpmock.JSONResponse(map[string]any{"status": tt.status, "merge_base_commit": map[string]any{"sha": tt.base}}))
420+
path := writeTempWorkflow(t, workflow, "actions/checkout@v4.2.1=sha1-"+parityTestSHA)
421+
_, stderr, err := runCommandWithHTTP(t, reg, "--verify", path)
422+
if tt.block {
423+
require.ErrorIs(t, err, errSilent, stderr)
424+
assert.Contains(t, stderr, "✗ Pinned commit not in upstream history actions/checkout@v4.2.1")
425+
assert.NotContains(t, stderr, "Tag now points past locked commit")
426+
assert.NotContains(t, stderr, "All 1 workflow valid")
427+
return
428+
}
429+
require.NoError(t, err, stderr)
430+
assert.Contains(t, stderr, "Tag now points past locked commit actions/checkout@v4.2.1")
431+
assert.Contains(t, stderr, "tag v4.2.1 now points at bbbbbbbbbbbb; locked commit 9ddf35b71a48 is still in its history")
432+
assert.Contains(t, stderr, "All 1 workflow valid")
433+
assert.NotContains(t, stderr, "not in upstream history")
434+
assert.NotContains(t, stderr, "--accept-moved")
435+
assert.NotContains(t, stderr, "✗")
436+
})
437+
}
405438
}
406439

407440
// TestParity_DefaultRunPrintsParityFinding: parity findings have no pin

‎internal/pipeline/parity_test.go‎

Lines changed: 4 additions & 0 deletions

0 commit comments

Comments
 (0)