From c105c50eef087ea5fd57e63ac5bac49b0bc48677 Mon Sep 17 00:00:00 2001 From: Manaswini Ragamouni <40575189+manaswini1920@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:07:17 -0700 Subject: [PATCH] Fix NPE when nested field type has no properties in doc-level monitor (#2049) A nested field with type 'nested' but no sub-properties causes a NullPointerException during doc-level monitor creation. The traverseMappingsAndUpdate method assumes all non-leaf nodes have a 'properties' map, but nested fields without sub-properties have none. Added a null guard before recursing into nested field properties. Fields with type 'nested' and no properties are now safely skipped. Resolves https://github.com/opensearch-project/security-analytics/issues/1472 Signed-off-by: Manaswini Ragamouni Signed-off-by: Manaswini Ragamouni Co-authored-by: Manaswini Ragamouni (cherry picked from commit 9b31fabfced6ece17d80b0891b2cec98d9e9e163) Signed-off-by: Manaswini Ragamouni --- .../alerting/util/DocLevelMonitorQueries.kt | 2 +- .../alerting/util/AlertingUtilsTests.kt | 82 ++++++++++++++++++- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt b/alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt index 1b63c0024..3351c923c 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt @@ -237,7 +237,7 @@ class DocLevelMonitorQueries(private val client: Client, private val clusterServ // This is all information we need to update this node val (oldName, newName, props) = processLeafFn(it.key, fullPath, it.value as MutableMap) newNodes.add(Triple(oldName, newName, props)) - } else { + } else if (nodeProps.containsKey(PROPERTIES) && nodeProps[PROPERTIES] != null) { // Internal(non-leaf) node - visit children traverseMappingsAndUpdate(nodeProps[PROPERTIES] as MutableMap, fullPath, processLeafFn, flattenPaths) } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/util/AlertingUtilsTests.kt b/alerting/src/test/kotlin/org/opensearch/alerting/util/AlertingUtilsTests.kt index 31dcb6591..ec9d91c33 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/util/AlertingUtilsTests.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/util/AlertingUtilsTests.kt @@ -5,6 +5,9 @@ package org.opensearch.alerting.util +import org.mockito.Mockito.mock +import org.opensearch.alerting.AlertService +import org.opensearch.alerting.MonitorRunnerService import org.opensearch.alerting.model.AlertContext import org.opensearch.alerting.randomAction import org.opensearch.alerting.randomBucketLevelTrigger @@ -14,8 +17,10 @@ import org.opensearch.alerting.randomQueryLevelTrigger import org.opensearch.alerting.randomTemplateScript import org.opensearch.alerting.script.BucketLevelTriggerExecutionContext import org.opensearch.alerting.script.DocumentLevelTriggerExecutionContext +import org.opensearch.client.Client +import org.opensearch.cluster.service.ClusterService +import org.opensearch.common.unit.TimeValue import org.opensearch.test.OpenSearchTestCase - class AlertingUtilsTests : OpenSearchTestCase() { fun `test parseSampleDocTags only returns expected tags`() { val expectedDocSourceTags = (0..3).map { "field$it" } @@ -176,4 +181,79 @@ class AlertingUtilsTests : OpenSearchTestCase() { triggers.forEach { trigger -> assertFalse(printsSampleDocData(trigger)) } } + + fun `test getCancelAfterTimeInterval returns -1 when setting is default`() { + val original = MonitorRunnerService.monitorCtx.cancelAfterTimeInterval + try { + MonitorRunnerService.monitorCtx.cancelAfterTimeInterval = TimeValue.timeValueMinutes(-1) + assertEquals(-1L, getCancelAfterTimeInterval()) + } finally { + MonitorRunnerService.monitorCtx.cancelAfterTimeInterval = original + } + } + + fun `test getCancelAfterTimeInterval returns at least ALERTS_SEARCH_TIMEOUT`() { + val original = MonitorRunnerService.monitorCtx.cancelAfterTimeInterval + try { + // Setting lower than ALERTS_SEARCH_TIMEOUT (5 min) should return 5 min + MonitorRunnerService.monitorCtx.cancelAfterTimeInterval = TimeValue.timeValueMinutes(1) + assertEquals(AlertService.ALERTS_SEARCH_TIMEOUT.minutes, getCancelAfterTimeInterval()) + } finally { + MonitorRunnerService.monitorCtx.cancelAfterTimeInterval = original + } + } + + fun `test getCancelAfterTimeInterval returns setting when higher than ALERTS_SEARCH_TIMEOUT`() { + val original = MonitorRunnerService.monitorCtx.cancelAfterTimeInterval + try { + MonitorRunnerService.monitorCtx.cancelAfterTimeInterval = TimeValue.timeValueMinutes(10) + assertEquals(10L, getCancelAfterTimeInterval()) + } finally { + MonitorRunnerService.monitorCtx.cancelAfterTimeInterval = original + } + } + + fun `test traverseMappingsAndUpdate with nested field type without properties succeeds`() { + // Verifies fix for https://github.com/opensearch-project/security-analytics/issues/1472 + val docLevelMonitorQueries = DocLevelMonitorQueries(mock(Client::class.java), mock(ClusterService::class.java)) + val mappings = mutableMapOf( + "message" to mutableMapOf("type" to "text"), + "http_request_headers" to mutableMapOf("type" to "nested") + ) + val flattenPaths = mutableMapOf>() + val leafProcessor = + fun(fieldName: String, _: String, props: MutableMap): + Triple> { + return Triple(fieldName, fieldName, props) + } + + docLevelMonitorQueries.traverseMappingsAndUpdate(mappings, "", leafProcessor, flattenPaths) + + assertTrue("Expected 'message' in flatten paths", flattenPaths.containsKey("message")) + assertFalse("Expected nested field to be skipped", flattenPaths.containsKey("http_request_headers")) + } + + fun `test traverseMappingsAndUpdate with nested field type with properties works`() { + val docLevelMonitorQueries = DocLevelMonitorQueries(mock(Client::class.java), mock(ClusterService::class.java)) + val mappings = mutableMapOf( + "message" to mutableMapOf("type" to "text"), + "dll" to mutableMapOf( + "type" to "nested", + "properties" to mutableMapOf( + "name" to mutableMapOf("type" to "keyword") + ) + ) + ) + val flattenPaths = mutableMapOf>() + val leafProcessor = + fun(fieldName: String, _: String, props: MutableMap): + Triple> { + return Triple(fieldName, fieldName, props) + } + + docLevelMonitorQueries.traverseMappingsAndUpdate(mappings, "", leafProcessor, flattenPaths) + + assertTrue("Expected 'message' in flatten paths", flattenPaths.containsKey("message")) + assertTrue("Expected 'dll.name' in flatten paths", flattenPaths.containsKey("dll.name")) + } }