Skip to content

Commit

Permalink
[Detections] EQL query validation: Separate syntax errors into a own …
Browse files Browse the repository at this point in the history
…error type (#10181) (#190149)

## Summary

Addresses elastic/security-team#10181

This PR is a refactoring of EQL query validator:
* to separate different validation errors passed from ES. Before we
marked `parsing_exception`, `verification_exception` and
`mapping_exception` as the same error of type `ERR_INVALID_EQL`. After
these changes we will split these errors into separate ones: syntax
(`parsing_exception`), invalid EQL (`verification_exception` and
`mapping_exception`; can be split in future if needed)
* to handle missing data source as a new EQL error of type
`MISSING_DATA_SOURCE`. Before
`data.search.search<EqlSearchStrategyRequest,
EqlSearchStrategyResponse>()` call would throw an exception in case data
source does not exist and we would handle it as a failed request and
show an error toast (see relevant ticket
#178611). After these changes we
would not show a toast and handle missing data source error as other EQL
validation errors - showing an error message in the EQL query bar.

This will allow us to distinguish between different types of EQL
validation errors and will help to decide on whether certain errors are
blocking during the rule creation/editing flow
(#180407).

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [Integration: Rule
execution](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6754)
(100 ESS & 100 Serverless)
- [Cypress: Detection
Engine](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6766)
(100 ESS & 100 Serverless)

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
e40pud and elasticmachine authored Aug 15, 2024
1 parent 74c9570 commit 2c5ae36
Show file tree
Hide file tree
Showing 8 changed files with 342 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,32 @@ export const getEqlResponseWithValidationErrors = (): ErrorResponse => ({
},
});

export const getEqlResponseWithMappingError = (): ErrorResponse => ({
error: {
root_cause: [
{
type: 'mapping_exception',
reason: "Inaccessible index 'restricted-*'",
},
],
type: 'mapping_exception',
reason: "Inaccessible index 'restricted-*'",
},
});

export const getEqlResponseWithParsingError = (): ErrorResponse => ({
error: {
root_cause: [
{
type: 'parsing_exception',
reason: "line 1:5: missing 'where' at 'demo'",
},
],
type: 'parsing_exception',
reason: "line 1:5: missing 'where' at 'demo'",
},
});

export const getEqlResponseWithNonValidationError = (): TransportResult['body'] => ({
error: {
root_cause: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,27 @@ export interface ErrorResponse {
const isValidationErrorType = (type: unknown): boolean =>
type === PARSING_ERROR_TYPE || type === VERIFICATION_ERROR_TYPE || type === MAPPING_ERROR_TYPE;

const isParsingErrorType = (type: unknown): boolean => type === PARSING_ERROR_TYPE;

const isVerificationErrorType = (type: unknown): boolean => type === VERIFICATION_ERROR_TYPE;

const isMappingErrorType = (type: unknown): boolean => type === MAPPING_ERROR_TYPE;

export const isErrorResponse = (response: unknown): response is ErrorResponse =>
has(response, 'error.type');

export const isValidationErrorResponse = (response: unknown): response is ErrorResponse =>
isErrorResponse(response) && isValidationErrorType(get(response, 'error.type'));

export const isParsingErrorResponse = (response: unknown): response is ErrorResponse =>
isErrorResponse(response) && isParsingErrorType(get(response, 'error.type'));

export const isVerificationErrorResponse = (response: unknown): response is ErrorResponse =>
isErrorResponse(response) && isVerificationErrorType(get(response, 'error.type'));

export const isMappingErrorResponse = (response: unknown): response is ErrorResponse =>
isErrorResponse(response) && isMappingErrorType(get(response, 'error.type'));

export const getValidationErrors = (response: ErrorResponse): string[] =>
response.error.root_cause
.filter((cause) => isValidationErrorType(cause.type))
Expand Down
162 changes: 162 additions & 0 deletions x-pack/plugins/security_solution/public/common/hooks/eql/api.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { of } from 'rxjs';

import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { searchServiceMock } from '@kbn/data-plugin/public/search/mocks';

import { validateEql } from './api';
import {
getEqlResponseWithMappingError,
getEqlResponseWithNonValidationError,
getEqlResponseWithParsingError,
getEqlResponseWithValidationError,
getEqlResponseWithValidationErrors,
} from '../../../../common/search_strategy/eql/validation/helpers.mock';

const searchMock = searchServiceMock.createStartContract();

const mockDataService = {
...dataPluginMock.createStartContract(),
search: searchMock,
};

const triggerValidateEql = () => {
const signal = new AbortController().signal;
return validateEql({
data: mockDataService,
dataViewTitle: 'logs-*',
query: 'any where true',
signal,
runtimeMappings: undefined,
options: undefined,
});
};

describe('validateEql', () => {
beforeEach(() => {
jest.clearAllMocks();
});

describe('handle EqlSearchStrategyResponse', () => {
it('should return valid set to true if validation response is not an error', async () => {
searchMock.search.mockImplementation(() => of({ rawResponse: 'Successful validation!' }));
const response = await triggerValidateEql();

expect(response).toEqual({ valid: true });
});

it('should return EQL_ERR_INVALID_EQL error in case of `verification_exception` error', async () => {
searchMock.search.mockImplementation(() =>
of({ rawResponse: getEqlResponseWithValidationError() })
);
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: {
code: 'EQL_ERR_INVALID_EQL',
messages: [
'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]',
],
},
});
});

it('should return EQL_ERR_INVALID_EQL error in case of multiple `verification_exception` errors', async () => {
searchMock.search.mockImplementation(() =>
of({ rawResponse: getEqlResponseWithValidationErrors() })
);
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: {
code: 'EQL_ERR_INVALID_EQL',
messages: [
'Found 2 problems\nline 1:1: Unknown column [event.category]\nline 1:13: Unknown column [event.name]',
"line 1:4: mismatched input '<EOF>' expecting 'where'",
],
},
});
});

it('should return EQL_ERR_INVALID_EQL error in case of `mapping_exception` error', async () => {
searchMock.search.mockImplementation(() =>
of({ rawResponse: getEqlResponseWithMappingError() })
);
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: { code: 'EQL_ERR_INVALID_EQL', messages: ["Inaccessible index 'restricted-*'"] },
});
});

it('should return EQL_ERR_INVALID_SYNTAX error in case of `parsing_exception` error', async () => {
searchMock.search.mockImplementation(() =>
of({ rawResponse: getEqlResponseWithParsingError() })
);
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: {
code: 'EQL_ERR_INVALID_SYNTAX',
messages: ["line 1:5: missing 'where' at 'demo'"],
},
});
});

it('should return EQL_ERR_FAILED_REQUEST error in case of non-validation error', async () => {
searchMock.search.mockImplementation(() =>
of({ rawResponse: getEqlResponseWithNonValidationError() })
);
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: {
code: 'EQL_ERR_FAILED_REQUEST',
error: expect.objectContaining(
new Error(JSON.stringify(getEqlResponseWithNonValidationError()))
),
},
});
});

it('should return EQL_ERR_MISSING_DATA_SOURCE error in case `data.search` throws an error which starts with `index_not_found_exception`', async () => {
const message = 'index_not_found_exception Found 1 problem line -1:-1: Unknown index [*,-*]';
searchMock.search.mockImplementation(() => {
throw new Error(message);
});
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: { code: 'EQL_ERR_MISSING_DATA_SOURCE', messages: [message] },
});
});

it('should return EQL_ERR_FAILED_REQUEST error in case `data.search` throws an error', async () => {
const message = 'Internal exception';
searchMock.search.mockImplementation(() => {
throw new Error(message);
});
const response = await triggerValidateEql();

expect(response).toEqual({
valid: false,
error: {
code: 'EQL_ERR_FAILED_REQUEST',
error: expect.objectContaining(new Error(message)),
},
});
});
});
});
88 changes: 63 additions & 25 deletions x-pack/plugins/security_solution/public/common/hooks/eql/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,18 @@ import type { EqlOptionsSelected } from '../../../../common/search_strategy';
import {
getValidationErrors,
isErrorResponse,
isValidationErrorResponse,
isMappingErrorResponse,
isParsingErrorResponse,
isVerificationErrorResponse,
} from '../../../../common/search_strategy/eql';

export enum EQL_ERROR_CODES {
FAILED_REQUEST = 'EQL_ERR_FAILED_REQUEST',
INVALID_EQL = 'EQL_ERR_INVALID_EQL',
INVALID_SYNTAX = 'EQL_ERR_INVALID_SYNTAX',
MISSING_DATA_SOURCE = 'EQL_ERR_MISSING_DATA_SOURCE',
}

interface Params {
dataViewTitle: string;
query: string;
Expand All @@ -34,31 +43,60 @@ export const validateEql = async ({
signal,
runtimeMappings,
options,
}: Params): Promise<{ valid: boolean; errors: string[] }> => {
const { rawResponse: response } = await firstValueFrom(
data.search.search<EqlSearchStrategyRequest, EqlSearchStrategyResponse>(
{
params: {
index: dataViewTitle,
body: { query, runtime_mappings: runtimeMappings, size: 0 },
timestamp_field: options?.timestampField,
tiebreaker_field: options?.tiebreakerField || undefined,
event_category_field: options?.eventCategoryField,
}: Params): Promise<{
valid: boolean;
error?: { code: EQL_ERROR_CODES; messages?: string[]; error?: Error };
}> => {
try {
const { rawResponse: response } = await firstValueFrom(
data.search.search<EqlSearchStrategyRequest, EqlSearchStrategyResponse>(
{
params: {
index: dataViewTitle,
body: { query, runtime_mappings: runtimeMappings, size: 0 },
timestamp_field: options?.timestampField,
tiebreaker_field: options?.tiebreakerField || undefined,
event_category_field: options?.eventCategoryField,
},
options: { ignore: [400] },
},
options: { ignore: [400] },
{
strategy: EQL_SEARCH_STRATEGY,
abortSignal: signal,
}
)
);
if (isParsingErrorResponse(response)) {
return {
valid: false,
error: { code: EQL_ERROR_CODES.INVALID_SYNTAX, messages: getValidationErrors(response) },
};
} else if (isVerificationErrorResponse(response) || isMappingErrorResponse(response)) {
return {
valid: false,
error: { code: EQL_ERROR_CODES.INVALID_EQL, messages: getValidationErrors(response) },
};
} else if (isErrorResponse(response)) {
return {
valid: false,
error: { code: EQL_ERROR_CODES.FAILED_REQUEST, error: new Error(JSON.stringify(response)) },
};
} else {
return { valid: true };
}
} catch (error) {
if (error instanceof Error && error.message.startsWith('index_not_found_exception')) {
return {
valid: false,
error: { code: EQL_ERROR_CODES.MISSING_DATA_SOURCE, messages: [error.message] },
};
}
return {
valid: false,
error: {
code: EQL_ERROR_CODES.FAILED_REQUEST,
error,
},
{
strategy: EQL_SEARCH_STRATEGY,
abortSignal: signal,
}
)
);

if (isValidationErrorResponse(response)) {
return { valid: false, errors: getValidationErrors(response) };
} else if (isErrorResponse(response)) {
throw new Error(JSON.stringify(response));
} else {
return { valid: true, errors: [] };
};
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
* 2.0.
*/

import { EQL_ERROR_CODES } from '../../../../common/hooks/eql/api';
import type { ValidationError } from '../../../../shared_imports';
import { ERROR_CODES } from './validators';

export const getEqlValidationError = (): ValidationError => ({
code: ERROR_CODES.INVALID_EQL,
code: EQL_ERROR_CODES.INVALID_EQL,
messages: ['line 1: WRONG\nline 2: ALSO WRONG'],
message: '',
});
Loading

0 comments on commit 2c5ae36

Please sign in to comment.