Skip to content

Conversation

claudiahdz
Copy link

@claudiahdz claudiahdz commented Aug 10, 2019

✏️ Changes

This PR introduces a new error in the case that a forbidden package was requested and blocked by the security proxy. If no manifest was found under the versions object, then a check is done to verify if the package is forbidden under the restrictedVersions object. If it is, a new error code is thrown E403 and the admin custom message will be shown on the CLI like:

➜  security-messaging npm i [email protected]
npm ERR! code E403
npm ERR! 403 Could not download [email protected] due to policy violations.
npm ERR! 403 Use `npm audit fix` to upgrade this dependency.
npm ERR! 403
npm ERR! 403 In most cases you or one of your dependencies are requesting
npm ERR! 403 a package version that is forbidden by your security policy
npm ERR! 403 please contact your npme admin.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/claudiahdz/.npm/_logs/2019-08-09T20_46_04_225Z-debug.log

If no policy restrictions are found on the packument then the behavior will continue as is right now and throw a ETARGET error.

🔗 References

RFC: https://npmjs.slab.com/posts/proposed-package-filtering-messaging-on-the-cli-kejlhyow

🔍 Testing

Automated testing

  • Checks if E403 when version is forbidden
  • Checks if errors when metadata has no versions or restricted versions

✅ This change has unit test coverage

⚠️ Warning (Blockers)

This PR depends on: npm/cli#234

@claudiahdz claudiahdz force-pushed the claudiahdz/403-error-messaging branch from 17a0e05 to c88c666 Compare August 12, 2019 17:21
const isForbidden = target && policyRestrictions && packument.policyRestrictions.restrictedVersions[target]
const pckg = `${packument.name}@${wanted}${
opts.enjoyBy
? ` with an Enjoy By date of ${

Choose a reason for hiding this comment

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

Curious. What is this 'Enjoy By' date about?

Copy link
Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Interesting. So what's the benefit of filtering out packages that are newer than a given date? Avoiding packages that haven't been thoroughly tested or widely deployed?

Copy link

Choose a reason for hiding this comment

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

@djsauble Reproducing installs as they would have happened at some point in the past.

Copy link

@djsauble djsauble left a comment

Choose a reason for hiding this comment

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

❤️

@claudiahdz claudiahdz force-pushed the claudiahdz/403-error-messaging branch from aa2c6c9 to 45b6df3 Compare August 19, 2019 22:31
@isaacs isaacs closed this in ad2a962 Aug 20, 2019
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.

3 participants