fix: guard against EmptyMap cast in doc-level monitor recreateRunContext and mapping traversal

[thecodingshrimp]

Summary

This PR adds defence-in-depth guards against ClassCastException / UnsupportedOperationException
caused by immutable empty maps being cast to MutableMap in the doc-level monitor subsystem.

Closes: #2220
Related: opensearch-project/common-utils#967 (root-cause fix belongs there)

---

Root cause

MonitorMetadata.lastRunContext defaults to kotlin.collections.EmptyMap (the immutable
singleton returned by mapOf()) when deserialized from .opensearch-alerting-config and
the stored document has no last_run_context field (monitor never ran, or created under
an older plugin version).

MonitorMetadataService.recreateRunContext() line 210 casts this directly to
MutableMap<String, MutableMap<String, Any>>. EmptyMap is not a MutableMap, so the
JVM throws ClassCastException on any PUT update to such a monitor.

The correct fix — changing mapOf() to mutableMapOf() in MonitorMetadata.kt — is
tracked in opensearch-project/common-utils#967. This PR adds a guard at the cast site so
that the crash cannot occur regardless of which common-utils version is deployed.

---

Changes

Fix A — MonitorMetadataService.kt (critical)

Guard the cast at line 210: if lastRunContext.isEmpty(), pass null to
createFullRunContext() (which accepts null and builds a fresh context).

Fix B — DocLevelMonitorQueries.kt (plausible crash path)

traverseMappingsAndUpdate() casts values from indexMetadata.mapping()?.sourceAsMap
directly to MutableMap. Java's Collections.emptyMap() (returned for empty nested
objects {}) is not mutable; .put() / .remove() would throw
UnsupportedOperationException. Replace bare casts with .toMutableMap() defensive
copies at the three cast sites inside traverseMappingsAndUpdate().

Fix C — MonitorFanOutUtils.kt / DocumentLevelMonitorRunner.kt (contract)

initializeNewLastRunContext() always returns a MutableMap but its return type is
declared as Map. Update the declaration and remove the now-unnecessary as MutableMap
cast at the call site in DocumentLevelMonitorRunner.kt and
RemoteDocumentLevelMonitorRunner.kt.

---

Files changed

• alerting/src/main/kotlin/org/opensearch/alerting/MonitorMetadataService.kt
• alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt
• alerting/src/main/kotlin/org/opensearch/alerting/MonitorFanOutUtils.kt
• alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt
• alerting/src/main/kotlin/org/opensearch/alerting/remote/monitors/RemoteDocumentLevelMonitorRunner.kt

---

Testing

• All existing unit and integration tests pass (./gradlew test)
• Manual verification: create a doc-level monitor, delete its metadata entry from
  .opensearch-alerting-config, then PUT an update to the monitor — no
  ClassCastException thrown
• Doc-level monitor mapping traversal with an index containing empty nested objects
  ({}) does not throw UnsupportedOperationException

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 276f481)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Guard EmptyMap cast in recreateRunContext Relevant files: alerting/src/main/kotlin/org/opensearch/alerting/MonitorMetadataService.kt Sub-PR theme: Defensive copies in traverseMappingsAndUpdate Relevant files: alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt Sub-PR theme: Tighten initializeNewLastRunContext return type to MutableMap Relevant files: alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt alerting/src/main/kotlin/org/opensearch/alerting/MonitorFanOutUtils.kt alerting/src/main/kotlin/org/opensearch/alerting/remote/monitors/RemoteDocumentLevelMonitorRunner.kt

⚡ Recommended focus areas for review Variable Scope Bug runContext is declared inside the if (monitor.monitorType.endsWith(...)) block but referenced outside it in return if (runContext != null). As shown in the diff, val runContext = ... is assigned only within a conditional expression, but the following return if (runContext != null) uses it at outer scope. If the new code structure places the val runContext inside a nested block rather than at the outer scope of the try, this will fail to compile. Verify that val runContext = if (...) { ... } else null is declared at the outer scope so it's visible to the return statement. val runContext = if (monitor.monitorType.endsWith(Monitor.MonitorType.DOC_LEVEL_MONITOR.value)) { @Suppress("UNCHECKED_CAST") val lastRunCtx = if (metadata.lastRunContext.isEmpty()) null else (metadata.lastRunContext as MutableMap<String, MutableMap<String, Any>>) createFullRunContext(monitorIndex, lastRunCtx) } else null return if (runContext != null) { Semantic Change traverseMappingsAndUpdate now operates on defensive copies (toMutableMap()) of child nodes rather than the original maps. If processLeafFn or the recursive traversal previously relied on in-place mutation of the original node map propagating back to callers, that mutation is now lost since changes are applied to copies. Confirm the traversal only relies on the returned newNodes list (applied via node.remove/node.put on the outer node) and not on nested-map mutation side effects. val nodeProps = (it.value as Map<String, Any>).toMutableMap() // If it has type property and type is not "nested" then this is a leaf if (nodeProps.containsKey(TYPE) && nodeProps[TYPE] != NESTED) { // At this point we know full path of node, so we add it to output array flattenPaths.put(fullPath, nodeProps) // Calls processLeafFn and gets old node name, new node name and new properties of node. // This is all information we need to update this node val (oldName, newName, props) = processLeafFn(it.key, fullPath, (it.value as Map<String, Any>).toMutableMap()) newNodes.add(Triple(oldName, newName, props)) } else if (nodeProps.containsKey(PROPERTIES) && nodeProps[PROPERTIES] != null) { // Internal(non-leaf) node - visit children traverseMappingsAndUpdate((nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap(), fullPath, processLeafFn, flattenPaths)

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 276f481

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Avoid redundant map casts and copies The it.value as Map<String, Any> cast is repeated three times, causing redundant work and risking inconsistency if the value changes. Reuse the already-computed nodeProps for both processLeafFn and the recursive traversal to avoid re-casting and re-copying. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [231-242] val nodeProps = (it.value as Map<String, Any>).toMutableMap() // If it has type property and type is not "nested" then this is a leaf if (nodeProps.containsKey(TYPE) && nodeProps[TYPE] != NESTED) { // At this point we know full path of node, so we add it to output array flattenPaths.put(fullPath, nodeProps) // Calls processLeafFn and gets old node name, new node name and new properties of node. // This is all information we need to update this node - val (oldName, newName, props) = processLeafFn(it.key, fullPath, (it.value as Map<String, Any>).toMutableMap()) + val (oldName, newName, props) = processLeafFn(it.key, fullPath, nodeProps) newNodes.add(Triple(oldName, newName, props)) } else if (nodeProps.containsKey(PROPERTIES) && nodeProps[PROPERTIES] != null) { // Internal(non-leaf) node - visit children traverseMappingsAndUpdate((nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap(), fullPath, processLeafFn, flattenPaths) } Suggestion importance[1-10]: 6 __ Why: Reusing the already computed nodeProps avoids redundant casts and copies, improving both efficiency and consistency. This is a valid minor improvement.	Low
Possible issue	Defensively convert map instead of casting Casting metadata.lastRunContext directly to MutableMap<String, MutableMap<String, Any>> will still fail with ClassCastException if the underlying map (or its inner values) is an immutable/EmptyMap-typed instance even when non-empty. Convert to a mutable map defensively via toMutableMap() with mutable inner maps to fully guard against the cast issue. alerting/src/main/kotlin/org/opensearch/alerting/MonitorMetadataService.kt [209-214] val runContext = if (monitor.monitorType.endsWith(Monitor.MonitorType.DOC_LEVEL_MONITOR.value)) { @Suppress("UNCHECKED_CAST") val lastRunCtx = if (metadata.lastRunContext.isEmpty()) null - else (metadata.lastRunContext as MutableMap<String, MutableMap<String, Any>>) + else metadata.lastRunContext.mapValues { + (it.value as Map<String, Any>).toMutableMap() + }.toMutableMap() createFullRunContext(monitorIndex, lastRunCtx) } else null Suggestion importance[1-10]: 5 __ Why: The suggestion addresses a real concern: the unchecked cast could still fail if the underlying map is immutable. However, whether this matters depends on how the map was originally constructed, so the impact is moderate.	Low

---

Previous suggestions

Suggestions up to commit ef25277

Category	Suggestion	Impact
Possible issue	Persist mutations made on copied child maps Since traverseMappingsAndUpdate mutates the passed node map (via newNodes.forEach updates later), creating a new mutable copy of nodeProps[PROPERTIES] means the recursive updates will be applied to the copy and discarded, not persisted back into the original mappings tree. Assign the copy back into nodeProps[PROPERTIES] (and ensure nodeProps itself is written back) or defensively cast when the original is already a MutableMap to preserve mutation semantics. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [240-242] } else if (nodeProps.containsKey(PROPERTIES) && nodeProps[PROPERTIES] != null) { // Internal(non-leaf) node - visit children - traverseMappingsAndUpdate((nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap(), fullPath, processLeafFn, flattenPaths) + val childProps = (nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap() + traverseMappingsAndUpdate(childProps, fullPath, processLeafFn, flattenPaths) + nodeProps[PROPERTIES] = childProps + @Suppress("UNCHECKED_CAST") + (node as MutableMap<String, Any>)[it.key] = nodeProps } Suggestion importance[1-10]: 8 __ Why: Valid concern: converting nodeProps[PROPERTIES] to a new mutable map via toMutableMap() breaks the mutation semantics of the recursive traversal, since updates in newNodes.forEach on the copy won't propagate back into the original mappings tree. This could be a real correctness regression from the change.	Medium
General	Defensively copy lastRunContext to mutable maps The cast to MutableMap<String, MutableMap<String, Any>> can still fail if metadata.lastRunContext is a non-empty immutable map (e.g., a deserialized Collections.unmodifiableMap or EmptyMap-like structure with entries). Consider defensively copying into a mutable structure using toMutableMap() (and mapping inner maps similarly) to guarantee safe mutation downstream. alerting/src/main/kotlin/org/opensearch/alerting/MonitorMetadataService.kt [209-214] val runContext = if (monitor.monitorType.endsWith(Monitor.MonitorType.DOC_LEVEL_MONITOR.value)) { @Suppress("UNCHECKED_CAST") val lastRunCtx = if (metadata.lastRunContext.isEmpty()) null - else (metadata.lastRunContext as MutableMap<String, MutableMap<String, Any>>) + else metadata.lastRunContext.mapValues { (it.value as Map<String, Any>).toMutableMap() }.toMutableMap() + as MutableMap<String, MutableMap<String, Any>> createFullRunContext(monitorIndex, lastRunCtx) } else null Suggestion importance[1-10]: 5 __ Why: Reasonable defensive suggestion: using toMutableMap() on the outer and inner maps would avoid ClassCastException at cast time (though Kotlin's unchecked cast doesn't throw immediately). Moderate impact on robustness.	Low

Suggestions up to commit dec1828

Category	Suggestion	Impact
Possible issue	Preserve child mapping updates after copy The recursive call now passes a copy of the children map (via toMutableMap()) instead of the original map. Any updates made by traverseMappingsAndUpdate to that copy will no longer be reflected in the parent's nodeProps[PROPERTIES], potentially breaking mapping updates. Consider casting via a safe unchecked cast to preserve the same underlying map reference, or reassigning the updated copy back into nodeProps[PROPERTIES]. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [242] } else if (nodeProps.containsKey(PROPERTIES) && nodeProps[PROPERTIES] != null) { // Internal(non-leaf) node - visit children - traverseMappingsAndUpdate((nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap(), fullPath, processLeafFn, flattenPaths) + val childProps = (nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap() + traverseMappingsAndUpdate(childProps, fullPath, processLeafFn, flattenPaths) + nodeProps[PROPERTIES] = childProps } Suggestion importance[1-10]: 8 __ Why: Valid concern: by passing toMutableMap() copy, any updates made by the recursive call are lost from the parent's nodeProps[PROPERTIES], potentially breaking mapping traversal behavior that previously mutated in-place.	Medium
General	Ensure mutable copy of lastRunContext Casting metadata.lastRunContext directly to MutableMap<String, MutableMap<String, Any>> may still fail if the underlying map is an immutable Map (e.g., deserialized as Collections$UnmodifiableMap or EmptyMap-like) even when non-empty. Consider defensively copying to a MutableMap via toMutableMap() and mapping inner values similarly to guarantee mutability. alerting/src/main/kotlin/org/opensearch/alerting/MonitorMetadataService.kt [209-213] val runContext = if (monitor.monitorType.endsWith(Monitor.MonitorType.DOC_LEVEL_MONITOR.value)) { val lastRunCtx = if (metadata.lastRunContext.isEmpty()) null - else @Suppress("UNCHECKED_CAST") (metadata.lastRunContext as MutableMap<String, MutableMap<String, Any>>) + else @Suppress("UNCHECKED_CAST") + metadata.lastRunContext.mapValues { (it.value as Map<String, Any>).toMutableMap() }.toMutableMap() + as MutableMap<String, MutableMap<String, Any>> createFullRunContext(monitorIndex, lastRunCtx) } else null Suggestion importance[1-10]: 5 __ Why: Reasonable defensive suggestion—if the map is immutable, the unchecked cast could still lead to runtime errors when createFullRunContext mutates it. The impact depends on how lastRunContext is typically constructed.	Low

Suggestions up to commit 31032ab

Category	Suggestion	Impact
Possible issue	Avoid copying maps that must be mutated in place Since traverseMappingsAndUpdate now passes a fresh toMutableMap() copy for the PROPERTIES child, subsequent updates to leaves inside that copy will not be reflected back into the original parent's PROPERTIES map. This breaks the in-place mutation semantics the traversal relies on to update the mapping tree. Pass nodeProps (or the original inner map) so updates persist to the parent tree. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [231-242] val nodeProps = (it.value as Map<String, Any>).toMutableMap() -// If it has type property and type is not "nested" then this is a leaf if (nodeProps.containsKey(TYPE) && nodeProps[TYPE] != NESTED) { - // At this point we know full path of node, so we add it to output array flattenPaths.put(fullPath, nodeProps) - // Calls processLeafFn and gets old node name, new node name and new properties of node. - // This is all information we need to update this node - val (oldName, newName, props) = processLeafFn(it.key, fullPath, (it.value as Map<String, Any>).toMutableMap()) + val (oldName, newName, props) = processLeafFn(it.key, fullPath, nodeProps) newNodes.add(Triple(oldName, newName, props)) } else if (nodeProps.containsKey(PROPERTIES) && nodeProps[PROPERTIES] != null) { - // Internal(non-leaf) node - visit children - traverseMappingsAndUpdate((nodeProps[PROPERTIES] as Map<String, Any>).toMutableMap(), fullPath, processLeafFn, flattenPaths) + @Suppress("UNCHECKED_CAST") + traverseMappingsAndUpdate(nodeProps[PROPERTIES] as MutableMap<String, Any>, fullPath, processLeafFn, flattenPaths) } Suggestion importance[1-10]: 9 __ Why: This is a critical correctness issue: creating a new toMutableMap() copy for the child PROPERTIES map means updates during traversal will not propagate back to the original mapping tree, breaking the in-place mutation semantics the function relies on.	High
General	Use mutable copies instead of unsafe casts Casting an immutable Map (e.g. EmptyMap or a Collections.unmodifiableMap) directly to MutableMap can succeed at runtime but throw UnsupportedOperationException on later modification. To be safe, convert the map to a mutable copy instead of casting. alerting/src/main/kotlin/org/opensearch/alerting/MonitorMetadataService.kt [209-213] val runContext = if (monitor.monitorType.endsWith(Monitor.MonitorType.DOC_LEVEL_MONITOR.value)) { val lastRunCtx = if (metadata.lastRunContext.isEmpty()) null - else @Suppress("UNCHECKED_CAST") (metadata.lastRunContext as MutableMap<String, MutableMap<String, Any>>) + else @Suppress("UNCHECKED_CAST") + (metadata.lastRunContext as Map<String, Map<String, Any>>) + .mapValues { it.value.toMutableMap() } + .toMutableMap() createFullRunContext(monitorIndex, lastRunCtx) } else null Suggestion importance[1-10]: 4 __ Why: The concern about casting an immutable map to MutableMap is valid in principle, but in practice metadata.lastRunContext is likely already a mutable map from prior code paths, so the risk is moderate rather than critical.	Low

[github-actions]

Persistent review updated to latest commit dec1828

[github-actions]

Persistent review updated to latest commit ef25277

[github-actions]

Persistent review updated to latest commit 276f481