Feature exclude tactic name

[frederikb96]

Pull Request

Summary - What I changed

1. You use the repository for DaC and have a custom rule folder for your company rules
2. You export rules from kibana to the folder via the repos CLI
3. The name of the toml files for the rules are all prefixed with the tactic name but you dont want this since you have a clear naming convention for all your rules and the toml file name should be just the rule name (no tactic prefix)
4. So I added this new flag: "--no-tactic-filename"
5. With this the toml files are simply named like the rule name, nice!

The adjustment was straightforward, check the code diff :)

How To Test

1. Export rules via CLI

python -m detection_rules kibana export-rules -d custom-rules

2. And now use the new flag

python -m detection_rules kibana export-rules -d custom-rules --no-tactic-filename

Filenames are just the rule name now 🚀

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?

[eric-forte-elastic]

👋 Thanks for the PR and suggestion! A few other things that we should think about and/or address is that we have a unit test that our users would want to bypass if this approach. This can be done if adding the following bypass line to the test_config.yaml

- tests.test_all_rules.TestRuleFiles.test_rule_file_name_tactic

Additionally, even with this test we have a warning message as part of the loading of rules in a RuleCollection that will warn that the name does not match.
E.g.

WARNING: Rule path does not match required path: signin_console_login_no_mfa.toml != initial_access_aws_signin_single_factor_console_login_with_federated_user.toml

To test this, you can use any of the commands that load our rules into a RulesCollection, but for DaC testing specifically you can use something like the following:

 python -m detection_rules kibana import-rules -d new_dac_demo/rules/ -e -ac -o

You may want to consider adding this as a configuration option that will disable this warning if used.

Also, we should add this functionality to import-rules-to-repo to have feature parity. And add both cli options (or config options) to the appropriate places in the repo. If this is a new CLI option it should be in the CLI.md doc.

[frederikb96]

Thank you very much for the swift reply and suggestions. I really appreciate it 🙂

I added some commits to also implement the same flag for the import to prevent the warning. Moreover, I adjusted the help texts and the CLI.md to include the info about the unit test.

About the parity with import-rules-to-repo. I guess they dont use the tactic at all so far or am I wrong, here the code. So I would just keep it as it is.

I hope I got all your points :)

[eric-forte-elastic]

@frederikb96 Happy to, this is a great PR 🚀

A few notes, could you bump the patch version in the pyproject.toml? And yes you are correct with the parity that field is not available so you would already have achived parity 🙂, so there is no need to have an argument for import-rules-to-repo. Also, the multi-collection cli argument may not be needed, but that would depend on the implementation to allow for config support. Typically, for broader changes like this we would have an value in _config.yaml or an environment variable to allow for setting this in a more global sense, if we are impacting more than one workflow. In this case, because we are impacting the validation warning, and the import-rules process. If this is added to the config yaml then a few updates will be needed. First one would need to update the following config.py file to support this field.

detection-rules/detection_rules/config.py

Lines 175 to 195 in 753e8d8

	class RulesConfig:
	"""Detection rules config file."""
	deprecated_rules_file: Path
	deprecated_rules: Dict[str, dict]
	packages_file: Path
	packages: Dict[str, dict]
	rule_dirs: List[Path]
	stack_schema_map_file: Path
	stack_schema_map: Dict[str, dict]
	test_config: TestConfig
	version_lock_file: Path
	version_lock: Dict[str, dict]
	action_dir: Optional[Path] = None
	action_connector_dir: Optional[Path] = None
	auto_gen_schema_file: Optional[Path] = None
	bbr_rules_dirs: Optional[List[Path]] = field(default_factory=list)
	bypass_version_lock: bool = False
	exception_dir: Optional[Path] = None
	normalize_kql_keywords: bool = True
	bypass_optional_elastic_validation: bool = False

Additionally, we would need to update the detection_rules/etc/_config.yaml with this field as a commented out option.

One last thing, if you could add the functionality to disable the warning message that would be great! (see below for where it checks for the naming convention)

detection-rules/detection_rules/cli_utils.py

Lines 103 to 106 in 753e8d8

	if rule.path.name != rule_name:
	click.secho(
	f"WARNING: Rule path does not match required path: {rule.path.name} != {rule_name}", fg="yellow"
	)

Also, to check for linting issues (there appear to be some) you can use python -m flake8 tests detection_rules --ignore D203,N815 --max-line-length 120 to help pass the unit tests 🙂

[frederikb96]

@eric-forte-elastic Thanks again, that was a great hint. Way nicer with the config option 👍

I adjusted again:

1. Version adjusted
2. Config option added in config.py, the _config.yaml as comment, the kibana export function and the kibana import function.
3. The warning is not displayed anymore when using the config or the flag option during kibana import-rules
4. Used the lint command and adjusted the code lines

[frederikb96]

@eric-forte-elastic Anything else or would it be ready to merge? :)

[eric-forte-elastic]

@eric-forte-elastic Anything else or would it be ready to merge? :)

This looks really good, will run some tests and put results here, but otherwise looks good to me 👍

[eric-forte-elastic]

@frederikb96 I think you may need to update the pyproject toml for the required patch version bump.

[frederikb96]

@eric-forte-elastic Ah yes, already bumped it once, but in the meantime there was an update on main, so bumped it again now 🙂

[Mikaayenson]

👋 Thanks for the contribution!

[eric-forte-elastic]

This looks very good! Going to go head and merge this on your behalf @frederikb96 so that we can get this in without having to go back and forth bumping the toml version again 😂

[frederikb96]

This looks very good! Going to go head and merge this on your behalf @frederikb96 so that we can get this in without having to go back and forth bumping the toml version again 😂

Yes nice, thanks 😊 😄