Fix escaping single quotes in env variable values (#16983)

Signed-off-by: Shrikant Sharat Kandula <shrikant@appsmith.com>

app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java

@@ -106,7 +106,7 @@ public class EnvManagerCEImpl implements EnvManagerCE {
      * respectively.
      */
     private static final Pattern ENV_VARIABLE_PATTERN = Pattern.compile(
-            "^(?<name>[A-Z\\d_]+)\\s*=\\s*(?<quote>[\"']?)(?<value>.*?)\\k<quote>$"
+            "^(?<name>[A-Z\\d_]+)\\s*=\\s*(?<value>.*)$"
     );
 
     private static final Set<String> VARIABLE_WHITELIST = Stream.of(EnvVariables.values())
@@ -185,25 +185,65 @@ public class EnvManagerCEImpl implements EnvManagerCE {
                         return line;
                     }
                     final String name = matcher.group("name");
-                    if (!changes.containsKey(name)) {
+                    return remainingChangedNames.remove(name)
-                        return line;
+                            ? String.format("%s=%s", name, escapeForShell(changes.get(name)))
-                    }
+                            : line;
-                    remainingChangedNames.remove(name);
-                    String safeValue = changes.get(name);
-                    if (safeValue.contains(" ") || safeValue.contains("?") || safeValue.contains("*") || safeValue.contains("#")) {
-                        safeValue = "'" + safeValue.replace("'", "'\"'\"'") + "'";
-                    }
-                    return String.format("%s=%s", name, safeValue);
                 })
                 .collect(Collectors.toList());
 
         for (final String name : remainingChangedNames) {
-            outLines.add(name + "=" + changes.get(name));
+            outLines.add(name + "=" + escapeForShell(changes.get(name)));
         }
 
         return outLines;
     }
 
+    private String escapeForShell(String input) {
+        if (org.apache.commons.lang3.StringUtils.containsAny(input, " ?*#'")) {
+            return ("'" + input.replace("'", "'\"'\"'") + "'")
+                    .replaceAll("^''", "")
+                    .replaceAll("''$", "");
+        }
+
+        return input;
+    }
+
+    private String unescapeFromShell(String input) {
+        final int len = input.length();
+        final StringBuilder valueBuilder = new StringBuilder();
+        Character inQuote = null;
+
+        for (int i = 0; i < len; ++i) {
+            final char c = input.charAt(i);
+
+            if (inQuote != null && inQuote == '\'') {
+                if (c == '\'') {
+                    inQuote = null;
+                } else {
+                    valueBuilder.append(c);
+                }
+
+            } else if (inQuote != null) {
+                // If `inQuote` is not null here, then it can only be the double-quote character.
+                // We don't do variable interpolation here, since we don't expect it to be present in the env file.
+                if (c == '"') {
+                    inQuote = null;
+                } else {
+                    valueBuilder.append(c);
+                }
+
+            } else if (c == '\'' || c == '"') {
+                inQuote = c;
+
+            } else {
+                valueBuilder.append(c);
+
+            }
+        }
+
+        return valueBuilder.toString();
+    }
+
     private Mono<Void> validateChanges(User user, Map<String, String> changes) {
         if (changes.containsKey(APPSMITH_ADMIN_EMAILS.name())) {
             String emailCsv = StringUtils.trimAllWhitespace(changes.get(APPSMITH_ADMIN_EMAILS.name()));
@@ -429,18 +469,7 @@ public class EnvManagerCEImpl implements EnvManagerCE {
                     if (matcher.matches()) {
                         final String name = matcher.group("name");
                         if (VARIABLE_WHITELIST.contains(name)) {
-                            String actualValue = matcher.group("value");
+                            data.put(name, unescapeFromShell(matcher.group("value")));
-                            final String quote = matcher.group("quote");
-                            if ("'".equals(quote)) {
-                                // Undo two common methods of escaping single quotes:
-                                actualValue = actualValue
-                                        .replace("'\"'\"'", "'")
-                                        .replace("'\\''", "'");
-                            } else if ("\"".equals(quote)) {
-                                // Undo escaped double quotes:
-                                actualValue = actualValue.replace("\\\"", "\"");
-                            }
-                            data.put(name, actualValue);
                         }
                     }
                 });

app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java

@@ -208,18 +208,6 @@ public class EnvManagerTest {
 
     }
 
-    public void parseTest() {
-
-        assertThat(envManager.parseToMap(
-                "APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL='second value'\n\nAPPSMITH_INSTANCE_NAME='third value'"
-        )).containsExactlyInAnyOrderEntriesOf(Map.of(
-                "APPSMITH_MONGODB_URI", "'first value'",
-                "APPSMITH_REDIS_URL", "'second value'",
-                "APPSMITH_INSTANCE_NAME", "'third value'"
-        ));
-
-    }
-
     @Test
     public void parseEmptyValues() {
 
@@ -252,6 +240,16 @@ public class EnvManagerTest {
 
     }
 
+    @Test
+    public void parseTestWithEscapes() {
+        assertThat(envManager.parseToMap(
+                "APPSMITH_ALLOWED_FRAME_ANCESTORS=\"'\"'none'\"'\"\nAPPSMITH_REDIS_URL='second\" value'\n"
+        )).containsExactlyInAnyOrderEntriesOf(Map.of(
+                "APPSMITH_ALLOWED_FRAME_ANCESTORS", "'none'",
+                "APPSMITH_REDIS_URL", "second\" value"
+        ));
+    }
+
     @Test
     public void disallowedVariable() {
         final String content = "APPSMITH_MONGODB_URI=first value\nDISALLOWED_NASTY_STUFF=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME=third value";
@@ -286,6 +284,25 @@ public class EnvManagerTest {
         );
     }
 
+    @Test
+    public void setValueWithQuotes() {
+        final String content = "APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL='quoted value'\n\nAPPSMITH_INSTANCE_NAME='third value'";
+
+        assertThat(envManager.transformEnvContent(
+                content,
+                Map.of(
+                        "APPSMITH_MONGODB_URI", "'just quotes'",
+                        "APPSMITH_DISABLE_TELEMETRY", "some quotes 'inside' it"
+                )
+        )).containsExactly(
+                "APPSMITH_MONGODB_URI=\"'\"'just quotes'\"'\"",
+                "APPSMITH_REDIS_URL='quoted value'",
+                "",
+                "APPSMITH_INSTANCE_NAME='third value'",
+                "APPSMITH_DISABLE_TELEMETRY='some quotes '\"'\"'inside'\"'\"' it'"
+        );
+    }
+
     @Test
     public void download_UserIsNotSuperUser_ThrowsAccessDenied() {
         User user = new User();