[FR] Add source_updated_at to Rule Schema as a Build Time Field

[terrancedejesus]

Issues

• [FR] Add source_updated_at to Rule Schema as a Build Time Field #2826

Summary

This pull request adds source_updated_at to the BaseRuleSchema, defines a minimum compatible stack for this field and dynamically generates it on rule loading. For more details, please refer to the issue referenced.

Testing

Testing Rule Version Adjustments

Based on the suggested changes, rule versions should not change even though source_updated_at is added to the final rule object. This is because we remove this key:value pair from the dictionary that is SHA256 hashed. This addresses the versioning concern of two separate versions between branches as it is not taken into consideration. We did this for a few reasons:

• We understand it is an exception, but the date is pulled from metadata, we never expect a rule author to add this in the rule schema, and it is controlled solely by us with no influence upstream
• We need a mechanism to bypass version breaking changes if we can and this supplies one; we also do this in endpoint rules as well so the approach is not net new to our repositories and logic

1. Open rule.py and comment out L982 (self._convert_add_source_updated_at(obj))
2. Run python -m detection_rules view-rule /Users/tdejesus/code/src/detection-rules/rules/windows/persistence_startup_folder_scripts.toml
3. Note the version is 109
4. Uncomment step 1. and run the CLI command again
5. Note the version is still 109

This is because regardless if the field is added or note, we simply remove it if found all the time when calculating SHA256. This would backport and happen for each branch, thus version would be the same but the field would be included in the package moving forward.

For compatibility, the field is still added to the BaseRuleData dataclass to support importing the rule.

Testing Field is Dynamically Generated

Remember that this field will only generate if the stack version (reflected by packages.yml) is >=8.12. Therefore we need to test that the field is included AND not included on on older branches

CLI Command: python -m detection_rules view-rule /Users/tdejesus/code/src/detection-rules/rules/windows/persistence_startup_folder_scripts.toml

The source_updated_at field should be visible in the output.

To re-create this for a lower stack version to ensure it does not appear, copy the rule.py and definitions.py files to a separate folder, checkout an older branch, move them back and then run the CLI command again; source_updated_at should not be added to the final rule output.

This is because of the logic here. This is also why the TestBuildTimeFields unit test class was removed as it is redundant and we do this check for every build time field.

[terrancedejesus]

@Mikaayenson - When working on DaC and import-rules, we should consider build time fields and implications in general when re-factoring this.

[brokensound77]

I think this is one of those times where, this arguably solves the problem we are aiming to solve, but with risks and ambiguity that may not justify it. IOW, potentially saying no to a good idea based on present and future implications.

This sets a precedent that I think introduces risk and ambiguity. This is something we have aimed to explicitly avoid in the past. A much safer approach would be to just add it as another field to the schema, populated by rule author -- or even at version lock time, where we grab the last commit time and insert it as a build time field.

Exempting fields from the calculation breaks the requirements of a build time field (which is why the exemption is needed).

It seems like all approaches have some kind of tradeoff to them, so there is no perfect solution, I just don't know that this is the best solution.

I know we have discussed this a ton, but I am blocking based on the need for further discussion and the implications of this approach

[terrancedejesus]

@brokensound77 - Happy to continue discussion more. Please let me know when you have time.
We have more details in the issue discussion: #2826 (comment)

[Mikaayenson]

Based on the update, this LGTM:

1. I personally like excluding explicitly this field from sha256 calculation. Whereas the other build time fields could introduce errors (e.g. field types changed over time, integrations populated at different stack versions), this date doesnt appear to have inconsequential consequences.
2. We talked about potentially a hybrid of both approaches (a. maintaining the field in the schema, and b. just building/including it only for packaging during the release), but the concern was that solution was too far removed from the schemas.

[Mikaayenson]

Update Feb 23

After speaking with D&R, another reason to hold on this is they anticipate more changes. It’s still unclear if they want to have a top-level field source_updated_at or an object with a field source.updated_at to make it future-proof.

So much more to come after our time away.

[Mikaayenson]

Update 28

Speaking with @terrancedejesus and @brokensound77 at EAH, here's the gist of how we can handle this:

• Treat the new field as a breaking change (don't push the schema updates until version+3 release is here)
• We also need to consider that global breaking changes introduce 1100+ rules into the rules package because of historical rules.

@banderror Alternatively is it possible on your end to either:

1. Could we use the updated_at that exists already?
2. Just pull the field from our metadata section (we would our updated date to the metadata)

[Mikaayenson]

Update Mar 14 - Simplified Protections Sync

• We're going to hold until 8.15 to release this.
• We don't need to update schemas are worry about import/export. The field will not be exported. It only needs to be dynamically generated and pushed on package release.

[Mikaayenson]

Update Aug 1

After speaking with @approksiu, we need to hold off until end of year until the rules management team is able to discuss further.

[shashank-elastic]

Removing this from Q2 docket