Skip to content

Commit 47ff257

Browse files
sjunglingsjungling
committed
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
1 parent 52cd6c7 commit 47ff257

File tree

10 files changed

+338
-117
lines changed

10 files changed

+338
-117
lines changed

‎src/main/java/org/openrewrite/github/security/ArtifactSecurityRecipe.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import lombok.Value;
2020
import org.jspecify.annotations.Nullable;
2121
import org.openrewrite.*;
22+
import org.openrewrite.github.traits.YamlScalarAccessor;
2223
import org.openrewrite.marker.SearchResult;
2324
import org.openrewrite.yaml.JsonPathMatcher;
2425
import org.openrewrite.yaml.YamlIsoVisitor;
@@ -78,7 +79,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
7879
);
7980
}
8081

81-
private static class ArtifactSecurityVisitor extends YamlIsoVisitor<ExecutionContext> {
82+
private static class ArtifactSecurityVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
8283

8384
private static final JsonPathMatcher STEP_USES_MATCHER = new JsonPathMatcher("$..steps[*].uses");
8485

@@ -108,7 +109,7 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC
108109

109110

110111
private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) {
111-
String usesValue = YamlHelper.getScalarValue(entry.getValue());
112+
String usesValue = getScalarValue(entry.getValue());
112113
if (usesValue == null) {
113114
return entry;
114115
}
@@ -133,7 +134,7 @@ private Yaml.Mapping.Entry checkCheckoutAction(Yaml.Mapping.Entry entry) {
133134
return entry;
134135
}
135136

136-
String persistCredentials = YamlHelper.findNestedScalarValue(stepMapping, "with", "persist-credentials");
137+
String persistCredentials = findNestedScalarValue(stepMapping, "with", "persist-credentials");
137138

138139
if (persistCredentials == null) {
139140
// 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) {
159160
return entry;
160161
}
161162

162-
String pathValue = YamlHelper.findNestedScalarValue(stepMapping, "with", "path");
163+
String pathValue = findNestedScalarValue(stepMapping, "with", "path");
163164
if (pathValue != null && hasDangerousArtifactPaths(pathValue)) {
164165
return SearchResult.found(entry,
165166
"Uploading potentially sensitive paths that may contain credentials or configuration files.");

‎src/main/java/org/openrewrite/github/security/CachePoisoningRecipe.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.openrewrite.ExecutionContext;
2121
import org.openrewrite.Recipe;
2222
import org.openrewrite.TreeVisitor;
23+
import org.openrewrite.github.traits.YamlScalarAccessor;
2324
import org.openrewrite.marker.SearchResult;
2425
import org.openrewrite.yaml.YamlIsoVisitor;
2526
import org.openrewrite.yaml.tree.Yaml;
@@ -90,7 +91,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
9091
return new CachePoisoningVisitor();
9192
}
9293

93-
private static class CachePoisoningVisitor extends YamlIsoVisitor<ExecutionContext> {
94+
private static class CachePoisoningVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
9495

9596
private boolean isPublishingWorkflow = false;
9697
private boolean hasPublisherAction = false;
@@ -131,14 +132,14 @@ private void analyzeWorkflow(Yaml.Document document) {
131132
}
132133

133134
private boolean isPublishingTrigger(Yaml.Block onValue) {
134-
String scalarTrigger = YamlHelper.getScalarValue(onValue);
135+
String scalarTrigger = getScalarValue(onValue);
135136
if (scalarTrigger != null) {
136137
return "release".equals(scalarTrigger);
137138
}
138139
if (onValue instanceof Yaml.Sequence) {
139140
Yaml.Sequence sequence = (Yaml.Sequence) onValue;
140141
for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) {
141-
String trigger = YamlHelper.getScalarValue(seqEntry.getBlock());
142+
String trigger = getScalarValue(seqEntry.getBlock());
142143
if ("release".equals(trigger)) {
143144
return true;
144145
}
@@ -184,7 +185,7 @@ private boolean hasReleaseBranches(Yaml.Block branchesValue) {
184185
if (branchesValue instanceof Yaml.Sequence) {
185186
Yaml.Sequence sequence = (Yaml.Sequence) branchesValue;
186187
for (Yaml.Sequence.Entry entry : sequence.getEntries()) {
187-
String branch = YamlHelper.getScalarValue(entry.getBlock());
188+
String branch = getScalarValue(entry.getBlock());
188189
if (branch != null && RELEASE_BRANCH_PATTERN.matcher(branch).matches()) {
189190
return true;
190191
}
@@ -230,7 +231,7 @@ private boolean jobHasPublisherAction(Yaml.Mapping jobMapping) {
230231
private boolean stepUsesPublisherAction(Yaml.Mapping stepMapping) {
231232
for (Yaml.Mapping.Entry entry : stepMapping.getEntries()) {
232233
if ("uses".equals(entry.getKey().getValue())) {
233-
String uses = YamlHelper.getScalarValue(entry.getValue());
234+
String uses = getScalarValue(entry.getValue());
234235
if (uses != null) {
235236
String actionName = extractActionName(uses);
236237
return PUBLISHER_ACTIONS.contains(actionName);
@@ -262,7 +263,7 @@ private boolean isCacheAwareActionStep(Yaml.Mapping.Entry entry) {
262263
return false;
263264
}
264265

265-
String uses = YamlHelper.getScalarValue(entry.getValue());
266+
String uses = getScalarValue(entry.getValue());
266267
if (uses == null) {
267268
return false;
268269
}
@@ -272,7 +273,7 @@ private boolean isCacheAwareActionStep(Yaml.Mapping.Entry entry) {
272273
}
273274

274275
private String getActionName(Yaml.Mapping.Entry entry) {
275-
String uses = YamlHelper.getScalarValue(entry.getValue());
276+
String uses = getScalarValue(entry.getValue());
276277
return uses != null ? extractActionName(uses) : "unknown";
277278
}
278279

‎src/main/java/org/openrewrite/github/security/ExcessivePermissionsRecipe.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import lombok.EqualsAndHashCode;
1919
import lombok.Value;
2020
import org.openrewrite.*;
21+
import org.openrewrite.github.traits.YamlScalarAccessor;
2122
import org.openrewrite.marker.SearchResult;
2223
import org.openrewrite.yaml.YamlIsoVisitor;
2324
import org.openrewrite.yaml.tree.Yaml;
@@ -58,7 +59,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
5859
);
5960
}
6061

61-
private static class ExcessivePermissionsVisitor extends YamlIsoVisitor<ExecutionContext> {
62+
private static class ExcessivePermissionsVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
6263

6364
@Override
6465
public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) {
@@ -76,7 +77,7 @@ private boolean isPermissionsEntry(Yaml.Mapping.Entry entry) {
7677
}
7778

7879
private Yaml.Mapping.Entry checkPermissions(Yaml.Mapping.Entry entry) {
79-
String scalarPermissionValue = YamlHelper.getScalarValue(entry.getValue());
80+
String scalarPermissionValue = getScalarValue(entry.getValue());
8081
if (scalarPermissionValue != null) {
8182
return checkScalarPermissions(entry, scalarPermissionValue);
8283
}
@@ -107,7 +108,7 @@ private Yaml.Mapping.Entry checkMappingPermissions(Yaml.Mapping.Entry entry, Yam
107108

108109
for (Yaml.Mapping.Entry permEntry : permissionsMapping.getEntries()) {
109110
String permissionName = permEntry.getKey().getValue();
110-
String permissionValue = YamlHelper.getScalarValue(permEntry.getValue());
111+
String permissionValue = getScalarValue(permEntry.getValue());
111112
if (permissionName != null && permissionValue != null) {
112113

113114
if ("write".equals(permissionValue)) {

‎src/main/java/org/openrewrite/github/security/GitHubEnvRecipe.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.openrewrite.ExecutionContext;
2121
import org.openrewrite.Recipe;
2222
import org.openrewrite.TreeVisitor;
23+
import org.openrewrite.github.traits.YamlScalarAccessor;
2324
import org.openrewrite.marker.SearchResult;
2425
import org.openrewrite.yaml.YamlIsoVisitor;
2526
import org.openrewrite.yaml.tree.Yaml;
@@ -75,7 +76,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
7576
return new GitHubEnvVisitor();
7677
}
7778

78-
private static class GitHubEnvVisitor extends YamlIsoVisitor<ExecutionContext> {
79+
private static class GitHubEnvVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
7980

8081
private boolean hasDangerousTriggers = false;
8182

@@ -109,14 +110,14 @@ private void analyzeTriggers(Yaml.Document document) {
109110
}
110111

111112
private boolean checkForDangerousTriggers(Yaml.Block onValue) {
112-
String scalarTrigger = YamlHelper.getScalarValue(onValue);
113+
String scalarTrigger = getScalarValue(onValue);
113114
if (scalarTrigger != null) {
114115
return DANGEROUS_TRIGGERS.contains(scalarTrigger);
115116
}
116117
if (onValue instanceof Yaml.Sequence) {
117118
Yaml.Sequence sequence = (Yaml.Sequence) onValue;
118119
for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) {
119-
String trigger = YamlHelper.getScalarValue(seqEntry.getBlock());
120+
String trigger = getScalarValue(seqEntry.getBlock());
120121
if (trigger != null && DANGEROUS_TRIGGERS.contains(trigger)) {
121122
return true;
122123
}
@@ -163,7 +164,7 @@ private boolean isRunStepEntry(Yaml.Mapping.Entry entry) {
163164
}
164165

165166
private String getRunContent(Yaml.Mapping.Entry entry) {
166-
return YamlHelper.getScalarValue(entry.getValue());
167+
return getScalarValue(entry.getValue());
167168
}
168169

169170
private boolean usesGitHubEnv(String runContent) {

‎src/main/java/org/openrewrite/github/security/SelfHostedRunnerRecipe.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import lombok.EqualsAndHashCode;
1919
import lombok.Value;
2020
import org.openrewrite.*;
21+
import org.openrewrite.github.traits.YamlScalarAccessor;
2122
import org.openrewrite.marker.SearchResult;
2223
import org.openrewrite.yaml.YamlIsoVisitor;
2324
import org.openrewrite.yaml.tree.Yaml;
@@ -47,7 +48,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
4748
);
4849
}
4950

50-
private static class SelfHostedRunnerVisitor extends YamlIsoVisitor<ExecutionContext> {
51+
private static class SelfHostedRunnerVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
5152

5253
@Override
5354
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
8788
}
8889

8990
private Yaml.Mapping.Entry checkRunsOnSequence(Yaml.Mapping.Entry entry, Yaml.Sequence sequence) {
90-
String firstValue = YamlHelper.getScalarValue(sequence.getEntries().get(0).getBlock());
91+
String firstValue = getScalarValue(sequence.getEntries().get(0).getBlock());
9192
if ("self-hosted".equals(firstValue)) {
9293
return SearchResult.found(entry,
9394
"Uses self-hosted runner which may have security implications in public repositories. " +
@@ -137,7 +138,7 @@ private boolean containsSelfHostedInMatrixValues(Yaml.Mapping matrixMapping) {
137138
if (matrixEntry.getValue() instanceof Yaml.Sequence) {
138139
Yaml.Sequence sequence = (Yaml.Sequence) matrixEntry.getValue();
139140
for (Yaml.Sequence.Entry seqEntry : sequence.getEntries()) {
140-
String value = YamlHelper.getScalarValue(seqEntry.getBlock());
141+
String value = getScalarValue(seqEntry.getBlock());
141142
if ("self-hosted".equals(value)) {
142143
return true;
143144
}

‎src/main/java/org/openrewrite/github/security/TemplateInjectionRecipe.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import lombok.Value;
2020
import org.jspecify.annotations.Nullable;
2121
import org.openrewrite.*;
22+
import org.openrewrite.github.traits.YamlScalarAccessor;
2223
import org.openrewrite.marker.SearchResult;
2324
import org.openrewrite.yaml.JsonPathMatcher;
2425
import org.openrewrite.yaml.YamlIsoVisitor;
@@ -93,7 +94,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
9394
);
9495
}
9596

96-
private static class TemplateInjectionVisitor extends YamlIsoVisitor<ExecutionContext> {
97+
private static class TemplateInjectionVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
9798

9899
private static final JsonPathMatcher STEP_RUN_MATCHER = new JsonPathMatcher("$..steps[*].run");
99100
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
122123
}
123124

124125
private Yaml.Mapping.Entry checkRunEntry(Yaml.Mapping.Entry entry) {
125-
String runCommand = YamlHelper.getScalarValue(entry.getValue());
126+
String runCommand = getScalarValue(entry.getValue());
126127
if (runCommand == null) {
127128
return entry;
128129
}
@@ -143,7 +144,7 @@ private Yaml.Mapping.Entry checkRunEntry(Yaml.Mapping.Entry entry) {
143144
}
144145

145146
private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) {
146-
String usesValue = YamlHelper.getScalarValue(entry.getValue());
147+
String usesValue = getScalarValue(entry.getValue());
147148
if (usesValue == null) {
148149
return entry;
149150
}
@@ -161,7 +162,7 @@ private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) {
161162
}
162163

163164
private Yaml.Mapping.Entry checkScriptEntry(Yaml.Mapping.Entry entry) {
164-
String scriptContent = YamlHelper.getScalarValue(entry.getValue());
165+
String scriptContent = getScalarValue(entry.getValue());
165166
if (scriptContent == null) {
166167
return entry;
167168
}

‎src/main/java/org/openrewrite/github/security/UnpinnedDockerImagesRecipe.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import lombok.EqualsAndHashCode;
1919
import lombok.Value;
2020
import org.openrewrite.*;
21+
import org.openrewrite.github.traits.YamlScalarAccessor;
2122
import org.openrewrite.marker.SearchResult;
2223
import org.openrewrite.yaml.YamlIsoVisitor;
2324
import org.openrewrite.yaml.tree.Yaml;
@@ -55,7 +56,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
5556
);
5657
}
5758

58-
private static class UnpinnedDockerImagesVisitor extends YamlIsoVisitor<ExecutionContext> {
59+
private static class UnpinnedDockerImagesVisitor extends YamlIsoVisitor<ExecutionContext> implements YamlScalarAccessor {
5960

6061
@Override
6162
public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) {
@@ -78,7 +79,7 @@ private boolean isImageEntry(Yaml.Mapping.Entry entry) {
7879
}
7980

8081
private String getImageValue(Yaml.Mapping.Entry entry) {
81-
return YamlHelper.getScalarValue(entry.getValue());
82+
return getScalarValue(entry.getValue());
8283
}
8384

8485
private boolean isUnpinnedDockerImage(String imageValue) {

‎src/main/java/org/openrewrite/github/security/YamlHelper.java‎

Lines changed: 0 additions & 90 deletions
This file was deleted.

0 commit comments

Comments
 (0)