Unexpected failure specifying https url in an npm credential

[randymarsh77]

Specifying a credential for a private npm registry using a url starting with https:// is invalid, since the logic results in querying a url like https://https://actual.registry.etc

This code is "the problem": https://github.com/dependabot/dependabot-core/blob/83043c7d61ec39cc2e7145dcd87982a1c3378124/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb#L104-L113

Notice that any http scheme is stripped from lockfile_registry, but not detailed_registry.

It's easy enough to workaround this by omitting the scheme from the registry url when specifying credentials, though this seems like an unfortunate pitfall.

The result is that the request fails in an unhandled way and the npm details are empty, so there is no error, but no updates will be found.

It seems like it would be perfectly safe to perform the replacement on both of those strings. (I tested this replacement and the code worked the same as omitting https:// from my credential.)

Package ecosystem npm

[jeffwidman]

Thanks for debugging!

This seems reasonable to me, want to submit a PR with the fix?

[andrcuns]

FYI, I'm fairly certain similar issue exists with docker registry credentials as well.

[github-actions[bot]]

👋 This issue has been marked as stale because it has been open for 2 years with no activity. You can comment on the issue to hold stalebot off for a while, or do nothing. If you do nothing, this issue will be closed eventually by the stalebot. Please see CONTRIBUTING.md for more policy details.

[abdulapopoola]

@sachin-sandhu , can you please help with a fix?

[sachin-sandhu]

Hi @randymarsh77 , can you give us the .npmrc file format that resulted in an error, is it similar to following

registry=https://npm.fury.io/dependabot/ https://npm.fury.io/dependabot/:_authToken=xxxxxx

[sachin-sandhu]

Hi @randymarsh77 , gentle reminder https://github.com/dependabot/dependabot-core/issues/5570#issuecomment-2339442879

[randymarsh77]

I don't believe I ever had an issue when using an npmrc. I had an issue using the dependabot-core library with the following code:

credentials = [
  {
    'type' => 'git_source',
    'host' => github_host,
    'username' => 'x-access-token',
    'password' => github_token
  }
]
azure_npm_token = options[:'az-npm-token']
unless azure_npm_token.nil? || azure_npm_token.empty?
  credentials.append(
    {
      'type' => 'npm_registry',
      'registry' => 'https://pkgs.dev.azure.com/REDACTED/Packages/_packaging/REDACTED/npm/registry/',
      'token' => azure_npm_token
    }
  )
end

# ...

# Maybe this part?

root_update_group = DependabotAction::UpdateGroup.new(
  source: Dependabot::Source.new(
    provider: 'github',
    hostname: github_host,
    api_endpoint: github_api_endpoint,
    repo: repo,
    directory: '.',
    branch: current_branch
  ),
  credentials: credentials,
  group_name: group_name,
  use_semantic_release_format: use_semantic_release_format,
  github_token: github_token,
  github_api_endpoint: github_api_endpoint
)

# ...

# Or, maybe this part?

fetcher = Dependabot::FileFetchers.for_package_manager(package_manager).new(
  source: source,
  credentials: credentials
)
files = fetcher.files
commit = fetcher.commit

# ...

# But, probably this part based on where I tracked the bug to:

checker = Dependabot::UpdateCheckers.for_package_manager(package_manager).new(
  dependency: dep,
  dependency_files: files,
  credentials: credentials
)

next if checker.up_to_date?

In any case, I just removed https:// from that string. I also noted that this is a "problem", quoted because it might be intentional that you must pass an http endpoint and omit the scheme as registry, but it sure looked like an oversight based on the surrounding code that handled the http scheme being included.

[randhircs]

@randymarsh77 Thank you for providing the details and suggested fix. To better understand and reproduce the problem you're experiencing, could you please share the sample files you're working with? Having access to these files will allow me to confirm the issue and test the solution.

[randhircs]

@randymarsh77 The code change you suggested is something that can be contributed directly from your end, please raise a PR here, with use case details and in that way we can merge your code after review. Since this platform is to fix the issue from our end, so closing this ticket. Feel free to create a new issue. Thank you!