From ad584398164c93bbd9b0ad5b004e60b7a48328b2 Mon Sep 17 00:00:00 2001 From: saksham2105 Date: Thu, 11 Jul 2024 12:41:28 +0530 Subject: [PATCH] Python as Scripting language for inline task --- core/build.gradle | 2 + .../core/events/ScriptEvaluator.java | 28 +++ .../execution/evaluators/PythonEvaluator.java | 128 ++++++++++ .../python/untrusted_code_validator.py | 44 ++++ .../evaluators/PythonEvaluatorTest.java | 225 ++++++++++++++++++ dependencies.gradle | 1 + 6 files changed, 428 insertions(+) create mode 100644 core/src/main/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluator.java create mode 100644 core/src/main/resources/python/untrusted_code_validator.py create mode 100644 core/src/test/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluatorTest.java diff --git a/core/build.gradle b/core/build.gradle index beef8f7a2..2bba0d17c 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -42,6 +42,8 @@ dependencies { implementation "com.github.ben-manes.caffeine:caffeine" implementation "org.openjdk.nashorn:nashorn-core:15.4" + implementation "org.python:jython-slim:${revJython}" + // JAXB is not bundled with Java 11, dependencies added explicitly // These are needed by Apache BVAL diff --git a/core/src/main/java/com/netflix/conductor/core/events/ScriptEvaluator.java b/core/src/main/java/com/netflix/conductor/core/events/ScriptEvaluator.java index c77f3860d..8da9c9c6c 100644 --- a/core/src/main/java/com/netflix/conductor/core/events/ScriptEvaluator.java +++ b/core/src/main/java/com/netflix/conductor/core/events/ScriptEvaluator.java @@ -12,12 +12,16 @@ */ package com.netflix.conductor.core.events; +import java.util.Map; + import javax.script.Bindings; import javax.script.ScriptEngine; import javax.script.ScriptEngineManager; import javax.script.ScriptException; import org.openjdk.nashorn.api.scripting.NashornScriptEngineFactory; +import org.python.core.PyObject; +import org.python.util.PythonInterpreter; public class ScriptEvaluator { @@ -48,12 +52,36 @@ public static Boolean evalBool(String script, Object input) throws ScriptExcepti * @return Generic object, the result of the evaluated expression. */ public static Object eval(String script, Object input) throws ScriptException { + if (input instanceof Map) { + Map inputs = (Map) input; + if (inputs.containsKey("evaluatorType") + && inputs.get("evaluatorType").toString().equals("python")) { + return evalPython(script, input); + } + } initEngine(false); Bindings bindings = engine.createBindings(); bindings.put("$", input); return engine.eval(script, bindings); } + /** + * Evaluates the script with the help of input provided. Set environment variable using Jython + * + * @param script Script to be evaluated. + * @param input Input parameters. + * @throws ScriptException + * @return Generic object, the result of the evaluated expression. + */ + private static Object evalPython(String script, Object input) throws ScriptException { + Map inputs = (Map) input; + String outputIdentifier = inputs.get("outputIdentifier").toString(); + PythonInterpreter interpreter = new PythonInterpreter(); + interpreter.exec(script); + PyObject result = interpreter.get(outputIdentifier); + return result.toString(); + } + // to mock in a test public static String getEnv(String name) { return System.getenv(name); diff --git a/core/src/main/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluator.java b/core/src/main/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluator.java new file mode 100644 index 000000000..0410c593c --- /dev/null +++ b/core/src/main/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluator.java @@ -0,0 +1,128 @@ +/* + * Copyright 2024 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package com.netflix.conductor.core.execution.evaluators; + +import java.io.BufferedReader; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javax.script.ScriptException; + +import org.python.core.PyObject; +import org.python.util.PythonInterpreter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +import com.netflix.conductor.core.events.ScriptEvaluator; +import com.netflix.conductor.core.exception.TerminateWorkflowException; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.jayway.jsonpath.DocumentContext; +import com.jayway.jsonpath.JsonPath; + +@Component(PythonEvaluator.NAME) +public class PythonEvaluator implements Evaluator { + + public static final String NAME = "python"; + private static final Logger LOGGER = LoggerFactory.getLogger(PythonEvaluator.class); + private static final PythonInterpreter pythonInterpreter = new PythonInterpreter(); + private static final ObjectMapper objectMapper = new ObjectMapper(); + private static final Pattern pattern = + Pattern.compile("\\$\\.([a-zA-Z0-9_\\.]+)"); // Regex Pattern to find all occurrences of + + // $.[variable] or $.[nested.property] in script + + @Override + public Object evaluate(String script, Object input) { + LOGGER.debug("Python evaluator -- script: {}", script); + try { + script = script.trim(); + Map inputs = (Map) input; + script = replaceVariablesInScript(script, inputs); + boolean scriptTrusted = isScriptTrusted(script); + if (!scriptTrusted) { + throw new ScriptException( + "Script execution is restricted due to policy violations."); + } + Object result = ScriptEvaluator.eval(script, input); + LOGGER.debug("Python evaluator -- result: {}", result); + return result; + } catch (Exception e) { + LOGGER.error("Error while evaluating script: {}", script, e); + throw new TerminateWorkflowException(e.getMessage()); + } + } + + private static boolean isScriptTrusted(String script) { + try (InputStream inputStream = + PythonEvaluator.class + .getClassLoader() + .getResourceAsStream("python/untrusted_code_validator.py"); + InputStreamReader isr = new InputStreamReader(inputStream, "UTF-8"); + BufferedReader br = new BufferedReader(isr)) { + if (inputStream == null) { + throw new FileNotFoundException( + String.format( + "Resource file %s not found.", "untrusted_code_validator.py")); + } + StringBuilder stringBuilder = new StringBuilder(); + String line; + while ((line = br.readLine()) != null) { + stringBuilder.append(line).append("\n"); + } + String untrustedCode = "'''" + script + "'''"; + String pythonScript = stringBuilder.toString().replace("${code}", untrustedCode); + pythonInterpreter.exec(pythonScript); + PyObject result = pythonInterpreter.get("codeTrusted"); + return result.toString().equals("True"); + } catch (Exception e) { + LOGGER.error("Some error encountered validating python script : {} as : {}", script, e); + return false; + } + } + + public String replaceVariablesInScript(String script, Map inputs) + throws IOException { + String inputJsonString = objectMapper.writeValueAsString(inputs); + DocumentContext jsonContext = JsonPath.parse(inputJsonString); + + // Use a Matcher to process all matches in the script + Matcher matcher = pattern.matcher(script); + StringBuffer updatedScript = new StringBuffer(); + + while (matcher.find()) { + String jsonPath = matcher.group(1); + try { + Object value = jsonContext.read("$." + jsonPath); + // Create the replacement string for the variable + String replacement = value != null ? value.toString() : ""; + // Escape $ to avoid issues in replacement string + String safeReplacement = replacement.replace("$", "\\$"); + // Append the new script with the replaced variable + matcher.appendReplacement(updatedScript, safeReplacement); + } catch (Exception e) { + // In case of an invalid JsonPath expression, keep the original placeholder + matcher.appendReplacement(updatedScript, "\\$." + jsonPath); + } + } + // Append the remaining part of the script after the last match + matcher.appendTail(updatedScript); + return updatedScript.toString(); + } +} diff --git a/core/src/main/resources/python/untrusted_code_validator.py b/core/src/main/resources/python/untrusted_code_validator.py new file mode 100644 index 000000000..23333e806 --- /dev/null +++ b/core/src/main/resources/python/untrusted_code_validator.py @@ -0,0 +1,44 @@ +### This script contains code to check if the code to run in inline_task +### is safe and not vulnerable +### This is being executed by @PythonEvaluator.java +### Trusted code must not have import statements and some restricted built_in functions + +import ast + +class ConductorBuiltInRestrictor(ast.NodeVisitor): + restricted_builtins = { + 'eval', 'exec', 'compile', 'execfile', 'del' ,'open', 'close', 'read', 'write', 'close', 'readlines', 'input', 'raw_input', 'open', '__import__', '__file__','__package__' + ,'__path__','__spec__','__doc__','__module__','__loader__','__annotations__','__builtins__','__cached__','__build_class__', 'getattr', + 'setattr', 'delattr', 'globals', 'locals', 'vars', 'dir', 'type', 'id', + 'help', 'super', 'object', 'staticmethod', 'classmethod', 'property', + 'basestring', 'bytearray', 'bytes', 'callable', 'classmethod', 'complex', + 'delattr', 'dict', 'enumerate', 'eval', 'filter', 'frozenset', 'getattr', + 'globals', 'hasattr', 'hash', 'help', 'id', 'input', 'isinstance', + 'issubclass', 'iter', 'locals', 'map', 'memoryview', + 'next', 'object', 'property', 'repr', 'reversed', + 'setattr', 'sorted', 'staticmethod', 'vars', 'zip', 'reload', 'exit', 'quit' + } + + def visit_Import(self, node): + raise ImportError("Import statements are not allowed.") + + def visit_ImportFrom(self, node): + raise ImportError("Import statements are not allowed.") + + def visit_Call(self, node): + if isinstance(node.func, ast.Name) and node.func.id in self.restricted_builtins: + raise ImportError("Usage of '%s' is not allowed." % node.func.id) + self.generic_visit(node) + + def isCodeTrusted(self, code): + try: + tree = ast.parse(code) + self.visit(tree) + return True + except ImportError as e: + return False + +# Example usage +restrictor = ConductorBuiltInRestrictor() +codeTrusted = restrictor.isCodeTrusted(${code}) # Should raise an ImportError +codeTrusted \ No newline at end of file diff --git a/core/src/test/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluatorTest.java b/core/src/test/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluatorTest.java new file mode 100644 index 000000000..c86750622 --- /dev/null +++ b/core/src/test/java/com/netflix/conductor/core/execution/evaluators/PythonEvaluatorTest.java @@ -0,0 +1,225 @@ +/* + * Copyright 2024 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package com.netflix.conductor.core.execution.evaluators; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import com.netflix.conductor.core.exception.TerminateWorkflowException; + +import static org.junit.Assert.assertEquals; + +public class PythonEvaluatorTest { + + private Evaluator pythonEvaluator = new PythonEvaluator(); + private static final String POLICY_VIOLATION_MESSAGE = + "Script execution is restricted due to policy violations."; + + @Test + public void testImportsRestrictionOs() { + String testPythonScript = + "import os\n" + + "os.system('rm -rf /')\n" + + "print(\"Test statement\")"; // Malicious: It deletes all files and + // directories in the root filesystem + Object result = null; + try { + result = pythonEvaluator.evaluate(testPythonScript, Map.of()); + } catch (TerminateWorkflowException terminateWorkflowException) { + assertEquals(terminateWorkflowException.getMessage(), POLICY_VIOLATION_MESSAGE); + } + } + + @Test + public void testImportsRestrictionSys() { + String testPythonScript = "import sys\n" + "import subprocess"; + Object result = null; + try { + result = pythonEvaluator.evaluate(testPythonScript, Map.of()); + } catch (TerminateWorkflowException terminateWorkflowException) { + assertEquals(terminateWorkflowException.getMessage(), POLICY_VIOLATION_MESSAGE); + } + } + + @Test + public void testRestrictedInBuiltFunctionEval() { + String testPythonScript = + "print(eval('1+1'))"; // Eval is a malicious function and code lead to remote code + // executions + Object result = null; + try { + result = pythonEvaluator.evaluate(testPythonScript, Map.of()); + } catch (TerminateWorkflowException terminateWorkflowException) { + assertEquals(terminateWorkflowException.getMessage(), POLICY_VIOLATION_MESSAGE); + } + } + + @Test + public void testRestrictedInBuiltFunctionExec() { + String testPythonScript = + "code = '''\n" + + "def add(a, b):\n" + + " return a + b\n" + + "\n" + + "result = add(1, 1)\n" + + "print(result)'''\n" + + "\n" + + "exec(code)"; // Exec is a malicious function and code lead to remote code + // executions + Object result = null; + try { + result = pythonEvaluator.evaluate(testPythonScript, Map.of()); + } catch (TerminateWorkflowException terminateWorkflowException) { + assertEquals(terminateWorkflowException.getMessage(), POLICY_VIOLATION_MESSAGE); + } + } + + @Test + public void testRestrictedInBuiltFunctionOpen() { + String testPythonScript = + "with open('example.txt', 'r') as file:\n" + + " content = file.read()\n" + + " print(content)\n"; // File crud operations should be restricted + Object result = null; + try { + result = pythonEvaluator.evaluate(testPythonScript, Map.of()); + } catch (TerminateWorkflowException terminateWorkflowException) { + assertEquals(terminateWorkflowException.getMessage(), POLICY_VIOLATION_MESSAGE); + } + } + + @Test + public void testSimpleAddFunction() { + String testPythonScript = + "def add(a, b):\n" + + " return a+b\n" + + "\n" + + "sum = add(10, 2)\n" + + "sum"; // File crud operations should be restricted + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "sum"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "12"); + } + + @Test + public void testPythonLoop() { + String testPythonScript = + "arr = [1, 2, 3, 4, 5]\n" + + "sumOfEven = 0\n" + + "for i in range(len(arr)):\n" + + " if arr[i] % 2 == 0:\n" + + " sumOfEven = sumOfEven + arr[i]\n" + + "sumOfEven"; + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "sumOfEven"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "6"); + } + + @Test + public void testReplacingIntegerParameters() { + String testPythonScript = + "def isEven():\n" + + " num = $.num\n" + + " return num % 2 == 0\n" + + "\n" + + "evenFlag = isEven();\n" + + "evenFlag"; + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("num", 2); // $.num is a parameter in above test script + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "evenFlag"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "True"); // True is boolean representation in python + } + + @Test + public void testReplacingArrays() { + String testPythonScript = + "arr = $.arr\n" + + "sum = 0\n" + + "for item in arr:\n" + + " sum = sum + item\n" + + "sum"; + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("arr", List.of(1, 2, 3, 4)); // $.arr is an array parameter in above test script + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "sum"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "10"); + } + + @Test + public void testReplacingStrings() { + String testPythonScript = + "name = \"$.name\"\n" + + // ' We used $.name inside "" since we directly replace variable with + // value ' + "name"; + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("name", "Foo"); // $.name is a parameter in above test script + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "name"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "Foo"); + } + + @Test + public void testReplacingNestedObjects() { + String testPythonScript = + "def greet():\n" + + " name = \"$.jsonObj.name\"\n" + + " age = $.jsonObj.age\n" + + " return \"Greetings \" + name + \" having age = \" + str(age)\n" + + "\n" + + "message = greet()\n" + + "message"; + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("jsonObj", Map.of("name", "John", "age", 27)); + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "message"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "Greetings John having age = 27"); + } + + @Test + public void testReplacingNestedObjectWithList() { + String testPythonScript = + "def greet():\n" + + " name = $.jsonObj.var[0]\n" + + // In case of list we don't wrap variable inside "" + " return \"Greetings \" + name\n" + + "\n" + + "message = greet()\n" + + "message"; + Map inputs = new HashMap<>(); + inputs.put("evaluatorType", "python"); + inputs.put("jsonObj", Map.of("var", List.of("Foo", "John"))); + inputs.put("expression", testPythonScript); + inputs.put("outputIdentifier", "message"); + Object result = pythonEvaluator.evaluate(testPythonScript, inputs); + assertEquals(result.toString(), "Greetings Foo"); + } +} diff --git a/dependencies.gradle b/dependencies.gradle index 93637ab1b..6438dfaf0 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -69,5 +69,6 @@ ext { revNats = '2.16.14' revStan = '2.2.3' revFlyway = '9.0.4' + revJython = '2.7.3' }