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

Guided remediation for npm (osv-scanner fix) failed to resolve private dependencies #899

Closed
khai-tran opened this issue Apr 2, 2024 · 13 comments · Fixed by #901
Closed
Assignees
Labels
bug Something isn't working

Comments

@khai-tran
Copy link

It looks like the guide doesn't work when my project has private dependencies. After some digging, I found out that the root cause is the guide's reliance on deps.dev, which can't access those private packages.

To reproduce, simply include any private dependencies in package.json and run osv-scanner fix -M package.json.
The error was:
cannot find matching versions for ^<version>: package NPM:@<package>: not found

I've got a couple ideas on how to tackle this:

  1. Ignore private dependencies for now. It's not ideal, but at least the guide would work for public packages. This could be an additional CLI boolean flag
  2. Switch to using the npm API (that works with local configurations, such as nprmc). It might be slower, but it would let the guide handle both public and private dependencies.

What do you think? I'd love to hear your thoughts on the best way forward. Fixing this would be a big help for folks like me who use private dependencies.

Thanks for all your hard work on OSV-Scanner!

@oliverchang
Copy link
Collaborator

Hi, thanks for trying this! Can you try using --data-source=native per https://google.github.io/osv-scanner/experimental/guided-remediation/#data-source ?

@khai-tran
Copy link
Author

Thanks for the pointer!

I tried --data-source=native and it's hitting a different error:
cannot find matching versions for ^<version>: 401 Unauthorized

The private registry token is in ~/.npmrc, configured via //<hostname>/:_authToken format. Running npm show <package> works just fine.

Note that this is not private npm-hosted package, but a different storage provider with compatible NPM APIs.

@khai-tran khai-tran changed the title Guided remediation for npm (osv-scanner fix) failed to resolve private depdendencies Guided remediation for npm (osv-scanner fix) failed to resolve private dependencies Apr 2, 2024
@oliverchang
Copy link
Collaborator

@michaelkedar can you take a look?

@michaelkedar
Copy link
Member

It should be setting the Authorization: Bearer <_authToken> header in the requests to the hostname.

I assumed the Authorization header would be enough, but it's possible that it's looking at some other headers for verification. Do you know/is it possible to share what your underlying storage provider is built on (e.g. Verdaccio seems to work as-is with just the auth header)

I guess it's also possible that the authToken isn't being correctly read. Is it possible to share how the registry & auth lines appear in your ~/.npmrc (with sensitive information removed)

@khai-tran
Copy link
Author

Hi @michaelkedar, thanks for taking a look. Sorry I made a mistake in my earlier comment, as my config is using Basic Auth with token-like pasword.

The token is passed via username/password pair, so I suspect it should be passed in Authorization: Basic header instead. The example configuration is as below:

@org:registry=https://hostname/node/virtual/
//hostname/node/virtual/:_password="<token>"
//hostname/node/virtual/:always-auth=true
//hostname/node/virtual/:username=<username>
registry=https://hostname/node/virtual/

Here are a few more examples with basic auth:

The API is similar to Jfrog/Artifactory, but it's compatible with basic npm commands like install, list, update.

@michaelkedar
Copy link
Member

@khai-tran I think I know what the issue is:
I had assumed that the auth sections in the npmrc file were only for hostnames without paths.

While I work out how to allow for per-path authentication, you could temporarily replace/override your .npmrc to omit the /node/virtual/ path e.g.:

//hostname:username=<username>
//hostname:_password="<token>"

Note that this will mean guided remediation will send the auth header with every request it makes to that hostname - it won't work if there's a second registry on the same host that requires different authentication.

@michaelkedar michaelkedar added the bug Something isn't working label Apr 3, 2024
@khai-tran
Copy link
Author

Thanks for taking a look! Unfortunately removing the web path broke npm client. Looking at the traffic, it looks like because it only attaches token in the http://hostname path, instead of http://hostname/node/virtual/.

@michaelkedar
Copy link
Member

michaelkedar commented Apr 3, 2024

Just to confirm, the following breaks on your npm?

# include the paths on registry
@org:registry=https://hostname/node/virtual/
registry=https://hostname/node/virtual/

# exclude the paths on auth (with or without trailing '/')
//hostname/:username=<username>
//hostname/:_password="<token>"
//hostname/:always-auth=true

What version of npm are you using?

@khai-tran
Copy link
Author

khai-tran commented Apr 4, 2024

  • npm version 10.2.4
  • osv-scanner version 1.7.1

with the above config (I missed trailing / at the end of hostname), npm show worked, but osv-scanner returns the same not found error as before:

cannot find matching versions for ^version: package NPM:@package: not found

that might suggest that auth work, but the path wasn't built correctly.

@khai-tran
Copy link
Author

Sorry, I forgot to include --data-source=native in my latest run. It works now, thanks!

@oliverchang
Copy link
Collaborator

Thanks @khai-tran! @michaelkedar is fixing the underlying issue in #901.

In the meantime, please let us know if you have any feedback or other issues! We're very keen to keep iterating on this new experimental feature.

michaelkedar added a commit that referenced this issue Apr 10, 2024
Should fix #899 
Changed the npmrc parsing logic when using `--data-source=native` so
that specifying registries with paths (e.g.
`//my.registry/package/path:_authToken`) will now correctly add the
authorization headers.

Used
[npm-registry-fetch](https://github.com/npm/npm-registry-fetch/tree/main)
as reference.
@michaelkedar
Copy link
Member

#901 has been merged now, which should allow the authorization headers to be correctly set with your original .npmrc file. It will be in the next osv-scanner release.

@khai-tran
Copy link
Author

Thanks so much @michaelkedar and @oliverchang!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants