fix: guard against null and immutable map returns in RuleIndices and CustomLogType

[thecodingshrimp]

Summary

fixes #1796
Two defensive guards for null/immutable map returns found during a code audit.
These are independent bugs; no upstream issue existed prior to this PR.

---

Finding 1 — NPE in RuleIndices.ingestQueries() (HIGH)

File: src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java, line 290

LogTypeService.getRuleFieldMappingsForBuiltinLogType() returns null when the
log type is not registered in builtinLogTypeLoader. That null is passed directly
into OSQueryBackend's constructor, which stores it as this.fieldMappings
when enableFieldMappings=true. getMappedField() then calls
this.fieldMappings.containsKey(field) unconditionally, causing a
NullPointerException during pre-packaged rule ingestion at startup for any
unrecognized rule category.

Before:

Map<String, String> fieldMappings = logTypeService.getRuleFieldMappingsForBuiltinLogType(category);
final QueryBackend backend = new OSQueryBackend(fieldMappings, true, true);

After:

Map<String, String> fieldMappings = logTypeService.getRuleFieldMappingsForBuiltinLogType(category);
if (fieldMappings == null) {
    fieldMappings = new HashMap<>();
}
final QueryBackend backend = new OSQueryBackend(fieldMappings, true, true);

---

Finding 2 — UnsupportedOperationException in CustomLogType(StreamInput) (MEDIUM)

File: src/main/java/org/opensearch/securityanalytics/model/CustomLogType.java, line 96

StreamInput.readMap() returns Collections.emptyMap() (Java's immutable singleton)
when the serialized map has zero entries. CustomLogType stores this value directly
as this.tags. Any subsequent call to getTags().put(...) throws
UnsupportedOperationException.

Before:

public CustomLogType(StreamInput sin) throws IOException {
    this(
            sin.readString(),
            sin.readLong(),
            sin.readString(),
            sin.readString(),
            sin.readString(),
            sin.readString(),
            sin.readMap()
    );
}

After:

public CustomLogType(StreamInput sin) throws IOException {
    this(
            sin.readString(),
            sin.readLong(),
            sin.readString(),
            sin.readString(),
            sin.readString(),
            sin.readString(),
            toMutableMap(sin.readMap())
    );
}

private static Map<String, Object> toMutableMap(Map<String, Object> map) {
    return map != null ? new HashMap<>(map) : null;
}

---

Finding 3 — Reviewed, no action (LOW / informational)

LogTypeService.java lines 656 and 791 return Map.of() for empty field-mapping
responses. All call sites are read-only (.containsKey(), .get()). No fix required.

---

Testing

• Existing unit and integration tests pass.
• TransportIndexDetectorAction.java is not modified in this PR.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 05e0744)

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 against null fieldMappings in RuleIndices.ingestQueries Relevant files: src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java Sub-PR theme: Make CustomLogType tags map mutable when deserialized Relevant files: src/main/java/org/opensearch/securityanalytics/model/CustomLogType.java

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 05e0744
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Deep-copy or document shallow mutability StreamInput.readMap() may return nested immutable maps/lists as values, so a shallow HashMap copy still leaves inner structures immutable. If downstream code mutates nested entries, consider a deep copy or document that only the top-level map is mutable to avoid subtle UnsupportedOperationException failures. src/main/java/org/opensearch/securityanalytics/model/CustomLogType.java [101-103] private static Map<String, Object> toMutableMap(Map<String, Object> map) { + // Note: only top-level map is made mutable; nested collections remain as returned by StreamInput return map != null ? new HashMap<>(map) : null; } Suggestion importance[1-10]: 4 __ Why: The observation about nested immutable collections is valid, but the suggestion only proposes adding a comment rather than fixing the underlying issue, providing limited practical impact.	Low

---

Previous suggestions

Suggestions up to commit 57f2bfb

Category	Suggestion	Impact
General	Return empty map instead of null Returning null from toMutableMap may still cause downstream NullPointerExceptions if callers assume a non-null map (which appears to be the motivation for this fix). Consider returning an empty mutable map when the input is null to consistently guarantee a mutable, non-null map. src/main/java/org/opensearch/securityanalytics/model/CustomLogType.java [101-103] -public CustomLogType(StreamInput sin) throws IOException { - this( - sin.readString(), - sin.readLong(), - sin.readString(), - sin.readString(), - sin.readString(), - sin.readString(), - toMutableMap(sin.readMap()) - ); +private static Map<String, Object> toMutableMap(Map<String, Object> map) { + return map != null ? new HashMap<>(map) : new HashMap<>(); } -private static Map<String, Object> toMutableMap(Map<String, Object> map) { - return map != null ? new HashMap<>(map) : null; -} - Suggestion importance[1-10]: 4 __ Why: Returning an empty map instead of null could be more defensive, but the original code also allowed null via sin.readMap(), so this change may alter existing behavior. The suggestion has some merit for consistency but is not critical.	Low

[github-actions]

Persistent review updated to latest commit 05e0744