From 0817bf4911d1ff3bfa68dc3e01509eed0b4aac4b Mon Sep 17 00:00:00 2001 From: Osmany Montero Date: Fri, 23 Oct 2026 16:45:58 +0100 Subject: [PATCH] fix[frontend+backend](alerts): row-to-filters emits only valid filters; invalid filters 400 not 500 (#2968) --- .../service/elasticsearch/SearchUtil.java | 16 +++++ .../SearchUtilInvalidFilterTest.java | 63 +++++++++++++++++++ .../row-to-filter/row-to-filters.component.ts | 35 +++++++++-- 3 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtilInvalidFilterTest.java diff --git a/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtil.java b/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtil.java index 9d9ebaf97..8f28ce058 100644 --- a/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtil.java +++ b/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtil.java @@ -38,6 +38,22 @@ public class SearchUtil { public static Query toQuery(List filters) { final String ctx = CLASSNAME + ".toQuery"; try { + // Upfront validation so a structurally invalid filter (e.g. an + // IS_ONE_OF whose value is not a list) fails fast with a 400 + // ApiException instead of a 500 after the query was half-built + // (#2968). FilterType.validate() uses Spring Assert, which throws + // IllegalArgumentException — convert it here to the 400 type. + // The per-operator build* methods re-validate unchanged. + if (!CollectionUtils.isEmpty(filters)) { + for (FilterType filter : filters) { + try { + filter.validate(); + } catch (IllegalArgumentException e) { + throw new ApiException("Invalid filter on field '" + filter.getField() + + "': " + e.getMessage(), HttpStatus.BAD_REQUEST); + } + } + } boolean hasShouldClauses = false; BoolQuery.Builder bool = new BoolQuery.Builder(); /*bool.filter(Query.of(q -> q.matchAll(MatchAllQuery.of(m -> m))));*/ diff --git a/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtilInvalidFilterTest.java b/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtilInvalidFilterTest.java new file mode 100644 index 000000000..9a5bd59ee --- /dev/null +++ b/backend/src/main/java/com/park/utmstack/service/elasticsearch/SearchUtilInvalidFilterTest.java @@ -0,0 +1,63 @@ +package com.park.utmstack.service.elasticsearch; + +import com.park.utmstack.domain.chart_builder.types.query.FilterType; +import com.park.utmstack.domain.chart_builder.types.query.OperatorType; +import com.park.utmstack.util.exceptions.ApiException; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; + +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * #2968: structurally invalid filters (IS_ONE_OF / IS_NOT_ONE_OF whose value + * is not a list) must surface as a 400 ApiException, not a 500 RuntimeException. + */ +public class SearchUtilInvalidFilterTest { + + private static FilterType f(OperatorType op, String field, Object value) { + return new FilterType(field, op, value); + } + + @Test + public void isOneOfWithObjectValueThrowsApiException400() { + // Row-to-filters used to emit the raw alert row fields: an object + // value (e.g. adversary = {host: ...}) reaches validate() as a + // LinkedHashMap. Must be a 400, not a 500. + ApiException ex = assertThrows(ApiException.class, + () -> SearchUtil.toQuery(List.of(f(OperatorType.IS_ONE_OF, "adversary", Map.of("host", "10.0.0.1"))))); + assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus()); + } + + @Test + public void isOneOfWithNullValueThrowsApiException400() { + // target was null on the alert row -> value null after sanitizing + // failed. Must be a 400, not a 500. + ApiException ex = assertThrows(ApiException.class, + () -> SearchUtil.toQuery(List.of(f(OperatorType.IS_ONE_OF, "target", null)))); + assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus()); + } + + @Test + public void isOneOfWithScalarValueThrowsApiException400() { + // echoes was a bare Integer (0) -> not a list -> 400. + ApiException ex = assertThrows(ApiException.class, + () -> SearchUtil.toQuery(List.of(f(OperatorType.IS_ONE_OF, "echoes", 0)))); + assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus()); + } + + @Test + public void isNotOneOfWithObjectValueThrowsApiException400() { + ApiException ex = assertThrows(ApiException.class, + () -> SearchUtil.toQuery(List.of(f(OperatorType.IS_NOT_ONE_OF, "impact", Map.of("confidentiality", 1))))); + assertEquals(HttpStatus.BAD_REQUEST, ex.getStatus()); + } + + @Test + public void isOneOfWithProperListStillBuildsQuery() { + // Sanity: a correctly-shaped filter keeps working. + assertDoesNotThrow(() -> SearchUtil.toQuery(List.of(f(OperatorType.IS_ONE_OF, "name", List.of("rule"))))); + } +} diff --git a/frontend/src/app/data-management/alert-management/shared/components/filters/row-to-filter/row-to-filters.component.ts b/frontend/src/app/data-management/alert-management/shared/components/filters/row-to-filter/row-to-filters.component.ts index 7a76cde7f..31e93db37 100644 --- a/frontend/src/app/data-management/alert-management/shared/components/filters/row-to-filter/row-to-filters.component.ts +++ b/frontend/src/app/data-management/alert-management/shared/components/filters/row-to-filter/row-to-filters.component.ts @@ -2,6 +2,7 @@ import {Component, EventEmitter, Input, OnInit, Output} from '@angular/core'; import {FormArray, FormBuilder, FormGroup, Validators} from '@angular/forms'; import {NgbActiveModal} from '@ng-bootstrap/ng-bootstrap'; import {ALERT_STATUS_FIELD, ALERT_TIMESTAMP_FIELD} from '../../../../../../shared/constants/alert/alert-field.constant'; +import {ElasticDataTypesEnum} from '../../../../../../shared/enums/elastic-data-types.enum'; import {ElasticOperatorsEnum} from '../../../../../../shared/enums/elastic-operators.enum'; import {ElasticFilterType} from '../../../../../../shared/types/filter/elastic-filter.type'; import {UtmFieldType} from '../../../../../../shared/types/table/utm-field.type'; @@ -41,11 +42,16 @@ export class RowToFiltersComponent implements OnInit { addValuesToFilter() { for (const field of this.fields) { if (field.visible && field.field !== ALERT_TIMESTAMP_FIELD) { + const value = extractValueFromObjectByPath(this.alert, field); + // Composite columns (type OBJECT) hold a nested object that cannot be + // expressed as a single filter value. They are offered UNCHECKED so + // the default "Apply filters" selection never includes them (#2968). + const isComposite = field.type === ElasticDataTypesEnum.OBJECT; this.filters.push(this.fb.group({ field: field.field, - value: extractValueFromObjectByPath(this.alert, field), + value, operator: ElasticOperatorsEnum.IS, - status: 'ACTIVE', + status: isComposite ? 'INACTIVE' : 'ACTIVE', label: field.label })); } @@ -72,16 +78,33 @@ export class RowToFiltersComponent implements OnInit { if (!(filter.field.includes(ALERT_STATUS_FIELD) || filter.field.includes(ALERT_TIMESTAMP_FIELD))) { filter.operator = filter.operator === ElasticOperatorsEnum.IS ? ElasticOperatorsEnum.IS_ONE_OF : ElasticOperatorsEnum.IS_NOT_ONE_OF; - if (typeof filter.value === 'string' && filter.value !== '-') { - filter.value = [filter.value]; - } - filter.value = filter.value === '-' ? null : filter.value; + filter.value = this.sanitizeFilterValue(filter.value); } } this.addRowToFilter.emit(filterEmit.filter(value => value.status === 'ACTIVE' && value.value !== null)); this.activeModal.close(); } + /** + * Normalise a row value into a shape the backend accepts for IS_ONE_OF / + * IS_NOT_ONE_OF (a list, never a bare object/null/undefined - see + * FilterType.validate). Returns null to drop filters that have no usable + * value: placeholder '-', booleans, composite (object) columns and empty + * collections. + */ + private sanitizeFilterValue(value: any): any { + if (value === null || value === undefined || value === '-') { + return null; + } + if (typeof value === 'object') { + return Array.isArray(value) && value.length > 0 ? value.map(v => String(v)) : null; + } + if (typeof value === 'boolean') { + return null; + } + return [String(value)]; + } + getFieldToRender(field: string) { return this.fields[this.fields.findIndex(value => value.field === field)]; }