SONARIAC-2495 Fix NPE in ShellFormOverExecFormCheck (#659)

GitOrigin-RevId: 7a6bdc8323038202150323d2e3c9cb67d8ebec7b

Author: petertrr

iac-extensions/docker/src/main/java/org/sonar/iac/docker/checks/ShellFormOverExecFormCheck.java

@@ -23,7 +23,6 @@
 import org.sonar.iac.common.api.checks.CheckContext;
 import org.sonar.iac.common.api.checks.IacCheck;
 import org.sonar.iac.common.api.checks.InitContext;
-import org.sonar.iac.common.api.tree.Tree;
 import org.sonar.iac.docker.checks.utils.MultiStageBuildInspector;
 import org.sonar.iac.docker.tree.api.Body;
 import org.sonar.iac.docker.tree.api.CmdInstruction;
@@ -33,7 +32,6 @@
 import org.sonar.iac.docker.tree.api.EntrypointInstruction;
 import org.sonar.iac.docker.tree.api.ShellCode;
 import org.sonar.iac.docker.tree.api.ShellInstruction;
-import org.sonar.iac.docker.tree.api.Variable;
 
 import static org.sonar.iac.docker.tree.TreeUtils.getDockerImageName;
 import static org.sonar.iac.docker.tree.TreeUtils.getParentDockerImageName;
@@ -43,7 +41,10 @@ public class ShellFormOverExecFormCheck implements IacCheck {
 
   private static final String DEFAULT_MESSAGE = "Replace this shell form with exec form.";
   private static final String WRAPPING_SCRIPT_MESSAGE = "Consider wrapping this instruction in a script file and call it with exec form.";
-  private static final Pattern UNSUPPORTED_FEATURES_IN_EXEC_FORM = Pattern.compile("&&|\\|\\||\\||;");
+  /**
+   * Because exec form doesn't create a shell, it does not support chaining commands or using variables.
+   */
+  private static final Pattern UNSUPPORTED_FEATURES_IN_EXEC_FORM = Pattern.compile("&&|\\|\\||\\||;|\\$");
 
   private final ShellInstructionsInfo checkContext = new ShellInstructionsInfo();
   private MultiStageBuildInspector multiStageBuildInspector = null;
@@ -84,8 +85,8 @@ private void checkCommandInstructionForm(CheckContext ctx, CodeInstruction instr
     }
   }
 
-  private boolean hasAnyParentDockerImageWithShellInstruction(CodeInstruction transferInstruction) {
-    return getParentDockerImageName(transferInstruction)
+  private boolean hasAnyParentDockerImageWithShellInstruction(CodeInstruction instruction) {
+    return getParentDockerImageName(instruction)
       .map((String parentDockerImageName) -> {
         if (hasShellInstruction(parentDockerImageName)) {
           return true;
@@ -101,11 +102,11 @@ private boolean hasShellInstruction(String imageName) {
   }
 
   private static boolean containFeatureNotSupportedByExecForm(ShellCode<?> code) {
-    return UNSUPPORTED_FEATURES_IN_EXEC_FORM.matcher(code.originalSourceCode()).find();
-  }
-
-  private static boolean hasVariableReference(Tree tree) {
-    return tree instanceof Variable || tree.children().stream().anyMatch(ShellFormOverExecFormCheck::hasVariableReference);
+    var originalCode = code.originalSourceCode();
+    if (originalCode != null) {
+      return UNSUPPORTED_FEATURES_IN_EXEC_FORM.matcher(originalCode).find();
+    }
+    return false;
   }
 
   static class ShellInstructionsInfo {

iac-extensions/docker/src/test/resources/checks/ShellFormOverExecFormCheck/dockerfile

@@ -19,6 +19,27 @@ HEALTHCHECK --interval=30s CMD node healthcheck.js
 
 HEALTHCHECK CMD curl --fail http://localhost:8080/healthcheck || exit 1
 
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo $message
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo ${message-default}
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo ${message:=default}
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo ${message:+var}
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo ${message:7}
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo ${message:7:0}
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo "$message"
+
 # FNs: exec form with explicit shell invocation are not supported
 CMD ["sh", "-c", "echo $message"]
 CMD ["sh", "-c", "echo ${message-default}"]
@@ -34,6 +55,21 @@ CMD ["sh", "-c", "echo \"Welcome\" ; echo \"Goodbye\""]
 CMD ["sh", "-c", "echo \"Welcome\" | echo \"Goodbye\""]
 CMD ["sh", "-c", "echo \"Welcome\" && echo \"Goodbye\" || echo \"Goodbye\" ; echo \"Goodbye\" | echo \"Goodbye\""]
 
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo "Welcome" && echo "Goodbye"
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo "Welcome" || echo "Goodbye"
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo "Welcome" ; echo "Goodbye"
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo "Welcome" | echo "Goodbye"
+
+# Noncompliant@+1 {{Consider wrapping this instruction in a script file and call it with exec form.}}
+CMD echo "Welcome" && echo "Goodbye" || echo "Goodbye" ; echo "Goodbye" | echo "Goodbye"
+
 # Noncompliant@+1
 CMD this is a very \
   long multi-line  \
@@ -43,13 +79,14 @@ CMD this is a very \
 CMD this is a very long instruction that is settled on a single line aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 #   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-
 RUN echo "Welcome!"
 CMD ["echo", "Welcome!"]
 ENTRYPOINT ["echo", "Welcome!"]
 
 SHELL ["sh", "-c"]
 # Compliant: if we met a SHELL instruction before, then we consider it's a conscious decision and don't raise an issue
+CMD echo "Welcome!"
+CMD echo "Welcome" && echo "Goodbye" || echo "Goodbye" ; echo "Goodbye" | echo "Goodbye"
 CMD ["echo", "\"Welcome!\""]
 CMD ["sh", "-c", "echo \"Welcome\" && echo \"Goodbye\" || echo \"Goodbye\" ; echo \"Goodbye\" | echo \"Goodbye\""]
 
@@ -82,3 +119,10 @@ FROM scratch
 SHELL ["sh", "-c"]
 # Compliant: extra use case with a SHELL instruction in an image without alias
 CMD ["echo", "\"Welcome!\""]
+
+ FROM scratch AS builder2
+# Invalid exec form: no quotes around python. Parsed as shell form.
+# Noncompliant@+1
+CMD [python, /usr/src/app/app.py]
+# Noncompliant@+1
+ENTRYPOINT [top, -b]