[Bug] [DaC] Metadata maturity field default mismatch and poor enforcement of rule naming conventions

[eric-forte-elastic]

Pull Request

Issue link(s):

Resolves #4282

Summary - What I changed

In this PR I added explicit warnings when the rule file path does not match the expected path convention as if the rule was created via Kibana or the Detection Rules CLI rule prompt. This check occurs when the rules are added to a collection in the kibana import-rules and the export-rules-from-repo logic. The other routes assign the filename using the expected convention. This check simply compares the real path with what the constructed path would be if the rule was being written to the repo. If these do not match, it writes a warning message via click.

Additionally, this PR addresses a bug where if a rule already exists in the repo, the maturity metadata field may change unexpectedly depending on the import path one uses. Originally, the default metadata value on importing rules to the repo was "development" if it came from an .ndjson file and production if it came from Kibana. In this way, rules could move from production back to development if an .ndjson file was used for the import instead of the Kibana CLI/API commands. This PR adds a check that if the file exists in the repo/on disk in the expected location, it will instead use the existing maturity value; otherwise, it will default to development

There is also an update that we may or may not want to include that is currently in this PR which, in addition to maturity, the creation and modified dates are now inherited from the file if they exist, and if not, the dates from Kibana are used. However, as this represents a potential change to how dates are calculated (before the date is specific to the date that the rule existed on disk in the repo) we may or may not want to include these changes and I would ask reviewers to leave their thoughts on this change in the PR.

How To Test

1. Create a rule in detection rules, not using the rule creation CLI where the file name does not match the naming convention and where the metadata created and modified dates are a date prior to the current day. Additionally, have a correctly named file where the metadata created and modified dates are a date prior to the current day and the maturity is set at production.
2. Import this rule into Kibana (or ndjson)
3. See the new warning printed to the screen
   e.g.

❯ python -m detection_rules kibana --space demo --ignore-ssl-errors true import-rules
Loaded config file: /home/forteea1/Code/dac_demo/detection-rules/.detection-rules-cfg.json

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█

WARNING: Rule path does not match required path: defense_evasion_sts_get_federation_token.toml != defense_evasion_first_occurrence_of_sts_getfederationtoken_request_by_user.toml
WARNING: Rule path does not match required path: execution_suspicious_jar_child_process.toml != execution_deprecated_suspicious_java_child_process.toml
3 rule(s) successfully imported
 - 7a5cc9a8-5ea3-11ef-beec-f661ea17fbce
 - 8acb7614-1d92-4359-bfcf-478b6d9de150
 - 5930e5ed-37af-46ee-a77a-43db82af689c

5. Export this rule from Kibana (or ndjson)
6. See that the rule where the name matches that the maturity did not change.

Checklist

• Added a label for the type of pr: bug, enhancement, schema, maintenance, Rule: New, Rule: Deprecation, Rule: Tuning, Hunt: New, or Hunt: Tuning so guidelines can be generated
• Added the meta:rapid-merge label if planning to merge within 24 hours
• Secret and sensitive material has been managed correctly
• Automated testing was updated or added to match the most common scenarios
• Documentation and comments were added for features that require explanation

Contributor checklist

• Have you signed the contributor license agreement?
• Have you followed the contributor guidelines?

[protectionsmachine]

Bug - Guidelines

These guidelines serve as a reminder set of considerations when addressing a bug in the code.

Documentation and Context

• Provide detailed documentation (description, screenshots, reproducing the bug, etc.) of the bug if not already documented in an issue.
• Include additional context or details about the problem.
• Ensure the fix includes necessary updates to the release documentation and versioning.

Code Standards and Practices

• Code follows established design patterns within the repo and avoids duplication.
• Code changes do not introduce new warnings or errors.
• Variables and functions are well-named and descriptive.
• Any unnecessary / commented-out code is removed.
• Ensure that the code is modular and reusable where applicable.
• Check for proper exception handling and messaging.

Testing

• New unit tests have been added to cover the bug fix or edge cases.
• Existing unit tests have been updated to reflect the changes.
• Provide evidence of testing and detecting the bug fix (e.g., test logs, screenshots).
• Validate that any rules affected by the bug are correctly updated.
• Ensure that performance is not negatively impacted by the changes.
• Verify that any release artifacts are properly generated and tested.

Additional Checks

• Ensure that the bug fix does not break existing functionality.
• Review the bug fix with a peer or team member for additional insights.
• Verify that the bug fix works across all relevant environments (e.g., different OS versions).
• Confirm that all dependencies are up-to-date and compatible with the changes.
• Confirm that the proper version label is applied to the PR patch, minor, major.

[terrancedejesus]

I may be reading this wrong.

1. For a given rule, get the first tactic and build expected file naming convention
2. If that rule exists, initialize rule collection and load that specific rule
3. If we successfully load that rule, overwrite maturity with what the rule has statically defined
4. Overwrite the updated and created dates by formatting what is statically in the fine --> Is this necessary?

Other than my question on 4. it looks good to me.

[eric-forte-elastic]

For question 4, short answer is no. This is not necessary and we may not want to do it, looking for input as to other's thoughts on this. More context from the PR summary above.

There is also an update that we may or may not want to include that is currently in this PR which, in addition to maturity, the creation and modified dates are now inherited from the file if they exist, and if not, the dates from Kibana are used. However, as this represents a potential change to how dates are calculated (before the date is specific to the date that the rule existed on disk in the repo) we may or may not want to include these changes and I would ask reviewers to leave their thoughts on this change in the PR.

[terrancedejesus]

Is click the best option we have without adding new dependencies for warnings?

[eric-forte-elastic]

We do not have a logging standard for Detection Rules as far as I am aware, and each entry point for the CLI commands uses click. Other than introducing something new that would break current convention, I do not see a strong alternative, but certainly open to suggestions.

[terrancedejesus]

Couple of questions. Otherwise looks good to me.