Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ public class ElasticQueryConstants {
* Should be replaced by user id in elastic query string queries
*/
public static final String USER_ID_TEMPLATE = "<<me>>";
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";
Comment on lines +8 to +11
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.

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import co.elastic.clients.elasticsearch._types.query_dsl.TermsQueryField;
import co.elastic.clients.elasticsearch._types.query_dsl.QueryBuilders;
import com.netgrif.application.engine.configuration.properties.DataConfigurationProperties;
import com.netgrif.application.engine.elastic.service.interfaces.IElasticQueryFactory;
import com.netgrif.application.engine.objects.auth.domain.LoggedUser;
import com.netgrif.application.engine.objects.elastic.domain.ElasticCase;
import com.netgrif.application.engine.elastic.domain.ElasticCaseRepository;
Expand Down Expand Up @@ -39,6 +40,8 @@
import org.springframework.data.elasticsearch.core.query.Order;
import org.springframework.stereotype.Service;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.*;
import java.util.function.BinaryOperator;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -86,6 +89,9 @@ public void setElasticProperties(DataConfigurationProperties.ElasticsearchProper
this.elasticProperties = elasticProperties;
}

@Autowired
protected IElasticQueryFactory queryFactory;

@Override
public void remove(String caseId) {
executors.execute(caseId, () -> {
Expand Down Expand Up @@ -430,7 +436,17 @@ protected void buildStringQuery(CaseSearchRequest request, BoolQuery.Builder que
return;
}

String populatedQuery = request.query.replaceAll(ElasticQueryConstants.USER_ID_TEMPLATE, user.getId().toString());
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());
}
Comment on lines +439 to +449
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.


query.must(QueryStringQuery.of(builder -> builder.query(populatedQuery).allowLeadingWildcard(true).analyzeWildcard(true))._toQuery());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
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();
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.

} catch (CompilationFailedException | ClassNotFoundException | IOException e) {
log.error("Cannot populate template from string query", e);
}
return populatedQuery;
}
Comment on lines +19 to +39
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.


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.netgrif.application.engine.elastic.service.interfaces;

import java.util.Map;

public interface IElasticQueryFactory {
String populateQuery(String query, Map<String, Object> queryContext);
}
Comment on lines +5 to +7
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.

Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class CaseSearchRequest implements Serializable {

public String query;

public boolean dynamicQuery;

Comment on lines +47 to +48
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

@JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY)
public List<String> id;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package com.netgrif.application.engine.elastic;

import com.netgrif.application.engine.ApplicationEngine;
import com.netgrif.application.engine.TestHelper
import com.netgrif.application.engine.adapter.spring.auth.domain.AuthorityImpl
import com.netgrif.application.engine.adapter.spring.workflow.domain.QCase
import com.netgrif.application.engine.auth.service.UserService
import com.netgrif.application.engine.elastic.domain.ElasticQueryConstants
import com.netgrif.application.engine.elastic.service.interfaces.IElasticQueryFactory
import com.netgrif.application.engine.objects.auth.domain.AbstractUser
import com.netgrif.application.engine.objects.auth.domain.ActorTransformer
import com.netgrif.application.engine.objects.auth.domain.Authority
import com.netgrif.application.engine.objects.auth.domain.User
import com.netgrif.application.engine.objects.auth.domain.enums.UserState
import com.netgrif.application.engine.objects.petrinet.domain.PetriNet
import com.netgrif.application.engine.objects.petrinet.domain.VersionType
import com.netgrif.application.engine.objects.petrinet.domain.roles.ProcessRole;
import com.netgrif.application.engine.startup.ImportHelper;
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken
import org.springframework.security.core.Authentication
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.junit.jupiter.SpringExtension
import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.setup.MockMvcBuilders
import org.springframework.web.context.WebApplicationContext

import java.sql.Timestamp
import java.time.Duration
import java.time.LocalDate
import java.time.LocalDateTime

import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;

@ExtendWith(SpringExtension.class)
@ActiveProfiles(["test"])
@SpringBootTest(
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
classes = ApplicationEngine.class
)
@AutoConfigureMockMvc
class ElasticQueryFactoryTest {

public static final String USER_EMAIL = "test@mail.sk"
public static final String USER_PASSWORD = "password"

@Autowired
private TestHelper testHelper

@Autowired
protected IElasticQueryFactory queryFactory;

@Autowired
private UserService userService

@Autowired
private ImportHelper importHelper

@Autowired
private WebApplicationContext wac

private MockMvc mvc
Map<String, Authority> auths

@BeforeEach
void before() {
testHelper.truncateDbs()

mvc = MockMvcBuilders
.webAppContextSetup(wac)
.apply(springSecurity())
.build()

auths = importHelper.createAuthorities(["user": Authority.user, "admin": Authority.admin])

importHelper.createUser(new User(firstName: "Test", lastName: "Integration", email: USER_EMAIL, password: USER_PASSWORD, state: UserState.ACTIVE),
[auths.get("user"), auths.get("admin")] as Authority[], [] as ProcessRole[])
}

@Test
void dynamicQueryTest() {
AbstractUser realUser = userService.findByEmail(USER_EMAIL, null)

UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken(ActorTransformer.toLoggedUser(realUser), ActorTransformer.toLoggedUser(realUser).getPassword(), ActorTransformer.toLoggedUser(realUser).getAuthoritySet() as Set<AuthorityImpl>);
SecurityContextHolder.getContext().setAuthentication(token)

def user = userService.getLoggedUser()
def context = 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
));

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;
}
Comment on lines +101 to +121
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.

}
Loading