Skip to content

Conversation

@Kovy95
Copy link
Contributor

@Kovy95 Kovy95 commented Oct 28, 2025

Description

  • implement ElasticQueryFactory to resolve dynamic queries as a valid java code
  • add boolean switch for dynamic queries

Implements NAE-2243

Dependencies

none

Third party dependencies

-No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

manually

Test Configuration

<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>

Name Tested on
OS linux mint 21
Runtime java 21
Dependency Manager maven 3.9.9
Framework version Spring boot 3.4.4
Run parameters
Other configuration

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes have been checked, personally or remotely, with @...
  • I have commented my code, particularly in hard-to-understand areas
  • I have resolved all conflicts with the target branch of the PR
  • I have updated and synced my code with the target branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes:
    • Lint test
    • Unit tests
    • Integration tests
  • I have checked my contribution with code analysis tools:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides
    • Migration Guides

Summary by CodeRabbit

  • New Features
    • Added dynamic query template support for case searches. Queries can now automatically substitute current date/time and logged-in user information using template variables.

- implement ElasticQueryFactory to resolve dynamic queries as a valid java code
- add boolean switch for dynamic queries
@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

Adds dynamic query templating for Elastic search. Introduces IElasticQueryFactory and its ElasticQueryFactory implementation using Groovy SimpleTemplateEngine. ElasticCaseService now conditionally populates query templates when request.dynamicQuery is true, supplying user and date/time context. Extends ElasticQueryConstants and adds dynamicQuery flag to CaseSearchRequest. Adds tests for template population.

Changes

Cohort / File(s) Summary
Elastic query constants
application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticQueryConstants.java
Added constants: DYNAMIC_USER_ID_TEMPLATE ("me"), LOCAL_DATE_NOW_TEMPLATE ("now"), LOCAL_DATE_TODAY_TEMPLATE ("today"), LOGGED_USER_TEMPLATE ("loggedUser").
Dynamic query factory (API + impl)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticQueryFactory.java, application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticQueryFactory.java
New interface declaring populateQuery(String, Map). New service implementing it using Groovy SimpleTemplateEngine; maintains internal context and merges provided map; logs errors and returns original query on failure.
Service integration
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
Autowired IElasticQueryFactory. In buildStringQuery: if request.dynamicQuery, populate template with context (me, now, today, loggedUser); else keep USER_ID_TEMPLATE replacement.
Request model
application-engine/src/main/java/com/netgrif/application/engine/elastic/web/requestbodies/CaseSearchRequest.java
Added public boolean field dynamicQuery.
Tests
application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticQueryFactoryTest.groovy
New Spring Boot test verifying populateQuery with date/time and user substitutions and Groovy expressions. Sets up test user/security context.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant ECS as ElasticCaseService
  participant EQF as IElasticQueryFactory

  Client->>ECS: search(request with query, dynamicQuery flag)
  alt dynamicQuery == true
    ECS->>ECS: Build context {me, now, today, loggedUser}
    ECS->>EQF: populateQuery(query, context)
    EQF-->>ECS: populatedQuery
    ECS->>ECS: Use populatedQuery
  else dynamicQuery == false
    ECS->>ECS: Replace USER_ID_TEMPLATE with userId
  end
  ECS-->>Client: results
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • ElasticQueryFactory: verify template evaluation behavior, error handling, and context merge semantics.
  • ElasticCaseService: confirm dynamicQuery branching and context values (user ID, now/today types).
  • Thread-safety: shared context map in ElasticQueryFactory.
  • Security: assess template content sources and execution via Groovy SimpleTemplateEngine.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "NAE-2243 Dynamic date resolution in Elastic query" is clearly and directly related to the changeset. The implementation introduces dynamic template resolution for Elastic queries using Groovy's SimpleTemplateEngine, with significant changes including new template constants (LOCAL_DATE_NOW_TEMPLATE, LOCAL_DATE_TODAY_TEMPLATE), a query factory that processes these templates, and modifications to CaseSearchRequest to support dynamic queries. The title accurately captures a major feature of these changes—dynamic date resolution—and the test coverage specifically validates date and timestamp resolution scenarios. While the PR also implements dynamic user ID resolution, the title's focus on date resolution remains accurate and representative of the primary functional addition.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f11efc and 412eb34.

📒 Files selected for processing (6)
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticQueryConstants.java (1 hunks)
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java (4 hunks)
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticQueryFactory.java (1 hunks)
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticQueryFactory.java (1 hunks)
  • application-engine/src/main/java/com/netgrif/application/engine/elastic/web/requestbodies/CaseSearchRequest.java (1 hunks)
  • application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticQueryFactoryTest.groovy (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java (1)
application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticQueryConstants.java (1)
  • ElasticQueryConstants (3-12)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticQueryFactory.java (1)
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java (1)
  • Service (53-541)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build

Comment on lines +8 to +11
public static final String DYNAMIC_USER_ID_TEMPLATE = "me";
public static final String LOCAL_DATE_NOW_TEMPLATE = "now";
public static final String LOCAL_DATE_TODAY_TEMPLATE = "today";
public static final String LOGGED_USER_TEMPLATE = "loggedUser";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider a reserved prefix to avoid collisions

Using very generic variable names increases accidental collisions. Consider names like "__ctx_me", "__ctx_now", "__ctx_today", "__ctx_userId".

🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticQueryConstants.java
around lines 8 to 11, the current template constants use very generic names (me,
now, today, loggedUser) that risk accidental collisions; rename them to a
reserved/prefixed form (for example __ctx_me, __ctx_now, __ctx_today,
__ctx_userId) and update all references/usages across the codebase to use the
new constant names to keep behavior unchanged but avoid collisions; ensure
constant string values and public constant names are updated consistently and
run tests/compile to catch any missed references.

Comment on lines +439 to +449
String populatedQuery;
if (request.dynamicQuery) {
populatedQuery = queryFactory.populateQuery(request.query, new HashMap<>(Map.of(
ElasticQueryConstants.DYNAMIC_USER_ID_TEMPLATE, user.getId().toString(),
ElasticQueryConstants.LOCAL_DATE_NOW_TEMPLATE, LocalDateTime.now(),
ElasticQueryConstants.LOCAL_DATE_TODAY_TEMPLATE, LocalDate.now(),
ElasticQueryConstants.LOGGED_USER_TEMPLATE, user
)));
} else {
populatedQuery = request.query.replaceAll(ElasticQueryConstants.USER_ID_TEMPLATE, user.getId().toString());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Remote code execution via Groovy templating of user input

request.query is user-controlled. Rendering it with Groovy (via queryFactory) executes arbitrary Groovy code in-process. This is an RCE vector.

Immediate mitigations:

  • Do not execute code in queries. Support only simple placeholders (me, now, today, loggedUserId) with safe string substitution.
  • Optionally gate dynamicQuery behind config/role checks and disable by default.
  • Do not expose full LoggedUser into templates; pass only userId.

Minor: Use replace instead of replaceAll for a fixed literal.
Apply this minimal change now:

-            populatedQuery = request.query.replaceAll(ElasticQueryConstants.USER_ID_TEMPLATE, user.getId().toString());
+            populatedQuery = request.query.replace(ElasticQueryConstants.USER_ID_TEMPLATE, user.getId().toString());

Would you like me to push a safe placeholder-based implementation and wire it here?


🏁 Script executed:

#!/bin/bash
# Find the queryFactory field declaration and type in ElasticCaseService
rg -nP "queryFactory\s*[=;:]" application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java -A2 -B2

Length of output: 94


🏁 Script executed:

#!/bin/bash
# Find the populateQuery method implementation
ast-grep --pattern 'populateQuery($$$) {
  $$$
}'

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for queryFactory class definition
rg -nP "class\s+\w*QueryFactory" --type=java -C5

Length of output: 1435


🏁 Script executed:

#!/bin/bash
# Check the request object type in the method signature around line 439
rg -nP "^\s*(public|private|protected).*\(" application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java | head -20

Length of output: 1859


🏁 Script executed:

#!/bin/bash
# Look for Groovy dependencies or imports
rg -n "groovy" application-engine/src/main/java/com/netgrif/application/engine/elastic/ -i

Length of output: 453


🏁 Script executed:

#!/bin/bash
# Check ElasticQueryConstants to understand the template values
cat application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticQueryConstants.java 2>/dev/null || rg -l "ElasticQueryConstants" --type=java | head -1 | xargs cat

Length of output: 503


🏁 Script executed:

#!/bin/bash
# Get the full ElasticQueryFactory implementation to see populateQuery
cat application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticQueryFactory.java

Length of output: 1336


🏁 Script executed:

#!/bin/bash
# Find CaseSearchRequest class definition
rg -nP "class\s+CaseSearchRequest" --type=java -A30

Length of output: 4468


🏁 Script executed:

#!/bin/bash
# Find API endpoints that use CaseSearchRequest to determine if query is user-controlled
rg -nP "@(PostMapping|GetMapping|RequestMapping|PutMapping).*search" --type=java -A10 | head -50

Length of output: 9906


🏁 Script executed:

#!/bin/bash
# Look for REST controllers that handle case search
rg -nP "CaseSearchRequest" --type=java -C3 | grep -E "(Controller|RestController|RequestBody)" -A3 -B3

Length of output: 4336


🏁 Script executed:

#!/bin/bash
# Check SingleCaseSearchRequestAsList definition
rg -nP "class\s+SingleCaseSearchRequestAsList" --type=java -A20

Length of output: 371


🏁 Script executed:

#!/bin/bash
# Look for @Valid, @Validated, or any constraint annotations on query field
rg -nP "@(Valid|Validated|NotNull|NotBlank|Pattern)" application-engine/src/main/java/com/netgrif/application/engine/elastic/web/requestbodies/CaseSearchRequest.java -B2

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check if there's any validation of dynamicQuery flag or query content
rg -nP "dynamicQuery|validateQuery|sanitize" --type=java -C3 | grep -E "elastic|service" -C3

Length of output: 16879


🏁 Script executed:

#!/bin/bash
# Look for any configuration that disables or controls dynamicQuery
rg -nP "dynamicQuery" --type=java -C2

Length of output: 1508


🏁 Script executed:

#!/bin/bash
# Check if ElasticTaskSearchRequest has similar dynamicQuery vulnerability
rg -nP "class\s+.*TaskSearchRequest" --type=java -A20 | grep -E "(query|dynamicQuery)" -B5 -A2

Length of output: 1116


🏁 Script executed:

#!/bin/bash
# Check if task search also uses queryFactory.populateQuery
rg -nP "populateQuery.*Task" --type=java -C5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check ElasticTaskSearchRequest full definition
cat application-engine/src/main/java/com/netgrif/application/engine/elastic/web/requestbodies/ElasticTaskSearchRequest.java

Length of output: 2857


🏁 Script executed:

#!/bin/bash
# Search for elasticTaskService or ElasticTaskService to see if it also uses populateQuery
rg -nP "class\s+.*ElasticTaskService" --type=java -A5

Length of output: 920


🏁 Script executed:

#!/bin/bash
# Check if ElasticTaskService uses queryFactory.populateQuery
rg -nP "queryFactory|populateQuery" application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.java -C3

Length of output: 0


Critical: Remote code execution via Groovy template injection

ElasticQueryFactory.populateQuery() passes user-controlled request.query directly to Groovy's SimpleTemplateEngine, which executes arbitrary code in template expressions. An authenticated attacker can exploit this by sending:

POST /case/search
{
  "query": "${Runtime.getRuntime().exec('command')}",
  "dynamicQuery": true
}

Required fixes:

  1. Remove Groovy templating entirely. Replace with safe string substitution supporting only the documented placeholders (me, now, today). Do not pass loggedUser—expose only userId.

  2. If templating must remain, gate it behind a configuration flag disabled by default and restrict access to admin roles only.

  3. Apply the minor optimization:

-populatedQuery = request.query.replaceAll(ElasticQueryConstants.USER_ID_TEMPLATE, user.getId().toString());
+populatedQuery = request.query.replace(ElasticQueryConstants.USER_ID_TEMPLATE, user.getId().toString());

This is an in-process code execution vulnerability requiring immediate remediation.

🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
around lines 439-449, the code currently passes request.query into Groovy's
SimpleTemplateEngine (via ElasticQueryFactory.populateQuery) which allows remote
code execution; remove Groovy templating and replace it with a safe
placeholder-only substitution: if request.dynamicQuery is true, only replace
documented placeholders (e.g. ElasticQueryConstants.DYNAMIC_USER_ID_TEMPLATE ->
user.getId().toString(), LOCAL_DATE_NOW_TEMPLATE ->
LocalDateTime.now().toString(), LOCAL_DATE_TODAY_TEMPLATE ->
LocalDate.now().toString()) using literal string replacement (no template
engine, no evaluated expressions) and do not expose/pass the full user object
(remove LOGGED_USER_TEMPLATE); if templating must remain, require a server-side
config flag disabled by default and enforce only admin roles can use dynamic
templating; additionally validate/limit request.query length and characters as
needed and prefer the non-dynamic branch (simple replaceAll of USER_ID_TEMPLATE)
for performance.

Comment on lines +19 to +39
private final SimpleTemplateEngine templateEngine;
@Getter
private final Map<String, Object> context;

public ElasticQueryFactory() {
this.templateEngine = new SimpleTemplateEngine();
this.context = new HashMap<>();
}

@Override
public String populateQuery(String query, Map<String, Object> queryContext) {
String populatedQuery = query;
try {
queryContext.putAll(context);
Template template = templateEngine.createTemplate(query);
populatedQuery = template.make(queryContext).toString();
} catch (CompilationFailedException | ClassNotFoundException | IOException e) {
log.error("Cannot populate template from string query", e);
}
return populatedQuery;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Blocker: Groovy template execution on untrusted input = RCE

SimpleTemplateEngine evaluates Groovy inside ${...}. With user-supplied query strings this enables arbitrary code execution.

Replace with safe placeholder substitution and remove Groovy. Also avoid mutating inputs and leaking mutable context.

Apply this safe, dependency-free implementation:

-package com.netgrif.application.engine.elastic.service;
-
-import com.netgrif.application.engine.elastic.service.interfaces.IElasticQueryFactory;
-import groovy.text.SimpleTemplateEngine;
-import groovy.text.Template;
-import lombok.Getter;
-import lombok.extern.slf4j.Slf4j;
-import org.codehaus.groovy.control.CompilationFailedException;
-import org.springframework.stereotype.Service;
-
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-
-@Slf4j
-@Service
-public class ElasticQueryFactory implements IElasticQueryFactory {
-
-    private final SimpleTemplateEngine templateEngine;
-    @Getter
-    private final Map<String, Object> context;
-
-    public ElasticQueryFactory() {
-        this.templateEngine = new SimpleTemplateEngine();
-        this.context = new HashMap<>();
-    }
-
-    @Override
-    public String populateQuery(String query, Map<String, Object> queryContext) {
-        String populatedQuery = query;
-        try {
-            queryContext.putAll(context);
-            Template template = templateEngine.createTemplate(query);
-            populatedQuery = template.make(queryContext).toString();
-        } catch (CompilationFailedException | ClassNotFoundException | IOException e) {
-            log.error("Cannot populate template from string query", e);
-        }
-        return populatedQuery;
-    }
-}
+package com.netgrif.application.engine.elastic.service;
+
+import com.netgrif.application.engine.elastic.service.interfaces.IElasticQueryFactory;
+import org.springframework.stereotype.Service;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Service
+public class ElasticQueryFactory implements IElasticQueryFactory {
+
+    // ${me}, ${today}, ${now}, ${loggedUser.id}
+    private static final Pattern VAR = Pattern.compile("\\$\\{([a-zA-Z_][a-zA-Z0-9_]*)(?:\\.([a-zA-Z_][a-zA-Z0-9_]*))?}");
+    private final Map<String, Object> context = new HashMap<>();
+
+    @Override
+    public String populateQuery(String query, Map<String, Object> queryContext) {
+        if (query == null || query.isEmpty()) return query;
+        Map<String, Object> ctx = new HashMap<>(context); // no side-effects
+        if (queryContext != null) ctx.putAll(queryContext);
+        Matcher m = VAR.matcher(query);
+        StringBuffer out = new StringBuffer();
+        while (m.find()) {
+            String var = m.group(1);
+            String prop = m.group(2);
+            Object value = switch (var) {
+                case "me" -> ctx.get("me");
+                case "today" -> ctx.get("today");
+                case "now" -> ctx.get("now");
+                case "loggedUser" -> "id".equals(prop) ? ctx.get("me") : null;
+                default -> null; // unknown variables render empty
+            };
+            String replacement = value == null ? "" : value.toString();
+            m.appendReplacement(out, Matcher.quoteReplacement(replacement));
+        }
+        m.appendTail(out);
+        return out.toString();
+    }
+}
  • Input map no longer mutated.
  • Internal context not exposed.
  • No code execution; only whitelisted variables allowed.

If richer functions are required (e.g., plusDays), implement explicit helpers rather than evaluating code.

I can extend this to support plusDays/plusHours safely if needed.

🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticQueryFactory.java
around lines 19 to 39, remove use of Groovy SimpleTemplateEngine and replace
populateQuery with a safe, dependency-free placeholder substitution: do not
evaluate code, do not mutate the input queryContext, and do not expose or leak
the internal mutable context. Implement populateQuery to create a new combined
map (copy of internal context + a shallow copy of queryContext), restrict
substitutions to a whitelist of allowed variable names (keys present in the
combined map), and replace occurrences of ${key} with the string value of that
key only; if a key is missing leave the placeholder or replace with empty string
per current behavior. Make the internal context unmodifiable (remove or change
@Getter to return an unmodifiable view) and ensure no dynamic code execution or
class loading can occur; add explicit helper functions if richer transformations
(e.g., plusDays) are required instead of evaluating expressions.

Comment on lines +5 to +7
public interface IElasticQueryFactory {
String populateQuery(String query, Map<String, Object> queryContext);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Constrain the template inputs

Accepting an arbitrary Map broadens attack surface and makes validation hard. Prefer a typed context (e.g., QueryVariables with me, today, now, loggedUserId) to whitelist allowed values.

Comment on lines +47 to +48
public boolean dynamicQuery;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

dynamicQuery is never parsed in the Map constructor

Requests built via the Map-based ctor will silently ignore the flag. Parse it to avoid unexpected defaults.

Apply this diff inside the constructor:

@@
         if (request.containsKey("query") && request.get("query") instanceof String) {
             this.query = (String) request.get("query");
         }
+        if (request.containsKey("dynamicQuery")) {
+            Object dq = request.get("dynamicQuery");
+            this.dynamicQuery = dq instanceof Boolean ? (Boolean) dq : Boolean.parseBoolean(String.valueOf(dq));
+        }
         if (request.containsKey("stringId") && request.get("stringId") instanceof List) {
             this.stringId = (List<String>) request.get("stringId");
         }

Also applies to: 64-106

Comment on lines +101 to +121
String query1 = '${String.valueOf(java.sql.Timestamp.valueOf(now.plus(java.time.Duration.parse("P1D"))).getTime())}'
String resultQuery1 = String.valueOf(Timestamp.valueOf(LocalDateTime.now() + Duration.parse("P1D")).getTime())

assert queryFactory.populateQuery(query1, context) <= resultQuery1;

String query2 = 'creationDate:${today.toString()}';
String res2 = LocalDate.now().toString()
String resultQuery2 = "creationDate:" + res2;

assert queryFactory.populateQuery(query2, context) == resultQuery2;

String query3 = 'author:${loggedUser.id}'
String resultQuery3 = "author:" + user.id;

assert queryFactory.populateQuery(query3, context) == resultQuery3;

String query4 = 'author:${me}';
String resultQuery4 = "author:" + user.id;

assert queryFactory.populateQuery(query4, context) == resultQuery4;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Tests assume executing code in templates; compare numbers as strings

If we switch to safe placeholders, these tests must change. Also, compare numeric values as numbers, not strings.

Apply these minimal fixes now:

-        String query1 = '${String.valueOf(java.sql.Timestamp.valueOf(now.plus(java.time.Duration.parse("P1D"))).getTime())}'
-        String resultQuery1 = String.valueOf(Timestamp.valueOf(LocalDateTime.now() + Duration.parse("P1D")).getTime())
-
-        assert queryFactory.populateQuery(query1, context) <= resultQuery1;
+        // If supporting only placeholders, drop code execution and assert variables are rendered
+        String query1 = 'ts:${now}'
+        String res1 = context.get("now").toString()
+        assert queryFactory.populateQuery(query1, context) == "ts:" + res1;
@@
-        assert queryFactory.populateQuery(query3, context) == resultQuery3;
+        assert queryFactory.populateQuery(query3, context) == resultQuery3;
@@
-        assert queryFactory.populateQuery(query4, context) == resultQuery4;
+        assert queryFactory.populateQuery(query4, context) == resultQuery4;

If you must keep arithmetic, I can add explicit helper functions (e.g., ${nowPlusDays(1)}) and update tests accordingly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
String query1 = '${String.valueOf(java.sql.Timestamp.valueOf(now.plus(java.time.Duration.parse("P1D"))).getTime())}'
String resultQuery1 = String.valueOf(Timestamp.valueOf(LocalDateTime.now() + Duration.parse("P1D")).getTime())
assert queryFactory.populateQuery(query1, context) <= resultQuery1;
String query2 = 'creationDate:${today.toString()}';
String res2 = LocalDate.now().toString()
String resultQuery2 = "creationDate:" + res2;
assert queryFactory.populateQuery(query2, context) == resultQuery2;
String query3 = 'author:${loggedUser.id}'
String resultQuery3 = "author:" + user.id;
assert queryFactory.populateQuery(query3, context) == resultQuery3;
String query4 = 'author:${me}';
String resultQuery4 = "author:" + user.id;
assert queryFactory.populateQuery(query4, context) == resultQuery4;
}
// If supporting only placeholders, drop code execution and assert variables are rendered
String query1 = 'ts:${now}'
String res1 = context.get("now").toString()
assert queryFactory.populateQuery(query1, context) == "ts:" + res1;
String query2 = 'creationDate:${today.toString()}';
String res2 = LocalDate.now().toString()
String resultQuery2 = "creationDate:" + res2;
assert queryFactory.populateQuery(query2, context) == resultQuery2;
String query3 = 'author:${loggedUser.id}'
String resultQuery3 = "author:" + user.id;
assert queryFactory.populateQuery(query3, context) == resultQuery3;
String query4 = 'author:${me}';
String resultQuery4 = "author:" + user.id;
assert queryFactory.populateQuery(query4, context) == resultQuery4;
}
🤖 Prompt for AI Agents
In
application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticQueryFactoryTest.groovy
around lines 101-121, the tests currently assume template expressions execute
Java code and compare numeric results as strings; update the tests to use the
safe placeholder behavior and compare numbers as numbers: replace embedded Java
expressions in query1 with the safe placeholder (or helper like nowPlusDays(1)
if you introduce it), then parse the populated timestamp string into a long (or
convert the expected Timestamp to a long) and assert numeric comparison (<=) on
longs instead of string comparison; keep creationDate and author/me checks as
string comparisons but ensure they use the placeholder names the factory will
populate (e.g., today, loggedUser.id or me mapping to user.id); if you choose to
add helper functions (nowPlusDays), update the test to call that helper
placeholder rather than inline Java code.

@machacjozef machacjozef changed the base branch from release/7.0.0-rev9 to release/7.0.0-rev8 November 9, 2025 11:12
try {
queryContext.putAll(context);
Template template = templateEngine.createTemplate(query);
populatedQuery = template.make(queryContext).toString();
Copy link
Member

@renczesstefan renczesstefan Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a security risk. I think it needs to be discussed.

@machacjozef machacjozef changed the base branch from release/7.0.0-rev8 to release/7.0.0-rev9 November 10, 2025 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants