Merge pull request #833 from forcedotcom/dev-3

ranekere-sfdc · web-flow · commit 279e173fc662 · 2022-09-26T10:35:25.000-07:00
@W-11792858@: Release activity for v3.5.1 - Merging dev-3 to release-3
diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java b/cli-messaging/src/main/java/com/salesforce/messaging/EventKey.java
@@ -38,9 +38,12 @@ public enum EventKey {
     WARNING_GENERAL("warning.sfgeWarnLog", 1, MessageType.WARNING, MessageHandler.UX, true),
 	WARNING_MULTIPLE_METHOD_TARGET_MATCHES("warning.multipleMethodTargetMatches", 3, MessageType.WARNING, MessageHandler.UX, false),
 	WARNING_NO_METHOD_TARGET_MATCHES("warning.noMethodTargetMatches", 2, MessageType.WARNING, MessageHandler.UX, false),
-    ERROR_GENERAL("error.internal.sfgeErrorLog", 1, MessageType.ERROR, MessageHandler.UX, false);
+    ERROR_GENERAL("error.internal.sfgeErrorLog", 1, MessageType.ERROR, MessageHandler.UX, false),
 
-	final String messageKey;
+    /** GENERAL PURPOSE */
+    INFO_TELEMETRY("info.telemetry", 1, MessageType.TELEMETRY, MessageHandler.INTERNAL, false);
+
+    final String messageKey;
 	final int argCount;
 	final MessageType messageType;
 	final MessageHandler messageHandler;
diff --git a/cli-messaging/src/main/java/com/salesforce/messaging/Message.java b/cli-messaging/src/main/java/com/salesforce/messaging/Message.java
@@ -57,6 +57,7 @@ enum MessageHandler {
 	}
 
 	enum MessageType {
+        TELEMETRY,
 		INFO,
 		WARNING,
 		ERROR
diff --git a/messages/EventKeyTemplates.js b/messages/EventKeyTemplates.js
@@ -15,7 +15,8 @@ module.exports = {
 		"sfgeFinishedBuildingGraph": "Added all compilation units to graph.",
 		"sfgePathEntryPointsIdentified": "Identified %s path entry point(s).",
 		"sfgeViolationsInPathProgress": "Detected %s violation(s) from %s path(s) on %s/%s entry point(s).",
-		"sfgeCompletedPathAnalysis": "Overall, analyzed %s path(s) from %s entry point(s). Detected %s violation(s)."
+		"sfgeCompletedPathAnalysis": "Overall, analyzed %s path(s) from %s entry point(s). Detected %s violation(s).",
+		"telemetry": "This message is unused."
 	},
 	"warning": {
 		"invalidCategorySkipped": "Cataloger: Skipping invalid PMD Category file '%s'.",
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
 	"name": "@salesforce/sfdx-scanner",
 	"description": "Static code scanner that applies quality and security rules to Apex code, and provides feedback.",
-	"version": "3.5.0",
+	"version": "3.5.1",
 	"author": "ISV SWAT",
 	"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",
 	"dependencies": {
diff --git a/retire-js/RetireJsVulns.json b/retire-js/RetireJsVulns.json
@@ -2592,7 +2592,7 @@
       },
       {
         "below": "2.29.2",
-        "severity": "medium",
+        "severity": "high",
         "identifiers": {
           "summary": "This vulnerability impacts npm (server) users of moment.js, especially if user provided locale string, eg fr is directly used to switch moment locale.",
           "CVE": [
@@ -2606,7 +2606,7 @@
       {
         "below": "2.29.4",
         "atOrAbove": "2.18.0",
-        "severity": "medium",
+        "severity": "high",
         "identifiers": {
           "summary": "Regular Expression Denial of Service (ReDoS), Affecting moment package, versions >=2.18.0 <2.29.4",
           "CVE": [
diff --git a/sfge/src/main/java/com/salesforce/TelemetryAppender.java b/sfge/src/main/java/com/salesforce/TelemetryAppender.java
@@ -0,0 +1,58 @@
+package com.salesforce;
+
+import com.salesforce.telemetry.TelemetryUtil;
+import java.io.Serializable;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.*;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.plugins.Plugin;
+import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
+import org.apache.logging.log4j.core.config.plugins.PluginElement;
+import org.apache.logging.log4j.core.config.plugins.PluginFactory;
+import org.apache.logging.log4j.core.layout.PatternLayout;
+
+/**
+ * Custom log4j2 appender to send logs as telemetry events through {@link
+ * com.salesforce.telemetry.TelemetryUtil}. This helps us capture telemetry events in response to
+ * unsupported/pathological scenarios. Invoked from log4j2.xml
+ */
+@Plugin(
+        name = "TelemetryAppender",
+        category = Core.CATEGORY_NAME,
+        elementType = Appender.ELEMENT_TYPE)
+public class TelemetryAppender extends AbstractAppender {
+    @PluginFactory
+    public static TelemetryAppender createAppender(
+            @PluginAttribute("name") String name,
+            @PluginElement("Layout") Layout<? extends Serializable> layout,
+            @PluginElement("Filter") final Filter filter) {
+        if (name == null) {
+            // Assign default name to avoid complaining
+            name = "TelemetryAppender";
+        }
+        if (layout == null) {
+            layout = PatternLayout.createDefaultLayout();
+        }
+        return new TelemetryAppender(name, filter, layout, true);
+    }
+
+    protected TelemetryAppender(
+            String name,
+            Filter filter,
+            Layout<? extends Serializable> layout,
+            final boolean ignoreExceptions) {
+        super(name, filter, layout, ignoreExceptions, null);
+    }
+
+    @Override
+    public void append(LogEvent event) {
+        Level level = event.getLevel();
+        if (Level.WARN.equals(level)) {
+            String eventMessage = event.getMessage().getFormattedMessage();
+            if (eventMessage.toLowerCase().startsWith("todo:")) {
+                TelemetryUtil.postWarningTelemetry(
+                        this.getLayout().toSerializable(event).toString(), event.getThrown());
+            }
+        }
+    }
+}
diff --git a/sfge/src/main/java/com/salesforce/apex/jorje/JorjeUtil.java b/sfge/src/main/java/com/salesforce/apex/jorje/JorjeUtil.java
@@ -129,7 +129,7 @@ private static void visitComments(JorjeNode wrapper, CommentVisitor visitor) {
         }
     }
 
-    public static class JorjeCompilationException extends SfgeRuntimeException {
+    public static final class JorjeCompilationException extends SfgeRuntimeException {
         JorjeCompilationException(String message) {
             super(message);
         }
diff --git a/sfge/src/main/java/com/salesforce/exception/DuplicateKeyException.java b/sfge/src/main/java/com/salesforce/exception/DuplicateKeyException.java
@@ -1,7 +1,7 @@
 package com.salesforce.exception;
 
 /** Indicates a developer error where the same key was added to a map twice */
-public class DuplicateKeyException extends SfgeRuntimeException {
+public final class DuplicateKeyException extends SfgeRuntimeException {
     public DuplicateKeyException(Object key, Object previousEntry, Object newEntry) {
         super("Duplicate keys. key=" + key + ", previous=" + previousEntry + ", new=" + newEntry);
     }
diff --git a/sfge/src/main/java/com/salesforce/exception/ProgrammingException.java b/sfge/src/main/java/com/salesforce/exception/ProgrammingException.java
@@ -1,7 +1,7 @@
 package com.salesforce.exception;
 
 /** Indicates a programming logic error, such as an item being initialized more than once. */
-public class ProgrammingException extends SfgeRuntimeException {
+public final class ProgrammingException extends SfgeRuntimeException {
     public ProgrammingException(String message) {
         super(message);
     }
diff --git a/sfge/src/main/java/com/salesforce/exception/SfgeInterruptedException.java b/sfge/src/main/java/com/salesforce/exception/SfgeInterruptedException.java
@@ -5,4 +5,8 @@
  * stop. Any long running methods should periodically invoke {@link Thread#interrupted()} and throw
  * this exception if appropriate.
  */
-public final class SfgeInterruptedException extends SfgeRuntimeException {}
+public final class SfgeInterruptedException extends SfgeRuntimeException {
+    public SfgeInterruptedException() {
+        super();
+    }
+}
diff --git a/sfge/src/main/java/com/salesforce/exception/SfgeRuntimeException.java b/sfge/src/main/java/com/salesforce/exception/SfgeRuntimeException.java
@@ -1,19 +1,25 @@
 package com.salesforce.exception;
 
+import com.salesforce.telemetry.TelemetryUtil;
+
 public abstract class SfgeRuntimeException extends RuntimeException {
     public SfgeRuntimeException() {
         super();
+        TelemetryUtil.postExceptionTelemetry(this);
     }
 
     public SfgeRuntimeException(Throwable cause) {
         super(cause);
+        TelemetryUtil.postExceptionTelemetry(this, cause);
     }
 
     public SfgeRuntimeException(String msg) {
         super(msg);
+        TelemetryUtil.postExceptionTelemetry(this);
     }
 
     public SfgeRuntimeException(String msg, Throwable cause) {
         super(msg, cause);
+        TelemetryUtil.postExceptionTelemetry(this, cause);
     }
 }
diff --git a/sfge/src/main/java/com/salesforce/exception/TodoException.java b/sfge/src/main/java/com/salesforce/exception/TodoException.java
@@ -1,6 +1,6 @@
 package com.salesforce.exception;
 
-public class TodoException extends SfgeRuntimeException {
+public final class TodoException extends SfgeRuntimeException {
     public TodoException() {
         this("TODO");
     }
diff --git a/sfge/src/main/java/com/salesforce/exception/UnexpectedException.java b/sfge/src/main/java/com/salesforce/exception/UnexpectedException.java
@@ -12,7 +12,7 @@
  * Thrown when an AST related assumption is violated. Some code still uses this where a {@link
  * TodoException} is more appropriate.
  */
-public class UnexpectedException extends SfgeRuntimeException {
+public final class UnexpectedException extends SfgeRuntimeException {
     public UnexpectedException(Object obj) {
         super(obj != null ? obj.toString() : "<null>");
     }
diff --git a/sfge/src/main/java/com/salesforce/exception/UserActionException.java b/sfge/src/main/java/com/salesforce/exception/UserActionException.java
@@ -4,7 +4,7 @@
  * Denotes exceptions caused by incorrect code. Code may have compiled but user needs to take an
  * action to fix their code.
  */
-public class UserActionException extends SfgeRuntimeException {
+public final class UserActionException extends SfgeRuntimeException {
     public UserActionException(String message) {
         super(message);
     }
diff --git a/sfge/src/main/java/com/salesforce/exception/VisitNotOverriddenException.java b/sfge/src/main/java/com/salesforce/exception/VisitNotOverriddenException.java
@@ -7,7 +7,7 @@
  * pattern relies on compile time dispatch to the correct overloaded visitor method in order for the
  * pattern to work.
  */
-public class VisitNotOverriddenException extends SfgeRuntimeException {
+public final class VisitNotOverriddenException extends SfgeRuntimeException {
     public VisitNotOverriddenException() {
         this("Concrete classes need to override visit or else the visitor won't work.");
     }
diff --git a/sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpanderRuntimeException.java b/sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpanderRuntimeException.java
@@ -3,4 +3,8 @@
 import com.salesforce.exception.SfgeRuntimeException;
 
 /** Base class for all runtime exceptions thrown by ApexPathExpander related code */
-public abstract class ApexPathExpanderRuntimeException extends SfgeRuntimeException {}
+public abstract class ApexPathExpanderRuntimeException extends SfgeRuntimeException {
+    protected ApexPathExpanderRuntimeException(String msg) {
+        super(msg);
+    }
+}
diff --git a/sfge/src/main/java/com/salesforce/graph/ops/expander/NullValueAccessedException.java b/sfge/src/main/java/com/salesforce/graph/ops/expander/NullValueAccessedException.java
@@ -12,7 +12,7 @@
  * <p>The ApexValue can be null because it is a real bug in the code, or it could be that the org is
  * setup in such a way that what we interpret as a null condition will never happen in practice.
  */
-public class NullValueAccessedException extends ApexPathExpanderRuntimeException {
+public final class NullValueAccessedException extends ApexPathExpanderRuntimeException {
     private final ApexValue<?> apexValue;
     private final MethodCallExpressionVertex vertex;
 
@@ -22,6 +22,7 @@ public class NullValueAccessedException extends ApexPathExpanderRuntimeException
      */
     public NullValueAccessedException(
             ApexValue<?> apexValue, @Nullable MethodCallExpressionVertex vertex) {
+        super("ApexValue=" + apexValue + ", vertex=" + (vertex != null ? vertex : "<null>"));
         this.apexValue = apexValue;
         this.vertex = vertex;
     }
diff --git a/sfge/src/main/java/com/salesforce/graph/ops/expander/switches/ApexPathCaseStatementExcluder.java b/sfge/src/main/java/com/salesforce/graph/ops/expander/switches/ApexPathCaseStatementExcluder.java
@@ -114,5 +114,9 @@ private static final class LazyHolder {
     private ApexPathCaseStatementExcluder() {}
 
     /** Thrown by ExcludeCaseStatementVertexVisitor if the when block should be excluded */
-    static final class CaseStatementExcludedException extends SfgeRuntimeException {}
+    static final class CaseStatementExcludedException extends SfgeRuntimeException {
+        CaseStatementExcludedException() {
+            super();
+        }
+    }
 }
diff --git a/sfge/src/main/java/com/salesforce/graph/visitor/ApexPathWalker.java b/sfge/src/main/java/com/salesforce/graph/visitor/ApexPathWalker.java
@@ -274,6 +274,8 @@ private static final class ThrowStatementVertexVisitedException extends SfgeRunt
         private final ThrowStatementVertex vertex;
 
         private ThrowStatementVertexVisitedException(ThrowStatementVertex vertex) {
+            // NOTE: We're very deliberately NOT calling `super()` here, since this
+            // exception type doesn't require telemetry.
             this.vertex = vertex;
         }
     }
diff --git a/sfge/src/main/java/com/salesforce/telemetry/TelemetryUtil.java b/sfge/src/main/java/com/salesforce/telemetry/TelemetryUtil.java
@@ -0,0 +1,118 @@
+package com.salesforce.telemetry;
+
+import com.google.gson.Gson;
+import com.salesforce.exception.SfgeRuntimeException;
+import com.salesforce.messaging.CliMessager;
+import com.salesforce.messaging.EventKey;
+import java.util.Arrays;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+
+public final class TelemetryUtil {
+    private static final String MAIN_THREAD_NAME = "main";
+
+    /**
+     * Posts a telemetry event in response to a {@link org.apache.logging.log4j.Logger#warn} call.
+     *
+     * @param message - The message being logged.
+     * @param cause - The exception attached to the log, if available.
+     */
+    public static void postWarningTelemetry(String message, @Nullable Throwable cause) {
+        EventType eventType =
+                isMainThread() ? EventType.MAIN_THREAD_WARNING : EventType.RULE_THREAD_WARNING;
+        StackTraceElement[] trace =
+                cause == null ? Thread.currentThread().getStackTrace() : cause.getStackTrace();
+        postTelemetry(message, trace, eventType);
+    }
+
+    /**
+     * Posts a telemetry event in response to an {@link
+     * com.salesforce.exception.SfgeRuntimeException} being thrown.
+     *
+     * @param runtimeException - The exception being thrown.
+     */
+    public static void postExceptionTelemetry(SfgeRuntimeException runtimeException) {
+        postExceptionTelemetry(runtimeException, null);
+    }
+
+    /**
+     * Posts a telemetry event in response to an {@link
+     * com.salesforce.exception.SfgeRuntimeException} being thrown.
+     *
+     * @param runtimeException - The exception being thrown.
+     * @param cause - The exception that caused the runtimeException, if available.
+     */
+    public static void postExceptionTelemetry(
+            SfgeRuntimeException runtimeException, @Nullable Throwable cause) {
+        EventType eventType =
+                isMainThread() ? EventType.MAIN_THREAD_EXCEPTION : EventType.RULE_THREAD_EXCEPTION;
+        StackTraceElement[] trace =
+                cause == null ? runtimeException.getStackTrace() : cause.getStackTrace();
+        postTelemetry(runtimeException.getMessage(), trace, eventType);
+    }
+
+    private static void postTelemetry(
+            String message, StackTraceElement[] trace, EventType eventType) {
+        TelemetryData telemetryData = new TelemetryData(message, trace, eventType);
+        CliMessager.postMessage(
+                "TelemetryData", EventKey.INFO_TELEMETRY, new Gson().toJson(telemetryData));
+    }
+
+    /** Returns a boolean indicating whether this thread is the main thread. */
+    private static boolean isMainThread() {
+        // Assumption: The name of the main thread will consistently be "main".
+        // If this assumption is invalidated, this code must change.
+        return MAIN_THREAD_NAME.equalsIgnoreCase(Thread.currentThread().getName());
+    }
+
+    /** An object that can be used as the base for an SFDX telemetry event. */
+    private static class TelemetryData {
+        /** Necessary property for telemetry objects. */
+        private final String eventName = "SFGE_TELEMETRY";
+
+        private final String message;
+        private final EventType eventType;
+        private final String stackTrace;
+
+        public TelemetryData(String message, StackTraceElement[] trace, EventType eventType) {
+            this.message = message;
+            this.eventType = eventType;
+            this.stackTrace =
+                    Arrays.stream(trace)
+                            // We want five-ish stack frames of meaningful context, and it's
+                            // possible for the first frame to just be a TelemetryUtil method.
+                            // So we'll get the first six frames.
+                            .limit(6)
+                            .map(StackTraceElement::toString)
+                            .collect(Collectors.joining("\n"));
+        }
+    }
+
+    private enum EventType {
+        /**
+         * Indicates that an unsupported scenario was encountered in the main thread, and in
+         * response a warning was logged but execution continued to the best of its ability.
+         */
+        MAIN_THREAD_WARNING,
+        /**
+         * Indicates that an unsupported scenario was encountered in a rule executor thread, and in
+         * response a warning was logged but execution continued to the best of its ability.
+         */
+        RULE_THREAD_WARNING,
+        /**
+         * Indicates than an unsupported scenario was encountered in the main thread, and that an
+         * exception was thrown in response. This typically means that the process as a whole was
+         * forced to exit unsuccessfully.
+         */
+        MAIN_THREAD_EXCEPTION,
+        /**
+         * Indicates that an unsupported scenario was encountered in a rule executor thread, and
+         * that an exception was thrown in response. This typically means that rules can no longer
+         * meaningfully execute against the relevant path entrypoint, but other entrypoints can
+         * still be evaluated.
+         */
+        RULE_THREAD_EXCEPTION;
+    }
+
+    private TelemetryUtil() {}
+}
diff --git a/sfge/src/main/resources/log4j2.xml b/sfge/src/main/resources/log4j2.xml

Original file line number	Diff line number	Diff line change
`@@ -57,6 +57,7 @@ enum MessageHandler {`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`enum MessageType {`
	`60`	`+ TELEMETRY,`
`60`	`61`	`INFO,`
`61`	`62`	`WARNING,`
`62`	`63`	`ERROR`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@salesforce/sfdx-scanner",`
`3`	`3`	`"description": "Static code scanner that applies quality and security rules to Apex code, and provides feedback.",`
`4`		`- "version": "3.5.0",`
	`4`	`+ "version": "3.5.1",`
`5`	`5`	`"author": "ISV SWAT",`
`6`	`6`	`"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",`
`7`	`7`	`"dependencies": {`
Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ private static void visitComments(JorjeNode wrapper, CommentVisitor visitor) {`
`129`	`129`	`}`
`130`	`130`	`}`
`131`	`131`
`132`		`- public static class JorjeCompilationException extends SfgeRuntimeException {`
	`132`	`+ public static final class JorjeCompilationException extends SfgeRuntimeException {`
`133`	`133`	`JorjeCompilationException(String message) {`
`134`	`134`	`super(message);`
`135`	`135`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`package com.salesforce.exception;`
`2`	`2`
`3`	`3`	`/** Indicates a developer error where the same key was added to a map twice */`
`4`		`-public class DuplicateKeyException extends SfgeRuntimeException {`
	`4`	`+public final class DuplicateKeyException extends SfgeRuntimeException {`
`5`	`5`	`public DuplicateKeyException(Object key, Object previousEntry, Object newEntry) {`
`6`	`6`	`super("Duplicate keys. key=" + key + ", previous=" + previousEntry + ", new=" + newEntry);`
`7`	`7`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`package com.salesforce.exception;`
`2`	`2`
`3`	`3`	`/** Indicates a programming logic error, such as an item being initialized more than once. */`
`4`		`-public class ProgrammingException extends SfgeRuntimeException {`
	`4`	`+public final class ProgrammingException extends SfgeRuntimeException {`
`5`	`5`	`public ProgrammingException(String message) {`
`6`	`6`	`super(message);`
`7`	`7`	`}`