You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by sh...@apache.org on 2026/09/24 07:11:30 UTC
(unomi) branch bound-diagnostic-and-script-caches updated (9a4f6b65b -> e683891d1)
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a change to branch bound-diagnostic-and-script-caches
in repository https://gitbox.apache.org/repos/asf/unomi.git
omit 9a4f6b65b Bound diagnostic type tracking and the MVEL expression cache.
omit 1ee5c319d Deny public MVEL eval APIs from the scripting class loader.
omit cdf479c2e Keep personalization fallback when mixed contents include a script value.
omit ecb2b3c82 Write filtered personalization contents back onto the request.
omit 81504c791 UNOMI-978: Gate cross-profile merge and systemProperties writes on a trusted caller
add ab028177a UNOMI-982: Drop a condition whose optional parameter the context does not supply (#864)
add aab6c9e5e Keep Jetty client types out of GraphQLWebSocketIT method signatures (#882)
add 668a15fbf UNOMI-976: Restrict Groovy action upload to system administrators (#852)
add ce3eff265 UNOMI-978: Gate cross-profile merge and systemProperties writes on a trusted caller (#854)
add a51bf3c9e Write filtered personalization contents back onto the request (#876)
add cf52160eb Deny public MVEL eval APIs from the scripting class loader (#877)
new e683891d1 Bound diagnostic type tracking and the MVEL expression cache.
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (9a4f6b65b)
\
N -- N -- N refs/heads/bound-diagnostic-and-script-caches (e683891d1)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
THREAT_MODEL.md | 5 +-
.../karaf-kar/src/main/feature/feature.xml | 2 +
extensions/groovy-actions/rest/pom.xml | 10 +
.../groovy/actions/rest/GroovyActionsEndPoint.java | 5 +
.../rest/GroovyActionsEndPointRoleTest.java} | 28 +--
.../services/impl/GroovyActionsServiceImpl.java | 23 +-
.../impl/GroovyActionsServiceImplTest.java | 49 ++++
...Action.groovy => fieldInitializerAction.groovy} | 22 +-
.../test/java/org/apache/unomi/itests/AllITs.java | 1 +
.../test/java/org/apache/unomi/itests/BaseIT.java | 17 ++
.../test/java/org/apache/unomi/itests/BasicIT.java | 7 +-
.../org/apache/unomi/itests/ContextServletIT.java | 4 +
.../apache/unomi/itests/CorePersistenceITs.java | 1 +
.../GroovyActionsEndpointRoleSecurityIT.java | 113 +++++++++
.../org/apache/unomi/itests/ProfileMergeIT.java | 20 +-
.../unomi/itests/PropertiesUpdateActionIT.java | 69 ++++--
.../unomi/itests/graphql/GraphQLWebSocketIT.java | 29 ++-
manual/src/main/asciidoc/builtin-event-types.adoc | 11 +-
manual/src/main/asciidoc/configuration.adoc | 28 ++-
.../asciidoc/migrations/migrate-3.0-to-3.1.adoc | 3 +-
manual/src/main/asciidoc/recipes.adoc | 6 +-
manual/src/main/asciidoc/samples/login-sample.adoc | 6 +-
.../main/resources/etc/custom.system.properties | 10 +-
.../ConditionESQueryBuilderDispatcher.java | 18 +-
.../ConditionOSQueryBuilderDispatcher.java | 18 +-
.../spi/conditions/ConditionContextHelper.java | 28 ++-
.../impl/ConditionEvaluatorDispatcherImpl.java | 15 +-
.../spi/conditions/ConditionContextHelperTest.java | 33 ++-
.../impl/ConditionEvaluatorDispatcherImplTest.java | 48 ++++
.../plugins/baseplugin/actions/IdentityTrust.java | 44 ++++
.../actions/MergeProfilesOnPropertyAction.java | 38 ++-
.../baseplugin/actions/UpdatePropertiesAction.java | 68 +++---
.../actions/ActionIdentityIntrusionTest.java | 271 +++++++++++++++++++++
.../actions/MergeProfilesOnPropertyActionTest.java | 44 +++-
.../actions/UpdatePropertiesActionTest.java | 88 +++++++
samples/login-integration/setup.sh | 6 +-
.../apache/unomi/samples/login/LoginServlet.java | 10 +-
.../unomi/samples/login/LoginServletTest.java | 149 +++++++++++
.../apache/unomi/scripting/MvelScriptExecutor.java | 59 +++--
.../scripting/SecureFilteringClassLoader.java | 14 +-
.../internal/ExpressionFilterFactoryImpl.java | 19 +-
.../unomi/scripting/MvelScriptExecutorTest.java | 72 +++++-
.../scripting/SecureFilteringClassLoaderTest.java | 27 ++
.../internal/ExpressionFilterFactoryImplTest.java | 87 +++++++
44 files changed, 1383 insertions(+), 242 deletions(-)
copy extensions/{json-schema/services/src/main/java/org/apache/unomi/schema/api/ValidationException.java => groovy-actions/rest/src/test/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPointRoleTest.java} (51%)
copy extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/{testSaveAction.groovy => fieldInitializerAction.groovy} (64%)
create mode 100644 itests/src/test/java/org/apache/unomi/itests/GroovyActionsEndpointRoleSecurityIT.java
create mode 100644 plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IdentityTrust.java
create mode 100644 plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/actions/ActionIdentityIntrusionTest.java
create mode 100644 scripting/src/test/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImplTest.java
(unomi) 01/01: Bound diagnostic type tracking and the MVEL expression cache.
Posted by sh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch bound-diagnostic-and-script-caches
in repository https://gitbox.apache.org/repos/asf/unomi.git
commit e683891d1283a84eafcf68eb67ed960fcd0fa0b1
Author: Serge Huber <sh...@apache.org>
AuthorDate: Tue Sep 8 21:01:08 2026 +0200
Bound diagnostic type tracking and the MVEL expression cache.
Public input can feed unique ids and rejected scripts; keep those maps from growing without a cap.
---
.../apache/unomi/scripting/MvelScriptExecutor.java | 38 +++++++++++++++++++---
.../unomi/scripting/MvelScriptExecutorTest.java | 32 ++++++++++++++++++
.../services/impl/TypeResolutionServiceImpl.java | 34 ++++++++++++++++---
.../services/impl/events/EventServiceImpl.java | 5 ++-
.../impl/TypeResolutionServiceImplTest.java | 22 +++++++++++++
5 files changed, 122 insertions(+), 9 deletions(-)
diff --git a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java
index 6cd53e0ad..37766c862 100644
--- a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java
+++ b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java
@@ -21,15 +21,35 @@ import org.mvel2.ParserConfiguration;
import org.mvel2.ParserContext;
import java.io.Serializable;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.Collections;
+import java.util.HexFormat;
+import java.util.LinkedHashMap;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
/**
* MVEL script executor implementation
*/
public class MvelScriptExecutor implements ScriptExecutor {
- private Map<String, Serializable> mvelExpressions = new ConcurrentHashMap<>();
+ private static final int DEFAULT_EXPRESSIONS_CACHE_MAX_SIZE = 1000;
+
+ private final int expressionsCacheMaxSize = Integer.getInteger(
+ "org.apache.unomi.scripting.mvel.expressions.cache.max.size", DEFAULT_EXPRESSIONS_CACHE_MAX_SIZE);
+
+ /**
+ * Size-bounded LRU cache keyed by a fixed-size hash of the script text. Only accepted expressions
+ * are stored. An unbounded map keyed by the raw script would grow without limit.
+ */
+ private final Map<String, Serializable> mvelExpressions = Collections.synchronizedMap(
+ new LinkedHashMap<String, Serializable>(16, 0.75f, true) {
+ @Override
+ protected boolean removeEldestEntry(Map.Entry<String, Serializable> eldest) {
+ return size() > expressionsCacheMaxSize;
+ }
+ });
private SecureFilteringClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(getClass().getClassLoader());
private ExpressionFilterFactory expressionFilterFactory;
@@ -57,7 +77,8 @@ public class MvelScriptExecutor implements ScriptExecutor {
return null;
}
- Serializable compiledExpression = mvelExpressions.get(script);
+ String scriptCacheKey = getScriptCacheKey(script);
+ Serializable compiledExpression = mvelExpressions.get(scriptCacheKey);
if (compiledExpression == null) {
ParserConfiguration parserConfiguration = new ParserConfiguration();
parserConfiguration.setClassLoader(secureFilteringClassLoader);
@@ -75,11 +96,20 @@ public class MvelScriptExecutor implements ScriptExecutor {
parserContext.addImport("SecurityManager", String.class);
compiledExpression = MVEL.compileExpression(script, parserContext);
- mvelExpressions.put(script, compiledExpression);
+ mvelExpressions.put(scriptCacheKey, compiledExpression);
}
return MVEL.executeExpression(compiledExpression, context);
} finally {
Thread.currentThread().setContextClassLoader(tccl);
}
}
+
+ private static String getScriptCacheKey(String script) {
+ try {
+ byte[] hash = MessageDigest.getInstance("SHA-256").digest(script.getBytes(StandardCharsets.UTF_8));
+ return HexFormat.of().formatHex(hash);
+ } catch (NoSuchAlgorithmException e) {
+ return script;
+ }
+ }
}
diff --git a/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java
index 539a5b98b..dcc0e8566 100644
--- a/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java
+++ b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java
@@ -23,6 +23,7 @@ import org.junit.Test;
import java.io.File;
import java.io.IOException;
+import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -150,6 +151,37 @@ public class MvelScriptExecutorTest {
assertFalse("Vulnerability successfully executed ! File created at " + vulnFile.getCanonicalPath(), vulnFile.exists());
}
+ @Test
+ public void testExpressionCacheIsBoundedAndSkipsRejectedScripts() throws Exception {
+ System.setProperty("org.apache.unomi.scripting.mvel.expressions.cache.max.size", "10");
+ try {
+ MvelScriptExecutor boundedExecutor = new MvelScriptExecutor();
+ boundedExecutor.setExpressionFilterFactory(emptyAllowList());
+ String padding = "x".repeat(4096);
+ Map<String, Object> ctx = new HashMap<>();
+ for (int i = 0; i < 50; i++) {
+ boundedExecutor.execute("rejected-script-" + i + "-" + padding, ctx);
+ }
+ Field mvelExpressionsField = MvelScriptExecutor.class.getDeclaredField("mvelExpressions");
+ mvelExpressionsField.setAccessible(true);
+ Map<?, ?> cache = (Map<?, ?>) mvelExpressionsField.get(boundedExecutor);
+ assertEquals("Rejected scripts must not be stored", 0, cache.size());
+
+ boundedExecutor.setExpressionFilterFactory(allowAllExpressions());
+ for (int i = 0; i < 30; i++) {
+ boundedExecutor.execute(i + "+" + i, ctx);
+ }
+ assertTrue("Accepted-script cache must be size-bounded but grew to " + cache.size(),
+ cache.size() <= 10);
+ for (Object cacheKey : cache.keySet()) {
+ assertTrue("Cache keys must be fixed-size hashes, not the raw script text",
+ ((String) cacheKey).length() <= 64);
+ }
+ } finally {
+ System.clearProperty("org.apache.unomi.scripting.mvel.expressions.cache.max.size");
+ }
+ }
+
private void assertPublicEvalDoesNotReturnTwo(String expression) {
Object result = null;
try {
diff --git a/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
index 329756a75..0e99cc571 100644
--- a/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
+++ b/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
@@ -45,6 +45,12 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
private static final Logger LOGGER = LoggerFactory.getLogger(TypeResolutionServiceImpl.class.getName());
private static final int MAX_RECURSION_DEPTH = 1000;
+
+ /**
+ * Upper bound for diagnostic de-duplication collections. Some identifiers come from public
+ * input (for example condition type ids on context filters), so these sets must not grow without limit.
+ */
+ private static final int MAX_TRACKED_DIAGNOSTIC_ENTRIES = 1000;
private volatile DefinitionsService definitionsService;
@@ -127,7 +133,7 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
}
ConditionType conditionType = definitionsService.getConditionType(conditionTypeId);
if (conditionType == null) {
- if (unresolvedConditionTypes.add(conditionTypeId)) {
+ if (addBoundedDiagnostic(unresolvedConditionTypes, conditionTypeId)) {
LOGGER.warn("Couldn't resolve condition type: {} for {}", conditionTypeId, contextObjectName);
}
return false;
@@ -217,6 +223,20 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
}
}
+ /**
+ * Adds a value to a bounded diagnostic de-duplication set. Returns {@code true} if the value
+ * was newly stored (the caller should log). Once the cap is reached, further values are ignored.
+ */
+ private static boolean addBoundedDiagnostic(Set<String> set, String value) {
+ if (set.contains(value)) {
+ return false;
+ }
+ if (set.size() >= MAX_TRACKED_DIAGNOSTIC_ENTRIES) {
+ return false;
+ }
+ return set.add(value);
+ }
+
@Override
public boolean resolveActionTypes(Rule rule, boolean ignoreErrors) {
if (definitionsService == null) {
@@ -228,7 +248,7 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
if (rule.getActions() == null) {
if (!ignoreErrors) {
// Only warn once per rule to avoid log spam
- if (warnedRulesWithNullActions.add(ruleId)) {
+ if (addBoundedDiagnostic(warnedRulesWithNullActions, ruleId)) {
LOGGER.warn("Rule {}:{} has null actions", ruleId, rule.getMetadata() != null ? rule.getMetadata().getName() : "unknown");
}
}
@@ -237,7 +257,7 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
if (rule.getActions().isEmpty()) {
if (!ignoreErrors) {
// Only warn once per rule to avoid log spam
- if (warnedRulesWithNullActions.add(ruleId)) {
+ if (addBoundedDiagnostic(warnedRulesWithNullActions, ruleId)) {
LOGGER.warn("Rule {}:{} has empty actions", ruleId, rule.getMetadata() != null ? rule.getMetadata().getName() : "unknown");
}
}
@@ -262,7 +282,7 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
unresolvedActionTypes.remove(action.getActionTypeId());
action.setActionType(actionType);
} else {
- if (unresolvedActionTypes.add(action.getActionTypeId())) {
+ if (addBoundedDiagnostic(unresolvedActionTypes, action.getActionTypeId())) {
LOGGER.warn("Couldn't resolve action type : {}", action.getActionTypeId());
}
return false;
@@ -454,6 +474,12 @@ public class TypeResolutionServiceImpl implements TypeResolutionService {
}
Map<String, InvalidObjectInfo> typeMap = invalidObjects.computeIfAbsent(objectType, k -> new ConcurrentHashMap<>());
+
+ if (!typeMap.containsKey(objectId) && typeMap.size() >= MAX_TRACKED_DIAGNOSTIC_ENTRIES) {
+ LOGGER.debug("Invalid object tracking for type {} is full ({} entries), not tracking {}",
+ objectType, MAX_TRACKED_DIAGNOSTIC_ENTRIES, objectId);
+ return;
+ }
InvalidObjectInfo newInfo = new InvalidObjectInfo(
objectType,
diff --git a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
index 3442a59ac..493cc03ca 100644
--- a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
+++ b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
@@ -48,6 +48,7 @@ public class EventServiceImpl implements EventService {
/** Event-type chains already logged at recursion limit (see {@link #recursionChainKey}). */
private static final Set<String> LOGGED_RECURSION_CHAINS = ConcurrentHashMap.newKeySet();
+ private static final int MAX_LOGGED_RECURSION_CHAINS = 1000;
/**
* Simple data class to hold event information for recursion tracking.
@@ -221,7 +222,9 @@ public class EventServiceImpl implements EventService {
// Original allowed depths 0-10 (11 calls), blocking at depth 11
if (eventStack.size() > MAX_RECURSION_DEPTH) {
String chainKey = recursionChainKey(eventStack);
- if (LOGGED_RECURSION_CHAINS.add(chainKey)) {
+ if (!LOGGED_RECURSION_CHAINS.contains(chainKey)
+ && LOGGED_RECURSION_CHAINS.size() < MAX_LOGGED_RECURSION_CHAINS
+ && LOGGED_RECURSION_CHAINS.add(chainKey)) {
EventInfo currentEventInfo = new EventInfo(event);
if (tracer != null) {
tracer.trace("Max recursion depth reached for event: " + event.getEventType(), event.getItemId());
diff --git a/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java b/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
index 9e62633e8..db69770ae 100644
--- a/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
+++ b/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
@@ -1362,5 +1362,27 @@ public class TypeResolutionServiceImplTest {
assertTrue(info.getReason().contains("missingLeafType"),
"Reason text must name the actual failing leaf type");
}
+
+ @Test
+ public void unresolvedConditionTypeTracking_isBounded() {
+ when(definitionsService.getConditionType(startsWith("unknownType-"))).thenReturn(null);
+
+ for (int i = 0; i < 2500; i++) {
+ Condition condition = new Condition();
+ condition.setConditionTypeId("unknownType-" + i);
+ assertFalse(typeResolutionService.resolveConditionType(condition, "boundedness test"));
+ }
+
+ java.lang.reflect.Field field;
+ try {
+ field = TypeResolutionServiceImpl.class.getDeclaredField("unresolvedConditionTypes");
+ field.setAccessible(true);
+ Set<?> tracked = (Set<?>) field.get(typeResolutionService);
+ assertTrue(tracked.size() <= 1000,
+ "unresolved condition type tracking must stay bounded, but had " + tracked.size() + " entries");
+ } catch (ReflectiveOperationException e) {
+ throw new AssertionError(e);
+ }
+ }
}