From 62047f6b7aca6ff0e74e135f347b8268ad6fc0f6 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 3 Sep 2026 12:24:48 +0200 Subject: [PATCH] The comment in PinGitHubActionsToSha to use the most-specific tag --- .../security/PinGitHubActionsToSha.java | 41 ++++++- .../security/PinGitHubActionsToShaTest.java | 103 ++++++++++++++++++ 2 files changed, 142 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/github/security/PinGitHubActionsToSha.java b/src/main/java/org/openrewrite/github/security/PinGitHubActionsToSha.java index 1b0ba24..2198e4a 100644 --- a/src/main/java/org/openrewrite/github/security/PinGitHubActionsToSha.java +++ b/src/main/java/org/openrewrite/github/security/PinGitHubActionsToSha.java @@ -231,11 +231,11 @@ public Yaml.Document.End visitDocumentEnd(Yaml.Document.End end, ExecutionContex return null; } - // Replace the scalar value, retain the original ref as a comment + // Replace the scalar value, annotate with the most-specific tag for the SHA Yaml.Scalar originalScalar = (Yaml.Scalar) e.getValue(); return new PinResult( e.withValue(originalScalar.withValue(actionPath + "@" + sha)), - formatRefComment(ref) + resolveDisplayTag(knownShas, actionPath, sha, ref) ); } @@ -325,6 +325,43 @@ static String formatRefComment(String ref) { return ref + " @ " + LocalDate.now(ZoneOffset.UTC); } + /** + * Resolve the comment tag written next to a pinned SHA. The static mapping already lists every + * version tag of an action, so a moving tag like {@code v4} shares its SHA with the exact patch + * release it points at (e.g. {@code v4.4.0}). This scans the mapping for tags of the same action + * resolving to {@code sha} and returns the most-specific one, so a reader sees {@code # v4.4.0} + * rather than {@code # v4}. Falls back to the user's original ref when the SHA isn't in the map + * (e.g. resolved via the API, or a branch reference). + */ + static String resolveDisplayTag(Map knownShas, String actionPath, String sha, String originalRef) { + String prefix = actionPath + "@"; + String best = null; + for (Map.Entry entry : knownShas.entrySet()) { + if (!entry.getKey().startsWith(prefix) || !sha.equals(entry.getValue())) { + continue; + } + String tag = entry.getKey().substring(prefix.length()); + if (TAG_REF_PATTERN.matcher(tag).matches() && (best == null || isMoreSpecific(tag, best))) { + best = tag; + } + } + return best != null ? best : formatRefComment(originalRef); + } + + /** + * Compare two version tags by specificity: a tag with more version components (e.g. {@code v4.4.0}) + * is more specific than one with fewer ({@code v4}); an equal number of components is broken by the + * greater tag so the result is stable regardless of map iteration order. + */ + static boolean isMoreSpecific(String candidate, String current) { + long candidateDots = candidate.chars().filter(c -> c == '.').count(); + long currentDots = current.chars().filter(c -> c == '.').count(); + if (candidateDots != currentDots) { + return candidateDots > currentDots; + } + return candidate.compareTo(current) > 0; + } + private static boolean matchesAllowList(String actionPath, List allowList) { // actionPath may be "owner/repo" or "owner/repo/subpath". int firstSlash = actionPath.indexOf('/'); diff --git a/src/test/java/org/openrewrite/github/security/PinGitHubActionsToShaTest.java b/src/test/java/org/openrewrite/github/security/PinGitHubActionsToShaTest.java index 90e7fd9..4a7a7cb 100644 --- a/src/test/java/org/openrewrite/github/security/PinGitHubActionsToShaTest.java +++ b/src/test/java/org/openrewrite/github/security/PinGitHubActionsToShaTest.java @@ -23,6 +23,7 @@ import java.time.LocalDate; import java.time.ZoneOffset; import java.util.List; +import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.yaml.Assertions.yaml; @@ -758,6 +759,108 @@ void emptyAllowListBehavesAsDefault() { ); } + @Test + void shouldUseMostSpecificPatchTagInComment() { + rewriteRun( + spec -> spec.recipe(new PinGitHubActionsToSha(true, null, null, null)), + yaml( + """ + name: CI + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + name: Checkout + """, + """ + name: CI + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@0717577d45739eb3c851188b29f50ed6c0b2194e # v2.8.0 + name: Checkout + """, + sourceSpecs -> sourceSpecs.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void shouldKeepAlreadySpecificPatchTagInComment() { + rewriteRun( + spec -> spec.recipe(new PinGitHubActionsToSha(true, null, null, null)), + yaml( + """ + name: CI + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.8.0 + name: Checkout + """, + """ + name: CI + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@0717577d45739eb3c851188b29f50ed6c0b2194e # v2.8.0 + name: Checkout + """, + sourceSpecs -> sourceSpecs.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void resolveDisplayTagPicksMostSpecificTagForSha() { + // given + Map knownShas = Map.of( + "actions/checkout@v2", "0717577d45739eb3c851188b29f50ed6c0b2194e", + "actions/checkout@v2.8.0", "0717577d45739eb3c851188b29f50ed6c0b2194e", + "actions/checkout@v2.7.0", "ee0669bd1cc54295c223e0bb666b733df41de1c5"); + + // when + String tag = PinGitHubActionsToSha.resolveDisplayTag( + knownShas, "actions/checkout", "0717577d45739eb3c851188b29f50ed6c0b2194e", "v2"); + + // then + assertThat(tag).isEqualTo("v2.8.0"); + } + + @Test + void resolveDisplayTagFallsBackToOriginalRefWhenShaUnknown() { + // given + Map knownShas = Map.of( + "actions/checkout@v2", "0717577d45739eb3c851188b29f50ed6c0b2194e"); + String today = LocalDate.now(ZoneOffset.UTC).toString(); + + // when + String tagRef = PinGitHubActionsToSha.resolveDisplayTag( + knownShas, "some/action", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "v9"); + String branchRef = PinGitHubActionsToSha.resolveDisplayTag( + knownShas, "some/action", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "main"); + + // then + assertThat(tagRef).isEqualTo("v9"); + assertThat(branchRef).isEqualTo("main @ " + today); + } + + @Test + void isMoreSpecificPrefersMoreDotsThenGreaterTag() { + assertThat(PinGitHubActionsToSha.isMoreSpecific("v4.4.0", "v4")).isTrue(); + assertThat(PinGitHubActionsToSha.isMoreSpecific("v4", "v4.4.0")).isFalse(); + assertThat(PinGitHubActionsToSha.isMoreSpecific("v4.5.0", "v4.4.0")).isTrue(); + assertThat(PinGitHubActionsToSha.isMoreSpecific("v4.4.0", "v4.4.0")).isFalse(); + } + @Test void formatRefCommentEmitsTagRefsUnchanged() { assertThat( PinGitHubActionsToSha.formatRefComment( "v4" ) ).isEqualTo( "v4" );