Skip to content

Limit tenant checks to multi-doc actions - #6335

Merged
DarshitChanpura merged 1 commit into
opensearch-project:mainfrom
cwperks:agent/fix-tenant-monitor-requests
Jul 27, 2026
Merged

Limit tenant checks to multi-doc actions#6335
DarshitChanpura merged 1 commit into
opensearch-project:mainfrom
cwperks:agent/fix-tenant-monitor-requests

Conversation

@cwperks

@cwperks cwperks commented Jul 27, 2026

Copy link
Copy Markdown
Member

Description

Narrows the legacy Dashboards multi-tenancy concrete-index check to the multi-document request types it is intended to protect: bulk, multi-get, multi-search, and multi-term-vectors.

The check was being applied to every action. As a result, APIs such as _list/indices were denied when they issued internal indices:monitor/stats and cluster:monitor/health requests against a page containing a concrete Dashboards tenant index.

This is a tactical fix for #6327. A follow-up can carry trusted request provenance from the originating list-indices request so Security can distinguish internal derived requests from user requests that explicitly target a concrete tenant index.

Testing

  • Added an integration test covering _list/indices/.kib* with a tenant header.
  • Existing cross-tenant multi-document request coverage remains enabled.
  • ./gradlew :integrationTest --tests 'org.opensearch.security.privileges.int_tests.DashboardMultiTenancyIntTests' -Pcrypto.standard=FIPS-140-3
  • ./gradlew :precommit -Pcrypto.standard=FIPS-140-3

The aggregate precommit task also reaches the sample resource plugin and currently fails on two pre-existing forbidden URL.openStream() uses in its transport actions; the root :precommit task passes.

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

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 7338a02.

PathLineSeverityDescription
src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesInterceptor.java244mediumThe cross-tenant protection check is narrowed from applying to all non-local-all requests to only four specific request types (BulkRequest, MultiGetRequest, MultiSearchRequest, MultiTermVectorsRequest). All other request types — individual GET, DELETE, INDEX, UPDATE, plain SearchRequest, etc. — now bypass this code path entirely. The stated justification (fixing _list/indices API compatibility) could be addressed more narrowly, and the broad restriction of a tenant isolation check in a security interceptor warrants verification that alternative protections cover the excluded request types.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 1 | Low: 0


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@cwperks cwperks added the backport 3.8 Backports to 3.8 label Jul 27, 2026
@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 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.36%. Comparing base (14f6351) to head (7338a02).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...eges/actionlevel/legacy/PrivilegesInterceptor.java 0.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6335      +/-   ##
==========================================
- Coverage   75.41%   75.36%   -0.06%     
==========================================
  Files         456      456              
  Lines       29923    29924       +1     
  Branches     4529     4530       +1     
==========================================
- Hits        22567    22552      -15     
- Misses       5264     5277      +13     
- Partials     2092     2095       +3     
Files with missing lines Coverage Δ
...eges/actionlevel/legacy/PrivilegesInterceptor.java 73.97% <0.00%> (-1.92%) ⬇️

... and 11 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.

@DarshitChanpura
DarshitChanpura merged commit 283edfc into opensearch-project:main Jul 27, 2026
68 of 70 checks passed
@cwperks

cwperks commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

The coordinated provenance follow-up is now working in three draft PRs:

Core carries an all-indices-origin marker through stats/health child requests and broadcast fan-out. Security accepts it only with the local Core ThreadContext proof or authenticated node-to-node transport; the serialized marker alone is not sufficient. ML Commons establishes that trusted local context only when ListIndexTool originally targeted all indices.

The positive legacy list/system-index flow passes all 27 integration tests, direct concrete tenant-index stats/health requests remain denied, and the ML caller tests verify that explicit-index requests receive neither the marker nor local proof.

The tactical fix in this PR can remain narrowly scoped while those coordinated changes mature.

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

Labels

backport 3.8 Backports to 3.8

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants