verify-local: report ./ refs instead of migrating them on disk (#158) · github/gh-actions-lock@e9c1c33 · GitHub
Skip to content

Commit e9c1c33

Browse files
nodeselectornodeselector
authored
verify-local: report ./ refs instead of migrating them on disk (#158)
* verify-local: report ./ refs instead of migrating them on disk --verify-local is an offline read-only check, but it ran the ./ -> $/ migration first, leaving a dirty tree in pre-commit and CI. Drop the migration and emit the same local-action finding --verify does. * README: separate fix-run migration from verify-mode reporting
1 parent 4f28243 commit e9c1c33

8 files changed

Lines changed: 134 additions & 50 deletions

File tree

‎README.md‎

Lines changed: 7 additions & 5 deletions

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,12 @@ func bindCheckFlags(cmd *cobra.Command, opts *checkOptions) {
8989
cmd.Flags().BoolVar(&opts.verify, "verify", false, "Frozen check: run the same checks as the default without writing; fails if the lockfile is incomplete or the runner would reject a pin")
9090
cmd.Flags().BoolVar(&opts.verifyLocal, "verify-local", false,
9191
"Offline lockfile coverage check: verify every action ref has a lockfile entry.\n"+
92-
"No network calls, no authentication required — ideal for pre-commit hooks.\n"+
92+
"No network calls, no authentication, no file writes — ideal for pre-commit hooks.\n"+
9393
"It does not check that the entries are trustworthy; use --verify in CI for that.")
9494
cmd.Flags().StringVar(&opts.profileDir, "profile", "", "Enable profiling: write trace, CPU profile, and HTTP log to `dir`")
9595
cmd.Flags().BoolVar(&opts.noMigrateLocalActions, "no-migrate-local-actions", false,
9696
"Do not rewrite same-repo `uses: ./…` actions to the inherently-pinned `uses: $/…` form.\n"+
97-
"Migration runs by default on fix runs; this leaves `./…` refs on disk untouched.")
97+
"Migration runs by default on fix runs (never under --verify or --verify-local); this leaves `./…` refs on disk untouched.")
9898
}
9999

100100
// validateOutputFlags rejects incoherent structured-output flag combinations.

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

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ package main
66
// for every locked pin, without writing. Requires auth.
77
//
88
// --verify-local Offline static coverage check. Every action ref must have
9-
// a lockfile entry. No network, no auth — ideal for pre-commit.
9+
// a lockfile entry. No network, no auth, no writes — ideal for
10+
// pre-commit. Same-repo `./…` refs are reported, not migrated.
1011

1112
import (
1213
"fmt"
@@ -30,27 +31,14 @@ func applyVerifyFlags(opts *checkOptions) {
3031

3132
// runVerifyLocal performs the offline lockfile coverage check. It parses all
3233
// workflows and verifies that every action ref has a corresponding lockfile
33-
// entry — no network, no auth required.
34+
// entry — no network, no auth, never writes to disk.
3435
func runVerifyLocal(opts *checkOptions, out io.Writer, console *ui.UI) error {
3536
workflowsDir := os.Getenv("GH_ACTIONS_LOCK_WORKFLOWS_DIR")
3637
paths, err := discoverWorkflowPaths(opts.workflowPaths, workflowsDir, false)
3738
if err != nil {
3839
return err
3940
}
4041

41-
// Rewrite same-repo `./…` refs to `$/…` on disk before the coverage check
42-
// sees them. Runs by default on fix runs even in this otherwise read-only
43-
// mode; opt out with --no-migrate-local-actions, and never under --no-fix.
44-
if !opts.noMigrateLocalActions && !opts.noFix {
45-
migrated, _, mErr := migrateLocalActions(paths)
46-
if mErr != nil {
47-
return mErr
48-
}
49-
if migrated > 0 && opts.jsonFields == "" {
50-
console.TermSuccess("Migrated %d local %s to `$/…`", migrated, ui.Pluralize(migrated, "action", "actions"))
51-
}
52-
}
53-
5442
// Load the lockfile without a resolver (no network).
5543
var store *lockfile.State
5644
if workflowsDir != "" {
@@ -80,7 +68,7 @@ func runVerifyLocal(opts *checkOptions, out io.Writer, console *ui.UI) error {
8068

8169
if !valid {
8270
if opts.jsonFields == "" {
83-
console.TermDetail("Run without --verify-local to resolve and pin missing actions.")
71+
console.TermDetail("Run without --verify-local to resolve and pin missing actions and migrate `./…` refs to `$/…`.")
8472
}
8573
return errSilent
8674
}

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package main
22

33
import (
4+
"fmt"
45
"os"
6+
"path/filepath"
7+
"strings"
58
"testing"
69

710
"github.com/github/gh-actions-lock/internal/ghapi/httpmock"
@@ -103,3 +106,56 @@ func TestValidateOutputFlags_VerifyConflicts(t *testing.T) {
103106
})
104107
}
105108
}
109+
110+
func TestVerifyLocalReportsLocalActionsWithoutWriting(t *testing.T) {
111+
for _, extra := range [][]string{nil, {"--no-migrate-local-actions"}, {"--no-fix"}} {
112+
for _, jsonOut := range []bool{true, false} {
113+
t.Run(strings.Join(append(extra, fmt.Sprintf("json=%v", jsonOut)), " "), func(t *testing.T) {
114+
path := writeTempWorkflow(t, `
115+
name: ci
116+
on: push
117+
jobs:
118+
test:
119+
runs-on: ubuntu-latest
120+
steps:
121+
- uses: tenant/internal@v1
122+
- uses: ./my-action
123+
call:
124+
uses: ./.github/workflows/reusable.yml
125+
`, "tenant/internal@v1=sha1-"+tenantSHA)
126+
require.NoError(t, os.Mkdir(".git", 0o755))
127+
require.NoError(t, os.Mkdir("my-action", 0o755))
128+
require.NoError(t, os.WriteFile(filepath.Join("my-action", "action.yml"), []byte("runs:\n using: composite\n steps: []\n"), 0o644))
129+
require.NoError(t, os.WriteFile(filepath.Join(".github", "workflows", "reusable.yml"), []byte("on: workflow_call\njobs:\n a:\n runs-on: ubuntu-latest\n steps:\n - run: echo\n"), 0o644))
130+
before, err := os.ReadFile(path)
131+
require.NoError(t, err)
132+
lockBefore := readTempLockfilePins(t)
133+
134+
args := append([]string{"--verify-local", "--no-interactive"}, extra...)
135+
if jsonOut {
136+
args = append(args, "--json=valid,findings")
137+
}
138+
stdout, stderr, err := runCommandWithHTTP(t, &httpmock.Registry{}, append(args, path)...)
139+
140+
require.ErrorIs(t, err, errSilent)
141+
if jsonOut {
142+
assert.Contains(t, stdout, `"valid": false`)
143+
assert.Contains(t, stdout, `"category": "local-action"`)
144+
assert.Contains(t, stdout, `"severity": "error"`)
145+
} else {
146+
out := stdout + stderr
147+
assert.Contains(t, out, "local path actions")
148+
assert.Contains(t, out, "Run without --verify-local")
149+
assert.NotContains(t, out, "Migrated")
150+
}
151+
after, err := os.ReadFile(path)
152+
require.NoError(t, err)
153+
assert.Equal(t, string(before), string(after))
154+
action, err := os.ReadFile(filepath.Join("my-action", "action.yml"))
155+
require.NoError(t, err)
156+
assert.NotContains(t, string(action), "$/")
157+
assert.Equal(t, lockBefore, readTempLockfilePins(t))
158+
})
159+
}
160+
}
161+
}

‎internal/pipeline/diagnose.go‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -253,26 +253,7 @@ func precheckWorkflow(pw checks.ParsedWorkflow, store *lockfile.State) (checks.W
253253
}
254254

255255
if len(pw.LocalPaths) > 0 {
256-
wfKey := workflowfile.KeyFromPath(pw.Path)
257-
if store != nil && store.HasWorkflow(wfKey) {
258-
wr.Findings = append(wr.Findings, checks.Finding{
259-
WorkflowPath: pw.Path,
260-
Category: checks.LocalAction,
261-
Severity: checks.SeverityError,
262-
Confidence: checks.ConfidenceHigh,
263-
Detail: "workflow uses local path actions which are not supported; remove local path actions to continue using the lockfile",
264-
Remediation: "rewrite same-repo `uses: ./…` to `uses: $/…` (automatic on fix runs; disable with --no-migrate-local-actions), or remove the `./…` steps",
265-
})
266-
} else {
267-
wr.Findings = append(wr.Findings, checks.Finding{
268-
WorkflowPath: pw.Path,
269-
Category: checks.LocalAction,
270-
Severity: checks.SeverityWarning,
271-
Confidence: checks.ConfidenceHigh,
272-
Detail: "workflow uses local path actions; lockfile onboarding is not supported",
273-
Remediation: "rewrite same-repo `uses: ./…` to `uses: $/…` (automatic on fix runs; disable with --no-migrate-local-actions)",
274-
})
275-
}
256+
wr.Findings = append(wr.Findings, localActionFinding(pw, store))
276257
hasTerminalFinding = true
277258
}
278259

@@ -313,6 +294,30 @@ func precheckWorkflow(pw checks.ParsedWorkflow, store *lockfile.State) (checks.W
313294
return wr, false
314295
}
315296

297+
// localActionFinding reports same-repo `./…` refs: an error once the workflow
298+
// is onboarded to the lockfile, a warning before. Shared by --verify and
299+
// --verify-local so both modes agree.
300+
func localActionFinding(pw checks.ParsedWorkflow, store *lockfile.State) checks.Finding {
301+
if store != nil && store.HasWorkflow(workflowfile.KeyFromPath(pw.Path)) {
302+
return checks.Finding{
303+
WorkflowPath: pw.Path,
304+
Category: checks.LocalAction,
305+
Severity: checks.SeverityError,
306+
Confidence: checks.ConfidenceHigh,
307+
Detail: "workflow uses local path actions which are not supported; remove local path actions to continue using the lockfile",
308+
Remediation: "rewrite same-repo `uses: ./…` to `uses: $/…` (automatic on fix runs; disable with --no-migrate-local-actions), or remove the `./…` steps",
309+
}
310+
}
311+
return checks.Finding{
312+
WorkflowPath: pw.Path,
313+
Category: checks.LocalAction,
314+
Severity: checks.SeverityWarning,
315+
Confidence: checks.ConfidenceHigh,
316+
Detail: "workflow uses local path actions; lockfile onboarding is not supported",
317+
Remediation: "rewrite same-repo `uses: ./…` to `uses: $/…` (automatic on fix runs; disable with --no-migrate-local-actions)",
318+
}
319+
}
320+
316321
func preserveExistingInventory(wr *checks.WorkflowReport, pw checks.ParsedWorkflow) {
317322
wr.Deps = pw.ExistingDeps
318323
for _, d := range pw.ExistingDeps {

‎internal/pipeline/verify_local.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,14 @@ func VerifyLocalCoverage(parsed []checks.ParsedWorkflow, store *lockfile.State)
8383
})
8484
}
8585

86-
if len(pw.Refs) == 0 && len(pw.LocalPaths) == 0 {
86+
// Same-repo `./…` refs are terminal, matching --verify.
87+
if len(pw.LocalPaths) > 0 {
88+
wr.Findings = append(wr.Findings, localActionFinding(pw, store))
89+
results = append(results, wr)
90+
continue
91+
}
92+
93+
if len(pw.Refs) == 0 {
8794
results = append(results, wr)
8895
continue
8996
}

‎internal/pipeline/verify_local_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,29 @@ func TestVerifyLocalCoverage_DepsError(t *testing.T) {
210210
assert.True(t, report.Workflows[0].Findings[0].IsRemediableNotPinned())
211211
assert.Contains(t, report.Workflows[0].Findings[0].Detail, "failed to read dependencies")
212212
}
213+
214+
func TestVerifyLocalCoverage_LocalPathsMatchVerify(t *testing.T) {
215+
store := writeTestLockfile(t, t.TempDir(), testLockfileContent)
216+
217+
for _, tc := range []struct {
218+
path string
219+
severity checks.Severity
220+
}{
221+
{".github/workflows/ci.yml", checks.SeverityError},
222+
{".github/workflows/new.yml", checks.SeverityWarning},
223+
} {
224+
t.Run(tc.path, func(t *testing.T) {
225+
pw := checks.ParsedWorkflow{
226+
Path: tc.path,
227+
Refs: []parserlock.ActionRef{{Owner: "actions", Repo: "checkout", Ref: "v4"}},
228+
LocalPaths: []string{"./my-action"},
229+
}
230+
report := VerifyLocalCoverage([]checks.ParsedWorkflow{pw}, store)
231+
require.Len(t, report.Workflows[0].Findings, 1)
232+
f := report.Workflows[0].Findings[0]
233+
assert.Equal(t, checks.LocalAction, f.Category)
234+
assert.Equal(t, tc.severity, f.Severity)
235+
assert.Equal(t, localActionFinding(pw, store), f)
236+
})
237+
}
238+
}

‎test/scenarios/catalog.yml‎

Lines changed: 6 additions & 6 deletions

0 commit comments

Comments
 (0)