Add validation check for doc level query name during monitor creation (…

…opensearch-project#1506) (opensearch-project#1511)

* add validation check for doc level query name during monitor creation



* change to 0-256 chars



* change validation message and move validation loc



---------


(cherry picked from commit 636b43f)

Signed-off-by: Joanne Wang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt

@@ -16,10 +16,13 @@ import org.opensearch.commons.alerting.action.AlertingActions
 import org.opensearch.commons.alerting.action.IndexMonitorRequest
 import org.opensearch.commons.alerting.action.IndexMonitorResponse
 import org.opensearch.commons.alerting.model.BucketLevelTrigger
+import org.opensearch.commons.alerting.model.DocLevelMonitorInput
 import org.opensearch.commons.alerting.model.DocumentLevelTrigger
 import org.opensearch.commons.alerting.model.Monitor
 import org.opensearch.commons.alerting.model.QueryLevelTrigger
 import org.opensearch.commons.alerting.model.ScheduledJob
+import org.opensearch.commons.utils.getInvalidNameChars
+import org.opensearch.commons.utils.isValidName
 import org.opensearch.core.rest.RestStatus
 import org.opensearch.core.xcontent.ToXContent
 import org.opensearch.core.xcontent.XContentParser.Token
@@ -118,8 +121,10 @@ class RestIndexMonitorAction : BaseRestHandler() {
                         throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor")
                     }
                 }
+                validateDocLevelQueryName(monitor)
             }
         }
+
         val seqNo = request.paramAsLong(IF_SEQ_NO, SequenceNumbers.UNASSIGNED_SEQ_NO)
         val primaryTerm = request.paramAsLong(IF_PRIMARY_TERM, SequenceNumbers.UNASSIGNED_PRIMARY_TERM)
         val refreshPolicy = if (request.hasParam(REFRESH)) {
@@ -134,6 +139,19 @@ class RestIndexMonitorAction : BaseRestHandler() {
         }
     }
 
+    private fun validateDocLevelQueryName(monitor: Monitor) {
+        monitor.inputs.filterIsInstance<DocLevelMonitorInput>().forEach { docLevelMonitorInput ->
+            docLevelMonitorInput.queries.forEach { dlq ->
+                if (!isValidName(dlq.name)) {
+                    throw IllegalArgumentException(
+                        "Doc level query name may not start with [_, +, -], contain '..', or contain: " +
+                            getInvalidNameChars().replace("\\", "")
+                    )
+                }
+            }
+        }
+    }
+
     private fun validateDataSources(monitor: Monitor) { // Data Sources will currently be supported only at transport layer.
         if (monitor.dataSources != null) {
             if (

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt

@@ -26,6 +26,8 @@ import org.opensearch.alerting.randomAnomalyDetector
 import org.opensearch.alerting.randomAnomalyDetectorWithUser
 import org.opensearch.alerting.randomBucketLevelMonitor
 import org.opensearch.alerting.randomBucketLevelTrigger
+import org.opensearch.alerting.randomDocLevelMonitorInput
+import org.opensearch.alerting.randomDocLevelQuery
 import org.opensearch.alerting.randomDocumentLevelMonitor
 import org.opensearch.alerting.randomDocumentLevelTrigger
 import org.opensearch.alerting.randomQueryLevelMonitor
@@ -48,6 +50,7 @@ import org.opensearch.commons.alerting.model.Monitor
 import org.opensearch.commons.alerting.model.QueryLevelTrigger
 import org.opensearch.commons.alerting.model.ScheduledJob
 import org.opensearch.commons.alerting.model.SearchInput
+import org.opensearch.commons.utils.getInvalidNameChars
 import org.opensearch.core.common.bytes.BytesReference
 import org.opensearch.core.rest.RestStatus
 import org.opensearch.core.xcontent.ToXContent
@@ -1266,6 +1269,50 @@ class MonitorRestApiIT : AlertingRestTestCase() {
         }
     }
 
+    fun `test creating and updating a document monitor with invalid query name`() {
+        // creating a monitor with an invalid query name
+        val invalidQueryName = "_Invalid .. query ! n>ame"
+        val queries = listOf(randomDocLevelQuery(name = invalidQueryName))
+        val randomDocLevelMonitorInput = randomDocLevelMonitorInput(queries = queries)
+        val inputs = listOf(randomDocLevelMonitorInput)
+        val trigger = randomDocumentLevelTrigger()
+        var monitor = randomDocumentLevelMonitor(inputs = inputs, triggers = listOf(trigger))
+
+        try {
+            client().makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity())
+            fail("Doc level monitor with invalid query name should be rejected")
+        } catch (e: ResponseException) {
+            assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus())
+            val expectedMessage = "Doc level query name may not start with [_, +, -], contain '..', or contain: " +
+                getInvalidNameChars().replace("\\", "")
+            e.message?.let { assertTrue(it.contains(expectedMessage)) }
+        }
+
+        // successfully creating monitor with valid query name
+        val testIndex = createTestIndex()
+        val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "valid (name)", fields = listOf())
+        val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery))
+
+        monitor = createMonitor(randomDocumentLevelMonitor(inputs = listOf(docLevelInput), triggers = listOf(trigger)))
+
+        // updating monitor with invalid query name
+        val updatedDocQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = invalidQueryName, fields = listOf())
+        val updatedDocLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(updatedDocQuery))
+
+        try {
+            client().makeRequest(
+                "PUT", monitor.relativeUrl(),
+                emptyMap(), monitor.copy(inputs = listOf(updatedDocLevelInput)).toHttpEntity()
+            )
+            fail("Doc level monitor with invalid query name should be rejected")
+        } catch (e: ResponseException) {
+            assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus())
+            val expectedMessage = "Doc level query name may not start with [_, +, -], contain '..', or contain: " +
+                getInvalidNameChars().replace("\\", "")
+            e.message?.let { assertTrue(it.contains(expectedMessage)) }
+        }
+    }
+
     /**
      * This use case is needed by the frontend plugin for displaying alert counts on the Monitors list page.
      * https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/server/services/MonitorService.js#L235