[Bug] ECS Checks Overly Broad in Certain Integrations by eric-forte-elastic · Pull Request #6491 · elastic/detection-rules

eric-forte-elastic · 2026-07-23T15:48:11Z

Pull Request

Issue link(s):

Resolves https://github.com/elastic/ia-trade-team/issues/1036
Related: https://github.com/elastic/ia-trade-team/issues/1028

Summary - What I changed

KQL/EQL rule validation passed for ECS fields that a rule's related integrations never populate. The canonical example: a network_traffic rule using process.title validated successfully, although the network_traffic package does not populate that field for any of its data streams. The cause was that integration validation unioned the full ECS schema into every package's field schema, so any ECS field was accepted for any integration.

This PR removes that union. Integration validation now checks a query against:

the fields the package's field files declare (fields/*.yml, with external: ecs multi-fields such as process.name.text expanded), plus
the fields Elastic Agent adds to every document regardless of package (the ECS agent.*, cloud.* and host.* sets, data_stream.*, event.ingested, event.agent_id_status), folded into every data stream when integration-schemas.json.gz is built, plus
detection_rules/etc/non-ecs-schema.json entries for the rule's index patterns, as before.

Every integration is treated the same way. There is no per-package classification of which integrations get strict validation: a package that relies on the ecs@mappings component template and does not declare the ECS fields it populates has those fields added to non-ecs-schema.json under the rule's index pattern, in the same place we already record vendor-specific fields.

Legacy approaches/Context

KQL/EQL rule validation passed for ECS fields that a rule's related integrations never populate. The canonical example: a network_traffic rule using process.title validated successfully, although the network_traffic package does not populate that field for any of its data streams. For this integration only a subset of ECS fields are supported. In the network_traffic case, it is in ecs.yml. Not every integration specifies a subset, but where they are specified they are authoritative and the field validation has been upgraded to match this.

Primary change in the code is to restrict the ECS fields to the subset if the integration specifies a subset. Additionaly, for cases where we can manually confirm that there is an ecs field present, this PR adds another varient of a non-ecs schema called detection_rules/etc/integration-ecs-additions.json.

[!NOTE]
This could be included in the non-ecs schema file instead of a separate file. If folks have a preference, please comment in this issue stating either way.

Legacy approach

No longer needed.

For DaC users, we also added a find-ecs-scope-violations as this change may require a number of custom rules to be updated. This command can be used to help triage those rules. This can be especially useful if one is using auto generated schema where this change would not be overt as it would end up being automatically added to the auto gen schema.

This command can be run via python -m detection_rules dev integrations find-ecs-scope-violations

[!NOTE]
The validation path is different for ES|QL and other languages, so you will see in the PR changes

How To Test

Before (main):

$ # add `and process.title:"test"` to a network_traffic rule query
$ python -m detection_rules validate-rule rules/network/discovery_icmp_timestamp_or_information_request_from_the_internet.toml
Rule validation successful        # <- bug: network_traffic never populates process.title

After (this branch):

$ python -m detection_rules validate-rule rules/network/discovery_icmp_timestamp_or_information_request_from_the_internet.toml
kql.errors.KqlParseError: Error at line:8,column:7
Unknown field
  and process.title: "test"
      ^^^^^^^^^^^^^
integration_types: [network_traffic]

Try adding event.module or event.dataset to specify integration module

Checked against packages [network_traffic]; stack: 9.6.0; ecs: 9.4.0

Fields the integration does declare (e.g. destination.ip for network_traffic/icmp)
continue to validate, as do all ECS fields for ecs@mappings packages (cloud_defend).

$ python -m detection_rules find-ecs-scope-violations
0 rule(s) with ECS fields not declared by their integrations

$ python -m detection_rules validate-all
Rule validation successful

$ python -m pytest tests/test_integrations.py -q
49 passed

Schema regeneration (self-backfills any cache entry missing the new metadata):

$ python -m detection_rules dev integrations build-schemas

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-07-23T15:48:25Z

Copilot

Pull request overview

This PR tightens rule field validation so KQL/EQL/ES|QL queries only allow ECS fields that the referenced integration(s) actually declare (via ecs.yml-style files), with an explicit override mechanism for ECS fields injected by pipelines/agent defaults.

Changes:

Add ECS scoping metadata to integration schema cache generation and use it to restrict ECS field validation to declared ECS fields.
Introduce integration-ecs-additions.json to allow-list ECS fields populated outside package field definitions, and wire it into both local and ES|QL remote validation paths.
Add a new devtools command (find-ecs-scope-violations) plus unit tests to detect and validate ECS scope violations.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`tests/test_specific_rules.py`	Updates schema union logic to use `collect_schema_fields` and exclude new metadata keys.
`tests/test_integrations.py`	Adds tests covering strict vs full ECS behavior, cache metadata expectations, and ES
`tests/test_all_rules.py`	Updates schema union logic to use `collect_schema_fields` and exclude new metadata keys.
`pyproject.toml`	Bumps project version to 2.0.5.
`detection_rules/rule.py`	Adjusts required-fields ECS classification based on integration-scoped schemas.
`detection_rules/integrations.py`	Adds schema-cache metadata (`_ecs_declared`, `_uses_ecs_mappings`), implements strict ECS scoping, and loads ECS additions.
`detection_rules/index_mappings.py`	Adds ES
`detection_rules/etc/integration-ecs-additions.json`	New override file listing pipeline/agent-injected ECS fields per package/data stream.
`detection_rules/ecs.py`	Excludes new schema-cache metadata keys when building flattened schema.
`detection_rules/devtools.py`	Adds `find-ecs-scope-violations` command for DaC users to identify impacted rules.

eric-forte-elastic · 2026-08-07T17:03:27Z

Peer review with @Mikaayenson, instead of pulling the ingest pipeline fields into a non-ecs equivalent which is not versioned, we should instead explore baking this into the integration schema itself.

Example Alternative

eric-forte-elastic · 2026-09-09T18:02:54Z

 sequence by source.port, source.ip, destination.ip with maxspan=5s
 [network where data_stream.dataset == "suricata.eve" and event.kind == "alert" and
  event.severity != 3 and source.ip != null and destination.ip != null and
-  not source.domain : ("*nessusscan*", "SCCMPS*") and


Added this tuning here since the rule will fail if this PR merges (also verified in telemetry). source.domain is not reliably populated in Suricata.

That's fine. would be better to get this in separately to decouple the rule fix from this logic fix.

Sure, will decouple, then we can merge from main into this and it will be gone 👍

sodhikirti07

The schema checks looks good! @eric-forte-elastic thanks for fixing this.

elastic-vault-github-plugin-prod · 2026-09-10T18:22:37Z

⛔️ Test failed

Results

❌ Suricata and Elastic Defend Network Correlation (eql)
- coverage_issue: no_rta
- stack_validation_failed: no_rta

elastic-vault-github-plugin-prod · 2026-09-10T18:23:15Z

⛔️ Test failed

Results

❌ Suricata and Elastic Defend Network Correlation (eql)
- coverage_issue: no_rta
- stack_validation_failed: no_rta

elastic-vault-github-plugin-prod · 2026-09-10T18:25:39Z

⛔️ Test failed

Results

❌ Suricata and Elastic Defend Network Correlation (eql)
- coverage_issue: no_rta
- stack_validation_failed: no_rta

Mikaayenson

overall, lgtm. I tested locally against the release branches with no errors. We could probably refactor the helpers resolve_rule_packages, rule_datasets_by_package, rule_integrations_are_ecs_scoped, esql_indices_covered_by_packages to be a bit thinner, but not worth blocking over.

Mikaayenson · 2026-09-10T18:51:14Z

+# cover (o365 audit 3, crowdstrike alert 4); unmigrated streams declare far more (auditd_manager 42,
+# network_traffic 77+, fortinet_fortigate 146). Non-ECS names in the file (entityanalytics_entra_id
+# lists 36 `asset.*` names in ecs.yml) and multi-field expansions (`process.name.text`) do not count.
+MIN_DECLARED_ECS_FIELDS = 20


Where does this come from? Can we dynamically pull this from somewhere?

This is an arbitrary limit, in effect I just picked 20, not the best solution, but potentially better than the alternatives. In prior commits, we folded in the sample event (so be pseudo dynamic) but that was not deterministic enough to work.

In short, to do this correctly we would need to know what each ecs@mappings populates per integration. Given that we do not, we have to have some way of picking which sets of integrations get assumed to have full ECS compatibility and those that do not.

In this case, If the fields are above 20, then the package gets strict validation. We are assuming that having less than 20 fields represents residual definitions left over from after converting to using the ecs@mappings component template (elastic/integrations#10135) If we were to use strict validation on everything, then we would have to add every not explicitly defined ECS field to non-ecs, notionally maintaining an ecs@mappings definition by hand.

As far as why I picked 20, I was going off of this distribution:

┌─────────────────────┬─────────┐ │ Declared ECS fields │ Streams │ ├─────────────────────┼─────────┤ │ 0 │ 1 │ ├─────────────────────┼─────────┤ │ 1 to 9 │ 120 │ ├─────────────────────┼─────────┤ │ 10 to 19 │ 7 │ ├─────────────────────┼─────────┤ │ 20 to 39 │ 13 │ ├─────────────────────┼─────────┤ │ 40+ │ 61 │ └─────────────────────┴─────────┘

The 20 then determines which packages are in scope vs out of scope, but the packages can be manually moved if misclassified by the threshold.

Peer reviewed with @Mikaayenson and we will be now trying a non-ecs based approach where everything is strict as initial tests show that the increase of non ecs fields is small.

Updated Here: 80624e0

Mikaayenson · 2026-09-10T18:52:52Z

+
+
+@cached
+def elastic_agent_envelope_fields() -> dict[str, str]:


Does this change/need to be calculated or would a simple static dict suffice?

A fully static dict will not work, as this needs to be checked for updated every time we refresh the beats or ECS schema (this would have changed with 9.5.0 as an example). We could generate a static dict or file when we regenerate the beats and ecs schemas but we would still need a function like this to inject them, so it would end up being more complex rather than less complex.

Mikaayenson · 2026-09-10T18:59:56Z

+        f"{PACKAGE}.{INTEGRATION}.request.type": "long",
+    }
+    if scoped:
+        data_stream["_ecs_scoped"] = True


nit: instead of assigning _ecs_scoped here, what do you think about putting it higher in the structure? e.g. next to the integration version

"_meta": { "ecs_scoped": ["dns", "http"] # or {"dns": true, "http": true} }

this way we dont have to strip _ prefixed keys etc.

you could then later do

version_schema.setdefault("_meta", {}).setdefault("ecs_scoped", []) version_schema["_meta"]["ecs_scoped"].append(integration_name)

Updated in b52f9c2

eric-forte-elastic · 2026-09-10T19:18:56Z

+        selected = {
+            name: info
+            for name, info in flat.items()
+            if name.split(".", 1)[0] in ELASTIC_AGENT_ENVELOPE_FIELDSETS


Question from peer review with @Mikaayenson, can they be static? They may either always be the same, or change infrequently enough that we can include them in the shema objects (json)

We may be able to do this dynamically on build instead of on ingest.

Generally speaking not worth it given: #6491 (comment)

Co-authored-by: Mika Ayenson, PhD <Mikaayenson@users.noreply.github.com>

eric-forte-elastic · 2026-09-11T00:34:47Z

Re-test with new approach

Mikaayenson

im in favor of transparency despite the growing non-ecs-schema file. we can either work with the integration teams to update the integration if needed, but explicitly defining areas where the integration does / does not emit ecs fields will help us catch runtime errors.

Mikaayenson · 2026-09-11T01:12:49Z

 from .utils import cached, get_etc_path, read_gzip, unzip

 if TYPE_CHECKING:
+    import zipfile


do we need this if we're using quoted "zipfile.ZipFile"

Nope, good call, removed in 78f1466

eric-forte-elastic · 2026-09-11T01:31:27Z

Tested on release branches, found gap addressed in ae80907

Testing artifacts
pr6491_release_branch_test_results.zip

eric-forte-elastic added 3 commits July 23, 2026 10:33

init ecs integration update

b7e7d3e

ecs field file patterns

7e8b072

update docstring

725f119

eric-forte-elastic requested review from bryans3c, sodhikirti07 and terrancedejesus July 23, 2026 15:48

eric-forte-elastic self-assigned this Jul 23, 2026

eric-forte-elastic added bug Something isn't working python Internal python for the repository schema patch labels Jul 23, 2026

eric-forte-elastic added 4 commits July 23, 2026 12:05

esql validation

6612fbc

add ECS integration pipeline fields

8687952

Add source.domain to Suricata

4cd9d15

fix unit tests

23db8e1

eric-forte-elastic marked this pull request as ready for review July 23, 2026 18:06

eric-forte-elastic requested a review from Mikaayenson as a code owner July 23, 2026 18:06

Copilot AI review requested due to automatic review settings July 23, 2026 18:06

github-actions Bot added the backport: auto label Jul 23, 2026

Copilot started reviewing on behalf of eric-forte-elastic July 23, 2026 18:06 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread detection_rules/index_mappings.py Outdated

Comment thread tests/test_integrations.py Outdated

eric-forte-elastic marked this pull request as draft July 23, 2026 18:20

eric-forte-elastic added 2 commits July 23, 2026 14:37

Fix unit tests and integrations mappings

6de039e

Use separate module

94b5760

eric-forte-elastic marked this pull request as ready for review July 23, 2026 20:17

eric-forte-elastic and others added 2 commits July 23, 2026 16:30

Merge branch 'main' into ecs_integration_update

525ca44

patch bump

f338fdf

eric-forte-elastic marked this pull request as draft August 7, 2026 17:03

eric-forte-elastic commented Sep 9, 2026

View reviewed changes

eric-forte-elastic added the Rule: Tuning tweaking or tuning an existing rule label Sep 9, 2026

eric-forte-elastic marked this pull request as ready for review September 9, 2026 18:07

sodhikirti07 approved these changes Sep 10, 2026

View reviewed changes

eric-forte-elastic mentioned this pull request Sep 10, 2026

[Rule Tuning] Suricata and Elastic Defend Network Correlation #6763

Merged

5 tasks

eric-forte-elastic added 2 commits September 10, 2026 14:21

Merge remote-tracking branch 'origin/main' into ecs_integration_update

b86aeb8

update non-ecs for ingest pipeline

77cd5dd

linting

46a4b91

Merge branch 'main' into ecs_integration_update

7a6f9ff

Mikaayenson approved these changes Sep 10, 2026

View reviewed changes

eric-forte-elastic commented Sep 10, 2026

View reviewed changes

eric-forte-elastic and others added 3 commits September 10, 2026 15:38

update scoping

b52f9c2

update non-ecs

50e7a3a

Update to use non-ecs

80624e0

Co-authored-by: Mika Ayenson, PhD <Mikaayenson@users.noreply.github.com>

eric-forte-elastic force-pushed the ecs_integration_update branch from 9fe1eb9 to 80624e0 Compare September 11, 2026 00:27

remove duplicate tests

1b7a407

eric-forte-elastic and others added 3 commits September 10, 2026 20:47

fix unit tests

a236fcd

Merge branch 'main' into ecs_integration_update

e242262

non evs for 8_19

ae80907

Mikaayenson approved these changes Sep 11, 2026

View reviewed changes

update imports

78f1466

eric-forte-elastic merged commit d48b515 into main Sep 11, 2026
16 checks passed

eric-forte-elastic deleted the ecs_integration_update branch September 11, 2026 01:44

This was referenced Sep 11, 2026

[Bug] Missing Non ECS fields for ES|QL Rules post 6491 merge #6765

Merged

[Bug] Test Remote Rules Using Old Fields #6794

Merged



		@cached
		def elastic_agent_envelope_fields() -> dict[str, str]:

Conversation

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

Uh oh!

Pull Request

Summary - What I changed

How To Test

Checklist

Contributor checklist

Uh oh!

github-actions Bot commented Jul 23, 2026

Bug - Guidelines

Documentation and Context

Code Standards and Practices

Testing

Additional Checks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sodhikirti07 left a comment

Choose a reason for hiding this comment

Uh oh!

elastic-vault-github-plugin-prod Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elastic-vault-github-plugin-prod Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elastic-vault-github-plugin-prod Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mikaayenson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-forte-elastic commented Sep 11, 2026

eric-forte-elastic commented Jul 23, 2026 •

edited

Loading

eric-forte-elastic commented Aug 7, 2026 •

edited

Loading

eric-forte-elastic Sep 9, 2026 •

edited

Loading

eric-forte-elastic Sep 10, 2026 •

edited

Loading

elastic-vault-github-plugin-prod Bot commented Sep 10, 2026 •

edited

Loading

elastic-vault-github-plugin-prod Bot commented Sep 10, 2026 •

edited

Loading

elastic-vault-github-plugin-prod Bot commented Sep 10, 2026 •

edited

Loading

eric-forte-elastic Sep 10, 2026 •

edited

Loading

eric-forte-elastic Sep 10, 2026 •

edited

Loading