Skip to content

Apply excluded event IDs as a must_not -> terms filter - #27400

Open
dennisoelkers wants to merge 6 commits into
masterfrom
fix/security-events-new-status-filter-query-length
Open

dennisoelkers wants to merge 6 commits into
masterfrom
fix/security-events-new-status-filter-query-length

Conversation

@dennisoelkers

Copy link
Copy Markdown
Member

Note: This needs a backport to previous versions.

Description

Motivation and Context

Event searches that exclude a set of event IDs (e.g. filtering for NEW events) inlined them into the Lucene query string as NOT (id:a OR id:b ...). OpenSearch rejects any query_string longer than search.query.max_query_string_length (32000 characters by default) with Failed to parse query, so such a search failed outright once the exclusion set reached roughly 969 IDs.

This PR is raising the ceiling for this, by using a must_not terms filter, which is not subject to that limit (instead, the terms limit applies, which defaults to 64k actual terms).

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have requested a documentation update.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

dennisoelkers and others added 5 commits September 15, 2026 15:33
Event searches that exclude a set of event IDs inlined them into the
Lucene query string as "NOT (id:a OR id:b ...)". OpenSearch rejects any
query_string longer than search.query.max_query_string_length (32000
characters by default) with "Failed to parse query", so such a search
failed outright once the exclusion set reached roughly 969 IDs.

Carry the IDs on EventsSearchFilter instead and let the storage adapters
apply them as a must_not terms filter, which is not subject to that
limit. The new adapter parameter is added behind a defaulting overload so
existing callers are unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Full backend tests could only seed MongoDB from a static fixture file.
Generating the ~1000 documents needed to exercise the security events
"New" status filter that way would mean committing about a megabyte of
generated JSON, so surface the collection that MongoDBTestService already
holds and let such tests build their documents in code.

Also shortens the comments added along with the must_not terms filter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
excludedIds is computed server-side and never sent by a client, but
exposing it as a JSON property added a required excluded_ids field to the
generated TypeScript type, breaking the frontend type check at two call
sites that build an EventsSearchFilter.

Mark it @JsonIgnore so the REST contract is unchanged; the field is still
carried in-process from the security events code down to the adapters.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@janheise janheise left a comment

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.

LGTM, with manual testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants