From 33f26b3998d78b42566a17cb71aec3e4e766b03f Mon Sep 17 00:00:00 2001 From: Marshall Main Date: Fri, 29 Jul 2022 11:58:15 -0700 Subject: [PATCH 1/2] Require all fields to be accounted for in internal schema to response conversion --- .../schemas/request/rule_schemas.test.ts | 209 +++++++++++++++++- .../schemas/request/rule_schemas.ts | 12 +- 2 files changed, 217 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.test.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.test.ts index ad4350b864499b..12a0c08582a0fb 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.test.ts @@ -5,8 +5,9 @@ * 2.0. */ +import * as t from 'io-ts'; import type { CreateRulesSchema, SavedQueryCreateSchema } from './rule_schemas'; -import { createRulesSchema } from './rule_schemas'; +import { createRulesSchema, responseSchema } from './rule_schemas'; import { exactCheck, foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts-utils'; import { pipe } from 'fp-ts/lib/pipeable'; import { left } from 'fp-ts/lib/Either'; @@ -20,7 +21,7 @@ import { } from './rule_schemas.mock'; import { getListArrayMock } from '../types/lists.mock'; -describe('create rules schema', () => { +describe('rules schema', () => { test('empty objects do not validate', () => { const payload = {}; @@ -1287,4 +1288,208 @@ describe('create rules schema', () => { expect(getPaths(left(message.errors))).toEqual(['invalid keys "data_view_id"']); }); }); + + describe('response', () => { + const testSchema = { + required: { + testRequiredString: t.string, + }, + optional: { + testOptionalString: t.string, + }, + defaultable: { + testDefaultableString: t.string, + }, + }; + const schema = responseSchema(testSchema.required, testSchema.optional, testSchema.defaultable); + + describe('required fields', () => { + test('should allow required fields with the correct type', () => { + const payload = { + testRequiredString: 'required_string', + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('should not allow required fields to be undefined', () => { + const payload = { + testRequiredString: undefined, + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "testRequiredString"', + ]); + expect(message.schema).toEqual({}); + }); + + test('should not allow required fields to be omitted entirely', () => { + const payload = { + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "testRequiredString"', + ]); + expect(message.schema).toEqual({}); + }); + + test('should not allow required fields with an incorrect type', () => { + const payload = { + testRequiredString: 5, + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "5" supplied to "testRequiredString"', + ]); + expect(message.schema).toEqual({}); + }); + }); + + describe('optional fields', () => { + test('should allow optional fields with the correct type', () => { + const payload: t.TypeOf = { + testRequiredString: 'required_string', + testOptionalString: 'optional_string', + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('should allow optional fields to be undefined', () => { + const payload: t.TypeOf = { + testRequiredString: 'required_string', + testOptionalString: undefined, + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('should allow optional fields to be omitted entirely', () => { + const payload = { + testRequiredString: 'required_string', + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('should not allow optional fields with an incorrect type', () => { + const payload = { + testRequiredString: 'required_string', + testOptionalString: 5, + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "5" supplied to "testOptionalString"', + ]); + expect(message.schema).toEqual({}); + }); + }); + + describe('defaultable fields', () => { + test('should allow defaultable fields with the correct type', () => { + const payload = { + testRequiredString: 'required_string', + testDefaultableString: 'defaultable_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('should not allow defaultable fields to be undefined', () => { + const payload = { + testRequiredString: 'required_string', + testDefaultableString: undefined, + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "testDefaultableString"', + ]); + expect(message.schema).toEqual({}); + }); + + test('should allow defaultable fields to be omitted entirely', () => { + const payload = { + testRequiredString: 'required_string', + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "undefined" supplied to "testDefaultableString"', + ]); + expect(message.schema).toEqual({}); + }); + + test('should not allow defaultable fields with an incorrect type', () => { + const payload = { + testRequiredString: 'required_string', + testDefaultableString: 5, + }; + + const decoded = schema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "5" supplied to "testDefaultableString"', + ]); + expect(message.schema).toEqual({}); + }); + }); + }); }); diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts index e55fb9b6d763cc..b6c3ecfc731c4f 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts @@ -110,7 +110,11 @@ const patchSchema = < ]); }; -const responseSchema = < +type OrUndefined

= { + [K in keyof P]: P[K] | t.UndefinedC; +}; + +export const responseSchema = < Required extends t.Props, Optional extends t.Props, Defaultable extends t.Props @@ -119,9 +123,13 @@ const responseSchema = < optionalFields: Optional, defaultableFields: Defaultable ) => { + const optionalWithUndefined = Object.keys(optionalFields).reduce((acc, key) => { + acc[key] = t.union([optionalFields[key], t.undefined]); + return acc; + }, {}) as OrUndefined; return t.intersection([ t.exact(t.type(requiredFields)), - t.exact(t.partial(optionalFields)), + t.exact(t.type(optionalWithUndefined)), t.exact(t.type(defaultableFields)), ]); }; From 5afdd0b8db439a6f4cc95e28bc66ff2eee3c6894 Mon Sep 17 00:00:00 2001 From: Marshall Main Date: Fri, 29 Jul 2022 13:16:38 -0700 Subject: [PATCH 2/2] Add comment --- .../common/detection_engine/schemas/request/rule_schemas.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts index b6c3ecfc731c4f..46186479bf7268 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.ts @@ -123,6 +123,12 @@ export const responseSchema = < optionalFields: Optional, defaultableFields: Defaultable ) => { + // This bit of logic is to force all fields to be accounted for in conversions from the internal + // rule schema to the response schema. Rather than use `t.partial`, which makes each field optional, + // we make each field required but possibly undefined. The result is that if a field is forgotten in + // the conversion from internal schema to response schema TS will report an error. If we just used t.partial + // instead, then optional fields can be accidentally omitted from the conversion - and any actual values + // in those fields internally will be stripped in the response. const optionalWithUndefined = Object.keys(optionalFields).reduce((acc, key) => { acc[key] = t.union([optionalFields[key], t.undefined]); return acc;