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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b11642e
[Detections] EQL query validation: Separate syntax errors into a own …
e40pud Aug 8, 2024
f106eca
EQL error code namings
e40pud Aug 8, 2024
7d75286
Add `MISSING_DATA_SOURCE` validation error
e40pud Aug 8, 2024
d2eb8da
Fix types
e40pud Aug 8, 2024
5ca831b
Fix tests
e40pud Aug 9, 2024
7b11a6e
Merge branch 'main' into security/feature/EQL-query-validation
e40pud Aug 9, 2024
7c31ac6
Merge branch 'main' into security/feature/EQL-query-validation
e40pud Aug 9, 2024
6992dba
Merge branch 'main' into security/feature/EQL-query-validation
elasticmachine Aug 11, 2024
c178c76
Merge branch 'main' into security/feature/EQL-query-validation
e40pud Aug 13, 2024
da821b0
Review feedback: add jest tests for EQL validation response handler
e40pud Aug 14, 2024
48742a1
Merge branch 'main' into security/feature/EQL-query-validation
e40pud Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions x-pack/plugins/security_solution/common/search_strategy/eql/validation/helpers.ts
View file
Open in desktop
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
88 changes: 63 additions & 25 deletions x-pack/plugins/security_solution/public/common/hooks/eql/api.ts
View file
Open in desktop
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: [] };
};
}
};
4 changes: 2 additions & 2 deletions ...tion/public/detection_engine/rule_creation_ui/components/eql_query_bar/validators.mock.ts
View file
Open in desktop
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: '',
});
22 changes: 9 additions & 13 deletions ..._solution/public/detection_engine/rule_creation_ui/components/eql_query_bar/validators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,10 @@ import { isEqlRule } from '../../../../../common/detection_engine/utils';
import { KibanaServices } from '../../../../common/lib/kibana';
import type { DefineStepRule } from '../../../../detections/pages/detection_engine/rules/types';
import { DataSourceType } from '../../../../detections/pages/detection_engine/rules/types';
import { validateEql } from '../../../../common/hooks/eql/api';
import { validateEql, EQL_ERROR_CODES } from '../../../../common/hooks/eql/api';
import type { FieldValueQueryBar } from '../query_bar';
import * as i18n from './translations';

export enum ERROR_CODES {
FAILED_REQUEST = 'ERR_FAILED_REQUEST',
INVALID_EQL = 'ERR_INVALID_EQL',
}

/**
* Unlike lodash's debounce, which resolves intermediate calls with the most
* recent value, this implementation waits to resolve intermediate calls until
Expand Down Expand Up @@ -54,7 +49,7 @@ export const debounceAsync = <Args extends unknown[], Result extends Promise<unk

export const eqlValidator = async (
...args: Parameters<ValidationFunc>
): Promise<ValidationError<ERROR_CODES> | void | undefined> => {
): Promise<ValidationError<EQL_ERROR_CODES> | void | undefined> => {
const [{ value, formData }] = args;
const { query: queryValue } = value as FieldValueQueryBar;
const query = queryValue.query as string;
Expand Down Expand Up @@ -93,14 +88,15 @@ export const eqlValidator = async (

if (response?.valid === false) {
return {
code: ERROR_CODES.INVALID_EQL,
code: response.error?.code,
message: '',
messages: response.errors,
messages: response.error?.messages,
error: response.error?.error,
};
}
} catch (error) {
return {
code: ERROR_CODES.FAILED_REQUEST,
code: EQL_ERROR_CODES.FAILED_REQUEST,
message: i18n.EQL_VALIDATION_REQUEST_ERROR,
error,
};
Expand All @@ -117,10 +113,10 @@ export const getValidationResults = <T = unknown>(
const [error] = field.errors;
const message = error.message;

if (error.code === ERROR_CODES.INVALID_EQL) {
return { isValid, message, messages: error.messages };
} else {
if (error.code === EQL_ERROR_CODES.FAILED_REQUEST) {
return { isValid, message, error: error.error };
} else {
return { isValid, message, messages: error.messages };
}
} else {
return { isValid, message: '' };
Expand Down
62 changes: 62 additions & 0 deletions ...tion_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
EQL_OPTIONS_TIMESTAMP_INPUT,
EQL_QUERY_INPUT,
EQL_QUERY_VALIDATION_ERROR,
EQL_QUERY_VALIDATION_ERROR_CONTENT,
RULES_CREATION_FORM,
} from '../../../../screens/create_new_rule';

Expand Down Expand Up @@ -222,4 +223,65 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => {
cy.get(EQL_QUERY_VALIDATION_ERROR).should('not.exist');
});
});

describe('EQL query validation', () => {
it('validates missing data source', () => {
login();
visit(CREATE_RULE_URL);
selectEqlRuleType();
getIndexPatternClearButton().click();
getRuleIndexInput().type('endgame-*{enter}');

cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('exist');
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('be.visible');
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type('any where true');

cy.get(EQL_QUERY_VALIDATION_ERROR).should('be.visible');
cy.get(EQL_QUERY_VALIDATION_ERROR).should('have.text', '1');
cy.get(EQL_QUERY_VALIDATION_ERROR).click();
cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should('be.visible');
cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should(
'have.text',
`EQL Validation Errorsindex_not_found_exception\n\tCaused by:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]\n\tRoot causes:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]`
);
});

it('validates missing data fields', () => {
login();
visit(CREATE_RULE_URL);
selectEqlRuleType();

cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('exist');
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('be.visible');
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type('any where field1');

cy.get(EQL_QUERY_VALIDATION_ERROR).should('be.visible');
cy.get(EQL_QUERY_VALIDATION_ERROR).should('have.text', '1');
cy.get(EQL_QUERY_VALIDATION_ERROR).click();
cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should('be.visible');
cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should(
'have.text',
'EQL Validation ErrorsFound 1 problem\nline 1:11: Unknown column [field1]'
);
});

it('validates syntax errors', () => {
login();
visit(CREATE_RULE_URL);
selectEqlRuleType();

cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('exist');
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('be.visible');
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type('test any where true');

cy.get(EQL_QUERY_VALIDATION_ERROR).should('be.visible');
cy.get(EQL_QUERY_VALIDATION_ERROR).should('have.text', '1');
cy.get(EQL_QUERY_VALIDATION_ERROR).click();
cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should('be.visible');
cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should(
'have.text',
`EQL Validation Errorsline 1:6: extraneous input 'any' expecting 'where'`
);
});
});
});
3 changes: 3 additions & 0 deletions x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ export const EQL_QUERY_VALIDATION_LABEL = '.euiFormLabel-isInvalid';

export const EQL_QUERY_VALIDATION_ERROR = '[data-test-subj="eql-validation-errors-popover-button"]';

export const EQL_QUERY_VALIDATION_ERROR_CONTENT =
'[data-test-subj="eql-validation-errors-popover-content"]';

export const EQL_OPTIONS_POPOVER_TRIGGER = '[data-test-subj="eql-settings-trigger"]';

export const EQL_OPTIONS_TIMESTAMP_INPUT =
Expand Down