Added audit log categories for cluster and index settings changes

[Rishav9852Kumar]

Description

Added new audit log categories CLUSTER_SETTINGS_CHANGED and INDEX_SETTINGS_CHANGED
to track changes to cluster and index settings.

Both categories are disabled by default for transport (matching the existing behavior of AUTHENTICATED and GRANTED_PRIVILEGES). Users can enable them by removing them from disabled_transport_categories in their audit config.

Each setting change is logged with an operation field:

• set: The setting was explicitly assigned a value (new_value is non-null). This includes both first-time sets and updates to existing values.
• removed: The setting was reset to its default by passing null as the value (new_value is null). The old_value field captures what the setting was before removal.

Each setting change is logged with an scope field:

• persistent — Cluster setting that survives restarts (
  PUT /_cluster/settings with persistent)
• transient — Cluster setting that is lost on restart (
  PUT /_cluster/settings with transient)
• index — Index-level setting (PUT //_settings)

---

• Category: New feature

• Why these changes are required?
  Currently, the audit log feature has no category to track changes to cluster or index settings. Operators have no way to audit what settings were changed, when, by whom, or what the previous values were. The existing INDEX_EVENT
  category only logs the request body without old values or sensitive setting redaction.

• What is the old behavior before changes and new behavior after changes?

  • Old behavior: Settings changes are only captured under INDEX_EVENT with the request body. No old values, no per-setting breakdown, no sensitive value redaction.
  • New behavior: Two new audit categories produce structured audit_settings_changes entries with:
    • Per-setting setting, old_value, new_value, operation (set/removed), scope (persistent/transient/index)
    • Automatic redaction of sensitive settings via SecureSetting detection with pattern fallback
    • Index wildcard resolution to concrete index names
    • Independent enable/disable via existing disabled_transport_categories config

Example: CLUSTER_SETTINGS_CHANGED

{
  "audit_category": "CLUSTER_SETTINGS_CHANGED",
  "audit_request_effective_user": "admin",
  "audit_transport_action": "cluster:admin/settings/update",
  "audit_settings_changes": [
    {
      "setting": "cluster.max_shards_per_node",
      "old_value": null,
      "new_value": "2000",
      "operation": "set",
      "scope": "persistent"
    }
  ]
}

Example: INDEX_SETTINGS_CHANGED

{
  "audit_category": "INDEX_SETTINGS_CHANGED",
  "audit_request_effective_user": "admin",
  "audit_transport_action": "indices:admin/settings/update",
  "audit_trace_indices": ["my-index-*"],
  "audit_trace_resolved_indices": ["my-index-001", "my-index-002"],
  "audit_settings_changes": [
    {
      "setting": "index.number_of_replicas",
      "old_value": "1",
      "new_value": "2",
      "operation": "set",
      "scope": "index"
    }
  ]
}

Issues Resolved

Resolves #5320

Testing

Unit Tests

Added SettingsChangeAuditTest.java with 27 tests covering all new functionality. Updated existing tests in DisabledCategoriesTest, AuditCategoryTest, AuditConfigSerializeTest, and AuditConfigFilterTest for the new categories and default-disabled behavior.

Area	Tests
Cluster settings	testClusterPersistentSettingChange, testClusterTransientSettingChange, testClusterBothPersistentAndTransientSettings, testClusterSettingResetToDefault, testClusterSettingOldValueCaptured
Index settings	testIndexSettingChange, testIndexSettingChangeIncludesIndices
Request routing	testRoutingClusterUpdateSettingsRequest, testRoutingUpdateSettingsRequest, testRoutingUnknownRequestTypeProducesNoMessage
Empty request	testEmptyClusterSettingsProducesNoMessage
Redaction	testSensitiveSettingRedactionByPasswordPattern, testSensitiveSettingRedactionBySecretPattern, testSensitiveSettingRedactionByTokenPattern, testNonSensitiveSettingNotRedacted, testSensitiveOldValueAlsoRedacted, testSensitiveSettingRedactionWhenRegistryReturnsFalse
Cluster state unavailable	testClusterStateUnavailableFallsBackGracefully, testIndexSettingsClusterStateUnavailableFallsBackGracefully
Category disable	testClusterSettingsChangedCategoryDisabled, testIndexSettingsChangedCategoryDisabled
AuditMessage field	testAuditMessageSettingsChangesField, testAuditMessageSettingsChangesNullIgnored, testAuditMessageSettingsChangesEmptyIgnored
Action field	testClusterSettingsChangeIncludesAction, testIndexSettingsChangeIncludesAction
Multiple settings	testMultipleSettingsInOneClusterRequest

Integ Test

Area	Tests
Settings change audit	testSettingsChangeAudit (persistent cluster setting, transient cluster setting, reset to default, index setting change, wildcard index resolution, sensitive setting redaction)
Category disable	testSettingsChangeCategoryDisabled

Manual Testing

Tested on OpenSearch 3.7.0-SNAPSHOT with security plugin 3.7.0.0-SNAPSHOT, single node cluster.

Test 1: Set persistent cluster setting

Request:

curl -XPUT "https://localhost:9200/_cluster/settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{"persistent": {"cluster.max_shards_per_node": 2000}}'

Audit log:

{
  "audit_category": "CLUSTER_SETTINGS_CHANGED",
  "audit_request_effective_user": "admin",
  "audit_transport_action": "cluster:admin/settings/update",
  "audit_transport_request_type": "ClusterUpdateSettingsRequest",
  "audit_request_origin": "REST",
  "audit_request_layer": "TRANSPORT",
  "@timestamp": "2026-04-27T19:17:37.811+00:00",
  "audit_format_version": 4,
  "audit_settings_changes": [
    {
      "setting": "cluster.max_shards_per_node",
      "old_value": null,
      "new_value": "2000",
      "operation": "set",
      "scope": "persistent"
    }
  ]
}

✅ Persistent setting captured with correct old/new values, operation, and scope.

Test 2: Set transient cluster setting

Request:

curl -XPUT "https://localhost:9200/_cluster/settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{"transient": {"cluster.routing.allocation.enable": "primaries"}}'

Audit log:

{
  "audit_category": "CLUSTER_SETTINGS_CHANGED",
  "audit_transport_action": "cluster:admin/settings/update",
  "audit_settings_changes": [
    {
      "setting": "cluster.routing.allocation.enable",
      "old_value": null,
      "new_value": "primaries",
      "operation": "set",
      "scope": "transient"
    }
  ]
}

✅ Transient scope correctly identified.

Test 3: Reset setting to default (null → removed)

Request:

curl -XPUT "https://localhost:9200/_cluster/settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{"persistent": {"cluster.max_shards_per_node": null}}'

Audit log:

{
  "audit_category": "CLUSTER_SETTINGS_CHANGED",
  "audit_settings_changes": [
    {
      "setting": "cluster.max_shards_per_node",
      "old_value": "2000",
      "new_value": null,
      "operation": "removed",
      "scope": "persistent"
    }
  ]
}

✅ Old value captured, operation correctly set to removed.

Test 4: Index setting change

Request:

curl -XPUT "https://localhost:9200/test-audit-index?pretty" -u 'admin:Rishav@123' --insecure
curl -XPUT "https://localhost:9200/test-audit-index/_settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{"index.number_of_replicas": 2}'

Audit log:

{
  "audit_category": "INDEX_SETTINGS_CHANGED",
  "audit_transport_action": "indices:admin/settings/update",
  "audit_trace_indices": ["test-audit-index"],
  "audit_trace_resolved_indices": ["test-audit-index"],
  "audit_settings_changes": [
    {
      "setting": "index.number_of_replicas",
      "old_value": "1",
      "new_value": "2",
      "operation": "set",
      "scope": "index"
    }
  ]
}

✅ Index name captured in audit_trace_indices and audit_trace_resolved_indices.

Test 5: Multiple settings in one call (persistent + transient)

Request:

curl -XPUT "https://localhost:9200/_cluster/settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{
    "persistent": {
      "cluster.max_shards_per_node": 3000,
      "cluster.routing.rebalance.enable": "none"
    },
    "transient": {
      "cluster.routing.allocation.enable": "all"
    }
  }'

Audit log:

{
  "audit_category": "CLUSTER_SETTINGS_CHANGED",
  "audit_settings_changes": [
    {
      "setting": "cluster.max_shards_per_node",
      "old_value": null,
      "new_value": "3000",
      "operation": "set",
      "scope": "persistent"
    },
    {
      "setting": "cluster.routing.rebalance.enable",
      "old_value": null,
      "new_value": "none",
      "operation": "set",
      "scope": "persistent"
    },
    {
      "setting": "cluster.routing.allocation.enable",
      "old_value": "primaries",
      "new_value": "all",
      "operation": "set",
      "scope": "transient"
    }
  ]
}

✅ Single audit entry with all 3 changes across both scopes.

Test 6: Sensitive setting redaction

Request:

curl -XPUT "https://localhost:9200/_cluster/settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{
    "persistent": {
      "plugins.security.ssl.transport.keystore_password": "transport_pass",
      "plugins.security.ssl.http.keystore_password": "http_pass",
      "plugins.security.auth_token_provider": "my_auth_token_123",
      "cluster.max_shards_per_node": 5000
    }
  }'

Audit log (audit_settings_changes):

[
  { "setting": "cluster.max_shards_per_node", "old_value": "3000", "new_value": "5000", "operation": "set", "scope": "persistent" },
  { "setting": "plugins.security.auth_token_provider", "old_value": null, "new_value": "***REDACTED***", "operation": "set", "scope": "persistent" },
  { "setting": "plugins.security.ssl.http.keystore_password", "old_value": null, "new_value": "***REDACTED***", "operation": "set", "scope": "persistent" },
  { "setting": "plugins.security.ssl.transport.keystore_password", "old_value": null, "new_value": "***REDACTED***", "operation": "set", "scope": "persistent" }
]

✅ Settings with password, secret, or token in the name are redacted. Non-sensitive settings show actual values.

Test 7: Wildcard index pattern with resolved indices

Request:

curl -XPUT "https://localhost:9200/test-wild-001?pretty" -u 'admin:Rishav@123' --insecure
curl -XPUT "https://localhost:9200/test-wild-002?pretty" -u 'admin:Rishav@123' --insecure
curl -XPUT "https://localhost:9200/test-wild-*/_settings?pretty" \
  -u 'admin:Rishav@123' --insecure \
  -H 'Content-Type: application/json' \
  -d '{"index.number_of_replicas": 0}'

Audit log:

{
  "audit_category": "INDEX_SETTINGS_CHANGED",
  "audit_trace_indices": ["test-wild-*"],
  "audit_trace_resolved_indices": ["test-wild-001", "test-wild-002"],
  "audit_settings_changes": [
    {
      "setting": "index.number_of_replicas",
      "old_value": "1",
      "new_value": "0",
      "operation": "set",
      "scope": "index"
    }
  ]
}

✅ Wildcard pattern preserved in audit_trace_indices, concrete indices resolved in audit_trace_resolved_indices.

Test 8: Disable category via audit.yml

Added to disabled_transport_categories in audit.yml:

disabled_transport_categories:
  - AUTHENTICATED
  - GRANTED_PRIVILEGES
  - CLUSTER_SETTINGS_CHANGED
  - INDEX_SETTINGS_CHANGED

Applied via securityadmin.sh -t audit, then changed cluster settings — no new audit entries created.

✅ Categories can be independently disabled via existing config mechanism.

Summary

#	Test	Result
1	Set persistent cluster setting	✅
2	Set transient cluster setting	✅
3	Reset to default (null → removed)	✅
4	Index setting change	✅
5	Multiple settings in one call	✅
6	Sensitive setting redaction	✅
7	Wildcard index pattern + resolved indices	✅
8	Disable category via audit.yml	✅

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR —
  N/A
• API changes companion pull request [created](https://github.com/opensearch
  -project/opensearch-api-specification/blob/main/DEVELOPER_GUIDE.md) — N/A
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit befff58)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue When multiple indices match a wildcard pattern in logIndexSettingsChange, only the first resolved index's current settings are retrieved (line 404). If indices have different current values for the same setting, the audit log will incorrectly show the first index's old value for all indices. This produces misleading audit records when updating settings across indices with divergent current values. Settings currentSettings = Settings.EMPTY; if (resolvedIndices.length > 0) { try { final var indexMetadata = clusterService.state().metadata().index(resolvedIndices[0]); if (indexMetadata != null) { currentSettings = indexMetadata.getSettings(); } } catch (final Exception e) { log.debug("Unable to retrieve current index settings for audit", e); } } Possible Issue The isSensitiveSetting method catches all exceptions when checking clusterService.getClusterSettings().isSensitiveSetting(key) (line 474). If the cluster service throws an exception for reasons other than the setting not being registered (e.g., cluster service unavailable, internal error), the method silently falls back to pattern matching. This could fail to redact genuinely sensitive settings that are registered as SecureSetting but trigger an exception during lookup. try { // Looks up the Setting object by key and checks setting.isSensitive(), // which returns true for SecureSetting instances — the proper way to identify secrets if (clusterService.getClusterSettings().isSensitiveSetting(key)) { return true; } } catch (Exception e) { // Setting not registered in cluster settings — fall through to pattern match }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to befff58

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null check for cluster state Add a null check for clusterService.state() before calling resolver.concreteIndexNames(). If the cluster state is unavailable (returns null), the method should return the original indices array to prevent potential NullPointerException during index resolution. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [483-493] private String[] resolveIndices(String[] indices) { if (indices == null || indices.length == 0) { return new String[0]; } try { + if (clusterService.state() == null) { + return indices; + } return resolver.concreteIndexNames(clusterService.state(), IndicesOptions.lenientExpandOpen(), indices); } catch (Exception e) { log.debug("Unable to resolve indices for settings change audit", e); return indices; } } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential NullPointerException when clusterService.state() returns null. However, the existing code already has a try-catch block that handles exceptions, making this a defensive improvement rather than a critical fix. The test at line 540-555 confirms this scenario is already handled gracefully.	Medium
	Guard cluster state access with null check Add a null check for clusterService.state() before accessing metadata(). When the cluster state is unavailable during startup or testing, this will throw NullPointerException. Guard the access with a null check to ensure graceful fallback to Settings.EMPTY. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [401-411] private void logIndexSettingsChange(String action, UpdateSettingsRequest request, Task task) { ... // Use settings from the first resolved index as representative current state Settings currentSettings = Settings.EMPTY; if (resolvedIndices.length > 0) { try { - final var indexMetadata = clusterService.state().metadata().index(resolvedIndices[0]); - if (indexMetadata != null) { - currentSettings = indexMetadata.getSettings(); + final ClusterState state = clusterService.state(); + if (state != null) { + final var indexMetadata = state.metadata().index(resolvedIndices[0]); + if (indexMetadata != null) { + currentSettings = indexMetadata.getSettings(); + } } } catch (final Exception e) { log.debug("Unable to retrieve current index settings for audit", e); } } Suggestion importance[1-10]: 7 __ Why: Similar to suggestion 1, this adds defensive null checking for clusterService.state(). The existing try-catch already handles this case, but the explicit null check improves code clarity and prevents relying on exception handling for control flow. The test at line 560-573 shows this is already handled.	Medium
	Prevent NPE when accessing cluster state Add a null check for clusterService.state() before accessing metadata(). The current code catches exceptions but doesn't explicitly handle the case where state() returns null, which can occur during cluster initialization or in test environments. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [349-358] private void logClusterSettingsChange(String action, ClusterUpdateSettingsRequest request, Task task) { ... final Settings persistentSettings = request.persistentSettings(); if (!persistentSettings.isEmpty()) { Settings currentPersistent = Settings.EMPTY; try { - currentPersistent = clusterService.state().metadata().persistentSettings(); + final ClusterState state = clusterService.state(); + if (state != null) { + currentPersistent = state.metadata().persistentSettings(); + } } catch (final Exception e) { log.debug("Unable to retrieve current persistent settings for audit", e); } changes.addAll(buildSettingsChanges(persistentSettings, currentPersistent, "persistent")); } Suggestion importance[1-10]: 7 __ Why: This suggestion addresses the same pattern as suggestions 1 and 2, adding explicit null checking for clusterService.state() in the persistent settings retrieval path. While the try-catch handles this, the explicit check is clearer. The test at line 540-555 validates the fallback behavior works correctly.	Medium

---

Previous suggestions

Suggestions up to commit b025aec

Category	Suggestion	Impact
General	Fix misleading operation label for unset settings When newValue is null and the setting is sensitive, the new_value is correctly set to null, but the old_value is still redacted. However, the operation is set to "removed" regardless of whether the key actually existed before. If oldValue is also null (key was never set), logging it as "removed" is misleading. Consider setting operation to "removed" only when oldValue != null, otherwise use "no-op" or skip the entry. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [448-454] if (newValue == null) { change.put("new_value", null); - change.put("operation", "removed"); + change.put("operation", oldValue != null ? "removed" : "no-op"); } else { change.put("new_value", isSensitive ? "***REDACTED***" : newValue); change.put("operation", "set"); } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that setting operation to "removed" when oldValue is also null is semantically misleading. This is a valid correctness concern for audit log accuracy, though it's a minor edge case.	Low
	Assert HTTP status in sensitive setting test The sensitive setting test does not assert the HTTP response status code, unlike all other sub-tests in the same method. If the PUT request fails (e.g., the setting is rejected), the audit log may still be empty and the test could produce a false positive. Add a status code assertion to ensure the request was actually processed. src/test/java/org/opensearch/security/auditlog/integration/SettingsChangeAuditIntegrationTest.java [124-136] // test sensitive setting redaction TestAuditlogImpl.clear(); -rh.executePutRequest( +response = rh.executePutRequest( "_cluster/settings", "{\"persistent\":{\"plugins.security.ssl.transport.keystore_password\":\"mysecret\"}}", encodeBasicHeader("admin", "admin") ); +assertThat(response.getStatusCode(), is(HttpStatus.SC_OK)); Thread.sleep(1500); auditlogs = TestAuditlogImpl.sb.toString(); Assert.assertTrue(auditlogs.contains("CLUSTER_SETTINGS_CHANGED")); Assert.assertTrue(auditlogs.contains("***REDACTED***")); Assert.assertFalse(auditlogs.contains("mysecret")); Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies an inconsistency where the sensitive setting test doesn't capture or assert the HTTP response status, unlike all other sub-tests. This could lead to false positives if the request fails silently, making it a valid test quality improvement.	Low
	Guard settings audit call with type check logSettingsChange is called unconditionally for every allowed request, even those that are not settings-related. While logSettingsChange internally checks the request type and returns early for non-settings requests, this adds unnecessary overhead on every allowed action. Consider guarding the call with an instanceof check at the call site, consistent with how logIndexEvent is typically scoped. src/main/java/org/opensearch/security/filter/SecurityFilter.java [454-460] if (pres.isAllowed()) { auditLog.logGrantedPrivileges(action, request, task); auditLog.logIndexEvent(action, request, task); - auditLog.logSettingsChange(action, request, task); + if (request instanceof ClusterUpdateSettingsRequest || request instanceof UpdateSettingsRequest) { + auditLog.logSettingsChange(action, request, task); + } if (!dlsFlsValve.invoke(context, listener)) { return; } Suggestion importance[1-10]: 4 __ Why: While logSettingsChange already does an internal type check, adding a guard at the call site avoids unnecessary method call overhead on every allowed request. However, the performance impact is minimal and the improved_code introduces imports that aren't shown in the diff context.	Low
	Document multi-index old-value limitation clearly When a request targets multiple indices (e.g., a wildcard pattern resolving to several indices), only the first resolved index is used as the representative current state. This means old values for all other indices are silently dropped, which can produce misleading audit records. Consider either logging per-index changes or documenting this limitation clearly. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [400-411] -// Use settings from the first resolved index as representative current state +// Use settings from the first resolved index as representative current state. +// NOTE: For multi-index requests, old values are only captured from the first resolved index. +// This is a known limitation; per-index old-value capture would require one audit message per index. Settings currentSettings = Settings.EMPTY; if (resolvedIndices.length > 0) { try { final var indexMetadata = clusterService.state().metadata().index(resolvedIndices[0]); if (indexMetadata != null) { currentSettings = indexMetadata.getSettings(); } } catch (final Exception e) { log.debug("Unable to retrieve current index settings for audit", e); } } Suggestion importance[1-10]: 2 __ Why: This suggestion only adds a comment to document a known limitation. While the limitation is real, the suggestion doesn't fix the underlying issue and only adds documentation, which scores low.	Low

Suggestions up to commit 63168c2

Category	Suggestion	Impact
General	Use locale-safe lowercase for sensitive setting detection The pattern fallback uses key.toLowerCase() without specifying a Locale, which can produce unexpected results in locale-sensitive environments (e.g., Turkish locale where "I".toLowerCase() is "ı"). Use key.toLowerCase(Locale.ROOT) to ensure consistent, locale-independent behavior. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [467-479] private boolean isSensitiveSetting(String key) { try { - // Looks up the Setting object by key and checks setting.isSensitive(), - // which returns true for SecureSetting instances — the proper way to identify secrets if (clusterService.getClusterSettings().isSensitiveSetting(key)) { return true; } } catch (Exception e) { // Setting not registered in cluster settings — fall through to pattern match } - // Pattern fallback for settings not registered as SecureSetting (e.g., plugin SSL settings) - final String lowerKey = key.toLowerCase(); + final String lowerKey = key.toLowerCase(java.util.Locale.ROOT); return lowerKey.contains("password") || lowerKey.contains("secret") || lowerKey.contains("token"); Suggestion importance[1-10]: 5 __ Why: Using key.toLowerCase(Locale.ROOT) instead of key.toLowerCase() is a correctness improvement that prevents locale-sensitive bugs (e.g., Turkish locale). This is a well-known Java best practice and the fix is accurate and directly applicable.	Low
	Assert HTTP status in sensitive setting redaction test The sensitive setting test does not assert the HTTP response status code, so it may silently pass even if the request is rejected (e.g., 400 Bad Request for an unrecognized setting). This could cause the audit log to never contain CLUSTER_SETTINGS_CHANGED, making the assertion vacuously pass. Add a status code assertion to ensure the request was actually processed. src/test/java/org/opensearch/security/auditlog/integration/SettingsChangeAuditIntegrationTest.java [124-136] // test sensitive setting redaction TestAuditlogImpl.clear(); -rh.executePutRequest( +HttpResponse sensitiveResponse = rh.executePutRequest( "_cluster/settings", "{\"persistent\":{\"plugins.security.ssl.transport.keystore_password\":\"mysecret\"}}", encodeBasicHeader("admin", "admin") ); +assertThat(sensitiveResponse.getStatusCode(), is(HttpStatus.SC_OK)); Thread.sleep(1500); auditlogs = TestAuditlogImpl.sb.toString(); Assert.assertTrue(auditlogs.contains("CLUSTER_SETTINGS_CHANGED")); Assert.assertTrue(auditlogs.contains("***REDACTED***")); Assert.assertFalse(auditlogs.contains("mysecret")); validateMsgs(TestAuditlogImpl.messages); Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that without asserting the HTTP response status, the test could pass vacuously if the request is rejected. Adding the status assertion strengthens the test's validity and is a straightforward improvement.	Low
	Fix misleading old_value for multi-index settings changes When a request targets multiple indices (e.g., a wildcard pattern), only the first resolved index is used as the representative current state. This means the old_value in the audit log may be incorrect or misleading for all other indices. Consider either logging per-index changes or documenting this limitation clearly, as it can produce inaccurate audit records. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [400-411] -// Use settings from the first resolved index as representative current state +// Build per-index changes for accurate old_value capture Settings currentSettings = Settings.EMPTY; -if (resolvedIndices.length > 0) { +for (String resolvedIndex : resolvedIndices) { try { - final var indexMetadata = clusterService.state().metadata().index(resolvedIndices[0]); + final var indexMetadata = clusterService.state().metadata().index(resolvedIndex); if (indexMetadata != null) { currentSettings = indexMetadata.getSettings(); + break; // Use first available; document that old_value is representative } } catch (final Exception e) { - log.debug("Unable to retrieve current index settings for audit", e); + log.debug("Unable to retrieve current index settings for audit on index {}", resolvedIndex, e); } } +// NOTE: old_value reflects the first resolved index only; may differ for other indices in a multi-index request Suggestion importance[1-10]: 4 __ Why: The suggestion is valid in identifying that only the first resolved index is used for old_value, which can be misleading for multi-index requests. However, the improved_code is functionally equivalent to the existing code (it still uses the first available index), so the actual improvement is minimal and mainly adds a comment.	Low
	Log unhandled request types for better observability The logSettingsChange method silently ignores unknown request types without any logging. If a new settings request type is introduced in the future, it will be silently dropped from audit logs. Adding a debug-level log for unhandled request types would improve observability and help diagnose missing audit entries. src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java [334-338] if (request instanceof ClusterUpdateSettingsRequest) { logClusterSettingsChange(action, (ClusterUpdateSettingsRequest) request, task); } else if (request instanceof UpdateSettingsRequest) { logIndexSettingsChange(action, (UpdateSettingsRequest) request, task); +} else if (request != null) { + log.debug("logSettingsChange: unhandled request type {}, no audit message produced", request.getClass().getName()); } Suggestion importance[1-10]: 4 __ Why: Adding a debug log for unhandled request types in logSettingsChange is a reasonable observability improvement, helping diagnose missing audit entries when new settings request types are introduced. The improvement is minor but valid.	Low

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit befff58.

Path	Line	Severity	Description
src/main/java/org/opensearch/security/support/ConfigConstants.java	228	low	New audit categories CLUSTER_SETTINGS_CHANGED and INDEX_SETTINGS_CHANGED are added to the default disabled transport categories list, meaning settings-change events are silently suppressed unless an administrator explicitly enables them. While consistent with how AUTHENTICATED and GRANTED_PRIVILEGES are handled, cluster/index settings changes are high-value security events and disabling them by default reduces visibility.
src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java	460	low	The isSensitiveSetting() fallback pattern only matches keys containing 'password', 'secret', or 'token'. Settings containing sensitive material under other naming conventions (e.g., 'key', 'cert', 'credential', 'api_key', 'private') would not be redacted and would appear in plaintext in audit logs. This is a security gap in the redaction logic, though it appears unintentional.

The table above displays the top 10 most important findings.

Total: 2 | Critical: 0 | High: 0 | Medium: 0 | Low: 2

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

Persistent review updated to latest commit b025aec

[codecov]

Codecov Report

❌ Patch coverage is 92.98246% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.92%. Comparing base (4d6f34f) to head (befff58).

Files with missing lines	Patch %	Lines
...earch/security/auditlog/impl/AbstractAuditLog.java	93.68%	2 Missing and 4 partials ⚠️
...org/opensearch/security/auditlog/NullAuditLog.java	0.00%	1 Missing ⚠️
...org/opensearch/security/filter/SecurityFilter.java	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6113      +/-   ##
==========================================
+ Coverage   74.83%   74.92%   +0.09%     
==========================================
  Files         447      447              
  Lines       28470    28582     +112     
  Branches     4327     4341      +14     
==========================================
+ Hits        21306    21416     +110     
- Misses       5172     5175       +3     
+ Partials     1992     1991       -1     

Files with missing lines	Coverage Δ
...ava/org/opensearch/security/auditlog/AuditLog.java	100.00% <ø> (ø)
...ensearch/security/auditlog/config/AuditConfig.java	97.16% <ø> (ø)
...ensearch/security/auditlog/impl/AuditCategory.java	100.00% <100.00%> (ø)
...pensearch/security/auditlog/impl/AuditLogImpl.java	90.69% <100.00%> (+0.33%)	⬆️
...pensearch/security/auditlog/impl/AuditMessage.java	81.74% <100.00%> (+0.59%)	⬆️
...nsearch/security/dlic/rest/api/AuditApiAction.java	94.02% <ø> (ø)
...g/opensearch/security/support/ConfigConstants.java	95.65% <100.00%> (+1.20%)	⬆️
...org/opensearch/security/auditlog/NullAuditLog.java	5.00% <0.00%> (-0.27%)	⬇️
...org/opensearch/security/filter/SecurityFilter.java	68.48% <66.66%> (-0.03%)	⬇️
...earch/security/auditlog/impl/AbstractAuditLog.java	77.56% <93.68%> (+3.13%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Persistent review updated to latest commit befff58