Skip to content

[Security Solution] Replaced the incorrect runtime type used for ruleSource#184004

Merged
xcrzx merged 1 commit intoelastic:mainfrom
xcrzx:fix-rule-schema
May 23, 2024
Merged

[Security Solution] Replaced the incorrect runtime type used for ruleSource#184004
xcrzx merged 1 commit intoelastic:mainfrom
xcrzx:fix-rule-schema

Conversation

@xcrzx
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

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

@xcrzx xcrzx self-assigned this May 22, 2024
Copy link
Copy Markdown
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 area labels May 22, 2024
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

💚 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
@kibanamachine kibanamachine added the backport:skip This PR 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 PR does not require backporting bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area 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