patch vulnerabilities

[mark-omarov]

Hi 👋
The security department didn't like Snyk's report on this action, and I didn't like the idea of making a custom action for the same job, so here we go. We will be using the forked version for now, but you might also want to consider this.

Changes:

• Moved some dev dependencies from dependencies to devDependencies
• Forced resolution of vulnerable packages

I've tested these changes on my fork by releasing the changes and going through the PR raised by the action. I might have missed something.

Cheers!

[changeset-bot]

🦋 Changeset detected

Latest commit: 3295764

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@changesets/action	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mattlunn]

Our security team are similarly reluctant to allow us to use changesets in our hosted GitHub Enterprise runners, because of the vulnerabilities found in the Snyk report;

Would be great if this PR was merged!

[MrTibbles]

This PR is also of great interest as we wish to use in our hosted Github enterprise runners but can not due to Snyk report. @Andarist are you able to provide any insights into how the approval process works, i am asking you as you made the most recent commit to the repo! :)

[MelSumner]

👋 +1 on this PR as well, this is a blocker for us at work and we'd love to be able to keep using this awesome package. @Andarist is there anything that we can do to help this move forward?

P.S. Thank you to @mark-omarov for doing this work! 💪 🙇

[mark-omarov]

There are conflicts needs to be resolved. I'm occupied with other things right now, but I'll try to get to it sometime this weekend.

Thanks everyone.

[mark-omarov]

I got a minute to resolve conflicts, did a scan with Dependabot, and added resolutions to patch reported vulnerabilities. As of now, there should be no vulnerabilities.

Sorry to bother @Andarist, but I'd appreciate it if you could take a look.
I tested the latest changes again on the fork, but don't quote me. I might have missed something.

[alex-ju]

hey @Andarist! sorry to keep nudging you on reviewing this, but I would hate for us (and potentially others) to have to fork away from this repo because of security compliance. it would be lovely if you could make some time to review this 🙏

[mark-omarov]

Hi @emmatown , sorry to bother you, in the hope of getting this PR noticed, I figured you might be able to help. If no - apologies for annoyance. ☺️

[Andarist]

Do we have a breakdown of what packages depend on some outdated versions of those? Couldn't we update some packages to fix this instead of overriding what is installed using resolutions?

[Andarist]

This is a dependency of yargs and smartwrap depends on it. It's a bummer because we actually don't depend on any parts of smartwrap that depend on that... the problem is that smartwrap ships both its CLI and a runtime library in a single package.

[Andarist]

Hm, I also don't quite see that package in the Snyk report.

[Andarist]

This one comes from remark-parse and to fix it we'd have to update a major of this library. I agree that it's best to do it in a separate PR.

[Andarist]

I'm not sure what this one is about. I don't see any package to depend on this.

[Andarist]

after your updates packages don't depend on this at all so this is redundant (likely the same story for minimist)

[Andarist]

I tweaked this PR a little bit because I think that some bits were overeager and that some of those "vulnerabilities" went away when some of the packages were updated.

Let's publish this and then recheck with Snyk again.

[mark-omarov]

Thanks, @Andarist !
I might have forgotten to rescan after resolving conflicts, some vulnerabilities might have indeed been resolved by then.
I'm gonna ask the security team to rescan next week.