Following Instructions in Documentation Results in Security Vulnerabilities 😞

[Mike-E-angelo]

Bug Report

Current Behavior
Greetings. Installing Node and Probot both for the first time here. Following the directions at the following location:

https://probot.github.io/docs/development/

Running the following command and filling out its prompts:

npx create-probot-app <dir>
npx: installed 229 in 14.44s

Let's create a Probot app!
Hit enter to accept the suggestion.

? App name: <name>
? Description of app: <description>
? Author's full name: <author>
? Author's email address: <email>
? GitHub user or org name: <organization>
? Repository name: <repository>
? Which template would you like to use? basic-ts
created file: <dir>\.env.example
created file: <dir>\app.yml
created file: <dir>\CODE_OF_CONDUCT.md
created file: <dir>\CONTRIBUTING.md
created file: <dir>\.gitignore
created file: <dir>\jest.config.js
created file: <dir>\LICENSE
created file: <dir>\package.json
created file: <dir>\README.md
created file: <dir>\tsconfig.json
created file: <dir>\src\index.ts
created file: <dir>\test\index.test.ts
created file: <dir>\test\fixtures\issues.opened.json
created file: <dir>\test\fixtures\mock-cert.pem

Finished scaffolding files!

Initialized a Git repository.

Installing dependencies. This may take a few minutes...

npm WARN deprecated @types/nock@11.1.0: This is a stub types definition. nock provides its own type definitions, so you do not need this installed.
npm WARN deprecated eslint-plugin-typescript@0.14.0: Deprecated: Use @typescript-eslint/eslint-plugin instead
npm WARN deprecated superagent@3.8.3: Please note that v5.0.1+ of superagent removes User-Agent header by default, therefore you may need to add it yourself (e.g. GitHub blocks requests without a User-Agent header).  This notice will go away with v5.0.2+ once it is released.
npm WARN deprecated fsevents@1.2.9: One of your dependencies needs to upgrade to fsevents v2: 1) Proper nodejs v10+ support 2) No more fetching binaries from AWS, smaller package size
npm WARN deprecated left-pad@1.3.0: use String.prototype.padStart()

> dtrace-provider@0.8.8 install <dir>\node_modules\dtrace-provider
> node-gyp rebuild || node suppress-error.js


<dir>\node_modules\dtrace-provider>if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "C:\Program Files\nodejs\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js" rebuild )
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.

> nodemon@1.19.4 postinstall <dir>\node_modules\nodemon
> node bin/postinstall || exit 0

npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN acorn-jsx@5.1.0 requires a peer of acorn@^6.0.0 || ^7.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.9 (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.9: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

added 893 packages from 639 contributors and audited 879969 packages in 31.001s
found 8 vulnerabilities (2 moderate, 6 high)
  run `npm audit fix` to fix them, or `npm audit` for details

This seems to differ vastly from the output found within the documentation page.

In particular:

found 8 vulnerabilities (2 moderate, 6 high)
  run `npm audit fix` to fix them, or `npm audit` for details

running npm audit fix:

npm WARN acorn-jsx@5.1.0 requires a peer of acorn@^6.0.0 || ^7.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.9 (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.9: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

added 1 package from 1 contributor and updated 1 package in 5.952s
fixed 4 of 8 vulnerabilities in 879969 scanned packages
  4 vulnerabilities required manual review and could not be updated

So it would seem that simply creating a Probot project has now introduced 8 vulnerabilities into my development environment, 6 of which are considered high impact. After running the suggested guidance of npm audit fix there are now 4 remaining, of which all could be high impact, but this is unknown as it doesn't really specify.

As a new user to both Node/NPM and Probot, I am now saddled with the stressful and disruptive responsibility of figuring out how to "manually review" these vulnerabilities -- let alone submitting an issue on GitHub documenting my experience! -- rather than diving directly into your nifty product and learning some cool magic as your documentation portends.

Hardly a pleasant first impression. 😞

Expected behavior/code
Creating a new Probot project that doesn't result in installing vulnerabilities into my development environment would be a nice start. 😁

Environment

• Probot version(s): 9.6.6
• Node/npm version: 12.13.1/6.12.1
• OS: Windows 10 1909 64-bit

[welcome]

Thanks for opening this issue. A contributor should be by to give feedback soon. In the meantime, please check out the contributing guidelines and explore other ways you can get involved.

[issue-label-bot]

Issue-Label Bot is automatically applying the label bug 🐞 to this issue, with a confidence of 0.97. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[Mike-E-angelo]

hey worth taking the trouble to post here since now I know about issue-label-bot and have installed it on our repo, as well. 😁 👍

[gr2m]

The vulnerabilities might be likely caused by a module maintained by npm itself. It's a dependency of semantic-release and there is not much I can can do about it, although I'm one of the maintainers of semantic-release, too. We are waiting for npm to fix their modules

[Mike-E-angelo]

Ah ok thank you for the additional context, @gr2m. As I mentioned, it was my first install and experience with npm. I've heard the horror stories too with event-stream so I guess I am hyper-vigilant here as well due to that.

The frustrating aspect, of course, is that running the command that says should fix it (npm audit fix) doesn't really fix it. It would seem that it should be easy to get into a state where 0 vulnerabilities are reported, especially in the context of creating a new project where everything is built from scratch with the latest packaging (in theory).

I realize that is not entirely on Probot, but at the moment I am unsure how much of this is due to Probot (that is, with its package references) and how much of it is due to npm as you have mentioned.

My main concern is making sure everyone is aware of the experience but also the vulnerabilities to ascertain their impact and/or legitimacy. I mean, I realize I am the last developer on earth to finally jump on the npm train and no one else will ever experience this, but still. 😅

[MaximDevoir]

The 4 security vulnerabilities that could not be fixed by npm audit fix stem from the latest version of the dependency hbs - which is depending on an outdated version of the handlebars package. An issue has already been created over there - pillarjs/hbs#185 - and a PR created to solve the security vulnerabilities in hbs with pillarjs/hbs#186.

[Mike-E-angelo]

OK... that works for me, @MaximDevoir. Maybe if someone runs into this same issue we've a little more contextual keywords to help let them know that this is a known issue that is unfortunately outside of Probot's control ATM and should be fixed "soon." 😏

Closing this on my side. Please feel free to re-open if I have something fundamentally misunderstood here. Thank you for your assistance and insights!

[MeFisto94]

I was about to open a duplicate of this. Having an issue open to track an open vulnerability would've certainly helped. Especially this Issue can then be used to follow up and update the hbs dependency in this repo once they have been fixed.

On topic: What is hbs used for here? Is it possible to run probot without hbs support? And does the input stem from user code or is it passed from e.g. github comments (and thus every bot insecure)?

Just a little insight on the impact of this issue would be nice as a fresh probot user I can't judge the security problems this could entail, but DoS, Arbitrary Code Execution etc don't sound nice.

[Mike-E-angelo]

Yeah, good point @MeFisto94 ... I was trying to be a good citizen in keeping issue count low but if others are expressing the same concerns it's probably better to let the maintainers decide when to close it.

[MaximDevoir]

Probot uses hbs to render templates with Express. The built-in templates with Probot are used for registering your GitHub App while in the development environment - these templates are not affected by the vulnerabilities in hbs.

Probot apps may be susceptible to these vulnerabilities if they use Probot's built-in Express router's res.render() method.

Example of .render() method:

module.exports = app => {
  // Get Probot's built-in express router, which has the view engine set to hbs
  const router = app.route('/app')

  router.get('/endpoint', (req, res) => {
    res.render('malicious-template.hbs')
  })
}

If your Probot app never uses the .render() method, your application is not affected.

If you Probot app uses the .render() method in a manner similar to the above example, I'd recommend looking through the advisories at pillarjs/hbs#186 (comment) to see if your hbs templates could be affected.

---

A quick search through GitHub doesn't show any public Probot apps using the built-in .render() method.

Hope this helps.

[MaximDevoir]

These security warnings have been resolved in the latest version of Probot (9.9.1), which includes changes from #1109.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[gr2m]

This is an evergreen issue. We try to stay on top of it as good as we can :)