Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Make changes in the internal rule schema #180124

Closed
Tracked by #174168
jpdjere opened this issue Apr 5, 2024 · 3 comments · Fixed by #181581
Closed
Tracked by #174168

[Security Solution] Make changes in the internal rule schema #180124

jpdjere opened this issue Apr 5, 2024 · 3 comments · Fixed by #181581
Assignees
Labels
8.15 candidate Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0

Comments

@jpdjere
Copy link
Contributor

jpdjere commented Apr 5, 2024

Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168
Depends on: #180122, #180121

Needed for: all migration tickets listed in #174168

Summary

As part of the preparatory changes for the work in Milestone 3, we want to adapt the internal rule schema to the changes proposed in the RFC. In particular, we should:

  • add the ruleSource field to the BaseRuleParams schema, as an optional field.
  • change the immutable field in the BaseRuleParams schema to be optional.

The ruleSource field type should be based on the rule_source schema defined for the API schema, which uses camelCase. We should use a transformation function implemented as part of #180121 to avoid creating two versions of the schema (one camelCase and one snake_case).


Before migration on write takes place, the value for ruleSource will be undefined for all rules, while immutable will continue to have the same value. Once migration on write start to progressively happen, ruleSource will be populated, while immutable will be deleted from ES data.

Background

#### Internal rule schema
**The internal rule schema** needs to represent that the `immutable` and the new `ruleSource` field may not always exist, so they must be optional.
_Source: [x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts)_
```ts
export type BaseRuleParams = z.infer<typeof BaseRuleParams>;
export const BaseRuleParams = z.object({
// [...]
immutable: IsRuleImmutable.optional(),
ruleSource: RuleSource.transform(camelize).optional(),
// [...]
});
```
> Notice that in the internal schema we cannot simply reuse the `RuleSource` attribute defined for the API schema, since it should have CamelCase and the API schema uses snake_case. We need to apply a transformation to our Zod type. See this [issue](https://github.com/colinhacks/zod/issues/486#issuecomment-1567296747).
In the internal rule schema, there are two additional important reasons why we need to make sure that these two fields optional:
- When rules are executed, a call to the method `validateRuleTypeParams` is done, which is a method that validates the passed rule's parameters using the validators defined in `x-pack/plugins/security_solution/server/lib/detection_engine/rule_types`, within each of the rule query types files (for [EQL rules](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/eql/create_eql_alert_type.ts#L27), for example). The validation is done based on the internal rule schema `BaseRulesParams` displayed above. Having `ruleSource` as required field would cause custom rules or prebuilt rules that haven't had their schema migrated to fail during runtime.
- The Rule Client `update` method also calls the `validateRuleTypeParams` to validate the rule's params. Since the Rule Client's `update` method is used in our endpoint handlers, such as and `/rules/patch` and `/_bulk_actions`, these would fail when executed against a payload of custom rule.

@jpdjere jpdjere added triage_needed Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area labels Apr 5, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.15 candidate Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants