From 76ba6724a4065d1014ae5058145638a8b7974782 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 17 Oct 2023 21:55:26 -0500 Subject: [PATCH] Validate EQL search strategy requests without `ignore` client option Previously (since https://github.com/elastic/kibana/pull/77493), we had been leveraging the following facts: * EQL search endpoint returns syntax errors as a 400 response * ES client can specify an `ignore` option to opt out of the "throw unsuccessful responses" behavior in order to perform EQL "validation" in a normal, non-exception control flow. This commit effectively replaces that with a try/catch, where we no longer pass (nor _can_ pass) `ignore` to the search strategy, and instead indicate we'd like the "validation" behavior via a new `validate` param. When set to true, we will capture the 400 response and, if valid, return it as before; in all other situations that syntax error will result in an error response from the search strategy. --- .../search/strategies/eql_search/types.ts | 13 ++--- .../eql_search/eql_search_strategy.ts | 47 ++++++++++++------- .../strategies/eql_search/response_utils.ts | 40 +++++++++++++++- .../public/common/hooks/eql/api.ts | 2 +- 4 files changed, 74 insertions(+), 28 deletions(-) diff --git a/src/plugins/data/common/search/strategies/eql_search/types.ts b/src/plugins/data/common/search/strategies/eql_search/types.ts index 985dbb409e20..0a8dc861a016 100644 --- a/src/plugins/data/common/search/strategies/eql_search/types.ts +++ b/src/plugins/data/common/search/strategies/eql_search/types.ts @@ -7,19 +7,16 @@ */ import type { EqlSearchRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; -import type { TransportResult, TransportRequestOptions } from '@elastic/elasticsearch'; +import type { TransportResult } from '@elastic/elasticsearch'; import { IKibanaSearchRequest, IKibanaSearchResponse } from '../../types'; export const EQL_SEARCH_STRATEGY = 'eql'; -export type EqlRequestParams = EqlSearchRequest; - -export interface EqlSearchStrategyRequest extends IKibanaSearchRequest { - /** - * @deprecated: use IAsyncSearchOptions.transport instead. - */ - options?: TransportRequestOptions; +export interface EqlRequestParams extends EqlSearchRequest { + validate?: boolean; } +export type EqlSearchStrategyRequest = IKibanaSearchRequest; + export type EqlSearchStrategyResponse = IKibanaSearchResponse>; diff --git a/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts b/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts index d6f5d948c784..e62281717477 100644 --- a/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts +++ b/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts @@ -16,7 +16,11 @@ import { IAsyncSearchOptions, pollSearch, } from '../../../../common'; -import { toEqlKibanaSearchResponse } from './response_utils'; +import { + errorToEqlKibanaSearchResponse, + isEqlValidationResponseError, + toEqlKibanaSearchResponse, +} from './response_utils'; import { EqlSearchResponse } from './types'; import { ISearchStrategy } from '../../types'; import { getDefaultSearchParams } from '../es_search'; @@ -47,6 +51,7 @@ export const eqlSearchStrategyProvider = ( const { track_total_hits: _, ...defaultParams } = await getDefaultSearchParams( uiSettingsClient ); + const { validate: isValidationRequest, ...eqlParams } = request.params ?? {}; const params = id ? getCommonDefaultAsyncGetParams(searchConfig, options, { /* disable until full eql support */ disableSearchSessions: true, @@ -57,27 +62,33 @@ export const eqlSearchStrategyProvider = ( ...getCommonDefaultAsyncGetParams(searchConfig, options, { /* disable until full eql support */ disableSearchSessions: true, }), - ...request.params, + ...eqlParams, }; - const response = id - ? await client.get( - { ...params, id }, - { - ...request.options, + + try { + const response = id + ? await client.get( + { ...params, id }, + { + ...options.transport, + signal: options.abortSignal, + meta: true, + } + ) + : // @ts-expect-error optional key cannot be used since search doesn't expect undefined + await client.search(params, { ...options.transport, signal: options.abortSignal, meta: true, - } - ) - : // @ts-expect-error optional key cannot be used since search doesn't expect undefined - await client.search(params as EqlSearchStrategyRequest['params'], { - ...request.options, - ...options.transport, - signal: options.abortSignal, - meta: true, - }); - - return toEqlKibanaSearchResponse(response as TransportResult); + }); + return toEqlKibanaSearchResponse(response as TransportResult); + } catch (e) { + if (isValidationRequest && isEqlValidationResponseError(e)) { + return errorToEqlKibanaSearchResponse(e); + } else { + throw e; + } + } }; const cancel = async () => { diff --git a/src/plugins/data/server/search/strategies/eql_search/response_utils.ts b/src/plugins/data/server/search/strategies/eql_search/response_utils.ts index f9bdf5bc7de3..ffe5c2f73d9d 100644 --- a/src/plugins/data/server/search/strategies/eql_search/response_utils.ts +++ b/src/plugins/data/server/search/strategies/eql_search/response_utils.ts @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import type { TransportResult } from '@elastic/elasticsearch'; +import { get } from 'lodash'; +import { errors, TransportResult } from '@elastic/elasticsearch'; import { EqlSearchResponse } from './types'; import { EqlSearchStrategyResponse } from '../../../../common'; @@ -24,3 +25,40 @@ export function toEqlKibanaSearchResponse( isRunning: response.body.is_running, }; } + +export function errorToEqlKibanaSearchResponse( + response: EqlResponseError +): EqlSearchStrategyResponse { + return { + id: undefined, + rawResponse: response.meta, + isPartial: false, + isRunning: false, + }; +} + +const PARSING_ERROR_TYPE = 'parsing_exception'; +const VERIFICATION_ERROR_TYPE = 'verification_exception'; +const MAPPING_ERROR_TYPE = 'mapping_exception'; + +interface ErrorCause { + type: string; + reason: string; +} + +interface EqlErrorBody { + error: ErrorCause & { root_cause: ErrorCause[] }; +} + +export interface EqlResponseError extends errors.ResponseError { + meta: TransportResult; +} + +const isValidationErrorType = (type: unknown): boolean => + type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE || type === MAPPING_ERROR_TYPE; + +export const isEqlResponseError = (response: unknown): response is EqlResponseError => + response instanceof errors.ResponseError; + +export const isEqlValidationResponseError = (response: unknown): response is EqlResponseError => + isEqlResponseError(response) && isValidationErrorType(get(response, 'meta.body.error.type')); diff --git a/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts b/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts index d12491699bf8..8147f242d79a 100644 --- a/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts +++ b/x-pack/plugins/security_solution/public/common/hooks/eql/api.ts @@ -38,8 +38,8 @@ export const validateEql = async ({ params: { index: dataViewTitle, body: { query, runtime_mappings: runtimeMappings, size: 0 }, + validate: true, }, - options: { ignore: [400] }, }, { strategy: EQL_SEARCH_STRATEGY,