[Bug] TOML string outputs are not properly escaped by eric-forte-elastic · Pull Request #6000 · elastic/detection-rules

eric-forte-elastic · 2026-04-29T01:41:16Z

Pull Request

Issue link(s):

Resolves #5182

Related to: #6045

Summary - What I changed

In RuleTomlEncoder.dump_str, the multiline branch now escapes backslashes per line before emitting the """…""" block. Literal newlines and single " characters remain valid inside triple-DQ basic strings and are left alone. Only \ is doubled so that a source \: becomes TOML \\:, which the TOML parser decodes back to the original \:.

Before (invalid TOML):

query = """
file.path:
"xxx…"
file.path: Hello\:World
"""

After (valid TOML, same decoded value):

query = """
file.path:
"xxx…"
file.path: Hello\\:World
"""

The PR also adds this filter to the test_toml.jdson to test for this creating invalid toml.

This formating update also addresses the core bug of #6045 where newlines are introduced going from JSON -> TOML -> JSON. With this PR merged, only the TOML will show the newline. This is not ideal, but it makes this a cosmetic issue rather than breaking a DaC pipeline. For full fix, see #6046

Left is new desired output, and the right is the old output.

Alternatives

Also considered alternatives such as trying to use NESTED_PRESERVED_FIELD_NAMES or similar approaches as other formatting PRs. However, none were found that would reach the core issue. In the case of NESTED_PRESERVED_FIELD_NAMES, among other reasons we have a hard-coded skip for any key named query not just the base rule query (which perhaps should be updated in another PR).

How To Test

Either create a rule with a filter similar to that of the issue or use the supplied test ndjson.

issue_5182_rule.ndjson.txt

Once downloaded, run an import rule to repo command to see the newly created TOML and verify.

python -m detection_rules import-rules-to-repo ~/Downloads/issue_5182_rule.ndjson --required-only

Also run make test-remote-cli to ensure no formatting issues on testing rules.

Details

detection-rules on  5182-bug-toml-string-outputs-are-not-properly-escaped [$?] is  v1.6.29 via  v3.12.13 (detection-rules-build) on  eric.forte 
❯ make test-remote-cli
Installing all dependencies...
./env/detection-rules-build/bin/pip install .[dev]
Successfully built detection_rules
Installing collected packages: detection_rules
  Attempting uninstall: detection_rules
    Found existing installation: detection_rules 1.6.28
    Uninstalling detection_rules-1.6.28:
      Successfully uninstalled detection_rules-1.6.28
Successfully installed detection_rules-1.6.29

[notice] A new release of pip is available: 25.0.1 -> 26.1
[notice] To update, run: pip install --upgrade pip
./env/detection-rules-build/bin/pip install lib/kibana
Looking in indexes: https://pypi.org/simple, https://protections-bot:****@artifactory.elastic.dev/artifactory/api/pypi/pypi-endgame/simple
Processing ./lib/kibana
Successfully installed detection-rules-kibana-0.4.5

[notice] A new release of pip is available: 25.0.1 -> 26.1
Successfully installed detection-rules-kql-0.1.10

[notice] A new release of pip is available: 25.0.1 -> 26.1
[notice] To update, run: pip install --upgrade pip
Executing test_remote_cli script...
Running detection-rules remote CLI tests...
Performing a quick rule alerts search...
Requires .detection-rules-cfg.json credentials file set.
Loaded config file: /tmp/detection-rules/.detection-rules-cfg.json

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

No alerts detected
Setting Up Custom Directory...
Loaded config file: /tmp/detection-rules/.detection-rules-cfg.json

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

Created directory: tmp-custom/actions
Created directory: tmp-custom/action_connectors
Created directory: tmp-custom/exceptions
Created directory: tmp-custom/rules
Created directory: tmp-custom/rules_building_block
Created directory: tmp-custom/etc
Created file with default content: tmp-custom/etc/deprecated_rules.json
Created file with default content: tmp-custom/etc/version.lock.json
Created file with default content: tmp-custom/etc/packages.yaml
Created file with default content: tmp-custom/etc/stack-schema-map.yaml
Created file with default content: tmp-custom/etc/test_config.yaml
Created file with default content: tmp-custom/_config.yaml

# For details on how to configure the _config.yaml file,
# consult: /tmp/detection-rules/detection_rules/etc/_config.yaml
# or the docs: /tmp/detection-rules/docs-dev/custom-rules-management.md
Performing a rule conversion from ndjson to toml files...
Loaded config file: /tmp/detection-rules/.detection-rules-cfg.json

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

[+] Building rule for tmp-custom/rules/test_kql_rule.toml
[+] Building rule for tmp-custom/rules/test_kql_with_alert_supprestion_and_investigation_fileds.toml
[+] Building rule for tmp-custom/rules/test_kql_with_alert_suppression.toml
[+] Building rule for tmp-custom/rules/test_eql_rule.toml
[+] Building rule for tmp-custom/rules/test_esql_rule_with_shared_rule_exception.toml
/tmp/detection-rules/env/detection-rules-build/lib/python3.12/site-packages/elasticsearch/_sync/client/__init__.py:399: SecurityWarning: Connecting to 'https://127.0.0.1:9200' using TLS with verify_certs=False is insecure
  _transport = transport_class(
/tmp/detection-rules/detection_rules/index_mappings.py:366: ElasticsearchWarning: No limit defined, adding default limit of [1000]
  response = elastic_client.esql.query(query=query)
[+] Building rule for tmp-custom/rules/test_new_terms_rule_with_shared_rule_exception.toml
[+] Building rule for tmp-custom/rules/test_indicator_match_rule_with_email_actions.toml
[+] Building rule for tmp-custom/rules/test_threshold_with_rule_exception.toml
[+] Building rule for tmp-custom/rules/test_machine_learning_rule_with_index_action_connector.toml
[+] Building exception(s) for /tmp/detection-rules/tmp-custom/exceptions/1c8a1378-8f0d-4565-9ae0-abeeaf3981ca_exceptions.toml
[+] Building exception(s) for /tmp/detection-rules/tmp-custom/exceptions/0a4124f8-2074-450b-8689-d7dee319c666_exceptions.toml
[+] Building action connector(s) for /tmp/detection-rules/tmp-custom/action_connectors/e1b418e7-78df-4042-bfb0-1cc5fb6f7a4e_actions.toml
[+] Building action connector(s) for /tmp/detection-rules/tmp-custom/action_connectors/1b8d347f-2542-4390-85de-2653518311e2_actions.toml
15 results exported
9 rules converted
4 exceptions exported
2 actions connectors exported
Performing a rule import to kibana...
Loaded config file: /tmp/detection-rules/.detection-rules-cfg.json

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

/tmp/detection-rules/env/detection-rules-build/lib/python3.12/site-packages/elasticsearch/_sync/client/__init__.py:399: SecurityWarning: Connecting to 'https://127.0.0.1:9200' using TLS with verify_certs=False is insecure
  _transport = transport_class(
/tmp/detection-rules/detection_rules/index_mappings.py:366: ElasticsearchWarning: No limit defined, adding default limit of [1000]
  response = elastic_client.esql.query(query=query)
8 rule(s) successfully imported
 - 2cc8f325-e1b1-4201-8b8d-88a51c94992b
 - 7e0f6dae-5847-465f-89e9-a6de0e9ef918
 - 4c589d81-2622-4036-8cc7-372ea8f0e038
 - bcbd5906-fc38-4cbe-8b54-c2dba5d4b127
 - 2c6c5352-11cb-40a5-9294-e61ef5f1954f
 - 742feb36-ac4c-45e0-b8a5-3b3cfa66b6d2
 - 2390c9dd-ad90-4af6-97a4-1d607ba0f092
 - d46a29ca-9b5b-4cbd-b11f-35c6b59f207b
1 rule(s) failed to import!
 - 8a3296e2-4a74-4d51-b819-8d4e58377bf7: (400) Your license does not support machine learning. Please upgrade your license.
Performing a rule export...
Loaded config file: /tmp/detection-rules/.detection-rules-cfg.json

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

/tmp/detection-rules/env/detection-rules-build/lib/python3.12/site-packages/elasticsearch/_sync/client/__init__.py:399: SecurityWarning: Connecting to 'https://127.0.0.1:9200' using TLS with verify_certs=False is insecure
  _transport = transport_class(
/tmp/detection-rules/detection_rules/index_mappings.py:366: ElasticsearchWarning: No limit defined, adding default limit of [1000]
  response = elastic_client.esql.query(query=query)
14 results exported
9 rules converted
2 exceptions exported
1 action connectors exported
9 rules saved to tmp-custom
2 exception lists saved to /tmp/detection-rules/tmp-custom/exceptions
1 action connectors saved to /tmp/detection-rules/tmp-custom/action_connectors
Testing ESQL Rules...
========================================================================================================== test session starts ==========================================================================================================
platform linux -- Python 3.12.13, pytest-9.0.3, pluggy-1.6.0
rootdir: /tmp/detection-rules
configfile: pyproject.toml
plugins: typeguard-4.5.1
collected 17 items                                                                                                                                                                                                                      

tests/test_rules_remote.py .................                                                                                                                                                                                      [100%]

=========================================================================================================== warnings summary ============================================================================================================
tests/test_rules_remote.py: 20 warnings
  /tmp/detection-rules/env/detection-rules-build/lib/python3.12/site-packages/elasticsearch/_sync/client/__init__.py:399: SecurityWarning: Connecting to 'https://127.0.0.1:9200' using TLS with verify_certs=False is insecure
    _transport = transport_class(

tests/test_rules_remote.py: 14 warnings
  /tmp/detection-rules/detection_rules/index_mappings.py:366: ElasticsearchWarning: No limit defined, adding default limit of [1000]
    response = elastic_client.esql.query(query=query)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================================================== 17 passed, 34 warnings in 23.81s ====================================================================================================
Removing generated files...
Detection-rules Remote CLI tests completed!

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?

github-actions · 2026-04-29T01:41:30Z

…y-escaped

shashank-elastic

Local Testing works as expected

Testing 🟢


detection-rules on  5182-bug-toml-string-outputs-are-not-properly-escaped [$?] is 📦 v1.6.29 via 🐍 v3.12.8 (.venv) on ☁️  shashank.suryanarayana@elastic.co 
❯ python -m detection_rules import-rules-to-repo sample_rule.ndjson --required-only
Loaded config file: /Users/shashankks/elastic_workspace/detection-rules/.detection-rules-cfg.json

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

[+] Building rule for /Users/shashankks/elastic_workspace/detection-rules/rules/sample_escaped_backslash_in_query_string_filter.toml
1 results exported
1 rules converted
0 exceptions exported
0 actions connectors exported

detection-rules on  5182-bug-toml-string-outputs-are-not-properly-escaped [$?] is 📦 v1.6.29 via 🐍 v3.12.8 (.venv) on ☁️  shashank.suryanarayana@elastic.co 
❯

eric-forte-elastic · 2026-05-04T16:34:05Z

Updated testing for 0e034aa

make_test_remote_cli.txt

…y-escaped

https://github.com/elastic/detection-rules into 5182-bug-toml-string-outputs-are-not-properly-escaped

eric-forte-elastic · 2026-05-04T21:46:08Z

With the current implementation of #6046, we also resolve the primary issue affecting the community member who reported the bug. However, another bug specific to the escape character handling on multi line strings should be opened in its place if we use that PR to address it.

wingiti · 2026-05-05T14:09:13Z

+            # (as long as ``"""`` doesn't appear, which is guarded above via ``TRIPLE_DQ not in v``),
+            # but backslashes must be escaped so that e.g. ``Hello\:World`` is serialized as
+            # ``Hello\\:World`` -- otherwise ``\:`` is an invalid TOML escape sequence (issue #5182).
+            escaped_lines = [line.replace("\\", "\\\\") for line in lines]


This will break the fields "note", "setup", "description", "osquery" if they are multiline as they are somehow additionally escaped below in lines 253-274.

Same issue occurred in PR #6074

Query needs to be handled in another way or, from my perspective preferred, multiline should be escaped in general (like in this PR or #6074) but lines 253-274 needs to be removed. As I don't know what impact this might have I will wait for some further feedback for now.

Removing lines 253-274 without performing a fairly broad refactor of our toml_write process would cause a number of issues in our current pipeline.

The fields you mentioned will always be additionally escaped when written to TOML to counteract the escape invocation that is currently present in toml_write. Having json.dumps creates an issue as you described in #6074; however, adding an additional escape does not necessarily break these other fields. Their values change, but it does not inherently cause them to not function correctly (as the json dumps does). This is only of particular importance to os_query, as it is a functional field rather than a descriptive one. However, given that in the current bug is that os_query is broken when it goes to multi-line (sufficient length), this change is intended as it can address the issue.

Not sure about the os_query stuff, haven't had such an example so far but you are correct this might be the only field with impact on functionality.

Nevertheless if you use json dumps or your approach above (currently I don't see a difference in things I tested) you will add more escape characters than needed and which will be removed.
e.g. if the input of note field (on system 1) is: "some \ example" you will end up with a note field (on system 2) of: "some \ example"

This distorts the result (even it has no impact on functionality) and will drive you nuts if you compare two rules on two systems.

100% agree, generally speaking I am going to close this PR and put together a refactor proposal which should resolve this issue in light of:
#6000 (comment)

eric-forte-elastic added 3 commits April 27, 2026 11:00

Fix quote support

f4d204e

preserve_all_strings

1450037

Reduce complexity

fefd114

eric-forte-elastic requested review from shashank-elastic, terrancedejesus and w0rk3r April 29, 2026 01:41

eric-forte-elastic self-assigned this Apr 29, 2026

eric-forte-elastic added the bug Something isn't working label Apr 29, 2026

eric-forte-elastic linked an issue Apr 29, 2026 that may be closed by this pull request

[Bug] [DaC] TOML string outputs are not properly escaped #5182

Open

eric-forte-elastic added patch python Internal python for the repository labels Apr 29, 2026

eric-forte-elastic marked this pull request as ready for review April 29, 2026 01:45

eric-forte-elastic requested a review from Mikaayenson as a code owner April 29, 2026 01:45

Merge branch 'main' into 5182-bug-toml-string-outputs-are-not-properl…

3aa971e

…y-escaped

github-actions Bot added the backport: auto label Apr 29, 2026

patch bump

f65dd24

shashank-elastic approved these changes Apr 29, 2026

View reviewed changes

eric-forte-elastic added the community label May 4, 2026

Fix max string wrap

0e034aa

eric-forte-elastic added the detections-as-code label May 4, 2026

eric-forte-elastic and others added 3 commits May 4, 2026 12:36

Merge branch 'main' into 5182-bug-toml-string-outputs-are-not-properl…

9fa8d81

…y-escaped

patch bump

c5daa10

Merge branch '5182-bug-toml-string-outputs-are-not-properly-escaped' of

bbda2eb

https://github.com/elastic/detection-rules into 5182-bug-toml-string-outputs-are-not-properly-escaped

eric-forte-elastic mentioned this pull request May 4, 2026

[DaC] [Bug] Rule Formatter Line Wrapping Breaks Query String Filters #6045

Closed

5 tasks

eric-forte-elastic added 3 commits May 4, 2026 13:30

Add noqa

e2d945a

Revert rule formatter changes, handled in a different PR

fb49fb8

fix bump

5f08e5b

wingiti mentioned this pull request May 5, 2026

add json.dumps to fix multiline escape issue #6074

Closed

5 tasks

wingiti reviewed May 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug] TOML string outputs are not properly escaped#6000

[Bug] TOML string outputs are not properly escaped#6000
eric-forte-elastic wants to merge 12 commits into
mainfrom
5182-bug-toml-string-outputs-are-not-properly-escaped

eric-forte-elastic commented Apr 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

shashank-elastic left a comment

Uh oh!

eric-forte-elastic commented May 4, 2026 •

edited

Loading

Uh oh!

eric-forte-elastic commented May 4, 2026

Uh oh!

wingiti May 5, 2026

Uh oh!

eric-forte-elastic May 5, 2026

Uh oh!

wingiti May 5, 2026

Uh oh!

eric-forte-elastic May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

eric-forte-elastic commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Summary - What I changed

Alternatives

How To Test

Checklist

Contributor checklist

Uh oh!

github-actions Bot commented Apr 29, 2026

Bug - Guidelines

Documentation and Context

Code Standards and Practices

Testing

Additional Checks

Uh oh!

shashank-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

eric-forte-elastic commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-forte-elastic commented May 4, 2026

Uh oh!

wingiti May 5, 2026

Choose a reason for hiding this comment

Uh oh!

eric-forte-elastic May 5, 2026

Choose a reason for hiding this comment

Uh oh!

wingiti May 5, 2026

Choose a reason for hiding this comment

Uh oh!

eric-forte-elastic May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eric-forte-elastic commented Apr 29, 2026 •

edited

Loading

eric-forte-elastic commented May 4, 2026 •

edited

Loading