Tweak lang-mustache factory (#92211)#4
Open
MitchLewis930 wants to merge 1 commit intopr_014_beforefrom
Open
Conversation
This commits makes a few very minor tweaks to our Mustache scripting capabilities.
1. Switches from `DefaultMustacheFactory` to `SafeMustacheFactory`
In the event that the security manager were disabled (or removed, as is threatened in subsequent JDK releases), Mustache's "partial template" (`{{>partial}}`) support is a security risk because it would allow reading from an arbitrary URL or file on disk. Switching to the "Safe" version and passing in an empty set into the parent constructor disallows using partial Mustache templates. This also switches from `Function<String, String>` to the `TemplateFunction` value for the built-in functions.
3. Minor internal optimization
This removes useless grouping for one of the built-in mustache functions, and removes the call to `CollectionUtils.ensureNoSelfReferences` used in `CustomReflectionObjectHandler`. This check during stringification should not be necessary because Mustache templates without "partial" support cannot be self-referencing.
There was a problem hiding this comment.
Pull request overview
This PR refactors the lang-mustache factory implementation by moving self-reference validation from Mustache rendering to the payload creation phase in Watcher. The change prevents self-referencing data structures from being detected during template rendering and instead catches them earlier when payloads are constructed.
Changes:
- Moved self-reference validation from
CustomReflectionObjectHandler.stringify()toPayload.Simpleconstructor - Changed
CustomMustacheFactoryto extendSafeMustacheFactoryinstead ofDefaultMustacheFactory - Updated test expectations to reflect that exceptions now originate from transform actions rather than Mustache rendering
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| x-pack/plugin/watcher/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/painless/40_exception.yml | Updated test to verify self-reference errors occur during transform instead of Mustache rendering |
| x-pack/plugin/watcher/qa/rest/build.gradle | Added skip rule for updated test in v7 compatibility mode |
| x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/watch/Payload.java | Added self-reference validation to Simple payload constructor |
| modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomReflectionObjectHandler.java | Removed stringify override that performed self-reference checking |
| modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java | Changed parent class to SafeMustacheFactory and updated function signatures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static class CustomJoinerCode extends JoinerCode { | ||
|
|
||
| private static final Pattern PATTERN = Pattern.compile("^(?:" + CODE + " delimiter='(.*)')$"); | ||
| private static final Pattern PATTERN = Pattern.compile("^" + CODE + " delimiter='(.*)'$"); |
There was a problem hiding this comment.
The regex pattern has been changed to remove the non-capturing group (?:...), which may affect pattern matching behavior. The original pattern ^(?:" + CODE + " delimiter='(.*)')$ had specific grouping logic that is now removed. Verify that the pattern still correctly matches the expected input format and that any code relying on capture group indices is updated accordingly.
Suggested change
| private static final Pattern PATTERN = Pattern.compile("^" + CODE + " delimiter='(.*)'$"); | |
| private static final Pattern PATTERN = Pattern.compile("^(?:" + CODE + " delimiter='(.*)')$"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR_014