Register detector and correlation-rule indices as system indices

[DarshitChanpura]

Description

Onboards .opensearch-sap-detectors-config and .opensearch-sap-correlation-rules-config as OpenSearch system indices.

This is a prerequisite for onboarding security-analytics to the centralized resource-sharing framework (#1735). The resource-sharing framework requires resource indices to be declared as system indices so the security plugin can apply document-level access control and grant the plugin's own subject access.

Changes

Production:

• Add both indices to SecurityAnalyticsPlugin.getSystemIndexDescriptors
• Route the internal access sites that were not already stashing the thread context through the system context (via the existing StashedThreadContext.run() helper):
  • TransportIndexCorrelationRuleAction — stash at start of async flow (create-index, mapping-update, write)
  • TransportDeleteCorrelationRuleAction — delete-by-query
  • TransportDeleteRuleAction — detectors index search
  • DetectorUtils.getAllDetectorInputs — detectors index search
• Detector CRUD transport actions already stashed context and are unaffected.

Testing / known follow-ups

Registering these as system indices changes how security-enabled integration tests must read the indices — tests that search the detectors index directly via the REST client need to be updated to a system-index-capable read path. Iterating on those test updates in this PR.

Check List

• Commits are signed per the DCO using --signoff.

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

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 8b15a78.

Path	Line	Severity	Description
src/main/java/org/opensearch/securityanalytics/transport/TransportIndexCorrelationRuleAction.java	83	low	stashContext() is called directly without using the StashedThreadContext.run() wrapper used consistently elsewhere in this diff. The StoredContext returned by stashContext() must be closed to restore the original security context; if the try block does not close it in a finally clause, the thread's security context may remain elevated beyond the intended scope. This is likely a coding inconsistency rather than malicious intent, but it is worth verifying that the corresponding close/restore occurs in the visible try block.

The table above displays the top 10 most important findings.

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

---

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]

PR Reviewer Guide 🔍

(Review updated until commit 8b15a78)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Register correlation-rule index as a system index and stash thread context for its access Relevant files: src/main/java/org/opensearch/securityanalytics/SecurityAnalyticsPlugin.java src/main/java/org/opensearch/securityanalytics/transport/TransportIndexCorrelationRuleAction.java src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteCorrelationRuleAction.java Sub-PR theme: Register detectors index as a system index and adjust callers/tests Relevant files: src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteRuleAction.java src/main/java/org/opensearch/securityanalytics/util/DetectorUtils.java src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java

⚡ Recommended focus areas for review Thread context not restored start() calls client.threadPool().getThreadContext().stashContext() directly without using try-with-resources on the returned StoredContext or the StashedThreadContext.run() helper used elsewhere in this PR. The original caller's thread context (including headers such as user identity, authentication, and transient values) is discarded and never restored when the async operation completes. This can leak the system context into subsequent operations on the same thread and break downstream authorization/auditing. Prefer wrapping the async flow with StashedThreadContext.run(client, () -> ...) or storing the returned StoredContext and restoring it, consistent with the other changes in this PR. void start() { // Correlation rule index is a system index; stash the thread context so the // plugin can create/update/write to it when the security plugin is enabled. client.threadPool().getThreadContext().stashContext(); try {

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 8b15a78
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Restore thread context after stashing Calling stashContext() directly leaks a stashed context because the returned StoredContext is never closed/restored, which can pollute subsequent operations on the same thread. Use StashedThreadContext.run(client, () -> ...) (as done in the other transport actions in this PR) or wrap the stash in a try-with-resources to properly restore the original context. src/main/java/org/opensearch/securityanalytics/transport/TransportIndexCorrelationRuleAction.java [83-85] void start() { // Correlation rule index is a system index; stash the thread context so the // plugin can create/update/write to it when the security plugin is enabled. - client.threadPool().getThreadContext().stashContext(); + StashedThreadContext.run(client, () -> { try { Suggestion importance[1-10]: 8 __ Why: Directly calling stashContext() without closing/restoring the returned StoredContext can leak the stashed context and pollute subsequent operations on the same thread. Using StashedThreadContext.run(...) (as done elsewhere in the PR) is the correct pattern.	Medium
	Stash context for async follow-up calls The async client.search callback (onResponse) subsequently calls deleteRule(...) and other client operations that also write to the system detectors index, but those callbacks execute after the stashed context has already been restored. Ensure the follow-up operations inside the listener also run within a StashedThreadContext.run(...) block so they retain system-index access. src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteRuleAction.java [152-154] // Detectors index is a system index; stash the thread context so the plugin // can search it when the security plugin is enabled. StashedThreadContext.run(client, () -> client.search(searchRequest, new ActionListener<>() { + // ensure downstream client calls in onResponse also use StashedThreadContext.run(...) Suggestion importance[1-10]: 6 __ Why: Valid concern that follow-up operations in the async onResponse callback may execute outside the stashed context, potentially losing system-index access. However, the suggestion is somewhat vague and the improved_code only adds a comment rather than a concrete fix.	Low

---

Previous suggestions

Suggestions up to commit 80a5bf1

Category	Suggestion	Impact
Possible issue	Restore stashed thread context properly Calling stashContext() without using try-with-resources leaks the stashed context, permanently discarding the original thread context (including user/auth headers) for subsequent work on this thread. Use StashedThreadContext.run(...) (as done elsewhere in this PR) or wrap in a try-with-resources on the StoredContext to restore it after the operation completes. src/main/java/org/opensearch/securityanalytics/transport/TransportIndexCorrelationRuleAction.java [85] void start() { // Correlation rule index is a system index; stash the thread context so the // plugin can create/update/write to it when the security plugin is enabled. - client.threadPool().getThreadContext().stashContext(); + StashedThreadContext.run(client, () -> { try { Suggestion importance[1-10]: 8 __ Why: Correctly identifies that calling stashContext() without try-with-resources or using StashedThreadContext.run(...) leaks the stashed context, which can cause subsequent operations on the thread to lose auth headers. This is a legitimate concern, though the proposed improved_code is incomplete (opens a lambda without closing it).	Medium
	Use ids query for exact id match Using matchQuery on _id performs full-text analysis and may not reliably match the exact document id; this can cause the delete-by-query to match zero or wrong documents. Use QueryBuilders.idsQuery().addIds(correlationRuleId) (or termQuery) to filter by the exact id. src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteCorrelationRuleAction.java [66-71] StashedThreadContext.run(client, () -> new DeleteByQueryRequestBuilder(client, DeleteByQueryAction.INSTANCE) .source(CorrelationRule.CORRELATION_RULE_INDEX) - .filter(QueryBuilders.matchQuery("_id", correlationRuleId)) + .filter(QueryBuilders.idsQuery().addIds(correlationRuleId)) .refresh(true) .execute(new ActionListener<>() { Suggestion importance[1-10]: 6 __ Why: Using matchQuery on _id is not ideal; idsQuery or termQuery is more appropriate for exact id matching. However, this code was pre-existing in the PR (only the wrapping was changed), so the impact is moderate.	Low

[github-actions]

Persistent review updated to latest commit 8b15a78