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] Replaced the incorrect runtime type used for ruleSource #184004

Merged
merged 1 commit into from
May 23, 2024

Conversation

xcrzx
Copy link
Contributor

@xcrzx xcrzx commented May 22, 2024

‼️ Part of critical path ‼️
Needed for: #180141 and #180128

Summary

This PR replaces the incorrect Zod schema for the ruleSource rule param.

Previously, the rule source field schema was implemented using a Zod transformation that automatically converted the snake-cased is_customized attribute to its camel-cased version isCustomized.

const RuleSourceCamelCased = RuleSource.transform(convertObjectKeysToCamelCase);

const RuleSource = z.object({
  type: z.literal('external'),
  is_customized: IsExternalRuleCustomized,
});

However, this meant that the expected input type for the schema was snake-cased, as the transformation happened only after validation.

Valid payload before:

{
  "ruleSource": {
    "type": "external",
    "is_customized": false // <- it should be camel cased
  }
}

To overcome this issue, the rule source schema was implemented without using the transformation (revert #180121).

Valid payload after:

{
  "ruleSource": {
    "type": "external",
    "isCustomized": false
  }
}

Important Note

This rule param schema change is considered safe because we do not currently use this field in the code. All values of this field are currently undefined. However, to ensure a Serverless release rollout without breaking changes, we need to release this schema change before we start writing any actual data.

@xcrzx xcrzx requested review from a team as code owners May 22, 2024 11:08
@xcrzx xcrzx requested review from dplumlee and rylnd May 22, 2024 11:08
@xcrzx xcrzx added release_note:skip Skip the PR/issue when compiling release notes 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 v8.15.0 labels May 22, 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)

@xcrzx xcrzx self-assigned this May 22, 2024
Copy link
Contributor

@nikitaindik nikitaindik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xcrzx! Changes LGTM.

@banderror banderror added bug Fixes for quality problems that affect the customer experience impact:critical This issue should be addressed immediately due to a critical level of impact on the product. Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules labels May 22, 2024
@@ -84,7 +84,6 @@ import {
TimestampOverrideFallbackDisabled,
} from '../../../../../common/api/detection_engine/model/rule_schema';
import type { SERVER_APP_ID } from '../../../../../common/constants';
import { convertObjectKeysToCamelCase } from '../../../../utils/object_case_converters';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also do something with this utility? For example:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function works as expected. It accepts any object and converts its keys to camel case. It has nothing to do with Zod and can be used (and is used) in other places, so I believe we can leave it as is.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #211478 succeeded 74b0bfbb96b30332a85467271d7a13777f44b901

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @xcrzx

@xcrzx xcrzx merged commit 6150a22 into elastic:main May 23, 2024
36 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 23, 2024
@xcrzx xcrzx deleted the fix-rule-schema branch May 23, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules impact:critical This issue should be addressed immediately due to a critical level of impact on the product. release_note:skip Skip the PR/issue when compiling release notes 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 this pull request may close these issues.

8 participants