Expose dynamic audit config via cluster settings in SSL-only mode

[Taiwo435]

Description

Narrow the audit settings filter in SSL-only (standalone audit) mode so the non-secret dynamic audit configuration is readable via GET _cluster/settings, while continuing to hide the credential-bearing sink settings.

• Why these changes are required?

  In SSL-only mode there is no .opendistro_security index, so the audit configuration is stored in cluster settings and is managed through PUT/GET _cluster/settings. The standalone audit configuration panel in the security dashboards plugin reads its state back through GET _cluster/settings. OpenSearchSecurityPlugin.getSettingsFilter() stripped the entire plugins.security.audit.* subtree from settings responses, which also hid the ~17 non-secret dynamic config keys the panel needs. Writes worked (the filter only affects reads), but reads came back empty — the panel could not display the current configuration.

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

  • Old: getSettingsFilter() always added plugins.security.audit.*, so the whole subtree — including the non-secret dynamic config — was stripped from every settings response in every mode.
  • New: in SSL-only mode the filter strips only the credential-bearing group settings (plugins.security.audit.endpoints.* and plugins.security.audit.routes.*); the dynamic config (plugins.security.audit.config.* and plugins.security.audit.compliance.*) is now returned. FGAC and all other modes keep the original broad plugins.security.audit.* filter, because their real audit config lives in the security index, not cluster settings.

  Credential safety is preserved:

  • The default-endpoint secrets (username, password, webhook.url, pem*, salt) are registered with Property.Filtered, so OpenSearch core strips them from settings responses regardless of this filter.
  • The nested credentials on secondary endpoints/routes are not Property.Filtered, so they are kept out of responses by the retained endpoints.* / routes.* filters.

  Additionally, three static external-sink settings under config.* — http_endpoints (the backend audit host list), enabled_ssl_ciphers, and enabled_ssl_protocols — were previously hidden only by the broad plugins.security.audit.* wildcard and carry no Property.Filtered annotation. Narrowing the wildcard would have exposed this infrastructure topology to unauthenticated clients in SSL-only mode via GET _nodes/settings. These are static (non-dynamic) sink settings the audit panel does not manage, so they are now registered Property.Filtered alongside their credential siblings — closing the disclosure in all modes with no functional impact (the sink still reads them directly from settings).

• Note for backport: Adding Property.Filtered to http_endpoints, enabled_ssl_ciphers, and enabled_ssl_protocols means these settings will also disappear from _nodes/settings output in FGAC/default mode (not just SSL-only). This is intentional — they are static external-sink infrastructure config that should not have been exposed to settings readers in any mode.

Issues Resolved

Read-path gap for the standalone audit configuration panel in SSL-only mode.

Testing

Added two integration tests under src/integrationTest:

• StandaloneAuditSettingsFilterTest (SSL-only mode):

  • dynamicAuditConfigIsReadableViaClusterSettings — writes a representative slice of the dynamic settings across both prefixes (config.* and compliance.*) and both value shapes (list + boolean) via PUT _cluster/settings, then asserts they come back through GET _cluster/settings. The filter is prefix-based, so covering both subtrees and both value shapes proves all ~17 dynamic keys flow through.
  • auditSecretsAreNotExposedInNodeSettings — statically configures the Property.Filtered default-endpoint secrets plus a secondary endpoint credential and a route credential, then asserts via GET _nodes/settings that the non-secret config surfaces while none of the secret values or the endpoints.* / routes.* keys appear. Also configures http_endpoints, enabled_ssl_ciphers, and enabled_ssl_protocols under config.* and asserts they stay hidden (regression guard for the Property.Filtered additions).

• StandaloneAuditFgacFilterUnchangedTest (FGAC mode):

  • fgacModeStillFiltersAuditSettings — statically configures a dynamic audit setting and a nested endpoints.* credential, then asserts the broad plugins.security.audit.* filter still strips both from GET _nodes/settings, confirming the narrowing is gated to SSL-only mode and does not change FGAC behavior. (The values are set statically rather than via PUT because in FGAC SecurityFilter blocks a runtime cluster-settings update to a sensitive key unless the caller holds a restapi.roles_enabled role — a write guard orthogonal to the read filter under test.)

Check List

• New functionality includes testing
• New functionality has been documented (code comments on the filter behavior)
• 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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 18e5c32)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 Security concerns Potential sensitive information exposure: The PR narrows the settings filter in SSL-only mode to allow plugins.security.audit.config.* and plugins.security.audit.compliance.* through settings responses. Credential safety now depends on every individual sink credential under config.* being registered with Property.Filtered. The new AuditConfigSettingsFilterSafetyTest guards against regressions using a substring pattern, but any future credential-bearing key whose name doesn't match the pattern (e.g., "credential", "apikey", "auth", "bearer") would slip through undetected. Consider expanding the regex or inverting the check (require Property.Filtered on all config.* keys except an explicit allowlist of known non-secrets).

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Filter scope for compliance The new SSL-only branch adds filters for endpoints.* and routes.* but relies on the separate plugins.security.compliance.* filter (note: different prefix from plugins.security.audit.compliance.*). Verify that any credential-bearing keys under plugins.security.audit.compliance.* (if any exist or are added in the future) are individually marked Property.Filtered, since the narrowed audit filter intentionally exposes this subtree in SSL-only mode. If a compliance setting stores a secret, it would now be exposed. // In SSL-only (standalone audit) mode there is no security index, so the audit config is stored in // cluster settings and the dashboards audit panel must read it back via GET _cluster/settings. Narrow // the audit filter to expose the dynamic config (plugins.security.audit.config.* and .compliance.*) // while keeping the credential-bearing sink settings (endpoints/routes) hidden. Secrets registered with // Property.Filtered (sink username/password/webhook.url, pem*, salt) remain stripped by core regardless. // FGAC keeps the original broad filter (its real config lives in the security index, not cluster settings). if (SSLConfig.isSslOnlyMode()) { settingsFilter.add("plugins.security.audit.endpoints.*"); settingsFilter.add("plugins.security.audit.routes.*"); } else { settingsFilter.add("plugins.security.audit.*"); } settingsFilter.add("plugins.security.compliance.*");

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 18e5c32

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Case-insensitive match in safety test The regex uses lowercase-only alternation against key without lowercasing, so any future setting with mixed-case identifiers (e.g., containing Password or Webhook) would silently bypass the safety check. Normalize the key to lowercase before matching to make the guard robust to case variations. src/test/java/org/opensearch/security/auditlog/AuditConfigSettingsFilterSafetyTest.java [47-51] for (Setting<?> s : new OpenSearchSecurityPlugin(disabled, null).getSettings()) { String key = s.getKey(); - if (key.startsWith(ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX) && secretish.matcher(key).find()) { + if (key.startsWith(ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX) && secretish.matcher(key.toLowerCase()).find()) { assertThat(key + " must be Property.Filtered", s.getProperties().contains(Setting.Property.Filtered), equalTo(true)); } } Suggestion importance[1-10]: 5 __ Why: A reasonable defensive improvement to make the safety-net regex robust to future mixed-case setting keys, though OpenSearch setting keys conventionally use lowercase, limiting practical impact.	Low
Security	Also filter threadpool subtree in SSL-only mode The narrowed SSL-only filter only strips endpoints. and routes., but any setting registered directly under plugins.security.audit.config. (or plugins.security.audit. outside these two groups) that is not marked Property.Filtered will now leak. Consider also adding plugins.security.audit.threadpool.* or an explicit allowlist approach, and double-check that non-config keys under the audit prefix (e.g., plugins.security.audit.type, plugins.security.audit.threadpool.size) are intended to be exposed. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [2746-2752] if (SSLConfig.isSslOnlyMode()) { settingsFilter.add("plugins.security.audit.endpoints.*"); settingsFilter.add("plugins.security.audit.routes.*"); + settingsFilter.add("plugins.security.audit.threadpool.*"); } else { settingsFilter.add("plugins.security.audit.*"); } settingsFilter.add("plugins.security.compliance.*"); Suggestion importance[1-10]: 4 __ Why: The suggestion raises a legitimate concern about non-credential keys under the audit prefix being exposed in SSL-only mode, but it's speculative — the PR intentionally narrows the filter to expose non-secret dynamic config, and threadpool.* may not exist or be sensitive. The improvement is not strongly justified.	Low

---

Previous suggestions

Suggestions up to commit 55910ed

Category	Suggestion	Impact
General	Tighten secret-detection regex to avoid false matches The alternation ssl_ followed by |key|cert|... creates a token ssl_ (matching any key containing "ssl_") and separate key/cert tokens. The key token will match any setting containing the substring "key" (e.g. "keystore_type", "disable_key_..." or even innocuous keys), causing false positives, while ssl_ may miss keys that use camelCase or different separators. Tighten the regex with word/character boundaries or a more explicit set to avoid false positives that would force Property.Filtered on non-sensitive keys. src/test/java/org/opensearch/security/auditlog/AuditConfigSettingsFilterSafetyTest.java [38-40] Pattern secretish = Pattern.compile( - "password|username|token|webhook|pem|salt|jks|http_endpoints|ssl_" + "|key|cert|secret|keystore|truststore|pkcs" + "(password|username|token|webhook|pem|salt|jks|http_endpoints|ssl_ciphers|ssl_protocols|private_key|cert|secret|keystore|truststore|pkcs)" ); Suggestion importance[1-10]: 5 __ Why: The observation about the broad key token causing potential false positives is reasonable and could improve test precision, but it only affects a safety-net test and current registered settings likely don't trigger false positives.	Low
Security	Reduce reliance on per-setting Filtered flags The narrowed filter only strips endpoints. and routes. group settings, but any future non-panel key registered directly under plugins.security.audit.config. or plugins.security.audit.. without Property.Filtered will be exposed. Consider also filtering plugins.security.audit.config.* sub-keys that are known non-dynamic (or invert the logic: only expose the specific dynamic prefixes needed by the panel) to reduce reliance on per-setting Property.Filtered annotations. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [2746-2751] if (SSLConfig.isSslOnlyMode()) { + // Only strip credential-bearing group settings; individual secrets under config.* must + // carry Property.Filtered (enforced by AuditConfigSettingsFilterSafetyTest). settingsFilter.add("plugins.security.audit.endpoints.*"); settingsFilter.add("plugins.security.audit.routes.*"); } else { settingsFilter.add("plugins.security.audit.*"); } settingsFilter.add("plugins.security.compliance.*"); Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid design concern about relying on per-setting Property.Filtered annotations, but the improved_code only adds a comment without functionally changing the filter logic, limiting its impact.	Low

Suggestions up to commit a5eef32

Category	Suggestion	Impact
Security	Use allow-list for exposed audit prefixes In SSL-only mode, the narrowed filter exposes the entire plugins.security.audit. subtree except endpoints./routes.. Any statically-configured key directly under plugins.security.audit. (e.g. plugins.security.audit.type, plugins.security.audit.threadpool., or future top-level audit keys) will now surface in settings responses. Consider explicitly listing only the intended-to-expose prefixes (config., compliance.) rather than defaulting to expose-all-minus-two, to avoid accidental disclosure when new audit settings are added. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [2746-2752] if (SSLConfig.isSslOnlyMode()) { + // Expose only the panel-managed dynamic subtrees; keep everything else under audit.* filtered. settingsFilter.add("plugins.security.audit.endpoints.*"); settingsFilter.add("plugins.security.audit.routes.*"); + settingsFilter.add("plugins.security.audit.threadpool.*"); + settingsFilter.add("plugins.security.audit.type"); + settingsFilter.add("plugins.security.audit.ignore_users"); + // ...only config.* and compliance.* intentionally pass through } else { settingsFilter.add("plugins.security.audit.*"); } settingsFilter.add("plugins.security.compliance.*"); Suggestion importance[1-10]: 6 __ Why: Valid security concern: the current deny-list approach could expose future audit settings added directly under plugins.security.audit.*. However, the improved code is illustrative and includes keys like ignore_users that are actually panel-managed dynamic config, so the exact suggested change would need refinement.	Low
	Broaden secret-detection regex in safety test The regex-based heuristic can silently miss newly added credential keys that don't match any of the listed substrings (e.g. secret, key, credential, auth, bearer, passphrase). Since this test is the safety net protecting SSL-only mode from credential disclosure, broaden the pattern to include more common secret keywords to reduce the chance of a future credential slipping through. src/test/java/org/opensearch/security/auditlog/AuditConfigSettingsFilterSafetyTest.java [38] -Pattern secretish = Pattern.compile("password|username|token|webhook|pem|salt|jks|http_endpoints|ssl_"); +Pattern secretish = Pattern.compile( + "password|passphrase|username|user|token|webhook|pem|salt|jks|http_endpoints|ssl_|secret|credential|bearer|auth|key" +); -for (Setting<?> s : new OpenSearchSecurityPlugin(disabled, null).getSettings()) { - String key = s.getKey(); - if (key.startsWith(ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX) && secretish.matcher(key).find()) { - assertThat(key + " must be Property.Filtered", s.getProperties().contains(Setting.Property.Filtered), equalTo(true)); - } -} - Suggestion importance[1-10]: 5 __ Why: Reasonable defense-in-depth improvement to the safety-net test. Broadening the regex reduces the risk of missing new credential keys, though adding generic terms like key or auth may cause false positives on non-secret settings.	Low

Suggestions up to commit d32b363

Category	Suggestion	Impact
Security	Filter non-dynamic audit keys in SSL-only mode The narrowed SSL-only filter only strips endpoints. and routes., but plugins.security.audit.type and plugins.security.audit.threadpool. (and other non-config./compliance. audit keys) are now exposed. Consider filtering explicitly by allow-listing the two dynamic subtrees to expose (e.g., keep a broad plugins.security.audit. filter and add exemptions) or explicitly add filters for plugins.security.audit.type and plugins.security.audit.threadpool.* to avoid disclosing sink type and threadpool sizing. src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java [2737-2742] if (SSLConfig.isSslOnlyMode()) { settingsFilter.add("plugins.security.audit.endpoints.*"); settingsFilter.add("plugins.security.audit.routes.*"); + settingsFilter.add("plugins.security.audit.threadpool.*"); + settingsFilter.add("plugins.security.audit.type"); } else { settingsFilter.add("plugins.security.audit.*"); } settingsFilter.add("plugins.security.compliance.*"); Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid security concern that narrowing the filter may expose non-panel-managed audit settings like plugins.security.audit.type and threadpool.*, which could disclose sink type and configuration. However, exposure of these values may be intentional or acceptable for the panel functionality, and the impact is limited to information disclosure rather than credential leakage.	Low

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.47%. Comparing base (5e8e5f1) to head (d32b363).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6392      +/-   ##
==========================================
+ Coverage   75.33%   75.47%   +0.13%     
==========================================
  Files         456      456              
  Lines       30075    30143      +68     
  Branches     4564     4571       +7     
==========================================
+ Hits        22657    22749      +92     
+ Misses       5297     5270      -27     
- Partials     2121     2124       +3     

Files with missing lines	Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java	84.04% <100.00%> (+0.04%)	⬆️

... and 11 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]

Nice PR — the write-up made this easy to follow, and the diagnosis is right. I checked the pieces I was worried about: isSslOnlyMode() is the instance field so that's fine, and Property.Sensitive only gates writes, it doesn't filter reads, so narrowing the filter is genuinely the right fix here. Pulling the three Filtered flags onto http_endpoints/ciphers/protocols in the same PR is correct too — without them the narrowing would leak those, so they belong here.

I'm good with the design tradeoff. In ssl-only mode the audit config only lives in cluster settings and there's no authz layer anyway, so exposing the non-secret dynamic config to settings readers is consistent with what that mode already is. Not going to bikeshed keeping compliance.* filtered.

One thing I do want before merging: the safety story is now "every secret under config.* must be individually Property.Filtered," since the wildcard no longer covers them in ssl-only. That's fine today, but it's a trap for whoever adds the next sink credential. Can you add a short comment at the config.* registration block spelling that out, plus a test that fails if a config.* key with a secret-ish name isn't Filtered? Something like:

@Test
public void allSensitiveConfigSettingsAreFiltered() {
    Settings disabled = Settings.builder().put(ConfigConstants.SECURITY_DISABLED, true).build();
    Pattern secretish = Pattern.compile("password|username|token|webhook|pem|salt|jks|http_endpoints|ssl_");
    for (Setting<?> s : new OpenSearchSecurityPlugin(disabled, null).getSettings()) {
        String key = s.getKey();
        if (key.startsWith(ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX) && secretish.matcher(key).find()) {
            assertThat(key + " must be Property.Filtered", s.getProperties().contains(Setting.Property.Filtered), equalTo(true));
        }
    }
}

(Building the plugin with disabled=true is the easy way to reach getSettings() without dragging in configPath/TLS setup — a plain new OpenSearchSecurityPlugin(Settings.EMPTY, null) throws. If it fights the unit-test module for any reason, just fold the same secret-not-present assertions into StandaloneAuditSettingsFilterTest.)

Couple of small things, non-blocking: the tests hardcode the plugins.security.audit.compliance. prefix — worth exposing the constant from SecuritySettings so it can't drift. And add the skip-changelog label. Also worth a line in the description that http_endpoints/ciphers/protocols will disappear from settings output in FGAC/default too, so we don't get surprised on backport.

[github-actions]

PR Code Analyzer ❗

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

⛔ Hard block: Issues at High severity or above will block this PR from merging.

Path	Line	Severity	Description
src/test/java/org/opensearch/security/auditlog/AuditConfigSettingsFilterSafetyTest.java	42	medium	The 'secretish' regex pattern enforcing Property.Filtered annotations omits common credential-bearing substrings: 'key' (API keys, private keys), 'cert' (certificate material), 'secret', 'keystore', 'truststore', and 'pkcs'. In SSL-only mode the broad plugins.security.audit.* wildcard is no longer applied to the config.* subtree, so any future sink credential whose key matches none of the listed terms will bypass this safety net and surface in unauthenticated settings responses.

The table above displays the top 10 most important findings.

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

---

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 a5eef32

[github-actions]

Persistent review updated to latest commit 55910ed

[github-actions]

Persistent review updated to latest commit 18e5c32