feat(audit): Implement standalone audit logging for SSL-only mode

[Taiwo435]

Enable audit logging as an independent capability in SSL-only mode (no auth/RBAC required). Users running OpenSearch without full security can now have a complete audit trail for compliance (SOC2, HIPAA, PCI-DSS).

Key features:

• New REQUEST_AUDIT category for all REST/transport actions
• Client certificate identity enrichment (mTLS CN/SAN as effective_user)
• Request body logging with sensitive header exclusion
• Index resolution (wildcards expanded to actual indices)
• Bulk per-item event tracking (configurable granularity)
• Ignore-users and ignore-requests filtering
• Transport-layer interception (shard-level operations)
• Document-level compliance tracking (COMPLIANCE_DOC_WRITE and COMPLIANCE_DOC_READ) via IndexingOperationListener and lightweight ComplianceReadIndexSearcherWrapper
• All sinks supported (internal_opensearch, log4j, webhook, Kafka)
• 17 dynamic settings configurable at runtime via PUT _cluster/settings without node restart

New production files:

• AuditActionFilter: lightweight ActionFilter for non-FGAC modes
• AuditTransportInterceptor: captures inter-node transport traffic
• ComplianceReadIndexSearcherWrapper: field-level read tracking
• SecuritySettings: dynamic Setting constants for cluster settings

New test files:

• AuditActionFilterTest: 11 unit tests
• AuditTransportInterceptorTest: 11 unit tests
• StandaloneAuditLoggingTest: 21 integration tests (basic events)
• StandaloneAuditFilterFeaturesTest: 25 integration tests
• StandaloneAuditComplianceDocTest: 14 integration tests
• StandaloneAuditDynamicFilterSettingsTest: 10 integration tests
• StandaloneAuditDynamicComplianceSettingsTest: 7 integration tests
• StandaloneAuditDynamicConfigTest: 2 integration tests
• StandaloneAuditTransportInterceptTest: 2 integration tests
• StandaloneAuditMtlsTest: 1 integration test
• StandaloneAuditDisabledCategoryTest: 1 integration test
• StandaloneAuditSinksTest: 3 integration tests
• StandaloneAuditWebhookSinkTest: 1 integration test

Total tests: 128 (43 unit + 85 integration)

Description

Category: New feature

Why these changes are required?

Users running OpenSearch in SSL-only mode (TLS without authentication/authorization) currently have zero audit trail. Compliance frameworks (SOC2, HIPAA, PCI-DSS, GDPR) require audit logs even without access
control. This change makes audit logging an independent capability that works without FGAC.

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

• Before: Audit logging only works when full security (FGAC) is active. In SSL-only mode, no audit events are produced regardless of configuration.
• After: In SSL-only mode (plugins.security.ssl_only: true), configuring an audit sink (plugins.security.audit.type: log4j) now enables full audit logging — capturing all REST/transport actions, request
  bodies, client cert identity, compliance doc read/write tracking, with 17 dynamically configurable settings.

Issues Resolved

Resolves #6303

Testing

• Unit tests: 43 tests (AuditActionFilterTest, AuditTransportInterceptorTest, DisabledCategoriesTest, AuditConfigFilterTest)
• Integration tests: 85 tests across 11 test classes covering all features, edge cases, dynamic config toggling, compliance tracking, transport interception, identity, sinks
• Manual testing: Docker cluster (SSL-only mode, log4j sink) verified all phases — basic events, identity (mTLS), index resolution, bulk per-item, ignore patterns, compliance doc write/read, dynamic config
  toggle, transport intercept

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• 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 Code Analyzer ❗

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

Path	Line	Severity	Description
	1013	medium	All incoming REST headers including Authorization, Cookie, and custom auth tokens are stored verbatim into ThreadContext under SECURITY_AUDIT_REST_HEADERS before filtering. AuditHeaderUtils.filterHeaders only strips 'Authorization' when exclude_sensitive_headers=true; other sensitive headers (e.g. Cookie, X-Auth-Token) pass through to the audit sink unfiltered.
	76	low	The self-loop guard relies on a static prefix extracted at startup from the index pattern. If the audit index uses a date-based Joda pattern without single-quote delimiters (e.g. audit-YYYY.MM.dd), the prefix extraction falls back to the raw pattern string which will never match resolved index names, disabling the self-loop protection entirely.
	447	low	setWatchedReadFields references this.readEnabledFieldsCache.invalidateAll() but readEnabledFieldsCache is declared volatile and initialized in the constructor; if called concurrently before the constructor completes (e.g. via a cluster settings update racing with initialization), a NullPointerException would result.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 0 | Medium: 1 | 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]

PR Reviewer Guide 🔍

(Review updated until commit f29b3aa)

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: Compliance doc-level tracking (read wrapper + dynamic compliance settings) Relevant files: src/main/java/org/opensearch/security/compliance/ComplianceConfig.java src/main/java/org/opensearch/security/compliance/ComplianceReadIndexSearcherWrapper.java src/integrationTest/java/org/opensearch/security/StandaloneAuditComplianceDocTest.java src/integrationTest/java/org/opensearch/security/StandaloneAuditDynamicComplianceSettingsTest.java Sub-PR theme: Transport-layer audit interceptor Relevant files: src/main/java/org/opensearch/security/filter/AuditTransportInterceptor.java src/test/java/org/opensearch/security/filter/AuditTransportInterceptorTest.java src/integrationTest/java/org/opensearch/security/StandaloneAuditTransportInterceptTest.java Sub-PR theme: Core AuditActionFilter for SSL-only mode Relevant files: src/main/java/org/opensearch/security/filter/AuditActionFilter.java src/main/java/org/opensearch/security/filter/AuditHeaderUtils.java src/test/java/org/opensearch/security/filter/AuditActionFilterTest.java src/integrationTest/java/org/opensearch/security/StandaloneAuditLoggingTest.java src/integrationTest/java/org/opensearch/security/StandaloneAuditFilterFeaturesTest.java

⚡ Recommended focus areas for review Possible NPE setWatchedReadFields calls this.readEnabledFieldsCache.invalidateAll(), but readEnabledFieldsCache is declared as a field with no visible initializer in the diff. If it is initialized lazily or only in some code paths, invoking this setter (e.g. via the dynamic cluster settings update consumer registered in OpenSearchSecurityPlugin.initStandaloneAuditIfEnabled for COMPLIANCE_READ_WATCHED_FIELDS) before the cache is created will throw a NullPointerException and prevent the setting from being applied. Please confirm the cache is always non-null at setter invocation time or guard the invalidateAll call. public void setWatchedReadFields(List<String> readFields) { // Single-pass: parse format "indexpattern,field1,field2,..." and build both maps simultaneously final Map<String, List<String>> parsed = new HashMap<>(readFields.size()); final Map<WildcardMatcher, Set<String>> enabled = new HashMap<>(readFields.size()); for (final String entry : readFields) { final String[] split = entry.split(","); if (split.length == 0 || split[0].isEmpty()) { continue; } final List<String> fields = split.length == 1 ? List.of("*") : List.of(Arrays.copyOfRange(split, 1, split.length)); parsed.put(split[0], fields); enabled.put(WildcardMatcher.from(split[0]), Set.copyOf(fields)); } // Atomic swap — readers always see a consistent pair of maps this.watchedReadFields = Map.copyOf(parsed); this.watchedReadState = new WatchedReadState(Map.copyOf(parsed), Map.copyOf(enabled)); this.readEnabledFieldsCache.invalidateAll(); } Fragile prefix extraction getAuditIndexPrefix returns the raw pattern string when the configured index setting has no Joda-style quoted literal (e.g. audit-YYYY.MM.dd or a plain name), and this value is passed to AuditActionFilter/AuditTransportInterceptor as the self-loop-guard prefix. In that case the guard prefix will never match the actual resolved index names (which contain the substituted date), so audit writes to the audit index itself will not be suppressed and can potentially cause a self-loop of audit events. The test testGetAuditIndexPrefixNoQuotesNoDatastreamReturnsRawPattern acknowledges this as a limitation — consider detecting unquoted date tokens and either extracting the literal prefix up to the first date pattern character or logging a WARN so operators know the guard is effectively disabled. public static String getAuditIndexPrefix(Settings settings) { String indexPattern = settings.get( ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, "'security-auditlog-'YYYY.MM.dd" ); // Extract literal prefix from Joda pattern: text between first pair of single quotes if (indexPattern.startsWith("'")) { int endQuote = indexPattern.indexOf("'", 1); if (endQuote > 1) { return indexPattern.substring(1, endQuote); } } // No quotes — check if it's the datastream name setting String dataStreamName = settings.get( ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_DATASTREAM_NAME, null ); if (dataStreamName != null) { return dataStreamName; } // Fallback: use the raw pattern as prefix (handles plain index names) return indexPattern; } SSL principal extraction cost The new REST handler wrapper calls engine.getSession().getPeerCertificates() on every HTTP request when needClientAuth/wantClientAuth is set, and constructs a new DefaultPrincipalExtractor() per request. getPeerCertificates() can be relatively expensive and throws SSLPeerUnverifiedException when the client didn't present a cert (common with OPTIONAL/wantClientAuth), meaning every unauthenticated request hits the catch block. The rate-limited warn helps, but consider fast-pathing when the session has no peer principal (check getSession().getPeerPrincipal() availability first) and reusing a single PrincipalExtractor instance to avoid per-request allocation on the hot path. // Extract client cert principal (CN/SAN) when mTLS is configured SslHandler sslHandler = request.getHttpChannel().get("ssl_http", SslHandler.class).orElse(null); if (sslHandler != null) { SSLEngine engine = sslHandler.engine(); if (engine.getNeedClientAuth() || engine.getWantClientAuth()) { try { Certificate[] certs = engine.getSession().getPeerCertificates(); if (certs != null && certs.length > 0 && certs[0] instanceof X509Certificate) { String principal = new DefaultPrincipalExtractor().extractPrincipal( (X509Certificate) certs[0], PrincipalExtractor.Type.HTTP ); if (principal != null) { threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL, principal); } } } catch (Exception e) { long now = System.currentTimeMillis(); if (now - lastCertWarnTime.get() > CERT_WARN_INTERVAL_MS) { lastCertWarnTime.set(now); log.warn( "Failed to extract client certificate identity: {}. " + "Audit events will have no effective_user. " + "Enable DEBUG logging for full stack trace.", e.getMessage() ); } else if (log.isDebugEnabled()) { log.debug("Failed to extract client certificate identity", e); } } } }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f29b3aa

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Restrict compliance branch to standalone modes This else if branch will register the compliance listener and reader wrapper for the normal FGAC mode as well (since !disabled && !client is true there when audit is configured), causing a duplicate reader wrapper registration and conflicting with the FGAC branch's own setReaderWrapper call above. Restrict this branch to standalone modes only by adding SSLConfig.isSslOnlyMode() (or equivalent gating) so it only applies to non-FGAC modes. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1196-1207] -} else if (!disabled && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { - // Non-FGAC mode: register compliance listener for standalone audit +} else if (SSLConfig.isSslOnlyMode() && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { + // Standalone (SSL-only) mode: register compliance listener for standalone audit final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog, threadPool); indexModule.addIndexOperationListener(ciol); // Compliance read tracking — lightweight reader wrapper (no DLS/FLS) // Also sets IndexService on ciol so write_log_diffs can retrieve original docs indexModule.setReaderWrapper(indexService -> { ciol.setIs(indexService); return new ComplianceReadIndexSearcherWrapper(indexService, threadPool, cs, auditLog); }); } Suggestion importance[1-10]: 8 __ Why: Valid concern: the else if (!disabled && !client ...) branch could conflict with FGAC mode which already sets its own reader wrapper. However, in FGAC mode auditLog may still be non-null, so this could cause a duplicate setReaderWrapper call. Important correctness issue worth investigating.	Medium
	Guard against null cache during invalidation The readEnabledFieldsCache field is declared but never initialized in the visible constructor path, so calling invalidateAll() on it will throw a NullPointerException when the setter is invoked. Add a null-check before invalidating, or ensure the cache is initialized before this setter can be called. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [442] public void setWatchedReadFields(List<String> readFields) { // Single-pass: parse format "indexpattern,field1,field2,..." and build both maps simultaneously final Map<String, List<String>> parsed = new HashMap<>(readFields.size()); final Map<WildcardMatcher, Set<String>> enabled = new HashMap<>(readFields.size()); for (final String entry : readFields) { final String[] split = entry.split(","); if (split.length == 0 || split[0].isEmpty()) { continue; } final List<String> fields = split.length == 1 ? List.of("*") : List.of(Arrays.copyOfRange(split, 1, split.length)); parsed.put(split[0], fields); enabled.put(WildcardMatcher.from(split[0]), Set.copyOf(fields)); } // Atomic swap — readers always see a consistent pair of maps this.watchedReadFields = Map.copyOf(parsed); this.watchedReadState = new WatchedReadState(Map.copyOf(parsed), Map.copyOf(enabled)); - this.readEnabledFieldsCache.invalidateAll(); + if (this.readEnabledFieldsCache != null) { + this.readEnabledFieldsCache.invalidateAll(); + } } Suggestion importance[1-10]: 8 __ Why: The readEnabledFieldsCache field is declared volatile without initialization in the visible constructor, and calling invalidateAll() on a null reference would cause an NPE when the dynamic setter is triggered. This is a valid correctness concern.	Medium
	Null-check bulk item requests before logging BulkItemRequest.request() can return null (e.g., for items that have already been processed/consumed), and item itself may also be null in some cases. Add a null check on the inner request before dereferencing it inside logBulkItem, since innerRequest.index(), innerRequest.getClass(), and innerRequest.id() will otherwise throw NPE. src/main/java/org/opensearch/security/filter/AuditActionFilter.java [183-193] if (request instanceof BulkShardRequest) { BulkShardRequest bulkShardRequest = (BulkShardRequest) request; for (BulkItemRequest item : bulkShardRequest.items()) { + if (item == null || item.request() == null) { + continue; + } logBulkItem( item.request(), action, task, effectiveUser, remoteAddress, filteredHeaders, bulkShardRequest.shardId() ); } Suggestion importance[1-10]: 5 __ Why: Adding null-checks for BulkItemRequest items adds defensive protection, but the outer try/catch already handles unexpected NPEs, so the impact is moderate.	Low
General	Respect transport audit enabled flag This code also does not check filter.isTransportApiAuditEnabled(), so the transport interceptor will emit events even when transport auditing has been explicitly disabled via the enable_transport setting. Add a check for filter.isTransportApiAuditEnabled() to respect this setting. src/main/java/org/opensearch/security/filter/AuditTransportInterceptor.java [102-105] // Skip if TRANSPORT_AUDIT is disabled -if (!filter.getDisabledTransportCategories().contains(AuditCategory.TRANSPORT_AUDIT) +if (filter.isTransportApiAuditEnabled() + && !filter.getDisabledTransportCategories().contains(AuditCategory.TRANSPORT_AUDIT) && !filter.getDisabledCategories().contains(AuditCategory.TRANSPORT_AUDIT)) { // Skip ignored requests (action or class name) if (!filter.isRequestAuditDisabled(action) && !filter.isRequestAuditDisabled(request.getClass().getSimpleName())) { Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the transport interceptor ignores the isTransportApiAuditEnabled() setting, which could cause transport audit events to fire even when transport audit is disabled. This is a valid functional gap.	Medium
	Align action filter and interceptor conditions The condition here is only reached when client || disabled || SSLConfig.isSslOnlyMode() is true (due to the preceding if), but as written it can also register the AuditActionFilter in the disabled mode where the plugin performs no other action wiring—this may be intentional, however combined with the transport interceptor gate below (which explicitly excludes disabled), the behavior is inconsistent. Align these two blocks so audit filter and transport interceptor are registered under the same conditions to avoid partial audit coverage. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1218-1229] -} else if (!client && auditLog != null && !(auditLog instanceof NullAuditLog)) { +} else if (!client && (disabled || SSLConfig.isSslOnlyMode()) && auditLog != null && !(auditLog instanceof NullAuditLog)) { filters.add( new AuditActionFilter( auditLog, cs, threadPool, ((AbstractAuditLog) auditLog).getFilter(), new IndexNameExpressionResolver(threadPool.getThreadContext()), getAuditIndexPrefix(pluginSettings) ) ); } Suggestion importance[1-10]: 6 __ Why: Reasonable observation about inconsistency between the action filter registration and the transport interceptor gating. Aligning them would improve audit coverage consistency, though the current asymmetry may be intentional.	Low
	Prioritize datastream name in prefix extraction When the index setting is explicitly configured to a raw Joda pattern without quotes (e.g., audit-YYYY.MM.dd), this returns the literal string "audit-YYYY.MM.dd" as the prefix, which will never match resolved index names like audit-2026.01.01, so the self-loop guard silently breaks. Also, when the datastream name is set, its value should be checked first regardless of the index pattern configuration since datastreams take precedence. Consider prioritizing the datastream name setting and logging a warning when the index pattern is a raw (unquoted) Joda pattern. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [500-525] public static String getAuditIndexPrefix(Settings settings) { - String indexPattern = settings.get( - ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, - "'security-auditlog-'YYYY.MM.dd" - ); - - // Extract literal prefix from Joda pattern: text between first pair of single quotes - if (indexPattern.startsWith("'")) { - int endQuote = indexPattern.indexOf("'", 1); - if (endQuote > 1) { - return indexPattern.substring(1, endQuote); - } - } - - // No quotes — check if it's the datastream name setting + // Check datastream name first — it takes precedence when configured String dataStreamName = settings.get( ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_DATASTREAM_NAME, null ); if (dataStreamName != null) { return dataStreamName; } - // Fallback: use the raw pattern as prefix (handles plain index names) + String indexPattern = settings.get( + ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, + "'security-auditlog-'YYYY.MM.dd" + ); + + if (indexPattern.startsWith("'")) { + int endQuote = indexPattern.indexOf("'", 1); + if (endQuote > 1) { + return indexPattern.substring(1, endQuote); + } + } + return indexPattern; } Suggestion importance[1-10]: 6 __ Why: Reasonable suggestion to reorder datastream check first, since datastream name and index pattern can both be set. Also correctly identifies the limitation of raw unquoted Joda patterns which the PR acknowledges in tests.	Low
	Handle missing client cert separately getPeerCertificates() throws SSLPeerUnverifiedException when the client did not present a certificate (common in wantClientAuth mode). The current catch block treats this as a warn-worthy error and rate-limits, but every unauthenticated request will still hit the catch path and produce debug logs. Explicitly handle SSLPeerUnverifiedException as a benign case (no client cert presented) and only warn on other exceptions to avoid noise and misleading warnings. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [971-976] SslHandler sslHandler = request.getHttpChannel().get("ssl_http", SslHandler.class).orElse(null); if (sslHandler != null) { SSLEngine engine = sslHandler.engine(); if (engine.getNeedClientAuth() || engine.getWantClientAuth()) { try { Certificate[] certs = engine.getSession().getPeerCertificates(); + // ... existing extraction logic + } catch (javax.net.ssl.SSLPeerUnverifiedException peerUnverified) { + // No client cert presented — benign in wantClientAuth mode + if (log.isDebugEnabled()) { + log.debug("No client certificate presented: {}", peerUnverified.getMessage()); + } + } catch (Exception e) { + // existing rate-limited warn path + } + } +} Suggestion importance[1-10]: 5 __ Why: Valid point about SSLPeerUnverifiedException being a benign case in wantClientAuth mode. Handling it separately would reduce misleading warn logs, though the existing rate-limiting mitigates the noise.	Low
	Defensively copy mutable constructor inputs watchedReadFields and watchedWriteIndicesPatterns are now volatile mutable fields but the constructor stores the caller-provided references directly. If the caller mutates the original collection later, readers would see inconsistent state. Store defensive copies to preserve the immutability guarantee. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [149] -this.watchedReadFields = watchedReadFields; +this.watchedReadFields = Map.copyOf(watchedReadFields); this.watchedWriteIndicesMatcher = WildcardMatcher.from(watchedWriteIndicesPatterns); Suggestion importance[1-10]: 4 __ Why: Defensive copies improve safety for mutable volatile fields, but since these fields are overwritten atomically via dynamic setters, the risk of external mutation is relatively low.	Low

---

Previous suggestions

Suggestions up to commit f2125ed

Category	Suggestion	Impact
Possible issue	Fix inverted disabled-category suppression logic The condition combining REST-disabled and generic-disabled transport categories uses &&, meaning the event is skipped only when both sets contain TRANSPORT_AUDIT. This is inverted logic — a category should be suppressed if EITHER set contains it. Change && to || so disabling via either category set actually suppresses transport audit events. src/main/java/org/opensearch/security/filter/AuditTransportInterceptor.java [102-114] +// Skip if TRANSPORT_AUDIT is disabled in either the general or transport-specific list +if (filter.getDisabledTransportCategories().contains(AuditCategory.TRANSPORT_AUDIT) + || filter.getDisabledCategories().contains(AuditCategory.TRANSPORT_AUDIT)) { + actualHandler.messageReceived(request, channel, task); + return; +} // Skip ignored requests (action or class name) if (!filter.isRequestAuditDisabled(action) && !filter.isRequestAuditDisabled(request.getClass().getSimpleName())) { - // Skip ignored users String principal = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL); User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); String effectiveUser = user != null ? user.getName() : principal; if (effectiveUser == null || !filter.isAuditDisabled(effectiveUser)) { logTransportEvent(action, request, task); } } Suggestion importance[1-10]: 9 __ Why: The existing code uses !A && !B to gate logging, meaning logging proceeds unless both are disabled. This is inverted — disabling TRANSPORT_AUDIT in either category set should suppress events. This is a clear correctness bug.	High
	Guard cache invalidation against null readEnabledFieldsCache is declared but never initialized in the shown code, so calling invalidateAll() here will throw a NullPointerException when the setter is invoked dynamically. Guard the invalidation against a null cache to avoid crashing the settings-update consumer. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [426-443] public void setWatchedReadFields(List<String> readFields) { // Single-pass: parse format "indexpattern,field1,field2,..." and build both maps simultaneously final Map<String, List<String>> parsed = new HashMap<>(readFields.size()); final Map<WildcardMatcher, Set<String>> enabled = new HashMap<>(readFields.size()); for (final String entry : readFields) { final String[] split = entry.split(","); if (split.length == 0 || split[0].isEmpty()) { continue; } final List<String> fields = split.length == 1 ? List.of("*") : List.of(Arrays.copyOfRange(split, 1, split.length)); parsed.put(split[0], fields); enabled.put(WildcardMatcher.from(split[0]), Set.copyOf(fields)); } // Atomic swap — readers always see a consistent pair of maps this.watchedReadFields = Map.copyOf(parsed); this.watchedReadState = new WatchedReadState(Map.copyOf(parsed), Map.copyOf(enabled)); - this.readEnabledFieldsCache.invalidateAll(); + if (this.readEnabledFieldsCache != null) { + this.readEnabledFieldsCache.invalidateAll(); + } } Suggestion importance[1-10]: 8 __ Why: The readEnabledFieldsCache field is declared as volatile but there's no visible initialization in the diff. If it remains null, invoking setWatchedReadFields dynamically would trigger an NPE. The null-guard is a safe defensive fix.	Medium
	Restrict compliance branch to non-FGAC modes This else if branch runs whenever standalone audit is enabled, including in FGAC mode where a full security stack is bootstrapped. The onIndexModule callback fires for every index, and the primary if branch (which handles FGAC compliance) may not be taken when SSL-only mode is off but audit is disabled. Verify that this branch is only intended for non-FGAC modes — the comment says so, but the condition !disabled && !client alone is true in FGAC too. Consider adding (disabled || SSLConfig.isSslOnlyMode()) to the guard to avoid double-registering compliance listeners in FGAC mode. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1196-1207] -} else if (!disabled && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { +} else if ((disabled || SSLConfig.isSslOnlyMode()) && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { // Non-FGAC mode: register compliance listener for standalone audit final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog, threadPool); indexModule.addIndexOperationListener(ciol); - // Compliance read tracking — lightweight reader wrapper (no DLS/FLS) - // Also sets IndexService on ciol so write_log_diffs can retrieve original docs indexModule.setReaderWrapper(indexService -> { ciol.setIs(indexService); return new ComplianceReadIndexSearcherWrapper(indexService, threadPool, cs, auditLog); }); } Suggestion importance[1-10]: 5 __ Why: The else if follows an if block (not shown fully) that likely handles FGAC. Given the branch is else if, it only fires when the first condition is false. The suggestion may be redundant depending on the primary if condition, but adding an explicit mode check improves clarity and safety.	Low
	Guard AuditActionFilter to non-FGAC modes The else if guard !client && auditLog != null && !(auditLog instanceof NullAuditLog) will match FGAC mode when auditLog is initialized as a real AuditLogImpl, because the primary if requires !SSLConfig.isSslOnlyMode() and both conditions could theoretically be reached in some initialization orderings. Since this branch is intended only for non-FGAC (SSL-only or disabled) modes, add an explicit mode check to prevent registering AuditActionFilter twice or in the wrong mode. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1218-1229] -} else if (!client && auditLog != null && !(auditLog instanceof NullAuditLog)) { +} else if (!client && (disabled || SSLConfig.isSslOnlyMode()) && auditLog != null && !(auditLog instanceof NullAuditLog)) { filters.add( new AuditActionFilter( auditLog, cs, threadPool, ((AbstractAuditLog) auditLog).getFilter(), new IndexNameExpressionResolver(threadPool.getThreadContext()), getAuditIndexPrefix(pluginSettings) ) ); } Suggestion importance[1-10]: 4 __ Why: Since this is an else if after !client && !disabled && !SSLConfig.isSslOnlyMode(), the branch only fires when at least one non-FGAC condition applies. The added explicit guard is defensive but likely redundant given the existing else-if structure.	Low
General	Prefer datastream name over index pattern The datastream name should be preferred first when audit type is internal_opensearch and a datastream is configured, regardless of the index pattern. Currently, if index is set to any value with quotes, the datastream name is ignored, which can cause the self-loop guard to use the wrong prefix and either miss the audit index writes or incorrectly suppress unrelated indices. Consider checking the datastream setting first. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [500-525] public static String getAuditIndexPrefix(Settings settings) { + // Prefer datastream name when configured — it takes precedence over index pattern for datastream sinks + String dataStreamName = settings.get( + ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_DATASTREAM_NAME, + null + ); + if (dataStreamName != null && !dataStreamName.isEmpty()) { + return dataStreamName; + } + String indexPattern = settings.get( ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, "'security-auditlog-'YYYY.MM.dd" ); // Extract literal prefix from Joda pattern: text between first pair of single quotes if (indexPattern.startsWith("'")) { int endQuote = indexPattern.indexOf("'", 1); if (endQuote > 1) { return indexPattern.substring(1, endQuote); } } - // No quotes — check if it's the datastream name setting - String dataStreamName = settings.get( - ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_DATASTREAM_NAME, - null - ); - if (dataStreamName != null) { - return dataStreamName; - } - // Fallback: use the raw pattern as prefix (handles plain index names) return indexPattern; } Suggestion importance[1-10]: 6 __ Why: Reasonable improvement: if a datastream name is configured, it should take precedence for the self-loop guard prefix. The current code only falls back to it when no quotes are present in the index pattern, which may miss valid datastream configurations.	Low
	Handle missing optional client cert quietly When getWantClientAuth() is true but the client did not present a certificate, getPeerCertificates() throws SSLPeerUnverifiedException on every request, triggering the throttled warn log path continuously. Consider catching SSLPeerUnverifiedException separately and treating it as a non-error case (no client cert presented) rather than logging a warning, since this is expected behavior when client auth is optional. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [973-977] SSLEngine engine = sslHandler.engine(); if (engine.getNeedClientAuth() || engine.getWantClientAuth()) { try { Certificate[] certs = engine.getSession().getPeerCertificates(); if (certs != null && certs.length > 0 && certs[0] instanceof X509Certificate) { + // ... existing logic + } + } catch (javax.net.ssl.SSLPeerUnverifiedException peerUnverified) { + // Expected when clientauth_mode=OPTIONAL and client didn't send a cert — no identity to extract + if (log.isDebugEnabled()) { + log.debug("No client certificate presented (clientauth_mode=OPTIONAL)"); + } + } catch (Exception e) { Suggestion importance[1-10]: 6 __ Why: Valid concern: when wantClientAuth is true but no cert is presented, SSLPeerUnverifiedException is thrown on every request and triggers unnecessary warn logs. Catching it separately as debug is a reasonable UX improvement.	Low
	Avoid duplicated mutable state fields watchedReadFields is now both a standalone volatile field and duplicated inside watchedReadState. Keeping two authoritative copies risks divergence: setWatchedReadFields updates both, but the private constructor only updates watchedReadFields directly (line assigning this.watchedReadFields = watchedReadFields;) while watchedReadState holds a Map.copyOf. Any JSON serialization path or getter that reads watchedReadFields alongside watchedReadState may show inconsistent data. Consider deriving watchedReadFields from watchedReadState (single source of truth) or documenting the duplication invariant explicitly. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [91-92] @JsonProperty("read_watched_fields") -private volatile Map<String, List<String>> watchedReadFields; +public Map<String, List<String>> getWatchedReadFields() { + return watchedReadState.watchedReadFields; +} Suggestion importance[1-10]: 5 __ Why: Valid observation about duplicated state that could diverge, though the improved code changes the field to a getter which alters serialization semantics and may not compile as-is. Still, flagging the maintainability concern is worthwhile.	Low

Suggestions up to commit 3d2d12d

Category	Suggestion	Impact
Possible issue	Guard cache invalidation against null readEnabledFieldsCache is declared but never initialized in the diff, and calling invalidateAll() on it will throw an NPE when the dynamic setter is invoked. Guard the call with a null check, or ensure the cache is initialized in the constructor before this setter can run. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [426-443] public void setWatchedReadFields(List<String> readFields) { // Single-pass: parse format "indexpattern,field1,field2,..." and build both maps simultaneously final Map<String, List<String>> parsed = new HashMap<>(readFields.size()); final Map<WildcardMatcher, Set<String>> enabled = new HashMap<>(readFields.size()); for (final String entry : readFields) { final String[] split = entry.split(","); if (split.length == 0 || split[0].isEmpty()) { continue; } final List<String> fields = split.length == 1 ? List.of("*") : List.of(Arrays.copyOfRange(split, 1, split.length)); parsed.put(split[0], fields); enabled.put(WildcardMatcher.from(split[0]), Set.copyOf(fields)); } // Atomic swap — readers always see a consistent pair of maps this.watchedReadFields = Map.copyOf(parsed); this.watchedReadState = new WatchedReadState(Map.copyOf(parsed), Map.copyOf(enabled)); - this.readEnabledFieldsCache.invalidateAll(); + if (this.readEnabledFieldsCache != null) { + this.readEnabledFieldsCache.invalidateAll(); + } } Suggestion importance[1-10]: 8 __ Why: The readEnabledFieldsCache field is declared volatile but shown initialization is not visible in the diff; if it hasn't been initialized when setWatchedReadFields is invoked dynamically, invalidateAll() will throw NPE. Adding a null check is a reasonable defensive fix.	Medium
	Ensure compliance listener registers in SSL-only mode This branch registers the compliance listener/reader wrapper whenever a non-null audit log exists, but it triggers only in FGAC mode (the outer method's earlier branches gate on !SSLConfig.isSslOnlyMode()). In SSL-only mode, onIndexModule is not reached via that path, so the standalone compliance write/read tracking will never be wired up. Ensure this branch is also invoked for SSL-only mode (e.g., add an SSLConfig.isSslOnlyMode() clause) so ComplianceIndexingOperationListenerImpl and ComplianceReadIndexSearcherWrapper are actually registered as advertised by the tests. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1196-1207] -} else if (!disabled && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { - // Non-FGAC mode: register compliance listener for standalone audit +} else if (!client && auditLog != null && !(auditLog instanceof NullAuditLog) && (SSLConfig.isSslOnlyMode() || disabled == false)) { + // Non-FGAC / SSL-only / disabled modes: register compliance listener for standalone audit final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog, threadPool); indexModule.addIndexOperationListener(ciol); - // Compliance read tracking — lightweight reader wrapper (no DLS/FLS) - // Also sets IndexService on ciol so write_log_diffs can retrieve original docs indexModule.setReaderWrapper(indexService -> { ciol.setIs(indexService); return new ComplianceReadIndexSearcherWrapper(indexService, threadPool, cs, auditLog); }); } Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about whether the compliance listener is registered in SSL-only mode. However, without seeing the full onIndexModule context, it's unclear if the outer branches actually gate on !isSslOnlyMode(). The integration tests for compliance in SSL-only mode suggest the current code path works, so the concern may not be accurate.	Low
Security	Prevent audit bypass via mixed-index requests Skipping the entire request when any single target index matches the audit-index prefix allows an attacker (or misbehaving client) to bypass audit by simply including the audit index name alongside other indices in a multi-index request. Only skip when all indices are audit indices, or filter out only the audit indices from the logged set. src/main/java/org/opensearch/security/filter/AuditActionFilter.java [134-144] // Skip requests targeting the audit index (prevent self-referential loop) if (request instanceof IndicesRequest) { String[] indices = ((IndicesRequest) request).indices(); - if (indices != null) { + if (indices != null && indices.length > 0) { + boolean allAudit = true; for (String idx : indices) { - if (isAuditIndex(idx)) { - chain.proceed(task, action, request, listener); - return; + if (!isAuditIndex(idx)) { + allAudit = false; + break; } + } + if (allAudit) { + chain.proceed(task, action, request, listener); + return; } } } Suggestion importance[1-10]: 8 __ Why: Valid security concern: skipping audit when any single index matches the audit-index prefix allows bypass via mixed-index requests. Changing to require all indices to be audit indices closes this gap.	Medium
General	Handle unverified peer cert case separately SSLEngine.getSession().getPeerCertificates() throws SSLPeerUnverifiedException whenever the peer did not present a certificate (common when getWantClientAuth() is true but the client chose not to send one). The current catch (Exception e) block will treat every such request as an extraction failure and emit warn logs, adding noise. Distinguish the "no cert provided" case (log at debug/trace once) from actual extraction errors to avoid log spam on every anonymous request in WANT client-auth mode. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [971-976] SslHandler sslHandler = request.getHttpChannel().get("ssl_http", SslHandler.class).orElse(null); if (sslHandler != null) { SSLEngine engine = sslHandler.engine(); if (engine.getNeedClientAuth() || engine.getWantClientAuth()) { try { Certificate[] certs = engine.getSession().getPeerCertificates(); + // ... existing handling ... + } catch (javax.net.ssl.SSLPeerUnverifiedException peerUnverified) { + if (log.isDebugEnabled()) { + log.debug("No client certificate presented; audit effective_user will be empty"); + } + } catch (Exception e) { + // existing warn/throttle logic + } + } +} Suggestion importance[1-10]: 7 __ Why: Valid concern: SSLPeerUnverifiedException is expected in WANT client-auth mode when no cert is presented, and the current broad catch will log warnings for every anonymous request. Distinguishing this case avoids log noise while preserving actual error visibility.	Medium
	Harden audit index prefix extraction logic When a user sets a custom index pattern that contains a Joda date literal without quotes (e.g., audit-YYYY.MM.dd), this method returns the raw pattern which will never match resolved index names — silently disabling the self-loop guard and causing infinite audit recursion. Additionally, the datastream fallback is only consulted when the pattern has no quotes at all, ignoring cases where the datastream name is configured alongside a normal index pattern. Consider checking the datastream setting first when it is explicitly set, and log a warning when the prefix cannot be reliably derived so operators are aware the self-loop guard may be ineffective. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [500-525] public static String getAuditIndexPrefix(Settings settings) { + // Prefer explicit datastream name if configured + String dataStreamName = settings.get( + ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_DATASTREAM_NAME, + null + ); + if (dataStreamName != null && !dataStreamName.isEmpty()) { + return dataStreamName; + } + String indexPattern = settings.get( ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, "'security-auditlog-'YYYY.MM.dd" ); - // Extract literal prefix from Joda pattern: text between first pair of single quotes if (indexPattern.startsWith("'")) { int endQuote = indexPattern.indexOf("'", 1); if (endQuote > 1) { return indexPattern.substring(1, endQuote); } } - // No quotes — check if it's the datastream name setting - String dataStreamName = settings.get( - ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_DATASTREAM_NAME, - null - ); - if (dataStreamName != null) { - return dataStreamName; - } - - // Fallback: use the raw pattern as prefix (handles plain index names) + log.warn("Could not derive a reliable audit index prefix from '{}'; self-loop guard may be ineffective.", indexPattern); return indexPattern; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that unquoted Joda patterns fall back to the raw pattern which won't match resolved index names, potentially disabling the self-loop guard. Adding a warn log and prioritizing datastream name are reasonable improvements, though the existing test testGetAuditIndexPrefixNoQuotesNoDatastreamReturnsRawPattern acknowledges this as a known limitation.	Low
	Reuse shared IndexNameExpressionResolver instance Creating a new IndexNameExpressionResolver here (and again in initStandaloneAuditIfEnabled) with the current ThreadContext is redundant and potentially incorrect — the resolver should ideally be the one OpenSearch injects/shares. If a resolver instance is available (e.g., via createComponents), reuse it; otherwise, at least ensure a single instance is created once per plugin lifecycle to avoid subtle inconsistencies between the filter and other audit components. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1218-1228] -if (!client && !disabled && !SSLConfig.isSslOnlyMode()) { - filters.add(Objects.requireNonNull(sf)); - - // !(auditLog instanceof NullAuditLog) prevents registering AuditActionFilter when there's no real sink to send events to. No - // point intercepting every request just to discard the message. } else if (!client && auditLog != null && !(auditLog instanceof NullAuditLog)) { filters.add( new AuditActionFilter( auditLog, cs, threadPool, ((AbstractAuditLog) auditLog).getFilter(), - new IndexNameExpressionResolver(threadPool.getThreadContext()), + this.indexNameExpressionResolver, // reuse the shared resolver instance getAuditIndexPrefix(pluginSettings) ) ); } Suggestion importance[1-10]: 4 __ Why: Reasonable minor improvement to avoid redundant resolver instances, but creating a new IndexNameExpressionResolver with the same ThreadContext should behave equivalently, so the impact is low.	Low
	Handle duplicate index patterns explicitly Using HashMap initial capacity equal to readFields.size() can trigger a resize once the load factor is exceeded; more importantly, if the input list contains duplicate index patterns, later entries silently overwrite earlier ones. Consider logging or merging on duplicate keys to avoid silent data loss when compliance policies are dynamically updated. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [426-438] public void setWatchedReadFields(List<String> readFields) { // Single-pass: parse format "indexpattern,field1,field2,..." and build both maps simultaneously final Map<String, List<String>> parsed = new HashMap<>(readFields.size()); final Map<WildcardMatcher, Set<String>> enabled = new HashMap<>(readFields.size()); for (final String entry : readFields) { final String[] split = entry.split(","); if (split.length == 0 || split[0].isEmpty()) { continue; } final List<String> fields = split.length == 1 ? List.of("*") : List.of(Arrays.copyOfRange(split, 1, split.length)); - parsed.put(split[0], fields); + if (parsed.putIfAbsent(split[0], fields) != null) { + continue; + } enabled.put(WildcardMatcher.from(split[0]), Set.copyOf(fields)); } Suggestion importance[1-10]: 3 __ Why: Duplicate keys causing silent overwrites is a minor concern; the suggested fix (skip duplicates) doesn't necessarily improve behavior over overwriting and doesn't clearly address the concern raised.	Low
	Add @OverRide to interceptor handler method The messageReceived override lacks the @Override annotation, and more importantly the anonymous handler does not delegate other methods (like toString) that some request handler implementations expose. Consider using a delegating wrapper (or explicitly annotating @Override) to catch signature drifts across OpenSearch versions and ensure correctness. src/main/java/org/opensearch/security/filter/AuditTransportInterceptor.java [102-103] +// Skip if TRANSPORT_AUDIT is disabled +if (!filter.getDisabledTransportCategories().contains(AuditCategory.TRANSPORT_AUDIT) + && !filter.getDisabledCategories().contains(AuditCategory.TRANSPORT_AUDIT)) { - Suggestion importance[1-10]: 2 __ Why: The suggestion's improved_code is identical to the existing_code, and the summary is a minor stylistic point about @Override annotation that doesn't materially affect correctness.	Low

Suggestions up to commit 5216482

Category	Suggestion	Impact
Possible issue	Guard against NPE on cache invalidation readEnabledFieldsCache is declared but never initialized in the diff, so calling invalidateAll() here will throw a NullPointerException if the cache has not been assigned. Add a null check before invalidating to prevent runtime failures during dynamic setting updates. src/main/java/org/opensearch/security/compliance/ComplianceConfig.java [440-442] this.watchedReadFields = Map.copyOf(parsed); this.watchedReadState = new WatchedReadState(Map.copyOf(parsed), Map.copyOf(enabled)); -this.readEnabledFieldsCache.invalidateAll(); +if (this.readEnabledFieldsCache != null) { + this.readEnabledFieldsCache.invalidateAll(); +} Suggestion importance[1-10]: 8 __ Why: The readEnabledFieldsCache field is declared volatile but not initialized in the shown diff; if it's null when setWatchedReadFields is invoked, an NPE would occur. This is a valid defensive check for dynamic setting updates.	Medium
	Restrict standalone compliance listener to SSL-only mode The condition !disabled && !client will be true for FGAC mode as well, but this branch is intended for standalone audit only (SSL-only mode). In FGAC mode, the compliance listener is already registered in the earlier if block, so this else if could double-register listeners or run in an unintended mode. Add an explicit SSLConfig.isSslOnlyMode() check to restrict this to standalone audit mode. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1196-1207] -} else if (!disabled && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { +} else if (SSLConfig.isSslOnlyMode() && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { // Non-FGAC mode: register compliance listener for standalone audit final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog, threadPool); indexModule.addIndexOperationListener(ciol); Suggestion importance[1-10]: 7 __ Why: Valid concern — the !disabled && !client condition could match FGAC mode where the compliance listener is already registered in the earlier if block. Adding an explicit SSLConfig.isSslOnlyMode() check would prevent potential double registration.	Medium
	Gate standalone AuditActionFilter to non-FGAC modes This else if branch runs when the first condition (!client && !disabled && !SSLConfig.isSslOnlyMode()) is false. That means it will trigger for FGAC nodes when client is false but security is otherwise enabled — however, auditLog is only initialized as a real AuditLogImpl in SSL-only or disabled modes here. In FGAC, auditLog is initialized elsewhere as AuditLogImpl, so this branch may register AuditActionFilter alongside SecurityFilter, causing duplicate audit events. Add an explicit mode check to gate this to standalone modes only. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1216-1228] // !(auditLog instanceof NullAuditLog) prevents registering AuditActionFilter when there's no real sink to send events to. No // point intercepting every request just to discard the message. -} else if (!client && auditLog != null && !(auditLog instanceof NullAuditLog)) { +} else if (!client && (disabled || SSLConfig.isSslOnlyMode()) && auditLog != null && !(auditLog instanceof NullAuditLog)) { filters.add( new AuditActionFilter( Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the else if branch could potentially register AuditActionFilter in FGAC mode if auditLog is initialized. However, since the first if guards this with a !SSLConfig.isSslOnlyMode() check, the else branch already implicitly targets standalone modes. Explicit gating improves clarity and safety.	Low
General	Honor transport audit enabled flag The interceptor does not check filter.isTransportApiAuditEnabled(), so transport auditing cannot be disabled via the standard enable_transport flag or its dynamic setter. Include this check so operators can toggle transport auditing without having to add categories to the disabled list. src/main/java/org/opensearch/security/filter/AuditTransportInterceptor.java [102-103] -if (!filter.getDisabledTransportCategories().contains(AuditCategory.TRANSPORT_AUDIT) +if (filter.isTransportApiAuditEnabled() + && !filter.getDisabledTransportCategories().contains(AuditCategory.TRANSPORT_AUDIT) && !filter.getDisabledCategories().contains(AuditCategory.TRANSPORT_AUDIT)) { Suggestion importance[1-10]: 7 __ Why: The isTransportApiAuditEnabled flag exists with a dynamic setter, but the interceptor doesn't respect it, making the top-level enable/disable toggle ineffective for transport auditing. This is a meaningful functional gap.	Medium
	Handle SSLPeerUnverifiedException separately to avoid noise SSLEngine.getSession().getPeerCertificates() throws SSLPeerUnverifiedException when the client did not present a certificate (common with wantClientAuth=true). This exception is caught by the generic catch (Exception e) below but generates a warning every minute even for legitimate non-mTLS clients. Consider catching SSLPeerUnverifiedException separately and silently skipping principal extraction to avoid spurious warnings under wantClientAuth. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [973-977] SSLEngine engine = sslHandler.engine(); if (engine.getNeedClientAuth() || engine.getWantClientAuth()) { try { Certificate[] certs = engine.getSession().getPeerCertificates(); if (certs != null && certs.length > 0 && certs[0] instanceof X509Certificate) { + // ... existing extraction logic ... + } + } catch (javax.net.ssl.SSLPeerUnverifiedException peerUnverified) { + // No client cert presented (expected under wantClientAuth) — skip silently + if (log.isDebugEnabled()) { + log.debug("No client certificate presented; skipping principal extraction"); + } + } catch (Exception e) { + // existing warn-throttle logic + } Suggestion importance[1-10]: 6 __ Why: Reasonable improvement — SSLPeerUnverifiedException is expected when wantClientAuth=true and no cert is provided. Handling it separately avoids spurious warnings, though the current throttled warning already mitigates the noise concern.	Low
	Warn when audit index pattern has no literal prefix When the Joda pattern doesn't start with a quote (e.g., audit-YYYY.MM.dd) and no datastream fallback is set, the method returns the raw pattern (e.g., audit-YYYY.MM.dd) as the prefix. This prefix will never match actual index names like audit-2026.07.20, silently disabling the self-loop guard. Consider stripping trailing date-format characters or logging a warning so operators know to configure a quoted literal prefix. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [507-513] // Extract literal prefix from Joda pattern: text between first pair of single quotes if (indexPattern.startsWith("'")) { int endQuote = indexPattern.indexOf("'", 1); if (endQuote > 1) { return indexPattern.substring(1, endQuote); } +} else if (indexPattern.matches(".*[YMdHmsS].*")) { + log.warn("Audit index pattern '{}' has no quoted literal prefix; self-loop guard may not match resolved index names. " + + "Wrap literal text in single quotes, e.g. \"'my-audit-'YYYY.MM.dd\".", indexPattern); } Suggestion importance[1-10]: 5 __ Why: Valid observability improvement. When the pattern lacks quotes but contains Joda date chars, the self-loop guard silently fails. A warning helps operators diagnose misconfiguration.	Low
	Guard indices() call against runtime exceptions Some IndicesRequest implementations throw UnsupportedOperationException or NullPointerException when indices() is called before request resolution (e.g., certain composite/remote requests). Wrap the call in a try/catch so a misbehaving request type does not abort the entire filter chain and drop the request. src/main/java/org/opensearch/security/filter/AuditActionFilter.java [134-144] if (request instanceof IndicesRequest) { - String[] indices = ((IndicesRequest) request).indices(); - if (indices != null) { - for (String idx : indices) { - if (isAuditIndex(idx)) { - chain.proceed(task, action, request, listener); - return; + try { + String[] indices = ((IndicesRequest) request).indices(); + if (indices != null) { + for (String idx : indices) { + if (isAuditIndex(idx)) { + chain.proceed(task, action, request, listener); + return; + } } } + } catch (Exception e) { + log.debug("Failed to read indices from request for self-loop check: {}", e.getMessage()); } } Suggestion importance[1-10]: 4 __ Why: Wrapping indices() in a try/catch adds resilience against misbehaving request types, though such issues are uncommon. The improvement is minor and mostly defensive.	Low
	Avoid inconsistent state between paired volatile fields Set and matcher are assigned in two separate volatile writes, so a concurrent reader can observe the new set with the old matcher (or vice versa) — leading to inconsistent filtering decisions. Assign the matcher first and the set second (or bundle them in an immutable holder) so at minimum the matcher used at query time is always at least as current as the set. src/main/java/org/opensearch/security/auditlog/config/AuditConfig.java [576-588] public void setIgnoredAuditUsers(List<String> users) { Set<String> newSet = ImmutableSet.copyOf(users); WildcardMatcher newMatcher = WildcardMatcher.from(newSet); + this.ignoredAuditUsersMatcher = newMatcher; this.ignoredAuditUsers = newSet; - this.ignoredAuditUsersMatcher = newMatcher; } public void setIgnoredAuditRequests(List<String> requests) { Set<String> newSet = ImmutableSet.copyOf(requests); WildcardMatcher newMatcher = WildcardMatcher.from(newSet); + this.ignoredAuditRequestsMatcher = newMatcher; this.ignoredAuditRequests = newSet; - this.ignoredAuditRequestsMatcher = newMatcher; } Suggestion importance[1-10]: 3 __ Why: Reordering writes only slightly improves consistency; the fundamental issue of non-atomic paired updates remains. A holder object would be a stronger fix, as briefly noted in the suggestion.	Low

Suggestions up to commit c46a1e2

Category	Suggestion	Impact
Possible issue	Restrict compliance registration to non-FGAC modes The compliance index-level components are only registered in SSL-only/disabled modes elsewhere, but this else if also triggers in FGAC mode (!disabled && !client covers regular mode). This will register a second compliance listener and reader wrapper on top of the existing FGAC ones, causing duplicate audit events and potentially overwriting the DLS/FLS reader wrapper. Restrict this branch to non-FGAC modes with SSLConfig.isSslOnlyMode(). src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [1196-1207] -} else if (!disabled && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { +} else if (SSLConfig.isSslOnlyMode() && !client && auditLog != null && !(auditLog instanceof NullAuditLog)) { // Non-FGAC mode: register compliance listener for standalone audit final ComplianceIndexingOperationListener ciol = new ComplianceIndexingOperationListenerImpl(auditLog, threadPool); indexModule.addIndexOperationListener(ciol); // Compliance read tracking — lightweight reader wrapper (no DLS/FLS) // Also sets Index...

[github-actions]

Persistent review updated to latest commit 1d57f01

[github-actions]

Persistent review updated to latest commit 7b7804d

[github-actions]

Persistent review updated to latest commit 8f10b41

[github-actions]

Persistent review updated to latest commit 8ee2707

[cwperks]

@Taiwo435 please look into the test failures. Also address the code hygiene failure by running ./gradlew spotlessApply and check in the formatted files.

[github-actions]

Persistent review updated to latest commit 104720d

[github-actions]

Persistent review updated to latest commit 5011ebd

[github-actions]

Persistent review updated to latest commit f55e52d

[github-actions]

Persistent review updated to latest commit d077550

[codecov]

Codecov Report

❌ Patch coverage is 84.82850% with 115 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.34%. Comparing base (e8b9083) to head (f29b3aa).
⚠️ Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
.../opensearch/security/OpenSearchSecurityPlugin.java	90.39%	20 Missing and 12 partials ⚠️
...compliance/ComplianceReadIndexSearcherWrapper.java	63.09%	30 Missing and 1 partial ⚠️
.../opensearch/security/filter/AuditActionFilter.java	85.91%	5 Missing and 15 partials ⚠️
...ensearch/security/compliance/ComplianceConfig.java	75.67%	7 Missing and 2 partials ⚠️
...earch/security/auditlog/impl/AbstractAuditLog.java	50.00%	1 Missing and 6 partials ⚠️
...rch/security/filter/AuditTransportInterceptor.java	88.88%	0 Missing and 7 partials ⚠️
...ensearch/security/auditlog/config/AuditConfig.java	79.31%	6 Missing ⚠️
...org/opensearch/security/auditlog/NullAuditLog.java	0.00%	2 Missing ⚠️
...g/opensearch/security/filter/AuditHeaderUtils.java	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6304      +/-   ##
==========================================
+ Coverage   75.10%   75.34%   +0.23%     
==========================================
  Files         451      455       +4     
  Lines       29275    29892     +617     
  Branches     4416     4516     +100     
==========================================
+ Hits        21987    22522     +535     
- Misses       5259     5280      +21     
- Partials     2029     2090      +61     

Files with missing lines	Coverage Δ
...ava/org/opensearch/security/auditlog/AuditLog.java	100.00% <ø> (ø)
...ensearch/security/auditlog/impl/AuditCategory.java	100.00% <100.00%> (ø)
...pensearch/security/auditlog/impl/AuditLogImpl.java	92.55% <100.00%> (+1.24%)	⬆️
...g/opensearch/security/support/ConfigConstants.java	95.65% <ø> (ø)
.../opensearch/security/support/SecuritySettings.java	97.91% <100.00%> (+12.20%)	⬆️
...g/opensearch/security/filter/AuditHeaderUtils.java	85.71% <85.71%> (ø)
...org/opensearch/security/auditlog/NullAuditLog.java	8.33% <0.00%> (+3.78%)	⬆️
...ensearch/security/auditlog/config/AuditConfig.java	94.24% <79.31%> (-2.68%)	⬇️
...earch/security/auditlog/impl/AbstractAuditLog.java	76.32% <50.00%> (-0.28%)	⬇️
...rch/security/filter/AuditTransportInterceptor.java	88.88% <88.88%> (ø)
... and 4 more

... and 17 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.

[DarshitChanpura]

Thanks @Taiwo435. This is a solid first PR towards getting Audit Logging to work with SSL-only mode.

I've left a few comments around potential bugs, perf improvement and nits.

[github-actions]

Persistent review updated to latest commit 38a12e2

[github-actions]

Persistent review updated to latest commit 939edc4

[github-actions]

Persistent review updated to latest commit c46a1e2

[github-actions]

Persistent review updated to latest commit 5216482

[github-actions]

Persistent review updated to latest commit 3d2d12d

[DarshitChanpura]

Left a couple more comments.

[github-actions]

Persistent review updated to latest commit f2125ed

[github-actions]

Persistent review updated to latest commit f29b3aa

[DarshitChanpura]

LGTM!

[cwperks]

@DarshitChanpura @Taiwo435 Please add Setting.Property.Sensitive in the list of settings properties for all of these. When Setting.Property.Sensitive is present, the setting is only toggle-able by a user mapped to one of the roles from plugins.security.restapi.roles_enabled: ["all_access","xyz_role"] (colloquially referred to as a "security admin"). Its crude, but does at least move towards some semblance of fine grained controls for cluster settings. All of the settings in here should only be toggle-able by a security admin of the cluster.

Even in SSL-Only mode we theoretically could enforce that a cluster admin is the user that presents the admin certificate. So even when in SSL-Only mode (i.e. no users and roles) we can still differentiate between a trusted and non-trusted caller by presence of that certificate.

[cwperks]

IMO !filter.isAuditDisabled doesn't read naturally. Prefer filter.isAuditEnabled()

[cwperks]

@DarshitChanpura @Taiwo435 Can we use the IndexResolverReplacer here? Can we please make sure to test with both legacy and v4 privilege evaluation code paths.

[cwperks]

Is there any opportunity to make this more generic in case the same code can be re-used?