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 b025aec)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Core audit log implementation for CLUSTER_SETTINGS_CHANGED and INDEX_SETTINGS_CHANGED Relevant files: src/main/java/org/opensearch/security/auditlog/AuditLog.java src/main/java/org/opensearch/security/auditlog/NullAuditLog.java src/main/java/org/opensearch/security/auditlog/impl/AuditCategory.java src/main/java/org/opensearch/security/auditlog/impl/AuditLogImpl.java src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java src/main/java/org/opensearch/security/filter/SecurityFilter.java Sub-PR theme: Configuration and default category settings for new audit categories Relevant files: src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java src/main/java/org/opensearch/security/support/ConfigConstants.java src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java config/audit.yml src/test/java/org/opensearch/security/auditlog/config/AuditConfigFilterTest.java src/test/java/org/opensearch/security/auditlog/config/AuditConfigSerializeTest.java Sub-PR theme: Tests for settings change audit categories Relevant files: src/test/java/org/opensearch/security/auditlog/impl/SettingsChangeAuditTest.java src/test/java/org/opensearch/security/auditlog/integration/SettingsChangeAuditIntegrationTest.java src/test/java/org/opensearch/security/auditlog/impl/DisabledCategoriesTest.java src/test/java/org/opensearch/security/auditlog/impl/AuditCategoryTest.java

⚡ Recommended focus areas for review Single Index Old Value In logIndexSettingsChange, when a request targets multiple indices (e.g., a wildcard pattern), only the first resolved index is used to capture the old/current settings. This means the audit log may record incorrect or misleading old_value data for all other indices in the request. Consider logging per-index changes or documenting this limitation clearly. // 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(); } } catch (final Exception e) { log.debug("Unable to retrieve current index settings for audit", e); } } Race Condition The cluster state is read before the settings change is applied. While this is intentional for capturing old values, there is a potential race condition in concurrent environments where another settings change could be applied between the state read and the actual update, leading to incorrect old_value entries in the audit log. final Settings persistentSettings = request.persistentSettings(); if (!persistentSettings.isEmpty()) { Settings currentPersistent = Settings.EMPTY; try { currentPersistent = clusterService.state().metadata().persistentSettings(); } catch (final Exception e) { log.debug("Unable to retrieve current persistent settings for audit", e); } changes.addAll(buildSettingsChanges(persistentSettings, currentPersistent, "persistent")); } final Settings transientSettings = request.transientSettings(); if (!transientSettings.isEmpty()) { Settings currentTransient = Settings.EMPTY; try { currentTransient = clusterService.state().metadata().transientSettings(); } catch (final Exception e) { log.debug("Unable to retrieve current transient settings for audit", e); } changes.addAll(buildSettingsChanges(transientSettings, currentTransient, "transient")); Pre-Auth Logging logSettingsChange is called in the admin/pass-through path (line 311) before the request is actually processed by the cluster. If the request subsequently fails (e.g., validation error, node failure), the audit log will still record a settings change that never actually took effect. This could mislead operators reviewing audit logs. if (userIsAdmin || confRequest || internalRequest || passThroughRequest) { if (userIsAdmin && !confRequest && !internalRequest && !passThroughRequest) { auditLog.logGrantedPrivileges(action, request, task); auditLog.logIndexEvent(action, request, task); auditLog.logSettingsChange(action, request, task); } Flaky Sleep The integration tests use Thread.sleep(1500) to wait for audit messages to be processed. This is a fragile approach that can cause flaky tests in slow CI environments or false positives in fast ones. Consider using a polling/await mechanism instead. Thread.sleep(1500); Sensitive Setting Fallback The isSensitiveSetting method catches all exceptions from clusterService.getClusterSettings().isSensitiveSetting(key) silently and falls back to pattern matching. If getClusterSettings() returns null (e.g., during early startup), this will throw a NullPointerException that is silently swallowed, potentially masking issues. Consider explicitly checking for null before calling isSensitiveSetting. 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(); return lowerKey.contains("password") || lowerKey.contains("secret") || lowerKey.contains("token"); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to b025aec
Explore these optional code suggestions:

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

---

Previous suggestions

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 b025aec.

Path	Line	Severity	Description
src/main/java/org/opensearch/security/support/ConfigConstants.java	225	low	The new CLUSTER_SETTINGS_CHANGED and INDEX_SETTINGS_CHANGED audit categories are added to the disabled-by-default transport category list (OPENDISTRO_SECURITY_AUDIT_DISABLED_TRANSPORT_CATEGORIES_DEFAULT), and the sample config/audit.yml also explicitly disables them. This means security-relevant settings changes will not be audited in default deployments. While consistent with the existing pattern of disabling high-volume categories (AUTHENTICATED, GRANTED_PRIVILEGES), settings changes are significant security events; defaulting them to off reduces audit coverage without operator awareness.

The table above displays the top 10 most important findings.

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

---

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.88%. Comparing base (88c407c) to head (b025aec).

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.80%   74.88%   +0.08%     
==========================================
  Files         447      447              
  Lines       28463    28575     +112     
  Branches     4328     4342      +14     
==========================================
+ Hits        21291    21399     +108     
- Misses       5176     5182       +6     
+ Partials     1996     1994       -2     

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% <ø> (+2.98%)	⬆️
...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 10 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.