Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Detections] EQL query validation: Separate syntax errors into a own error type (#10181) #190149

Merged
merged 11 commits into from
Aug 15, 2024
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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function is growing in complexity, it's probably time to add some unit tests around it (more for documentation of behavior than anything else); maybe extract a function that receives a response and test that? That would also allow us to document the shape of response(s) we handle.

type EqlValidationTransformer = (response: unknown) => {
  valid: boolean;
  error?: { code: EQL_ERROR_CODES; messages?: string[]; error?: Error };
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point! I will add tests for this functionality

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