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

Check package permission on the registry #4

Open
pvdlg opened this issue Nov 26, 2017 · 13 comments
Open

Check package permission on the registry #4

pvdlg opened this issue Nov 26, 2017 · 13 comments

Comments

@pvdlg
Copy link
Member

pvdlg commented Nov 26, 2017

Currently we only check if the user is authenticated on the configured registry.
But if a user set a validNPM_TOKEN, that doesn't have the permission to deploy the current package the release will fail during the publish phase. Similarly if the package name is already taken the release will fail.

To avoid that we should check in the verifyConditions if the package exists, and if it does, make sure the user has read-write access to the package.
Unfortunately the npm-registry-client doesn't implement the method. So it would have to be done with npm access ls-collaborators <@scope/package>.

In addition it would be great, in case of a restricted package to be able to check if the organization is private and paid. But I'm didn't find a way to do that.
During publish if the organization is not private and paid, the registry returns a 402 error.

@kopax
Copy link

kopax commented Jul 7, 2018

During the npm release I ended up having an error:

npm ERR! This package has been marked as private
npm ERR! Remove the 'private' field from the package.json to publish it.

This could be prevented within verify stage by controlling if the package.json is having a false value for the key private.

It should break with:

{
+ "private": "true"
}

@pvdlg
Copy link
Member Author

pvdlg commented Jul 7, 2018

Maybe private: true should have the same effect as npmPublish: false, i.e. skip publish and just create the package locally, without erroring out?

@kopax
Copy link

kopax commented Jul 7, 2018

@pvdlg according to https://docs.npmjs.com/files/package.json#private:

If you set "private": true in your package.json, then npm will refuse to publish it.
This is a way to prevent accidental publication of private repositories. If you would like to ensure that a given package is only ever published to a specific registry (for example, an internal registry), then use the publishConfig dictionary described below to override the registry config param at publish-time.

I believe private: true and npmPublish: false are different configuration. private: true is specifically designed to return an exit code 1.

@pvdlg
Copy link
Member Author

pvdlg commented Jul 7, 2018

But in the context of semantic-release you can set private: false to make sure the package is not published on npm but still want to update the version in package.json, commit the package.json to the repo, make a release on GitHub, create a tag etc...

If you throw an error when private is true that means you can't use this plugin at all, therefore you can't update the version in package.json and you can't generate the package with npm pack.

Having a package with private set to true for safety (like outside of the semantic-release context) and wanting to update the version seems a valid use to me.

@kopax
Copy link

kopax commented Jul 7, 2018

you can't generate the package with npm pack.

This is not true, this is how we install our private package which are not released on any registries.

We use private: true by default, and we have a specific command to remove it from our package.json.

I agree that a warning would be sufficient as we may want to tag a GitLab/GitHub version.

@pvdlg
Copy link
Member Author

pvdlg commented Jul 7, 2018

This is not true, this is how we install our private package which are not released on any registry.

What I'm saying is that if @semantic-release/npm throws an error when private: true that means you can't use it to generate the package with npm pack.
Currently if you set npmPublish: false the plugin will run npm pack instead of npm publish.

My point is that there is no reason to throw an error is private: false because it would prevent users in such situation to use other feature of the plugin.

@felixfbecker
Copy link

This just bit me:

I just had a case where a user mistakenly didn't have public rights to the npm package (after changing tokens and using a different user for the token a couple weeks ago).

However, the verifyConditions didn't catch this. It was only caught when the npm publish step failed with a 403 error message stating the user doesn't have publish rights.

That meant the the git tag was still pushed to the repo, leaving the tag out of sync. Retrying the build would then not publish, because the tag was already pushed.

@jedwards1211
Copy link

okay npm access list packages $(npm whoami) <pkg name> seems to work well for getting just the permissions for the package in question, it will output

<pkg name>: read-write

@jedwards1211
Copy link

jedwards1211 commented Aug 21, 2024

@travi @gr2m @babblebey I just realized, I can't run this check on the first publish because the package won't exist in npm yet.

Therefore, I should do something like

if (context.lastRelease.version) {
  // check npm access list packages ...
}

But context.lastRelease.version won't be available until after the Get last release step, so I can't do this check in the Verify conditions step. Is Verify release an appropriate step to perform this check in? It's vague to me what that step is intended for.

@babblebey
Copy link
Member

babblebey commented Aug 21, 2024

I just realized, I can't run this check on the first publish because the package won't exist in npm yet.

One way we could go about this is.... Conditionally run this permission check based on existence of the package in registry.. I haven't figured exactly how but I believe we could do a search before-hand to figure whether the package exists in registry.

I found the npm-search command https://docs.npmjs.com/cli/v7/commands/npm-search

@jedwards1211
Copy link

jedwards1211 commented Aug 21, 2024

@babblebey searching for private packages won't work if the token doesn't have access, it will just look like the package doesn't exist. That's why we need to use context.lastRelease.version to know if the package should exist yet

@jedwards1211
Copy link

jedwards1211 commented Aug 30, 2024

Argh, turns out npm's API is insufficient for this right now. npm access list packages $(npm whoami) <pkg name> works for a classic token, but for a granular token it 403s, even if the granular token has read-write permission on the package. I've opened a discussion asking them to provide a command for this.

@jedwards1211
Copy link

jedwards1211 commented Aug 30, 2024

Hmmm, I could make a check that correctly errors out on a classic token with insufficient permission, but allows any granular token for now; then if npm provides a way to properly check the token's permissions someday, we could update the verification logic.

Here's what the verify logic would be:

  • Run npm access list packages $(npm whoami) <pkgname>:
    • If it 403s: pass (must be a granular token)
    • If it outputs <pkgname>: read-write: pass
    • If it outputs <pkgname>: read: fail
    • If it outputs blank (package may not exist yet):
      • If the package name is unscoped: pass (anyone should be able to claim an unscoped package)
      • Run npm org ls <package scope> $(npm whoami) --json:
        • If it outputs { <username>: "admin" }: pass
        • Otherwise fail

Thoughts? @travi @gr2m @babblebey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants