change reporting usage of `handlebars` to `@kbn/handlebars` by pmuellr · Pull Request #217778 · elastic/kibana

pmuellr · 2025-04-10T04:47:08Z

Summary

Change reporting's usage of handlebars to @kbn/handlebars. Also added a test to ensure user input is HTML escaped (it always has been, this just tests it).

There should be no change to the final rendered output, at all. These changes only affect PDF and PNG reports, not CSV reports.

Checklist

Check the PR satisfies following conditions.

still renders the title in the page header
when "PDF footer image" is set in Advanced Settings, it's rendered in the footer; also rendered will be Powered by Elastic beneath it
when "PDF footer image" is NOT set in Advanced Settings, the default icon is rendered in the footer; Powered by Elastic will not be rendered.

Reviewers should verify this PR satisfies this list as well.

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2025-04-10T10:18:18Z

Pinging @elastic/response-ops (Team:ResponseOps)

azasypkin · 2025-04-10T10:49:12Z

x-pack/platform/plugins/shared/screenshotting/server/browsers/chromium/templates/index.ts

+async function compileTemplate<T>(pathToTemplate: string): Promise<TemplateDelegate<T>> {
  const contentsBuffer = await fs.readFile(pathToTemplate);
-  return Handlebars.compile(contentsBuffer.toString());
+  return Handlebars.compileAST(contentsBuffer.toString());


question: can we limit helpers in this context?

return Handlebars.compileAST(contentsBuffer.toString(), { knownHelpersOnly: true });

and maybe even?

return Handlebars.compileAST(contentsBuffer.toString(), { knownHelpers: { set all not needed built-in helpers to `false`}, knownHelpersOnly: true });

I suspect we can, since the HB usage here is pretty simple. Simple variable usage, and and #if block/section.

I'm unfamiliar with these options though, so ... any advice?

Just committed this - 69dd2d7 - locking down everything but #if - and it seems to do everything correctly.

pmuellr · 2025-04-10T15:06:20Z

I tested PNG, PDF, and PDF "for print" - there's a problem with PDF "for print" which I suspect is unrelated. (it is a known problem: https://github.com/elastic/response-ops-team/issues/310 )

The other two, the titles work fine, and for PDF setting the footer image in Advanced Settings works fine - renders the image and the "Powered by Elastic" copy.

pmuellr · 2025-04-10T21:24:02Z

/ci

pmuellr · 2025-04-15T14:28:49Z

@elasticmachine merge upstream

azasypkin

Looks good from a security perspective, thanks for tightening down Handlebars compile options!

tsullivan

LGTM

Tested the code using a Docker container, no regressions.

pmuellr · 2025-04-16T14:55:23Z

@elasticmachine merge upstream

pmuellr · 2025-04-28T20:34:48Z

@elasticmachine merge upstream

elasticmachine · 2025-04-28T21:34:29Z

💛 Build succeeded, but was flaky

Failed CI Steps

FTR Configs #51

Test Failures

[job] [logs] FTR Configs #51 / saved objects tagging - functional tests maps integration listing allows to manually type tag filter query

Metrics [docs]

✅ unchanged

History

💚 Build #293824 succeeded 000125c
💔 Build #293282 failed ac02c05
💚 Build #292133 succeeded 69dd2d7
💚 Build #291818 succeeded 26f91e5

kibanamachine · 2025-04-29T12:08:48Z

Starting backport for target branches: 7.17, 8.17, 8.18, 8.19, 9.0

https://github.com/elastic/kibana/actions/runs/14730841139

…217778) Change reporting's usage of `handlebars` to `@kbn/handlebars`. Also added a test to ensure user input is HTML escaped (it always has been, this just tests it). There should be no change to the final rendered output, at all. These changes only affect PDF and PNG reports, not CSV reports. (cherry picked from commit 3b5e96a)

kibanamachine · 2025-04-29T12:16:03Z

💔 Some backports could not be created

Status	Branch	Result
❌	7.17	Backport failed because of merge conflicts You might need to backport the following PRs to 7.17: - chore(deps): bump `formidable` from to 3.5.2 to 3.5.4 (#219385)
❌	8.17	Backport failed because of merge conflicts You might need to backport the following PRs to 8.17: - [ResponseOps][Cases]Allow dashes in host names in Observables (#219038) - [Security Solution] Add new fields to indices metadata (#219246) - [Synthetics]: Fixed monitor status rule control (#218994)
✅	8.18
✅	8.19
✅	9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 217778

Questions ?

Please refer to the Backport tool documentation

…217778) Change reporting's usage of `handlebars` to `@kbn/handlebars`. Also added a test to ensure user input is HTML escaped (it always has been, this just tests it). There should be no change to the final rendered output, at all. These changes only affect PDF and PNG reports, not CSV reports. (cherry picked from commit 3b5e96a) # Conflicts: # x-pack/plugins/screenshotting/server/browsers/chromium/templates/index.test.ts

…217778) Change reporting's usage of `handlebars` to `@kbn/handlebars`. Also added a test to ensure user input is HTML escaped (it always has been, this just tests it). There should be no change to the final rendered output, at all. These changes only affect PDF and PNG reports, not CSV reports. (cherry picked from commit 3b5e96a) # Conflicts: # x-pack/platform/plugins/shared/screenshotting/server/browsers/chromium/templates/index.ts # x-pack/platform/plugins/shared/screenshotting/tsconfig.json

pmuellr · 2025-04-29T12:58:15Z

💚 All backports created successfully

Status	Branch	Result
✅	8.17
✅	7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…217778) (#219527) # Backport This will backport the following commits from `main` to `8.19`: - [change reporting usage of `handlebars` to `@kbn/handlebars` (#217778)](#217778)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Patrick Mueller <patrick.mueller@elastic.co>

…17778) (#219528) # Backport This will backport the following commits from `main` to `9.0`: - [change reporting usage of `handlebars` to `@kbn/handlebars` (#217778)](#217778)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Patrick Mueller <patrick.mueller@elastic.co>

…217778) (#219526) # Backport This will backport the following commits from `main` to `8.18`: - [change reporting usage of `handlebars` to `@kbn/handlebars` (#217778)](#217778)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Patrick Mueller <patrick.mueller@elastic.co>

…217778) (#219543) # Backport This will backport the following commits from `main` to `8.17`: - [change reporting usage of `handlebars` to `@kbn/handlebars` (#217778)](#217778)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

…217778) Change reporting's usage of `handlebars` to `@kbn/handlebars`. Also added a test to ensure user input is HTML escaped (it always has been, this just tests it). There should be no change to the final rendered output, at all. These changes only affect PDF and PNG reports, not CSV reports.

change reporint usage of handlebars to @kbn/handlebars

26f91e5

pmuellr added Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Feature:Reporting:Framework Reporting issues pertaining to the overall framework Feature:Reporting:Screenshot Reporting issues pertaining to PNG/PDF file export labels Apr 10, 2025

pmuellr marked this pull request as ready for review April 10, 2025 10:18

pmuellr requested a review from a team as a code owner April 10, 2025 10:18

pmuellr added backport:all-open Backport to all branches that could still receive a release release_note:skip Skip the PR/issue when compiling release notes labels Apr 10, 2025

azasypkin reviewed Apr 10, 2025

View reviewed changes

pmuellr added ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project ci:cloud-deploy Create or update a Cloud deployment labels Apr 10, 2025

lockdown hb a bit more with known helpers

69dd2d7

pmuellr removed the ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project label Apr 11, 2025

Merge branch 'main' into fix-reporting-handlebars-usage

ac02c05

pmuellr requested a review from azasypkin April 15, 2025 14:29

azasypkin approved these changes Apr 15, 2025

View reviewed changes

tsullivan approved these changes Apr 15, 2025

View reviewed changes

Merge branch 'main' into fix-reporting-handlebars-usage

000125c

Merge branch 'main' into fix-reporting-handlebars-usage

d3227a6

pmuellr merged commit 3b5e96a into elastic:main Apr 29, 2025
10 checks passed

kibanamachine added the v9.1.0 label Apr 29, 2025

kibanamachine mentioned this pull request Apr 29, 2025

[8.18] change reporting usage of handlebars to @kbn/handlebars (#217778) #219526

Merged

kibanamachine mentioned this pull request Apr 29, 2025

[8.19] change reporting usage of handlebars to @kbn/handlebars (#217778) #219527

Merged

kibanamachine mentioned this pull request Apr 29, 2025

[9.0] change reporting usage of handlebars to @kbn/handlebars (#217778) #219528

Merged

pmuellr mentioned this pull request Apr 29, 2025

[8.17] change reporting usage of handlebars to @kbn/handlebars (#217778) #219543

Merged

pmuellr mentioned this pull request Apr 29, 2025

[7.17] change reporting usage of handlebars to @kbn/handlebars (#217778) #219545

Closed

kibanamachine added the v8.19.0 label Apr 29, 2025

kibanamachine added the v9.0.1 label Apr 29, 2025

kibanamachine added the v8.18.1 label Apr 29, 2025

kibanamachine added the v8.17.6 label Apr 29, 2025

Conversation

pmuellr commented Apr 10, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Apr 10, 2025

Uh oh!

azasypkin Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

pmuellr Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

pmuellr Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

pmuellr commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pmuellr commented Apr 10, 2025

Uh oh!

pmuellr commented Apr 15, 2025

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

tsullivan left a comment

Choose a reason for hiding this comment

Uh oh!

pmuellr commented Apr 16, 2025

Uh oh!

pmuellr commented Apr 28, 2025

Uh oh!

elasticmachine commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

History

Uh oh!

Uh oh!

kibanamachine commented Apr 29, 2025

Uh oh!

kibanamachine commented Apr 29, 2025

💔 Some backports could not be created

Manual backport

Questions ?

Uh oh!

pmuellr commented Apr 29, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pmuellr commented Apr 10, 2025 •

edited by kibanamachine

Loading

pmuellr commented Apr 10, 2025 •

edited

Loading

elasticmachine commented Apr 28, 2025 •

edited

Loading