Refactoring/general code imporvements (#68) (#70)

- extract sonar issue reporting functionality to higher level rule - add SecureRandomGetInstance rule to java - add ITranslationProcess to python module - enhance logging - add java docs - refactoring --------- Signed-off-by: Nicklas Körtge <nicklas.koertge1@ibm.com>
IBM · Jul 22, 2024 · cb05d70 · cb05d70
1 parent c961df9
commit cb05d70
Show file tree

Hide file tree

Showing 32 changed files with 4,354 additions and 608 deletions.
diff --git a/engine/src/main/java/com/ibm/engine/hooks/HookRepository.java b/engine/src/main/java/com/ibm/engine/hooks/HookRepository.java
@@ -24,7 +24,11 @@
 import com.ibm.engine.callstack.CallContext;
 import com.ibm.engine.callstack.IGetNotifiedWhenNewCallWasAddedToCallStack;
 import com.ibm.engine.detection.Handler;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.EnumMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nonnull;
 
@@ -102,9 +106,10 @@ public void notify(@Nonnull Event event, @Nonnull IHook<R, T, S, P> object) {
     }
 
     /**
-     * Check if hook is registered to trigger hook detection event
+     * Check if hook is registered to trigger hook detection event for {@link CallContext}.
      *
-     * @param callContext
+     * @param callContext The context in which to update the hooks. This should include information
+     *     about the current state of the system, such as the current request and response objects.
      */
     @Override
     public void update(@Nonnull final CallContext<R, T> callContext) {

diff --git a/engine/src/main/java/com/ibm/engine/language/ILanguageSupport.java b/engine/src/main/java/com/ibm/engine/language/ILanguageSupport.java
@@ -19,38 +19,102 @@
  */
 package com.ibm.engine.language;
 
-import com.ibm.engine.detection.*;
+import com.ibm.engine.detection.DetectionStore;
+import com.ibm.engine.detection.EnumMatcher;
+import com.ibm.engine.detection.IBaseMethodVisitorFactory;
+import com.ibm.engine.detection.IDetectionEngine;
+import com.ibm.engine.detection.MatchContext;
+import com.ibm.engine.detection.MethodMatcher;
 import com.ibm.engine.executive.DetectionExecutive;
 import com.ibm.engine.rule.IDetectionRule;
 import java.util.Optional;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+/** Interface for adding language support. */
 public interface ILanguageSupport<R, T, S, P> {
+    /**
+     * Returns an object that can perform translation from language-specific objects to a language
+     * agnostic representation.
+     *
+     * @return an object that can perform translation from one language to a language agnostic
+     *     representation.
+     */
     @Nonnull
     ILanguageTranslation<T> translation();
 
+    /**
+     * Creates a new {@link DetectionExecutive} instance for the specified tree and detection rule.
+     *
+     * @param tree the tree representing the code to analyze
+     * @param detectionRule the detection rule to use
+     * @param scanContext the context for the scan, which provides information about the file and
+     *     the current state of the analysis
+     * @return a new {@link DetectionExecutive} instance
+     */
     @Nonnull
     DetectionExecutive<R, T, S, P> createDetectionExecutive(
             @Nonnull T tree,
             @Nonnull IDetectionRule<T> detectionRule,
             @Nonnull IScanContext<R, T> scanContext);
 
+    /**
+     * Creates a new {@link IDetectionEngine} instance for the specified detection store.
+     *
+     * <p>The created engine will be used to detect code matching the detection rule provided by the
+     * detection store.
+     *
+     * @param detectionStore the detection store that contains the detection rule adn will store the
+     *     detected values.
+     * @return a new {@link IDetectionEngine} instance
+     */
     @Nonnull
     IDetectionEngine<T, S> createDetectionEngineInstance(
             @Nonnull DetectionStore<R, T, S, P> detectionStore);
 
-    @Nonnull
-    IScanContext<R, T> createScanContext(@Nonnull P publisher);
-
+    /**
+     * Returns an object that can be used to visit methods and perform analysis.
+     *
+     * <p>The returned visitor will be used to analyze the methods in the code represented by the
+     * specified tree.
+     *
+     * @return an object that can be used to visit methods and perform analysis
+     */
     @Nonnull
     IBaseMethodVisitorFactory<T, S> getBaseMethodVisitorFactory();
 
+    /**
+     * Returns the enclosing method for the specified expression.
+     *
+     * @param expression the expression for which to find the enclosing method
+     * @return the enclosing method for the specified expression, or null if there is no such method
+     */
     @Nonnull
     Optional<T> getEnclosingMethod(@Nonnull T expression);
 
+    /**
+     * Creates a new {@link MethodMatcher} instance based on the specified method definition.
+     *
+     * <p>The returned matcher will be used to match methods in the code represented by the
+     * specified tree against the specified method definition.
+     *
+     * @param methodDefinition the method definition to use for matching
+     * @return a new {@link MethodMatcher} instance
+     */
     @Nullable MethodMatcher<T> createMethodMatcherBasedOn(@Nonnull T methodDefinition);
 
+    /**
+     * Creates a new {@link EnumMatcher} instance based on the specified enum identifier and match
+     * context.
+     *
+     * <p>The returned matcher will be used to match enum values in the code represented by the
+     * specified tree against the specified enum identifier and match context.
+     *
+     * @param enumIdentifier the enum identifier to use for matching
+     * @param matchContext the match context that provides information about the file and the
+     *     current state of the analysis
+     * @return a new {@link EnumMatcher} instance
+     */
     @Nullable EnumMatcher<T> createSimpleEnumMatcherFor(
             @Nonnull T enumIdentifier, @Nonnull MatchContext matchContext);
 }
diff --git a/engine/src/main/java/com/ibm/engine/language/java/JavaLanguageSupport.java b/engine/src/main/java/com/ibm/engine/language/java/JavaLanguageSupport.java
@@ -19,7 +19,13 @@
  */
 package com.ibm.engine.language.java;
 
-import com.ibm.engine.detection.*;
+import com.ibm.engine.detection.DetectionStore;
+import com.ibm.engine.detection.EnumMatcher;
+import com.ibm.engine.detection.Handler;
+import com.ibm.engine.detection.IBaseMethodVisitorFactory;
+import com.ibm.engine.detection.IDetectionEngine;
+import com.ibm.engine.detection.MatchContext;
+import com.ibm.engine.detection.MethodMatcher;
 import com.ibm.engine.executive.DetectionExecutive;
 import com.ibm.engine.language.ILanguageSupport;
 import com.ibm.engine.language.ILanguageTranslation;
@@ -37,7 +43,11 @@
 import org.sonar.plugins.java.api.JavaCheck;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.semantic.Symbol;
-import org.sonar.plugins.java.api.tree.*;
+import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.MethodTree;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.TypeTree;
 
 public final class JavaLanguageSupport
         implements ILanguageSupport<JavaCheck, Tree, Symbol, JavaFileScannerContext> {
@@ -73,13 +83,6 @@ public ILanguageTranslation<Tree> translation() {
         return new JavaDetectionEngine(detectionStore, this.handler);
     }
 
-    @Nonnull
-    @Override
-    public @NotNull IScanContext<JavaCheck, Tree> createScanContext(
-            @Nonnull JavaFileScannerContext publisher) {
-        return new JavaScanContext(publisher);
-    }
-
     @Nonnull
     @Override
     public @NotNull IBaseMethodVisitorFactory<Tree, Symbol> getBaseMethodVisitorFactory() {

diff --git a/engine/src/main/java/com/ibm/engine/language/python/PythonLanguageSupport.java b/engine/src/main/java/com/ibm/engine/language/python/PythonLanguageSupport.java
@@ -21,7 +21,13 @@
 
 import static com.ibm.engine.detection.MethodMatcher.ANY;
 
-import com.ibm.engine.detection.*;
+import com.ibm.engine.detection.DetectionStore;
+import com.ibm.engine.detection.EnumMatcher;
+import com.ibm.engine.detection.Handler;
+import com.ibm.engine.detection.IBaseMethodVisitorFactory;
+import com.ibm.engine.detection.IDetectionEngine;
+import com.ibm.engine.detection.MatchContext;
+import com.ibm.engine.detection.MethodMatcher;
 import com.ibm.engine.executive.DetectionExecutive;
 import com.ibm.engine.language.ILanguageSupport;
 import com.ibm.engine.language.ILanguageTranslation;
@@ -35,7 +41,11 @@
 import org.sonar.plugins.python.api.PythonCheck;
 import org.sonar.plugins.python.api.PythonVisitorContext;
 import org.sonar.plugins.python.api.symbols.Symbol;
-import org.sonar.plugins.python.api.tree.*;
+import org.sonar.plugins.python.api.tree.FileInput;
+import org.sonar.plugins.python.api.tree.FunctionDef;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.ParameterList;
+import org.sonar.plugins.python.api.tree.Tree;
 
 public class PythonLanguageSupport
         implements ILanguageSupport<PythonCheck, Tree, Symbol, PythonVisitorContext> {
@@ -67,12 +77,6 @@ public ILanguageTranslation<Tree> translation() {
         return new PythonDetectionEngine(detectionStore, this.handler);
     }
 
-    @Override
-    public @NotNull IScanContext<PythonCheck, Tree> createScanContext(
-            @NotNull PythonVisitorContext publisher) {
-        return new PythonScanContext(publisher);
-    }
-
     @Override
     public @NotNull IBaseMethodVisitorFactory<Tree, Symbol> getBaseMethodVisitorFactory() {
         return PythonBaseMethodVisitor::new;
@@ -84,9 +88,9 @@ public ILanguageTranslation<Tree> translation() {
         // otherwise we return the highest level root node corresponding to all the content of the
         // current file.
         if (expression instanceof FunctionDef functionDefTree) {
-            return Optional.ofNullable(functionDefTree);
+            return Optional.of(functionDefTree);
         } else if (expression instanceof FileInput fileInputTree) {
-            return Optional.ofNullable(fileInputTree);
+            return Optional.of(fileInputTree);
         } else if (expression.parent() != null) {
             return getEnclosingMethod(expression.parent());
         }
@@ -101,14 +105,11 @@ public MethodMatcher<Tree> createMethodMatcherBasedOn(@NotNull Tree methodDefini
             // name, to which we remove the function name.
             String invocationObjectName;
             Optional<String> invocationObjectNameOptional =
-                    Optional.ofNullable(functionDefTree)
+                    Optional.of(functionDefTree)
                             .map(FunctionDef::name)
                             .map(Name::symbol)
                             .map(Symbol::fullyQualifiedName);
-            invocationObjectName =
-                    invocationObjectNameOptional.isPresent()
-                            ? invocationObjectNameOptional.get()
-                            : "";
+            invocationObjectName = invocationObjectNameOptional.orElse("");
 
             int lastDotIndex = invocationObjectName.lastIndexOf(".");
             lastDotIndex = lastDotIndex == -1 ? invocationObjectName.length() : lastDotIndex;
@@ -121,7 +122,6 @@ public MethodMatcher<Tree> createMethodMatcherBasedOn(@NotNull Tree methodDefini
             if (parameterList != null) {
                 String[] parameters =
                         parameterList.all().stream()
-                                // TODO: I don't think we can do better tham using type ANY for each
                                 // parameter
                                 .map(param -> ANY)
                                 .toArray(String[]::new);

diff --git a/enricher/src/main/java/com/ibm/enricher/Enricher.java b/enricher/src/main/java/com/ibm/enricher/Enricher.java
@@ -26,8 +26,19 @@
 import java.util.Map;
 import javax.annotation.Nonnull;
 
+/**
+ * This enricher instance operates on a language-agnostic level, meaning it will enrich the given
+ * list of nodes with general cryptographic knowledge, such as OIDs. This does not include
+ * language-specific information like default key sizes. Therefore, create a library-specific
+ * enricher instance as part of the language package.
+ */
 public class Enricher implements IEnricher {
 
+    /**
+     * Enriches a list of nodes with additional information.
+     *
+     * @param nodes The list of nodes to enrich
+     */
     public static void enrich(@Nonnull final List<INode> nodes) {
         final Enricher enricher = new Enricher();
         nodes.forEach(enricher::enrich);
@@ -39,6 +50,11 @@ public static void enrich(@Nonnull final List<INode> nodes) {
                 });
     }
 
+    /**
+     * Enriches the given node with additional information.
+     *
+     * @param node The node to enrich
+     */
     @Override
     public void enrich(@Nonnull INode node) {
         if (node instanceof Algorithm algorithm) {

diff --git a/java/src/main/java/com/ibm/plugin/rules/JavaInventoryRule.java b/java/src/main/java/com/ibm/plugin/rules/JavaInventoryRule.java
@@ -19,15 +19,20 @@
  */
 package com.ibm.plugin.rules;
 
+import com.ibm.engine.detection.DetectionStore;
 import com.ibm.engine.rule.IDetectionRule;
-import com.ibm.mapper.reorganizer.IReorganizerRule;
 import com.ibm.plugin.rules.detection.JavaBaseDetectionRule;
 import com.ibm.plugin.rules.detection.JavaDetectionRules;
 import com.ibm.plugin.translation.reorganizer.JavaReorganizerRules;
 import java.util.List;
 import javax.annotation.Nonnull;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Unmodifiable;
 import org.jetbrains.annotations.VisibleForTesting;
 import org.sonar.check.Rule;
+import org.sonar.plugins.java.api.JavaCheck;
+import org.sonar.plugins.java.api.JavaFileScannerContext;
+import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.tree.Tree;
 
 @Rule(key = "Inventory")
@@ -41,10 +46,18 @@ protected JavaInventoryRule(@Nonnull List<IDetectionRule<Tree>> detectionRules)
         super(detectionRules, JavaReorganizerRules.rules());
     }
 
-    @VisibleForTesting
-    protected JavaInventoryRule(
-            @Nonnull List<IDetectionRule<Tree>> detectionRules,
-            @Nonnull List<IReorganizerRule> reorganizerRules) {
-        super(detectionRules, reorganizerRules);
+    @Override
+    public void report(
+            @NotNull @Unmodifiable
+                    DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
+            @NotNull JavaCheck rule) {
+        detectionStore
+                .getDetectionValues()
+                .forEach(
+                        iValue ->
+                                detectionStore
+                                        .getScanContext()
+                                        .reportIssue(
+                                                rule, iValue.getLocation(), iValue.asString()));
     }
 }