Skip to content

Commit 135bae5

Browse files
committed
add validation check for doc level query name during monitor creation
Signed-off-by: Joanne Wang <[email protected]>
1 parent d2a590e commit 135bae5

File tree

2 files changed

+65
-0
lines changed

2 files changed

+65
-0
lines changed

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

+21
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import org.opensearch.commons.alerting.action.AlertingActions
1616
import org.opensearch.commons.alerting.action.IndexMonitorRequest
1717
import org.opensearch.commons.alerting.action.IndexMonitorResponse
1818
import org.opensearch.commons.alerting.model.BucketLevelTrigger
19+
import org.opensearch.commons.alerting.model.DocLevelMonitorInput
1920
import org.opensearch.commons.alerting.model.DocumentLevelTrigger
2021
import org.opensearch.commons.alerting.model.Monitor
2122
import org.opensearch.commons.alerting.model.QueryLevelTrigger
@@ -46,6 +47,11 @@ private val log = LogManager.getLogger(RestIndexMonitorAction::class.java)
4647
*/
4748
class RestIndexMonitorAction : BaseRestHandler() {
4849

50+
// allowed characters [- : , ( ) [ ] ' _]
51+
private val allowedChars = "-:,\\(\\)\\[\\]\'_"
52+
// regex to restrict string to alphanumeric and allowed chars, must be between 1 - 256 characters
53+
val regex = "[\\w\\s$allowedChars]{1,256}"
54+
4955
override fun getName(): String {
5056
return "index_monitor_action"
5157
}
@@ -116,9 +122,11 @@ class RestIndexMonitorAction : BaseRestHandler() {
116122
if (it !is DocumentLevelTrigger) {
117123
throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor")
118124
}
125+
validateDocLevelQueryName(monitor)
119126
}
120127
}
121128
}
129+
122130
val seqNo = request.paramAsLong(IF_SEQ_NO, SequenceNumbers.UNASSIGNED_SEQ_NO)
123131
val primaryTerm = request.paramAsLong(IF_PRIMARY_TERM, SequenceNumbers.UNASSIGNED_PRIMARY_TERM)
124132
val refreshPolicy = if (request.hasParam(REFRESH)) {
@@ -133,6 +141,19 @@ class RestIndexMonitorAction : BaseRestHandler() {
133141
}
134142
}
135143

144+
private fun validateDocLevelQueryName(monitor: Monitor) {
145+
monitor.inputs.filterIsInstance<DocLevelMonitorInput>().forEach { docLevelMonitorInput ->
146+
docLevelMonitorInput.queries.forEach { dlq ->
147+
if (!dlq.name.matches(Regex(regex))) {
148+
throw IllegalArgumentException(
149+
"Doc level query name, ${dlq.name}, may only contain alphanumeric values and " +
150+
"these special characters: ${allowedChars.replace("\\","")}"
151+
)
152+
}
153+
}
154+
}
155+
}
156+
136157
private fun validateDataSources(monitor: Monitor) { // Data Sources will currently be supported only at transport layer.
137158
if (monitor.dataSources != null) {
138159
if (

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

+44
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import org.opensearch.alerting.randomAnomalyDetector
2626
import org.opensearch.alerting.randomAnomalyDetectorWithUser
2727
import org.opensearch.alerting.randomBucketLevelMonitor
2828
import org.opensearch.alerting.randomBucketLevelTrigger
29+
import org.opensearch.alerting.randomDocLevelMonitorInput
30+
import org.opensearch.alerting.randomDocLevelQuery
2931
import org.opensearch.alerting.randomDocumentLevelMonitor
3032
import org.opensearch.alerting.randomDocumentLevelTrigger
3133
import org.opensearch.alerting.randomQueryLevelMonitor
@@ -1285,6 +1287,48 @@ class MonitorRestApiIT : AlertingRestTestCase() {
12851287
}
12861288
}
12871289

1290+
fun `test creating and updating a document monitor with invalid query name`() {
1291+
// creating a monitor with an invalid query name
1292+
val invalidQueryName = "Invalid @ query ! name"
1293+
val queries = listOf(randomDocLevelQuery(name = invalidQueryName))
1294+
val randomDocLevelMonitorInput = randomDocLevelMonitorInput(queries = queries)
1295+
val inputs = listOf(randomDocLevelMonitorInput)
1296+
val trigger = randomDocumentLevelTrigger()
1297+
var monitor = randomDocumentLevelMonitor(inputs = inputs, triggers = listOf(trigger))
1298+
1299+
try {
1300+
client().makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity())
1301+
fail("Doc level monitor with invalid query name should be rejected")
1302+
} catch (e: ResponseException) {
1303+
assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus())
1304+
val expectedMessage = "Doc level query name, $invalidQueryName, may only contain alphanumeric values"
1305+
e.message?.let { assertTrue(it.contains(expectedMessage)) }
1306+
}
1307+
1308+
// successfully creating monitor with valid query name
1309+
val testIndex = createTestIndex()
1310+
val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "valid name", fields = listOf())
1311+
val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery))
1312+
1313+
monitor = createMonitor(randomDocumentLevelMonitor(inputs = listOf(docLevelInput), triggers = listOf(trigger)))
1314+
1315+
// updating monitor with invalid query name
1316+
val updatedDocQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = invalidQueryName, fields = listOf())
1317+
val updatedDocLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(updatedDocQuery))
1318+
1319+
try {
1320+
client().makeRequest(
1321+
"PUT", monitor.relativeUrl(),
1322+
emptyMap(), monitor.copy(inputs = listOf(updatedDocLevelInput)).toHttpEntity()
1323+
)
1324+
fail("Doc level monitor with invalid query name should be rejected")
1325+
} catch (e: ResponseException) {
1326+
assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus())
1327+
val expectedMessage = "Doc level query name, $invalidQueryName, may only contain alphanumeric values"
1328+
e.message?.let { assertTrue(it.contains(expectedMessage)) }
1329+
}
1330+
}
1331+
12881332
/**
12891333
* This use case is needed by the frontend plugin for displaying alert counts on the Monitors list page.
12901334
* https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/server/services/MonitorService.js#L235

0 commit comments

Comments
 (0)