From 165eadc9a31bea445c18981c6a184bf102e27a34 Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Mon, 15 May 2023 16:52:37 +0000 Subject: [PATCH 1/6] fix to targetFilePath/processExecutable regex, and null check --- x-pack/plugins/cloud_defend/public/common/utils.ts | 4 ++-- .../components/control_general_view_selector/index.test.tsx | 3 +++ .../components/control_yaml_view/hooks/policy_schema.json | 4 ++-- x-pack/plugins/cloud_defend/public/types.ts | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/cloud_defend/public/common/utils.ts b/x-pack/plugins/cloud_defend/public/common/utils.ts index 2e10d8982f82e..ccbdcd35dda73 100644 --- a/x-pack/plugins/cloud_defend/public/common/utils.ts +++ b/x-pack/plugins/cloud_defend/public/common/utils.ts @@ -178,14 +178,14 @@ export function validateMaxSelectorsAndResponses(selectors: Selector[], response return errors; } -export function validateStringValuesForCondition(condition: SelectorCondition, values: string[]) { +export function validateStringValuesForCondition(condition: SelectorCondition, values?: string[]) { const errors: string[] = []; const maxValueBytes = SelectorConditionsMap[condition].maxValueBytes || MAX_CONDITION_VALUE_LENGTH_BYTES; const { pattern, patternError } = SelectorConditionsMap[condition]; - values.forEach((value) => { + values?.forEach((value) => { if (pattern && !new RegExp(pattern).test(value)) { if (patternError) { errors.push(patternError); diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx b/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx index e54047a169a6e..b2e0863312e8b 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx @@ -306,12 +306,15 @@ describe('', () => { userEvent.type(el, '/*{enter}'); updatedSelector = onChange.mock.calls[2][0]; + expect(updatedSelector.hasErrors).toBeFalsy(); + rerender(); expect(findByText(errorStr)).toMatchObject({}); userEvent.type(el, 'badpath{enter}'); updatedSelector = onChange.mock.calls[3][0]; + expect(updatedSelector.hasErrors).toBeTruthy(); rerender(); diff --git a/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json b/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json index 0849fcb912e79..bee96b4e6b49c 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json +++ b/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json @@ -177,7 +177,7 @@ "minItems": 1, "items": { "type": "string", - "pattern": "^(?:\\/[^\\/\\*]+)+(?:\\/\\*|\\/\\*\\*)?$" + "pattern": "^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$" } }, "ignoreVolumeMounts": { @@ -319,7 +319,7 @@ "minItems": 1, "items": { "type": "string", - "pattern": "^(?:\\/[^\\/\\*]+)+(?:\\/\\*|\\/\\*\\*)?$" + "pattern": "^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$" } }, "processName": { diff --git a/x-pack/plugins/cloud_defend/public/types.ts b/x-pack/plugins/cloud_defend/public/types.ts index d8f23302b3baa..2e9b864db5a44 100755 --- a/x-pack/plugins/cloud_defend/public/types.ts +++ b/x-pack/plugins/cloud_defend/public/types.ts @@ -134,7 +134,7 @@ export const SelectorConditionsMap: SelectorConditionsMapProps = { selectorType: 'file', type: 'stringArray', maxValueBytes: 255, - pattern: '^(?:\\/[^\\/\\*]+)+(?:\\/\\*|\\/\\*\\*)?$', + pattern: '^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$', patternError: i18n.errorInvalidTargetFilePath, }, ignoreVolumeFiles: { selectorType: 'file', type: 'flag', not: ['ignoreVolumeMounts'] }, @@ -143,7 +143,7 @@ export const SelectorConditionsMap: SelectorConditionsMapProps = { selectorType: 'process', type: 'stringArray', not: ['processName'], - pattern: '^(?:\\/[^\\/\\*]+)+(?:\\/\\*|\\/\\*\\*)?$', + pattern: '^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$', patternError: i18n.errorInvalidProcessExecutable, }, processName: { From 6b76a1deb61a659d66a58e168d2155bd259cd132 Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Mon, 15 May 2023 17:01:09 +0000 Subject: [PATCH 2/6] min length 1 added to targetFilePath and processExecutable, as regex allows empty string --- .../components/control_yaml_view/hooks/policy_schema.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json b/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json index bee96b4e6b49c..33d4a1c010caa 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json +++ b/x-pack/plugins/cloud_defend/public/components/control_yaml_view/hooks/policy_schema.json @@ -177,7 +177,8 @@ "minItems": 1, "items": { "type": "string", - "pattern": "^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$" + "pattern": "^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$", + "minLength": 1 } }, "ignoreVolumeMounts": { @@ -319,7 +320,8 @@ "minItems": 1, "items": { "type": "string", - "pattern": "^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$" + "pattern": "^(?:\\/[^\\/\\*]+)*(?:\\/\\*|\\/\\*\\*)?$", + "minLength": 1 } }, "processName": { From 96352a183eb90d0e1df2e2af2588c7df6bd7060e Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Mon, 15 May 2023 17:15:05 +0000 Subject: [PATCH 3/6] more tests, fixed empty value checks for string conditions --- x-pack/plugins/cloud_defend/public/common/utils.ts | 9 ++++++++- .../control_general_view_selector/index.test.tsx | 13 +++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/cloud_defend/public/common/utils.ts b/x-pack/plugins/cloud_defend/public/common/utils.ts index ccbdcd35dda73..93294bc0c219a 100644 --- a/x-pack/plugins/cloud_defend/public/common/utils.ts +++ b/x-pack/plugins/cloud_defend/public/common/utils.ts @@ -186,7 +186,14 @@ export function validateStringValuesForCondition(condition: SelectorCondition, v const { pattern, patternError } = SelectorConditionsMap[condition]; values?.forEach((value) => { - if (pattern && !new RegExp(pattern).test(value)) { + if (value.length === 0) { + errors.push( + i18n.translate('xpack.cloudDefend.errorGenericEmptyValue', { + defaultMessage: '"{condition}" values cannot be empty', + values: { condition, pattern }, + }) + ); + } else if (pattern && !new RegExp(pattern).test(value)) { if (patternError) { errors.push(patternError); } else { diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx b/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx index b2e0863312e8b..824d0d88a120e 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx @@ -348,27 +348,32 @@ describe('', () => { } updatedSelector = onChange.mock.calls[1][0]; + expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); userEvent.type(el, '/*{enter}'); updatedSelector = onChange.mock.calls[2][0]; + expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); userEvent.type(el, '/usr/bin/ls{enter}'); updatedSelector = onChange.mock.calls[3][0]; + expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); userEvent.type(el, 'badpath{enter}'); updatedSelector = onChange.mock.calls[4][0]; - + expect(updatedSelector.hasErrors).toBeTruthy(); rerender(); + expect(getByText(errorStr)).toBeTruthy(); + userEvent.type(el, ' {enter}'); + updatedSelector = onChange.mock.calls[4][0]; + expect(updatedSelector.hasErrors).toBeTruthy(); + rerender(); expect(getByText(errorStr)).toBeTruthy(); }); From dae8695cc4cf0d9d8c60e741330b8b2b8b3c8e77 Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Mon, 15 May 2023 17:19:54 +0000 Subject: [PATCH 4/6] more testing --- .../index.test.tsx | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx b/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx index 824d0d88a120e..5ab2886de9604 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view_selector/index.test.tsx @@ -300,25 +300,27 @@ describe('', () => { } updatedSelector = onChange.mock.calls[1][0]; + expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); userEvent.type(el, '/*{enter}'); updatedSelector = onChange.mock.calls[2][0]; expect(updatedSelector.hasErrors).toBeFalsy(); - rerender(); - expect(findByText(errorStr)).toMatchObject({}); userEvent.type(el, 'badpath{enter}'); updatedSelector = onChange.mock.calls[3][0]; expect(updatedSelector.hasErrors).toBeTruthy(); - rerender(); - expect(getByText(errorStr)).toBeTruthy(); + + userEvent.type(el, ' {enter}'); + updatedSelector = onChange.mock.calls[4][0]; + expect(updatedSelector.hasErrors).toBeTruthy(); + rerender(); + expect(getByText('"targetFilePath" values cannot be empty')).toBeTruthy(); }); it('validates processExecutable conditions values', async () => { @@ -339,7 +341,7 @@ describe('', () => { 'input' ); - const errorStr = i18n.errorInvalidProcessExecutable; + const regexError = i18n.errorInvalidProcessExecutable; if (el) { userEvent.type(el, '/usr/bin/**{enter}'); @@ -350,31 +352,31 @@ describe('', () => { updatedSelector = onChange.mock.calls[1][0]; expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); + expect(findByText(regexError)).toMatchObject({}); userEvent.type(el, '/*{enter}'); updatedSelector = onChange.mock.calls[2][0]; expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); + expect(findByText(regexError)).toMatchObject({}); userEvent.type(el, '/usr/bin/ls{enter}'); updatedSelector = onChange.mock.calls[3][0]; expect(updatedSelector.hasErrors).toBeFalsy(); rerender(); - expect(findByText(errorStr)).toMatchObject({}); + expect(findByText(regexError)).toMatchObject({}); userEvent.type(el, 'badpath{enter}'); updatedSelector = onChange.mock.calls[4][0]; expect(updatedSelector.hasErrors).toBeTruthy(); rerender(); - expect(getByText(errorStr)).toBeTruthy(); + expect(getByText(regexError)).toBeTruthy(); userEvent.type(el, ' {enter}'); updatedSelector = onChange.mock.calls[4][0]; expect(updatedSelector.hasErrors).toBeTruthy(); rerender(); - expect(getByText(errorStr)).toBeTruthy(); + expect(getByText('"processExecutable" values cannot be empty')).toBeTruthy(); }); it('validates containerImageFullName conditions values', async () => { @@ -393,7 +395,7 @@ describe('', () => { 'input' ); - const errorStr = i18n.errorInvalidFullContainerImageName; + const regexError = i18n.errorInvalidFullContainerImageName; if (el) { userEvent.type(el, 'docker.io/nginx{enter}'); @@ -404,13 +406,13 @@ describe('', () => { updatedSelector = onChange.mock.calls[1][0]; rerender(); - expect(findByText(errorStr)).toMatchObject({}); + expect(findByText(regexError)).toMatchObject({}); userEvent.type(el, 'nginx{enter}'); updatedSelector = onChange.mock.calls[2][0]; rerender(); - expect(getByText(errorStr)).toBeTruthy(); + expect(getByText(regexError)).toBeTruthy(); }); it('validates kubernetesPodLabel conditions values', async () => { From 75b44f59c9fe2a07cc5a0d4efce3268fb105cb25 Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Mon, 15 May 2023 17:51:24 +0000 Subject: [PATCH 5/6] i18n fix --- x-pack/plugins/cloud_defend/public/common/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/cloud_defend/public/common/utils.ts b/x-pack/plugins/cloud_defend/public/common/utils.ts index 93294bc0c219a..0423a3ad44722 100644 --- a/x-pack/plugins/cloud_defend/public/common/utils.ts +++ b/x-pack/plugins/cloud_defend/public/common/utils.ts @@ -190,7 +190,7 @@ export function validateStringValuesForCondition(condition: SelectorCondition, v errors.push( i18n.translate('xpack.cloudDefend.errorGenericEmptyValue', { defaultMessage: '"{condition}" values cannot be empty', - values: { condition, pattern }, + values: { condition }, }) ); } else if (pattern && !new RegExp(pattern).test(value)) { From c6a12735e26c170671542435c7b1071b777acd42 Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Mon, 15 May 2023 18:02:19 +0000 Subject: [PATCH 6/6] another null check --- x-pack/plugins/cloud_defend/public/common/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/cloud_defend/public/common/utils.ts b/x-pack/plugins/cloud_defend/public/common/utils.ts index 0423a3ad44722..8200bb866db8a 100644 --- a/x-pack/plugins/cloud_defend/public/common/utils.ts +++ b/x-pack/plugins/cloud_defend/public/common/utils.ts @@ -186,7 +186,7 @@ export function validateStringValuesForCondition(condition: SelectorCondition, v const { pattern, patternError } = SelectorConditionsMap[condition]; values?.forEach((value) => { - if (value.length === 0) { + if (value?.length === 0) { errors.push( i18n.translate('xpack.cloudDefend.errorGenericEmptyValue', { defaultMessage: '"{condition}" values cannot be empty',