Skip to content

DEV-15453: Updated ES sanitizing functions to handle sanitization byp… - #4707

Merged
gregrholden merged 2 commits into
qatfrom
fix/DEV-15453-sanitize-opensearch-input
Jul 29, 2026
Merged

DEV-15453: Updated ES sanitizing functions to handle sanitization byp…#4707
gregrholden merged 2 commits into
qatfrom
fix/DEV-15453-sanitize-opensearch-input

Conversation

@gregrholden

Copy link
Copy Markdown
Contributor

Description:

DEV-15453: Sanitize OpenSearch Input

  • Vulnerability scan finding f022 stated that the es_minimal_sanitize length-comparison logic returns unsanitized input when the removals equal escapes.
  • The fix was to ensure the sanitizing function returns the sanitized input regardless of length comparison and then apply that sanitization to the keyword search class.
  • Unit tests were added to cover the reported vulnerability parameters/vectors.

Technical Details:

  1. In search/v2/es_sanitization.py (lines 38-54): Changed function to always return the processed_string instead of the keyword variable that got assigned the processed_string value only if the lengths were not equal, which eliminates the length-equality bypass vulnerability and ensures the return value is always a sanitized version of the input.
  2. In common/query_with_filters.py (lines 197-202): Applied the above sanitization to the _KeywordSearch class before passing values to the ES_Q query string.
  3. Added more test cases to search/tests/unit/test_elasticsearch_helpers.py to validate changes made.
  4. Fixed missing return types and import ordering to pass lint tests.

Requirements for PR Merge:

  1. Unit & integration tests updated
  2. API documentation updated (examples listed below)
    1. API Contracts
    2. API UI
    3. Comments
  3. Data validation completed (examples listed below)
    1. Does this work well with the current frontend? Or is the frontend aware of a needed change?
    2. Is performance impacted in the changes (e.g., API, pipeline, downloads, etc.)?
    3. Is the expected data returned with the expected format?
  4. Appropriate Operations ticket(s) created
  5. Jira Ticket(s)
    1. DEV-0

Explain N/A in above checklist:

@gregrholden gregrholden added the ready for review [PR] ready to be reviewed label Jul 27, 2026

@james-at-kc james-at-kc 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. instead of returning original string. now returns sanitized string regardless if len is same or not. Ready for merge.

@gregrholden
gregrholden merged commit 01ae414 into qat Jul 29, 2026
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review [PR] ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants