Skip to content

Fix parent-child query detection across classloaders - #6346

Draft
cwperks wants to merge 1 commit into
opensearch-project:mainfrom
cwperks:agent/fix-parent-child-query-detection
Draft

Fix parent-child query detection across classloaders#6346
cwperks wants to merge 1 commit into
opensearch-project:mainfrom
cwperks:agent/fix-parent-child-query-detection

Conversation

@cwperks

@cwperks cwperks commented Jul 29, 2026

Copy link
Copy Markdown
Member

Description

Detect has_parent and has_child queries by their registered NamedWriteable names instead of using instanceof against the concrete parent-join query builder classes.

The Security plugin bundles parent-join-client, while an installed OpenSearch distribution loads the parent-join module with a separate classloader. As a result, the query builder instance created by Core and the class referenced by Security have the same class name but different class identities, causing both instanceof checks to return false.

This could bypass the filter-level DLS guard for parent/child queries. In a locally installed distribution, the affected TLQ-DLS has_parent request returned HTTP 200 before this change instead of the expected rejection. With this change, it returns HTTP 500 with Unable to handle filter level DLS for parent or child queries.

The existing integration test did not expose this because it loads both plugins on the test JVM application classloader.

Changes

  • Compare QueryBuilder#getWriteableName() with the registered has_parent and has_child names.
  • Remove runtime type references to the parent-join query builder classes from the detector.
  • Add tests using generic QueryBuilder instances to verify detection is based on the registered name rather than concrete class identity.

Validation

  • ./gradlew :test --tests 'org.opensearch.security.util.ParentChildrenQueryDetectorTest' -Pcrypto.standard=FIPS-140-3
  • ./gradlew :integrationTest --tests 'org.opensearch.security.ParentChildRelationTests.hasParentWithTlqDls' -Pcrypto.standard=FIPS-140-3
  • ./gradlew :precommit -Pcrypto.standard=FIPS-140-3
  • Built the Security plugin and installed it into a locally built OpenSearch distribution; verified the TLQ-DLS has_parent request is rejected with the expected HTTP 500 response under the real separated classloader topology.

The aggregate multi-project precommit invocation additionally encountered pre-existing forbidden-API findings for URL.openStream() in the sample-resource plugin; the scoped root :precommit task passes.

Signed-off-by: Craig Perkins <craig5008@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.37%. Comparing base (283edfc) to head (d2da2f5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6346      +/-   ##
==========================================
- Coverage   75.39%   75.37%   -0.02%     
==========================================
  Files         456      456              
  Lines       29924    29925       +1     
  Branches     4530     4531       +1     
==========================================
- Hits        22560    22557       -3     
- Misses       5265     5271       +6     
+ Partials     2099     2097       -2     
Files with missing lines Coverage Δ
...rch/security/util/ParentChildrenQueryDetector.java 100.00% <100.00%> (ø)

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant