support attributes from request headers in DLS by rursprung · Pull Request #6310 · opensearch-project/security

rursprung · 2026-07-17T09:45:22Z

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 · 2026-07-17T09:46:05Z

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 · 2026-07-17T09:46:37Z

PR Code Analyzer ❗

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

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

Path	Line	Severity	Description
src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java	189	medium	MultiValueDlsRequestHeader.serialize() wraps values with raw quote characters ("\"" + s + "\"") without JSON-escaping the content before interpolation into DLS query templates. If an admin configures a permissive validationRegex (e.g., '.*') that permits double-quote characters, a user could craft header values like 'foobar","injected' to manipulate the resulting OpenSearch query DSL. Defense-in-depth would require JSON-encoding each value rather than raw string concatenation.
src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java	196	medium	Jackson polymorphic deserialization is used with @JsonTypeInfo(use = Id.SIMPLE_NAME) and @JsonSubTypes for deserializing DlsRequestHeader objects forwarded between cluster nodes. While @JsonSubTypes limits the recognized types to SingleValueDlsRequestHeader and MultiValueDlsRequestHeader, the combination of SIMPLE_NAME resolution and transport-layer JSON (originating from a potentially compromised node) creates a deserialization attack surface. A malicious internal node could send crafted JSON to trigger unexpected behavior depending on the Jackson version and any registered custom deserializers.

The table above displays the top 10 most important findings.

Total: 2 | Critical: 0 | High: 0 | Medium: 2 | 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 · 2026-07-17T09:47:46Z

PR Reviewer Guide 🔍

(Review updated until commit `551d00e`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 Security concerns DLS query injection: DLS query substitution is a raw string substitution. The safety of this feature relies entirely on operators configuring a strict `validationRegex`. If an operator misconfigures the regex to permit characters like `"` or `\`, an attacker controlling the request header can break out of the JSON string literal in the DLS query and gain unintended data access. The default rejects everything, which is good, but `MultiValueDlsRequestHeader.serialize()` still emits quotes around values without escaping — consider always JSON-escaping or hard-blocking `"`/`\` regardless of regex. Also note that the feature is intentionally shipped as unsupported/undocumented.
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review Case-sensitivity Bug `toDlsRequestHeader` is called with `allowedDlsRequestHeaders.get(e.getKey().toLowerCase())`, but `allowedDlsRequestHeaders` keys are already lowercased. However, the filter above uses `equalsIgnoreCase` while the incoming header key `e.getKey()` may be in any case. If a header arrives with mixed/upper case, `e.getKey().toLowerCase()` will still match, but the map lookup relies on `Locale.ROOT` semantics. Using `toLowerCase()` without a Locale can cause issues in locales like Turkish (e.g. `I` → `ı`), potentially causing the lookup to return null and a NullPointerException in `toDlsRequestHeader`. Consider using `toLowerCase(Locale.ROOT)` consistently here and in `UserAttributes.replaceProperties` (`header.name().toLowerCase()`). final var dlsRequestHeaders = rawDlsRequestHeaders.entrySet() .stream() .map(e -> toDlsRequestHeader(allowedDlsRequestHeaders.get(e.getKey().toLowerCase()), e.getKey(), e.getValue())) .toList(); Injection via Header Name `MultiValueDlsRequestHeader.serialize()` wraps values in double quotes without escaping. If `validationRegex` is configured too permissively by an operator (e.g. allowing `"` or `\`), an attacker could break out of the string literal in the DLS query and inject arbitrary JSON/query fragments. Since this is being merged as an unsupported/experimental feature, at minimum consider explicitly rejecting `"` and `\` in header values regardless of the configured regex, or JSON-escape the values during serialization to reduce risk of misconfiguration. public record MultiValueDlsRequestHeader(String name, List<String> values) implements DlsRequestHeader, Serializable { @Override public String serialize() { return values.parallelStream().map(s -> "\"" + s + "\"").collect(Collectors.joining(",")); } } Unchecked Exception `extractAndStoreDlsRequestHeaders` calls `DefaultObjectMapper.objectMapper().writerFor(...).writeValueAsString(dlsRequestHeaders)` but does not declare/handle Jackson exceptions. If serialization ever fails, an unchecked exception is propagated up through the REST/gRPC filter, potentially producing an unclear error to callers. Consider wrapping this in a try/catch and translating to a clean 400/authentication error consistent with other validation failures in this class. final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper() .writerFor(new TypeReference<List<DlsRequestHeader>>() { }) .writeValueAsString(dlsRequestHeaders); threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); }

github-actions · 2026-07-17T09:48:24Z

PR Code Suggestions ✨

Latest suggestions up to 551d00e

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Escape quotes when serializing header values Values are validated against a regex but not escaped when serialized into a JSON-like DLS query string. If the regex configured by admins allows characters such as `"` or `</code>, the serialized output could break out of the string literal and inject arbitrary DLS query content. Escape embedded quotes/backslashes to defend against misconfigured regexes.` src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [211-216] 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]: 6 __ Why: Values are validated by regex which admins configure; if regex permits " or \, the serialized DLS query could be injected. Escaping provides defense in depth against misconfiguration.	Low
Possible issue	Prevent duplicate header assignment exception This method is called on every REST/gRPC request, including for requests that may be forwarded to other nodes where the header may already be set. Calling `putHeader` on a `ThreadContext` that already contains the header will throw an `IllegalStateException`. Check for the header's presence first, or ensure this is only invoked at the request boundary. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [109-115] -if (!dlsRequestHeaders.isEmpty()) { +if (!dlsRequestHeaders.isEmpty() && threadContext.getHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS) == null) { 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: Calling `putHeader` twice on a `ThreadContext` with the same key can throw; this defensive check may help avoid issues if the method is inadvertently invoked more than once, though the current call sites appear to be at request boundaries.	Low
Possible issue	Guard against null header settings lookup `toDlsRequestHeader` receives `null` for `dlsRequestHeaderSettings` when the header name in the request differs in case from the configured name but is not lowercase (e.g. request header `X-Example-Header` looked up as `x-example-header` succeeds, but the filter uses case-insensitive match). Since `allowedDlsRequestHeaders` keys are lowercased, but the request key may be mixed-case, `allowedDlsRequestHeaders.get(e.getKey().toLowerCase())` should be safe — however if header configuration name itself is not lowercase, the lookup returns null. Ensure the settings map is always keyed by a normalized lowercase name and add a null check to fail fast with a clear error. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [70-81] -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 -> { + final var s = allowedDlsRequestHeaders.get(e.getKey().toLowerCase()); + if (s == null) { + throw new IllegalStateException("No DLS request header settings found for " + e.getKey()); + } + return toDlsRequestHeader(s, e.getKey(), e.getValue()); + }) .toList(); Suggestion importance[1-10]: 4 __ Why: The filter uses case-insensitive matching against lowercase keys, so a null return is only possible in edge cases; the added null check is defensive but of limited practical impact.	Low
General	Validate config entries with clearer errors If `name` is missing from a group setting, this will produce a `DlsRequestHeaderSettings` with a null name and throw NPE at `toLowerCase()`. Additionally, invalid regex patterns will throw `PatternSyntaxException` at settings parse time with a confusing message. Validate presence of `name` and wrap `Pattern.compile` to provide a clearer error indicating which config entry is invalid. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [162-176] 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"), - 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) - ) - ) + .map(entry -> { + final var s = entry.getValue(); + final var name = s.get("name"); + if (name == null \|\| name.isBlank()) { + throw new IllegalArgumentException("Missing 'name' for DLS request header config: " + entry.getKey()); + } + final Pattern pattern; + try { + pattern = Pattern.compile(s.get("validationRegex", "^$")); + } catch (Exception ex) { + throw new IllegalArgumentException("Invalid validationRegex for DLS request header " + name, ex); + } + return new DlsRequestHeaderSettings(name, s.getAsBoolean("isMultiValue", false), pattern, s.getAsInt("maxValueLength", 256)); + }) .collect(Collectors.toUnmodifiableMap(e -> e.name.toLowerCase(), identity())); } Suggestion importance[1-10]: 3 __ Why: Improving error messages for missing name or invalid regex enhances usability but is a minor improvement addressing edge cases in admin configuration.	Low

Previous suggestions

Suggestions up to commit 842817f

Category	Suggestion	Impact
Security	Escape values and avoid parallelStream Values are placed into a JSON/DLS query string without escaping quotes or backslashes, which can break the resulting query or enable injection into the DLS query even though the values passed the regex filter. Additionally, `parallelStream` here is unnecessary and may reorder values, which changes query semantics. Use a sequential stream and properly escape the values. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [211-216] 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]: 8 __ Why: Valid security concern: even though values are regex-validated, the default regex or admin-defined regex may allow characters like quotes/backslashes that would break the resulting DLS JSON query. Also `parallelStream` may reorder values which can affect query semantics.	Medium
Security	Enforce header count limit earlier The `MAX_HEADER_COUNT` check runs after `toDlsRequestHeader` has already iterated and validated all header values via regex. A malicious client sending a very large number of header values could cause significant CPU work (regex matching per value) before the DOS limit is enforced. Move the total-count check before the per-value validation/mapping to enforce the limit early. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [83-93] 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 DoS mitigation: performing the count check before per-value regex validation prevents excessive CPU work on malicious inputs.	Low
Possible issue	Normalize header name matching consistently `allowedDlsRequestHeaders` is keyed by lowercase names, but the filter uses `equalsIgnoreCase` against original (mixed-case) keys. If a client sends a header whose casing does not match any key exactly by `equalsIgnoreCase` for the configured `name` value (which may itself be non-lowercase in config), the lookup via `e.getKey().toLowerCase()` may still miss because the config `name` was normalized to lowercase for the map key but not for `equalsIgnoreCase` matching. Normalize both sides consistently to avoid a `NullPointerException` in `toDlsRequestHeader` when the settings `name` casing differs from the header casing. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [70-81] 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())) .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())) .toList(); Suggestion importance[1-10]: 7 __ Why: Valid observation. The filter uses `equalsIgnoreCase` against `allowedDlsRequestHeaders.keySet()` which is already lowercased, so the semantics are correct, but the improved consistency (using `containsKey` with `toLowerCase`) avoids subtle bugs and matches the later lookup pattern.	Medium
General	Handle serialization errors explicitly The method is declared without `throws`, but `writeValueAsString` on Jackson's writer may throw a checked `JsonProcessingException` (or Jackson 3's equivalent). If so, this will not compile; if it throws unchecked, callers get an opaque failure. Wrap the serialization to convert failures into a clear `IllegalStateException` (or declare the checked exception) so the request fails deterministically instead of leaking an unrelated exception to the client. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [107-115] // 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 if (!dlsRequestHeaders.isEmpty()) { - final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper() - .writerFor(new TypeReference<List<DlsRequestHeader>>() { - }) - .writeValueAsString(dlsRequestHeaders); + final String serializedDlsRequestHeaders; + try { + serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper() + .writerFor(new TypeReference<List<DlsRequestHeader>>() { + }) + .writeValueAsString(dlsRequestHeaders); + } catch (Exception e) { + throw new IllegalStateException("Failed to serialize DLS request headers", e); + } threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); } } Suggestion importance[1-10]: 3 __ Why: Minor improvement for error clarity. Jackson's `writeValueAsString` in tools.jackson may throw runtime exceptions; wrapping is a defensive practice but not critical.	Low

Suggestions up to commit 7e6cb3b

Category	Suggestion	Impact
Security	Escape special characters when serializing values The `serialize()` method embeds header values directly into a DLS query as quoted strings without escaping. If a header value contains a double quote or backslash (even one that passes the regex if a permissive regex is used), it would break the JSON structure of the DLS query or allow query injection. Escape special characters (at minimum `"` and `</code>) before embedding, and prefer sequential over parallel stream for such small collections.` src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [211-216] 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: Legitimate security concern: unescaped double quotes/backslashes in header values could break JSON query structure or enable injection, though validationRegex partially mitigates this.	Medium
Security	Guard against external header injection `extractAndStoreDlsRequestHeaders` is called from both REST and gRPC filters early in request processing. If this header is already present in the incoming request headers (e.g., a malicious client sets `_opendistro_security_dls_request_headers` directly), `threadContext.putHeader` will throw an IllegalStateException, or worse, the transient value from the untrusted source could be honored. Verify that any pre-existing header value is stripped/overwritten to prevent header injection from external clients. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [107-115] // 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 if (!dlsRequestHeaders.isEmpty()) { + if (threadContext.getHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS) != null) { + throw new IllegalStateException("DLS request headers already present in thread context; possible header injection"); + } final var serializedDlsRequestHeaders = DefaultObjectMapper.objectMapper() .writerFor(new TypeReference<List<DlsRequestHeader>>() { }) .writeValueAsString(dlsRequestHeaders); threadContext.putHeader(OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, serializedDlsRequestHeaders); } } Suggestion importance[1-10]: 6 __ Why: Reasonable defense-in-depth concern about clients potentially injecting the internal header, though the fix (throwing an exception) may not be the ideal approach—overwriting/stripping would be safer.	Low
Possible issue	Use Locale.ROOT for header name lowercasing The `allowedDlsRequestHeaders` map keys are lowercased, but the lookup uses `e.getKey().toLowerCase()` while the filter uses case-insensitive comparison via stream. This is inconsistent and inefficient. Also, `toLowerCase()` without a `Locale` is locale-sensitive (e.g., Turkish locale) which can cause header names to fail lookup. Use `Locale.ROOT` explicitly to avoid locale-dependent behavior. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [70-81] 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: Valid point about locale-sensitive `toLowerCase()` causing potential inconsistency. The suggestion improves robustness though the practical impact for ASCII header names is limited.	Low
General	Use Locale.ROOT for lowercasing header names Using `toLowerCase()` without a Locale is locale-sensitive and may produce inconsistent keys across environments (e.g., Turkish locale converts 'I' unexpectedly). Since DLS query templates reference these keys by exact name, this can cause silent substitution failures. Use `Locale.ROOT` explicitly. 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: Valid locale-safety concern for `toLowerCase()`, consistent with suggestion 1. Impact is limited for typical ASCII header names but improves correctness.	Low

Suggestions up to commit 756f8d4

Category	Suggestion	Impact
Security	Properly escape values in DLS output Values used in DLS queries are inserted as JSON string literals but are only validated by a user-defined regex. If the regex allows characters like `"` or `</code>, the interpolation could break the query or enable injection. Properly JSON-escape each value instead of naive quoting to make the output robust regardless of regex configuration.` src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [211-216] 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 -> { + try { + return DefaultObjectMapper.objectMapper().writeValueAsString(s); + } catch (Exception e) { + throw new IllegalStateException(e); + } + }) + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 6 __ Why: The suggestion highlights a legitimate concern about injection risk if the admin-configured regex is too permissive. However, the validation regex is admin-controlled, mitigating the risk. Proper JSON escaping would add defense in depth.	Low
General	Use Locale.ROOT for case conversion `String.toLowerCase()` without a locale is locale-sensitive and can produce unexpected results (e.g., Turkish locale with "I"). Since header names are ASCII and the allowlist keys are stored lowercased, use `Locale.ROOT` explicitly to ensure consistent case-insensitive matching. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [70-81] 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: Valid concern about locale-dependent `toLowerCase()` causing inconsistent behavior in Turkish locale. Using `Locale.ROOT` improves correctness for case-insensitive header matching, though the impact is minor in practice since headers are ASCII.	Low
	Use Locale.ROOT for lowercasing keys `String.toLowerCase()` without a locale can produce different results based on the JVM default locale (e.g., Turkish "I"). Since DLS variable names should be locale-independent, use `Locale.ROOT` to guarantee consistent substitution keys. 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: Same locale-independence concern as suggestion 1, applied to substitution keys. Valid but minor correctness improvement given headers are typically ASCII.	Low
	Handle serialization exception explicitly `extractAndStoreDlsRequestHeaders` is declared without `throws`, but `writeValueAsString` may throw a checked exception. This can cause a compilation issue or force silent swallowing. Wrap in try/catch and convert to a runtime exception, or explicitly declare the exception, to ensure serialization errors are handled deterministically. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [109-115] if (!dlsRequestHeaders.isEmpty()) { - 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: Jackson's `writeValueAsString` in newer versions throws unchecked exceptions, so the compilation issue claim may be inaccurate. The improvement is a minor code quality suggestion for explicit error handling.	Low

Suggestions up to commit f489388

Category	Suggestion	Impact
Security	Escape header values to prevent DLS injection The serialized value is inlined verbatim into a JSON DLS query, so any `"` or `</code> in a header value would break out of the string context and allow query injection. Escape JSON special characters (at minimum backslash and double-quote) before quoting. Also, parallelStream is unnecessary overhead here.` src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [211-216] 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]: 8 __ Why: Legitimate security concern: header values are inlined into JSON DLS queries without escaping quotes/backslashes, potentially enabling query injection despite the regex validation (though the default regex may mitigate this if properly configured).	Medium
Possible issue	Avoid duplicate-key exceptions from header casing HTTP headers can appear multiple times with different casing and `toUnmodifiableMap` will throw an `IllegalStateException` on duplicate keys, which could be triggered by a client repeating a header with mixed cases. Also, `allowedDlsRequestHeaders` keys are already lowercased, so use `containsKey(e.getKey().toLowerCase(Locale.ROOT))` and a merge function to safely combine values. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [70-81] 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( + e -> e.getKey().toLowerCase(java.util.Locale.ROOT), + Map.Entry::getValue, + (a, b) -> { final var merged = new java.util.ArrayList<String>(a.size() + b.size()); merged.addAll(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]: 7 __ Why: Valid concern: if a client sends the same header with different casing, `toUnmodifiableMap` would throw `IllegalStateException` on duplicate keys, which is a real edge case bug.	Medium
Possible issue	Handle checked exception during header serialization `writeValueAsString` on Jackson's `ObjectWriter` typically declares a checked `IOException` (or `JsonProcessingException`), but this method's signature does not declare it. Wrap the call in a try/catch and rethrow as an unchecked exception (or declare the throws) to avoid a compilation error and to produce a clear failure at runtime. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [110-114] -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 RuntimeException("Failed to serialize DLS request headers", e); +} Suggestion importance[1-10]: 3 __ Why: In Jackson 3.x (`tools.jackson.core`), `writeValueAsString` throws unchecked exceptions, not checked ones. The suggestion is likely incorrect for the Jackson version being used, but wrapping is not strictly required.	Low
General	Use Locale.ROOT for case normalization Use an explicit `Locale.ROOT` when lowercasing to avoid locale-sensitive behavior (e.g., Turkish locale) causing header names to be normalized differently than the allowlist keys, which also lowercase without a locale. Consistent lowercasing prevents subtle mismatches during variable substitution. 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: Valid minor concern about locale-sensitive `toLowerCase()` causing inconsistent behavior (e.g., Turkish locale), which is a known Java pitfall for security-relevant matching.	Low

Suggestions up to commit d86d247

Category	Suggestion	Impact
Security	Escape quotes in header values The header values are inserted directly into a JSON/DLS query template via quotation without escaping. If a header value contains a double quote or backslash (even if allowed by the regex), it can break out of the string context and enable DLS query injection. Escape special characters like `"` and `</code> before wrapping.` src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [204-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.parallelStream() + .map(s -> "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"") + .collect(Collectors.joining(",")); } } Suggestion importance[1-10]: 7 __ Why: Legitimate security concern: while the configured validation regex may restrict special chars, the default serialization does not defensively escape quotes or backslashes, which could allow DLS query injection if regex is misconfigured.	Medium
Security	Fail fast on header count to prevent DoS The DoS protection check runs after `toDlsRequestHeader` has already iterated through all header values (including regex validation and length checks). An attacker could send many headers with large values to consume CPU before the `MAX_HEADER_COUNT` check triggers. Move the count check before the mapping step to fail fast. 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]: 6 __ Why: Valid observation: moving the `MAX_HEADER_COUNT` check before the mapping step would fail faster and reduce CPU exposure to malicious inputs. Impact is moderate since regex validation is bounded per header.	Low
General	Avoid key collision on case-variant headers If the incoming headers contain duplicate keys differing only in case (e.g., `X-Example-Header` and `x-example-header`), `toUnmodifiableMap` will throw `IllegalStateException` due to key collision after later `toLowerCase()` lookup. Additionally, the subsequent `.get(e.getKey().toLowerCase())` assumes the header name normalizes; consider consistently normalizing keys and merging duplicate values. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [63-69] 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; } + )); Suggestion importance[1-10]: 6 __ Why: Valid edge case: duplicate header names differing only in case would cause `toUnmodifiableMap` to throw `IllegalStateException`. Normalizing keys and merging values improves robustness.	Low
Possible issue	Handle serialization exception properly `writeValueAsString` may throw a checked serialization exception, but the method signature does not declare it and no try/catch is present. This will lead to a compile error or unhandled exception at runtime. Wrap the serialization in a proper exception handler that surfaces a meaningful error. src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java [102-108] if (!dlsRequestHeaders.isEmpty()) { - 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: Jackson's `writeValueAsString` in newer versions throws unchecked exceptions, so the claim of a compile error is likely incorrect. The suggestion to add a meaningful error wrapper is minor.	Low

github-actions · 2026-07-17T09:54:50Z

Persistent review updated to latest commit a0a03b4

github-actions · 2026-07-17T10:15:02Z

Persistent review updated to latest commit b1dde53

github-actions · 2026-07-17T10:29:10Z

Persistent review updated to latest commit 956ae1e

github-actions · 2026-07-17T11:27:25Z

Persistent review updated to latest commit 8fb20cf

github-actions · 2026-07-17T14:08:22Z

Persistent review updated to latest commit 7d6694c

github-actions · 2026-07-17T14:27:34Z

Persistent review updated to latest commit 67a2d5a

rursprung · 2026-07-22T19:35:46Z

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 · 2026-07-22T19:37:43Z

Persistent review updated to latest commit e8f9304

github-actions · 2026-08-10T12:34:29Z

Persistent review updated to latest commit 9fa7e22

github-actions · 2026-08-10T12:38:58Z

Persistent review updated to latest commit 7d0b476

rursprung · 2026-08-10T14:34:36Z

odd that HeaderAttrInDlsIntegrationTest.testGrpcChannel fails only on ubuntu-latest with JDK 25 but not any of the other three builds in the matrix?

2026-08-10T12:50:53.4658382Z 2026-08-10 12:50:53 opensearch[cluster_manager_0][grpc][T#4] ERROR SecurityFilter:376 - No user found for indices:data/read/search from null GRPC via null {_opensearch_security_dls_request_headers=[{"class":"SingleValueDlsRequestHeader","name":"x-example-header","value":"foobar"}]}
2026-08-10T12:50:53.5668078Z 
2026-08-10T12:50:53.5672498Z > Task :integrationTest
2026-08-10T12:50:53.5673608Z 
2026-08-10T12:50:53.5674368Z HeaderAttrInDlsIntegrationTest > testGrpcChannel FAILED
2026-08-10T12:50:53.5676859Z     io.grpc.StatusRuntimeException: INTERNAL: OpenSearchSecurityException[No user found for indices:data/read/search]; details={"error":{"root_cause":[{"type":"security_exception","reason":"No user found for indices:data/read/search"}],"type":"security_exception","reason":"No user found for indices:data/read/search"},"status":500}
2026-08-10T12:50:53.5679582Z         at app//io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:368)
2026-08-10T12:50:53.5680682Z         at app//io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:349)
2026-08-10T12:50:53.5681759Z         at app//io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:174)
2026-08-10T12:50:53.5683265Z         at app//org.opensearch.protobufs.services.SearchServiceGrpc$SearchServiceBlockingStub.search(SearchServiceGrpc.java:298)
2026-08-10T12:50:53.5684942Z         at app//org.opensearch.security.grpc.GrpcHelpers.doMatchAll(GrpcHelpers.java:334)
2026-08-10T12:50:53.5686607Z         at app//org.opensearch.security.dlsfls.HeaderAttrInDlsIntegrationTest.testGrpcChannel(HeaderAttrInDlsIntegrationTest.java:174)
2026-08-10T12:50:53.8655426Z

i'll have a look tomorrow

nibix

I have rerun the tests, it looks like they are completely green now. Added just some minor nitpicks.

rursprung · 2026-08-12T10:22:16Z

I have rerun the tests, it looks like they are completely green now. Added just some minor nitpicks.

awesome, thanks! i've incorporated fixes for your findings and rebased. the test runs fine locally => should be good now (i guess any CI failures will just be flaky tests again)

github-actions · 2026-08-12T10:23:42Z

Persistent review updated to latest commit d86d247

codecov · 2026-08-12T12:53:56Z

Codecov Report

❌ Patch coverage is 96.51163% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.46%. Comparing base (f9e24a7) to head (842817f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...curity/privileges/PrivilegesEvaluationContext.java	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6310      +/-   ##
==========================================
+ Coverage   75.40%   75.46%   +0.05%     
==========================================
  Files         456      457       +1     
  Lines       30255    30339      +84     
  Branches     4575     4585      +10     
==========================================
+ Hits        22815    22896      +81     
- Misses       5304     5312       +8     
+ Partials     2136     2131       -5

Files with missing lines	Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java	`84.06% <100.00%> (+0.01%)`	⬆️
...opensearch/security/filter/SecurityGrpcFilter.java	`68.62% <100.00%> (+3.32%)`	⬆️
...opensearch/security/filter/SecurityRestFilter.java	`88.95% <100.00%> (+0.06%)`	⬆️
...opensearch/security/privileges/UserAttributes.java	`95.00% <100.00%> (+0.55%)`	⬆️
...es/actionlevel/legacy/PrivilegesEvaluatorImpl.java	`87.06% <100.00%> (+0.04%)`	⬆️
...s/actionlevel/nextgen/PrivilegesEvaluatorImpl.java	`87.68% <100.00%> (+0.04%)`	⬆️
...urity/privileges/dlsfls/DlsRequestHeadersUtil.java	`100.00% <100.00%> (ø)`
...g/opensearch/security/support/ConfigConstants.java	`96.55% <ø> (ø)`
...search/security/transport/SecurityInterceptor.java	`80.00% <100.00%> (+0.76%)`	⬆️
...rch/security/transport/SecurityRequestHandler.java	`59.17% <100.00%> (+1.24%)`	⬆️
... and 1 more

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

nibix

Thank you. I have added two more tiny remarks (sorry!), but still the current state is already approvable.

github-actions · 2026-08-19T07:15:12Z

Persistent review updated to latest commit f489388

github-actions · 2026-08-20T08:23:09Z

Persistent review updated to latest commit 756f8d4

the plugin configuration and the authorization handling is always the same => these can be centralized in `GrpcHelpers` as well. Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com>

if one of the lists didn't contain any entries it'd just crash. now it causes a proper test failure, which is clearer. Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com>

rursprung · 2026-08-20T09:41:14Z

CI builds failed due to an infrastructure issue (i rebased to try to trigger them again, but that led to the same problem). i've reported it on slack

nibix · 2026-08-20T10:20:12Z

Retriggered the jobs ...

github-actions · 2026-08-20T10:21:08Z

Persistent review updated to latest commit 7e6cb3b

nibix · 2026-08-20T12:34:47Z

The CI now seems to be completely stuck, some jobs just seem to be hanging. Unfortunately I cannot retrigger jobs in this state.

Let's maybe wait a bit whether these will wake up again. Otherwise, @rursprung maybe force-push once again to retrigger the whole CI.

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 opensearch-project#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). resolves opensearch-project#6265 Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com>

github-actions · 2026-08-20T12:48:17Z

Persistent review updated to latest commit 842817f

rursprung · 2026-08-20T12:50:44Z

maybe force-push once again to retrigger the whole CI.

done, though there are flaky tests again.

final results: one test run failed with the usual:

java.io.IOException: Failed to load eclipse jdt formatter: java.lang.RuntimeException: java.net.SocketTimeoutException: timeout

and one integration-tests run with this:

2026-08-20T13:12:19.7276769Z Tests with failures:
2026-08-20T13:12:19.7277561Z - org.opensearch.security.dlsfls.HeaderAttrInDlsIntegrationTest.testGrpcChannel
2026-08-20T13:12:19.7279069Z - org.opensearch.security.StandaloneAuditSinksTest.internalOpenSearchSinkShouldCreateAuditIndex
2026-08-20T13:12:19.7280476Z - org.opensearch.security.privileges.ApiTokenTest.testFlushCacheReloadsFromIndex

in both cases the other matrix entries worked fine => these are flaky

i checked the logs for dlsfls.HeaderAttrInDlsIntegrationTest.testGrpcChannel since i introduced it with this PR, but don't see why it'd fail with this (esp. because it worked fine on windows & both jdk21 runs and it runs fine locally and ran fine in a previous CI run before i fixed the nit-picking, which was unrelated / javadocs):

2026-08-20T13:11:49.8335783Z HeaderAttrInDlsIntegrationTest > testGrpcChannel FAILED
2026-08-20T13:11:49.8352078Z io.grpc.StatusRuntimeException: INTERNAL: OpenSearchSecurityException[No user found for indices:data/read/search]; details={"error":{"root_cause":[{"type":"security_exception","reason":"No user found for indices:data/read/search"}],"type":"security_exception","reason":"No user found for indices:data/read/search"},"status":500}
2026-08-20T13:11:49.8354682Z at app//io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:368)
2026-08-20T13:11:49.8355965Z at app//io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:349)
2026-08-20T13:11:49.8357025Z at app//io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:174)
2026-08-20T13:11:49.8358521Z at app//org.opensearch.protobufs.services.SearchServiceGrpc$SearchServiceBlockingStub.search(SearchServiceGrpc.java:298)
2026-08-20T13:11:49.8360045Z at app//org.opensearch.security.grpc.GrpcHelpers.doMatchAll(GrpcHelpers.java:334)
2026-08-20T13:11:49.8361847Z at app//org.opensearch.security.dlsfls.HeaderAttrInDlsIntegrationTest.testGrpcChannel(HeaderAttrInDlsIntegrationTest.java:181)

it seems that in some cases it either doesn't do the user setup correctly or uses it wrong - either way i think that's probably an issue with the test infrastructure code or with the gRPC code causing it to be flaky.

re-running the test will probably make it pass

nibix · 2026-08-20T17:06:23Z

dlsfls.HeaderAttrInDlsIntegrationTest.testGrpcChannel

I think I also saw that failure during previous test runs. I am going to check whether I can identify any issue that might be causing this.

github-actions · 2026-08-20T17:51:31Z

Persistent review updated to latest commit 551d00e

rursprung requested review from DarshitChanpura, Rishav9852Kumar, RyanL1997, cwperks, derek-ho, finnegancarroll, nibix, reta, shikharj05 and willyborankin as code owners July 17, 2026 09:45

rursprung force-pushed the user-attr-from-http-header branch from 1ead70c to a0a03b4 Compare July 17, 2026 09:52

rursprung force-pushed the user-attr-from-http-header branch from a0a03b4 to b1dde53 Compare July 17, 2026 10:12

rursprung force-pushed the user-attr-from-http-header branch from b1dde53 to 956ae1e Compare July 17, 2026 10:27

rursprung force-pushed the user-attr-from-http-header branch from 956ae1e to 8fb20cf Compare July 17, 2026 11:25

reta reviewed Jul 17, 2026

View reviewed changes

Comment thread src/integrationTest/java/org/opensearch/security/dlsfls/HeaderAttrInDlsIntegrationTest.java

rursprung mentioned this pull request Jul 17, 2026

[FEATURE] migrate DLS variable substitution away from text-based substitution #6311

Open

rursprung force-pushed the user-attr-from-http-header branch from 8fb20cf to 7d6694c Compare July 17, 2026 14:06

rursprung force-pushed the user-attr-from-http-header branch from 7d6694c to 67a2d5a Compare July 17, 2026 14:24

nibix reviewed Jul 17, 2026

View reviewed changes

Comment thread src/integrationTest/java/org/opensearch/security/dlsfls/HeaderAttrInDlsIntegrationTest.java Outdated

nibix reviewed Jul 17, 2026

View reviewed changes

Comment thread src/integrationTest/java/org/opensearch/security/dlsfls/HeaderAttrInDlsIntegrationTest.java Outdated

rursprung force-pushed the user-attr-from-http-header branch from e8f9304 to 9fa7e22 Compare August 10, 2026 12:31

rursprung force-pushed the user-attr-from-http-header branch from 9fa7e22 to 7d0b476 Compare August 10, 2026 12:36

nibix reviewed Aug 12, 2026

View reviewed changes

Comment thread src/integrationTest/java/org/opensearch/security/dlsfls/HeaderAttrInDlsIntegrationTest.java Outdated

Comment thread src/integrationTest/java/org/opensearch/security/dlsfls/HeaderAttrInDlsIntegrationTest.java Outdated

rursprung force-pushed the user-attr-from-http-header branch from 7d0b476 to d86d247 Compare August 12, 2026 10:21

nibix approved these changes Aug 13, 2026

View reviewed changes

Comment thread src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java Outdated

Comment thread src/main/java/org/opensearch/security/privileges/dlsfls/DlsRequestHeadersUtil.java Outdated

rursprung force-pushed the user-attr-from-http-header branch from d86d247 to f489388 Compare August 19, 2026 07:12

rursprung added 2 commits August 20, 2026 11:19

simplify gRPC tests by centralizing more utilities

598fd3a

the plugin configuration and the authorization handling is always the same => these can be centralized in `GrpcHelpers` as well. Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com>

WhoAmITests: make test failure more explicit

09e0a7f

if one of the lists didn't contain any entries it'd just crash. now it causes a proper test failure, which is clearer. Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com>

rursprung force-pushed the user-attr-from-http-header branch from 756f8d4 to 7e6cb3b Compare August 20, 2026 09:19

rursprung force-pushed the user-attr-from-http-header branch from 7e6cb3b to 842817f Compare August 20, 2026 12:45

Merge branch 'main' into user-attr-from-http-header

551d00e

Uh oh!

Conversation

rursprung commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Testing

Check List

Uh oh!

rursprung commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Analyzer ❗

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 551d00e)

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

Uh oh!

Uh oh!

rursprung commented Jul 22, 2026

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

github-actions Bot commented Aug 10, 2026

Uh oh!

github-actions Bot commented Aug 10, 2026

Uh oh!

rursprung commented Aug 10, 2026

Uh oh!

nibix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rursprung commented Aug 12, 2026

Uh oh!

github-actions Bot commented Aug 12, 2026

Uh oh!

codecov Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nibix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 19, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

rursprung commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nibix commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

nibix commented Aug 20, 2026

rursprung commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading

(Review updated until commit `551d00e`)

github-actions Bot commented Jul 17, 2026 •

edited

Loading

codecov Bot commented Aug 12, 2026 •

edited

Loading

rursprung commented Aug 20, 2026 •

edited

Loading

rursprung commented Aug 20, 2026 •

edited

Loading