Skip to content

Commit 8e979de

Browse files
authored
Provided semgrep scan detection tool metadata (#344)
Populate detectionTool metadata for Semgrep codemods (`@ProvidedSemgrepScan`) Issue #314
1 parent 860a7a8 commit 8e979de

File tree

10 files changed

+138
-13
lines changed

10 files changed

+138
-13
lines changed

core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.ArrayList;
1414
import java.util.List;
1515
import java.util.Objects;
16+
import java.util.Optional;
1617
import javax.inject.Inject;
1718

1819
/** This codemod knows how to translate */
@@ -46,6 +47,11 @@ public DetectorRule getDetectorRule() {
4647
"https://semgrep.dev/r?q=java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli");
4748
}
4849

50+
@Override
51+
public Optional<FixedFinding> getFixedFinding(String id) {
52+
return Optional.of(new FixedFinding(id, getDetectorRule()));
53+
}
54+
4955
@Override
5056
public CodemodFileScanningResult visit(
5157
final CodemodInvocationContext context, final CompilationUnit cu) {
@@ -103,8 +109,7 @@ public CodemodFileScanningResult visit(
103109

104110
MethodCallExpr methodCallExpr = supportedSqlMethodCallsOnThatLine.get(0);
105111
if (SQLParameterizerWithCleanup.checkAndFix(methodCallExpr)) {
106-
FixedFinding fixedFinding = new FixedFinding(id, getDetectorRule());
107-
changes.add(CodemodChange.from(line, fixedFinding));
112+
changes.add(CodemodChange.from(line, getFixedFinding(id).get()));
108113
} else {
109114
UnfixedFinding unfixableFinding =
110115
new UnfixedFinding(

core-codemods/src/main/java/io/codemodder/codemods/SemgrepOverlyPermissiveFilePermissionsCodemod.java

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
import com.github.javaparser.ast.expr.StringLiteralExpr;
1212
import com.github.javaparser.ast.stmt.ExpressionStmt;
1313
import io.codemodder.*;
14+
import io.codemodder.codetf.DetectorRule;
1415
import io.codemodder.javaparser.ChangesResult;
1516
import io.codemodder.providers.sarif.semgrep.ProvidedSemgrepScan;
17+
import io.codemodder.providers.sarif.semgrep.SemgrepSarifJavaParserChanger;
1618
import java.util.Optional;
1719
import javax.inject.Inject;
1820

@@ -27,10 +29,7 @@ public final class SemgrepOverlyPermissiveFilePermissionsCodemod
2729

2830
@Inject
2931
public SemgrepOverlyPermissiveFilePermissionsCodemod(
30-
@ProvidedSemgrepScan(
31-
ruleId =
32-
"java.lang.security.audit.overly-permissive-file-permission.overly-permissive-file-permission")
33-
final RuleSarif sarif) {
32+
@ProvidedSemgrepScan(ruleId = RULE_ID) final RuleSarif sarif) {
3433
super(
3534
new PermissionAddCallChanger(sarif),
3635
new FromStringChanger(sarif),
@@ -45,7 +44,7 @@ public SemgrepOverlyPermissiveFilePermissionsCodemod(
4544
* PosixFilePermissions.fromString("rwxrwxrwx"));}
4645
*/
4746
private static final class InlineFromStringChanger
48-
extends SarifPluginJavaParserChanger<ExpressionStmt> {
47+
extends SemgrepSarifJavaParserChanger<ExpressionStmt> {
4948

5049
private InlineFromStringChanger(final RuleSarif sarif) {
5150
super(
@@ -82,6 +81,11 @@ public ChangesResult onResultFound(
8281
}
8382
return ChangesResult.noChanges;
8483
}
84+
85+
@Override
86+
public DetectorRule getDetectorRule() {
87+
return new DetectorRule(RULE_ID, RULE_NAME, RULE_URL);
88+
}
8589
}
8690

8791
/**
@@ -90,7 +94,7 @@ public ChangesResult onResultFound(
9094
* <p>{@code var p = Permissions.fromString("rwxrwxrwx")}
9195
*/
9296
private static final class FromStringChanger
93-
extends SarifPluginJavaParserChanger<ExpressionStmt> {
97+
extends SemgrepSarifJavaParserChanger<ExpressionStmt> {
9498
private FromStringChanger(final RuleSarif sarif) {
9599
super(
96100
sarif,
@@ -118,6 +122,11 @@ public ChangesResult onResultFound(
118122
}
119123
return ChangesResult.noChanges;
120124
}
125+
126+
@Override
127+
public DetectorRule getDetectorRule() {
128+
return new DetectorRule(RULE_ID, RULE_NAME, RULE_URL);
129+
}
121130
}
122131

123132
private static ChangesResult fixFromString(final MethodCallExpr fromStringCall) {
@@ -139,7 +148,7 @@ private static ChangesResult fixFromString(final MethodCallExpr fromStringCall)
139148
* java.util.Set#add(Object)} call.
140149
*/
141150
private static final class PermissionAddCallChanger
142-
extends SarifPluginJavaParserChanger<MethodCallExpr> {
151+
extends SemgrepSarifJavaParserChanger<MethodCallExpr> {
143152

144153
private PermissionAddCallChanger(final RuleSarif sarif) {
145154
super(
@@ -170,6 +179,11 @@ public ChangesResult onResultFound(
170179
return ChangesResult.noChanges;
171180
}
172181

182+
@Override
183+
public DetectorRule getDetectorRule() {
184+
return new DetectorRule(RULE_ID, RULE_NAME, RULE_URL);
185+
}
186+
173187
private ChangesResult fixAdd(final MethodCallExpr call) {
174188
NodeList<Expression> arguments = call.getArguments();
175189
Expression permissionArgument = arguments.get(0);
@@ -190,4 +204,11 @@ private ChangesResult fixAdd(final MethodCallExpr call) {
190204
return ChangesResult.changesApplied;
191205
}
192206
}
207+
208+
private static final String RULE_ID =
209+
"java.lang.security.audit.overly-permissive-file-permission.overly-permissive-file-permission";
210+
private static final String RULE_NAME =
211+
"Fix overly permissive file permissions (issue discovered by Semgrep)";
212+
private static final String RULE_URL =
213+
"https://registry.semgrep.dev/rule/java.lang.security.audit.overly-permissive-file-permission.overly-permissive-file-permission";
193214
}

core-codemods/src/test/java/io/codemodder/codemods/SemgrepOverlyPermissiveFilePermissionsCodemodTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@
77
codemodType = SemgrepOverlyPermissiveFilePermissionsCodemod.class,
88
testResourceDir = "semgrep-overly-permissive-file-permission",
99
doRetransformTest = false,
10-
dependencies = {})
10+
dependencies = {},
11+
expectingFixesAtLines = {14, 15, 25, 22})
1112
final class SemgrepOverlyPermissiveFilePermissionsCodemodTest implements CodemodTestMixin {}

framework/codemodder-base/src/main/java/io/codemodder/CodemodChange.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@ private CodemodChange(final int lineNumber, final List<DependencyGAV> dependenci
3535
this.fixedFindings = List.of();
3636
}
3737

38+
private CodemodChange(
39+
final int lineNumber,
40+
final List<DependencyGAV> dependenciesNeeded,
41+
final List<FixedFinding> fixedFindings) {
42+
this.lineNumber = lineNumber;
43+
this.dependenciesNeeded = Objects.requireNonNull(dependenciesNeeded, "dependenciesNeeded");
44+
this.parameters = List.of();
45+
this.description = null;
46+
this.fixedFindings = fixedFindings;
47+
}
48+
3849
private CodemodChange(final int lineNumber, final FixedFinding finding) {
3950
this.lineNumber = lineNumber;
4051
this.dependenciesNeeded = List.of();
@@ -138,6 +149,11 @@ public static CodemodChange from(final int line, final FixedFinding finding) {
138149
return new CodemodChange(line, finding);
139150
}
140151

152+
public static CodemodChange from(
153+
final int line, final List<DependencyGAV> dependencyGAVS, final FixedFinding finding) {
154+
return new CodemodChange(line, dependencyGAVS, List.of(finding));
155+
}
156+
141157
/** A {@link } */
142158
public static CodemodChange from(
143159
final int line, final Parameter parameter, final String valueUsed) {

framework/codemodder-base/src/main/java/io/codemodder/FixOnlyCodeChanger.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package io.codemodder;
22

33
import io.codemodder.codetf.DetectorRule;
4+
import io.codemodder.codetf.FixedFinding;
5+
import java.util.Optional;
46

57
/**
68
* A codemod that only fixes issues and does not perform its own detection, instead relying on
@@ -15,4 +17,7 @@ public interface FixOnlyCodeChanger {
1517

1618
/** A description of the rule. */
1719
DetectorRule getDetectorRule();
20+
21+
/** Abstract method to build a {@link FixedFinding} object */
22+
Optional<FixedFinding> getFixedFinding(String id);
1823
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package io.codemodder;
2+
3+
import com.github.javaparser.ast.Node;
4+
import io.codemodder.codetf.FixedFinding;
5+
import java.util.Optional;
6+
7+
public abstract class SarifPluginFixOnlyCodeChanger<T extends Node>
8+
extends SarifPluginJavaParserChanger<T> implements FixOnlyCodeChanger {
9+
10+
protected SarifPluginFixOnlyCodeChanger(
11+
final RuleSarif sarif,
12+
final Class<? extends Node> nodeType,
13+
final RegionNodeMatcher regionNodeMatcher,
14+
final CodemodReporterStrategy reporterStrategy) {
15+
super(sarif, nodeType, regionNodeMatcher, reporterStrategy);
16+
}
17+
18+
@Override
19+
public Optional<FixedFinding> getFixedFinding(String id) {
20+
return Optional.of(new FixedFinding(id, getDetectorRule()));
21+
}
22+
}

framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
import com.github.javaparser.ast.CompilationUnit;
77
import com.github.javaparser.ast.Node;
88
import com.google.common.annotations.VisibleForTesting;
9+
import io.codemodder.codetf.FixedFinding;
910
import io.codemodder.javaparser.ChangesResult;
1011
import io.codemodder.javaparser.JavaParserChanger;
1112
import java.util.ArrayList;
1213
import java.util.List;
1314
import java.util.Objects;
15+
import java.util.Optional;
1416

1517
/**
1618
* Provides base functionality for making JavaParser-based changes based on results found by a sarif
@@ -131,9 +133,18 @@ public CodemodFileScanningResult visit(
131133
if (regionNodeMatcher.matches(region, range)) {
132134
ChangesResult changeSuccessful = onResultFound(context, cu, (T) node, result);
133135
if (changeSuccessful.areChangesApplied()) {
134-
codemodChanges.add(
135-
CodemodChange.from(
136-
region.start().line(), changeSuccessful.getDependenciesRequired()));
136+
final Optional<FixedFinding> optionalFixedFinding = getFixedFinding();
137+
if (optionalFixedFinding.isPresent()) {
138+
codemodChanges.add(
139+
CodemodChange.from(
140+
region.start().line(),
141+
changeSuccessful.getDependenciesRequired(),
142+
optionalFixedFinding.get()));
143+
} else {
144+
codemodChanges.add(
145+
CodemodChange.from(
146+
region.start().line(), changeSuccessful.getDependenciesRequired()));
147+
}
137148
}
138149
}
139150
}
@@ -144,6 +155,10 @@ public CodemodFileScanningResult visit(
144155
return CodemodFileScanningResult.withOnlyChanges(codemodChanges);
145156
}
146157

158+
protected Optional<FixedFinding> getFixedFinding() {
159+
return Optional.empty();
160+
}
161+
147162
@Override
148163
public boolean shouldRun() {
149164
List<Run> runs = sarif.rawDocument().getRuns();

framework/codemodder-base/src/test/java/io/codemodder/DefaultCodemodExecutorTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,11 @@ public String vendorName() {
802802
public DetectorRule getDetectorRule() {
803803
return rule;
804804
}
805+
806+
@Override
807+
public Optional<FixedFinding> getFixedFinding(String id) {
808+
return Optional.of(new FixedFinding(id, getDetectorRule()));
809+
}
805810
}
806811

807812
private static final DependencyGAV dependency1 =
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package io.codemodder.providers.sarif.semgrep;
2+
3+
import com.github.javaparser.ast.Node;
4+
import io.codemodder.CodemodReporterStrategy;
5+
import io.codemodder.RegionNodeMatcher;
6+
import io.codemodder.RuleSarif;
7+
import io.codemodder.SarifPluginFixOnlyCodeChanger;
8+
9+
/**
10+
* Provides foundational functionality for modifying Java code using JavaParser based on findings
11+
* from a SARIF file generated by Semgrep analysis.
12+
*/
13+
public abstract class SemgrepSarifJavaParserChanger<T extends Node>
14+
extends SarifPluginFixOnlyCodeChanger<T> {
15+
16+
protected SemgrepSarifJavaParserChanger(
17+
final RuleSarif sarif,
18+
final Class<? extends Node> nodeType,
19+
final RegionNodeMatcher regionNodeMatcher,
20+
final CodemodReporterStrategy reporterStrategy) {
21+
super(sarif, nodeType, regionNodeMatcher, reporterStrategy);
22+
}
23+
24+
@Override
25+
public String vendorName() {
26+
return "Semgrep";
27+
}
28+
}

plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
import com.github.javaparser.ast.CompilationUnit;
55
import com.github.javaparser.ast.Node;
66
import io.codemodder.*;
7+
import io.codemodder.codetf.FixedFinding;
78
import io.codemodder.javaparser.ChangesResult;
89
import io.codemodder.javaparser.JavaParserChanger;
910
import io.codemodder.providers.sonar.api.Issue;
1011
import java.util.ArrayList;
1112
import java.util.List;
1213
import java.util.Objects;
14+
import java.util.Optional;
1315

1416
/** Provides base functionality for making JavaParser-based changes based on Sonar results. */
1517
public abstract class SonarPluginJavaParserChanger<T extends Node> extends JavaParserChanger
@@ -112,4 +114,9 @@ public String vendorName() {
112114
*/
113115
public abstract ChangesResult onIssueFound(
114116
CodemodInvocationContext context, CompilationUnit cu, T node, Issue issue);
117+
118+
@Override
119+
public Optional<FixedFinding> getFixedFinding(String id) {
120+
return Optional.of(new FixedFinding(id, getDetectorRule()));
121+
}
115122
}

0 commit comments

Comments
 (0)