[DevOps]: Node Security

[joshbruce]

Marked version: 0.3.18

Markdown flavor: n/a

Proposal type: other

What pain point are you perceiving?

None. More of a curiosity.

What solution are you suggesting?

Was making the rounds to those projects who became dependent on 8fold-marked to make sure they switched back to latest. Noticed this in their pipeline:

Node security... @UziTech what is it, should we have it? (Believe there was a REGEX tool mentioned elsewhere at some point.)

[UziTech]

That is NSP. It is free for OSPs

There is a free Snyk App that might be helpful as well.

It would also be nice to enable Appveyor for testing on windows.

[joshbruce]

Leave it to you, brother. Definitely think Snyk should be a thing since they're the ones nice enough to talk to us. :)

[styfle]

I also think we should add the policy to prevent merging PRs without having all the checks pass:

• CI
• security check
• 2+ approvals

I can enable 2+ approvals through the github policy.
Is anyone against this? @joshbruce @UziTech @davisjam

[davisjam]

+1

[joshbruce]

I'm okay with the 2+ approvals. If this can be done through the GitHub review process, please post a link of a how-to, if you have one handy.

[styfle]

Visit "Setting" > "Branches" https://github.com/markedjs/marked/settings/branches

Click "Edit" next to master

Then check the boxes for the policies

---

Update: I accidentally enabled "Require branches to be up to date before merging" and I realize this is going to be painful for each PR so I disabled that option. Sorry about that 😛

[styfle]

I just enabled that, however I am unable to install Snyk.
I think only @joshbruce can do it: https://github.com/marketplace/snyk

(note: node security is no longer accepting new repos)

[UziTech]

NSP has been aquired by NPM and have stopped accepting new accounts. But should probably still do Snyk

[joshbruce]

Added Snyk via the MarkedJSBot account. (Starting to think admin might be an owner...but still considering operations for that.)

• Testing weekly (daily felt like overkill).
• Fail if the repo has any vulnerabilities.

[joshbruce]

@UziTech and @styfle: In theory, we don't need this but might good for us to have awareness of just in case for CI: https://snyk.io/docs/using-snyk?utm_campaign=welcome_email&utm_campaign=GitHub%20Onboarding&utm_medium=email&utm_medium=email&utm_source=DocsCommandLine&utm_source=hs_automation&utm_content=61782021&_hsenc=p2ANqtz-9XvrdTanuF8D9kJMUpg6eZIkB6GmW7zFpt02S424C2HJb403WhfIn0PI6e3FpaEN1HrnE1YE86p9-zAqqWsrCa21NpVQ&_hsmi=61782021

[styfle]

So it doesnt check every PR?

[joshbruce]

If "it" in this context is Snyk, it should be every PR - but they have a CLI to extend it would seem. See #1229

[styfle]

It looks like npm@6.0.0 will have this security audit built in.

See the blog post for details.

[UziTech]

should we add npm audit to travis after lint? (once the registry actually allows it)

[DanielRuf]

Currently the security check for RegEx fails due to some violations.

[davisjam]

@DanielRuf Some fixes and discussion in #1226.