This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch deny-mvel-public-eval-apis in repository https://gitbox.apache.org/repos/asf/unomi.git
commit b07ce09463c0122102e00184c6b7121d4b94972a Author: Serge Huber <[email protected]> AuthorDate: Tue Sep 8 21:56:06 2026 +0200 Disable MVEL scripting by default and add an opt-in for existing rules. Starting with 3.1, script:: expressions are not compiled unless org.apache.unomi.scripting.mvel.enabled is true. Allow-list and class-loader filters still apply when the engine is turned back on. --- docker/README.md | 1 + manual/src/main/asciidoc/configuration.adoc | 12 ++++- .../asciidoc/migrations/migrate-3.0-to-3.1.adoc | 7 ++- .../main/resources/etc/custom.system.properties | 5 ++ .../apache/unomi/scripting/MvelScriptExecutor.java | 29 +++++++++++- .../unomi/scripting/MvelScriptExecutorTest.java | 55 ++++++++++++++++++---- 6 files changed, 98 insertions(+), 11 deletions(-) diff --git a/docker/README.md b/docker/README.md index a5dc49bde..d3bbe3b94 100644 --- a/docker/README.md +++ b/docker/README.md @@ -128,6 +128,7 @@ Note: Linux doesn't support the host.docker.internal DNS lookup method yet, it s - `UNOMI_HEALTHCHECK_PASSWORD`: Required health-check (`health`) password — no known default - `UNOMI_AUTO_START`: Boolean to specify if unomi auto start with karaf (defaults to `true`) - `UNOMI_DISTRIBUTION`: Specifies the Unomi Distribution Feature to use (`unomi-distribution-elasticsearch` or `unomi-distribution-opensearch`, defaults to `unomi-distribution-elasticsearch`) +- `UNOMI_SCRIPTING_MVEL_ENABLED`: Enable MVEL `script::` expressions (defaults to `false` in 3.1; set `true` only for existing rules/conditions that still need MVEL) ### ElasticSearch-specific Variables - `UNOMI_ELASTICSEARCH_ADDRESSES`: ElasticSearch host:port (default: localhost:9200) diff --git a/manual/src/main/asciidoc/configuration.adoc b/manual/src/main/asciidoc/configuration.adoc index 5f51a0561..9c26ed223 100644 --- a/manual/src/main/asciidoc/configuration.adoc +++ b/manual/src/main/asciidoc/configuration.adoc @@ -534,6 +534,10 @@ curl -X POST "http://localhost:8181/cxs/segments"; \ === Scripting security +Starting with Apache Unomi 3.1, MVEL scripting is *disabled by default*. Expressions that start with `script::` are not compiled or executed unless you set `org.apache.unomi.scripting.mvel.enabled` to `true` (environment variable `UNOMI_SCRIPTING_MVEL_ENABLED=true`). + +Leave it off unless existing rules or conditions still need MVEL. When it is on, the allow-list, forbid list, and filtering class loader still apply. Built-in definitions that use `script::` (for example `sessionDurationCondition` and the session size update in `_i6iqbc3x8_sessionPageReferrer`) also require the engine to be enabled. + ==== Multi-layer scripting filtering system The scripting security system is multi-layered. @@ -643,6 +647,7 @@ From https://github.com/apache/unomi/blob/unomi-1.5.x/plugins/baseplugin/src/mai As we see in the above example, we use an MVEL script with the setPropertyAction to set a property value. Starting with version 1.5.2, any expression use in rules MUST be allow-listed. +Starting with version 3.1, MVEL itself is also off by default: set `org.apache.unomi.scripting.mvel.enabled=true` before those expressions will run. By default, Apache Unomi comes with some built-in allowed expressions that cover all the internal uses cases. @@ -686,6 +691,11 @@ Alongside with the allow-listing technology, there are new configuration paramet [source] ---- +# MVEL compilation and execution for script:: expressions. Default false starting with Unomi 3.1. +# Set to true if existing rules or conditions still use script:: (allow-list and class-loader filters still apply). +# Must be the string true (not 1). +org.apache.unomi.scripting.mvel.enabled=${env:UNOMI_SCRIPTING_MVEL_ENABLED:-false} + # These parameters control the list of classes that are allowed or forbidden when executing expressions. org.apache.unomi.scripting.allow=${env:UNOMI_ALLOW_SCRIPTING_CLASSES:-org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,java.lang.Object,java.util.Map,java.util.HashMap,java.lang.Integer,org.mvel2.*} org.apache.unomi.scripting.forbid=${env:UNOMI_FORBID_SCRIPTING_CLASSES:-} @@ -932,7 +942,7 @@ org.apache.unomi.services.actions;version ==== Scripting roadmap -Scripting will probably undergo major changes in future versions of Apache Unomi, with the likely retirement of MVEL in favor of Groovy Actions detailed above. +Starting with 3.1, MVEL is off unless operators opt in (see above). Scripting will probably undergo further changes in later versions of Apache Unomi, with the likely retirement of MVEL in favor of Groovy Actions detailed above. These changes will not happen on maintenance versions of Apache Unomi, only in the next major version. Maintenance versions will of course maintain compatibility with existing scripting solutions. diff --git a/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc b/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc index 59350e8ec..b858b35eb 100644 --- a/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc +++ b/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc @@ -294,6 +294,10 @@ In addition to tenant API keys, Unomi 3.1 hardens several contracts that can bre |`mergeProfilesOnProperty` / cross-profile `updateProperties` / `systemProperties.*` |Sometimes driven from public context events |Cross-profile merge/update and `systemProperties` writes require a trusted caller (system or tenant admin). Public callers may still update the **current** cookie-bound profile's normal properties when the event type allows it. + +|MVEL `script::` expressions +|Compiled and executed when they passed the allow-list +|**Disabled by default.** Set `org.apache.unomi.scripting.mvel.enabled=true` (env `UNOMI_SCRIPTING_MVEL_ENABLED=true`) if existing rules or conditions still need MVEL. Allow-list and class-loader filters still apply when it is on. |=== [WARNING] @@ -333,6 +337,7 @@ console (port 8102) is not covered by that check. * Ops scripts and Docker: export both password env vars; replace `karaf:karaf` with `karaf:$UNOMI_ROOT_PASSWORD`. * Windows deployments: confirm the passwords actually took effect after upgrading — the startup check warns but cannot halt `karaf.bat` (see the warning above). * Automation that uploaded Groovy actions or managed Router import/export with a tenant private key: switch to system administrator credentials. +* Deployments whose rules or conditions still use `script::` MVEL expressions (including built-in `sessionDurationCondition` and the session size action on `_i6iqbc3x8_sessionPageReferrer`): set `org.apache.unomi.scripting.mvel.enabled=true` / `UNOMI_SCRIPTING_MVEL_ENABLED=true`. Expression allow-lists and class-loader filters still apply when the engine is on. New 3.1 deployments can leave the default (`false`). ==== Benefits of Multi-Tenancy in 3.1 @@ -351,7 +356,7 @@ Before starting the migration, please ensure that: - You are currently running Apache Unomi 3.0 (or a later 3.0.x version) - You understand the multi-tenancy impact on your data model - You have a plan to update client applications to tenant API keys (or temporary <<_v2_compatibility_mode,V2 compatibility mode>> only if coming from 2.x) -- You have reviewed the <<_client_facing_hardening_3_1,client-facing hardening>> notes (cookie-only public profile binding, HttpOnly default, required passwords, privileged REST roles) +- You have reviewed the <<_client_facing_hardening_3_1,client-facing hardening>> notes (cookie-only public profile binding, HttpOnly default, required passwords, privileged REST roles, MVEL off by default) - You know how to obtain plaintext API keys after upgrade (regenerate via `/cxs/tenants/{id}/apikeys`; create responses expose masked keys only) === Migration Process diff --git a/package/src/main/resources/etc/custom.system.properties b/package/src/main/resources/etc/custom.system.properties index 4dd435a78..e932ea715 100644 --- a/package/src/main/resources/etc/custom.system.properties +++ b/package/src/main/resources/etc/custom.system.properties @@ -30,6 +30,11 @@ ${optionals}=unomi.custom.system.properties org.apache.unomi.security.root.password=${env:UNOMI_ROOT_PASSWORD} org.apache.unomi.healthcheck.password=${env:UNOMI_HEALTHCHECK_PASSWORD} +# MVEL compilation and execution for script:: expressions. Default false starting with Unomi 3.1. +# Set to true if existing rules or conditions still use script:: (allow-list and class-loader filters still apply). +# Must be the string true (not 1). +org.apache.unomi.scripting.mvel.enabled=${env:UNOMI_SCRIPTING_MVEL_ENABLED:-false} + # These parameters control the list of classes that are allowed or forbidden when executing expressions. org.apache.unomi.scripting.allow=${env:UNOMI_ALLOW_SCRIPTING_CLASSES:-org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,java.lang.Object,java.util.Map,java.util.HashMap,java.lang.Integer,org.mvel2.*,java.lang.String} org.apache.unomi.scripting.forbid=${env:UNOMI_FORBID_SCRIPTING_CLASSES:-} 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 cb706050f..007f47f81 100644 --- a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java +++ b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java @@ -19,22 +19,43 @@ package org.apache.unomi.scripting; import org.mvel2.MVEL; import org.mvel2.ParserConfiguration; import org.mvel2.ParserContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.Serializable; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; /** * MVEL script executor implementation */ public class MvelScriptExecutor implements ScriptExecutor { - private final static String INVALID_SCRIPT_MARKER = "--- Invalid Script Marker ---"; + /** + * System property that enables MVEL compilation and execution for {@code script::} expressions. + * Default is {@code false} starting with Unomi 3.1. Set to {@code true} to restore the previous behaviour + * for deployments that still rely on MVEL in rules or conditions. + */ + public static final String ENABLED_PROPERTY = "org.apache.unomi.scripting.mvel.enabled"; + + private static final Logger LOGGER = LoggerFactory.getLogger(MvelScriptExecutor.class.getName()); + private static final String INVALID_SCRIPT_MARKER = "--- Invalid Script Marker ---"; + private static final AtomicBoolean DISABLED_NOTICE_LOGGED = new AtomicBoolean(); private Map<String, Serializable> mvelExpressions = new ConcurrentHashMap<>(); private SecureFilteringClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(getClass().getClassLoader()); private ExpressionFilterFactory expressionFilterFactory; + /** + * Whether MVEL compilation and execution are enabled. + * + * @return {@code true} only when {@value #ENABLED_PROPERTY} is {@code true} + */ + public static boolean isEnabled() { + return Boolean.parseBoolean(System.getProperty(ENABLED_PROPERTY, "false")); + } + /** * Sets the factory used to obtain expression filters per script language. * @@ -46,6 +67,12 @@ public class MvelScriptExecutor implements ScriptExecutor { @Override public Object execute(String script, Map<String, Object> context) { + if (!isEnabled()) { + if (DISABLED_NOTICE_LOGGED.compareAndSet(false, true)) { + LOGGER.info("MVEL scripting is disabled ({}=false); script expressions will not run. Set it to true to restore MVEL for existing rules and conditions.", ENABLED_PROPERTY); + } + return null; + } final ClassLoader tccl = Thread.currentThread().getContextClassLoader(); try { 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 d843c43d5..fa7e4f6ce 100644 --- a/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java +++ b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java @@ -18,6 +18,7 @@ package org.apache.unomi.scripting; import org.apache.unomi.api.CustomItem; import org.apache.unomi.api.Event; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -29,7 +30,9 @@ import java.util.Map; import java.util.Set; import java.util.regex.Pattern; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; public class MvelScriptExecutorTest { @@ -42,18 +45,34 @@ public class MvelScriptExecutorTest { @Before public void setup() { - scriptExecutor.setExpressionFilterFactory(new ExpressionFilterFactory() { - @Override - public ExpressionFilter getExpressionFilter(String filterCollection) { - Set<Pattern> allowedExpressions = new HashSet<>(); - Set<Pattern> forbiddenExpressions = new HashSet<>(); - return new ExpressionFilter(allowedExpressions, forbiddenExpressions); - } - }); + System.clearProperty(MvelScriptExecutor.ENABLED_PROPERTY); + scriptExecutor.setExpressionFilterFactory(emptyAllowList()); + } + + @After + public void restoreMvelEnabledProperty() { + System.clearProperty(MvelScriptExecutor.ENABLED_PROPERTY); + } + + @Test + public void testMvelDisabledByDefaultEvenWhenExpressionIsAllowed() { + scriptExecutor.setExpressionFilterFactory(allowAllExpressions()); + Object result = scriptExecutor.execute("1+1", new HashMap<String, Object>()); + assertNull(result); + assertFalse(MvelScriptExecutor.isEnabled()); + } + + @Test + public void testMvelEnabledEvaluatesAllowlistedExpression() { + System.setProperty(MvelScriptExecutor.ENABLED_PROPERTY, "true"); + scriptExecutor.setExpressionFilterFactory(allowAllExpressions()); + Object result = scriptExecutor.execute("1+1", new HashMap<String, Object>()); + assertEquals(2, ((Number) result).intValue()); } @Test public void testMVELSecurity() throws IOException { + System.setProperty(MvelScriptExecutor.ENABLED_PROPERTY, "true"); Map<String, Object> ctx = new HashMap<>(); Event mockEvent = generateMockEvent(); ctx.put("event", mockEvent); @@ -115,6 +134,26 @@ public class MvelScriptExecutorTest { assertFalse("Vulnerability successfully executed ! File created at " + vulnFile.getCanonicalPath(), vulnFile.exists()); } + private static ExpressionFilterFactory emptyAllowList() { + return new ExpressionFilterFactory() { + @Override + public ExpressionFilter getExpressionFilter(String filterCollection) { + Set<Pattern> allowedExpressions = new HashSet<>(); + Set<Pattern> forbiddenExpressions = new HashSet<>(); + return new ExpressionFilter(allowedExpressions, forbiddenExpressions); + } + }; + } + + private static ExpressionFilterFactory allowAllExpressions() { + return new ExpressionFilterFactory() { + @Override + public ExpressionFilter getExpressionFilter(String filterCollection) { + return new ExpressionFilter(null, null); + } + }; + } + private static Event generateMockEvent() { Event mockEvent = new Event(); CustomItem targetItem = new CustomItem();
