Create parent-linked sharing entries for child resources written without a user

[DarshitChanpura]

Description

ResourceIndexListener.postIndex requires an authenticated user in the thread context to create a resource-sharing entry. When none is present, it fails with an uncaught NPE (null user subject) or skips silently at debug level. Resources written in a genuinely user-less context — scheduled jobs running under job-scheduler (e.g. scheduled report instances), provisioning steps executed under system context — therefore never receive sharing entries and stay permanently invisible to the resource-sharing APIs, including to the owner of the parent resource that triggered them.

Changes

• Null-safe user-subject extraction
• No user + provider declares a parent (parentType/parentIdField): the sharing entry is created by inheriting tenant/created_by from the parent's sharing record, linked via parentType/parentId so ResourceAccessHandler delegates evaluation to the parent
• No user + no parent: skip is logged at WARN (previously silent), making the failure mode visible to operators
• Entry-indexing failures now log at WARN instead of debug

Testing

New integration tests (SystemContextChildResourceTests, sample plugin hierarchy, all passing) index resource documents through the internal node client — no user in context, mirroring plugin/system-subject writes:

• child resources receive a parent-linked entry inheriting the parent owner; sharing the parent grants access to the system-created child
• parent-less resources are skipped without an entry
• children referencing a missing parent record are skipped

Investigation note (correction)

This PR was initially motivated by on-demand report instances missing sharing records on a 3.8.0 snapshot. Deeper investigation showed that case was actually caused by listener-attachment timing on that build: the listener attaches per-index in onIndexModule filtered by protected_types, so a type added to the dynamic setting after its index was already open never got a listener until restart (current main attaches based on the unfiltered registered set, so main appears immune). On-demand instance writes do carry the authenticated user and work once the listener is attached. The user-less gap addressed by this PR remains real for scheduled/system-context writes, as covered by the new tests.

Companion PRs

• Declare report-instance as child resource of report-definition reporting#1213 (report-instance → report-definition)
• Declare workflow_state as child resource of workflow flow-framework#1455 (workflow_state → workflow)

Category

Bug fix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[DarshitChanpura]

Companion reporting PR: opensearch-project/reporting#

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 9d27c4f)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Tenant Mismatch Risk When a child resource is created without an authenticated user, the sharing entry inherits tenant and createdBy from the parent's sharing record. If the parent was created under a specific tenant but the child write occurs in a system/plugin context that should logically belong to a different tenant (or none), the child will silently be attributed to the parent's tenant. This may be the intended design, but it should be verified — especially when multi-tenancy is enabled — as it can lead to cross-tenant visibility if a parent's tenant differs from the effective context of the child write. ResourceSharing sharingInfo = ResourceSharing.builder() .resourceId(resourceId) .resourceType(resourceType) .tenant(parentSharing.getTenant()) .createdBy(parentSharing.getCreatedBy()) .parentType(parentType) .parentId(parentId) .build(); this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, sharingInfo, listener);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 9d27c4f

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Guard against null parent id when building When user != null and parentType != null, the code sets parentId without validating it is non-null, which can lead to an inconsistent sharing entry if the parent id field is missing from the source. Guard the parent-attribution branch to only set parent fields when parentId is also non-null, consistent with the user-less path. src/main/java/org/opensearch/security/resources/ResourceIndexListener.java [124-140] if (user != null) { try { // User.getRequestedTenant() is null if multi-tenancy is disabled ResourceSharing.Builder builder = ResourceSharing.builder() .resourceId(resourceId) .resourceType(resourceType) .tenant(user.getRequestedTenant()) .createdBy(new CreatedBy(user.getName())); - if (parentType != null) { + if (parentType != null && parentId != null) { builder.parentType(parentType).parentId(parentId); } this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, builder.build(), listener); } catch (IOException e) { log.warn("Failed to create a resource sharing entry for resource: {}", resourceId, e); } return; } Suggestion importance[1-10]: 6 __ Why: Valid consistency improvement: the user-less path skips creation when parentId is null, but the user path would set parentType with a null parentId, potentially causing an inconsistent sharing entry. Moderate impact since it's an edge case.	Low

---

Previous suggestions

Suggestions up to commit 8dfd22f

Category	Suggestion	Impact
General	Validate missing parent id for child resource When a user is present but parentType is declared and parentId is null (e.g., child resource missing the parent id field), the code silently creates a sharing entry with parentType set but parentId null. Guard against this case to avoid inconsistent sharing records, matching the validation applied in the user-less branch. src/main/java/org/opensearch/security/resources/ResourceIndexListener.java [124-140] if (user != null) { try { // User.getRequestedTenant() is null if multi-tenancy is disabled ResourceSharing.Builder builder = ResourceSharing.builder() .resourceId(resourceId) .resourceType(resourceType) .tenant(user.getRequestedTenant()) .createdBy(new CreatedBy(user.getName())); if (parentType != null) { + if (parentId == null) { + log.warn("Skipping resource-sharing entry for child resource {}: parent id field {} missing.", resourceId, provider.parentIdField()); + return; + } builder.parentType(parentType).parentId(parentId); } this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, builder.build(), listener); } catch (IOException e) { log.warn("Failed to create a resource sharing entry for resource: {}", resourceId, e); } return; } Suggestion importance[1-10]: 5 __ Why: The suggestion addresses a real edge case where a child resource with an authenticated user could produce a sharing entry with parentType set but a null parentId, resulting in an inconsistent record. It's a reasonable consistency improvement, though the practical impact depends on how likely parentId is null in this branch.	Low

Suggestions up to commit a34fb50

Category	Suggestion	Impact
Possible issue	Guard against null parentId when building entry When a user is authenticated and the resource declares a parent, parentId may still be null if the parent field is missing from the document. Persisting a sharing entry with parentType set but parentId null yields an inconsistent record that cannot be resolved by parent-based access checks. Guard against null parentId similarly to the user-less branch. src/main/java/org/opensearch/security/resources/ResourceIndexListener.java [124-140] if (user != null) { try { // User.getRequestedTenant() is null if multi-tenancy is disabled ResourceSharing.Builder builder = ResourceSharing.builder() .resourceId(resourceId) .resourceType(resourceType) .tenant(user.getRequestedTenant()) .createdBy(new CreatedBy(user.getName())); - if (parentType != null) { + if (parentType != null && parentId != null) { builder.parentType(parentType).parentId(parentId); } this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, builder.build(), listener); } catch (IOException e) { log.warn("Failed to create a resource sharing entry for resource: {}", resourceId, e); } return; } Suggestion importance[1-10]: 6 __ Why: Valid concern: if parentType is set but parentId is null (missing parent field in the doc), the sharing entry would have inconsistent parent metadata. Adding a null check improves data consistency, though the actual runtime impact depends on whether such states are reachable in practice.	Low

[codecov]

Codecov Report

❌ Patch coverage is 76.92308% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.43%. Comparing base (f9e24a7) to head (9d27c4f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...arch/security/resources/ResourceIndexListener.java	76.92%	7 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6373      +/-   ##
==========================================
+ Coverage   75.40%   75.43%   +0.03%     
==========================================
  Files         456      456              
  Lines       30255    30280      +25     
  Branches     4575     4580       +5     
==========================================
+ Hits        22815    22843      +28     
+ Misses       5304     5302       -2     
+ Partials     2136     2135       -1     

Files with missing lines	Coverage Δ
...arch/security/resources/ResourceIndexListener.java	89.53% <76.92%> (-7.24%)	⬇️

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

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 9d27c4f.

⛔ Hard block: Issues at High severity or above will block this PR from merging.

Path	Line	Severity	Description
src/main/java/org/opensearch/security/resources/ResourceIndexListener.java	160	medium	When no authenticated user exists in thread context, the code inherits the parent resource's sharing entry (including createdBy and tenant) for the child resource. The parentId value comes directly from the indexed document's field (provider.parentIdField()), meaning any code path able to write to the resource index without an authenticated user could craft a document pointing to a high-privilege parent and inherit that parent's ownership/sharing permissions. The design is intentional and well-documented for plugin/system writes, but the trust placed in the parentId field from document content—without additional authorization checks—creates a potential privilege-inheritance vector if write access to the monitored index is insufficiently restricted.

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.

[github-actions]

Persistent review updated to latest commit 8dfd22f

[github-actions]

Persistent review updated to latest commit 9d27c4f