Skip to content

Commit ce31dd8

Browse files
maximpnmaximpn
committed
[Security Solution] Fix enable/disable action for rules with non-migrated lastRun.outcomeMsg (#258105)
**Resolves: #177852 **Relates to: #251164 ## Summary This PR fixes an issue blocking enabling and disabling security rules with non-migrated `lastRun.outcomeMsg` field. ## Details Some time ago Alerting Framework migrated `lastRun.outcomeMsg` from `string` to `string[]`. At that moment usual migrations were deprecated due to Serverless and model version migration wasn't ready yet. This migration went smoothly thanks to the according changes to the codebase. However, the changes didn't cover customers upgrading from `7.x` stack version and having something written to `lastRun.outcomeMsg`. #251164 fixed the issue appearing in attempt to update prebuilt rules. This PR fixes the left issue blocking enabling and disabling non-migrated security rules with `lastRun.outcomeMsg` type `string` field. ## Testing - Start Kibana - Log in under `system_indices_superuser` to be able to write to the system indices - Create a rule by using the command below - Enable/Disable the rule - Perform any other bulk and non-bulk actions on the rule ER: All actions should work without errors. Without this fix enable/disable action will result in error. <details> <summary>ES command to put a non-migrated rule</summary> ``` PUT .kibana_alerting_cases/_doc/alert:d62167ce-1022-4b2c-915d-024fe3e6e557 { "alert": { "name": "Test rule 1", "tags": [], "enabled": false, "alertTypeId": "siem.queryRule", "consumer": "siem", "legacyId": null, "schedule": { "interval": "5m" }, "actions": [], "params": { "author": [], "description": "123", "falsePositives": [], "from": "now-6m", "ruleId": "5ecfb16d-5af2-4a31-b880-319a4a2ca92b", "immutable": false, "ruleSource": { "type": "internal" }, "license": "", "outputIndex": "", "meta": { "kibana_siem_app_url": "http://localhost:5601/kbn/app/security" }, "maxSignals": 100, "riskScore": 21, "riskScoreMapping": [], "severity": "low", "severityMapping": [], "threat": [], "to": "now", "references": [], "version": 1, "exceptionsList": [], "relatedIntegrations": [], "requiredFields": [], "setup": "", "type": "query", "language": "kuery", "index": [ "apm-*-transaction*", "auditbeat-*", "endgame-*", "filebeat-*", "logs-*", "packetbeat-*", "traces-apm*", "winlogbeat-*", "-*elastic-cloud-logs-*" ], "query": "*:*", "filters": [] }, "mapped_params": { "risk_score": 21, "severity": "20-low" }, "createdBy": "elastic", "updatedBy": "elastic", "createdAt": "2026-03-17T10:52:28.688Z", "updatedAt": "2026-03-17T10:52:28.688Z", "apiKey": null, "apiKeyOwner": null, "apiKeyCreatedByUser": null, "throttle": null, "notifyWhen": null, "muteAll": false, "mutedInstanceIds": [], "executionStatus": { "status": "pending", "lastExecutionDate": "2026-03-17T10:52:28.688Z" }, "monitoring": { "run": { "history": [], "calculated_metrics": { "success_ratio": 0 }, "last_run": { "timestamp": "2026-03-17T10:52:28.688Z", "metrics": { "duration": 0, "total_search_duration_ms": null, "total_indexing_duration_ms": null, "total_alerts_detected": null, "total_alerts_created": null, "gap_duration_s": null } } } }, "lastRun": { "outcome": "failed", "outcomeMsg": "security_exception: [security_exception] Reason: missing authentication credentials for REST request [/_security/user/_has_privileges], caused by: \"\"", "warning": "read", "alertsCount": {}, "outcomeOrder": 20 }, "snoozeSchedule": [], "revision": 0, "running": false, "artifacts": { "dashboards": [], "investigation_guide": { "blob": "" } }, "meta": { "versionApiKeyLastmodified": "9.4.0" } }, "type": "alert", "references": [], "managed": false, "namespaces": [ "default" ], "coreMigrationVersion": "8.8.0", "typeMigrationVersion": "10.10.0", "updated_at": "2026-03-17T10:52:28.688Z", "created_at": "2026-03-17T10:52:28.688Z" } ``` </details> (cherry picked from commit b48ac09)
1 parent 286eb5c commit ce31dd8

13 files changed

Lines changed: 588 additions & 53 deletions

File tree

‎x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@kbn/core/server/mocks';
1515
import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
1616
import type { SavedObject } from '@kbn/core-saved-objects-server';
17+
import type { RawRule } from '../../../../types';
1718
import { ruleTypeRegistryMock } from '../../../../rule_type_registry.mock';
1819
import { alertingAuthorizationMock } from '../../../../authorization/alerting_authorization.mock';
1920
import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
@@ -111,7 +112,9 @@ describe('bulkDisableRules', () => {
111112
let actionsClient: jest.Mocked<ActionsClient>;
112113

113114
const mockCreatePointInTimeFinderAsInternalUser = (
114-
response = { saved_objects: [enabledRule1, enabledRule2] }
115+
response: { saved_objects: Array<SavedObject<Partial<RawRule>>> } = {
116+
saved_objects: [enabledRule1, enabledRule2],
117+
}
115118
) => {
116119
encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest
117120
.fn()
@@ -552,6 +555,109 @@ describe('bulkDisableRules', () => {
552555
});
553556
});
554557

558+
describe('lastRun outcome message migration', () => {
559+
test('migrates legacy string lastRun.outcomeMsg to string[] when bulk disabling', async () => {
560+
mockCreatePointInTimeFinderAsInternalUser({
561+
saved_objects: [
562+
{
563+
...enabledRule1,
564+
attributes: {
565+
...enabledRule1.attributes,
566+
lastRun: {
567+
outcome: 'failed',
568+
// @ts-expect-error test legacy outcomeMsg migration
569+
outcomeMsg: 'legacy message',
570+
},
571+
},
572+
},
573+
enabledRule2,
574+
],
575+
});
576+
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
577+
saved_objects: [disabledRuleForBulkDisable1, disabledRuleForBulkDisable2],
578+
});
579+
580+
await rulesClient.bulkDisableRules({ filter: 'fake_filter' });
581+
582+
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
583+
expect.arrayContaining([
584+
expect.objectContaining({
585+
id: 'id1',
586+
attributes: expect.objectContaining({
587+
lastRun: {
588+
outcome: 'failed',
589+
outcomeMsg: ['legacy message'],
590+
},
591+
}),
592+
}),
593+
]),
594+
{ overwrite: true }
595+
);
596+
});
597+
598+
test('leaves lastRun unchanged when outcomeMsg is already a string array', async () => {
599+
const lastRun = {
600+
outcome: 'succeeded' as const,
601+
outcomeMsg: ['msg a', 'msg b'],
602+
alertsCount: {
603+
new: 0,
604+
ignored: 0,
605+
recovered: 0,
606+
active: 0,
607+
},
608+
};
609+
mockCreatePointInTimeFinderAsInternalUser({
610+
saved_objects: [
611+
{
612+
...enabledRule1,
613+
attributes: {
614+
...enabledRule1.attributes,
615+
lastRun,
616+
},
617+
},
618+
],
619+
});
620+
mockUnsecuredSavedObjectFind(1);
621+
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
622+
saved_objects: [disabledRuleForBulkDisable1],
623+
});
624+
625+
await rulesClient.bulkDisableRules({ filter: 'fake_filter' });
626+
627+
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
628+
expect.arrayContaining([
629+
expect.objectContaining({
630+
id: 'id1',
631+
attributes: expect.objectContaining({
632+
lastRun,
633+
}),
634+
}),
635+
]),
636+
{ overwrite: true }
637+
);
638+
});
639+
640+
test('does not add lastRun when the rule has no lastRun', async () => {
641+
mockCreatePointInTimeFinderAsInternalUser({
642+
saved_objects: [enabledRule1],
643+
});
644+
mockUnsecuredSavedObjectFind(1);
645+
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
646+
saved_objects: [disabledRuleForBulkDisable1],
647+
});
648+
649+
await rulesClient.bulkDisableRules({ filter: 'fake_filter' });
650+
651+
const bulkCreateObjects = unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0] as Array<{
652+
id: string;
653+
attributes: Record<string, unknown>;
654+
}>;
655+
const attributesForRule = bulkCreateObjects.find((o) => o.id === 'id1')?.attributes;
656+
expect(attributesForRule).toBeDefined();
657+
expect(attributesForRule).not.toHaveProperty('lastRun');
658+
});
659+
});
660+
555661
describe('taskManager', () => {
556662
test('should call task manager bulkDisable', async () => {
557663
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({

‎x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
untrackRuleAlerts,
2828
updateMeta,
2929
bulkMigrateLegacyActions,
30+
migrateLegacyLastRunOutcomeMsg,
3031
} from '../../../../rules_client/lib';
3132
import { transformRuleAttributesToRuleDomain, transformRuleDomainToRule } from '../../transforms';
3233
import type {
@@ -180,6 +181,9 @@ const bulkDisableRulesWithOCC = async (
180181
: null,
181182
updatedBy: username,
182183
updatedAt: new Date().toISOString(),
184+
...(castedAttributes.lastRun
185+
? { lastRun: migrateLegacyLastRunOutcomeMsg(castedAttributes.lastRun) }
186+
: {}),
183187
});
184188

185189
rulesToDisable.push({

‎x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.test.ts‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
savedObjectsRepositoryMock,
1313
uiSettingsServiceMock,
1414
} from '@kbn/core/server/mocks';
15+
import type { SavedObject } from '@kbn/core/server';
16+
import type { RawRule } from '../../../../types';
1517
import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
1618
import { ruleTypeRegistryMock } from '../../../../rule_type_registry.mock';
1719
import { alertingAuthorizationMock } from '../../../../authorization/alerting_authorization.mock';
@@ -115,7 +117,9 @@ describe('bulkEnableRules', () => {
115117
let actionsClient: jest.Mocked<ActionsClient>;
116118

117119
const mockCreatePointInTimeFinderAsInternalUser = (
118-
response = { saved_objects: [disabledRule1, disabledRule2] }
120+
response: { saved_objects: Array<SavedObject<Partial<RawRule>>> } = {
121+
saved_objects: [disabledRule1, disabledRule2],
122+
}
119123
) => {
120124
encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest
121125
.fn()
@@ -660,6 +664,110 @@ describe('bulkEnableRules', () => {
660664
});
661665
});
662666

667+
describe('lastRun outcome message migration', () => {
668+
test('migrates legacy string lastRun.outcomeMsg to string[] when bulk enabling', async () => {
669+
mockCreatePointInTimeFinderAsInternalUser({
670+
saved_objects: [
671+
{
672+
...disabledRule1,
673+
attributes: {
674+
...disabledRule1.attributes,
675+
lastRun: {
676+
outcome: 'failed',
677+
// @ts-expect-error test legacy outcomeMsg migration
678+
outcomeMsg: 'legacy message',
679+
},
680+
},
681+
},
682+
disabledRule2,
683+
],
684+
});
685+
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
686+
saved_objects: [enabledRuleForBulkOps1, enabledRuleForBulkOps2],
687+
});
688+
689+
await rulesClient.bulkEnableRules({ filter: 'fake_filter' });
690+
691+
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
692+
expect.arrayContaining([
693+
expect.objectContaining({
694+
id: 'id1',
695+
attributes: expect.objectContaining({
696+
lastRun: {
697+
outcome: 'failed',
698+
outcomeMsg: ['legacy message'],
699+
},
700+
}),
701+
}),
702+
]),
703+
{ overwrite: true }
704+
);
705+
});
706+
707+
test('leaves lastRun unchanged when outcomeMsg is already a string array', async () => {
708+
const lastRun = {
709+
outcome: 'succeeded' as const,
710+
outcomeMsg: ['msg a', 'msg b'],
711+
alertsCount: {
712+
new: 0,
713+
ignored: 0,
714+
recovered: 0,
715+
active: 0,
716+
},
717+
};
718+
719+
mockCreatePointInTimeFinderAsInternalUser({
720+
saved_objects: [
721+
{
722+
...disabledRule1,
723+
attributes: {
724+
...disabledRule1.attributes,
725+
lastRun,
726+
},
727+
},
728+
],
729+
});
730+
mockUnsecuredSavedObjectFind(1);
731+
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
732+
saved_objects: [enabledRuleForBulkOps1],
733+
});
734+
735+
await rulesClient.bulkEnableRules({ filter: 'fake_filter' });
736+
737+
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
738+
expect.arrayContaining([
739+
expect.objectContaining({
740+
id: 'id1',
741+
attributes: expect.objectContaining({
742+
lastRun,
743+
}),
744+
}),
745+
]),
746+
{ overwrite: true }
747+
);
748+
});
749+
750+
test('does not add lastRun when the rule has no lastRun', async () => {
751+
mockCreatePointInTimeFinderAsInternalUser({
752+
saved_objects: [disabledRule1],
753+
});
754+
mockUnsecuredSavedObjectFind(1);
755+
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
756+
saved_objects: [enabledRuleForBulkOps1],
757+
});
758+
759+
await rulesClient.bulkEnableRules({ filter: 'fake_filter' });
760+
761+
const bulkCreateObjects = unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0] as Array<{
762+
id: string;
763+
attributes: Record<string, unknown>;
764+
}>;
765+
const attributesForRule = bulkCreateObjects.find((o) => o.id === 'id1')?.attributes;
766+
expect(attributesForRule).toBeDefined();
767+
expect(attributesForRule).not.toHaveProperty('lastRun');
768+
});
769+
});
770+
663771
describe('taskManager', () => {
664772
test('should return task id if enabling task failed', async () => {
665773
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({

‎x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
createNewAPIKeySet,
3838
updateMetaAttributes,
3939
bulkMigrateLegacyActions,
40+
migrateLegacyLastRunOutcomeMsg,
4041
} from '../../../../rules_client/lib';
4142
import type { RulesClientContext, BulkOperationError } from '../../../../rules_client/types';
4243
import { validateScheduleLimit } from '../get_schedule_frequency';
@@ -255,6 +256,9 @@ const bulkEnableRulesWithOCC = async (
255256
warning: null,
256257
},
257258
scheduledTaskId: rule.id,
259+
...(rule.attributes.lastRun
260+
? { lastRun: migrateLegacyLastRunOutcomeMsg(rule.attributes.lastRun) }
261+
: {}),
258262
});
259263

260264
const shouldScheduleTask = await getShouldScheduleTask(

‎x-pack/platform/plugins/shared/alerting/server/application/rule/methods/update/update_rule.ts‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
createNewAPIKeySet,
3030
updateMetaAttributes,
3131
bulkMigrateLegacyActions,
32+
migrateLegacyLastRunOutcomeMsg,
3233
} from '../../../../rules_client/lib';
3334
import type { RuleParams } from '../../types';
3435
import type { UpdateRuleData } from './types';
@@ -382,25 +383,3 @@ async function updateRuleAttributes<Params extends RuleParams = never>({
382383
// without fixing all of other solution types
383384
return rule as SanitizedRule<Params>;
384385
}
385-
386-
/**
387-
* Migrates legacy lastRun.outcomeMsg from string to string[]
388-
*
389-
* Rule SO schema forces lastRun.outcomeMsg to be string[].
390-
* However, some rules may have lastRun.outcomeMsg as string after upgrading from 7.x due to
391-
* lack of migration. lastRun.outcomeMsg schema change from string to string[] happened after
392-
* classical migrations were deprecated due to Serverless. And quite often it's not an issue
393-
* as lastRun is absent.
394-
*/
395-
function migrateLegacyLastRunOutcomeMsg<LastRun extends { outcomeMsg?: unknown }>(
396-
lastRun: LastRun
397-
): LastRun {
398-
if (typeof lastRun.outcomeMsg === 'string') {
399-
return {
400-
...lastRun,
401-
outcomeMsg: [lastRun.outcomeMsg],
402-
};
403-
}
404-
405-
return lastRun;
406-
}

0 commit comments

Comments
 (0)