-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Description
Summary
Now with 6 different rule types in the detection engine we've accumulated some tech debt. Below are areas that I think we can refactor to reduce maintenance burden and make the detection engine easier to reason about.
High level goals:
-
Reduce data structure duplication by moving towards a single source of truth for schemas
- We have 3 different schemas that each redefine parts of the rule schema and therefore must remain in sync -
signalSchema,RuleTypeParams, andRulesSchema. A single schema should represent rules and should be used as the basis for derivative schemas (for example, the rule APIs return a flattened version of the rule in the response - this flattened schema can be defined in terms of the source of truth so we avoid duplication)- When validating rule params in the executor, we use
signalSchemafor validation, but then use the params as typeRuleTypeParamswhich is defined independently. Some fields are defined withschema.nullableinsignalSchema, but the corresponding field inRuleTypeParamsis possiblyundefinednotnull. This can lead to some unexpected behavior withnullvalues showing up in rules and eventually in signals created by those rules.
- When validating rule params in the executor, we use
- Most or all of the rule params schema is copied in 30+ places as parameters for various routes and functions, e.g. create/update/patch rule. This makes it tedious and error prone to add new fields to the rule schema. Additionally, it's not always obvious when certain fields are purposely excluded from the list of parameters. In general I propose defining the parameters by using
PickorOmiton the source of truth so it's easy to update and clear when fields are included/omitted. - Rules are destructured and passed as a long list of parameters into functions such as
searchAfterAndBulkCreate, among others. Often many of these parameters are then passed through to other functions inside. This makes it hard to add new fields to rules since every function definition on the call stack has to be modified to pass each field through. Passing the entire rule through as a single unit makes it easier to add new fields, and IMO easier to reason about where each parameter comes from.
- We have 3 different schemas that each redefine parts of the rule schema and therefore must remain in sync -
-
Establish clear divisions between rule types by creating separate schemas for each rule type, merged (via union) into the single source of truth
- Each rule type could have its own params schema, e.g.
KQLRuleParams,EQLRuleParams,ThreatRuleParams, etc. The overall rule params schema then becomesKQLRuleParams | EQLRuleParams | ThreatRuleParams | ....- This would eliminate the need for extra functions that do validation that depends on the
typeof the rule
- This would eliminate the need for extra functions that do validation that depends on the
- Currently, the general design is one large params schema that covers all rule types. Fields that are specific to some subset of rule types are optional in the complete schema, even if they are required for the rule type (e.g.
thresholdis required for threshold rules, therefore it is optional in the complete rule schema). This means we have a number of functions that do some additional validation based on thetypewhen creating/updating rules, ensuring that these optional fields are there when required. However, these functions have to be maintained in sync with the schema and add complexity which is hard to reason about.
- Each rule type could have its own params schema, e.g.
I propose that the single source of truth is the rule SO data structure, which in turn is represented using a union of the individual rule types. The schemas for the various create/update/patch requests would be defined separately and should be the few places where we see some schema duplication due to the need for setting defaults for certain fields in the routes. However, unit tests can verify that the output from decoding with the create/update/patch schemas is a valid rule by following up with a decode using the source of truth rule schema. This ensures that the route schemas stay in sync with the source of truth.
Prior refactoring art
- [Security Solution][Detections] Adds framework for replacing API schemas #82462 (on the API schemas side)
- [Security Solution] Converge detection engine on single schema representation #96186 (on the BE schemas side)