[8.12] Adding check to escape pushes to output in kbn-handlebars (ela…

…stic#175490) (elastic#175828)

# Backport

This will backport the following commits from `main` to `8.12`:
- [Adding check to escape pushes to output in kbn-handlebars
(elastic#175490)](elastic#175490)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Kurt","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-01-29T17:02:20Z","message":"Adding
check to escape pushes to output in kbn-handlebars (elastic#175490)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/174865\r\n\r\nA bug was
reported that noticed a difference between kbn-handlbars and\r\nthe
original handlebars when it came to escaping nested
values.\r\n\r\nAdditional checks have been added to verify whether
escaping should be\r\nperformed.\r\n\r\n## Testing\r\n\r\nWhen reviewing
the unit tests notice the permutations of\r\nnested/un-nested inputs for
each type of template(simple, helper, block)\r\nwith all values
`noEscape` compiler option (default(false), false, true)\r\n\r\n##
Release Note\r\n\r\nFixed issue with `@kbn-handlebars` where nested
inputs were not being\r\nescaped
properly\r\n\r\n---------\r\n\r\nCo-authored-by: Jeramy Soucy
<[email protected]>","sha":"100975f280704c0cb6210c0817052afce71f7f2d","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Security","backport:all-open","v8.13.0"],"title":"Adding
check to escape pushes to output in
kbn-handlebars","number":175490,"url":"https://github.com/elastic/kibana/pull/175490","mergeCommit":{"message":"Adding
check to escape pushes to output in kbn-handlebars (elastic#175490)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/174865\r\n\r\nA bug was
reported that noticed a difference between kbn-handlbars and\r\nthe
original handlebars when it came to escaping nested
values.\r\n\r\nAdditional checks have been added to verify whether
escaping should be\r\nperformed.\r\n\r\n## Testing\r\n\r\nWhen reviewing
the unit tests notice the permutations of\r\nnested/un-nested inputs for
each type of template(simple, helper, block)\r\nwith all values
`noEscape` compiler option (default(false), false, true)\r\n\r\n##
Release Note\r\n\r\nFixed issue with `@kbn-handlebars` where nested
inputs were not being\r\nescaped
properly\r\n\r\n---------\r\n\r\nCo-authored-by: Jeramy Soucy
<[email protected]>","sha":"100975f280704c0cb6210c0817052afce71f7f2d"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/175490","number":175490,"mergeCommit":{"message":"Adding
check to escape pushes to output in kbn-handlebars (elastic#175490)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/174865\r\n\r\nA bug was
reported that noticed a difference between kbn-handlbars and\r\nthe
original handlebars when it came to escaping nested
values.\r\n\r\nAdditional checks have been added to verify whether
escaping should be\r\nperformed.\r\n\r\n## Testing\r\n\r\nWhen reviewing
the unit tests notice the permutations of\r\nnested/un-nested inputs for
each type of template(simple, helper, block)\r\nwith all values
`noEscape` compiler option (default(false), false, true)\r\n\r\n##
Release Note\r\n\r\nFixed issue with `@kbn-handlebars` where nested
inputs were not being\r\nescaped
properly\r\n\r\n---------\r\n\r\nCo-authored-by: Jeramy Soucy
<[email protected]>","sha":"100975f280704c0cb6210c0817052afce71f7f2d"}}]}]
BACKPORT-->

Co-authored-by: Kurt <[email protected]>

packages/kbn-handlebars/index.test.ts

@@ -20,18 +20,226 @@ it('Handlebars.create', () => {
 
 describe('Handlebars.compileAST', () => {
   describe('compiler options', () => {
-    it('noEscape', () => {
-      expectTemplate('{{value}}').withInput({ value: '<foo>' }).toCompileTo('&lt;foo&gt;');
-
-      expectTemplate('{{value}}')
-        .withCompileOptions({ noEscape: false })
-        .withInput({ value: '<foo>' })
-        .toCompileTo('&lt;foo&gt;');
-
-      expectTemplate('{{value}}')
-        .withCompileOptions({ noEscape: true })
-        .withInput({ value: '<foo>' })
-        .toCompileTo('<foo>');
+    describe('noEscape', () => {
+      describe('basic', () => {
+        it('should escape a non-nested value with default value set for `noEscape`', () => {
+          expectTemplate('{{value}}').withInput({ value: '<foo>' }).toCompileTo('&lt;foo&gt;');
+        });
+
+        it('should escape a nested value with default value set for `noEscape`', () => {
+          expectTemplate('{{nested.value}}')
+            .withInput({ nested: { value: '<foo>' } })
+            .toCompileTo('&lt;foo&gt;');
+        });
+
+        it('should escape a non-nested value with `noEscape` set to false', () => {
+          expectTemplate('{{value}}')
+            .withCompileOptions({ noEscape: false })
+            .withInput({ value: '<foo>' })
+            .toCompileTo('&lt;foo&gt;');
+        });
+
+        it('should escape a nested value with `noEscape` set to false', () => {
+          expectTemplate('{{nested.value}}')
+            .withCompileOptions({ noEscape: false })
+            .withInput({ nested: { value: '<foo>' } })
+            .toCompileTo('&lt;foo&gt;');
+        });
+
+        it('should not escape a non-nested value with `noEscape` set to true', () => {
+          expectTemplate('{{value}}')
+            .withCompileOptions({ noEscape: true })
+            .withInput({ value: '<foo>' })
+            .toCompileTo('<foo>');
+        });
+
+        it('should not escape a nested value with `noEscape` set to true', () => {
+          expectTemplate('{{nested.value}}')
+            .withCompileOptions({ noEscape: true })
+            .withInput({ nested: { value: '<foo>' } })
+            .toCompileTo('<foo>');
+        });
+      });
+
+      describe('known helper', () => {
+        it('should escape a non-nested value with a known helper and default value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{value}}{{/with}}')
+            .withInput({ foo: { value: '<bar>' } })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should escape a nested value with a known helper and default value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{nested.value}}{{/with}}')
+            .withInput({ foo: { nested: { value: '<bar>' } } })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should escape a non-nested value with a known helper and false value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{value}}{{/with}}')
+            .withCompileOptions({ noEscape: false })
+            .withInput({ foo: { value: '<bar>' } })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should escape a nested value with a known helper and false value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{nested.value}}{{/with}}')
+            .withCompileOptions({ noEscape: false })
+            .withInput({ foo: { nested: { value: '<bar>' } } })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should not escape a non-nested value with a known helper and true value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{value}}{{/with}}')
+            .withCompileOptions({ noEscape: true })
+            .withInput({ foo: { value: '<bar>' } })
+            .toCompileTo('<bar>');
+        });
+
+        it('should not escape a nested value with a known helper and true value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{nested.value}}{{/with}}')
+            .withCompileOptions({ noEscape: true })
+            .withInput({ foo: { nested: { value: '<bar>' } } })
+            .toCompileTo('<bar>');
+        });
+      });
+
+      describe('unknown helper', () => {
+        it('should escape a non-nested value with an unknown helper and no value set for `noEscape`', () => {
+          expectTemplate('{{foo value}}')
+            .withHelper('foo', (value: string) => {
+              return value + 'baz';
+            })
+            .withInput({ value: '<bar>' })
+            .toCompileTo('&lt;bar&gt;baz');
+        });
+
+        it('should escape a nested value with an unknown helper and no value set for `noEscape`', () => {
+          expectTemplate('{{foo nested.value}}')
+            .withHelper('foo', (value: string) => {
+              return value + 'baz';
+            })
+            .withInput({ nested: { value: '<bar>' } })
+            .toCompileTo('&lt;bar&gt;baz');
+        });
+
+        it('should escape a non-nested value with an unknown helper and false value set for `noEscape`', () => {
+          expectTemplate('{{foo value}}')
+            .withHelper('foo', (value: string) => {
+              return value + 'baz';
+            })
+            .withCompileOptions({ noEscape: false })
+            .withInput({ value: '<bar>' })
+            .toCompileTo('&lt;bar&gt;baz');
+        });
+
+        it('should escape a nested value with an unknown helper and false value set for `noEscape`', () => {
+          expectTemplate('{{foo nested.value}}')
+            .withHelper('foo', (value: string) => {
+              return value + 'baz';
+            })
+            .withCompileOptions({ noEscape: false })
+            .withInput({ nested: { value: '<bar>' } })
+            .toCompileTo('&lt;bar&gt;baz');
+        });
+
+        it('should not escape a non-nested value with an unknown helper and true value set for `noEscape`', () => {
+          expectTemplate('{{foo value}}')
+            .withHelper('foo', (value: string) => {
+              return value + 'baz';
+            })
+            .withCompileOptions({ noEscape: true })
+            .withInput({ value: '<bar>' })
+            .toCompileTo('<bar>baz');
+        });
+
+        it('should not escape a nested value with an unknown helper and true value set for `noEscape`', () => {
+          expectTemplate('{{foo nested.value}}')
+            .withHelper('foo', (value: string) => {
+              return value + 'baz';
+            })
+            .withCompileOptions({ noEscape: true })
+            .withInput({ nested: { value: '<bar>' } })
+            .toCompileTo('<bar>baz');
+        });
+      });
+
+      describe('blocks', () => {
+        it('should escape a non-nested value with a block input and default value for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{#../myFunction}}{{value}}{{/../myFunction}}{{/with}}')
+            .withInput({
+              foo: { value: '<bar>' },
+              myFunction() {
+                return this;
+              },
+            })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should escape a non-nested value with an block input and false value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{#../myFunction}}{{value}}{{/../myFunction}}{{/with}}')
+            .withInput({
+              foo: { value: '<bar>' },
+              myFunction() {
+                return this;
+              },
+            })
+            .withCompileOptions({ noEscape: false })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should not escape a non-nested value with an block input and true value set for `noEscape`', () => {
+          expectTemplate('{{#with foo}}{{#../myFunction}}{{value}}{{/../myFunction}}{{/with}}')
+            .withInput({
+              foo: { value: '<bar>' },
+              myFunction() {
+                return this;
+              },
+            })
+            .withCompileOptions({ noEscape: true })
+            .toCompileTo('<bar>');
+        });
+
+        it('should escape a nested value with an block input and default value for `noEscape`', () => {
+          expectTemplate(
+            '{{#with foo}}{{#../myFunction}}{{nested.value}}{{/../myFunction}}{{/with}}'
+          )
+            .withInput({
+              foo: { nested: { value: '<bar>' } },
+              myFunction() {
+                return this;
+              },
+            })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should escape a nested value with an block input and false value for `noEscape`', () => {
+          expectTemplate(
+            '{{#with foo}}{{#../myFunction}}{{nested.value}}{{/../myFunction}}{{/with}}'
+          )
+            .withInput({
+              foo: { nested: { value: '<bar>' } },
+              myFunction() {
+                return this;
+              },
+            })
+            .withCompileOptions({ noEscape: false })
+            .toCompileTo('&lt;bar&gt;');
+        });
+
+        it('should escape a nested value with an block input and true value for `noEscape`', () => {
+          expectTemplate(
+            '{{#with foo}}{{#../myFunction}}{{nested.value}}{{/../myFunction}}{{/with}}'
+          )
+            .withInput({
+              foo: { nested: { value: '<bar>' } },
+              myFunction() {
+                return this;
+              },
+            })
+            .withCompileOptions({ noEscape: true })
+            .toCompileTo('<bar>');
+        });
+      });
     });
   });
 

packages/kbn-handlebars/src/spec/.upstream_git_hash

@@ -1 +1 @@
-eab1d141cb4a1d93375d7380ed070aa1f576a2c9
+7de4b41c344a5d702edca93d1841b59642fa32bd

packages/kbn-handlebars/src/visitor.ts

@@ -405,6 +405,18 @@ export class ElasticHandlebarsVisitor extends Handlebars.Visitor {
     return this.resolvePath(data, path);
   }
 
+  private pushToOutputWithEscapeCheck(result: any, node: ProcessableNodeWithPathParts) {
+    if (
+      !(node as hbs.AST.MustacheStatement).escaped ||
+      this.compileOptions.noEscape === true ||
+      typeof result !== 'string'
+    ) {
+      this.output.push(result);
+    } else {
+      this.output.push(Handlebars.escapeExpression(result));
+    }
+  }
+
   private processSimpleNode(node: ProcessableNodeWithPathParts) {
     const path = node.path;
     // @ts-expect-error strict is not a valid property on PathExpression, but we used in the same way it's also used in the original handlebars
@@ -415,7 +427,7 @@ export class ElasticHandlebarsVisitor extends Handlebars.Visitor {
     if (isBlock(node)) {
       this.blockValue(node, lambdaResult);
     } else {
-      this.output.push(lambdaResult);
+      this.pushToOutputWithEscapeCheck(lambdaResult, node);
     }
   }
 
@@ -435,7 +447,6 @@ export class ElasticHandlebarsVisitor extends Handlebars.Visitor {
   private processHelperNode(node: ProcessableNodeWithPathParts) {
     const path = node.path;
     const name = path.parts[0];
-
     if (this.compileOptions.knownHelpers && this.compileOptions.knownHelpers[name]) {
       this.invokeKnownHelper(node);
     } else if (this.compileOptions.knownHelpersOnly) {
@@ -455,7 +466,8 @@ export class ElasticHandlebarsVisitor extends Handlebars.Visitor {
     const helper = this.setupHelper(node, name);
     // TypeScript: `helper.fn` might be `undefined` at this point, but to match the upstream behavior we call it without any guards
     const result = helper.fn!.call(helper.context, ...helper.params, helper.options);
-    this.output.push(result);
+
+    this.pushToOutputWithEscapeCheck(result, node);
   }
 
   // Pops off the helper's parameters, invokes the helper,
@@ -482,7 +494,7 @@ export class ElasticHandlebarsVisitor extends Handlebars.Visitor {
     // TypeScript: `helper.fn` might be `undefined` at this point, but to match the upstream behavior we call it without any guards
     const result = helper.fn!.call(helper.context, ...helper.params, helper.options);
 
-    this.output.push(result);
+    this.pushToOutputWithEscapeCheck(result, node);
   }
 
   private invokePartial(partial: hbs.AST.PartialStatement | hbs.AST.PartialBlockStatement) {