You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/01 23:18:45 UTC

(superset) branch fix/ag-grid-in-range-numeric-validation created (now ace7f72f296)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a change to branch fix/ag-grid-in-range-numeric-validation
in repository https://gitbox.apache.org/repos/asf/superset.git


      at ace7f72f296 fix(ag-grid-table): validate numeric bounds in IN_RANGE filter clause

This branch includes the following new commits:

     new ace7f72f296 fix(ag-grid-table): validate numeric bounds in IN_RANGE filter clause

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix(ag-grid-table): validate numeric bounds in IN_RANGE filter clause

Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch fix/ag-grid-in-range-numeric-validation
in repository https://gitbox.apache.org/repos/asf/superset.git

commit ace7f72f2966c5bf1a7d76034a5e6bf24351c842
Author: Claude Code <no...@anthropic.com>
AuthorDate: Mon Jun 1 16:18:27 2026 -0700

    fix(ag-grid-table): validate numeric bounds in IN_RANGE filter clause
    
    simpleFilterToWhereClause interpolated the IN_RANGE (BETWEEN) bounds directly
    into the WHERE clause string without escaping or type validation, unlike the
    ILIKE and string-comparison branches which escape via escapeSQLString. The
    range values come from the AG Grid filter model, which is client-controlled
    and serialized into the query, and filterTo was never validated at all.
    
    Coerce both bounds with Number() and emit the BETWEEN clause only when both
    are finite, dropping the condition otherwise. Numeric strings from serialized
    filter state still work; non-numeric input can no longer be concatenated as
    raw SQL. Date ranges are unaffected (handled separately and normalized through
    Date.toISOString()).
    
    Add regression tests: a compound inRange filter with a SQL payload in the
    bounds is dropped, and numeric/numeric-string bounds still produce the
    expected BETWEEN clause.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
 .../src/utils/agGridFilterConverter.ts             | 11 ++++-
 .../test/utils/agGridFilterConverter.test.ts       | 51 ++++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts
index 6389f61ca8e..9b5246cf3cc 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts
@@ -379,7 +379,16 @@ function simpleFilterToWhereClause(
   }
 
   if (type === FILTER_OPERATORS.IN_RANGE && filterTo !== undefined) {
-    return `${columnName} ${SQL_OPERATORS.BETWEEN} ${value} AND ${filterTo}`;
+    // BETWEEN bounds are interpolated directly into the WHERE clause, so only
+    // accept finite numeric bounds (date ranges are handled separately above).
+    // Numeric strings from serialized filter state are coerced; anything that
+    // isn't a finite number is dropped rather than concatenated as raw SQL.
+    const from = Number(value);
+    const to = Number(filterTo);
+    if (!Number.isFinite(from) || !Number.isFinite(to)) {
+      return '';
+    }
+    return `${columnName} ${SQL_OPERATORS.BETWEEN} ${from} AND ${to}`;
   }
 
   const formattedValue = formatValueForOperator(type, value!);
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts
index a6f95a2932e..b386e7e7829 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts
@@ -771,6 +771,57 @@ describe('agGridFilterConverter', () => {
       // Should reject column names longer than 255 characters
       expect(result.simpleFilters).toHaveLength(0);
     });
+
+    test('should drop inRange bounds that are not numeric', () => {
+      const filterModel: AgGridFilterModel = {
+        age: {
+          filterType: 'number',
+          operator: 'AND',
+          condition1: {
+            filterType: 'number',
+            type: 'inRange',
+            filter: '0 AND 1=1--',
+            filterTo: '100',
+          },
+          condition2: {
+            filterType: 'number',
+            type: 'greaterThan',
+            filter: 5,
+          },
+        } as AgGridCompoundFilter,
+      };
+
+      const result = convertAgGridFiltersToSQL(filterModel);
+
+      // The malicious range condition is dropped, so its payload never reaches
+      // the WHERE clause; the sibling numeric condition is unaffected.
+      expect(result.complexWhere ?? '').not.toContain('1=1');
+      expect(result.complexWhere ?? '').not.toContain('BETWEEN');
+    });
+
+    test('should keep numeric inRange bounds (including numeric strings)', () => {
+      const filterModel: AgGridFilterModel = {
+        age: {
+          filterType: 'number',
+          operator: 'AND',
+          condition1: {
+            filterType: 'number',
+            type: 'inRange',
+            filter: '18',
+            filterTo: 65,
+          },
+          condition2: {
+            filterType: 'number',
+            type: 'lessThan',
+            filter: 100,
+          },
+        } as AgGridCompoundFilter,
+      };
+
+      const result = convertAgGridFiltersToSQL(filterModel);
+
+      expect(result.complexWhere).toContain('BETWEEN 18 AND 65');
+    });
   });
 
   describe('Edge cases', () => {