support attributes from request headers in DLS

[rursprung]

Description

• Category: Enhancement

with this it is now possible to specify request headers which should be
available as substitutions in DLS queries. both HTTP and gRPC headers
are supported.

the headers have to be configured under the config key
plugins.security.unsupported.dls.allowed_request_headers. this is a
map (the key doesn't matter) with the following content for each entry:

• name: the actual name of the gRPC / HTTP header (case-insensitive)
• isMultiValue: whether the header can have more than one value
  (default: false)
• validationRegex: a regex to ensure that the header value cannot be
  used for code injection into the DLS query
• maxValueLength: the maximum length each header value is allowed to
  have (default: 256)

since DLS query substitution is pure string substitution and headers,
unlike other user attributes, are fully under the control of the caller
(and thus a potential attacker) the content must be carefully validated
to ensure that it does not pose a risk. for this the validationRegex
needs to be used - by default it rejects all content, thus it must be
configured explicitly. it should be configured to only allow explicitly
the patterns which are absolutely needed.

due to the risk associated with this feature it is currently being
treated as unsupported/experimental and will also not be documented.

if you, dear reader, stumble upon this PR / commit please beware: only
use this is if you are absolutely sure that you know what you are doing!
you have been warned!

the substitution is done using ${attr.header.[header name]}, e.g.
${attr.header.x-example-header}.
if isMultiValue is set to true then the substitution will always
contain quotes around the values and has to be treated as a list, i.e.
you should enclose it in []:

{ "terms": { "testfield": [${attr.header.x-example-header-mv}] } }

while if isMultiValue is set to false then the value will be added
verbatim and you need to quote it:

{ "term": { "testfield": "${attr.header.x-example-header}" } }

to prevent the risk of DOS attacks through the header forwarding both a
limit on the length of header values has been introduced (configurable
per header, see maxValueLength; default: 256 characters) as well as a
global limit on the amount of headers (see
DlsRequestHeadersUtil#MAX_HEADER_COUNT; arbitrarily set to 256) has
been introduced.

the config options can only be set via the config file (they are
intentionally not marked as Dynamic) so that it has to be a clear
decision to set this. this restriction can be lifted at a later point.

once #6311 is implemented the risk posed by this feature will go down
since then it will no longer be possible to modify the query with a
crafted request (which this feature tries to prevent by having the admin
specify a regex for the validation).

Issues Resolved

resolves #6265

Testing

integration tests, manual tests

Check List

• New functionality includes testing
• New functionality has been documented - intentionally undocumented!
• 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.

[rursprung]

CC @nibix & @cwperks since we already discussed this in some details

if this could land in 3.8.0 that'd be great 🙏 (presuming there are no blockers)

[github-actions]

PR Code Analyzer ❗

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

Path	Line	Severity	Description
src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java	197	medium	MultiValueDlsRequestHeader.serialize() concatenates header values with literal quote characters (map s -> '"' + s + '"') without escaping. If an admin configures a permissive validation regex that allows quote characters, a user could inject arbitrary DSL into the DLS query template (e.g., the MULTI_VALUE_DLS_QUERY template substitution). The feature is entirely dependent on the admin-configured regex for safety against query injection, but the code itself provides no escaping safeguard.
src/main/java/org/opensearch/security/filter/SecurityRestFilter.java	186	low	DLS request headers are extracted from the request and stored into the thread context before authentication is completed. If the authentication check later fails, the stored header values remain in thread context briefly. This is likely intentional but represents a minor ordering anomaly where header state is populated for unauthenticated requests.
src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java	109	low	Jackson polymorphic deserialization is used (via @JsonTypeInfo with Id.SIMPLE_NAME) when restoring DLS headers from transport-layer thread context headers in SecurityRequestHandler. While @JsonSubTypes limits allowed types, deserializing attacker-controlled JSON using class-name-based type resolution is a recognized attack surface if the Jackson version has gadget chain vulnerabilities.
src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java	105	low	The call to writeValueAsString() can throw a checked JsonProcessingException, but the method extractAndStoreDlsRequestHeaders() neither declares it nor wraps it in a try-catch. If serialization fails, the header will be stored transiently but not forwarded to other shards, causing silently inconsistent DLS enforcement across distributed query execution.

The table above displays the top 10 most important findings.

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

---

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

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 Security concerns Potential DLS query injection: The feature exposes request-header-controlled values into DLS queries via string substitution. The safety depends entirely on the admin-configured validationRegex. The default (^$) is safe, but there is no enforcement preventing an admin from configuring a regex that permits " or \, which would allow an attacker to break out of the JSON string context in MultiValueDlsRequestHeader.serialize() (which adds unescaped quotes) and inject arbitrary query fragments. Consider escaping quotes/backslashes on serialization or refusing regexes that could match them. The PR description acknowledges the risk and marks the feature as unsupported/experimental.

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Refactor gRPC test helpers to deduplicate plugin/auth setup Relevant files: src/integrationTest/java/org/opensearch/security/grpc/GrpcHelpers.java src/integrationTest/java/org/opensearch/security/grpc/BasicAuthGrpcTest.java src/integrationTest/java/org/opensearch/security/grpc/GrpcAnonymousAuthTest.java src/integrationTest/java/org/opensearch/security/grpc/JWTGrpcDefaultAuthHeaderTest.java src/integrationTest/java/org/opensearch/security/grpc/JWTGrpcDisabledAuthDomainTest.java src/integrationTest/java/org/opensearch/security/grpc/JWTGrpcInterceptorTest.java Sub-PR theme: Support request-header attributes in DLS queries Relevant files: src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java src/main/java/org/opensearch/security/filter/SecurityGrpcFilter.java src/main/java/org/opensearch/security/filter/SecurityRestFilter.java src/main/java/org/opensearch/security/privileges/UserAttributes.java src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java src/main/java/org/opensearch/security/privileges/actionlevel/legacy/PrivilegesEvaluatorImpl.java src/main/java/org/opensearch/security/privileges/actionlevel/nextgen/PrivilegesEvaluatorImpl.java src/main/java/org/opensearch/security/support/ConfigConstants.java src/main/java/org/opensearch/security/transport/SecurityInterceptor.java src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java src/integrationTest/java/org/opensearch/security/dlsfls/HeaderAttrInDlsIntegrationTest.java

⚡ Recommended focus areas for review Injection Risk The default validationRegex is ^$ (empty), but MultiValueDlsRequestHeader.serialize() wraps each value in double quotes without escaping. If an operator specifies a validationRegex that permits " or \ characters (e.g., a broad regex like .* or one allowing quote characters), an attacker can break out of the JSON string in a DLS query and inject arbitrary query clauses. Consider either escaping quote/backslash characters during serialization or documenting/enforcing that quotes and backslashes must never be permitted by the regex. public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); } } Possible NPE getDlsRequestHeaderSettings uses s.get("name").toLowerCase() and if a configured group entry omits the name key, s.get("name") returns null, causing a NullPointerException at cluster startup with a confusing stack trace. Consider validating that name is present and produce a clear error message. public static Map<String, DlsRequestHeaderSettings> getDlsRequestHeaderSettings(final Settings settings) { final var allowedDlsRequestHeaderSettings = settings.getGroups(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS_CONFIG); return allowedDlsRequestHeaderSettings.values() .stream() .map( s -> new DlsRequestHeaderSettings( s.get("name"), s.getAsBoolean("isMultiValue", false), // by default only match empty values => regex *must* be specified by admin Pattern.compile(s.get("validationRegex", "^$")), s.getAsInt("maxValueLength", 256) ) ) .collect(Collectors.toUnmodifiableMap(e -> e.name.toLowerCase(), identity())); } Case Handling Header names are lowercased in the replacements map (header.name().toLowerCase()), but StringSubstitutor performs case-sensitive key lookups. If a role's DLS query references ${attr.header.X-Example-Header} with any uppercase characters, the substitution will silently fail and the literal placeholder remains in the query, potentially producing incorrect DLS results rather than an error. Document this constraint or normalize the placeholder key to lowercase before lookup. context.getHeadersForDls() .forEach(header -> replacementsWithDots.put("attr.header." + header.name().toLowerCase(), header.serialize())); Error Handling extractAndStoreDlsRequestHeaders calls objectMapper().writerFor(...).writeValueAsString(...) which can throw a checked serialization exception, but the method signature does not declare it. This may cause a compilation issue or, if wrapped, an unclear runtime failure when a header cannot be serialized. Verify the exception type and handle it explicitly. final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { }).writeValueAsString(dlsRequestHeaders); threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to e8f9304

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Escape quotes to prevent injection The values are directly interpolated into the DLS query string wrapped in quotes without escaping. If a value contains a double quote or backslash (even if validation regex allows it in some configurations), it would break out of the string literal and could enable DLS query injection. Escape embedded quotes/backslashes before wrapping. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [202-207] public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { - return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); + return values.stream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 7 __ Why: Values interpolated directly into DLS query strings without escaping could enable injection if regex validation permits special characters; escaping quotes/backslashes is a reasonable defense-in-depth measure.	Medium
Possible issue	Avoid duplicate header set exception putHeader throws IllegalStateException if the header is already set. When a request is forwarded on the receiving node, SecurityRequestHandler#messageReceivedDecorate restores the transient value, but if extractAndStoreDlsRequestHeaders is called again on the same context (e.g. re-authenticated request or nested handling), setting the header a second time will fail. Guard against re-setting or clear it before setting. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [102-106] -if (!dlsRequestHeaders.isEmpty()) { +if (!dlsRequestHeaders.isEmpty() && Strings.isNullOrEmpty(threadContext.getHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS))) { final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { }).writeValueAsString(dlsRequestHeaders); threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); } Suggestion importance[1-10]: 5 __ Why: ThreadContext.putHeader can throw if the header is already set; guarding against re-setting is a reasonable defensive measure, though it may not occur frequently in practice.	Low
General	Use Locale.ROOT for lowercasing allowedDlsRequestHeaders keys are stored lowercased in getDlsRequestHeaderSettings, but the filter above uses equalsIgnoreCase on raw keys — this works but the subsequent allowedDlsRequestHeaders.get(e.getKey().toLowerCase()) relies on Locale-default lowercasing. Use Locale.ROOT to avoid the Turkish-locale i/ı problem which could cause a NullPointerException when passing null settings into toDlsRequestHeader. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [63-74] final Map<String, List<String>> rawDlsRequestHeaders = securityRequestChannel.getHeaders() == null ? Map.of() : securityRequestChannel.getHeaders() .entrySet() .stream() - .filter(e -> allowedDlsRequestHeaders.keySet().stream().anyMatch(a -> a.equalsIgnoreCase(e.getKey()))) + .filter(e -> allowedDlsRequestHeaders.containsKey(e.getKey().toLowerCase(java.util.Locale.ROOT))) .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() - .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) + .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase(java.util.Locale.ROOT)), e.getKey(), e.getValue())) .toList(); Suggestion importance[1-10]: 5 __ Why: Using toLowerCase() without Locale.ROOT can cause the Turkish locale issue, which could result in a NullPointerException when looking up settings; a valid minor improvement.	Low
	Handle serialization exception explicitly writeValueAsString in Jackson can throw a checked JsonProcessingException (or unchecked variants depending on the Jackson version). The enclosing method extractAndStoreDlsRequestHeaders does not declare it, so compilation may fail or a runtime exception may propagate up ungracefully during authentication. Wrap in try/catch and translate to IllegalStateException or handle serialization failure explicitly. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [103-105] -final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { -}).writeValueAsString(dlsRequestHeaders); -threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); +try { + final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { + }).writeValueAsString(dlsRequestHeaders); + threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); +} catch (Exception e) { + throw new IllegalStateException("Failed to serialize DLS request headers", e); +} Suggestion importance[1-10]: 3 __ Why: Modern Jackson writeValueAsString throws unchecked exceptions, so compilation is not affected; wrapping in try/catch provides marginal benefit for clearer error messages.	Low

---

Previous suggestions

Suggestions up to commit b1945ac

Category	Suggestion	Impact
Security	Escape quotes in serialized header values Header values are inserted directly into a DLS JSON query string without escaping the quote or backslash characters. Even with a regex validation, if an admin configures a permissive regex allowing " or </code>, this can break the DLS query JSON or enable injection. Escape double quotes and backslashes when serializing to make this safe by default. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [205-210] public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { - return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); + return values.stream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 7 __ Why: Valid security-related suggestion: if an admin configures a permissive regex, unescaped quotes/backslashes could break the DLS query JSON or enable injection. Defense-in-depth escaping is a good practice.	Medium
	Enforce header count limit before validation The header count check runs after toDlsRequestHeader has already been invoked via .toList() on the stream above, meaning per-header validation (regex, length, single-value) has already executed for potentially unbounded input. Move the totalHeaderCount limit check before constructing dlsRequestHeaders to enforce the DOS-prevention limit prior to any per-header processing. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [76-86] final var totalHeaderCount = rawDlsRequestHeaders.values().stream().mapToInt(List::size).sum(); if (totalHeaderCount > MAX_HEADER_COUNT) { throw new IllegalArgumentException( String.format( "found %d headers for DLS variables which exceeds the global maximum of %d", totalHeaderCount, MAX_HEADER_COUNT ) ); } +final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() + .stream() + .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase(java.util.Locale.ROOT)), e.getKey(), e.getValue())) + .toList(); + Suggestion importance[1-10]: 6 __ Why: Correctly identifies that the DOS-prevention limit is checked after per-header validation has already run, which somewhat defeats its purpose. Reordering improves the effectiveness of the DOS check.	Low
Possible issue	Use locale-independent lowercasing for headers toLowerCase() without a locale is locale-sensitive and can produce unexpected results in Turkish locale (e.g. I -> ı). Since header names and settings keys are ASCII, explicitly use Locale.ROOT for both the map key normalization here and in getDlsRequestHeaderSettings to ensure consistent case-insensitive matching across JVM locales. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [71-74] final Map<String, List<String>> rawDlsRequestHeaders = securityRequestChannel.getHeaders() == null ? Map.of() : securityRequestChannel.getHeaders() .entrySet() .stream() .filter(e -> allowedDlsRequestHeaders.keySet().stream().anyMatch(a -> a.equalsIgnoreCase(e.getKey()))) .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() - .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) + .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase(java.util.Locale.ROOT)), e.getKey(), e.getValue())) .toList(); Suggestion importance[1-10]: 5 __ Why: Valid concern about locale-sensitive toLowerCase() which could cause issues in Turkish locale; a reasonable defensive improvement but unlikely to be triggered in practice.	Low
General	Avoid duplicate header put exceptions threadContext.putHeader throws if the header key already exists in the context. If this filter runs more than once for the same request (or the header was set upstream), this will fail with an IllegalArgumentException. Consider checking whether the header is already set before putting, or ensure this method is only invoked once per request context. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [102-109] final String serializedDlsRequestHeaders; if (dlsRequestHeaders.isEmpty()) { serializedDlsRequestHeaders = ""; } else { serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { }).writeValueAsString(dlsRequestHeaders); } -threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); +if (threadContext.getHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS) == null) { + threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); +} Suggestion importance[1-10]: 4 __ Why: Valid concern that putHeader throws on duplicate keys, but in practice this method is invoked once per request at the filter layer, making the risk low.	Low

Suggestions up to commit 45bca69

Category	Suggestion	Impact
Security	Enforce header count limit before validation The MAX_HEADER_COUNT check happens after toDlsRequestHeader has already iterated over all header values (including validation regex matching). A malicious client sending thousands of header values could cause significant CPU work via regex matching before the limit check triggers. Move the count check before the mapping step to enforce the DoS protection early. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [76-86] final var totalHeaderCount = rawDlsRequestHeaders.values().stream().mapToInt(List::size).sum(); if (totalHeaderCount > MAX_HEADER_COUNT) { throw new IllegalArgumentException( String.format( "found %d headers for DLS variables which exceeds the global maximum of %d", totalHeaderCount, MAX_HEADER_COUNT ) ); } +final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() + .stream() + .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) + .toList(); + Suggestion importance[1-10]: 6 __ Why: Valid observation that count check happens after regex validation of all values, which could allow more CPU work than intended before the DoS limit triggers. Moderate impact since regex work is still bounded by header values already received.	Low
	Escape quotes in serialized header values The serialize() method wraps each header value in double quotes without escaping. Since values are used to build a DLS query via string substitution, a header value containing a " character (if allowed by a permissive validationRegex) could break out of the quoted string and inject arbitrary JSON/query content. Escape double quotes and backslashes in the value before wrapping. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [205-210] public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { - return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); + return values.parallelStream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 6 __ Why: Legitimate concern about potential injection if a permissive validationRegex is configured. However, the default regex is restrictive and admins control the regex, mitigating risk somewhat. Still worth escaping as defense in depth.	Low
Possible issue	Handle duplicate header keys robustly If a client sends the same header with different casings (e.g., X-Example-Header and x-example-header), the collector toUnmodifiableMap will throw IllegalStateException due to duplicate keys after case-insensitive matching, resulting in an uncaught error rather than a proper IllegalArgumentException. Additionally, the lookup key for allowedDlsRequestHeaders was stored via name.toLowerCase() but the incoming header key casing may not match. Normalize the incoming header keys to lower-case and merge lists to handle this consistently. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [63-74] final Map<String, List<String>> rawDlsRequestHeaders = securityRequestChannel.getHeaders() == null ? Map.of() : securityRequestChannel.getHeaders() .entrySet() .stream() - .filter(e -> allowedDlsRequestHeaders.keySet().stream().anyMatch(a -> a.equalsIgnoreCase(e.getKey()))) - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); + .filter(e -> allowedDlsRequestHeaders.containsKey(e.getKey().toLowerCase())) + .collect(Collectors.toUnmodifiableMap( + e -> e.getKey().toLowerCase(), + Map.Entry::getValue, + (a, b) -> { List<String> merged = new java.util.ArrayList<>(a); merged.addAll(b); return merged; })); final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() - .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) + .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey()), e.getKey(), e.getValue())) .toList(); Suggestion importance[1-10]: 6 __ Why: Correctly identifies that duplicate header keys with different casings would cause an IllegalStateException from toUnmodifiableMap, resulting in a poor error experience. The fix improves robustness.	Low
General	Validate missing 'name' setting explicitly If the name setting is missing for a group, s.get("name") returns null, causing a NullPointerException on e.name.toLowerCase() and yielding an unhelpful error. Additionally, this method is called on every request in the hot path; consider caching or fail-fast validation at startup with a clear error message identifying which group is misconfigured. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [156-170] public static Map<String, DlsRequestHeaderSettings> getDlsRequestHeaderSettings(final Settings settings) { final var allowedDlsRequestHeaderSettings = settings.getGroups(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS_CONFIG); - return allowedDlsRequestHeaderSettings.values() + return allowedDlsRequestHeaderSettings.entrySet() .stream() - .map( - s -> new DlsRequestHeaderSettings( - s.get("name"), + .map(entry -> { + final var s = entry.getValue(); + final String name = s.get("name"); + if (name == null) { + throw new IllegalArgumentException("Missing 'name' for DLS request header group: " + entry.getKey()); + } + return new DlsRequestHeaderSettings( + name, s.getAsBoolean("isMultiValue", false), - // by default only match empty values => regex *must* be specified by admin Pattern.compile(s.get("validationRegex", "^$")), s.getAsInt("maxValueLength", 256) - ) - ) + ); + }) .collect(Collectors.toUnmodifiableMap(e -> e.name.toLowerCase(), identity())); } Suggestion importance[1-10]: 4 __ Why: Valid point that a missing 'name' would throw NPE rather than a clear error. Minor improvement for admin usability, though this is a configuration issue caught at startup/first request.	Low

Suggestions up to commit cc2b6e5

Category	Suggestion	Impact
Security	Enforce header count limit before validation The MAX_HEADER_COUNT check is performed after toDlsRequestHeader has already processed all headers. If an attacker submits an extremely large number of headers, resource-intensive validation (regex matching) runs on all of them before the DoS-protection limit is enforced. Enforce the count limit before mapping/validation to make the DoS protection effective. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [71-86] +final var totalHeaderCount = rawDlsRequestHeaders.values().stream().mapToInt(List::size).sum(); + +if (totalHeaderCount > MAX_HEADER_COUNT) { + throw new IllegalArgumentException( + String.format( + "found %d headers for DLS variables which exceeds the global maximum of %d", + totalHeaderCount, + MAX_HEADER_COUNT + ) + ); +} + final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) .toList(); -final var totalHeaderCount = rawDlsRequestHeaders.values().stream().mapToInt(List::size).sum(); - -if (totalHeaderCount > MAX_HEADER_COUNT) { - Suggestion importance[1-10]: 7 __ Why: Valid security-hardening point: enforcing MAX_HEADER_COUNT before running regex validation on all headers reduces the potential DoS impact of many-header requests.	Medium
	Escape values to prevent query injection The serialize() method wraps values in quotes but does not escape any embedded quotes or backslashes. Even though a regex validation is applied, if a user configures a permissive validationRegex (e.g. allowing quotes/backslashes), values could break out of the string literal and inject arbitrary DLS query content. Escape the values to prevent query injection regardless of the configured regex. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [205-210] public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { - return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); + return values.stream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 7 __ Why: Reasonable defensive escaping suggestion; while the regex normally restricts values, defense-in-depth escaping for embedded quotes/backslashes would mitigate misconfiguration risks.	Medium
Possible issue	Fix locale and duplicate-key handling allowedDlsRequestHeaders is keyed by lowercase names (per getDlsRequestHeaderSettings), but toDlsRequestHeader is called with e.getKey().toLowerCase() using the default locale, which can lead to lookup misses in locales like Turkish. Also, Collectors.toUnmodifiableMap will throw on duplicate keys if the same header comes in with differing case. Use Locale.ROOT and consider a merge function to avoid runtime failures. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [63-74] final Map<String, List<String>> rawDlsRequestHeaders = securityRequestChannel.getHeaders() == null ? Map.of() : securityRequestChannel.getHeaders() .entrySet() .stream() - .filter(e -> allowedDlsRequestHeaders.keySet().stream().anyMatch(a -> a.equalsIgnoreCase(e.getKey()))) - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); + .filter(e -> allowedDlsRequestHeaders.containsKey(e.getKey().toLowerCase(java.util.Locale.ROOT))) + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> { + final var merged = new java.util.ArrayList<String>(a.size() + b.size()); + merged.addAll(a); + merged.addAll(b); + return java.util.List.copyOf(merged); + })); final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() - .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) + .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase(java.util.Locale.ROOT)), e.getKey(), e.getValue())) .toList(); Suggestion importance[1-10]: 6 __ Why: Valid concerns about locale-dependent lowercasing and potential duplicate-key collisions in toUnmodifiableMap; the fix improves robustness though duplicate header names with different case are uncommon.	Low
General	Use locale-independent lowercase conversion Using toLowerCase() without a locale can produce unexpected results in locales like Turkish (e.g., 'I' → 'ı'), which would cause the header name key to differ from what the DLS query expects. Use toLowerCase(Locale.ROOT) for a deterministic, locale-independent transformation. src/main/java/org/opensearch/security/privileges/UserAttributes.java [54-55] context.getHeadersForDls() - .forEach(header -> replacementsWithDots.put("attr.header." + header.name().toLowerCase(), header.serialize())); + .forEach(header -> replacementsWithDots.put("attr.header." + header.name().toLowerCase(java.util.Locale.ROOT), header.serialize())); Suggestion importance[1-10]: 5 __ Why: Correct observation that toLowerCase() without a locale can yield inconsistent results (Turkish locale issue); using Locale.ROOT is a minor correctness improvement.	Low

Suggestions up to commit 67a2d5a

Category	Suggestion	Impact
Security	Enforce header count limit before validation The header count check occurs after toDlsRequestHeader is invoked, which performs regex matching and value validation on all provided headers. An attacker could send many headers to force expensive regex evaluations before hitting the limit. Move the MAX_HEADER_COUNT check to occur before the per-header validation to properly mitigate DoS. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [71-78] final Map<String, List<String>> rawDlsRequestHeaders = securityRequestChannel.getHeaders() == null ? Map.of() : securityRequestChannel.getHeaders() .entrySet() .stream() .filter(e -> allowedDlsRequestHeaders.keySet().stream().anyMatch(a -> a.equalsIgnoreCase(e.getKey()))) .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); +final var totalHeaderCount = rawDlsRequestHeaders.values().stream().mapToInt(List::size).sum(); + +if (totalHeaderCount > MAX_HEADER_COUNT) { + throw new IllegalArgumentException( + String.format( + "found %d headers for DLS variables which exceeds the global maximum of %d", + totalHeaderCount, + MAX_HEADER_COUNT + ) + ); +} + final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) .toList(); -final var totalHeaderCount = rawDlsRequestHeaders.values().stream().mapToInt(List::size).sum(); - if (totalHeaderCount > MAX_HEADER_COUNT) { Suggestion importance[1-10]: 6 __ Why: Valid observation—moving the MAX_HEADER_COUNT check before per-header regex validation would reduce DoS risk from expensive regex evaluation on many attacker-supplied headers. Impact is moderate since the count check exists and header values have a max length.	Low
	Escape quotes to prevent query injection The header values are serialized into a JSON-like string by wrapping in double quotes without escaping. Although the validationRegex is expected to guard against this, the default regex ^$ is permissive to configure incorrectly, and a value containing " would allow query injection. Escape embedded quotes/backslashes explicitly to defensively prevent DLS query injection. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [205-209] public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { - return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); + return values.stream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 5 __ Why: Defensive escaping of quotes/backslashes provides an additional layer against DLS query injection if the validation regex is misconfigured. However, the validation regex is intended to prevent this, so the improvement is defense-in-depth.	Low
General	Use locale-independent lowercasing for header keys Using the default locale for toLowerCase() can produce unexpected results in locales like Turkish (e.g., I becomes ı). Since the configured header keys are lowered via name.toLowerCase() without a locale as well, always use Locale.ROOT on both sides for consistent, locale-independent matching. src/main/java/org/opensearch/security/privileges/UserAttributes.java [54-55] context.getHeadersForDls() - .forEach(header -> replacementsWithDots.put("attr.header." + header.name().toLowerCase(), header.serialize())); + .forEach(header -> replacementsWithDots.put("attr.header." + header.name().toLowerCase(java.util.Locale.ROOT), header.serialize())); Suggestion importance[1-10]: 6 __ Why: Correct observation—default-locale toLowerCase() can behave unexpectedly (e.g., Turkish locale). Using Locale.ROOT ensures consistent header matching, which is important since the settings map also lowers keys.	Low
Possible issue	Avoid re-setting existing thread context header ThreadContext.putHeader will throw if the header is already set (e.g., when this method is invoked on a request that has been re-entered or on retries within the same context). Guard against re-setting the header, or skip setting it when empty to avoid downstream failures and unnecessary header propagation. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [100-109] // and as a header so that it is passed on to other threads / instances, where it will be restored into a transient entry // See SecurityRequestHandler#messageReceivedDecorate -final String serializedDlsRequestHeaders; -if (dlsRequestHeaders.isEmpty()) { - serializedDlsRequestHeaders = ""; -} else { - serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { - }).writeValueAsString(dlsRequestHeaders); +if (!dlsRequestHeaders.isEmpty() && threadContext.getHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS) == null) { + final String serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper() + .writerFor(new TypeReference<List<DlsRequestHeader>>() { + }) + .writeValueAsString(dlsRequestHeaders); + threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); } -threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); Suggestion importance[1-10]: 5 __ Why: Guarding against re-setting an already-set header can prevent runtime exceptions from ThreadContext.putHeader if the method is invoked multiple times in the same context, which is a reasonable defensive check.	Low

Suggestions up to commit 7d6694c

Category	Suggestion	Impact
Security	Prevent duplicate header put and spoofing ThreadContext.putHeader throws IllegalStateException if the header is already present on the context (e.g., when a request is authenticated again or filtered twice, or when a client sends the internal header themselves). Guard the put or explicitly reject/ignore incoming client-supplied values of OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS to prevent header spoofing and duplicate-put crashes. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [83-89] if (dlsRequestHeaders.isEmpty()) { serializedDlsRequestHeaders = ""; } else { serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper().writerFor(new TypeReference<List<DlsRequestHeader>>() { }).writeValueAsString(dlsRequestHeaders); } -threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); +if (threadContext.getHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS) == null) { + threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); +} Suggestion importance[1-10]: 7 __ Why: Important security consideration - a client-supplied header with the same internal name could either cause a crash on duplicate put or be used to spoof DLS headers. However, the exact behavior depends on the ThreadContext semantics.	Medium
	Escape JSON-special characters in header values Header values are inserted verbatim into a JSON DLS query with only quotes wrapping them. If a value contains a " or </code> character (which the default regex ^$ prevents, but permissive user-provided regexes may allow), this will produce invalid JSON or enable injection into the query body. Escape JSON-special characters before serialization. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [185-190] public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { - return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); + return values.stream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 7 __ Why: Valid security concern - if an admin configures a permissive validationRegex, quote/backslash characters could break out of the JSON string context in the DLS query, enabling query injection. Escaping is a defensive measure.	Medium
	Check length before running regex The length check happens after the regex check, so extremely long inputs still get pattern-matched, which can be a DoS vector with catastrophic-backtracking regexes. Enforce maxValueLength before running the regex to bound regex evaluation cost. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [102-118] for (final var headerValue : headerValues) { + if (headerValue.length() > dlsRequestHeaderSettings.maxValueLength()) { + throw new IllegalArgumentException( + String.format( + "header %s exceeds the maximum defined length! (%d > %d)", + headerName, + headerValue.length(), + dlsRequestHeaderSettings.maxValueLength() + ) + ); + } if (!dlsRequestHeaderSettings.validationPattern().matcher(headerValue).matches()) { throw new IllegalArgumentException( String.format("header %s does not match the specified pattern and has thus been rejected!", headerName) ); } - if (headerValue.length() > dlsRequestHeaderSettings.maxValueLength()) { Suggestion importance[1-10]: 6 __ Why: Reasonable defense-in-depth against ReDoS - checking length first bounds regex evaluation cost. The impact is moderate since regex is admin-configured, but the fix is trivial and worthwhile.	Low
General	Use locale-insensitive lowercasing for headers toLowerCase() without a Locale can produce incorrect results in locales such as Turkish (e.g., the letter 'I'). Use toLowerCase(Locale.ROOT) for locale-insensitive lowercasing to prevent lookup failures with header names containing certain characters. The same applies to the lowercase call in getDlsRequestHeaderSettings. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [66] -.map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) +.map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase(java.util.Locale.ROOT)), e.getKey(), e.getValue())) Suggestion importance[1-10]: 6 __ Why: Valid concern - using toLowerCase() without Locale.ROOT can cause issues in Turkish locales where 'I' behaves differently. This is a legitimate best-practice fix for locale-safe string comparison.	Low

[github-actions]

Persistent review updated to latest commit a0a03b4

[github-actions]

Persistent review updated to latest commit b1dde53

[github-actions]

Persistent review updated to latest commit 956ae1e

[github-actions]

Persistent review updated to latest commit 8fb20cf

[github-actions]

Persistent review updated to latest commit 7d6694c

[github-actions]

Persistent review updated to latest commit 67a2d5a

[rursprung]

i think that the test failures which are now still happening are flaky tests? i see nodes which become unreachable (but no clear error as to why and i've seen these errors in other PRs before) and i see gradle failing to download a gradle plugin (=> network issues)

[github-actions]

Persistent review updated to latest commit cc2b6e5

[DarshitChanpura]

@rursprung Can you look into test failures:

also, is there a reason for using .unsupported. prefix?

[rursprung]

@rursprung Can you look into test failures:

ok, i can see the failure in my test: i don't do } finally { channel.shutdown(); } in the GRPC test (and didn't notice that because i only ever ran the one, so it shut down afterwards anyway). i've added that.

i can indeed reproduce that WhoAmITests fails for me as well (though locally i have to run it as a single-node cluster; see slack for my problems with multi-node clusters in tests). grantedPrivilegesMessages has 5 entries on main but only 1 on my branch... and i have 0 explanation as to why that's the case. i don't see anything in my code which would indicate that it could cause this and debugging it i didn't see anything obvious either. and there are no exceptions in the logs as far as i can tell.
so for now i'm at a complete loss. also, i'm officially off. if anyone understand else has time to dig into this so that we have a chance to land it in this release that'd be awesome!

also, is there a reason for using .unsupported. prefix?

as explained in the commit-msg & PR description this feature has some security implications and for now we want to keep it unsupported/experimental and gather some experience with it first, and maybe clean up some relevant functionality (e.g. how attribute substitution works) first before we stabilise it (if ever).

[github-actions]

Persistent review updated to latest commit 45bca69

[github-actions]

Persistent review updated to latest commit b1945ac

[nibix]

So, I had a look into the test failures in WhoAmITests:

The changes in this PR has the effect that virtually any transport request carries the _opensearch_security_dls_request_headers header.

One example, generated by one test in WhoAmITests:

{
    "audit_cluster_name": "local_cluster_1",
    "audit_transport_headers": {
        "_opensearch_security_dls_request_headers": ""
    },
    "audit_node_name": "data_0",
    "audit_trace_task_id": "gpkyn_ViTr6T3l60GJ2Bow:77",
    "audit_transport_request_type": "GetSettingsRequest",
    "audit_category": "GRANTED_PRIVILEGES",
    "audit_request_origin": "REST",
    "audit_node_id": "gpkyn_ViTr6T3l60GJ2Bow",
    "audit_request_layer": "TRANSPORT",
    "@timestamp": "2026-07-22T14:26:02.652+00:00",
    "audit_format_version": 4,
    "audit_request_remote_address": "127.0.0.1",
    "audit_request_privilege": "indices:monitor/settings/get",
    "audit_node_host_address": "127.0.0.1",
    "audit_request_effective_user": "audit_log_verifier",
    "audit_node_host_name": "127.0.0.1"
}

This particular audit log message carried no headers before the change:

This influences a condition in AuditLogsRule:

security/src/integrationTest/java/org/opensearch/test/framework/audit/AuditLogsRule.java

Lines 147 to 168 in 06a098a

	public void onAuditMessage(AuditMessage auditMessage) {
	if (auditMessage.getAsMap().keySet().contains("audit_transport_headers")) {
	if (log.isDebugEnabled()) {
	log.debug(
	"New transport audit message received '{}', total number of transport audit messages '{}'.",
	auditMessage,
	currentTransportTestAuditMessages.size()
	);
	}
	currentTransportTestAuditMessages.add(auditMessage);
	} else {
	if (log.isDebugEnabled()) {
	log.debug(
	"New audit message received '{}', total number of audit messages '{}'.",
	auditMessage,
	currentTestAuditMessages.size()
	);
	}
	currentTestAuditMessages.add(auditMessage);
	}
	}
	}

I do not really understand why the presence of audit_transport_headers influences into which bucket the messages are sorted.

But, this does not matter so much. Rather, IMHO we should avoid having any side effects of the feature when the feature is not in use. In this case, it is the presence of the empty valued transport header. We should strive to have such observable changes only when the feature is in use.

Thus, I would recommend to modify the code to only write such headers when the settings reflect the use of it.

[rursprung]

thanks for the investigation @nibix!
i've updated the PR so that it now only adds the header if it really has any content. i did not expect this kind of side-effect of an empty header. the test now runs through with this change in place.

[github-actions]

Persistent review updated to latest commit e8f9304