feat(dsl): Add OpenMetadata DSL module for alert rules and governance#27605
feat(dsl): Add OpenMetadata DSL module for alert rules and governance#27605Dev0907 wants to merge 1 commit intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| private Object callFunction(String name, java.util.List<DSLExpression> args, DSLContext ctx) { | ||
| Function<DSLContext, ?> fn = functions.get(name); | ||
| if (fn == null) { | ||
| log.warn("Unknown function: {}", name); | ||
| return null; | ||
| } | ||
| return fn.apply(ctx); | ||
| } |
There was a problem hiding this comment.
🚨 Bug: callFunction ignores parsed arguments — all functions broken
The callFunction method retrieves the registered Function<DSLContext, Object> and calls fn.apply(ctx), which returns an inner lambda (e.g., (String s, String search) -> ...). However, it never invokes that inner lambda with the actual parsed arguments. The args parameter is completely ignored. This means every function call (contains(), hasTag(), isEmpty(), etc.) will return the lambda object itself rather than executing it, so no function in the DSL will work correctly.
Suggested fix:
Evaluate each argument via `evaluate(arg, context)`, then reflectively or via a typed dispatch invoke the returned functional interface with those evaluated arguments. Alternatively, redesign the function registry to accept `(DSLContext, List<Object>) -> Object` so callFunction can pass arguments directly.
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Dev0907
force-pushed
the
feature/openmetadata-dsl
branch
from
0bb08a7 to
2f0b2ad
Compare
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
- Add DSL engine for evaluating alert rules and governance policies - Support field access, comparisons (==, !=, >, <, >=, <=) - Support logical operators (AND, OR, NOT) - Built-in functions: contains, startsWith, length, matches, hasTag, hasChanged, isOwner - Fix ReDoS vulnerability by limiting regex complexity - Add comprehensive unit tests
Dev0907
force-pushed
the
feature/openmetadata-dsl
branch
from
2f0b2ad to
ee8f6c1
Compare
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
| private Object callFunction(String name, java.util.List<DSLExpression> args, DSLContext ctx) { | ||
| Function<DSLContext, ?> fn = functions.get(name); | ||
| if (fn == null) { | ||
| log.warn("Unknown function: {}", name); | ||
| return null; | ||
| } | ||
| // For functions that don't need args, just apply | ||
| try { | ||
| return fn.apply(ctx); | ||
| } catch (Exception e) { | ||
| log.error("Error calling function: {}", name, e); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚨 Bug: callFunction ignores parsed arguments — all functions broken
The callFunction method receives args (the parsed function arguments from the AST) but completely ignores them. It only calls fn.apply(ctx), which returns the inner lambda (e.g., (String s, String search) -> s.contains(search)) rather than actually invoking it with the evaluated arguments.
This means every function call (contains(), hasTag(), isEmpty(), etc.) returns a lambda object instead of a boolean/value result. Since a lambda object is not instanceof Boolean, evaluateCondition will always return false for any expression involving a function call. This effectively breaks the entire DSL engine for its core use cases (governance rules, alert conditions).
The fix requires evaluating each argument expression, then dispatching to the function with those evaluated values. The current two-layer lambda design (ctx -> (args) -> result) is unnecessarily complex — consider simplifying to a single BiFunction<DSLContext, List<Object>, Object> pattern.
Suggested fix:
// Replace the function registry type and callFunction:
private final Map<String, BiFunction<DSLContext, List<Object>, Object>> functions;
// Register like:
functions.put("contains", (ctx, args) -> {
String s = (String) args.get(0);
String search = (String) args.get(1);
return s != null && s.contains(search);
});
// In callFunction, evaluate args first:
List<Object> evalArgs = args.stream()
.map(a -> evaluate(a, context))
.collect(Collectors.toList());
return fn.apply(ctx, evalArgs);
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
| private Object evaluate(DSLExpression ast, DSLContext context) { | ||
| if (ast == null) { | ||
| return false; | ||
| } | ||
| switch (ast.getType()) { | ||
| case BOOLEAN: | ||
| return ast.getBooleanValue(); | ||
| case FIELD: | ||
| return getFieldValue(ast.getFieldName(), context); | ||
| case FUNCTION: | ||
| return callFunction(ast.getFunctionName(), ast.getArguments(), context); | ||
| case BINARY_EXPR: | ||
| return evaluateBinaryExpression(ast, context); | ||
| default: | ||
| return false; |
There was a problem hiding this comment.
⚠️ Bug: evaluate() missing STRING and NUMBER cases — literals ignored
The evaluate method's switch statement handles BOOLEAN, FIELD, FUNCTION, and BINARY_EXPR but omits STRING and NUMBER (and VARIABLE, CONDITIONAL). The parser correctly produces STRING and NUMBER expression nodes (e.g., for 'table' or 5 in comparisons), but evaluate() falls through to default: return false for these types.
This means the right-hand side of entity.entityType == 'table' evaluates to false (a Boolean) instead of the string "table", so equality comparisons with literals will always produce incorrect results.
Suggested fix:
case STRING:
return ast.getStringValue();
case NUMBER:
return ast.getNumberValue();
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
| <artifactId>antlr4-runtime</artifactId> | ||
| <version>4.9.3</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.springframework</groupId> | ||
| <artifactId>spring-core</artifactId> | ||
| <version>6.1.14</version> | ||
| </dependency> |
There was a problem hiding this comment.
⚠️ Quality: Unused antlr4 and spring-core deps with version conflicts
The openmetadata-dsl/pom.xml declares dependencies on antlr4-runtime:4.9.3 and spring-core:6.1.14, but neither is imported or used anywhere in the module's source code. The parser is hand-written using java.util.regex. Additionally, these hardcoded versions conflict with the parent POM's managed versions (antlr:4.13.2, spring:6.2.11), which could cause classpath issues at runtime if the module is included in a larger build. Remove unused dependencies or, if they are planned for future use, at least use ${antlr.version} / ${spring.version} from the parent POM.
Suggested fix:
Remove the unused dependencies:
<!-- Remove these from pom.xml -->
<dependency>
<groupId>org.antlr</groupId>
<artifactId>antlr4-runtime</artifactId>
...
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-core</artifactId>
...
</dependency>
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
| functions.put("matches", ctx -> (String s, String regex) -> { | ||
| if (s == null || regex == null) { | ||
| return false; | ||
| } | ||
| // Limit regex complexity to prevent ReDoS | ||
| if (regex.length() > 100) { | ||
| log.warn("Regex too long, rejecting for security"); | ||
| return false; | ||
| } | ||
| try { | ||
| return s.matches(regex); | ||
| } catch (Exception e) { | ||
| log.warn("Invalid regex pattern: {}", regex); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
⚠️ Security: Regex length check insufficient to prevent ReDoS
The matches() function limits regex length to 100 characters but does not restrict dangerous patterns. A short regex like (a+)+b (11 chars) can cause catastrophic backtracking on input "aaaaaaaaaaaaaaaaaaaaa". Since DSL expressions are likely user-supplied (alert rules, governance policies), this is an exploitable denial-of-service vector.
Consider running the regex with a timeout, using Pattern.compile with a bounded match via Matcher + interrupt, or restricting to a safe regex subset (e.g., no nested quantifiers).
Suggested fix:
// Execute regex with a timeout using a separate thread:
ExecutorService executor = Executors.newSingleThreadExecutor();
Future<Boolean> future = executor.submit(() -> s.matches(regex));
try {
return future.get(100, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
future.cancel(true);
log.warn("Regex timed out: {}", regex);
return false;
}
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
Code Review 🚫 Blocked 6 resolved / 11 findingsInitial OpenMetadata DSL module implementation introduces critical regressions where function arguments are ignored and literal evaluation is incomplete. Unresolved critical and important issues regarding dependency conflicts and ReDoS risks must be addressed before integration. 🚨 Bug: callFunction ignores parsed arguments — all functions broken📄 openmetadata-dsl/src/main/java/org/openmetadata/dsl/OpenMetadataDSL.java:152-159 The Suggested fix🚨 Bug: callFunction ignores parsed arguments — all functions broken📄 openmetadata-dsl/src/main/java/org/openmetadata/dsl/OpenMetadataDSL.java:171-184 📄 openmetadata-dsl/src/main/java/org/openmetadata/dsl/OpenMetadataDSL.java:36-50 The This means every function call ( The fix requires evaluating each argument expression, then dispatching to the function with those evaluated values. The current two-layer lambda design ( Suggested fix
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
2 participants
Description
This PR introduces the OpenMetadata DSL (Domain-Specific Language) module - a powerful engine for creating sophisticated alert rules, governance policies, and automation workflows using natural, expressive syntax.
Features Implemented
🚨 Smart Alerts Engine
🛡️ Data Governance
🔒 Security Functions
Files Changed
File
openmetadata-dsl/pom.xml
OpenMetadataDSL.java
DSLExpressionParser.java
DSLContext.java
DSLExpression.java
DSLVariable.java
DSLExamples.java
pom.xml (root)
Example Usage
// Data quality rule
"entity.entityType == 'table' AND isEmpty(entity.description)"
// Compliance rule
"contains(entity.name, 'customer') AND NOT hasTag('GDPR-Compliant')"
// Security rule
"contains(entity.name, 'ssn') AND NOT hasTag('Security-Classified')"
Summary by Gitar
matchesfunction with a 100-character limit to mitigate potential ReDoS attacks.NOTlogical operator and robust null handling for==and!=.serviceentity field access and implemented detailed exception logging for runtime evaluation errors.OpenMetadataDSLTestsuite covering operators, field access, function evaluation, and various entity states.This will update automatically on new commits.