Replace Handlebars with Template Strings

[aorinevo]

Support plan

• which support plan is this issue covered by? Community
• is this issue currently blocking your project? no
• is this issue affecting a production system? yes

Context

• node version: 12.16.1
• module version: 22.0.4
• environment (e.g. node, browser, native): node
• used with hapi
• any other relevant information: N/A

What problem are you trying to solve?

Lab depends on Handlebars to generate HTML for coverage reports. While Handlebars is a great templating language, it seems to have fallen out of maintenance. There are currently almost 200 vulnerabilities ranging in severity from critical to low (albeit the overwhelming majority are low). In addition, some advisories just cannot be resolved without resorting to forced resolutions or exceptions via nsprc. That is, npm audit --fix or npm update --depth={{depth}} do not work.

With a relatively low-level of effort, we can remove handlebars dependency altogether by using template strings. For example, see https://github.com/hapijs/lab/pull/978/files.

ROI

• removes 1 dependency (~80KB, includes compiler and runtime)
• reduces install time
• resolves existing and future Handlebars vulnerabilities
• increased testability of html reporter templates
• unlocks potential to remove all html files (i.e. reporters/html/partials/*.html and reporters/reporter.html

Notes

• There is an open PR on Handlebars to resolve some existing Handlebars vulnerabilities (see fix(4.x): migrate from optimist to yargs handlebars-lang/handlebars.js#1662)
• This refactor was largely inspired by https://medium.com/your-majesty-co/frameworkless-javascript-template-literals-the-best-thing-since-sliced-bread-d97f000ce955
• Some consideration needed for when passing user inputs into template strings (see comment in above medium article). Not applicable here as html reports are static.
• Maybe replace handlebars with VueJS v3. Bundle size is estimated to be ~10KB.
• Handlebars has deprecated use of optimist in favor of yargs which resolves some of the vulnerabilities discussed above. However, this change does not address all the vulnerabilities (I'll update the screen shots to reflect this).

Do you have a new or modified API suggestion to solve the problem?

The suggested feature does not require changes to the API.

[aorinevo]

Partial work towards this can be found here: https://github.com/hapijs/lab/pull/978/files

[aorinevo]

@hueniverse, I got this fully working.

This change is backwards compatible. Template String HTML reporter can be enabled via .labrc.js by setting enableTemplateStringHTMLReporter to 'true'.

With feature flag on, unit tests pass and coverage is at 100%. With feature flag off, all but one unit tests pass.

It is important to note that the failed unit test is a false positive as the unit test itself was updated to support template strings. Meaning that when feature flag is off, all functionality works as before but runs one unit test that is meant to pass only when feature flag is on.

It is possible to add backwards compatibility to the failed unit tests. That is, toggle between the appropriate assertions via the feature flag. Would love to get some guidance here.

Looking forward to your feedback :)

[stevendesu]

Just wanted to add on the "resolves [...] future Handlebars vulnerabilities" note -- this showed up in my NPM audit today:

stevenbarnett@MacBook-Pro hapi-test % npm audit 
                                                                                
                       === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @hapi/lab [dev]                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @hapi/lab > handlebars > optimist > minimist                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

[aorinevo]

@stevendesu, Handlebars has released v4.7.4 which deprecates use of optimist in favor of yargs.

As an interim solution, while this issue is being reviewed, you can try updating to v4.7.4 by way of running npm update --depth={{depth}} to see if that resolves the issue.

[aorinevo]

Closing this issue for now as the vulnerability was addressed through a combination of PRs to Handlebars:

• fix(4.x): migrate from optimist to yargs handlebars-lang/handlebars.js#1666
• Switch cmd parser to latest minimist handlebars-lang/handlebars.js#1672