Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Security Solution][Detections] Fix GET /api/detection_engine/rules?i…
…d={ruleId} endpoint (#122024)
**Ticket:** #120872 (this is a quick fix that [partially](#120872 (comment)) addresses issues described in there)
## Summary
When a Security rule fails on the Alerting Framework level (but not on the Detection Engine level), we return 500 error result from the Detections API which breaks the Rule Details page. This PR fixes that: instead, the API now returns 200 with the rule and its failed execution status.
## Details
When a rule fails on the Alerting Framework level, its `rule.executionStatus` might look something like that:
```
{
status: 'error',
lastExecutionDate: new Date('2021-12-24T04:44:44.961Z'),
error: {
reason: AlertExecutionStatusErrorReasons.Read,
message:
'security_exception: [security_exception] Reason: missing authentication credentials for REST request [/_security/user/_has_privileges], caused by: ""',
},
}
```
We merge the Framework's `rule.executionStatus` with our custom status based on the legacy `siem-detection-engine-rule-status` saved object, and return the resulting status from multiple endpoints, like `/api/detection_engine/rules/_find`, `/internal/detection_engine/rules/_find_status` and `/api/detection_engine/rules?id={ruleId}`.
The `/api/detection_engine/rules?id={ruleId}` route handler contained incorrect merging logic which, in the case of `rule.executionStatus.status === 'error'`, has been leading to an exception and 500 error result returned from it. This logic has been removed:
```ts
if (currentStatus != null && rule.executionStatus.status === 'error') {
currentStatus.attributes.lastFailureMessage = `Reason: ${rule.executionStatus.error?.reason} Message: ${rule.executionStatus.error?.message}`;
currentStatus.attributes.lastFailureAt =
rule.executionStatus.lastExecutionDate.toISOString();
currentStatus.attributes.statusDate =
rule.executionStatus.lastExecutionDate.toISOString();
currentStatus.attributes.status = RuleExecutionStatus.failed;
}
```
The proper logic of merging rule statuses is still there. Check `transform` -> `transformAlertToRule` -> `internalRuleToAPIResponse` -> `mergeAlertWithSidecarStatus`.
## Screenshots
**Before:** (this is how the page looks like if `/api/detection_engine/rules?id={ruleId}` returns 500)
**After:**
## How to test
I wasn't able to reproduce the original error described in #120872:
```
security_exception: [security_exception] Reason: missing authentication credentials for REST request [/_security/user/_has_privileges], caused by: ""
```
One way of getting it or a similar error from the Framework is by simulating it in the code.
Add this to `x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts`:
```ts
import { AlertExecutionStatusErrorReasons } from '../../../../../../alerting/common';
rule.executionStatus = {
status: 'error',
lastExecutionDate: new Date('2021-12-24T04:44:44.961Z'),
error: {
reason: AlertExecutionStatusErrorReasons.Read,
message:
'security_exception: [security_exception] Reason: missing authentication credentials for REST request [/_security/user/_has_privileges], caused by: ""',
},
};
```
Modify `getFailingRules` in `x-pack/plugins/security_solution/server/lib/detection_engine/routes/utils.ts`:
```ts
export const getFailingRules = async (
ids: string[],
rulesClient: RulesClient
): Promise<GetFailingRulesResult> => {
try {
const errorRules = await Promise.all(
ids.map(async (id) =>
rulesClient.resolve({
id,
})
)
);
return errorRules
.map((rule) => {
rule.executionStatus = {
status: 'error',
lastExecutionDate: new Date('2021-12-25T10:44:44.961Z'),
error: {
reason: AlertExecutionStatusErrorReasons.Read,
message:
'security_exception: [security_exception] Reason: missing authentication credentials for REST request [/_security/user/_has_privileges], caused by: ""',
},
};
return rule;
})
.filter((rule) => rule.executionStatus.status === 'error')
.reduce<GetFailingRulesResult>((acc, failingRule) => {
return {
[failingRule.id]: {
...failingRule,
},
...acc,
};
}, {});
} catch (exc) {
if (exc instanceof CustomHttpRequestError) {
throw exc;
}
throw new Error(`Failed to get executionStatus with RulesClient: ${(exc as Error).message}`);
}
};
```
Please note that `lastExecutionDate` should be greater than the date of the current legacy rule status SO.
### Checklist
- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios- Loading branch information
