fix: prevent FIND_RECORDS from silently dropping unresolved filter variables (#18814)

## Summary

Fixes #18744 — The workflow FIND_RECORDS action silently drops filter
conditions when a variable resolves to null/empty, causing the query to
return **all records** instead of erroring.

**Root cause (three compounding layers):**

1. **`variable-resolver.ts`** — `resolveString` returns `undefined` when
a variable lookup fails (e.g., `{{steps.trigger.output.userId}}` where
`userId` doesn't exist in context). The return type says `string` but
`evalFromContext` actually returns `undefined` at runtime.

2. **`checkIfShouldSkipFiltering.ts`** — Treats `undefined`/`null`/`""`
values as "skip this filter." This is correct for the **UI filter
builder** (user hasn't finished typing), but wrong for **workflow
execution** (variable resolution failed = misconfigured workflow).

3. **`find-records.workflow-action.ts`** — When all filters are silently
skipped, `computeRecordGqlOperationFilter` returns `{}` (match
everything). The query runs with no filter, returning all records —
silently succeeding with wrong results.

## Fix

Added validation in `find-records.workflow-action.ts` **after**
`resolveInput` but **before** `computeRecordGqlOperationFilter`. For
each filter with a value-requiring operand (i.e., not IS_EMPTY,
IS_NOT_EMPTY, IS_IN_PAST, IS_IN_FUTURE, IS_TODAY), if the resolved value
is `undefined`, `null`, or `""`, throw `INVALID_STEP_INPUT` with a
descriptive error message.

**Why this approach:**
- Scoped to the workflow executor — does **not** break the UI filter
builder's intentional skip-on-empty behavior
- Does not change shared utilities (`checkIfShouldSkipFiltering`,
`resolveInput`) used across the app
- Fails fast with a clear error instead of silently returning wrong data
- 1 file changed, 23 lines added

## Test plan

- [x] Backend typecheck passes
- [x] oxlint passes (0 warnings, 0 errors)
- [x] Prettier passes
- [ ] Manual: Create a workflow with FIND_RECORDS using a variable that
doesn't exist → should error with "Filter condition has an empty value
after variable resolution" instead of returning all records
- [ ] Manual: Create a workflow with FIND_RECORDS using IS_EMPTY operand
(no value needed) → should still work correctly

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
oniani1
2026-03-21 21:27:01 +04:00
committed by GitHub
parent 03a2abb305
commit d69e4d7008
16 changed files with 370 additions and 92 deletions
@@ -0,0 +1,72 @@
import { ViewFilterOperand } from '@/types/ViewFilterOperand';
import { filterOutInvalidRecordFilters } from '../filterOutInvalidRecordFilters';
describe('filterOutInvalidRecordFilters', () => {
it('should keep filters with valid values', () => {
const filters = [
{ operand: ViewFilterOperand.IS, value: 'some-value' },
{ operand: ViewFilterOperand.CONTAINS, value: 'search-term' },
];
expect(filterOutInvalidRecordFilters(filters)).toEqual(filters);
});
it('should remove filters with empty values for value-requiring operands', () => {
const filters = [
{ operand: ViewFilterOperand.IS, value: '' },
{ operand: ViewFilterOperand.CONTAINS, value: 'keep-me' },
{ operand: ViewFilterOperand.IS_NOT, value: '[]' },
];
expect(filterOutInvalidRecordFilters(filters)).toEqual([
{ operand: ViewFilterOperand.CONTAINS, value: 'keep-me' },
]);
});
it('should keep filters with operands that do not require a value', () => {
const filters = [
{ operand: ViewFilterOperand.IS_EMPTY, value: '' },
{ operand: ViewFilterOperand.IS_NOT_EMPTY, value: '' },
{ operand: ViewFilterOperand.IS_NOT_NULL, value: '' },
{ operand: ViewFilterOperand.IS_IN_PAST, value: '' },
{ operand: ViewFilterOperand.IS_IN_FUTURE, value: '' },
{ operand: ViewFilterOperand.IS_TODAY, value: '' },
];
expect(filterOutInvalidRecordFilters(filters)).toEqual(filters);
});
it('should return an empty array when all filters are invalid', () => {
const filters = [
{ operand: ViewFilterOperand.IS, value: '' },
{
operand: ViewFilterOperand.IS_NOT,
value: undefined as unknown as string,
},
];
expect(filterOutInvalidRecordFilters(filters)).toEqual([]);
});
it('should return an empty array for empty input', () => {
expect(filterOutInvalidRecordFilters([])).toEqual([]);
});
it('should preserve extra properties on the filter objects', () => {
const filters = [
{
id: 'filter-1',
operand: ViewFilterOperand.IS,
value: 'valid',
fieldMetadataId: 'field-1',
},
];
const result = filterOutInvalidRecordFilters(filters);
expect(result).toEqual(filters);
expect(result[0].id).toBe('filter-1');
expect(result[0].fieldMetadataId).toBe('field-1');
});
});
@@ -0,0 +1,48 @@
import { ViewFilterOperand } from '@/types/ViewFilterOperand';
import { isRecordFilterOperandExpectingValue } from '../isRecordFilterOperandExpectingValue';
describe('isRecordFilterOperandExpectingValue', () => {
const operandsNotExpectingValue: ViewFilterOperand[] = [
ViewFilterOperand.IS_NOT_NULL,
ViewFilterOperand.IS_EMPTY,
ViewFilterOperand.IS_NOT_EMPTY,
ViewFilterOperand.IS_IN_PAST,
ViewFilterOperand.IS_IN_FUTURE,
ViewFilterOperand.IS_TODAY,
];
const operandsExpectingValue: ViewFilterOperand[] = [
ViewFilterOperand.IS,
ViewFilterOperand.IS_NOT,
ViewFilterOperand.CONTAINS,
ViewFilterOperand.DOES_NOT_CONTAIN,
ViewFilterOperand.GREATER_THAN_OR_EQUAL,
ViewFilterOperand.LESS_THAN_OR_EQUAL,
ViewFilterOperand.IS_BEFORE,
ViewFilterOperand.IS_AFTER,
ViewFilterOperand.IS_RELATIVE,
ViewFilterOperand.VECTOR_SEARCH,
];
it.each(operandsNotExpectingValue)(
'should return false for %s',
(operand) => {
expect(isRecordFilterOperandExpectingValue(operand)).toBe(false);
},
);
it.each(operandsExpectingValue)('should return true for %s', (operand) => {
expect(isRecordFilterOperandExpectingValue(operand)).toBe(true);
});
it('should cover all ViewFilterOperand values', () => {
const allOperands = Object.values(ViewFilterOperand);
const coveredOperands = [
...operandsNotExpectingValue,
...operandsExpectingValue,
];
expect(coveredOperands.sort()).toEqual(allOperands.sort());
});
});
@@ -0,0 +1,120 @@
import { ViewFilterOperand } from '@/types/ViewFilterOperand';
import { isRecordFilterValueValid } from '../isRecordFilterValueValid';
describe('isRecordFilterValueValid', () => {
describe('operands not expecting a value', () => {
const operandsNotExpectingValue = [
ViewFilterOperand.IS_NOT_NULL,
ViewFilterOperand.IS_EMPTY,
ViewFilterOperand.IS_NOT_EMPTY,
ViewFilterOperand.IS_IN_PAST,
ViewFilterOperand.IS_IN_FUTURE,
ViewFilterOperand.IS_TODAY,
];
it.each(operandsNotExpectingValue)(
'should return true for %s regardless of value',
(operand) => {
expect(isRecordFilterValueValid({ operand, value: '' })).toBe(true);
expect(
isRecordFilterValueValid({
operand,
value: undefined as unknown as string,
}),
).toBe(true);
expect(isRecordFilterValueValid({ operand, value: '[]' })).toBe(true);
expect(isRecordFilterValueValid({ operand, value: 'some-value' })).toBe(
true,
);
},
);
});
describe('operands expecting a value', () => {
const operandsExpectingValue = [
ViewFilterOperand.IS,
ViewFilterOperand.IS_NOT,
ViewFilterOperand.CONTAINS,
ViewFilterOperand.DOES_NOT_CONTAIN,
ViewFilterOperand.GREATER_THAN_OR_EQUAL,
ViewFilterOperand.LESS_THAN_OR_EQUAL,
ViewFilterOperand.IS_BEFORE,
ViewFilterOperand.IS_AFTER,
ViewFilterOperand.IS_RELATIVE,
];
it.each(operandsExpectingValue)(
'should return true for %s with a valid value',
(operand) => {
expect(isRecordFilterValueValid({ operand, value: 'some-value' })).toBe(
true,
);
},
);
it.each(operandsExpectingValue)(
'should return false for %s with empty string',
(operand) => {
expect(isRecordFilterValueValid({ operand, value: '' })).toBe(false);
},
);
it.each(operandsExpectingValue)(
'should return false for %s with undefined',
(operand) => {
expect(
isRecordFilterValueValid({
operand,
value: undefined as unknown as string,
}),
).toBe(false);
},
);
it.each(operandsExpectingValue)(
'should return false for %s with empty array string "[]"',
(operand) => {
expect(isRecordFilterValueValid({ operand, value: '[]' })).toBe(false);
},
);
});
describe('edge cases', () => {
it('should return true for "0" as a valid value', () => {
expect(
isRecordFilterValueValid({
operand: ViewFilterOperand.IS,
value: '0',
}),
).toBe(true);
});
it('should return true for "false" as a valid value', () => {
expect(
isRecordFilterValueValid({
operand: ViewFilterOperand.IS,
value: 'false',
}),
).toBe(true);
});
it('should return true for whitespace-only string', () => {
expect(
isRecordFilterValueValid({
operand: ViewFilterOperand.IS,
value: ' ',
}),
).toBe(true);
});
it('should return true for a non-empty array string', () => {
expect(
isRecordFilterValueValid({
operand: ViewFilterOperand.IS,
value: '["value1","value2"]',
}),
).toBe(true);
});
});
});
@@ -1,27 +0,0 @@
import { ViewFilterOperand as RecordFilterOperand } from '@/types';
import { isDefined } from '@/utils';
import { isEmptinessOperand, type RecordFilter } from '@/utils/filter';
type CheckIfShouldSkipFilteringParams = {
recordFilter: Pick<RecordFilter, 'operand' | 'value'>;
};
export const checkIfShouldSkipFiltering = ({
recordFilter,
}: CheckIfShouldSkipFilteringParams) => {
const isAnEmptinessOperand = isEmptinessOperand(recordFilter.operand);
const isDateOperandWithoutValue = [
RecordFilterOperand.IS_IN_PAST,
RecordFilterOperand.IS_IN_FUTURE,
RecordFilterOperand.IS_TODAY,
].includes(recordFilter.operand);
const isFilterValueEmpty =
!isDefined(recordFilter.value) || recordFilter.value === '';
const shouldSkipFiltering =
!isAnEmptinessOperand && !isDateOperandWithoutValue && isFilterValueEmpty;
return shouldSkipFiltering;
};
@@ -0,0 +1,11 @@
import { type ViewFilterOperand } from '@/types';
import { isRecordFilterValueValid } from './isRecordFilterValueValid';
export const filterOutInvalidRecordFilters = <
T extends { operand: ViewFilterOperand; value: string },
>(
recordFilters: T[],
): T[] => {
return recordFilters.filter(isRecordFilterValueValid);
};
@@ -1,11 +1,13 @@
export * from './checkIfShouldComputeEmptinessFilter';
export * from './checkIfShouldSkipFiltering';
export * from './filterOutInvalidRecordFilters';
export * from './compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForEmails';
export * from './compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForLinks';
export * from './computeEmptyGqlOperationFilterForEmails';
export * from './computeEmptyGqlOperationFilterForLinks';
export * from './computeRecordGqlOperationFilter';
export * from './isEmptinessOperand';
export * from './isRecordFilterOperandExpectingValue';
export * from './isRecordFilterValueValid';
export * from './turnAnyFieldFilterIntoRecordGqlFilter';
export * from './turnRecordFilterGroupIntoGqlOperationFilter';
export * from './turnRecordFilterIntoGqlOperationFilter';
@@ -0,0 +1,17 @@
import { ViewFilterOperand } from '@/types';
export const isRecordFilterOperandExpectingValue = (
operand: ViewFilterOperand,
): boolean => {
switch (operand) {
case ViewFilterOperand.IS_NOT_NULL:
case ViewFilterOperand.IS_EMPTY:
case ViewFilterOperand.IS_NOT_EMPTY:
case ViewFilterOperand.IS_IN_PAST:
case ViewFilterOperand.IS_IN_FUTURE:
case ViewFilterOperand.IS_TODAY:
return false;
default:
return true;
}
};
@@ -0,0 +1,19 @@
import { type ViewFilterOperand } from '@/types';
import { isDefined } from '@/utils';
import { isRecordFilterOperandExpectingValue } from './isRecordFilterOperandExpectingValue';
export const isRecordFilterValueValid = (recordFilter: {
operand: ViewFilterOperand;
value: string;
}): boolean => {
if (!isRecordFilterOperandExpectingValue(recordFilter.operand)) {
return true;
}
return (
isDefined(recordFilter.value) &&
recordFilter.value !== '' &&
recordFilter.value !== '[]'
);
};
@@ -40,12 +40,12 @@ import {
import { type DateTimeFilter } from '@/types/RecordGqlOperationFilter';
import {
checkIfShouldComputeEmptinessFilter,
checkIfShouldSkipFiltering,
CustomError,
getFilterTypeFromFieldType,
getNextPeriodStart,
getPeriodStart,
isDefined,
isRecordFilterValueValid,
resolveDateFilter,
resolveDateTimeFilter,
resolveRelativeDateFilterStringified,
@@ -83,9 +83,7 @@ export const turnRecordFilterIntoRecordGqlOperationFilter = ({
return;
}
const shouldSkipFiltering = checkIfShouldSkipFiltering({ recordFilter });
if (shouldSkipFiltering) {
if (!isRecordFilterValueValid(recordFilter)) {
return;
}
+3 -1
View File
@@ -51,7 +51,6 @@ export { isFieldMetadataSelectKind } from './fieldMetadata/isFieldMetadataSelect
export { isFieldMetadataTextKind } from './fieldMetadata/isFieldMetadataTextKind';
export { extractFolderPathFilenameAndTypeOrThrow } from './files/extractFolderPathFilenameAndTypeOrThrow.util';
export { checkIfShouldComputeEmptinessFilter } from './filter/checkIfShouldComputeEmptinessFilter';
export { checkIfShouldSkipFiltering } from './filter/checkIfShouldSkipFiltering';
export { computeGqlOperationFilterForEmails } from './filter/compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForEmails';
export { computeGqlOperationFilterForLinks } from './filter/compute-record-gql-operation-filter/for-composite-field/computeGqlOperationFilterForLinks';
export { computeEmptyGqlOperationFilterForEmails } from './filter/computeEmptyGqlOperationFilterForEmails';
@@ -89,7 +88,10 @@ export { resolveRelativeDateTimeFilter } from './filter/dates/utils/resolveRelat
export { resolveRelativeDateTimeFilterStringified } from './filter/dates/utils/resolveRelativeDateTimeFilterStringified';
export { subUnitFromDateTime } from './filter/dates/utils/subUnitFromDateTime';
export { subUnitFromZonedDateTime } from './filter/dates/utils/subUnitFromZonedDateTime';
export { filterOutInvalidRecordFilters } from './filter/filterOutInvalidRecordFilters';
export { isEmptinessOperand } from './filter/isEmptinessOperand';
export { isRecordFilterOperandExpectingValue } from './filter/isRecordFilterOperandExpectingValue';
export { isRecordFilterValueValid } from './filter/isRecordFilterValueValid';
export { turnAnyFieldFilterIntoRecordGqlFilter } from './filter/turnAnyFieldFilterIntoRecordGqlFilter';
export type {
RecordFilter,