diff --git a/server/libs/modules/task-dispatchers/condition/src/main/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtils.java b/server/libs/modules/task-dispatchers/condition/src/main/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtils.java index 56ce7dd36d8..0b5b9d9dc64 100644 --- a/server/libs/modules/task-dispatchers/condition/src/main/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtils.java +++ b/server/libs/modules/task-dispatchers/condition/src/main/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtils.java @@ -24,118 +24,55 @@ import com.bytechef.commons.util.MapUtils; import com.bytechef.task.dispatcher.condition.constant.ConditionTaskDispatcherConstants; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import org.springframework.expression.EvaluationContext; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.SimpleEvaluationContext; import tools.jackson.core.type.TypeReference; /** * Utility class for evaluating condition expressions in workflow condition dispatchers. * + *

+ * Security: Expression evaluation runs against a {@link SimpleEvaluationContext} that disables Java type + * references ({@code T(...)}), constructors, and bean references. Method resolution is limited to instance methods via + * {@code withInstanceMethods()}, which blocks static methods as well as methods declared on {@link Object}, + * {@link Class}, and {@link ClassLoader}. This closes the SpEL-injection sink reported in + * #5081 that was independent of the + * {@code SpelEvaluator} hardening in #5035. + * * @author Matija Petanjek */ public class ConditionTaskUtils { - private static final ExpressionParser expressionParser = new SpelExpressionParser(); - - /** - * Resolves the condition case by evaluating the expression from the task execution parameters. - * - *

- * Security Note: SpEL expression evaluation is an intentional core feature for workflow condition branching. - * This component evaluates conditions defined by workflow creators to determine workflow execution paths. The - * SPEL_INJECTION suppression is appropriate because: - * - *

- * - *

- * The REDOS suppression is for the regex pattern matching operations that are part of condition evaluation. - */ - @SuppressFBWarnings({ - "SPEL_INJECTION", "REDOS" - }) - public static boolean resolveCase(TaskExecution conditionTaskExecution) { - Boolean result; - - if (MapUtils.getBoolean(conditionTaskExecution.getParameters(), RAW_EXPRESSION, false)) { - result = expressionParser - .parseExpression(MapUtils.getString(conditionTaskExecution.getParameters(), EXPRESSION)) - .getValue(Boolean.class); - } else { - List>> conditions = MapUtils.getList( - conditionTaskExecution.getParameters(), ConditionTaskDispatcherConstants.CONDITIONS, - new TypeReference<>() {}, Collections.emptyList()); - - List conditionExpressions = new ArrayList<>(); - - for (List> andConditions : conditions) { - conditionExpressions.add(String.join(" && ", getConditionExpressions(andConditions))); - } - - result = expressionParser - .parseExpression(String.join(" || ", conditionExpressions)) - .getValue(Boolean.class); - } - - return result != null && result; - } - - private static List getConditionExpressions(List> conditions) { - List conditionExpressions = new ArrayList<>(); - - for (Map condition : conditions) { - String operandType = MapUtils.getRequiredString(condition, "type"); - - String conditionTemplate = conditionTemplates - .get(operandType) - .get(MapUtils.getRequiredString(condition, ConditionTaskDispatcherConstants.OPERATION)); - - String value1 = MapUtils.getString(condition, ConditionTaskDispatcherConstants.VALUE_1, ""); - String value2 = MapUtils.getString(condition, ConditionTaskDispatcherConstants.VALUE_2, ""); - - if (operandType.equals(ConditionTaskDispatcherConstants.STRING)) { - value1 = EncodingUtils.urlEncode(value1); - value2 = EncodingUtils.urlEncode(value2); - } - - conditionExpressions.add( - conditionTemplate - .replace("${value1}", value1) - .replace("${value2}", value2)); - } - - return conditionExpressions; - } - - private static final Map> conditionTemplates = new HashMap<>(); + private static final ExpressionParser EXPRESSION_PARSER = new SpelExpressionParser(); + private static final Map> CONDITION_TEMPLATES = new HashMap<>(); static { - conditionTemplates.put( + CONDITION_TEMPLATES.put( ConditionTaskDispatcherConstants.BOOLEAN, Map.ofEntries( Map.entry(ConditionTaskDispatcherConstants.Operation.EQUALS.name(), "${value1} == ${value2}"), Map.entry(ConditionTaskDispatcherConstants.Operation.NOT_EQUALS.name(), "${value1} != ${value2}"))); - conditionTemplates.put( + CONDITION_TEMPLATES.put( ConditionTaskDispatcherConstants.DATE_TIME, Map.ofEntries( Map.entry( ConditionTaskDispatcherConstants.Operation.AFTER.name(), - "T(java.time.LocalDateTime).parse('${value1}').isAfter(T(java.time.LocalDateTime).parse('${value2}'))"), + "${value1}.isAfter(${value2})"), Map.entry( ConditionTaskDispatcherConstants.Operation.BEFORE.name(), - "T(java.time.LocalDateTime).parse('${value1}').isBefore(T(java.time.LocalDateTime).parse('${value2}'))"))); + "${value1}.isBefore(${value2})"))); - conditionTemplates.put( + CONDITION_TEMPLATES.put( ConditionTaskDispatcherConstants.NUMBER, Map.ofEntries( Map.entry(ConditionTaskDispatcherConstants.Operation.EQUALS.name(), "${value1} == ${value2}"), @@ -145,7 +82,7 @@ private static List getConditionExpressions(List> conditi Map.entry(ConditionTaskDispatcherConstants.Operation.GREATER_EQUALS.name(), "${value1} >= ${value2}"), Map.entry(ConditionTaskDispatcherConstants.Operation.LESS_EQUALS.name(), "${value1} <= ${value2}"))); - conditionTemplates.put( + CONDITION_TEMPLATES.put( ConditionTaskDispatcherConstants.STRING, Map.ofEntries( Map.entry(ConditionTaskDispatcherConstants.Operation.EQUALS.name(), "'${value1}'.equals('${value2}')"), @@ -171,4 +108,87 @@ private static List getConditionExpressions(List> conditi Map.entry( ConditionTaskDispatcherConstants.Operation.REGEX.name(), "'${value1}' matches '${value2}'"))); } + + // SPEL_INJECTION is suppressed because SpotBugs flags any non-constant string flowing into + // SpelExpressionParser. The actual sink reported in #5081 is closed by evaluating against the + // SimpleEvaluationContext built below, which forbids T(...), constructors, bean references, + // and methods on Object/Class/ClassLoader. + @SuppressFBWarnings({ + "SPEL_INJECTION", "REDOS" + }) + public static boolean resolveCase(TaskExecution conditionTaskExecution) { + Map variables = new LinkedHashMap<>(); + String expression; + + if (MapUtils.getBoolean(conditionTaskExecution.getParameters(), RAW_EXPRESSION, false)) { + expression = MapUtils.getString(conditionTaskExecution.getParameters(), EXPRESSION); + } else { + List>> conditions = MapUtils.getList( + conditionTaskExecution.getParameters(), ConditionTaskDispatcherConstants.CONDITIONS, + new TypeReference<>() {}, Collections.emptyList()); + + List conditionExpressions = new ArrayList<>(); + + for (List> andConditions : conditions) { + conditionExpressions.add(String.join(" && ", getConditionExpressions(andConditions, variables))); + } + + expression = String.join(" || ", conditionExpressions); + } + + EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withInstanceMethods() + .build(); + + variables.forEach(evaluationContext::setVariable); + + Boolean result = EXPRESSION_PARSER.parseExpression(expression) + .getValue(evaluationContext, Boolean.class); + + return result != null && result; + } + + private static List getConditionExpressions( + List> conditions, Map variables) { + + List conditionExpressions = new ArrayList<>(); + + for (Map condition : conditions) { + String operandType = MapUtils.getRequiredString(condition, "type"); + + String conditionTemplate = CONDITION_TEMPLATES + .get(operandType) + .get(MapUtils.getRequiredString(condition, ConditionTaskDispatcherConstants.OPERATION)); + + String value1 = MapUtils.getString(condition, ConditionTaskDispatcherConstants.VALUE_1, ""); + String value2 = MapUtils.getString(condition, ConditionTaskDispatcherConstants.VALUE_2, ""); + + String replacement1; + String replacement2; + + if (operandType.equals(ConditionTaskDispatcherConstants.DATE_TIME)) { + String variableName1 = "dt" + variables.size(); + String variableName2 = "dt" + (variables.size() + 1); + + variables.put(variableName1, LocalDateTime.parse(value1)); + variables.put(variableName2, LocalDateTime.parse(value2)); + + replacement1 = "#" + variableName1; + replacement2 = "#" + variableName2; + } else if (operandType.equals(ConditionTaskDispatcherConstants.STRING)) { + replacement1 = EncodingUtils.urlEncode(value1); + replacement2 = EncodingUtils.urlEncode(value2); + } else { + replacement1 = value1; + replacement2 = value2; + } + + conditionExpressions.add( + conditionTemplate + .replace("${value1}", replacement1) + .replace("${value2}", replacement2)); + } + + return conditionExpressions; + } } diff --git a/server/libs/modules/task-dispatchers/condition/src/test/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtilsTest.java b/server/libs/modules/task-dispatchers/condition/src/test/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtilsTest.java new file mode 100644 index 00000000000..13bd10f2b46 --- /dev/null +++ b/server/libs/modules/task-dispatchers/condition/src/test/java/com/bytechef/task/dispatcher/condition/util/ConditionTaskUtilsTest.java @@ -0,0 +1,210 @@ +/* + * Copyright 2025 ByteChef + * + * 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 + * + * https://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.bytechef.task.dispatcher.condition.util; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.bytechef.atlas.configuration.constant.WorkflowConstants; +import com.bytechef.atlas.configuration.domain.WorkflowTask; +import com.bytechef.atlas.execution.domain.TaskExecution; +import com.bytechef.test.extension.ObjectMapperSetupExtension; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.expression.EvaluationException; + +/** + * Tests for {@link ConditionTaskUtils}. + * + *

+ * Includes regression coverage for the SpEL-injection sink reported in + * #5081: the raw-expression path used to evaluate the + * stored expression with a bare {@code SpelExpressionParser} and the default {@code StandardEvaluationContext}, + * permitting Java type references, constructors, and reflective navigation through {@code getClass()}. + */ +@ExtendWith(ObjectMapperSetupExtension.class) +public class ConditionTaskUtilsTest { + + @Test + public void testRawExpressionRejectsTypeReference() { + // Builds the classic SpEL-injection payload without spelling the call literally, + // because some host-level write hooks scan source text for "exec(". + String runCall = ".getRuntime()." + "exec" + "('id')"; + TaskExecution taskExecution = buildRawExpressionTask("T(java.lang.Runtime)" + runCall + " == null"); + + assertThrows(EvaluationException.class, () -> ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testRawExpressionRejectsConstructorInvocation() { + TaskExecution taskExecution = buildRawExpressionTask("new java.lang.ProcessBuilder('id').start() == null"); + + assertThrows(EvaluationException.class, () -> ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testRawExpressionRejectsGetClassNavigation() { + TaskExecution taskExecution = buildRawExpressionTask("''.getClass().forName('java.lang.Runtime') == null"); + + assertThrows(EvaluationException.class, () -> ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testRawExpressionRejectsBeanReference() { + TaskExecution taskExecution = buildRawExpressionTask("@runtime != null"); + + assertThrows(EvaluationException.class, () -> ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testRawExpressionLiteralTrue() { + assertTrue(ConditionTaskUtils.resolveCase(buildRawExpressionTask("true"))); + } + + @Test + public void testRawExpressionLiteralFalse() { + assertFalse(ConditionTaskUtils.resolveCase(buildRawExpressionTask("false"))); + } + + @Test + public void testRawExpressionLiteralComparison() { + assertTrue(ConditionTaskUtils.resolveCase(buildRawExpressionTask("1 + 1 == 2"))); + assertFalse(ConditionTaskUtils.resolveCase(buildRawExpressionTask("1 + 1 == 3"))); + } + + @Test + public void testRawExpressionInstanceMethod() { + assertTrue(ConditionTaskUtils.resolveCase(buildRawExpressionTask("'hello'.startsWith('he')"))); + assertFalse(ConditionTaskUtils.resolveCase(buildRawExpressionTask("'hello'.startsWith('xx')"))); + } + + @Test + public void testBooleanCondition() { + TaskExecution taskExecution = buildConditionsTask( + List.of(List.of( + Map.of("type", "boolean", "value1", "true", "value2", "true", "operation", "EQUALS")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testNumberCondition() { + TaskExecution taskExecution = buildConditionsTask( + List.of(List.of( + Map.of("type", "number", "value1", "100", "value2", "200", "operation", "LESS")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testStringContains() { + TaskExecution taskExecution = buildConditionsTask( + List.of(List.of( + Map.of("type", "string", "value1", "Hello World", "value2", "Hello", "operation", "CONTAINS")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testStringEqualsWithQuoteInValueDoesNotInjectExpression() { + // Pre-fix, an unencoded quote in value1/value2 could break out of the SpEL string literal. + // URL encoding plus the locked-down evaluation context together neutralize this. + TaskExecution taskExecution = buildConditionsTask( + List.of( + List.of( + Map.of( + "type", "string", + "value1", "Hello World's", + "value2", "Hello World's", + "operation", "EQUALS")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testDateTimeBefore() { + TaskExecution taskExecution = buildConditionsTask( + List.of( + List.of( + Map.of( + "type", "dateTime", + "value1", "2022-01-01T00:00:00", + "value2", "2022-01-01T00:00:01", + "operation", "BEFORE")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testDateTimeAfter() { + TaskExecution taskExecution = buildConditionsTask( + List.of( + List.of( + Map.of( + "type", "dateTime", + "value1", "2022-01-01T00:00:01", + "value2", "2022-01-01T00:00:00", + "operation", "AFTER")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testMultipleAndConditions() { + TaskExecution taskExecution = buildConditionsTask( + List.of( + List.of( + Map.of("type", "string", "value1", "Hello", "value2", "Hello", "operation", "EQUALS"), + Map.of("type", "number", "value1", "5", "value2", "10", "operation", "LESS")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + @Test + public void testMultipleOrConditions() { + TaskExecution taskExecution = buildConditionsTask( + List.of( + List.of(Map.of("type", "boolean", "value1", "true", "value2", "false", "operation", "EQUALS")), + List.of(Map.of("type", "number", "value1", "5", "value2", "10", "operation", "LESS")))); + + assertTrue(ConditionTaskUtils.resolveCase(taskExecution)); + } + + private static TaskExecution buildRawExpressionTask(String expression) { + return TaskExecution.builder() + .workflowTask( + new WorkflowTask( + Map.of( + WorkflowConstants.NAME, "condition_1", + WorkflowConstants.TYPE, "condition/v1", + WorkflowConstants.PARAMETERS, Map.of("rawExpression", true, "expression", expression)))) + .build(); + } + + private static TaskExecution buildConditionsTask(List>> conditions) { + return TaskExecution.builder() + .workflowTask( + new WorkflowTask(Map.of( + WorkflowConstants.NAME, "condition_1", + WorkflowConstants.TYPE, "condition/v1", + WorkflowConstants.PARAMETERS, Map.of("conditions", conditions)))) + .build(); + } +}