[SIEM][Detection Engine] Removes internal tags when copying signals from rules (#57881)

## Summary

Removes internal tags when copying tags from rules to the signals

Before where you can see internal tags going in:
<img width="1565" alt="Screen Shot 2020-02-14 at 5 17 36 PM" src="https://user-images.githubusercontent.com/1151048/74578011-6d1f4d00-4f4f-11ea-9057-e89896944af8.png">

After where you can see internal tags not being pushed in:
<img width="1555" alt="Screen Shot 2020-02-14 at 5 22 52 PM" src="https://user-images.githubusercontent.com/1151048/74578023-81fbe080-4f4f-11ea-8f37-86f7e5fe29f5.png">

Notice that these two internal tags should no longer be there:
```
__internal_immutable:false
__internal_rule_id:3b1bcf8a-7826-43b8-833b-6485aa700c41
```

### Checklist

Delete any items that are not applicable to this PR.

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

~~- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)~~

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/build_signal.test.ts

@@ -5,8 +5,14 @@
  */
 
 import { sampleDocNoSortId, sampleRule } from './__mocks__/es_results';
-import { buildSignal, buildAncestor, buildAncestorsSignal } from './build_signal';
+import {
+  buildSignal,
+  buildAncestor,
+  buildAncestorsSignal,
+  removeInternalTagsFromRule,
+} from './build_signal';
 import { Signal, Ancestor } from './types';
+import { INTERNAL_RULE_ID_KEY, INTERNAL_IMMUTABLE_KEY } from '../../../../common/constants';
 
 describe('buildSignal', () => {
   beforeEach(() => {
@@ -132,6 +138,77 @@ describe('buildSignal', () => {
     expect(signal).toEqual(expected);
   });
 
+  test('it builds a signal as expected with original_event if is present and without internal tags in them', () => {
+    const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
+    doc._source.event = {
+      action: 'socket_opened',
+      dataset: 'socket',
+      kind: 'event',
+      module: 'system',
+    };
+    const rule = sampleRule();
+    rule.tags = [
+      'some fake tag 1',
+      'some fake tag 2',
+      `${INTERNAL_RULE_ID_KEY}:rule-1`,
+      `${INTERNAL_IMMUTABLE_KEY}:true`,
+    ];
+    const signal = buildSignal(doc, rule);
+    const expected: Signal = {
+      parent: {
+        rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
+        id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
+        type: 'event',
+        index: 'myFakeSignalIndex',
+        depth: 1,
+      },
+      ancestors: [
+        {
+          rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
+          id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
+          type: 'event',
+          index: 'myFakeSignalIndex',
+          depth: 1,
+        },
+      ],
+      original_time: 'someTimeStamp',
+      original_event: {
+        action: 'socket_opened',
+        dataset: 'socket',
+        kind: 'event',
+        module: 'system',
+      },
+      status: 'open',
+      rule: {
+        created_by: 'elastic',
+        description: 'Detecting root and admin users',
+        enabled: true,
+        false_positives: [],
+        from: 'now-6m',
+        id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
+        immutable: false,
+        index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'],
+        interval: '5m',
+        risk_score: 50,
+        rule_id: 'rule-1',
+        language: 'kuery',
+        max_signals: 100,
+        name: 'Detect Root/Admin Users',
+        output_index: '.siem-signals',
+        query: 'user.name: root or user.name: admin',
+        references: ['http://www.example.com', 'https://ww.example.com'],
+        severity: 'high',
+        updated_by: 'elastic',
+        tags: ['some fake tag 1', 'some fake tag 2'],
+        to: 'now',
+        type: 'query',
+        updated_at: signal.rule.updated_at,
+        created_at: signal.rule.created_at,
+      },
+    };
+    expect(signal).toEqual(expected);
+  });
+
   test('it builds a ancestor correctly if the parent does not exist', () => {
     const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
     doc._source.event = {
@@ -190,6 +267,51 @@ describe('buildSignal', () => {
     expect(signal).toEqual(expected);
   });
 
+  test('it builds a ancestor correctly if the parent does exist without internal tags in them', () => {
+    const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
+    doc._source.event = {
+      action: 'socket_opened',
+      dataset: 'socket',
+      kind: 'event',
+      module: 'system',
+    };
+    doc._source.signal = {
+      parent: {
+        rule: '98c0bf9e-4d38-46f4-9a6a-8a820426256b',
+        id: '730ddf9e-5a00-4f85-9ddf-5878ca511a87',
+        type: 'event',
+        index: 'myFakeSignalIndex',
+        depth: 1,
+      },
+      ancestors: [
+        {
+          rule: '98c0bf9e-4d38-46f4-9a6a-8a820426256b',
+          id: '730ddf9e-5a00-4f85-9ddf-5878ca511a87',
+          type: 'event',
+          index: 'myFakeSignalIndex',
+          depth: 1,
+        },
+      ],
+    };
+    const rule = sampleRule();
+    rule.tags = [
+      'some fake tag 1',
+      'some fake tag 2',
+      `${INTERNAL_RULE_ID_KEY}:rule-1`,
+      `${INTERNAL_IMMUTABLE_KEY}:true`,
+    ];
+
+    const signal = buildAncestor(doc, rule);
+    const expected: Ancestor = {
+      rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
+      id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
+      type: 'signal',
+      index: 'myFakeSignalIndex',
+      depth: 2,
+    };
+    expect(signal).toEqual(expected);
+  });
+
   test('it builds a signal ancestor correctly if the parent does not exist', () => {
     const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
     doc._source.event = {
@@ -258,4 +380,40 @@ describe('buildSignal', () => {
     ];
     expect(signal).toEqual(expected);
   });
+
+  test('it removes internal tags from a typical rule', () => {
+    const rule = sampleRule();
+    rule.tags = [
+      'some fake tag 1',
+      'some fake tag 2',
+      `${INTERNAL_RULE_ID_KEY}:rule-1`,
+      `${INTERNAL_IMMUTABLE_KEY}:true`,
+    ];
+    const noInternals = removeInternalTagsFromRule(rule);
+    expect(noInternals).toEqual(sampleRule());
+  });
+
+  test('it works with an empty array', () => {
+    const rule = sampleRule();
+    rule.tags = [];
+    const noInternals = removeInternalTagsFromRule(rule);
+    const expected = sampleRule();
+    expected.tags = [];
+    expect(noInternals).toEqual(expected);
+  });
+
+  test('it works if tags does not exist', () => {
+    const rule = sampleRule();
+    delete rule.tags;
+    const noInternals = removeInternalTagsFromRule(rule);
+    const expected = sampleRule();
+    delete expected.tags;
+    expect(noInternals).toEqual(expected);
+  });
+
+  test('it works if tags contains normal values and no internal values', () => {
+    const rule = sampleRule();
+    const noInternals = removeInternalTagsFromRule(rule);
+    expect(noInternals).toEqual(rule);
+  });
 });

x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/build_signal.ts

@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { INTERNAL_IDENTIFIER } from '../../../../common/constants';
 import { SignalSourceHit, Signal, Ancestor } from './types';
 import { OutputRuleAlertRest } from '../types';
 
@@ -45,6 +46,7 @@ export const buildAncestorsSignal = (
 };
 
 export const buildSignal = (doc: SignalSourceHit, rule: Partial<OutputRuleAlertRest>): Signal => {
+  const ruleWithoutInternalTags = removeInternalTagsFromRule(rule);
   const parent = buildAncestor(doc, rule);
   const ancestors = buildAncestorsSignal(doc, rule);
   const signal: Signal = {
@@ -52,10 +54,24 @@ export const buildSignal = (doc: SignalSourceHit, rule: Partial<OutputRuleAlertR
     ancestors,
     original_time: doc._source['@timestamp'],
     status: 'open',
-    rule,
+    rule: ruleWithoutInternalTags,
   };
   if (doc._source.event != null) {
     return { ...signal, original_event: doc._source.event };
   }
   return signal;
 };
+
+export const removeInternalTagsFromRule = (
+  rule: Partial<OutputRuleAlertRest>
+): Partial<OutputRuleAlertRest> => {
+  if (rule.tags == null) {
+    return rule;
+  } else {
+    const ruleWithoutInternalTags: Partial<OutputRuleAlertRest> = {
+      ...rule,
+      tags: rule.tags.filter(tag => !tag.startsWith(INTERNAL_IDENTIFIER)),
+    };
+    return ruleWithoutInternalTags;
+  }
+};

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts

@@ -79,7 +79,6 @@ export default ({ getService }: FtrProviderContext): void => {
         expect(bodyToCompare).to.eql(getSimpleRuleOutputWithoutRuleId());
       });
 
-      // TODO: This is a valid issue and will be fixed in an upcoming PR and then enabled once that PR is merged
       it('should return a 200 ok but have a 409 conflict if we attempt to create the same rule_id twice', async () => {
         const { body } = await supertest
           .post(`${DETECTION_ENGINE_RULES_URL}/_bulk_create`)