Compare commits

...

2 Commits

Author SHA1 Message Date
Evan
bade51a42a fix(ag-grid-table): reject null/empty IN_RANGE bounds and tighten tests
Number(null) and Number('') both coerce to 0, so a null/empty filterTo
would silently produce "BETWEEN x AND 0" instead of dropping the clause.
Explicitly reject non-coercible (null/empty) bounds before coercion.

Also strengthen the converter tests to assert the surviving sibling
condition and the full compound WHERE clause, so they fail if filter
composition or the upper bound regresses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 08:29:47 -07:00
Claude Code
ace7f72f29 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 <noreply@anthropic.com>
2026-06-01 16:18:27 -07:00
2 changed files with 71 additions and 1 deletions

View File

@@ -379,7 +379,23 @@ 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.
// Reject null/empty bounds explicitly: Number(null) and Number('') both
// coerce to 0, which would otherwise produce a misleading BETWEEN ... AND 0.
const isCoercibleBound = (bound: FilterValue): boolean =>
(typeof bound === 'number' || typeof bound === 'string') && bound !== '';
if (!isCoercibleBound(value) || !isCoercibleBound(filterTo)) {
return '';
}
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!);

View File

@@ -771,6 +771,60 @@ 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 survives unchanged.
expect(result.complexWhere ?? '').not.toContain('1=1');
expect(result.complexWhere ?? '').not.toContain('BETWEEN');
expect(result.complexWhere).toBe('age > 5');
});
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);
// Assert the full compound clause so the upper bound and the sibling
// condition are both validated, not just the BETWEEN fragment.
expect(result.complexWhere).toBe('(age BETWEEN 18 AND 65 AND age < 100)');
});
});
describe('Edge cases', () => {