diff --git a/internal/pin/pin.go b/internal/pin/pin.go index 5271d7a..cd67bd6 100644 --- a/internal/pin/pin.go +++ b/internal/pin/pin.go @@ -340,14 +340,12 @@ func PinFile(ctx context.Context, path string, resolver Resolver, w io.Writer, d continue } - // Replace version value in the HCL content. + // Replace version value in the HCL content, adding an inline comment + // with the original tag so the pinned SHA remains human-readable. oldLine := fmt.Sprintf(`version = %q`, ref.Version) - newLine := fmt.Sprintf(`version = %q`, sha) + newLine := fmt.Sprintf(`version = %q # %s`, sha, ref.Version) updated = strings.Replace(updated, oldLine, newLine, 1) - // Add or update the comment above the uses block. - updated = upsertUsesComment(updated, ref.Action, ref.Version) - _, _ = fmt.Fprintf(w, "pinned %s@%s → %s\n", ref.Action, ref.Version, sha[:12]) results = append(results, PinResult{ @@ -428,57 +426,6 @@ func findActionRefs(content string) ([]ActionRef, error) { return refs, nil } -// upsertUsesComment adds or updates the comment line above a uses block -// to document the original action and tag. For example: -// -// // actions/checkout v4 -// uses { -// action = "actions/checkout" -// version = "abc123..." -// } -func upsertUsesComment(content, action, tag string) string { - comment := fmt.Sprintf("// %s %s", action, tag) - actionLine := fmt.Sprintf(`action = %q`, action) - - idx := strings.Index(content, actionLine) - if idx <= 0 { - return content - } - - beforeAction := content[:idx] - usesIdx := strings.LastIndex(beforeAction, "uses {") - - if usesIdx < 0 { - return content - } - - // Find the indent by looking at what's before "uses {" on its line. - lineStart := strings.LastIndex(content[:usesIdx], "\n") + 1 - indent := content[lineStart:usesIdx] - - beforeUses := content[:lineStart] - afterUses := content[lineStart:] - lines := strings.Split(beforeUses, "\n") - - // Check if the line before uses (skipping blank line) is already a comment. - lastIdx := len(lines) - 1 - - if lastIdx >= 0 && strings.TrimSpace(lines[lastIdx]) == "" { - lastIdx-- - } - - if lastIdx >= 0 && strings.HasPrefix(strings.TrimSpace(lines[lastIdx]), "//") { - // Update existing comment, preserving its indent. - existingIndent := lines[lastIdx][:len(lines[lastIdx])-len(strings.TrimLeft(lines[lastIdx], " \t"))] - lines[lastIdx] = existingIndent + comment - - return strings.Join(lines, "\n") + afterUses - } - - // No existing comment — insert one before uses, same indent. - return beforeUses + indent + comment + "\n" + afterUses -} - // drainAndClose reads the remaining body to enable HTTP connection reuse, // then closes it. func drainAndClose(body io.ReadCloser) { diff --git a/internal/pin/pin_test.go b/internal/pin/pin_test.go index ce97107..b544f73 100644 --- a/internal/pin/pin_test.go +++ b/internal/pin/pin_test.go @@ -153,16 +153,16 @@ step "setup" { t.Fatal(err) } - if strings.Contains(string(updated), `"v4"`) { + if strings.Contains(string(updated), `version = "v4"`) { t.Error("v4 tag should have been replaced") } - if !strings.Contains(string(updated), `"abc123def456abc123def456abc123def456abc1"`) { - t.Error("expected checkout SHA in output") + if !strings.Contains(string(updated), `"abc123def456abc123def456abc123def456abc1" # v4`) { + t.Error("expected checkout SHA with inline tag comment in output") } - if !strings.Contains(string(updated), `"def456abc123def456abc123def456abc123def4"`) { - t.Error("expected setup-go SHA in output") + if !strings.Contains(string(updated), `"def456abc123def456abc123def456abc123def4" # v5`) { + t.Error("expected setup-go SHA with inline tag comment in output") } // Verify output messages. @@ -259,7 +259,7 @@ func TestPinFileResolveFails(t *testing.T) { } } -func TestPinFileAddsCommentWhenMissing(t *testing.T) { +func TestPinFileInlineComment(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "steps.hcl") @@ -293,57 +293,8 @@ func TestPinFileAddsCommentWhenMissing(t *testing.T) { t.Fatal(err) } - if !strings.Contains(string(updated), "// actions/setup-go v5") { - t.Errorf("expected comment to be added\ngot:\n%s", string(updated)) - } - - // Verify indent: comment should have same indent as "uses {" - if !strings.Contains(string(updated), " // actions/setup-go v5\n uses {") { - t.Errorf("comment should have same indent as uses block\ngot:\n%s", string(updated)) - } -} - -func TestPinFileUpdatesExistingComment(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "steps.hcl") - - content := `step "checkout" { - // actions/checkout v3 - uses { - action = "actions/checkout" - version = "v4" - } -} -` - - if err := os.WriteFile(path, []byte(content), 0644); err != nil { - t.Fatal(err) - } - - resolver := &mockResolver{ - shas: map[string]string{ - "actions/checkout@v4": "abc123def456abc123def456abc123def456abc1", - }, - } - - var buf bytes.Buffer - - _, err := PinFile(context.Background(), path, resolver, &buf, false) - if err != nil { - t.Fatal(err) - } - - updated, err := os.ReadFile(path) - if err != nil { - t.Fatal(err) - } - - if !strings.Contains(string(updated), "// actions/checkout v4") { - t.Errorf("expected comment to be updated to v4\ngot:\n%s", string(updated)) - } - - if strings.Contains(string(updated), "// actions/checkout v3") { - t.Error("old comment v3 should have been replaced") + if !strings.Contains(string(updated), `"def456abc123def456abc123def456abc123def4" # v5`) { + t.Errorf("expected inline tag comment on version line\ngot:\n%s", string(updated)) } } diff --git a/internal/pin/upgrade.go b/internal/pin/upgrade.go index 6ae54e8..f74f408 100644 --- a/internal/pin/upgrade.go +++ b/internal/pin/upgrade.go @@ -100,14 +100,11 @@ func UpgradeFile(ctx context.Context, path string, resolver Upgrader, w io.Write continue } - // Replace version value. + // Replace version value, adding an inline comment with the new tag. oldLine := fmt.Sprintf(`version = %q`, ref.Version) - newLine := fmt.Sprintf(`version = %q`, sha) + newLine := fmt.Sprintf(`version = %q # %s`, sha, latestTag) updated = strings.Replace(updated, oldLine, newLine, 1) - // Add or update comment. - updated = upsertUsesComment(updated, ref.Action, latestTag) - _, _ = fmt.Fprintf(w, "upgraded %s: %s → %s (%s)\n", ref.Action, ref.Version, latestTag, sha[:12]) results = append(results, UpgradeResult{ diff --git a/internal/pin/upgrade_test.go b/internal/pin/upgrade_test.go index da41f66..acdce67 100644 --- a/internal/pin/upgrade_test.go +++ b/internal/pin/upgrade_test.go @@ -141,24 +141,65 @@ func TestUpgradeDirectoryNoHCL(t *testing.T) { } } -func TestUpsertUsesCommentForUpgrade(t *testing.T) { - // Verify that upsertUsesComment works when upgrading from v4 to v6. +type stubUpgrader struct { + latestTags map[string]string + shas map[string]string +} + +func (s *stubUpgrader) ResolveTag(_ context.Context, owner, repo, tag string) (string, error) { + key := fmt.Sprintf("%s/%s@%s", owner, repo, tag) + + if sha, ok := s.shas[key]; ok { + return sha, nil + } + + return "", fmt.Errorf("tag not found: %s", key) +} + +func (s *stubUpgrader) LatestTag(_ context.Context, owner, repo string) (string, error) { + key := fmt.Sprintf("%s/%s", owner, repo) + + if tag, ok := s.latestTags[key]; ok { + return tag, nil + } + + return "", fmt.Errorf("no releases for %s", key) +} + +func TestUpgradeInlineComment(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "steps.hcl") + content := `step "checkout" { - // actions/checkout v4 uses { action = "actions/checkout" - version = "old-sha" + version = "aabbccddeeff00112233445566778899aabbccdd" } } ` - updated := upsertUsesComment(content, "actions/checkout", "v6") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } + + resolver := &stubUpgrader{ + latestTags: map[string]string{"actions/checkout": "v6"}, + shas: map[string]string{"actions/checkout@v6": "b80b16730d25b9d3b6b2df7ca91e17d1ca6b9ef5"}, + } - if !strings.Contains(updated, "// actions/checkout v6") { - t.Errorf("expected comment updated to v6\ngot:\n%s", updated) + var buf bytes.Buffer + + _, err := UpgradeFile(context.Background(), path, resolver, &buf, false) + if err != nil { + t.Fatal(err) + } + + updated, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) } - if strings.Contains(updated, "// actions/checkout v4") { - t.Error("old v4 comment should be replaced") + if !strings.Contains(string(updated), `"b80b16730d25b9d3b6b2df7ca91e17d1ca6b9ef5" # v6`) { + t.Errorf("expected inline tag comment on version line\ngot:\n%s", string(updated)) } } diff --git a/provider/github/parse_workflow.go b/provider/github/parse_workflow.go index 9522d4e..8b391fa 100644 --- a/provider/github/parse_workflow.go +++ b/provider/github/parse_workflow.go @@ -264,11 +264,7 @@ func parseJobConfig(cfg hclJobBlock, hv *hclparser.HCLVars) (map[string]any, err return nil, err } - if len(child) == 0 { - out["permissions"] = "read-all" - } else { - out["permissions"] = child - } + out["permissions"] = child } for _, block := range cfg.Defaults { @@ -375,11 +371,7 @@ func parseWorkflowConfig(cfg hclWorkflowBlock, hv *hclparser.HCLVars) (map[strin return nil, err } - if len(child) == 0 { - out["permissions"] = "read-all" - } else { - out["permissions"] = child - } + out["permissions"] = child } for _, block := range cfg.Defaults {