From 47ff257ad9f1f1b47fa07fb9c4b464e93f68bff6 Mon Sep 17 00:00:00 2001 From: Scott Jungling Date: Wed, 15 Oct 2025 13:53:13 -0700 Subject: [PATCH 1/3] Refactor: Replace YamlHelper utility with YamlScalarAccessor trait Migrated all security recipes from static YamlHelper utility methods to the YamlScalarAccessor trait pattern. This improves code organization and follows OpenRewrite best practices for LST manipulation. Changes: - Created YamlScalarAccessor trait with 6 default methods - Migrated 7 recipes to implement the trait - Deleted deprecated YamlHelper utility class - All tests pass --- .../security/ArtifactSecurityRecipe.java | 9 +- .../github/security/CachePoisoningRecipe.java | 15 +- .../security/ExcessivePermissionsRecipe.java | 7 +- .../github/security/GitHubEnvRecipe.java | 9 +- .../security/SelfHostedRunnerRecipe.java | 7 +- .../security/TemplateInjectionRecipe.java | 9 +- .../security/UnpinnedDockerImagesRecipe.java | 5 +- .../github/security/YamlHelper.java | 90 ------- .../github/traits/YamlScalarAccessor.java | 243 ++++++++++++++++++ .../github/traits/package-info.java | 61 +++++ 10 files changed, 338 insertions(+), 117 deletions(-) delete mode 100644 src/main/java/org/openrewrite/github/security/YamlHelper.java create mode 100644 src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java create mode 100644 src/main/java/org/openrewrite/github/traits/package-info.java diff --git a/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java b/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java index 1cf387e..5ff1a3d 100644 --- a/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java +++ b/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java @@ -19,6 +19,7 @@ import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.*; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.JsonPathMatcher; import org.openrewrite.yaml.YamlIsoVisitor; @@ -78,7 +79,7 @@ public TreeVisitor getVisitor() { ); } - private static class ArtifactSecurityVisitor extends YamlIsoVisitor { + private static class ArtifactSecurityVisitor extends YamlIsoVisitor implements YamlScalarAccessor { private static final JsonPathMatcher STEP_USES_MATCHER = new JsonPathMatcher("$..steps[*].uses"); @@ -108,7 +109,7 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) { - String usesValue = YamlHelper.getScalarValue(entry.getValue()); + String usesValue = getScalarValue(entry.getValue()); if (usesValue == null) { return entry; } @@ -133,7 +134,7 @@ private Yaml.Mapping.Entry checkCheckoutAction(Yaml.Mapping.Entry entry) { return entry; } - String persistCredentials = YamlHelper.findNestedScalarValue(stepMapping, "with", "persist-credentials"); + String persistCredentials = findNestedScalarValue(stepMapping, "with", "persist-credentials"); if (persistCredentials == null) { // No 'with' section or no persist-credentials means default behavior (persist-credentials: true) @@ -159,7 +160,7 @@ private Yaml.Mapping.Entry checkUploadArtifactAction(Yaml.Mapping.Entry entry) { return entry; } - String pathValue = YamlHelper.findNestedScalarValue(stepMapping, "with", "path"); + String pathValue = findNestedScalarValue(stepMapping, "with", "path"); if (pathValue != null && hasDangerousArtifactPaths(pathValue)) { return SearchResult.found(entry, "Uploading potentially sensitive paths that may contain credentials or configuration files."); diff --git a/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java b/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java index 08fca92..26c2293 100644 --- a/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java +++ b/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java @@ -20,6 +20,7 @@ import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; @@ -90,7 +91,7 @@ public TreeVisitor getVisitor() { return new CachePoisoningVisitor(); } - private static class CachePoisoningVisitor extends YamlIsoVisitor { + private static class CachePoisoningVisitor extends YamlIsoVisitor implements YamlScalarAccessor { private boolean isPublishingWorkflow = false; private boolean hasPublisherAction = false; @@ -131,14 +132,14 @@ private void analyzeWorkflow(Yaml.Document document) { } private boolean isPublishingTrigger(Yaml.Block onValue) { - String scalarTrigger = YamlHelper.getScalarValue(onValue); + String scalarTrigger = getScalarValue(onValue); if (scalarTrigger != null) { return "release".equals(scalarTrigger); } if (onValue instanceof Yaml.Sequence) { Yaml.Sequence sequence = (Yaml.Sequence) onValue; for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) { - String trigger = YamlHelper.getScalarValue(seqEntry.getBlock()); + String trigger = getScalarValue(seqEntry.getBlock()); if ("release".equals(trigger)) { return true; } @@ -184,7 +185,7 @@ private boolean hasReleaseBranches(Yaml.Block branchesValue) { if (branchesValue instanceof Yaml.Sequence) { Yaml.Sequence sequence = (Yaml.Sequence) branchesValue; for (Yaml.Sequence.Entry entry : sequence.getEntries()) { - String branch = YamlHelper.getScalarValue(entry.getBlock()); + String branch = getScalarValue(entry.getBlock()); if (branch != null && RELEASE_BRANCH_PATTERN.matcher(branch).matches()) { return true; } @@ -230,7 +231,7 @@ private boolean jobHasPublisherAction(Yaml.Mapping jobMapping) { private boolean stepUsesPublisherAction(Yaml.Mapping stepMapping) { for (Yaml.Mapping.Entry entry : stepMapping.getEntries()) { if ("uses".equals(entry.getKey().getValue())) { - String uses = YamlHelper.getScalarValue(entry.getValue()); + String uses = getScalarValue(entry.getValue()); if (uses != null) { String actionName = extractActionName(uses); return PUBLISHER_ACTIONS.contains(actionName); @@ -262,7 +263,7 @@ private boolean isCacheAwareActionStep(Yaml.Mapping.Entry entry) { return false; } - String uses = YamlHelper.getScalarValue(entry.getValue()); + String uses = getScalarValue(entry.getValue()); if (uses == null) { return false; } @@ -272,7 +273,7 @@ private boolean isCacheAwareActionStep(Yaml.Mapping.Entry entry) { } private String getActionName(Yaml.Mapping.Entry entry) { - String uses = YamlHelper.getScalarValue(entry.getValue()); + String uses = getScalarValue(entry.getValue()); return uses != null ? extractActionName(uses) : "unknown"; } diff --git a/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java b/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java index 3dedd87..6c10a96 100644 --- a/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java +++ b/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java @@ -18,6 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; @@ -58,7 +59,7 @@ public TreeVisitor getVisitor() { ); } - private static class ExcessivePermissionsVisitor extends YamlIsoVisitor { + private static class ExcessivePermissionsVisitor extends YamlIsoVisitor implements YamlScalarAccessor { @Override public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { @@ -76,7 +77,7 @@ private boolean isPermissionsEntry(Yaml.Mapping.Entry entry) { } private Yaml.Mapping.Entry checkPermissions(Yaml.Mapping.Entry entry) { - String scalarPermissionValue = YamlHelper.getScalarValue(entry.getValue()); + String scalarPermissionValue = getScalarValue(entry.getValue()); if (scalarPermissionValue != null) { return checkScalarPermissions(entry, scalarPermissionValue); } @@ -107,7 +108,7 @@ private Yaml.Mapping.Entry checkMappingPermissions(Yaml.Mapping.Entry entry, Yam for (Yaml.Mapping.Entry permEntry : permissionsMapping.getEntries()) { String permissionName = permEntry.getKey().getValue(); - String permissionValue = YamlHelper.getScalarValue(permEntry.getValue()); + String permissionValue = getScalarValue(permEntry.getValue()); if (permissionName != null && permissionValue != null) { if ("write".equals(permissionValue)) { diff --git a/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java b/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java index 9bd8eaa..4164e03 100644 --- a/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java +++ b/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java @@ -20,6 +20,7 @@ import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; @@ -75,7 +76,7 @@ public TreeVisitor getVisitor() { return new GitHubEnvVisitor(); } - private static class GitHubEnvVisitor extends YamlIsoVisitor { + private static class GitHubEnvVisitor extends YamlIsoVisitor implements YamlScalarAccessor { private boolean hasDangerousTriggers = false; @@ -109,14 +110,14 @@ private void analyzeTriggers(Yaml.Document document) { } private boolean checkForDangerousTriggers(Yaml.Block onValue) { - String scalarTrigger = YamlHelper.getScalarValue(onValue); + String scalarTrigger = getScalarValue(onValue); if (scalarTrigger != null) { return DANGEROUS_TRIGGERS.contains(scalarTrigger); } if (onValue instanceof Yaml.Sequence) { Yaml.Sequence sequence = (Yaml.Sequence) onValue; for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) { - String trigger = YamlHelper.getScalarValue(seqEntry.getBlock()); + String trigger = getScalarValue(seqEntry.getBlock()); if (trigger != null && DANGEROUS_TRIGGERS.contains(trigger)) { return true; } @@ -163,7 +164,7 @@ private boolean isRunStepEntry(Yaml.Mapping.Entry entry) { } private String getRunContent(Yaml.Mapping.Entry entry) { - return YamlHelper.getScalarValue(entry.getValue()); + return getScalarValue(entry.getValue()); } private boolean usesGitHubEnv(String runContent) { diff --git a/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java b/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java index 08de146..a979339 100644 --- a/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java +++ b/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java @@ -18,6 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; @@ -47,7 +48,7 @@ public TreeVisitor getVisitor() { ); } - private static class SelfHostedRunnerVisitor extends YamlIsoVisitor { + private static class SelfHostedRunnerVisitor extends YamlIsoVisitor implements YamlScalarAccessor { @Override public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { @@ -87,7 +88,7 @@ private Yaml.Mapping.Entry checkRunsOnValue(Yaml.Mapping.Entry entry, String run } private Yaml.Mapping.Entry checkRunsOnSequence(Yaml.Mapping.Entry entry, Yaml.Sequence sequence) { - String firstValue = YamlHelper.getScalarValue(sequence.getEntries().get(0).getBlock()); + String firstValue = getScalarValue(sequence.getEntries().get(0).getBlock()); if ("self-hosted".equals(firstValue)) { return SearchResult.found(entry, "Uses self-hosted runner which may have security implications in public repositories. " + @@ -137,7 +138,7 @@ private boolean containsSelfHostedInMatrixValues(Yaml.Mapping matrixMapping) { if (matrixEntry.getValue() instanceof Yaml.Sequence) { Yaml.Sequence sequence = (Yaml.Sequence) matrixEntry.getValue(); for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) { - String value = YamlHelper.getScalarValue(seqEntry.getBlock()); + String value = getScalarValue(seqEntry.getBlock()); if ("self-hosted".equals(value)) { return true; } diff --git a/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java b/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java index 21ca547..ceee308 100644 --- a/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java +++ b/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java @@ -19,6 +19,7 @@ import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.*; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.JsonPathMatcher; import org.openrewrite.yaml.YamlIsoVisitor; @@ -93,7 +94,7 @@ public TreeVisitor getVisitor() { ); } - private static class TemplateInjectionVisitor extends YamlIsoVisitor { + private static class TemplateInjectionVisitor extends YamlIsoVisitor implements YamlScalarAccessor { private static final JsonPathMatcher STEP_RUN_MATCHER = new JsonPathMatcher("$..steps[*].run"); private static final JsonPathMatcher STEP_USES_MATCHER = new JsonPathMatcher("$..steps[*].uses"); @@ -122,7 +123,7 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC } private Yaml.Mapping.Entry checkRunEntry(Yaml.Mapping.Entry entry) { - String runCommand = YamlHelper.getScalarValue(entry.getValue()); + String runCommand = getScalarValue(entry.getValue()); if (runCommand == null) { return entry; } @@ -143,7 +144,7 @@ private Yaml.Mapping.Entry checkRunEntry(Yaml.Mapping.Entry entry) { } private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) { - String usesValue = YamlHelper.getScalarValue(entry.getValue()); + String usesValue = getScalarValue(entry.getValue()); if (usesValue == null) { return entry; } @@ -161,7 +162,7 @@ private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) { } private Yaml.Mapping.Entry checkScriptEntry(Yaml.Mapping.Entry entry) { - String scriptContent = YamlHelper.getScalarValue(entry.getValue()); + String scriptContent = getScalarValue(entry.getValue()); if (scriptContent == null) { return entry; } diff --git a/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java b/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java index 6549f17..9ade155 100644 --- a/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java +++ b/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java @@ -18,6 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; +import org.openrewrite.github.traits.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; @@ -55,7 +56,7 @@ public TreeVisitor getVisitor() { ); } - private static class UnpinnedDockerImagesVisitor extends YamlIsoVisitor { + private static class UnpinnedDockerImagesVisitor extends YamlIsoVisitor implements YamlScalarAccessor { @Override public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { @@ -78,7 +79,7 @@ private boolean isImageEntry(Yaml.Mapping.Entry entry) { } private String getImageValue(Yaml.Mapping.Entry entry) { - return YamlHelper.getScalarValue(entry.getValue()); + return getScalarValue(entry.getValue()); } private boolean isUnpinnedDockerImage(String imageValue) { diff --git a/src/main/java/org/openrewrite/github/security/YamlHelper.java b/src/main/java/org/openrewrite/github/security/YamlHelper.java deleted file mode 100644 index 48a3acd..0000000 --- a/src/main/java/org/openrewrite/github/security/YamlHelper.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - *

- * Licensed under the Moderne Source Available License (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://docs.moderne.io/licensing/moderne-source-available-license - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.openrewrite.github.security; - -import org.jspecify.annotations.Nullable; -import org.openrewrite.yaml.tree.Yaml; - -/** - * Utility class containing common patterns for working with YAML LST in OpenRewrite recipes. - * These methods help eliminate code duplication and provide idiomatic ways to work with YAML structures. - */ -final class YamlHelper { - - private YamlHelper() { - // Utility class - prevent instantiation - } - - /** - * Safely extracts the string value from a YAML block if it's a scalar. - * - * @param block The YAML block to extract value from - * @return The string value if block is a Yaml.Scalar, null otherwise - */ - public static @Nullable String getScalarValue(Yaml.Block block) { - return block instanceof Yaml.Scalar ? ((Yaml.Scalar) block).getValue() : null; - } - - /** - * Finds a mapping entry with the given key in a YAML mapping. - * - * @param mapping The YAML mapping to search in - * @param key The key to look for - * @return The Yaml.Mapping if found, null otherwise - */ - private static Yaml.@Nullable Mapping findMappingWithKey(Yaml.Mapping mapping, String key) { - for (Yaml.Mapping.Entry entry : mapping.getEntries()) { - if (key.equals(entry.getKey().getValue()) && entry.getValue() instanceof Yaml.Mapping) { - return (Yaml.Mapping) entry.getValue(); - } - } - return null; - } - - /** - * Finds a scalar value for a given key in a YAML mapping. - * - * @param mapping The YAML mapping to search in - * @param key The key to look for - * @return The scalar value if found, null otherwise - */ - private static @Nullable String findScalarValue(Yaml.Mapping mapping, String key) { - for (Yaml.Mapping.Entry entry : mapping.getEntries()) { - if (key.equals(entry.getKey().getValue())) { - return getScalarValue(entry.getValue()); - } - } - return null; - } - - /** - * Finds a nested scalar value by traversing through a parent key to a child key. - * For example, findNestedScalarValue(mapping, "with", "path") would find the value at with.path - * - * @param mapping The YAML mapping to search in - * @param parentKey The parent key (e.g., "with") - * @param childKey The child key (e.g., "path") - * @return The nested scalar value if found, null otherwise - */ - public static @Nullable String findNestedScalarValue(Yaml.Mapping mapping, String parentKey, String childKey) { - Yaml.Mapping parentMapping = findMappingWithKey(mapping, parentKey); - if (parentMapping == null) { - return null; - } - return findScalarValue(parentMapping, childKey); - } - -} diff --git a/src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java b/src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java new file mode 100644 index 0000000..a8c6f20 --- /dev/null +++ b/src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java @@ -0,0 +1,243 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.github.traits; + +import org.jspecify.annotations.Nullable; +import org.openrewrite.yaml.tree.Yaml; + +/** + * A trait that provides safe access to scalar values from YAML LST elements. + * This trait encapsulates common patterns for extracting string values from + * YAML blocks, eliminating the need for repeated type checking and casting. + * + *

This trait is designed to be mixed into recipes or visitors that need to + * work with YAML scalar values, providing a cleaner API than static utility methods. + * It follows OpenRewrite best practices for trait-based recipe development.

+ * + *

Usage Example

+ *
+ * private static class MyVisitor extends YamlIsoVisitor<ExecutionContext>
+ *         implements YamlScalarAccessor {
+ *
+ *     {@literal @}Override
+ *     public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) {
+ *         // Extract scalar value from mapping entry
+ *         String value = getScalarValue(entry.getValue());
+ *         if (value != null) {
+ *             // Work with the scalar value
+ *         }
+ *         return super.visitMappingEntry(entry, ctx);
+ *     }
+ *
+ *     {@literal @}Override
+ *     public Yaml.Sequence visitSequence(Yaml.Sequence sequence, ExecutionContext ctx) {
+ *         // Extract values from sequence entries
+ *         for (Yaml.Sequence.Entry entry : sequence.getEntries()) {
+ *             String value = getSequenceEntryValue(entry);
+ *             if (value != null) {
+ *                 // Process each value
+ *             }
+ *         }
+ *         return super.visitSequence(sequence, ctx);
+ *     }
+ * }
+ * 
+ * + *

Migration from YamlHelper

+ *

This trait replaces the static {@code YamlHelper} utility class with a more + * composable and testable approach. To migrate existing code:

+ *
+ * // Before (using static utility):
+ * String value = YamlHelper.getScalarValue(block);
+ *
+ * // After (using trait):
+ * class MyVisitor extends YamlIsoVisitor<ExecutionContext>
+ *         implements YamlScalarAccessor {
+ *     void myMethod(Yaml.Block block) {
+ *         String value = getScalarValue(block);
+ *     }
+ * }
+ * 
+ * + * @see org.openrewrite.yaml.tree.Yaml.Scalar + * @see org.openrewrite.yaml.tree.Yaml.Block + */ +public interface YamlScalarAccessor { + + /** + * Safely extracts the string value from a YAML block if it's a scalar. + * + *

This method performs type checking and casting in a single operation, + * returning null if the block is not a Yaml.Scalar. This eliminates the need + * for explicit instanceof checks and casts throughout recipe code.

+ * + *

Example: + *

+     * Yaml.Block block = entry.getValue();
+     * String value = getScalarValue(block);
+     * if (value != null) {
+     *     // block was a Yaml.Scalar with a non-null value
+     * }
+     * 
+ *

+ * + * @param block The YAML block to extract value from, may be null + * @return The string value if block is a Yaml.Scalar, null otherwise + */ + default @Nullable String getScalarValue(Yaml.Block block) { + return block instanceof Yaml.Scalar ? ((Yaml.Scalar) block).getValue() : null; + } + + /** + * Safely extracts the string value from a YAML scalar. + * + *

This is a convenience method for when you already know the element is + * a scalar but want null-safe value extraction. Useful when working with + * APIs that return {@code Yaml.Scalar} directly.

+ * + *

Example: + *

+     * Yaml.Scalar scalar = (Yaml.Scalar) entry.getKey();
+     * String keyValue = getScalarValue(scalar);
+     * 
+ *

+ * + * @param scalar The YAML scalar to extract value from, may be null + * @return The string value if scalar is non-null, null otherwise + */ + default @Nullable String getScalarValue(Yaml.Scalar scalar) { + return scalar != null ? scalar.getValue() : null; + } + + /** + * Extracts scalar value from a mapping entry's value. + * + *

Common pattern when working with key-value pairs in YAML mappings. + * Equivalent to calling {@code getScalarValue(entry.getValue())} but more + * semantic and concise.

+ * + *

Example: + *

+     * for (Yaml.Mapping.Entry entry : mapping.getEntries()) {
+     *     String value = getEntryScalarValue(entry);
+     *     if (value != null) {
+     *         // Process entry's value
+     *     }
+     * }
+     * 
+ *

+ * + * @param entry The mapping entry to extract value from, may be null + * @return The string value if entry's value is a Yaml.Scalar, null otherwise + */ + default @Nullable String getEntryScalarValue(Yaml.Mapping.Entry entry) { + return entry != null ? getScalarValue(entry.getValue()) : null; + } + + /** + * Extracts scalar value from a sequence entry's block. + * + *

Common pattern when iterating over YAML sequences (arrays/lists). + * Equivalent to calling {@code getScalarValue(entry.getBlock())} but more + * semantic for sequence processing.

+ * + *

Example: + *

+     * Yaml.Sequence sequence = (Yaml.Sequence) onValue;
+     * for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) {
+     *     String trigger = getSequenceEntryValue(seqEntry);
+     *     if ("pull_request_target".equals(trigger)) {
+     *         // Found dangerous trigger
+     *     }
+     * }
+     * 
+ *

+ * + * @param entry The sequence entry to extract value from, may be null + * @return The string value if entry's block is a Yaml.Scalar, null otherwise + */ + default @Nullable String getSequenceEntryValue(Yaml.Sequence.Entry entry) { + return entry != null ? getScalarValue(entry.getBlock()) : null; + } + + /** + * Finds a scalar value for a given key in a YAML mapping. + * + *

This method searches through all entries in the mapping to find one + * with the specified key, then extracts its scalar value if present. Returns + * null if the key is not found or if the value is not a scalar.

+ * + *

Example: + *

+     * Yaml.Mapping jobMapping = (Yaml.Mapping) jobEntry.getValue();
+     * String runsOn = findScalarValueByKey(jobMapping, "runs-on");
+     * if ("self-hosted".equals(runsOn)) {
+     *     // Detected self-hosted runner
+     * }
+     * 
+ *

+ * + * @param mapping The YAML mapping to search in + * @param key The key to look for + * @return The scalar value if found, null otherwise + */ + default @Nullable String findScalarValueByKey(Yaml.Mapping mapping, String key) { + for (Yaml.Mapping.Entry entry : mapping.getEntries()) { + if (key.equals(entry.getKey().getValue())) { + return getScalarValue(entry.getValue()); + } + } + return null; + } + + /** + * Finds a nested scalar value by traversing through a parent key to a child key. + * + *

This method handles the common pattern of accessing nested YAML values like + * "with.path" by first finding the parent mapping, then searching for the child key. + * Returns null if either the parent key is not found, the parent value is not a + * mapping, the child key is not found, or the child value is not a scalar.

+ * + *

Example usage for GitHub Actions workflow: + *

+     * # YAML:
+     * - uses: actions/cache@v3
+     *   with:
+     *     path: ~/.cache
+     *     key: cache-key
+     *
+     * # Code:
+     * Yaml.Mapping stepMapping = (Yaml.Mapping) stepEntry.getBlock();
+     * String cachePath = findNestedScalarValue(stepMapping, "with", "path");
+     * // Returns: "~/.cache"
+     * 
+ *

+ * + * @param mapping The YAML mapping to search in + * @param parentKey The parent key (e.g., "with") + * @param childKey The child key (e.g., "path") + * @return The nested scalar value if found, null otherwise + */ + default @Nullable String findNestedScalarValue(Yaml.Mapping mapping, String parentKey, String childKey) { + for (Yaml.Mapping.Entry entry : mapping.getEntries()) { + if (parentKey.equals(entry.getKey().getValue()) && entry.getValue() instanceof Yaml.Mapping) { + Yaml.Mapping parentMapping = (Yaml.Mapping) entry.getValue(); + return findScalarValueByKey(parentMapping, childKey); + } + } + return null; + } +} diff --git a/src/main/java/org/openrewrite/github/traits/package-info.java b/src/main/java/org/openrewrite/github/traits/package-info.java new file mode 100644 index 0000000..3215a38 --- /dev/null +++ b/src/main/java/org/openrewrite/github/traits/package-info.java @@ -0,0 +1,61 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * OpenRewrite traits for GitHub Actions YAML processing. + * + *

This package contains reusable traits that provide common patterns for working with + * GitHub Actions workflow YAML files. Traits follow OpenRewrite best practices for + * composable, testable recipe development.

+ * + *

Available Traits

+ *
    + *
  • {@link org.openrewrite.github.traits.YamlScalarAccessor} - Safe extraction of + * scalar values from YAML LST elements
  • + *
+ * + *

Using Traits

+ *

Traits are designed to be implemented by recipe visitors to provide reusable + * functionality through default interface methods:

+ * + *
+ * private static class MyVisitor extends YamlIsoVisitor<ExecutionContext>
+ *         implements YamlScalarAccessor {
+ *
+ *     {@literal @}Override
+ *     public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) {
+ *         String value = getScalarValue(entry.getValue());
+ *         if (value != null) {
+ *             // Process the scalar value
+ *         }
+ *         return super.visitMappingEntry(entry, ctx);
+ *     }
+ * }
+ * 
+ * + *

Design Philosophy

+ *

Traits in this package follow these principles:

+ *
    + *
  • Composability - Multiple traits can be combined in a single visitor
  • + *
  • Java 8 Compatibility - All code uses Java 8 syntax only
  • + *
  • Null Safety - Methods handle null inputs gracefully using {@code @Nullable}
  • + *
  • Documentation - Comprehensive JavaDoc with real-world examples
  • + *
  • Testability - Traits can be tested independently
  • + *
+ * + * @see org.openrewrite.github.traits.YamlScalarAccessor + */ +package org.openrewrite.github.traits; From 797700b08a5aa85630c8085e2f2e0bc1a370b250 Mon Sep 17 00:00:00 2001 From: Scott Jungling Date: Wed, 15 Oct 2025 14:19:03 -0700 Subject: [PATCH 2/3] Rename traits package to util Renamed org.openrewrite.github.traits to org.openrewrite.github.util to better reflect that YamlScalarAccessor is a utility interface/mixin rather than an OpenRewrite trait (which should extend Trait). Changes: - Moved package from traits/ to util/ - Updated all imports in recipe files - Updated package-info documentation - All tests pass --- .../security/ArtifactSecurityRecipe.java | 2 +- .../github/security/CachePoisoningRecipe.java | 2 +- .../security/ExcessivePermissionsRecipe.java | 2 +- .../github/security/GitHubEnvRecipe.java | 2 +- .../security/SelfHostedRunnerRecipe.java | 2 +- .../security/TemplateInjectionRecipe.java | 2 +- .../security/UnpinnedDockerImagesRecipe.java | 2 +- .../{traits => util}/YamlScalarAccessor.java | 15 +++++------ .../github/{traits => util}/package-info.java | 26 +++++++++---------- 9 files changed, 27 insertions(+), 28 deletions(-) rename src/main/java/org/openrewrite/github/{traits => util}/YamlScalarAccessor.java (94%) rename src/main/java/org/openrewrite/github/{traits => util}/package-info.java (64%) diff --git a/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java b/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java index 5ff1a3d..ddb39e3 100644 --- a/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java +++ b/src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java @@ -19,7 +19,7 @@ import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.*; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.JsonPathMatcher; import org.openrewrite.yaml.YamlIsoVisitor; diff --git a/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java b/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java index 26c2293..451a7aa 100644 --- a/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java +++ b/src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java @@ -20,7 +20,7 @@ import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; diff --git a/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java b/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java index 6c10a96..07b6658 100644 --- a/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java +++ b/src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java @@ -18,7 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; diff --git a/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java b/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java index 4164e03..d8c46e0 100644 --- a/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java +++ b/src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java @@ -20,7 +20,7 @@ import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; diff --git a/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java b/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java index a979339..13116c4 100644 --- a/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java +++ b/src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java @@ -18,7 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; diff --git a/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java b/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java index ceee308..33b1a3e 100644 --- a/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java +++ b/src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java @@ -19,7 +19,7 @@ import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.*; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.JsonPathMatcher; import org.openrewrite.yaml.YamlIsoVisitor; diff --git a/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java b/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java index 9ade155..31bdb43 100644 --- a/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java +++ b/src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java @@ -18,7 +18,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; -import org.openrewrite.github.traits.YamlScalarAccessor; +import org.openrewrite.github.util.YamlScalarAccessor; import org.openrewrite.marker.SearchResult; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; diff --git a/src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java b/src/main/java/org/openrewrite/github/util/YamlScalarAccessor.java similarity index 94% rename from src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java rename to src/main/java/org/openrewrite/github/util/YamlScalarAccessor.java index a8c6f20..c4f0f28 100644 --- a/src/main/java/org/openrewrite/github/traits/YamlScalarAccessor.java +++ b/src/main/java/org/openrewrite/github/util/YamlScalarAccessor.java @@ -13,19 +13,18 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.github.traits; +package org.openrewrite.github.util; import org.jspecify.annotations.Nullable; import org.openrewrite.yaml.tree.Yaml; /** - * A trait that provides safe access to scalar values from YAML LST elements. - * This trait encapsulates common patterns for extracting string values from + * A utility interface that provides safe access to scalar values from YAML LST elements. + * This interface encapsulates common patterns for extracting string values from * YAML blocks, eliminating the need for repeated type checking and casting. * - *

This trait is designed to be mixed into recipes or visitors that need to - * work with YAML scalar values, providing a cleaner API than static utility methods. - * It follows OpenRewrite best practices for trait-based recipe development.

+ *

This interface is designed to be implemented by recipes or visitors that need to + * work with YAML scalar values, providing a cleaner API than static utility methods.

* *

Usage Example

*
@@ -57,13 +56,13 @@
  * 
* *

Migration from YamlHelper

- *

This trait replaces the static {@code YamlHelper} utility class with a more + *

This interface replaces the static {@code YamlHelper} utility class with a more * composable and testable approach. To migrate existing code:

*
  * // Before (using static utility):
  * String value = YamlHelper.getScalarValue(block);
  *
- * // After (using trait):
+ * // After (using interface):
  * class MyVisitor extends YamlIsoVisitor<ExecutionContext>
  *         implements YamlScalarAccessor {
  *     void myMethod(Yaml.Block block) {
diff --git a/src/main/java/org/openrewrite/github/traits/package-info.java b/src/main/java/org/openrewrite/github/util/package-info.java
similarity index 64%
rename from src/main/java/org/openrewrite/github/traits/package-info.java
rename to src/main/java/org/openrewrite/github/util/package-info.java
index 3215a38..b810fab 100644
--- a/src/main/java/org/openrewrite/github/traits/package-info.java
+++ b/src/main/java/org/openrewrite/github/util/package-info.java
@@ -15,20 +15,20 @@
  */
 
 /**
- * OpenRewrite traits for GitHub Actions YAML processing.
+ * Utility interfaces for GitHub Actions YAML processing.
  *
- * 

This package contains reusable traits that provide common patterns for working with - * GitHub Actions workflow YAML files. Traits follow OpenRewrite best practices for - * composable, testable recipe development.

+ *

This package contains reusable utility interfaces that provide common patterns for working with + * GitHub Actions workflow YAML files. These interfaces use default methods to provide mixins that + * can be implemented by recipe visitors.

* - *

Available Traits

+ *

Available Utilities

*
    - *
  • {@link org.openrewrite.github.traits.YamlScalarAccessor} - Safe extraction of + *
  • {@link org.openrewrite.github.util.YamlScalarAccessor} - Safe extraction of * scalar values from YAML LST elements
  • *
* - *

Using Traits

- *

Traits are designed to be implemented by recipe visitors to provide reusable + *

Using Utility Interfaces

+ *

These interfaces are designed to be implemented by recipe visitors to provide reusable * functionality through default interface methods:

* *
@@ -47,15 +47,15 @@
  * 
* *

Design Philosophy

- *

Traits in this package follow these principles:

+ *

Utility interfaces in this package follow these principles:

*
    - *
  • Composability - Multiple traits can be combined in a single visitor
  • + *
  • Composability - Multiple interfaces can be combined in a single visitor
  • *
  • Java 8 Compatibility - All code uses Java 8 syntax only
  • *
  • Null Safety - Methods handle null inputs gracefully using {@code @Nullable}
  • *
  • Documentation - Comprehensive JavaDoc with real-world examples
  • - *
  • Testability - Traits can be tested independently
  • + *
  • Testability - Utilities can be tested independently
  • *
* - * @see org.openrewrite.github.traits.YamlScalarAccessor + * @see org.openrewrite.github.util.YamlScalarAccessor */ -package org.openrewrite.github.traits; +package org.openrewrite.github.util; From 741211ce6e7c711b2b0165211e07ddafef83e330 Mon Sep 17 00:00:00 2001 From: Scott Jungling Date: Wed, 15 Oct 2025 16:45:30 -0700 Subject: [PATCH 3/3] refactor: explore usages of traits --- .../security/UnpinnedActionsRecipe.java | 88 ++--- .../openrewrite/github/util/ActionStep.java | 251 ++++++++++++++ .../github/util/YamlTraitMatcher.java | 89 +++++ .../github/util/ActionStepTest.java | 313 ++++++++++++++++++ 4 files changed, 685 insertions(+), 56 deletions(-) create mode 100644 src/main/java/org/openrewrite/github/util/ActionStep.java create mode 100644 src/main/java/org/openrewrite/github/util/YamlTraitMatcher.java create mode 100644 src/test/java/org/openrewrite/github/util/ActionStepTest.java diff --git a/src/main/java/org/openrewrite/github/security/UnpinnedActionsRecipe.java b/src/main/java/org/openrewrite/github/security/UnpinnedActionsRecipe.java index e549475..0cf6a8a 100644 --- a/src/main/java/org/openrewrite/github/security/UnpinnedActionsRecipe.java +++ b/src/main/java/org/openrewrite/github/security/UnpinnedActionsRecipe.java @@ -18,9 +18,8 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; +import org.openrewrite.github.util.ActionStep; import org.openrewrite.marker.SearchResult; -import org.openrewrite.yaml.YamlIsoVisitor; -import org.openrewrite.yaml.tree.Yaml; import java.util.regex.Pattern; @@ -32,8 +31,6 @@ public class UnpinnedActionsRecipe extends Recipe { "^([^/@]+/[^/@]+)(@(main|master|HEAD|latest|v?\\d+(\\.\\d+)*(\\.\\d+)*))??$" ); - private static final Pattern SHA_PATTERN = Pattern.compile("^[a-f0-9]{40}$"); - @Override public String getDisplayName() { return "Pin GitHub Actions to specific commits"; @@ -51,68 +48,47 @@ public String getDescription() { public TreeVisitor getVisitor() { return Preconditions.check( new FindSourceFiles(".github/workflows/*.yml"), - new UnpinnedActionsVisitor() + new ActionStep.Matcher().asVisitor((actionStep, ctx) -> { + if (isUnpinned(actionStep)) { + String actionRef = actionStep.getActionRef(); + return SearchResult.found(actionStep.getTree(), + "Action '" + actionRef + "' is not pinned to a commit SHA. " + + "Consider pinning to a specific commit for security and reproducibility."); + } + return actionStep.getTree(); + }) ); } - private static class UnpinnedActionsVisitor extends YamlIsoVisitor { - - @Override - public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { - Yaml.Mapping.Entry mappingEntry = super.visitMappingEntry(entry, ctx); - - if (isUsesEntry(mappingEntry)) { - String usesValue = getUsesValue(mappingEntry); - if (usesValue != null && isUnpinned(usesValue)) { - return SearchResult.found(mappingEntry, - "Action '" + usesValue + "' is not pinned to a commit SHA. " + - "Consider pinning to a specific commit for security and reproducibility."); - } - } - - return mappingEntry; + private static boolean isUnpinned(ActionStep actionStep) { + String actionRef = actionStep.getActionRef(); + if (actionRef == null) { + return false; } - private boolean isUsesEntry(Yaml.Mapping.Entry entry) { - // Broader approach - match any "uses" entry and let the logic handle context validation - return entry.getKey() instanceof Yaml.Scalar && - "uses".equals(((Yaml.Scalar) entry.getKey()).getValue()); + // Skip local actions (start with ./) + if (actionRef.startsWith("./")) { + return false; } - private String getUsesValue(Yaml.Mapping.Entry entry) { - if (entry.getValue() instanceof Yaml.Scalar) { - return ((Yaml.Scalar) entry.getValue()).getValue(); - } - return null; + // Skip Docker actions (start with docker://) + if (actionRef.startsWith("docker://")) { + return false; } - private boolean isUnpinned(String usesValue) { - // Skip local actions (start with ./) - if (usesValue.startsWith("./")) { - return false; - } - - // Skip Docker actions (start with docker://) - if (usesValue.startsWith("docker://")) { - return false; - } - - // Check if it's a repository action - String[] parts = usesValue.split("@", 2); - if (parts.length < 2) { - // No @ symbol means no version specified at all - return true; - } - - String version = parts[1]; - - // If it's already a SHA, it's pinned - if (SHA_PATTERN.matcher(version).matches()) { - return false; - } + // If it's already pinned to a SHA, it's not unpinned + if (actionStep.isVersionPinned()) { + return false; + } - // If it matches unpinned patterns (main, master, HEAD, latest, or version tags), it's unpinned - return UNPINNED_ACTION_PATTERN.matcher(usesValue).matches(); + // Check if version exists + String version = actionStep.getActionVersion(); + if (version == null) { + // No @ symbol means no version specified at all + return true; } + + // If it matches unpinned patterns (main, master, HEAD, latest, or version tags), it's unpinned + return UNPINNED_ACTION_PATTERN.matcher(actionRef).matches(); } } diff --git a/src/main/java/org/openrewrite/github/util/ActionStep.java b/src/main/java/org/openrewrite/github/util/ActionStep.java new file mode 100644 index 0000000..f63bdeb --- /dev/null +++ b/src/main/java/org/openrewrite/github/util/ActionStep.java @@ -0,0 +1,251 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.github.util; + +import lombok.AllArgsConstructor; +import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.trait.Trait; +import org.openrewrite.trait.VisitFunction2; +import org.openrewrite.yaml.YamlIsoVisitor; +import org.openrewrite.yaml.tree.Yaml; + +/** + * Represents a GitHub Actions workflow step that uses an external action (has a `uses` key). + *

+ * This trait provides read-only semantic methods for querying action references, + * versions, and metadata without manual cursor navigation or instanceof checks. + *

+ * Design Principle: This trait is designed for querying and matching only. + * For tree modifications, use the trait to identify targets, then build updated trees + * directly in your visitor and return them. + *

+ * Example workflow step: + *

+ * - uses: actions/checkout@v3
+ * - uses: actions/setup-java@a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0
+ * 
+ *

+ * Example usage: + *

+ * new ActionStep.Matcher()
+ *     .withRequiredAction("actions/checkout")
+ *     .asVisitor((actionStep, ctx) -> {
+ *         if (actionStep.isVersionPinned()) {
+ *             // Read-only operations
+ *             String owner = actionStep.getActionOwner();
+ *             String version = actionStep.getActionVersion();
+ *         }
+ *         return actionStep.getTree();
+ *     })
+ * 
+ */ +@AllArgsConstructor +public class ActionStep implements Trait { + Cursor cursor; + + @Override + public Cursor getCursor() { + return cursor; + } + + /** + * Get the full action reference (e.g., "actions/checkout@v3"). + * + * @return The full action reference, or null if not available + */ + public @Nullable String getActionRef() { + Yaml.Mapping.Entry entry = getTree(); + if (entry.getValue() instanceof Yaml.Scalar) { + return ((Yaml.Scalar) entry.getValue()).getValue(); + } + return null; + } + + /** + * Get the action owner (e.g., "actions" from "actions/checkout@v3"). + * + * @return The action owner, or null if not available or for local/docker actions + */ + public @Nullable String getActionOwner() { + String ref = getActionRef(); + if (ref == null || !ref.contains("/")) { + return null; + } + // Skip local and docker actions + if (ref.startsWith("./") || ref.startsWith("docker://")) { + return null; + } + String nameWithOwner = ref.contains("@") ? ref.substring(0, ref.indexOf("@")) : ref; + return nameWithOwner.substring(0, nameWithOwner.indexOf("/")); + } + + /** + * Get the action name without version (e.g., "actions/checkout" from "actions/checkout@v3"). + * + * @return The action name, or null if not available + */ + public @Nullable String getActionName() { + String ref = getActionRef(); + if (ref == null) { + return null; + } + return ref.contains("@") ? ref.substring(0, ref.indexOf("@")) : ref; + } + + /** + * Get the action version (e.g., "v3" from "actions/checkout@v3"). + * + * @return The action version, or null if not specified + */ + public @Nullable String getActionVersion() { + String ref = getActionRef(); + if (ref == null || !ref.contains("@")) { + return null; + } + return ref.substring(ref.indexOf("@") + 1); + } + + /** + * Check if the action is pinned to a full 40-character SHA. + * + * @return true if pinned to a SHA, false otherwise + */ + public boolean isVersionPinned() { + String version = getActionVersion(); + return version != null && version.matches("[a-f0-9]{40}"); + } + + /** + * Check if this action matches a given action pattern. + *

+ * Supports patterns like: + *

    + *
  • "actions/checkout" - exact match
  • + *
  • "actions/checkout@*" - match any version
  • + *
+ * + * @param actionPattern The pattern to match + * @return true if matches, false otherwise + */ + public boolean matchesAction(String actionPattern) { + String name = getActionName(); + if (name == null) { + return false; + } + if (actionPattern.endsWith("@*")) { + return name.equals(actionPattern.substring(0, actionPattern.length() - 2)); + } + return name.equals(actionPattern); + } + + /** + * Check if a cursor points to an action step (a mapping entry with key "uses"). + * + * @param cursor The cursor to check + * @return true if it's an action step, false otherwise + */ + public static boolean isActionStep(Cursor cursor) { + if (!(cursor.getValue() instanceof Yaml.Mapping.Entry)) { + return false; + } + Yaml.Mapping.Entry entry = cursor.getValue(); + return entry.getKey() instanceof Yaml.Scalar && + "uses".equals(((Yaml.Scalar) entry.getKey()).getValue()); + } + + /** + * Check if a cursor is within a steps array of a job. + * + * @param cursor The cursor to check + * @return true if within a steps array, false otherwise + */ + public static boolean withinStepsArray(Cursor cursor) { + Cursor parent = cursor.getParent(); + while (parent != null) { + if (parent.getValue() instanceof Yaml.Mapping.Entry) { + Yaml.Mapping.Entry entry = parent.getValue(); + if (entry.getKey() instanceof Yaml.Scalar && + "steps".equals(((Yaml.Scalar) entry.getKey()).getValue())) { + return true; + } + } + parent = parent.getParent(); + } + return false; + } + + /** + * Matcher for finding ActionStep traits in YAML documents. + */ + public static class Matcher extends YamlTraitMatcher { + @Nullable + protected String requiredAction; + + /** + * Filter to match only a specific action. + * + * @param action The action name to match (e.g., "actions/checkout") + * @return This matcher for chaining + */ + public Matcher withRequiredAction(String action) { + this.requiredAction = action; + return this; + } + + @Override + protected @Nullable ActionStep test(Cursor cursor) { + // Must be a mapping entry with key "uses" + if (!isActionStep(cursor)) { + return null; + } + + // Must be within a steps array + if (!withinStepsArray(cursor)) { + return null; + } + + // Create trait instance + ActionStep trait = new ActionStep(cursor); + + // Apply filter if configured + if (requiredAction != null && !trait.matchesAction(requiredAction)) { + return null; + } + + return trait; + } + + /** + * Override asVisitor for better performance - only visit mapping entries. + */ + @Override + public

TreeVisitor asVisitor(VisitFunction2 visitor) { + return new YamlIsoVisitor

() { + @Override + public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, P p) { + ActionStep actionStep = test(getCursor()); + if (actionStep != null) { + return (Yaml.Mapping.Entry) visitor.visit(actionStep, p); + } + return super.visitMappingEntry(entry, p); + } + }; + } + } +} diff --git a/src/main/java/org/openrewrite/github/util/YamlTraitMatcher.java b/src/main/java/org/openrewrite/github/util/YamlTraitMatcher.java new file mode 100644 index 0000000..7926b8a --- /dev/null +++ b/src/main/java/org/openrewrite/github/util/YamlTraitMatcher.java @@ -0,0 +1,89 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.github.util; + +import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; +import org.openrewrite.SourceFile; +import org.openrewrite.trait.SimpleTraitMatcher; +import org.openrewrite.trait.Trait; +import org.openrewrite.yaml.tree.Yaml; + +/** + * Base matcher for YAML traits in GitHub Actions workflows. + * Provides common utility methods for cursor navigation and value extraction. + * + * @param The trait type being matched + */ +public abstract class YamlTraitMatcher> extends SimpleTraitMatcher { + + /** + * Check if the cursor is within a YAML mapping. + * + * @param cursor The cursor to check + * @return true if within a mapping, false otherwise + */ + protected boolean withinMapping(Cursor cursor) { + return cursor.firstEnclosing(Yaml.Mapping.class) != null; + } + + /** + * Validate that the cursor is in a valid YAML context. + * + * @param cursor The cursor to validate + * @return true if in a valid YAML document, false otherwise + */ + protected boolean isValidYamlContext(Cursor cursor) { + SourceFile sourceFile = cursor.firstEnclosing(SourceFile.class); + return sourceFile instanceof Yaml.Documents; + } + + /** + * Safely extract a scalar value from a YAML block. + * + * @param block The YAML block to extract from + * @return The scalar value, or null if not a scalar + */ + protected @Nullable String getScalarValue(Yaml.Block block) { + return block instanceof Yaml.Scalar ? ((Yaml.Scalar) block).getValue() : null; + } + + /** + * Get the scalar key from a mapping entry. + * + * @param entry The mapping entry + * @return The key as a string, or null if not a scalar key + */ + protected @Nullable String getScalarKey(Yaml.Mapping.Entry entry) { + return entry.getKey() instanceof Yaml.Scalar ? ((Yaml.Scalar) entry.getKey()).getValue() : null; + } + + /** + * Find a scalar value by key in a mapping. + * + * @param mapping The mapping to search + * @param key The key to find + * @return The scalar value, or null if not found or not a scalar + */ + protected @Nullable String findScalarValueByKey(Yaml.Mapping mapping, String key) { + for (Yaml.Mapping.Entry entry : mapping.getEntries()) { + if (key.equals(getScalarKey(entry))) { + return getScalarValue(entry.getValue()); + } + } + return null; + } +} diff --git a/src/test/java/org/openrewrite/github/util/ActionStepTest.java b/src/test/java/org/openrewrite/github/util/ActionStepTest.java new file mode 100644 index 0000000..1814c95 --- /dev/null +++ b/src/test/java/org/openrewrite/github/util/ActionStepTest.java @@ -0,0 +1,313 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.github.util; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.ExecutionContext; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; +import org.openrewrite.yaml.YamlIsoVisitor; +import org.openrewrite.yaml.tree.Yaml; + +import java.util.List; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.openrewrite.yaml.Assertions.yaml; + +class ActionStepTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(RewriteTest.toRecipe(() -> new YamlIsoVisitor<>() { + // No-op visitor - we're testing trait matching, not transformation + })); + } + + @DocumentExample + @Test + void findsActionSteps() { + rewriteRun( + yaml( + """ + name: Test Workflow + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-java@v2 + with: + distribution: 'temurin' + - run: echo "Not an action step" + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + assertThat(steps).hasSize(2); + assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout"); + assertThat(steps.get(0).getActionVersion()).isEqualTo("v3"); + assertThat(steps.get(1).getActionName()).isEqualTo("actions/setup-java"); + }) + ) + ); + } + + @Test + void matcherWithFilterFindsSpecificAction() { + rewriteRun( + yaml( + """ + jobs: + test: + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-java@v2 + - uses: gradle/gradle-build-action@v2 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher() + .withRequiredAction("actions/setup-java"); + + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + assertThat(steps).hasSize(1); + assertThat(steps.get(0).getActionName()).isEqualTo("actions/setup-java"); + }) + ) + ); + } + + @Test + void doesNotMatchRunSteps() { + rewriteRun( + yaml( + """ + jobs: + test: + steps: + - run: echo "hello" + - name: Build + run: ./gradlew build + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + assertThat(steps).isEmpty(); + }) + ) + ); + } + + @Test + void extractsActionNameAndVersion() { + rewriteRun( + yaml( + """ + jobs: + build: + steps: + - uses: actions/checkout@v3 + - uses: octocat/hello-world-action@main + - uses: actions/setup-java@a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + // Test semantic version + assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout"); + assertThat(steps.get(0).getActionVersion()).isEqualTo("v3"); + assertThat(steps.get(0).getActionOwner()).isEqualTo("actions"); + assertThat(steps.get(0).isVersionPinned()).isFalse(); + + // Test branch reference + assertThat(steps.get(1).getActionName()).isEqualTo("octocat/hello-world-action"); + assertThat(steps.get(1).getActionVersion()).isEqualTo("main"); + assertThat(steps.get(1).isVersionPinned()).isFalse(); + + // Test SHA pinning + assertThat(steps.get(2).getActionName()).isEqualTo("actions/setup-java"); + assertThat(steps.get(2).isVersionPinned()).isTrue(); + }) + ) + ); + } + + @Test + void matchesActionPatterns() { + rewriteRun( + yaml( + """ + jobs: + build: + steps: + - uses: actions/setup-java@v2 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + ActionStep step = matcher.lower(results) + .findFirst() + .orElseThrow(); + + assertThat(step.matchesAction("actions/setup-java")).isTrue(); + assertThat(step.matchesAction("actions/setup-java@*")).isTrue(); + assertThat(step.matchesAction("actions/setup-node")).isFalse(); + }) + ) + ); + } + + @Test + void handlesActionWithoutVersion() { + rewriteRun( + yaml( + """ + jobs: + build: + steps: + - uses: actions/checkout + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + assertThat(steps).hasSize(1); + assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout"); + assertThat(steps.get(0).getActionVersion()).isNull(); + assertThat(steps.get(0).isVersionPinned()).isFalse(); + }) + ) + ); + } + + @Test + void doesNotMatchUsesOutsideSteps() { + rewriteRun( + yaml( + """ + jobs: + call-workflow: + uses: ./.github/workflows/reusable.yml + build: + steps: + - uses: actions/checkout@v3 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + // Should only find the step-level uses, not the job-level uses + assertThat(steps).hasSize(1); + assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout"); + }) + ) + ); + } + + @Test + void handlesMultipleJobsAndSteps() { + rewriteRun( + yaml( + """ + jobs: + build: + steps: + - uses: actions/checkout@v3 + - run: echo "build" + test: + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-java@v2 + deploy: + steps: + - uses: actions/checkout@v3 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + assertThat(steps).hasSize(4); + // Three checkouts and one setup-java + long checkouts = steps.stream() + .filter(s -> "actions/checkout".equals(s.getActionName())) + .count(); + assertThat(checkouts).isEqualTo(3); + }) + ) + ); + } + + @Test + void handlesLocalActions() { + rewriteRun( + yaml( + """ + jobs: + build: + steps: + - uses: ./local-action + - uses: actions/checkout@v3 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + // Both should match - local actions are still action steps + assertThat(steps).hasSize(2); + assertThat(steps.get(0).getActionRef()).isEqualTo("./local-action"); + assertThat(steps.get(0).getActionName()).isEqualTo("./local-action"); + assertThat(steps.get(0).getActionOwner()).isNull(); // No owner for local actions + }) + ) + ); + } + + @Test + void handlesDockerActions() { + rewriteRun( + yaml( + """ + jobs: + build: + steps: + - uses: docker://alpine:3.8 + - uses: actions/checkout@v3 + """, + spec -> spec.afterRecipe(results -> { + ActionStep.Matcher matcher = new ActionStep.Matcher(); + List steps = matcher.lower(results) + .collect(Collectors.toList()); + + assertThat(steps).hasSize(2); + assertThat(steps.get(0).getActionRef()).isEqualTo("docker://alpine:3.8"); + }) + ) + ); + } +}