From 9d8b80b82d04ecf8f592026512df736c50e45152 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Fri, 30 Oct 2026 23:23:34 +0000 Subject: [PATCH 1/3] Register the doc-level query and job lock indices as alerting system indices Alerting creates and owns .opensearch-alerting-queries* (the default doc-level query alias and its -000001 and rollover indices) and .opensearch-alerting-config-lock (the job lock index), but did not register them, unlike its alert, findings, comments and config indices. Add both to getSystemIndexDescriptors() from the existing constants. The query pattern does not cover query indices that a caller names in a monitor's data sources. Neither pattern overlaps another alerting pattern or one registered by the other plugins in the distribution. Add a unit test for the patterns and a secured IT for direct access, and move the ITs that wrote the lock index or created and closed the query index directly to the admin certificate client. Signed-off-by: Darshit Chanpura --- .../org/opensearch/alerting/AlertingPlugin.kt | 4 +- .../AlertingPluginSystemIndicesTests.kt | 52 ++++++++ .../alerting/DocumentMonitorRunnerIT.kt | 10 +- .../resthandler/SecureMonitorRestApiIT.kt | 37 +++--- .../SecureQueryAndLockIndexRestApiIT.kt | 123 ++++++++++++++++++ 5 files changed, 202 insertions(+), 24 deletions(-) create mode 100644 alerting/src/test/kotlin/org/opensearch/alerting/AlertingPluginSystemIndicesTests.kt create mode 100644 alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt b/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt index f90b769e2..ec7156a15 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt @@ -570,7 +570,9 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R SystemIndexDescriptor(ALL_ALERT_INDEX_PATTERN, "Alerting Plugin system index pattern"), SystemIndexDescriptor(ALL_FINDING_INDEX_PATTERN, "Alerting Findings system index pattern"), SystemIndexDescriptor(SCHEDULED_JOBS_INDEX, "Alerting Plugin Configuration index"), - SystemIndexDescriptor(ALL_COMMENTS_INDEX_PATTERN, "Alerting Comments system index pattern") + SystemIndexDescriptor(ALL_COMMENTS_INDEX_PATTERN, "Alerting Comments system index pattern"), + SystemIndexDescriptor("${ScheduledJob.DOC_LEVEL_QUERIES_INDEX}*", "Alerting doc-level query index pattern"), + SystemIndexDescriptor(LockService.LOCK_INDEX_NAME, "Alerting job lock index") ) } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingPluginSystemIndicesTests.kt b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingPluginSystemIndicesTests.kt new file mode 100644 index 000000000..2b1429e09 --- /dev/null +++ b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingPluginSystemIndicesTests.kt @@ -0,0 +1,52 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.alerting + +import org.apache.lucene.util.automaton.Operations +import org.opensearch.alerting.core.lock.LockService +import org.opensearch.common.regex.Regex +import org.opensearch.common.settings.Settings +import org.opensearch.commons.alerting.model.ScheduledJob +import org.opensearch.test.OpenSearchTestCase + +class AlertingPluginSystemIndicesTests : OpenSearchTestCase() { + + private val descriptors = AlertingPlugin().getSystemIndexDescriptors(Settings.EMPTY) + + fun `test doc-level query index and its rollovers are system indices`() { + val alias = ScheduledJob.DOC_LEVEL_QUERIES_INDEX + assertEquals(".opensearch-alerting-queries", alias) + // the alias name was a concrete index before the query index moved behind an alias + listOf(alias, "$alias-000001", "$alias-000002").forEach { assertSingleDescriptorFor(it) } + } + + fun `test job lock index is a system index`() { + assertEquals(".opensearch-alerting-config-lock", LockService.LOCK_INDEX_NAME) + assertSingleDescriptorFor(LockService.LOCK_INDEX_NAME) + } + + fun `test query indices that a caller names in data sources are not covered`() { + assertTrue(descriptors.none { it.matchesIndexPattern(".opensearch-sap-windows-detectors-queries-000001") }) + } + + // Core refuses to start a node whose system index patterns overlap, so this mirrors its check. + fun `test descriptors do not overlap`() { + descriptors.forEach { a -> + descriptors.filter { it !== a }.forEach { b -> + val intersection = Operations.intersection( + Regex.simpleMatchToAutomaton(a.indexPattern), + Regex.simpleMatchToAutomaton(b.indexPattern) + ) + assertTrue("[${a.indexPattern}] overlaps [${b.indexPattern}]", Operations.isEmpty(intersection)) + } + } + } + + private fun assertSingleDescriptorFor(index: String) { + val matching = descriptors.filter { it.matchesIndexPattern(index) } + assertEquals("descriptors matching [$index]: ${matching.map { it.indexPattern }}", 1, matching.size) + } +} diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt index 22f7b8876..0194a342d 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt @@ -641,18 +641,18 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() { indexDoc(testIndex, "1", testDoc) indexDoc(testIndex, "5", testDoc) OpenSearchTestCase.waitUntil({ - val response = client().makeRequest("HEAD", LockService.LOCK_INDEX_NAME) + val response = adminClient().makeRequest("HEAD", LockService.LOCK_INDEX_NAME) return@waitUntil (response.restStatus().status == 200) }, 240, TimeUnit.SECONDS) - var response = client().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search") + var response = adminClient().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search") var responseMap = entityAsMap(response) var noOfLocks = ((responseMap["hits"] as Map)["hits"] as List).size assertEquals(1, noOfLocks) deleteMonitor(monitor) refreshIndex(LockService.LOCK_INDEX_NAME) - response = client().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search") + response = adminClient().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search") responseMap = entityAsMap(response) noOfLocks = ((responseMap["hits"] as Map)["hits"] as List).size assertEquals(0, noOfLocks) @@ -3070,7 +3070,7 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() { val currTimeStampMinusTenMinutes = System.currentTimeMillis() - 600000L val lock = "{\"scheduled_job_id\":\"${monitor.id}\",\"lock_time\":${currTimeStampMinusTenMinutes / 1000},\"released\":false}" - updateDoc(client(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock) + updateDoc(adminClient(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock) updateMonitor(monitor.copy(enabled = true), true) @@ -3129,7 +3129,7 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() { val currTimeStamp = System.currentTimeMillis() val lock = "{\"scheduled_job_id\":\"${monitor.id}\",\"lock_time\":$currTimeStamp,\"released\":false}" - updateDoc(client(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock) + updateDoc(adminClient(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock) indexDoc(testIndex, "4", testDoc) indexDoc(testIndex, "5", testDoc) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt index 3394cb079..daa5d603d 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -48,7 +48,6 @@ import org.opensearch.alerting.randomTemplateScript import org.opensearch.client.Response import org.opensearch.client.ResponseException import org.opensearch.client.RestClient -import org.opensearch.common.settings.Settings import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.XContentType import org.opensearch.common.xcontent.json.JsonXContent @@ -2670,25 +2669,27 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { false ) val docLevelQueryIndex = ".opensearch-alerting-queries-000001" - createIndex( - docLevelQueryIndex, Settings.EMPTY, - """ - "properties" : { - "query": { - "type": "percolator_ext" - }, - "monitor_id": { - "type": "text" - }, - "index": { - "type": "text" - } - } + // the query index is a system index, so only the admin certificate can create and close it directly + adminClient().makeRequest( + "PUT", docLevelQueryIndex, + StringEntity( + """ + { + "mappings": { + "properties": { + "query": { "type": "percolator_ext" }, + "monitor_id": { "type": "text" }, + "index": { "type": "text" } + } + }, + "aliases": { ".opensearch-alerting-queries": {} } } - """.trimIndent(), - ".opensearch-alerting-queries" + """.trimIndent(), + ContentType.APPLICATION_JSON + ) ) - closeIndex(docLevelQueryIndex) // close index to simulate doc level query indexing failure + // close index to simulate doc level query indexing failure + adminClient().makeRequest("POST", "$docLevelQueryIndex/_close") try { val monitor = randomDocumentLevelMonitor( withMetadata = false, diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt new file mode 100644 index 000000000..305a6aefc --- /dev/null +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt @@ -0,0 +1,123 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.alerting.resthandler + +import org.apache.hc.core5.http.ContentType.APPLICATION_JSON +import org.apache.hc.core5.http.io.entity.StringEntity +import org.junit.After +import org.junit.Before +import org.junit.BeforeClass +import org.opensearch.alerting.AlertingRestTestCase +import org.opensearch.alerting.core.lock.LockService.Companion.LOCK_INDEX_NAME +import org.opensearch.alerting.makeRequest +import org.opensearch.alerting.randomDocumentLevelMonitor +import org.opensearch.client.ResponseException +import org.opensearch.client.RestClient +import org.opensearch.common.xcontent.XContentType +import org.opensearch.commons.alerting.model.DocLevelMonitorInput +import org.opensearch.commons.alerting.model.DocLevelQuery +import org.opensearch.commons.alerting.model.ScheduledJob.Companion.DOC_LEVEL_QUERIES_INDEX +import org.opensearch.commons.rest.SecureRestClientBuilder +import org.opensearch.core.rest.RestStatus +import org.opensearch.index.query.QueryBuilders +import org.opensearch.search.builder.SearchSourceBuilder + +/** + * The doc-level query indices and the job lock index are system indices, so a user can't read or write them directly, even + * with index permissions on every index. + */ +class SecureQueryAndLockIndexRestApiIT : AlertingRestTestCase() { + + companion object { + @BeforeClass + @JvmStatic fun setup() { + org.junit.Assume.assumeTrue(System.getProperty("security", "false")!!.toBoolean()) + } + } + + private val user = "queryLockUser" + private val role = "query_lock_all_index_role" + private var userClient: RestClient? = null + + @Before + fun create() { + createUser(user, arrayOf()) + // crud on every index plus the bulk permission a document write needs, so only the system index protection keeps the + // user out of these indices + createCustomIndexRole(role, "*", "indices:data/write/bulk") + createUserRolesMapping(role, arrayOf(user)) + userClient = SecureRestClientBuilder(clusterHosts.toTypedArray(), isHttps(), user, password) + .setSocketTimeout(60000) + .build() + } + + @After + fun cleanup() { + userClient?.close() + deleteRoleAndRoleMapping(role) + deleteUser(user) + } + + fun `test user cannot directly search doc-level query system index`() { + createDocLevelMonitorWithQuery() + assertSearchHits(adminClient(), "$DOC_LEVEL_QUERIES_INDEX*", expectAny = true) + + // The security plugin leaves the system index out of the user's search, so it succeeds with no hits rather than failing + assertSearchHits(userClient!!, "$DOC_LEVEL_QUERIES_INDEX*", expectAny = false) + } + + fun `test user cannot directly write to doc-level query system index`() { + createDocLevelMonitorWithQuery() + + assertWriteForbidden(DOC_LEVEL_QUERIES_INDEX) + } + + fun `test user cannot directly search job lock system index`() { + createLock() + assertSearchHits(adminClient(), LOCK_INDEX_NAME, expectAny = true) + + assertSearchHits(userClient!!, LOCK_INDEX_NAME, expectAny = false) + } + + fun `test user cannot directly write to job lock system index`() { + createLock() + + assertWriteForbidden(LOCK_INDEX_NAME) + } + + private fun createDocLevelMonitorWithQuery() { + val testIndex = createTestIndex() + val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3", fields = listOf()) + createMonitor( + randomDocumentLevelMonitor( + inputs = listOf(DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery))), + triggers = listOf(), + enabled = false + ) + ) + } + + private fun createLock() { + indexDocWithAdminClient(LOCK_INDEX_NAME, "test-lock", """{"scheduled_job_id": "test", "lock_time": 1, "released": true}""") + } + + 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 + assertEquals("Docs found in [$index]: $numberDocsFound", expectAny, numberDocsFound > 0) + } + + private fun assertWriteForbidden(index: String) { + val e = expectThrows(ResponseException::class.java) { + userClient!!.makeRequest("POST", "$index/_doc", StringEntity("""{"monitor_id": "fake"}""", APPLICATION_JSON)) + } + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) + } +} From c52af12ff52cf85639e6bbde2aaf768a9a8f7049 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Fri, 30 Oct 2026 23:30:00 +0000 Subject: [PATCH 2/3] Add a write control to the query and lock index IT The direct write tests expected a 403 but did not show that the same user can write elsewhere, so a missing role permission would also pass. Write the same document to an ordinary index first and expect 201. Also state precisely which direct operation on the query index the security plugin refuses in the create monitor clean up test. Signed-off-by: Darshit Chanpura --- .../alerting/resthandler/SecureMonitorRestApiIT.kt | 2 +- .../resthandler/SecureQueryAndLockIndexRestApiIT.kt | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt index daa5d603d..a5921aefe 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -2669,7 +2669,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { false ) val docLevelQueryIndex = ".opensearch-alerting-queries-000001" - // the query index is a system index, so only the admin certificate can create and close it directly + // the query index is a system index, so create and close it with the admin certificate; closing it as a user is refused adminClient().makeRequest( "PUT", docLevelQueryIndex, StringEntity( diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt index 305a6aefc..be889289e 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureQueryAndLockIndexRestApiIT.kt @@ -115,8 +115,13 @@ class SecureQueryAndLockIndexRestApiIT : AlertingRestTestCase() { } private fun assertWriteForbidden(index: String) { + // control: the same user can write the same document to an ordinary index, so the 403 below comes from the system index + val doc = """{"monitor_id": "fake"}""" + val control = userClient!!.makeRequest("POST", "${createTestIndex()}/_doc", StringEntity(doc, APPLICATION_JSON)) + assertEquals("Unexpected status", RestStatus.CREATED, control.restStatus()) + val e = expectThrows(ResponseException::class.java) { - userClient!!.makeRequest("POST", "$index/_doc", StringEntity("""{"monitor_id": "fake"}""", APPLICATION_JSON)) + userClient!!.makeRequest("POST", "$index/_doc", StringEntity(doc, APPLICATION_JSON)) } assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) } From 234158e19bb1458be684e27b6fad3f203fb6703b Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Mon, 2 Nov 2026 18:59:15 +0000 Subject: [PATCH 3/3] Create the clean up test's query index without the alias, as on main The create monitor clean up test in SecureMonitorRestApiIT failed on the secured CI run because monitor creation no longer failed. On main, the createIndex call created .opensearch-alerting-queries-000001 without the .opensearch-alerting-queries alias: its mapping string closed the request body before the aliases part, which the server ignored. With no write index behind the alias, updateQueryIndexMappings fails, so monitor creation fails and the test checks the clean up. The admin certificate body added in this PR also created the alias. A closed index behind the alias does not fail monitor creation: the mapping update succeeds on a closed index, and the test monitor has no queries to index. The test fails the same way with or without the new system index descriptors, so the descriptors do not change this behaviour. Create the same index, mappings and settings as main did, without the alias, still with the admin certificate. Signed-off-by: Darshit Chanpura --- .../alerting/resthandler/SecureMonitorRestApiIT.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt index a5921aefe..72c85fed2 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -2669,7 +2669,9 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { false ) val docLevelQueryIndex = ".opensearch-alerting-queries-000001" - // the query index is a system index, so create and close it with the admin certificate; closing it as a user is refused + // The query index is a system index, so only the admin certificate can create and close it. + // Create the backing index without the .opensearch-alerting-queries alias: with no write index behind the alias, + // indexing the doc level queries fails and monitor creation must clean up the monitor. adminClient().makeRequest( "PUT", docLevelQueryIndex, StringEntity( @@ -2681,14 +2683,12 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { "monitor_id": { "type": "text" }, "index": { "type": "text" } } - }, - "aliases": { ".opensearch-alerting-queries": {} } + } } """.trimIndent(), ContentType.APPLICATION_JSON ) ) - // close index to simulate doc level query indexing failure adminClient().makeRequest("POST", "$docLevelQueryIndex/_close") try { val monitor = randomDocumentLevelMonitor(