Skip to content

Commit

Permalink
[Security Solution] Set immutable param to true when updating prebuil…
Browse files Browse the repository at this point in the history
…t rules (elastic#161331)

Fixes: elastic#161305

## Summary

- Passes a new `immutable` params to the `upgradeRule` method that is
used when upgrading rules.
- Looks like we had a longstanding bug here in which rule updates of
rule types that changed the type of the rule were overwriting the
`immutable` prop to `false`. (Actually, those rules were deleted and
recreated with `immutable: false`)
- This was causing the `fetchAllInstalledRules` method of our
`ruleObjectsClient` NOT to retrieve these rules when they were already
installed.
- Since our installation `_review` and `_perform` endpoint depends on
this client, these rules that had had their types updated were being
incorrectly listed as available for installation.

## Testing

Repeat testing steps laid out in:
elastic#161305

Rules shouldn't be duplicated.


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
jpdjere authored Jul 7, 2023
1 parent 65fed09 commit 61fa0f5
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,32 @@ import {
import { createExceptionList, deleteExceptionList } from '../../tasks/api_calls/exceptions';
import { getExceptionList } from '../../objects/exception';
import { createRule } from '../../tasks/api_calls/rules';
import { cleanKibana, resetRulesTableState, deleteAlertsAndRules } from '../../tasks/common';
import {
cleanKibana,
resetRulesTableState,
deleteAlertsAndRules,
reload,
} from '../../tasks/common';
import { login, visitWithoutDateRange } from '../../tasks/login';

import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
import {
excessivelyInstallAllPrebuiltRules,
createAndInstallMockedPrebuiltRules,
getAvailablePrebuiltRulesCount,
preventPrebuiltRulesPackageInstallation,
} from '../../tasks/api_calls/prebuilt_rules';
import { createRuleAssetSavedObject } from '../../helpers/rules';

const EXPORTED_RULES_FILENAME = 'rules_export.ndjson';
const exceptionList = getExceptionList();

const prebuiltRules = Array.from(Array(7)).map((_, i) => {
return createRuleAssetSavedObject({
name: `Test rule ${i + 1}`,
rule_id: `rule_${i + 1}`,
});
});

describe('Export rules', () => {
const downloadsFolder = Cypress.config('downloadsFolder');

Expand All @@ -53,6 +67,8 @@ describe('Export rules', () => {
deleteAlertsAndRules();
// Rules get exported via _bulk_action endpoint
cy.intercept('POST', '/api/detection_engine/rules/_bulk_action').as('bulk_action');
// Prevent installation of whole prebuilt rules package, use mock prebuilt rules instead
preventPrebuiltRulesPackageInstallation();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
createRule(getNewRule({ name: 'Rule to export' })).as('ruleResponse');
});
Expand Down Expand Up @@ -83,23 +99,21 @@ describe('Export rules', () => {
});

it('shows a modal saying that no rules can be exported if all the selected rules are prebuilt', function () {
const expectedElasticRulesCount = 7;

excessivelyInstallAllPrebuiltRules();
createAndInstallMockedPrebuiltRules({ rules: prebuiltRules });

filterByElasticRules();
selectNumberOfRules(expectedElasticRulesCount);
selectNumberOfRules(prebuiltRules.length);
bulkExportRules();

cy.get(MODAL_CONFIRMATION_BODY).contains(
`${expectedElasticRulesCount} prebuilt Elastic rules (exporting prebuilt rules is not supported)`
`${prebuiltRules.length} prebuilt Elastic rules (exporting prebuilt rules is not supported)`
);
});

it('exports only custom rules', function () {
const expectedNumberCustomRulesToBeExported = 1;

excessivelyInstallAllPrebuiltRules();
createAndInstallMockedPrebuiltRules({ rules: prebuiltRules });

selectAllRules();
bulkExportRules();
Expand Down Expand Up @@ -151,8 +165,8 @@ describe('Export rules', () => {
// one rule with exception, one without it
const expectedNumberCustomRulesToBeExported = 2;

excessivelyInstallAllPrebuiltRules();

createAndInstallMockedPrebuiltRules({ rules: prebuiltRules });
reload();
selectAllRules();
bulkExportRules();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
waitForPrebuiltDetectionRulesToBeLoaded,
} from '../../tasks/alerts_detection_rules';
import {
excessivelyInstallAllPrebuiltRules,
getAvailablePrebuiltRulesCount,
createAndInstallMockedPrebuiltRules,
} from '../../tasks/api_calls/prebuilt_rules';
Expand Down Expand Up @@ -47,7 +46,6 @@ describe('Rules selection', () => {
});

it('should correctly update the selection label when rules are individually selected and unselected', () => {
excessivelyInstallAllPrebuiltRules();
waitForPrebuiltDetectionRulesToBeLoaded();

selectNumberOfRules(2);
Expand All @@ -60,7 +58,6 @@ describe('Rules selection', () => {
});

it('should correctly update the selection label when rules are bulk selected and then bulk un-selected', () => {
excessivelyInstallAllPrebuiltRules();
waitForPrebuiltDetectionRulesToBeLoaded();

cy.get(SELECT_ALL_RULES_BTN).click();
Expand All @@ -81,7 +78,6 @@ describe('Rules selection', () => {
});

it('should correctly update the selection label when rules are bulk selected and then unselected via the table select all checkbox', () => {
excessivelyInstallAllPrebuiltRules();
waitForPrebuiltDetectionRulesToBeLoaded();

cy.get(SELECT_ALL_RULES_BTN).click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const upgradeRule = async (

return createRules({
rulesClient,
immutable: true,
params: {
...rule,
// Force the prepackaged rule to use the enabled state from the existing rule,
Expand Down

0 comments on commit 61fa0f5

Please sign in to comment.