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

[Detection Engine][Rules] - Adds custom highlighted fields option #163235

Merged
merged 42 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
eb33083
trying to recreate changes for new custom highlighted fields
yctercero Aug 3, 2023
0b98a19
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 3, 2023
110eb99
got most of the code ported over
yctercero Aug 6, 2023
5a249b9
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 6, 2023
ccf1c24
adding back in tests and cleanup
yctercero Aug 6, 2023
cbbbbf9
continued cleanup
yctercero Aug 7, 2023
6f5ac58
cleanup and making by default additive
yctercero Aug 9, 2023
f416282
cleaning up types
yctercero Aug 10, 2023
a42910e
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 10, 2023
888c042
updating so now everywhere pulls latest custom highlighted fields
yctercero Aug 11, 2023
b8c3923
dont need to map it in alert since always accessing latest
yctercero Aug 11, 2023
b272000
continued cleanup
yctercero Aug 11, 2023
7b97501
continued cleanup
yctercero Aug 12, 2023
eda33ef
continued cleanup
yctercero Aug 12, 2023
5920bba
trying to fix tests
yctercero Aug 12, 2023
317873d
trying to fix tests
yctercero Aug 13, 2023
0363fb8
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 13, 2023
e0b6a90
trying to fix tests
yctercero Aug 13, 2023
bfda622
trying to fix tests
yctercero Aug 14, 2023
44aa7a5
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 14, 2023
013c437
cleanup, cleanup, everybody cleanup
yctercero Aug 14, 2023
7376b1b
addressing PR feedback
yctercero Aug 14, 2023
07d0e41
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 14, 2023
2fd6f51
updated jest test and linting issue
yctercero Aug 14, 2023
a7379e5
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 14, 2023
df9d604
working on cypress tests and renaming field to investigation_fields
yctercero Aug 15, 2023
3f67bc5
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 15, 2023
ba11dba
Merge branch 'custom_highlighted_fields' of github.com:yctercero/kiba…
yctercero Aug 15, 2023
97d5adf
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 15, 2023
2188d0d
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Aug 15, 2023
e0f8fc9
adding to older event details flyout after refactor
yctercero Aug 15, 2023
a416d1a
Merge branch 'custom_highlighted_fields' of github.com:yctercero/kiba…
yctercero Aug 15, 2023
5a225bf
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 15, 2023
49bdfc0
skipping flakey tests, linked to issue as required follow up for release
yctercero Aug 15, 2023
2d01586
some cleanup
yctercero Aug 15, 2023
294387d
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 15, 2023
9d6c1e3
Merge branch 'main' into custom_highlighted_fields
kibanamachine Aug 15, 2023
f62deca
Merge branch 'main' of github.com:elastic/kibana into custom_highligh…
yctercero Aug 16, 2023
e3b8078
trying to get ci to pass
yctercero Aug 16, 2023
983053e
Merge branch 'custom_highlighted_fields' of github.com:yctercero/kiba…
yctercero Aug 16, 2023
9cc5c9f
Merge branch 'main' into custom_highlighted_fields
kibanamachine Aug 16, 2023
d547c5e
Merge branch 'main' into custom_highlighted_fields
MadameSheema Aug 16, 2023
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
4 changes: 4 additions & 0 deletions packages/kbn-rule-data-utils/src/technical_field_names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ const ALERT_EVALUATION_VALUES = `${ALERT_NAMESPACE}.evaluation.values` as const;
// Fields pertaining to the rule associated with the alert
const ALERT_RULE_EXCEPTIONS_LIST = `${ALERT_RULE_NAMESPACE}.exceptions_list` as const;
const ALERT_RULE_NAMESPACE_FIELD = `${ALERT_RULE_NAMESPACE}.namespace` as const;
const ALERT_RULE_CUSTOM_HIGHLIGHTED_FIELDS =
`${ALERT_RULE_NAMESPACE}.custom_highlighted_fields` as const;

// Fields pertaining to the threat tactic associated with the rule
const ALERT_THREAT_FRAMEWORK = `${ALERT_RULE_THREAT_NAMESPACE}.framework` as const;
Expand Down Expand Up @@ -161,6 +163,7 @@ const fields = {
ALERT_RULE_TYPE_ID,
ALERT_RULE_UPDATED_AT,
ALERT_RULE_UPDATED_BY,
ALERT_RULE_CUSTOM_HIGHLIGHTED_FIELDS,
ALERT_RULE_VERSION,
ALERT_START,
ALERT_TIME_RANGE,
Expand Down Expand Up @@ -198,6 +201,7 @@ export {
ALERT_BUILDING_BLOCK_TYPE,
ALERT_EVALUATION_THRESHOLD,
ALERT_EVALUATION_VALUE,
ALERT_RULE_CUSTOM_HIGHLIGHTED_FIELDS,
ALERT_CONTEXT,
ALERT_EVALUATION_VALUES,
ALERT_RULE_EXCEPTIONS_LIST,
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-securitysolution-ecs/src/rule/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface RuleEcs {
rule_id?: string[];
name?: string[];
false_positives?: string[];
custom_highlighted_fields?: string[];
saved_id?: string[];
timeline_id?: string[];
timeline_title?: string[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const getRulesSchemaMock = (anchorDate: string = ANCHOR_DATE) => ({
enabled: true,
false_positives: ['false positive 1', 'false positive 2'],
from: 'now-6m',
custom_highlighted_fields: ['custom.field1', 'custom.field2'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: not sure why we test the transformDataToNdjson function against a mock rule, considering the function is quite generic:

export const transformDataToNdjson = (data: unknown[]): string => {
  if (data.length !== 0) {
    const dataString = data.map((item) => JSON.stringify(item)).join('\n');
    return `${dataString}\n`;
  } else {
    return '';
  }
};

I think we should test it against generic objects 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Not going to touch in this PR though.

immutable: false,
name: 'Query with a rule id',
query: 'user.name: root or user.name: admin',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export const RuleAuthorArray = t.array(t.string); // should be non-empty strings
export type RuleFalsePositiveArray = t.TypeOf<typeof RuleFalsePositiveArray>;
export const RuleFalsePositiveArray = t.array(t.string); // should be non-empty strings?

export type RuleCustomHighlightedFieldArray = t.TypeOf<typeof RuleCustomHighlightedFieldArray>;
export const RuleCustomHighlightedFieldArray = t.array(t.string); // should be non-empty strings?
yctercero marked this conversation as resolved.
Show resolved Hide resolved

export type RuleReferenceArray = t.TypeOf<typeof RuleReferenceArray>;
export const RuleReferenceArray = t.array(t.string); // should be non-empty strings?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const getResponseBaseParams = (anchorDate: string = ANCHOR_DATE): SharedResponse
description: 'some description',
enabled: true,
false_positives: ['false positive 1', 'false positive 2'],
custom_highlighted_fields: ['fiel.foo', 'field.bar'],
from: 'now-6m',
immutable: false,
name: 'Query with a rule id',
Expand Down Expand Up @@ -77,6 +78,7 @@ export const getRulesSchemaMock = (anchorDate: string = ANCHOR_DATE): QueryRule
saved_id: undefined,
response_actions: undefined,
alert_suppression: undefined,
custom_highlighted_fields: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

});

export const getSavedQuerySchemaMock = (anchorDate: string = ANCHOR_DATE): SavedQueryRule => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,37 @@ describe('Rule response schema', () => {
expect(message.schema).toEqual({});
});
});

describe('custom_highlighted_fields', () => {
test('it should validate rule with "custom_highlighted_fields"', () => {
const payload = getRulesSchemaMock();
payload.custom_highlighted_fields = ['foo', 'bar'];

const decoded = RuleResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const expected = { ...getRulesSchemaMock(), custom_highlighted_fields: ['foo', 'bar'] };

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(expected);
});

test('it should NOT validate a string for "custom_highlighted_fields"', () => {
const payload: Omit<RuleResponse, 'custom_highlighted_fields'> & {
custom_highlighted_fields: string;
} = {
...getRulesSchemaMock(),
custom_highlighted_fields: 'foo',
};

const decoded = RuleResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([
'Invalid value "foo" supplied to "custom_highlighted_fields"',
]);
expect(message.schema).toEqual({});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
RelatedIntegrationArray,
RequiredFieldArray,
RuleAuthorArray,
RuleCustomHighlightedFieldArray,
RuleDescription,
RuleFalsePositiveArray,
RuleFilterArray,
Expand Down Expand Up @@ -116,6 +117,7 @@ export const baseSchema = buildRuleSchemas({
output_index: AlertsIndex,
namespace: AlertsIndexNamespace,
meta: RuleMetadata,
custom_highlighted_fields: RuleCustomHighlightedFieldArray,
Copy link
Contributor

Choose a reason for hiding this comment

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

Posting for visibility here. As discussed over zoom with @yctercero, I think this field should be:

  • Defaultable instead of optional, meaning it should always be defined in RuleResponse and other types of rules we return from the API (QueryRule, ThresholdRule, etc).
  • Wrapped in an object to make space for other parameters we might want to add for this feature in the future. This would also make it more compatible with the DiffableRule interface and the rule diff/upgrade algorithm.
  • Renamed to something more UI-neutral and generic, e.g. "investigation fields".

So in a rule, it would be something like rule.investigation_fields.field_names.

We should consider writing a migration to add a "null" investigation_fields object to existing rules. Pseudocode:

rule.investigation_fields = {
  field_names: [],
};

// Throttle
throttle: RuleActionThrottle,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export class EndpointRuleAlertGenerator extends BaseDataGenerator {
'kibana.alert.rule.consumer': 'siem',
'kibana.alert.rule.created_at': '2022-10-26T21:02:00.237Z',
'kibana.alert.rule.created_by': 'some_user',
'kibana.alert.rule.custom_highlighted_fields': [],
'kibana.alert.rule.description':
'Generates a detection alert each time an Elastic Endpoint Security alert is received. Enabling this rule allows you to immediately begin investigating your Endpoint alerts.',
'kibana.alert.rule.enabled': true,
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugins/security_solution/common/field_maps/8.10.0/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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 type { RulesFieldMap810 } from './rules';
import { rulesFieldMap810 } from './rules';
export type { RulesFieldMap810 };
export { rulesFieldMap810 };
19 changes: 19 additions & 0 deletions x-pack/plugins/security_solution/common/field_maps/8.10.0/rules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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 { rulesFieldMap } from '../8.0.0';

export const rulesFieldMap810 = {
...rulesFieldMap,
'kibana.alert.rule.custom_highlighted_fields': {
type: 'keyword',
array: true,
required: false,
},
} as const;

export type RulesFieldMap810 = typeof rulesFieldMap810;
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export const ALERT_ORIGINAL_EVENT_MODULE = `${ALERT_ORIGINAL_EVENT}.module` as c
export const ALERT_RULE_ACTIONS = `${ALERT_RULE_NAMESPACE}.actions` as const;
export const ALERT_RULE_EXCEPTIONS_LIST = `${ALERT_RULE_NAMESPACE}.exceptions_list` as const;
export const ALERT_RULE_FALSE_POSITIVES = `${ALERT_RULE_NAMESPACE}.false_positives` as const;
export const ALERT_RULE_CUSTOM_HIGHLIGHTED_FIELDS =
`${ALERT_RULE_NAMESPACE}.custom_highlighted_fields` as const;
export const ALERT_RULE_IMMUTABLE = `${ALERT_RULE_NAMESPACE}.immutable` as const;
export const ALERT_RULE_MAX_SIGNALS = `${ALERT_RULE_NAMESPACE}.max_signals` as const;
export const ALERT_RULE_META = `${ALERT_RULE_NAMESPACE}.meta` as const;
Expand Down
Loading