Skip to content

Commit 084f282

Browse files
wyattwalterclaude
andauthored
fix(interfaces): validate WHERE clause column names in UQI filtering (#42224)
## Description Hardens column handling in the WHERE clause of UQI in-memory filtering (used by the Google Sheets and Amazon S3 plugins). - `FilterDataServiceCE.filterDataNew()` checks every column referenced in the WHERE condition, including nested AND/OR groups, against the columns present in the data before building the query. Unknown columns return a validation error. - Quote characters in column identifiers are escaped in the generated filter expression. Advisory: [GHSA-cm7r-f7h3-q33p](https://github.com/appsmithorg/appsmith/security/advisories/GHSA-cm7r-f7h3-q33p) Part of https://linear.app/appsmith/issue/APP-15874 ## Testing - [x] Server unit tests New tests in `FilterDataServiceTest`: - `testWhereWithUnknownColumn_throwsException` - `testWhereWithUnknownColumnInNestedGroup_throwsException` - `testWhereWithColumnNameContainingQuote_throwsException` - `testWhereWithKnownColumn_filtersRows` - `testGenerateLogicalExpression_quotesColumnNameContainingQuote` ## Impact on existing instances - Working queries: no change. - A WHERE condition on a column that isn't in the data already failed; it now fails before the query is built, with `<column> not found in the known column names`, instead of an in-memory database error. - Rollback: restores the previous behavior. ## Automation /ok-to-test tags="@tag.All" ## Communication - [x] No 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/34604046075> > Commit: 5633f15 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=34604046075&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Fri, 11 Sep 2026 14:25:05 UTC <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Filtering now rejects conditions that reference unknown or invalid column paths. * Nested `AND` and `OR` conditions are validated consistently. * Column names containing quotation marks are handled safely when generating filter queries. * Valid filters continue to work as expected. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 179453b commit 084f282

2 files changed

Lines changed: 93 additions & 2 deletions

File tree

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/services/ce/FilterDataServiceCE.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ public ArrayNode filterDataNew(
144144

145145
validateProjectionColumns(uqiDataFilterParams.getProjectionColumns(), schema);
146146
validateSortByColumns(uqiDataFilterParams.getSortBy(), schema);
147+
validateConditionColumns(uqiDataFilterParams.getCondition(), schema);
147148

148149
String tableName = generateTable(schema);
149150
try {
@@ -867,6 +868,30 @@ public boolean validConditionList(List<Condition> conditionList, Map<String, Dat
867868
return true;
868869
}
869870

871+
public void validateConditionColumns(Condition condition, Map<String, DataType> schema) {
872+
if (condition == null) {
873+
return;
874+
}
875+
876+
ConditionalOperator operator = condition.getOperator();
877+
if (operator == ConditionalOperator.AND || operator == ConditionalOperator.OR) {
878+
Object value = condition.getValue();
879+
if (value instanceof List) {
880+
List<Condition> childConditions = (List<Condition>) value;
881+
for (Condition child : childConditions) {
882+
validateConditionColumns(child, schema);
883+
}
884+
}
885+
} else {
886+
String path = condition.getPath();
887+
if (StringUtils.isNotEmpty(path) && !schema.containsKey(path)) {
888+
throw new AppsmithPluginException(
889+
AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR,
890+
path + " not found in the known column names :" + schema.keySet());
891+
}
892+
}
893+
}
894+
870895
public String generateLogicalExpression(
871896
List<Condition> conditions,
872897
List<PreparedStatementValueDTO> values,
@@ -898,9 +923,10 @@ public String generateLogicalExpression(
898923
sb.append(" " + logicOp);
899924
}
900925
if (StringUtils.isNotEmpty(path)) {
926+
String escapedPath = path.replace("\"", "\"\"");
901927
if (value == null || value.equals(StringUtils.EMPTY)) {
902928
sb.append(" ( ");
903-
sb.append("\"" + path + "\"");
929+
sb.append("\"" + escapedPath + "\"");
904930
sb.append(" ");
905931
if (Set.of(
906932
ConditionalOperator.EQ,
@@ -926,7 +952,7 @@ public String generateLogicalExpression(
926952
operator + " is not supported currently for filtering.");
927953
}
928954
sb.append(" ( ");
929-
sb.append("\"" + path + "\"");
955+
sb.append("\"" + escapedPath + "\"");
930956
sb.append(" ");
931957
sb.append(sqlOp);
932958
sb.append(" ");

app/server/appsmith-interfaces/src/test/java/com/appsmith/external/services/FilterDataServiceTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.appsmith.external.constants.ConditionalOperator;
44
import com.appsmith.external.constants.DataType;
5+
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
56
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
67
import com.appsmith.external.models.Condition;
78
import com.appsmith.external.models.UQIDataFilterParams;
@@ -1504,4 +1505,68 @@ public void testInvalidProjectionDoesNotLeakTable() throws IOException {
15041505
filterDataService.filterDataNew(items, new UQIDataFilterParams(null, List.of("id"), null, null));
15051506
assertEquals(2, result.size());
15061507
}
1508+
1509+
private Condition whereClause(Map<String, Object>... children) {
1510+
return parseWhereClause(Map.of("condition", "AND", "children", List.of(children)));
1511+
}
1512+
1513+
private void assertUnknownColumnRejected(Condition condition, String columnName) throws IOException {
1514+
ArrayNode items = (ArrayNode) objectMapper.readTree(SIMPLE_DATA);
1515+
1516+
AppsmithPluginException exception = assertThrows(
1517+
AppsmithPluginException.class,
1518+
() -> filterDataService.filterDataNew(items, new UQIDataFilterParams(condition, null, null, null)));
1519+
1520+
assertEquals(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, exception.getError());
1521+
assertThat(exception.getMessage()).startsWith(columnName + " not found in the known column names");
1522+
}
1523+
1524+
@Test
1525+
public void testWhereWithUnknownColumn_throwsException() throws IOException {
1526+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "unknownColumn", "value", "1"));
1527+
1528+
assertUnknownColumnRejected(condition, "unknownColumn");
1529+
}
1530+
1531+
@Test
1532+
public void testWhereWithUnknownColumnInNestedGroup_throwsException() throws IOException {
1533+
Condition condition = whereClause(
1534+
Map.of("condition", "EQ", "key", "id", "value", "1"),
1535+
Map.of(
1536+
"condition",
1537+
"OR",
1538+
"children",
1539+
List.of(Map.of("condition", "EQ", "key", "unknownColumn", "value", ""))));
1540+
1541+
assertUnknownColumnRejected(condition, "unknownColumn");
1542+
}
1543+
1544+
@Test
1545+
public void testWhereWithColumnNameContainingQuote_throwsException() throws IOException {
1546+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "na\"me", "value", ""));
1547+
1548+
assertUnknownColumnRejected(condition, "na\"me");
1549+
}
1550+
1551+
@Test
1552+
public void testWhereWithKnownColumn_filtersRows() throws IOException {
1553+
ArrayNode items = (ArrayNode) objectMapper.readTree(SIMPLE_DATA);
1554+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "id", "value", "1"));
1555+
1556+
ArrayNode result = filterDataService.filterDataNew(items, new UQIDataFilterParams(condition, null, null, null));
1557+
1558+
assertEquals(1, result.size());
1559+
assertEquals(1, result.get(0).get("id").asInt());
1560+
}
1561+
1562+
@Test
1563+
public void testGenerateLogicalExpression_quotesColumnNameContainingQuote() {
1564+
Map<String, DataType> schema = Map.of("na\"me", DataType.STRING);
1565+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "na\"me", "value", "Alice"));
1566+
1567+
String expression = filterDataService.generateLogicalExpression(
1568+
(List<Condition>) condition.getValue(), new ArrayList<>(), schema, condition.getOperator());
1569+
1570+
assertThat(expression).isEqualTo(" ( \"na\"\"me\" = ? ) ");
1571+
}
15071572
}

0 commit comments

Comments
 (0)