Fix Dependabot ignoring scoped registries if the lockfile resolved uri contains a port number

[brrygrdn]

Assuming a project has the following config:

dependabot.yml

version: 2
registries:
  private-npm:
    type: npm-registry
    url: example.com/artifactory/api/npm/example
    username: ${{secrets.USER}}
    password: ${{secrets.PASS}}
updates:
  - package-ecosystem: "npm"
    registries:
      - private-npm
    schedule:
      interval: "daily"

In their package.json they have a dependency from their private registry, which has a resolved URL in the form:

https://example.com:443/artifactory/api/npm/example/@example/example-client/-/@example-corp/example-client-1.0.0.tgz

This will result in confusing behaviour where we will correctly find the latest version in the private registry but fail to actually update the package with a private_source_authentication_failure.

The problem

The private_source_authentication_failure is actually a misdirection, using the debugger I was able to output the raw error which was coming from npm here

npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org/@example%2fexample-client - Not found
npm ERR! 404 
npm ERR! 404  '@example/example-client@1.0.1' is not in this registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

This indicates that we are actually calling the public npm registry and getting a 404 but then assume that it's a private registry problem here as this line is false:

UpdateChecker::RegistryFinder.central_registry?(reg)

The fix

After investigating I discovered that no 'scoped registry' line was being added to the .npmrc file we generate on the fly when performing the update.

It should have had a line like so:

@example=https://example.com/artifactory/api/npm/example

The reason this wasn't being added is that due to a fix for a duplicate trailing slash in the past (https://github.com/dependabot/dependabot-core/pull/1013/files), we started adding a trailing slash to the registry value used in this method unintentionally:

dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npmrc_builder.rb

Lines 174 to 198 in 3428ef7

	def registry_scopes(registry)
	# Central registries don't just apply to scopes
	return if CENTRAL_REGISTRIES.include?(registry)
	return unless dependency_urls
	other_regs =
	registry_credentials.map { |c| c.fetch("registry") } -
	[registry]
	affected_urls =
	dependency_urls.
	select do |url|
	next false unless url.include?(registry)
	other_regs.none? { |r| r.include?(registry) && url.include?(r) }
	end
	scopes = affected_urls.map do |url|
	url.split(/\%40|@/)[1]&.split(%r{\%2[fF]|/})&.first
	end
	# Registry used for unscoped packages
	return if scopes.include?(nil)
	scopes.map { |scope| "@#{scope}:registry=https://#{registry}" }
	end

The fix was to add the compulsory trailing slash later in the method, I've added test cases that verify this resolves the issue.

Unrelated changes

I fixed the bug in fd4fb97 to make the red/green pass as clear as possible. The final commit cleans up some test churn where we expected the scoped registry lines to have the trailing / - this isn't necessary and we tolerate it as a side-effect of the original bug fix.

[pavera]

🚀