Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d8a2be1
update event filters to include more match options
parkiino Nov 3, 2023
ca3dea7
make file path warning specific to file path
parkiino Nov 3, 2023
cdf7dae
fix test
parkiino Nov 6, 2023
0fc5496
remove console.log
parkiino Nov 6, 2023
667e55e
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 7, 2023
ed5ab97
fix test
parkiino Nov 8, 2023
02825c0
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 8, 2023
f0000a3
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 8, 2023
e1a5144
generalize wildcard warning
parkiino Nov 9, 2023
dea8dc1
adjust text, add simple unit tests
parkiino Nov 9, 2023
b441f5c
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 9, 2023
18d3e33
fix improt
parkiino Nov 10, 2023
7cece88
remove unnecessary tests and translations
parkiino Nov 10, 2023
65c75fb
add ashs suggestion
parkiino Nov 14, 2023
787c441
rename functions, add more unit tests'
parkiino Nov 14, 2023
99d6a87
update regex
parkiino Nov 14, 2023
0a6f34d
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 15, 2023
0d642c0
add does not match wildcard
parkiino Nov 16, 2023
173de02
fix function import
parkiino Nov 16, 2023
c5a1ced
fix broken tests
parkiino Nov 16, 2023
75f1a7d
remove does not match for now
parkiino Nov 16, 2023
0461dae
Revert "remove does not match for now"
parkiino Nov 20, 2023
cb569a7
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 21, 2023
fa71a65
Merge remote-tracking branch 'upstream/main' into task/event-filters-…
parkiino Nov 22, 2023
bd4c109
update warning messaging
parkiino Nov 22, 2023
c6c27e5
Merge branch 'main' into task/event-filters-matches
parkiino Nov 27, 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
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { AutocompleteFieldWildcardComponent } from '.';
import { useFieldValueAutocomplete } from '../hooks/use_field_value_autocomplete';
import { fields, getField } from '../fields/index.mock';
import { autocompleteStartMock } from '../autocomplete/index.mock';
import { FILENAME_WILDCARD_WARNING, FILEPATH_WARNING } from '@kbn/securitysolution-utils';
import { WILDCARD_WARNING, FILEPATH_WARNING } from '@kbn/securitysolution-utils';

jest.mock('../hooks/use_field_value_autocomplete');
jest.mock('../translations', () => ({
Expand Down Expand Up @@ -368,7 +368,7 @@ describe('AutocompleteFieldWildcardComponent', () => {
placeholder="Placeholder text"
selectedField={getField('file.path.text')}
selectedValue="invalid path"
warning={FILENAME_WILDCARD_WARNING}
warning={WILDCARD_WARNING}
/>
);

Expand All @@ -384,7 +384,7 @@ describe('AutocompleteFieldWildcardComponent', () => {
const helpText = wrapper
.find('[data-test-subj="valuesAutocompleteWildcardLabel"] div.euiFormHelpText')
.at(0);
expect(helpText.text()).toEqual(FILENAME_WILDCARD_WARNING);
expect(helpText.text()).toEqual(WILDCARD_WARNING);
expect(helpText.find('.euiToolTipAnchor')).toBeTruthy();
});
test('should show the warning helper text if the new value contains spaces when change', async () => {
Expand Down Expand Up @@ -412,7 +412,7 @@ describe('AutocompleteFieldWildcardComponent', () => {
placeholder="Placeholder text"
selectedField={getField('file.path.text')}
selectedValue="invalid path"
warning={FILENAME_WILDCARD_WARNING}
warning={WILDCARD_WARNING}
/>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ describe('operator', () => {
{ label: 'is one of' },
{ label: 'is not one of' },
{ label: 'matches' },
{ label: 'does not match' },
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export const EVENT_FILTERS_OPERATORS: OperatorOption[] = [
isOneOfOperator,
isNotOneOfOperator,
matchesOperator,
doesNotMatchOperator,
];

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,43 @@ import {
hasSimpleExecutableName,
OperatingSystem,
ConditionEntryField,
validatePotentialWildcardInput,
validateFilePathInput,
FILENAME_WILDCARD_WARNING,
validateWildcardInput,
WILDCARD_WARNING,
FILEPATH_WARNING,
} from '.';

describe('validatePotentialWildcardInput', () => {
it('warns on wildcard when field is file.path.text', () => {
expect(
validatePotentialWildcardInput({
field: 'file.path.text',
os: OperatingSystem.WINDOWS,
value: 'c:\\path*.exe',
})
).toEqual(WILDCARD_WARNING);
});
it('warns on wildcard when field is not file.path.text', () => {
expect(
validatePotentialWildcardInput({
field: 'event.category',
os: OperatingSystem.WINDOWS,
value: 'some*value',
})
).toEqual(WILDCARD_WARNING);
});
});

describe('validateWildcardInput', () => {
it('warns on wildcard for fields that are not file paths', () => {
expect(validateWildcardInput('*')).toEqual(WILDCARD_WARNING);
});
it('does not warn if no wildcard', () => {
expect(validateWildcardInput('non-wildcard')).toEqual(undefined);
});
});

describe('validateFilePathInput', () => {
describe('windows', () => {
const os = OperatingSystem.WINDOWS;
Expand All @@ -36,15 +68,13 @@ describe('validateFilePathInput', () => {
});

it('warns on wildcard in file name at the end of the path', () => {
expect(validateFilePathInput({ os, value: 'c:\\path*.exe' })).toEqual(
FILENAME_WILDCARD_WARNING
);
expect(validateFilePathInput({ os, value: 'c:\\path*.exe' })).toEqual(WILDCARD_WARNING);
expect(
validateFilePathInput({
os,
value: 'C:\\Windows\\*\\FILENAME.EXE-*.gz',
})
).toEqual(FILENAME_WILDCARD_WARNING);
).toEqual(WILDCARD_WARNING);
});

it('warns on unix paths or non-windows paths', () => {
Expand All @@ -65,20 +95,23 @@ describe('validateFilePathInput', () => {
: OperatingSystem.LINUX;

it('does not warn on valid filenames', () => {
expect(validateFilePathInput({ os, value: '/opt/*/FILENAME.EXE-1231205124.gz' })).not.toEqual(
FILENAME_WILDCARD_WARNING
);
expect(
validateFilePathInput({
os,
value: '/opt/*/FILENAME.EXE-1231205124.gz',
})
).not.toEqual(WILDCARD_WARNING);
expect(
validateFilePathInput({
os,
value: "/opt/*/test$ as2@13---12!@#A,DS.#$^&$!#~ 'as'd.华语.txt",
})
).not.toEqual(FILENAME_WILDCARD_WARNING);
).not.toEqual(WILDCARD_WARNING);
});
it('warns on wildcard in file name at the end of the path', () => {
expect(validateFilePathInput({ os, value: '/opt/bin*' })).toEqual(FILENAME_WILDCARD_WARNING);
expect(validateFilePathInput({ os, value: '/opt/bin*' })).toEqual(WILDCARD_WARNING);
expect(validateFilePathInput({ os, value: '/opt/FILENAME.EXE-*.gz' })).toEqual(
FILENAME_WILDCARD_WARNING
WILDCARD_WARNING
);
});

Expand Down
35 changes: 28 additions & 7 deletions packages/kbn-securitysolution-utils/src/path_validations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

import { i18n } from '@kbn/i18n';

export const FILENAME_WILDCARD_WARNING = i18n.translate('utils.filename.wildcardWarning', {
defaultMessage: `Using wildcards in file paths can impact Endpoint performance`,
export const WILDCARD_WARNING = i18n.translate('utils.wildcardWarning', {
defaultMessage: `Using wildcards can impact Endpoint performance`,
});

export const FILEPATH_WARNING = i18n.translate('utils.filename.pathWarning', {
Expand Down Expand Up @@ -52,39 +52,60 @@ export enum OperatingSystem {
export type EntryTypes = 'match' | 'wildcard' | 'match_any';
export type TrustedAppEntryTypes = Extract<EntryTypes, 'match' | 'wildcard'>;

export const validateFilePathInput = ({
export const validatePotentialWildcardInput = ({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We now see the FILEPATH_WARNING for any field when selecting for matches operator.

Screenshot 2023-11-13 at 9 04 00 AM

field = '',
os,
value = '',
}: {
field?: string;
os: OperatingSystem;
value?: string;
}): string | undefined => {
const textInput = value.trim();
if (field === 'file.path.text') {
return validateFilePathInput({ os, value: textInput });
}
return validateWildcardInput(textInput);
};

export const validateFilePathInput = ({
os,
value,
}: {
os: OperatingSystem;
value: string;
}): string | undefined => {
const isValidFilePath = isPathValid({
os,
field: 'file.path.text',
type: 'wildcard',
value: textInput,
value,
});
const hasSimpleFileName = hasSimpleExecutableName({
os,
type: 'wildcard',
value: textInput,
value,
});

if (!textInput.length) {
if (!value.length) {
return FILEPATH_WARNING;
}

if (isValidFilePath) {
if (hasSimpleFileName !== undefined && !hasSimpleFileName) {
return FILENAME_WILDCARD_WARNING;
return WILDCARD_WARNING;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I would expect to see only FILEPATH_WARNING in this function as it only deals with file.path.text field. What would make this simpler is if we combine the two if blocks in this function as

if (
    !isValidFilePath ||
    !value.length ||
    (hasSimpleFileName !== undefined && !hasSimpleFileName)
  ) {
    return FILEPATH_WARNING;
  }

and that's all the function should return.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The FILEPATH_WARNING warns about a malformed path whereas the WILDCARD_WARNING warns the user that a wildcard will affect performance. We actually still want the WILDCARD_WARNING in this case, given that the path is valid.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh yeah you're absolutely right! Perhaps the following may be simpler to reason with:

 if (!isValidFilePath || !value.length) {
   return FILEPATH_WARNING;
 }

 if (hasSimpleFileName !== undefined && !hasSimpleFileName) {
   return WILDCARD_WARNING;
 }

}
} else {
return FILEPATH_WARNING;
}
};

export const validateWildcardInput = (value?: string): string | undefined => {
if (/[*?]/.test(value ?? '')) {
return WILDCARD_WARNING;
}
};

export const hasSimpleExecutableName = ({
os,
type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
isOperator,
matchesOperator,
} from '@kbn/securitysolution-list-utils';
import { validateFilePathInput } from '@kbn/securitysolution-utils';
import { validatePotentialWildcardInput } from '@kbn/securitysolution-utils';
import { useFindListsBySize } from '@kbn/securitysolution-list-hooks';
import type { FieldSpec } from '@kbn/data-plugin/common';
import { fields, getField } from '@kbn/data-plugin/common/mocks';
Expand Down Expand Up @@ -1050,7 +1050,7 @@ describe('BuilderEntryItem', () => {
test('it invokes "setWarningsExist" when invalid value in field value input', async () => {
const mockSetWarningsExists = jest.fn();

(validateFilePathInput as jest.Mock).mockReturnValue('some warning message');
(validatePotentialWildcardInput as jest.Mock).mockReturnValue('some warning message');
wrapper = mount(
<BuilderEntryItem
autocompleteService={autocompleteStartMock}
Expand Down Expand Up @@ -1099,7 +1099,7 @@ describe('BuilderEntryItem', () => {
test('it does not invoke "setWarningsExist" when valid value in field value input', async () => {
const mockSetWarningsExists = jest.fn();

(validateFilePathInput as jest.Mock).mockReturnValue(undefined);
(validatePotentialWildcardInput as jest.Mock).mockReturnValue(undefined);
wrapper = mount(
<BuilderEntryItem
autocompleteService={autocompleteStartMock}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
EXCEPTION_OPERATORS_ONLY_LISTS,
FormattedBuilderEntry,
OperatorOption,
fieldSupportsMatches,
getEntryOnFieldChange,
getEntryOnListChange,
getEntryOnMatchAnyChange,
Expand All @@ -50,9 +51,9 @@ import {
OperatorComponent,
} from '@kbn/securitysolution-autocomplete';
import {
FILENAME_WILDCARD_WARNING,
OperatingSystem,
validateFilePathInput,
WILDCARD_WARNING,
validatePotentialWildcardInput,
} from '@kbn/securitysolution-utils';
import { DataViewBase, DataViewFieldBase } from '@kbn/es-query';
import type { AutocompleteStart } from '@kbn/unified-search-plugin/public';
Expand Down Expand Up @@ -310,11 +311,11 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({

const renderOperatorInput = (isFirst: boolean): JSX.Element => {
// for event filters forms
// show extra operators for wildcards when field is `file.path.text`
const isFilePathTextField = entry.field !== undefined && entry.field.name === 'file.path.text';
// show extra operators for wildcards when field supports matches
const doesFieldSupportMatches = entry.field !== undefined && fieldSupportsMatches(entry.field);
const isEventFilterList = listType === 'endpoint_events';
const augmentedOperatorsList =
operatorsList && isFilePathTextField && isEventFilterList
operatorsList && doesFieldSupportMatches && isEventFilterList
? operatorsList
: operatorsList?.filter((operator) => operator.type !== OperatorTypeEnum.WILDCARD);

Expand Down Expand Up @@ -358,8 +359,8 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
}
};

// show this when wildcard filename with matches operator
const getWildcardWarning = (precedingWarning: string): React.ReactNode => {
// show this when wildcard with matches operator
const getWildcardWarningInfo = (precedingWarning: string): React.ReactNode => {
return (
<p>
{precedingWarning}{' '}
Expand All @@ -368,7 +369,7 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
content={
<FormattedMessage
id="xpack.lists.exceptions.builder.exceptionMatchesOperator.warningMessage.wildcardInFilepath"
defaultMessage="To make a more efficient event filter, use multiple conditions and make them as specific as possible when using wildcards in the path values. For instance, adding a process.name or file.name field."
defaultMessage="To make a more efficient event filter, use multiple conditions and make them as specific as possible when using wildcards in the values. For instance, adding a process.name or file.name field. Creating event filters with both `matches` and `does not match` operators may significantly decrease performance."
/>
}
size="m"
Expand Down Expand Up @@ -430,11 +431,13 @@ export const BuilderEntryItem: React.FC<EntryItemProps> = ({
if (osTypes) {
[os] = osTypes as OperatingSystem[];
}
const warning = validateFilePathInput({ os, value: wildcardValue });
const warning = validatePotentialWildcardInput({
field: entry.field?.name,
os,
value: wildcardValue,
});
actualWarning =
warning === FILENAME_WILDCARD_WARNING
? warning && getWildcardWarning(warning)
: warning;
warning === WILDCARD_WARNING ? warning && getWildcardWarningInfo(warning) : warning;
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ describe('Exception builder helpers', () => {
expect(output).toEqual(expected);
});

test('it returns all fields unfiletered if "item.nested" is not "child" or "parent"', () => {
test('it returns all fields unfiltered if "item.nested" is not "child" or "parent"', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔥 thanks for fixing type

const payloadIndexPattern = getMockIndexPattern();
const payloadItem: FormattedBuilderEntry = getMockBuilderEntry();
const output = getFilteredIndexPatterns(payloadIndexPattern, payloadItem);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
OS_WINDOWS,
CONDITION_AND,
CONDITION_OPERATOR_TYPE_WILDCARD_MATCHES,
CONDITION_OPERATOR_TYPE_DOES_NOT_MATCH,
CONDITION_OPERATOR_TYPE_NESTED,
CONDITION_OPERATOR_TYPE_MATCH,
CONDITION_OPERATOR_TYPE_MATCH_ANY,
Expand Down Expand Up @@ -47,6 +48,7 @@ const OPERATOR_TYPE_LABELS_INCLUDED = Object.freeze({
const OPERATOR_TYPE_LABELS_EXCLUDED = Object.freeze({
[ListOperatorTypeEnum.MATCH_ANY]: CONDITION_OPERATOR_TYPE_NOT_MATCH_ANY,
[ListOperatorTypeEnum.MATCH]: CONDITION_OPERATOR_TYPE_NOT_MATCH,
[ListOperatorTypeEnum.WILDCARD]: CONDITION_OPERATOR_TYPE_DOES_NOT_MATCH,
});

const EuiFlexGroupNested = styled(EuiFlexGroup)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ export const CONDITION_OPERATOR_TYPE_WILDCARD_MATCHES = i18n.translate(
}
);

export const CONDITION_OPERATOR_TYPE_DOES_NOT_MATCH = i18n.translate(
'xpack.securitySolution.artifactCard.conditions.wildcardDoesNotMatchOperator',
{
defaultMessage: 'DOES NOT MATCH',
}
);

export const CONDITION_OPERATOR_TYPE_NESTED = i18n.translate(
'xpack.securitySolution.artifactCard.conditions.nestedOperator',
{
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -43451,7 +43451,6 @@
"unifiedDocViewer.sourceViewer.errorMessageTitle": "Une erreur s'est produite.",
"unifiedDocViewer.sourceViewer.refresh": "Actualiser",
"utils.filename.pathWarning": "Le chemin est peut-être incorrectement formé ; vérifiez la valeur",
"utils.filename.wildcardWarning": "l'utilisation de caractères génériques dans les chemins de fichier peut affecter les performances du point de terminaison",
"visTypeGauge.advancedSettings.visualization.legacyGaugeChartsLibrary.description": "Active la bibliothèque de graphiques héritée pour les graphiques de jauge dans Visualize.",
"visTypeGauge.advancedSettings.visualization.legacyGaugeChartsLibrary.name": "Bibliothèque de graphiques héritée pour les jauges",
"visTypeGauge.controls.gaugeOptions.alignmentLabel": "Alignement",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -43441,7 +43441,6 @@
"unifiedDocViewer.sourceViewer.errorMessageTitle": "エラーが発生しました",
"unifiedDocViewer.sourceViewer.refresh": "更新",
"utils.filename.pathWarning": "パスの形式が正しくない可能性があります。値を検証してください",
"utils.filename.wildcardWarning": "ファイルパスでワイルドカードを使用すると、エンドポイントのパフォーマンスに影響する可能性があります",
"visTypeGauge.advancedSettings.visualization.legacyGaugeChartsLibrary.description": "Visualizeでゲージグラフのレガシーグラフライブラリを有効にします。",
"visTypeGauge.advancedSettings.visualization.legacyGaugeChartsLibrary.name": "ゲージグラフのレガシーグラフライブラリ",
"visTypeGauge.controls.gaugeOptions.alignmentLabel": "アラインメント",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -43435,7 +43435,6 @@
"unifiedDocViewer.sourceViewer.errorMessageTitle": "发生错误",
"unifiedDocViewer.sourceViewer.refresh": "刷新",
"utils.filename.pathWarning": "路径的格式可能不正确;请验证值",
"utils.filename.wildcardWarning": "在文件路径中使用通配符可能会影响终端性能",
"visTypeGauge.advancedSettings.visualization.legacyGaugeChartsLibrary.description": "在 Visualize 中启用仪表盘图表的旧版图表库。",
"visTypeGauge.advancedSettings.visualization.legacyGaugeChartsLibrary.name": "仪表盘旧版图表库",
"visTypeGauge.controls.gaugeOptions.alignmentLabel": "对齐方式",
Expand Down