Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate plugin name before install #597

Merged
merged 3 commits into from
May 2, 2023
Merged

Conversation

cristiand391
Copy link
Member

Fixes: #594

Use validate-npm-package-name to validate the plugin name is a valid npm pkg name before doing any initialization.
https://github.com/npm/validate-npm-package-name

@W-13099057@

return {name, tag, type: 'npm'}
}
}

function validateNpmPkgName(name:string): void {
if (!validate(name).validForNewPackages) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at the npm docs https://github.com/npm/validate-npm-package-name#legacy-names

I can't tell how far in time you'd have to go to hit an "old name"

What do you think the chances are that someone using oclif has an npm package that violates the new rules (ex: has a Capital letter in it)?

For safety, I'd consider doing this

  1. make this a major version bump
  2. since we're doing a major anyway, this is a good time to bump pjson.engines to 16+

then we can bump this in our CLIs and most oclif CLIs could, too, unless they happen to need some old-npm -named-plugin?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this issue from 2013, which is before oclif was released:
npm/npm#3914

Not sure about any other legacy rule, let's do a new major 👍🏼

@mshanemc
Copy link
Member

mshanemc commented May 1, 2023

QA notes

using sf plugins:link to the branch locally

✅ throws on a bad package name
✅ allows a valid plugin name (@salesforce/plugin-signups)
✅ doesn't break JIT (uninstalled signups, then ran a command from it to trigger the installation)
✅ doesn't block installing from url sf plugins install https://github.com/salesforcecli/plugin-signups

@mshanemc mshanemc merged commit 4c328f7 into main May 2, 2023
@mshanemc mshanemc deleted the cd/sanitize-npm-pkg-name branch May 2, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential command injection vulnerability in plugin-plugins
2 participants