[DarshitChanpura]

Description

Problem: Alerting registers its alert, findings, comments and config indices as system indices, but not two other indices it creates and owns:

• .opensearch-alerting-queries*, the default doc-level query index. .opensearch-alerting-queries is an alias over .opensearch-alerting-queries-000001 and its rollovers (DocLevelMonitorQueries).
• .opensearch-alerting-config-lock, the job lock index (LockService.LOCK_INDEX_NAME).

So with the security plugin on, index permissions on these names allow direct reads and writes of monitor queries and job locks, which the other alerting indices do not allow.

Change: Add both to getSystemIndexDescriptors(), built from the existing constants:

SystemIndexDescriptor("${ScheduledJob.DOC_LEVEL_QUERIES_INDEX}*", "Alerting doc-level query index pattern"),
SystemIndexDescriptor(LockService.LOCK_INDEX_NAME, "Alerting job lock index")

The query pattern covers the alias name, -000001 and every rollover. It does not cover query indices that a transport caller names in a monitor's data_sources (security-analytics uses .opensearch-sap-<log type>-detectors-queries); those stay ordinary indices, as discussed in the issue.

Overlap: core refuses to start a node when two system index patterns overlap. Neither pattern overlaps alerting's other four, and neither overlaps the patterns registered by job-scheduler, security-analytics, anomaly-detection, index-management, notifications, ml-commons, security, flow-framework, observability, reporting, k-NN, sql, cross-cluster-replication, asynchronous-search, search-relevance, geospatial or learning-to-rank on their current main branches. This was checked by passing all of them to core's SystemIndices, which runs the same check as node start.

Upgrade: nothing is migrated. On restart the cluster manager marks existing indices that match the new patterns as system indices in their metadata; the data and mappings do not change. After that:

• Reads and writes on these indices run on the system_read and system_write thread pools, so the percolate search of doc-level monitors no longer runs on the search pool, and concurrent segment search is not used for it.
• With the security plugin enabled, users can no longer read or write these indices directly, even with all_access. Direct writes, deletes, mapping and alias changes and close need the admin certificate (or system:admin/system_index when system index permissions are enabled); direct searches return no hits. This includes a query-level monitor whose input matches these indices by a wildcard. Alerting's own access is unaffected, because it reaches these indices without a user attached, in the same flows that already write the registered alert, findings and config indices.
• Clusters without the security plugin are unaffected.

Open question, for the secured CI run: a few alerting calls on these two indices do not stash the context themselves, right after a step that restores the caller's context:

• delete monitor: the query cleanup and deleteLock in DeleteMonitorService, after TransportDeleteMonitorAction restores the caller's context in its coroutine;
• create and update monitor: indexDocLevelMonitorQueries and the query delete-by-query in TransportIndexMonitorAction, after the same restore. The config index writes beside them use the *Stashed helpers.

The coroutine usually resumes on a worker thread without a user, and secured runs on main where users without permissions on these names create and delete doc-level monitors point that way, but the code does not guarantee it. If the caller's user is present, these calls are now refused for all_access as well as for users without index permissions: a refused lock delete fails the delete request after the monitor document is gone, a refused query cleanup is only logged, and an update can leave the monitor and its queries out of step. This PR does not change those paths; running them with the plugin's own context is part of the planned move from stashContext() to the plugin client. The scheduled run, the lock acquire and release, and the percolate search run in the same context as the alert and findings writes that already work.

Release note: With the security plugin enabled, the default doc-level query indices (.opensearch-alerting-queries*) and the job lock index (.opensearch-alerting-config-lock) are now system indices, like the alert, findings and comments indices. Users can no longer read or write them directly. Clusters without the security plugin are unaffected.

Testing

• AlertingPluginSystemIndicesTests (unit): the alias name, -000001, -000002 and the lock index each match exactly one descriptor; a security-analytics query index name matches none; and no two alerting descriptors overlap, the same check core runs at node start. Removing either descriptor, dropping the * from the query pattern, adding an overlapping descriptor, or widening the query pattern each fails the matching test. Both index tests fail on main.
• SecureQueryAndLockIndexRestApiIT (runs with -Dsecurity=true): a user with read and write permissions on every index gets no hits from a direct search of each index, while the admin certificate finds the documents, and gets a 403 on a direct write, while the same user can write the same document to an ordinary index. It follows SecureFindingsRestApiIT.
• Existing ITs that wrote the lock index (DocumentMonitorRunnerIT, renewable and non-renewable lock tests) or created and closed the query index (SecureMonitorRestApiIT, create monitor clean up test) through the admin user now use the admin certificate, as createFinding does for the findings index. The old createIndex call in the clean up test never created the .opensearch-alerting-queries alias, because its mapping string closed the request body early; the new body creates it, so the test now fails monitor creation on the closed write index, as intended.
• Run locally: ./gradlew ktlint :alerting:test passes (476 tests, 4 new, 0 failures; 472 on main). The integration tests, secured and unsecured, have not been run locally and are left to CI.

Related Issues

Related: #2318

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

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

⚡ Recommended focus areas for review Possible Issue In the waitUntil block, val response = adminClient().makeRequest(...) is declared inside the lambda but then response is referenced outside the lambda (var responseMap = entityAsMap(response)). The outer reference now refers to a different response variable declared below with var response = adminClient().makeRequest("GET", ...). This ordering means the HEAD-based wait no longer shares a variable with the outer scope — verify the test still compiles and behaves correctly, since the previous code depended on response being accessible outside the lambda. OpenSearchTestCase.waitUntil({ val response = adminClient().makeRequest("HEAD", LockService.LOCK_INDEX_NAME) return@waitUntil (response.restStatus().status == 200) }, 240, TimeUnit.SECONDS) var response = adminClient().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search") var responseMap = entityAsMap(response) var noOfLocks = ((responseMap["hits"] as Map<String, Any>)["hits"] as List<Any>).size assertEquals(1, noOfLocks)

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid unsafe cast of hit count value The total field in a search response is typically returned as a long and may be deserialized as a Long rather than an Int, which will cause a ClassCastException at runtime when hit counts become non-trivial or depending on the parser. Cast to Number and call toInt() (or compare as Long) to be safe. alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt [113] private fun assertSearchHits(client: RestClient, index: String, expectAny: Boolean) { adminClient().makeRequest("POST", "/$index/_refresh?expand_wildcards=all") val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()).toString() val response = client.makeRequest("GET", "$index/_search", StringEntity(query, APPLICATION_JSON)) assertEquals("Unexpected status", RestStatus.OK, response.restStatus()) val hits = createParser(XContentType.JSON.xContent(), response.entity.content).map()["hits"] as Map<*, *> - val numberDocsFound = (hits["total"] as Map<*, *>)["value"] as Int + val numberDocsFound = ((hits["total"] as Map<*, *>)["value"] as Number).toInt() assertEquals("Docs found in [$index]: $numberDocsFound", expectAny, numberDocsFound > 0) } Suggestion importance[1-10]: 6 __ Why: The suggestion is valid: the total.value field from a search response is commonly deserialized as a Long, and casting directly to Int could cause a ClassCastException. Using Number.toInt() is safer, though in test contexts with small hit counts the issue may rarely manifest.	Low

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.83%. Comparing base (778e7ce) to head (c52af12).
⚠️ Report is 256 commits behind head on main.

❌ Your project check has failed because the head coverage (67.83%) is below the target coverage (70.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2320      +/-   ##
============================================
+ Coverage     67.72%   67.83%   +0.10%     
- Complexity      105     1815    +1710     
============================================
  Files           160      170      +10     
  Lines         10343    14324    +3981     
  Branches       1522     2090     +568     
============================================
+ Hits           7005     9716    +2711     
- Misses         2672     3726    +1054     
- Partials        666      882     +216     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.