Skip to content

Commit

Permalink
npm/yarn: Always use registry source when available
Browse files Browse the repository at this point in the history 
Previously there was some logic in place to determine if a package came
from a public package repo or not, and if it came from a registry
considered central/public, we would not parse its source.

However, in the case of private packages that specify yarnpkg.com as
their source, this resulted in some calls expecting that package to
exist on npmjs.org, which it technically is (yarnpkg.com is a proxy for
it), but we would have no credentials configured for npmjs.org, so those
requests will fail.

This PR changes that behavior to always parse a package's source and add
it to the requirement if it exists. We previously only did this for
registries we considered "private", hence the name `private_registry`,
but since we cannot reliably determine if a package hosted on
`yarnpkg.com` or `npmjs.org` is private or not, this has been renamed to
just `registry`.

This means we no longer fall back to `npmjs.org` for private packages
that go through `yarnpkg.com`, so any credentials that are configured
can be reliably used.

I considered always configuring creds for `npmjs.org` for `yarnpkg.com`
and vice versa, but it seemed a little gnarly, and I think this change
is more clear.
  • Loading branch information
@jurre
jurre committed Apr 14, 2021
1 parent 98eddfc commit 2d7c6be
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 113 deletions.
7 changes: 3 additions & 4 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,9 @@ def source_for(name, requirement, manifest_name)

return unless resolved_url
return unless resolved_url.start_with?("http")
return if CENTRAL_REGISTRIES.any? { |u| resolved_url.start_with?(u) }
return if resolved_url.match?(/(?<!pkg\.)github/)

private_registry_source_for(resolved_url, name)
registry_source_for(resolved_url, name)
end

def requirement_for(requirement)
Expand Down Expand Up @@ -287,7 +286,7 @@ def git_source_for(requirement)
}
end

def private_registry_source_for(resolved_url, name)
def registry_source_for(resolved_url, name)
url =
if resolved_url.include?("/~/")
# Gemfury format
Expand All @@ -305,7 +304,7 @@ def private_registry_source_for(resolved_url, name)
else resolved_url.split("/")[0..2].join("/")
end

{ type: "private_registry", url: url }
{ type: "registry", url: url }
end

def url_for_relevant_cred(resolved_url)
Expand Down
2 changes: 1 addition & 1 deletion npm_and_yarn/lib/dependabot/npm_and_yarn/metadata_finder.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def look_up_source

case source_type
when "git" then find_source_from_git_url
when "private_registry" then find_source_from_registry
when "registry" then find_source_from_registry
else raise "Unexpected source type: #{source_type}"
end
end
Expand Down
14 changes: 7 additions & 7 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/registry_finder.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ def first_registry_with_dependency_details

def registry_url
protocol =
if private_registry_source_url
private_registry_source_url.split("://").first
if registry_source_url
registry_source_url.split("://").first
else
"https"
end
Expand Down Expand Up @@ -92,10 +92,10 @@ def auth_token
end

def locked_registry
return unless private_registry_source_url
return unless registry_source_url

lockfile_registry =
private_registry_source_url.
registry_source_url.
gsub("https://", "").
gsub("http://", "")
detailed_registry =
Expand Down Expand Up @@ -210,16 +210,16 @@ def escaped_dependency_name
dependency.name.gsub("/", "%2F")
end

def private_registry_source_url
def registry_source_url
sources = dependency.requirements.
map { |r| r.fetch(:source) }.uniq.compact

# If there are multiple source types, or multiple source URLs, then
# it's unclear how we should proceed
raise "Multiple sources! #{sources.join(', ')}" if sources.map { |s| [s[:type], s[:url]] }.uniq.count > 1

# Otherwise we just take the URL of the first private registry
sources.find { |s| s[:type] == "private_registry" }&.fetch(:url)
# Otherwise we just take the URL of the first registry
sources.find { |s| s[:type] == "registry" }&.fetch(:url)
end
end
end
Expand Down
60 changes: 30 additions & 30 deletions npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
requirement: "^0.0.1",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand All @@ -84,7 +84,7 @@
requirement: "*",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand Down Expand Up @@ -127,7 +127,7 @@
requirement: "^1.1.4",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand All @@ -149,7 +149,7 @@
requirement: "^1.0.0",
file: "package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand Down Expand Up @@ -203,7 +203,7 @@
file: "package.json",
groups: ["dependencies"],
source: {
type: "private_registry",
type: "registry",
url: "http://registry.npm.taobao.org"
}
}]
Expand All @@ -224,7 +224,7 @@
file: "package.json",
groups: ["devDependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://npm.fury.io/dependabot"
}
}]
Expand All @@ -245,7 +245,7 @@
file: "package.json",
groups: ["devDependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://npm.pkg.github.com"
}
}]
Expand All @@ -266,7 +266,7 @@
file: "package.json",
groups: ["devDependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://gitlab.mydomain.com/api/v4/"\
"packages/npm"
}
Expand All @@ -288,7 +288,7 @@
file: "package.json",
groups: ["devDependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://artifactory01.mydomain.com/artifactory/api/"\
"npm/my-repo"
}
Expand All @@ -310,7 +310,7 @@
file: "package.json",
groups: ["dependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://artifactory01.mydomain.com/artifactory/api/"\
"npm/my-repo"
}
Expand All @@ -335,7 +335,7 @@
file: "package.json",
groups: ["dependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://artifactory01.mydomain.com/artifactory/"\
"api/npm/my-repo"
}
Expand All @@ -359,7 +359,7 @@
file: "package.json",
groups: ["dependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://artifactory01.mydomain.com/artifactory/"\
"api/npm/my-repo"
}
Expand All @@ -383,7 +383,7 @@
file: "package.json",
groups: ["devDependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://api.bintray.com/npm/dependabot/npm-private"
}
}]
Expand All @@ -409,7 +409,7 @@
requirement: "^1.0.0",
file: "package.json",
groups: ["optionalDependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand Down Expand Up @@ -714,7 +714,7 @@
requirement: "^0.0.1",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand Down Expand Up @@ -766,7 +766,7 @@
requirement: "^0.0.1",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand All @@ -788,7 +788,7 @@
requirement: "next",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand All @@ -810,7 +810,7 @@
requirement: "^1.0.0",
file: "package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand All @@ -834,7 +834,7 @@
requirement: "^1.0.0",
file: "package.json",
groups: ["optionalDependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand All @@ -860,7 +860,7 @@
requirement: "^0.1.0",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand Down Expand Up @@ -943,7 +943,7 @@
file: "package.json",
groups: ["dependencies"],
source: {
type: "private_registry",
type: "registry",
url: "http://registry.npm.taobao.org"
}
}]
Expand All @@ -964,7 +964,7 @@
file: "package.json",
groups: ["devDependencies"],
source: {
type: "private_registry",
type: "registry",
url: "https://npm.fury.io/dependabot"
}
}]
Expand Down Expand Up @@ -1195,12 +1195,12 @@
requirement: "^1.1.0",
file: "packages/package1/package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}, {
requirement: "^1.0.0",
file: "other_package/package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand All @@ -1218,17 +1218,17 @@
requirement: "1.2.0",
file: "package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}, {
requirement: "^1.2.1",
file: "other_package/package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}, {
requirement: "^1.2.1",
file: "packages/package1/package.json",
groups: ["dependencies"],
source: nil
source: { type: "registry", url: "https://registry.yarnpkg.com" }
}]
)
end
Expand All @@ -1249,7 +1249,7 @@
requirement: "^3.6.0",
file: "package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand All @@ -1264,12 +1264,12 @@
requirement: "^1.1.0",
file: "packages/package1/package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}, {
requirement: "^1.0.0",
file: "packages/other_package/package.json",
groups: ["devDependencies"],
source: nil
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
Expand Down
2 changes: 1 addition & 1 deletion npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/package_json_updater_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
requirement: "0.1.x",
groups: ["dependencies"],
source: {
type: "private_registry",
type: "registry",
url: "http://registry.npm.taobao.org"
}
}],
Expand Down
2 changes: 1 addition & 1 deletion npm_and_yarn/spec/dependabot/npm_and_yarn/metadata_finder_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@
requirement: "^1.0",
groups: [],
source: {
type: "private_registry",
type: "registry",
url: "https://npm.fury.io/dependabot"
}
}],
Expand Down
4 changes: 2 additions & 2 deletions npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/latest_version_finder_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@
requirement: "^1.0.0",
groups: [],
source: {
type: "private_registry",
type: "registry",
url: "https://npm.fury.io/dependabot"
}
}],
Expand Down Expand Up @@ -551,7 +551,7 @@
requirement: "^1.0.0",
groups: [],
source: {
type: "private_registry",
type: "registry",
url: "http://npm.fury.io/dependabot"
}
}],
Expand Down
4 changes: 2 additions & 2 deletions npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/registry_finder_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@

context "with a private registry source" do
let(:source) do
{ type: "private_registry", url: "https://npm.fury.io/dependabot" }
{ type: "registry", url: "https://npm.fury.io/dependabot" }
end

it { is_expected.to eq("npm.fury.io/dependabot") }
Expand Down Expand Up @@ -281,7 +281,7 @@

context "with a private registry source" do
let(:source) do
{ type: "private_registry", url: "http://npm.mine.io/dependabot/" }
{ type: "registry", url: "http://npm.mine.io/dependabot/" }
end

it { is_expected.to eq("http://npm.mine.io/dependabot/etag") }
Expand Down
Loading

0 comments on commit 2d7c6be

Please sign in to comment.