[Response Ops][Cases] Add more granular checks before decoding user actions (#124896)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co>
This commit is contained in:
Jonathan Buttner 2022-02-11 12:23:59 -05:00 committed by GitHub
parent d583a5c1c4
commit 97bd50da88
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 153 additions and 31 deletions

View file

@ -10,7 +10,7 @@ import {
SavedObjectMigrationContext,
SavedObjectsMigrationLogger,
} from 'kibana/server';
import { cloneDeep, omit } from 'lodash';
import { cloneDeep, omit, set } from 'lodash';
import { migrationMocks } from 'src/core/server/mocks';
import { removeRuleInformation } from './alerts';
@ -22,6 +22,89 @@ describe('alert user actions', () => {
context = migrationMocks.createContext();
});
describe('JSON.stringify throws an error', () => {
let jsonStringifySpy: jest.SpyInstance;
beforeEach(() => {
jsonStringifySpy = jest.spyOn(JSON, 'stringify').mockImplementation(() => {
throw new Error('failed to stringify');
});
});
afterEach(() => {
jsonStringifySpy.mockRestore();
});
it('logs an error when an error is thrown outside of the JSON.parse function', () => {
const doc = {
id: '123',
attributes: {
action: 'create',
action_field: ['comment'],
new_value:
'{"type":"generated_alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
},
type: 'abc',
references: [],
};
expect(removeRuleInformation(doc, context)).toEqual(doc);
const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"Failed to migrate user action alerts with doc id: 123 version: 8.0.0 error: failed to stringify",
Object {
"migrations": Object {
"userAction": Object {
"id": "123",
},
},
},
]
`);
});
});
describe('JSON.parse spy', () => {
let jsonParseSpy: jest.SpyInstance;
beforeEach(() => {
jsonParseSpy = jest.spyOn(JSON, 'parse');
});
afterEach(() => {
jsonParseSpy.mockRestore();
});
it.each([
['update', ['status']],
['update', ['comment']],
[undefined, ['comment']],
['create', ['status']],
['create', []],
])(
'does not modify the document and does not call JSON.parse when action: %s and action_field: %s',
(action, actionField) => {
const doc = {
id: '123',
attributes: {
action,
action_field: actionField,
new_value: 'open',
},
type: 'abc',
references: [],
};
expect(removeRuleInformation(doc, context)).toEqual(doc);
expect(jsonParseSpy).not.toBeCalled();
const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
expect(log.error).not.toBeCalled();
}
);
});
it('does not modify non-alert user action', () => {
const doc = {
id: '123',
@ -51,18 +134,7 @@ describe('alert user actions', () => {
expect(removeRuleInformation(doc, context)).toEqual(doc);
const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"Failed to migrate user action alerts with doc id: 123 version: 8.0.0 error: Unexpected end of JSON input",
Object {
"migrations": Object {
"userAction": Object {
"id": "123",
},
},
},
]
`);
expect(log.error).not.toBeCalled();
});
it('does not modify the document when new_value is null', () => {
@ -127,6 +199,27 @@ describe('alert user actions', () => {
ensureRuleFieldsAreNull(newDoc);
expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
expectParsedDocsToEqual(newDoc, doc);
});
it('sets the rule fields to null when the alert has an extra field', () => {
const doc = {
id: '123',
attributes: {
action: 'create',
action_field: ['comment'],
new_value:
'{"anExtraField": "some value","type":"alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
},
type: 'abc',
references: [],
};
const newDoc = removeRuleInformation(doc, context) as SavedObject<{ new_value: string }>;
ensureRuleFieldsAreNull(newDoc);
expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
expectParsedDocsToEqual(newDoc, doc);
});
it('sets the rule fields to null for a generated alert', () => {
@ -146,6 +239,7 @@ describe('alert user actions', () => {
ensureRuleFieldsAreNull(newDoc);
expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
expectParsedDocsToEqual(newDoc, doc);
});
it('preserves the references field', () => {
@ -165,10 +259,38 @@ describe('alert user actions', () => {
ensureRuleFieldsAreNull(newDoc);
expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
expectParsedDocsToEqual(newDoc, doc);
});
});
});
const expectParsedDocsToEqual = (
nulledRuleDoc: SavedObject<{ new_value: string }>,
originalDoc: SavedObject<{ new_value: string }>
) => {
expect(parseDoc(nulledRuleDoc)).toEqual(injectNullRuleFields(parseDoc(originalDoc)));
};
const parseDoc = (doc: SavedObject<{ new_value: string }>): SavedObject<{ new_value: {} }> => {
const copyOfDoc = cloneDeep(doc);
const decodedNewValue = JSON.parse(doc.attributes.new_value);
set(copyOfDoc, 'attributes.new_value', decodedNewValue);
return copyOfDoc;
};
const injectNullRuleFields = (doc: SavedObject<{ new_value: {} }>) => {
const copyOfDoc = cloneDeep(doc);
set(copyOfDoc, 'attributes.new_value', {
...doc.attributes.new_value,
rule: { id: null, name: null },
});
return copyOfDoc;
};
const docWithoutNewValue = (doc: {}) => {
const copyOfDoc = cloneDeep(doc);
return omit(copyOfDoc, 'attributes.new_value');

View file

@ -26,9 +26,13 @@ export function removeRuleInformation(
try {
const { new_value, action, action_field } = doc.attributes;
if (!isCreateComment(action, action_field)) {
return originalDocWithReferences;
}
const decodedNewValueData = decodeNewValue(new_value);
if (!isAlertUserAction(action, action_field, decodedNewValueData)) {
if (!isAlertUserAction(decodedNewValueData)) {
return originalDocWithReferences;
}
@ -66,15 +70,21 @@ function decodeNewValue(data?: string | null): unknown | null {
return null;
}
return JSON.parse(data);
try {
return JSON.parse(data);
} catch (error) {
return null;
}
}
function isAlertUserAction(
action?: string,
actionFields?: string[],
newValue?: unknown | null
): newValue is AlertCommentOptional {
return isCreateComment(action, actionFields) && isAlertObject(newValue);
function isAlertUserAction(newValue?: unknown | null): newValue is AlertCommentOptional {
const unsafeAlertData = newValue as AlertCommentOptional;
return (
unsafeAlertData !== undefined &&
unsafeAlertData !== null &&
(unsafeAlertData.type === GENERATED_ALERT || unsafeAlertData.type === CommentType.alert)
);
}
function isCreateComment(action?: string, actionFields?: string[]): boolean {
@ -89,13 +99,3 @@ function isCreateComment(action?: string, actionFields?: string[]): boolean {
type AlertCommentOptional = Partial<Omit<CommentRequestAlertType, 'type'>> & {
type: CommentType.alert | typeof GENERATED_ALERT;
};
function isAlertObject(data?: unknown | null): boolean {
const unsafeAlertData = data as AlertCommentOptional;
return (
unsafeAlertData !== undefined &&
unsafeAlertData !== null &&
(unsafeAlertData.type === GENERATED_ALERT || unsafeAlertData.type === CommentType.alert)
);
}