diff --git a/demo/try-it.sh b/demo/try-it.sh index a6e2ef62..9bdef1cf 100755 --- a/demo/try-it.sh +++ b/demo/try-it.sh @@ -19,6 +19,20 @@ RESET_COLOR='\033[0m' banner() { echo -e "\n${BOLD}${CYAN}── $1 ──${RESET_COLOR}\n"; } comment() { echo -e "${DIM}# $1${RESET_COLOR}"; } run() { echo -e "${GREEN}\$ $*${RESET_COLOR}"; "$@"; echo; } +show_workflow_summary() { + awk ' + /uses:/ || /^# Automatically generated/ || /^dependencies:/ || /^ # / || /^ - / { + print + } + ' "$1" +} +show_lockfile() { + awk ' + /^# Automatically generated/ || /^dependencies:/ || /^ # / || /^ - / { + print + } + ' "$1" +} scenarios=( "check-autofix" @@ -61,12 +75,12 @@ usage() { scenario_check_autofix() { banner "Auto-fix (non-interactive)" bash "$RESET" - comment "Unpinned workflow — 3 actions using mutable tags" + comment "Unpinned workflow — 4 actions using mutable tags, including a nested composite" run grep uses: demo/workflows-check/ci.yml comment "Pin all actions (non-interactive auto-fix)" run gh actions-pin check --no-interactive demo/workflows-check/ci.yml comment "Lockfile written — check the result" - run tail -6 demo/workflows-check/ci.yml + run show_lockfile demo/workflows-check/ci.yml comment "Subsequent check passes" run gh actions-pin check demo/workflows-check/ci.yml } @@ -106,11 +120,11 @@ scenario_upgrade_latest() { banner "Upgrade to latest" bash "$RESET" comment "Currently pinned to older versions" - run grep -E 'uses:| -' demo/workflows-upgrade/ci.yml + run show_workflow_summary demo/workflows-upgrade/ci.yml comment "Upgrade actions/checkout to latest" run gh actions-pin upgrade --action actions/checkout demo/workflows-upgrade/ci.yml comment "Updated refs and lockfile" - run grep -E 'uses:| -' demo/workflows-upgrade/ci.yml + run show_workflow_summary demo/workflows-upgrade/ci.yml } scenario_upgrade_version() { @@ -137,14 +151,14 @@ scenario_edit_repin() { comment "Re-pin with check" run gh actions-pin check --no-interactive demo/workflows-upgrade/ci.yml comment "New lockfile written" - run tail -5 demo/workflows-upgrade/ci.yml + run show_lockfile demo/workflows-upgrade/ci.yml } scenario_ref_moved() { banner "Ref moved — routine update" bash "$RESET" comment "Workflow pinned before tag moved forward (normal release)" - run grep -E 'uses:| -' demo/workflows-pwned/5-pinned-before-update.yml + run show_workflow_summary demo/workflows-pwned/5-pinned-before-update.yml comment "Check detects the tag now points to a newer commit" run gh actions-pin check demo/workflows-pwned/5-pinned-before-update.yml } @@ -153,7 +167,7 @@ scenario_imposter_commit() { banner "Imposter commit — fork injection" bash "$RESET" comment "Workflow pinned BEFORE the tag was hijacked" - run grep -E 'uses:| -' demo/workflows-pwned/1-pinned-before-hijack.yml + run show_workflow_summary demo/workflows-pwned/1-pinned-before-hijack.yml comment "Check detects the tag moved to an orphan commit" run gh actions-pin check demo/workflows-pwned/1-pinned-before-hijack.yml } diff --git a/demo/vhs/check-autofix.tape b/demo/vhs/check-autofix.tape index 80d477ad..a2854f72 100644 --- a/demo/vhs/check-autofix.tape +++ b/demo/vhs/check-autofix.tape @@ -10,7 +10,7 @@ Set PlaybackSpeed 1.5 Set TypingSpeed 50ms Set Theme "GitHub Dark" -Type "echo '# Unpinned workflow — 3 actions using mutable tags'" +Type "echo '# Unpinned workflow — 4 actions using mutable tags, including a nested composite'" Enter Sleep 1s @@ -30,6 +30,6 @@ Type "echo '# Lockfile written — check the result'" Enter Sleep 1s -Type "tail -6 demo/workflows-check/ci.yml" +Type "awk '/^# Automatically generated/ || /^dependencies:/ || /^ # / || /^ - / { print }' demo/workflows-check/ci.yml" Enter Sleep 3s diff --git a/demo/vhs/reset-fixtures.sh b/demo/vhs/reset-fixtures.sh index c5d08112..77c265cd 100755 --- a/demo/vhs/reset-fixtures.sh +++ b/demo/vhs/reset-fixtures.sh @@ -26,6 +26,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + - uses: nodeselector/actions-test-fixtures/nested-composite@updated - uses: golangci/golangci-lint-action@v6 with: version: latest diff --git a/demo/workflows-check/ci.yml b/demo/workflows-check/ci.yml index e083bf1c..8c1d4f6d 100644 --- a/demo/workflows-check/ci.yml +++ b/demo/workflows-check/ci.yml @@ -8,8 +8,8 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 + - uses: actions/checkout@v4.3.1 + - uses: actions/setup-go@v5.6.0 with: go-version-file: go.mod - run: go test ./... @@ -17,7 +17,20 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: golangci/golangci-lint-action@v6 + - uses: actions/checkout@v4.3.1 + - uses: nodeselector/actions-test-fixtures/nested-composite@updated + - uses: golangci/golangci-lint-action@v6.5.2 with: version: latest + +# Automatically generated and managed by gh-actions-pin +dependencies: + - github.com/actions/checkout@v4.3.1:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - github.com/actions/setup-go@v5.6.0:sha1-40f1582b2485089dde7abd97c1529aa768e1baff + - github.com/golangci/golangci-lint-action@v6.5.2:sha1-55c2c1448f86e01eaae002a5a3a9624417608d84 + - github.com/nodeselector/actions-test-fixtures/nested-composite@updated:sha1-ea53476fdc172d8552df5af9658a45a367e4f41d + + # Transitive dependencies + - github.com/nodeselector/actions-test-fixtures-b/simple-echo@main:sha1-92b7b0058bc223c6e9dd4e19ef9247c934ba7637 + - github.com/nodeselector/actions-test-fixtures/simple-composite@main:sha1-fbe04216bcc00ebe76c49276d4b8ee066a21ef22 + - github.com/nodeselector/actions-test-fixtures/simple-node@main:sha1-fbe04216bcc00ebe76c49276d4b8ee066a21ef22 diff --git a/internal/lockfile/lockfile.go b/internal/lockfile/lockfile.go index 749636e5..0ea62e85 100644 --- a/internal/lockfile/lockfile.go +++ b/internal/lockfile/lockfile.go @@ -435,17 +435,60 @@ func (f *File) ReadDependencies() ([]Dependency, error) { // WriteDependencies returns the workflow content with an updated dependencies: section. func (f *File) WriteDependencies(deps []Dependency) ([]byte, error) { content := string(f.Content) + directRefs, _, _ := f.ExtractActionRefs() + directKeys := make(map[string]bool, len(directRefs)) + for _, ref := range directRefs { + directKeys[ref.FullName()+"@"+ref.Ref] = true + } + + directDepsByKey := make(map[string]Dependency) + transitiveDepsByKey := make(map[string]Dependency) + for _, dep := range deps { + key := dep.Key() + if directKeys[key] { + directDepsByKey[key] = dep + delete(transitiveDepsByKey, key) + continue + } + if _, exists := directDepsByKey[key]; exists { + continue + } + if _, exists := transitiveDepsByKey[key]; !exists { + transitiveDepsByKey[key] = dep + } + } - sort.Slice(deps, func(i, j int) bool { - return deps[i].String() < deps[j].String() + directDeps := make([]Dependency, 0, len(directDepsByKey)) + for _, dep := range directDepsByKey { + directDeps = append(directDeps, dep) + } + transitiveDeps := make([]Dependency, 0, len(transitiveDepsByKey)) + for _, dep := range transitiveDepsByKey { + transitiveDeps = append(transitiveDeps, dep) + } + + sort.Slice(directDeps, func(i, j int) bool { + return directDeps[i].String() < directDeps[j].String() + }) + sort.Slice(transitiveDeps, func(i, j int) bool { + return transitiveDeps[i].String() < transitiveDeps[j].String() }) var sb strings.Builder sb.WriteString("\n# Automatically generated and managed by gh-actions-pin\n") sb.WriteString("dependencies:\n") - for _, dep := range deps { + for _, dep := range directDeps { sb.WriteString(" - " + dep.String() + "\n") } + if len(transitiveDeps) > 0 { + if len(directDeps) > 0 { + sb.WriteString("\n") + } + sb.WriteString(" # Transitive dependencies\n") + for _, dep := range transitiveDeps { + sb.WriteString(" - " + dep.String() + "\n") + } + } content = removeDependenciesSection(content) content = strings.TrimRight(content, "\n") + "\n" @@ -489,8 +532,8 @@ func (f *File) RewriteActionRefs(replacements map[string]string) ([]byte, int, e } var ( - reDepsSectionWithComment = regexp.MustCompile(`(?m)^\n?# Automatically generated and managed by[^\n]*\ndependencies:\n(?: - .*\n)*`) - reDepsSectionBare = regexp.MustCompile(`(?m)^dependencies:\n(?: - .*\n)*`) + reDepsSectionWithComment = regexp.MustCompile(`(?ms)^\n?# Automatically generated and managed by[^\n]*\ndependencies:\n(?: (?:#.*|- .*)\n|\n)*`) + reDepsSectionBare = regexp.MustCompile(`(?ms)^dependencies:\n(?: (?:#.*|- .*)\n|\n)*`) ) func removeDependenciesSection(content string) string { diff --git a/internal/lockfile/lockfile_test.go b/internal/lockfile/lockfile_test.go index 4464b068..2e5e5200 100644 --- a/internal/lockfile/lockfile_test.go +++ b/internal/lockfile/lockfile_test.go @@ -191,6 +191,50 @@ func TestWriteDependencies(t *testing.T) { assert.Contains(t, s, "github.com/actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683") assert.Contains(t, s, "github.com/actions/setup-go@v5:sha1-d35c59abb061a4a6fb18e82ac0862c26744d6ab5") assert.Contains(t, s, "# Automatically generated and managed by gh-actions-pin") + assert.NotContains(t, s, "# Direct dependencies") +} + +func TestWriteDependenciesAddsTransitiveSection(t *testing.T) { + f, err := Load("testdata/simple.yml") + require.NoError(t, err) + + deps := []Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: "11bd71901bbe5b1630ceea73d27597364c9af683", HashAlgo: "sha1"}, + {NWO: "actions/setup-go", Ref: "v5", SHA: "d35c59abb061a4a6fb18e82ac0862c26744d6ab5", HashAlgo: "sha1"}, + {NWO: "actions/cache/save", Ref: "v4", SHA: "5a3ec84eff668545956fd18022155c47e93e2684", HashAlgo: "sha1"}, + } + + output, err := f.WriteDependencies(deps) + require.NoError(t, err) + + s := string(output) + assert.Contains(t, s, "# Transitive dependencies") + assert.Contains(t, s, "github.com/actions/cache/save@v4:sha1-5a3ec84eff668545956fd18022155c47e93e2684") + assert.NotContains(t, s, "# Direct dependencies") + assert.Less(t, strings.Index(s, "github.com/actions/setup-go@v5"), strings.Index(s, "# Transitive dependencies")) +} + +func TestWriteDependenciesDeduplicatesDirectAndTransitive(t *testing.T) { + f, err := Load("testdata/simple.yml") + require.NoError(t, err) + + deps := []Dependency{ + {NWO: "actions/checkout", Ref: "v4", SHA: "11bd71901bbe5b1630ceea73d27597364c9af683", HashAlgo: "sha1"}, + {NWO: "actions/setup-go", Ref: "v5", SHA: "d35c59abb061a4a6fb18e82ac0862c26744d6ab5", HashAlgo: "sha1"}, + {NWO: "actions/setup-go", Ref: "v5", SHA: "d35c59abb061a4a6fb18e82ac0862c26744d6ab5", HashAlgo: "sha1"}, + {NWO: "actions/cache/save", Ref: "v4", SHA: "5a3ec84eff668545956fd18022155c47e93e2684", HashAlgo: "sha1"}, + {NWO: "actions/cache/save", Ref: "v4", SHA: "5a3ec84eff668545956fd18022155c47e93e2684", HashAlgo: "sha1"}, + } + + output, err := f.WriteDependencies(deps) + require.NoError(t, err) + + s := string(output) + assert.Equal(t, 1, strings.Count(s, "github.com/actions/setup-go@v5:sha1-d35c59abb061a4a6fb18e82ac0862c26744d6ab5")) + assert.Equal(t, 1, strings.Count(s, "github.com/actions/cache/save@v4:sha1-5a3ec84eff668545956fd18022155c47e93e2684")) + assert.Contains(t, s, "# Transitive dependencies") + assert.Less(t, strings.Index(s, "github.com/actions/setup-go@v5"), strings.Index(s, "# Transitive dependencies")) + assert.Greater(t, strings.Index(s, "github.com/actions/cache/save@v4"), strings.Index(s, "# Transitive dependencies")) } func TestWriteDependenciesRoundTrip(t *testing.T) { @@ -227,6 +271,7 @@ func TestWriteDependenciesReplacesExisting(t *testing.T) { assert.NotContains(t, s, "d35c59abb061a4a6fb18e82ac0862c26744d6ab5") assert.Contains(t, s, "0000000000000000000000000000000000000001") assert.Equal(t, 1, strings.Count(s, "dependencies:")) + assert.NotContains(t, s, "# Direct dependencies") } func TestRewriteActionRefs(t *testing.T) { @@ -343,6 +388,33 @@ func TestDependencySorted(t *testing.T) { assert.Greater(t, idxLast, idxFirst, "dependencies should be sorted alphabetically") } +func TestWriteDependenciesWithOnlyTransitiveDeps(t *testing.T) { + content := []byte(`name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: go test ./... +`) + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "transitive_only.yml") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + f, err := Load(path) + require.NoError(t, err) + + deps := []Dependency{{NWO: "actions/cache/save", Ref: "v4", SHA: "5a3ec84eff668545956fd18022155c47e93e2684", HashAlgo: "sha1"}} + + output, err := f.WriteDependencies(deps) + require.NoError(t, err) + + s := string(output) + assert.NotContains(t, s, "# Direct dependencies") + assert.Contains(t, s, "# Transitive dependencies") + assert.Contains(t, s, "github.com/actions/cache/save@v4:sha1-5a3ec84eff668545956fd18022155c47e93e2684") +} + func TestParseActionMeta(t *testing.T) { tests := []struct { name string