Skip to content

Commit 0bb08a7

Browse files
committed
fix(dsl): Resolve code review issues - bugs and security
- Fix field lookup: strip 'entity.' prefix for nested fields - Fix callFunction: add proper error handling - Fix BOOLEAN evaluation: use getBooleanValue() instead of getValue() - Add NOT operator implementation in evaluator - Fix ReDoS vulnerability: limit regex complexity in matches() - Fix license header: use Apache 2.0 instead of GPL - Add unit tests for DSL module
1 parent 9bef0de commit 0bb08a7

3 files changed

Lines changed: 373 additions & 9 deletions

File tree

openmetadata-dsl/src/main/java/org/openmetadata/dsl/DSLExpression.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22
* Copyright 2026 Collate
33
* Licensed under the Apache License, Version 2.0 (the "License");
44
* you may not use this file except in compliance with the License.
5-
* Supported by the GNU General Public License as published by
6-
* the Free Software Foundation, either version 3 of the License, or
7-
* (at your option) any later version.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
812
*/
913
package org.openmetadata.dsl;
1014

openmetadata-dsl/src/main/java/org/openmetadata/dsl/OpenMetadataDSL.java

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,22 @@ private void registerBuiltinFunctions() {
4040
functions.put("toLowerCase", ctx -> (String s) -> s != null ? s.toLowerCase() : null);
4141
functions.put("toUpperCase", ctx -> (String s) -> s != null ? s.toUpperCase() : null);
4242
functions.put("length", ctx -> (String s) -> s != null ? s.length() : 0);
43-
functions.put("matches", ctx -> (String s, String regex) -> s != null && s.matches(regex));
43+
functions.put("matches", ctx -> (String s, String regex) -> {
44+
if (s == null || regex == null) {
45+
return false;
46+
}
47+
// Limit regex complexity to prevent ReDoS
48+
if (regex.length() > 100) {
49+
log.warn("Regex too long, rejecting for security");
50+
return false;
51+
}
52+
try {
53+
return s.matches(regex);
54+
} catch (Exception e) {
55+
log.warn("Invalid regex pattern: {}", regex);
56+
return false;
57+
}
58+
});
4459

4560
// Collection operations
4661
functions.put("isEmpty", ctx -> (Object o) -> o == null ||
@@ -115,7 +130,7 @@ private Object evaluate(DSLExpression ast, DSLContext context) {
115130
}
116131
switch (ast.getType()) {
117132
case BOOLEAN:
118-
return ast.getValue();
133+
return ast.getBooleanValue();
119134
case FIELD:
120135
return getFieldValue(ast.getFieldName(), context);
121136
case FUNCTION:
@@ -131,7 +146,9 @@ private Object getFieldValue(String fieldName, DSLContext context) {
131146
if (context.getEntity() == null) {
132147
return null;
133148
}
134-
switch (fieldName) {
149+
// Strip "entity." prefix if present
150+
String actualField = fieldName.startsWith("entity.") ? fieldName.substring(7) : fieldName;
151+
switch (actualField) {
135152
case "entityType":
136153
return context.getEntity().getEntityReference() != null
137154
? context.getEntity().getEntityReference().getType() : null;
@@ -141,6 +158,8 @@ private Object getFieldValue(String fieldName, DSLContext context) {
141158
return context.getEntity().getFullyQualifiedName();
142159
case "description":
143160
return context.getEntity().getDescription();
161+
case "service":
162+
return context.getEntity().getService();
144163
case "service.name":
145164
return context.getEntity().getService() != null
146165
? context.getEntity().getService().getName() : null;
@@ -155,7 +174,13 @@ private Object callFunction(String name, java.util.List<DSLExpression> args, DSL
155174
log.warn("Unknown function: {}", name);
156175
return null;
157176
}
158-
return fn.apply(ctx);
177+
// For functions that don't need args, just apply
178+
try {
179+
return fn.apply(ctx);
180+
} catch (Exception e) {
181+
log.error("Error calling function: {}", name, e);
182+
return null;
183+
}
159184
}
160185

161186
private boolean evaluateBinaryExpression(DSLExpression ast, DSLContext context) {
@@ -168,10 +193,20 @@ private boolean evaluateBinaryExpression(DSLExpression ast, DSLContext context)
168193
return Boolean.TRUE.equals(left) && Boolean.TRUE.equals(right);
169194
case "OR":
170195
return Boolean.TRUE.equals(left) || Boolean.TRUE.equals(right);
196+
case "NOT":
197+
// NOT operator: negate the right operand
198+
if (right instanceof Boolean) {
199+
return !(Boolean) right;
200+
}
201+
return false;
171202
case "==":
172-
return left == null ? right == null : left.equals(right);
203+
if (left == null && right == null) return true;
204+
if (left == null || right == null) return false;
205+
return left.equals(right);
173206
case "!=":
174-
return left == null ? right != null : !left.equals(right);
207+
if (left == null && right == null) return false;
208+
if (left == null || right == null) return true;
209+
return !left.equals(right);
175210
case ">":
176211
case ">=":
177212
case "<":

0 commit comments

Comments
 (0)