Skip to content

Commit

Permalink
Normalize Swift package names (#7648)
Browse files Browse the repository at this point in the history
In other places in Github, like in Security Advisories, or the
Dependency Graph page, a different naming schema is used.

This means that when we try to find security updates to resolve a
certain alert for a dependency, we won't be able to match the dependency
with the advisory in the alert, and Dependabot will see it as non
vulnerable.

So, normalize the schema to what the rest of GitHub uses.
  • Loading branch information
deivid-rodriguez authored Jul 31, 2023
1 parent 5e41074 commit 8c107a5
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 27 deletions.
1 change: 1 addition & 0 deletions common/lib/dependabot/file_parsers/base/dependency_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def combined_dependency(old_dep, new_dep)
version: version,
requirements: requirements,
package_manager: old_dep.package_manager,
metadata: old_dep.metadata,
subdependency_metadata: subdependency_metadata
)
end
Expand Down
2 changes: 2 additions & 0 deletions common/lib/dependabot/update_checkers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ def updated_dependency_without_unlock
previous_version: previous_version,
previous_requirements: dependency.requirements,
package_manager: dependency.package_manager,
metadata: dependency.metadata,
subdependency_metadata: dependency.subdependency_metadata
)
end
Expand All @@ -181,6 +182,7 @@ def updated_dependency_with_own_req_unlock
previous_version: previous_version,
previous_requirements: dependency.requirements,
package_manager: dependency.package_manager,
metadata: dependency.metadata,
subdependency_metadata: dependency.subdependency_metadata
)
end
Expand Down
3 changes: 2 additions & 1 deletion swift/lib/dependabot/swift/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ def parse
name: dep.name,
version: dep.version,
package_manager: dep.package_manager,
requirements: requirements
requirements: requirements,
metadata: dep.metadata
)
else
dependency_set << dep
Expand Down
13 changes: 11 additions & 2 deletions swift/lib/dependabot/swift/file_parser/dependency_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "dependabot/shared_helpers"
require "dependabot/dependency"
require "json"
require "uri"

module Dependabot
module Swift
Expand Down Expand Up @@ -47,12 +48,14 @@ def subdependencies(data, level: 0)
end

def all_dependencies(data, level: 0)
name = data["identity"]
identity = data["identity"]
url = data["url"]
name = normalize(url)
version = data["version"]

source = { type: "git", url: url, ref: version, branch: nil }
args = { name: name, version: version, package_manager: "swift", requirements: [] }
metadata = { identity: identity }
args = { name: name, version: version, package_manager: "swift", requirements: [], metadata: metadata }

if level.zero?
args[:requirements] << { requirement: nil, groups: ["dependencies"], file: nil, source: source }
Expand All @@ -65,6 +68,12 @@ def all_dependencies(data, level: 0)
[dep, *subdependencies(data, level: level + 1)].compact
end

def normalize(source)
uri = URI.parse(source.downcase)

"#{uri.host}#{uri.path}".delete_prefix("www.").delete_suffix(".git")
end

attr_reader :dependency_files, :repo_contents_path, :credentials
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def updated_lockfile_content
File.write(manifest.name, manifest.content)

SharedHelpers.with_git_configured(credentials: credentials) do
try_lockfile_update(dependency.name)
try_lockfile_update(dependency.metadata[:identity])

File.read("Package.resolved")
end
Expand Down
41 changes: 28 additions & 13 deletions swift/spec/dependabot/swift/file_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@
url = expected[:url]
version = expected[:version]
name = expected[:name]
identity = expected[:identity]
source = { type: "git", url: url, ref: version, branch: nil }

dependency = dependencies[index]

expect(dependency).to be_a(Dependabot::Dependency)
expect(dependency.name).to eq(name)
expect(dependency.version).to eq(version)
expect(dependency.metadata).to eq({ identity: identity })

if expected[:requirement]
expect(dependency.requirements).to eq([
Expand Down Expand Up @@ -90,7 +92,8 @@
let(:expectations) do
[
{
name: "reactiveswift",
identity: "reactiveswift",
name: "github.com/reactivecocoa/reactiveswift",
url: "https://github.com/ReactiveCocoa/ReactiveSwift.git",
version: "7.1.0",
requirement: "= 7.1.0",
Expand All @@ -99,7 +102,8 @@
requirement_string: "exact: \"7.1.0\""
},
{
name: "swift-docc-plugin",
identity: "swift-docc-plugin",
name: "github.com/apple/swift-docc-plugin",
url: "https://github.com/apple/swift-docc-plugin",
version: "1.0.0",
requirement: ">= 1.0.0, < 2.0.0",
Expand All @@ -108,15 +112,17 @@
requirement_string: "from: \"1.0.0\""
},
{
name: "swift-benchmark",
identity: "swift-benchmark",
name: "github.com/google/swift-benchmark",
url: "https://github.com/google/swift-benchmark",
version: "0.1.1",
requirement: ">= 0.1.0, < 0.1.2",
declaration_string: ".package(url: \"https://github.com/google/swift-benchmark\", \"0.1.0\"..<\"0.1.2\")",
requirement_string: "\"0.1.0\"..<\"0.1.2\""
},
{
name: "swift-argument-parser",
identity: "swift-argument-parser",
name: "github.com/apple/swift-argument-parser",
url: "https://github.com/apple/swift-argument-parser",
version: "0.5.0",
requirement: ">= 0.4.0, <= 0.5.0",
Expand All @@ -125,7 +131,8 @@
requirement_string: "\"0.4.0\" ... \"0.5.0\""
},
{
name: "combine-schedulers",
identity: "combine-schedulers",
name: "github.com/pointfreeco/combine-schedulers",
url: "https://github.com/pointfreeco/combine-schedulers",
version: "0.10.0",
requirement: ">= 0.9.2, <= 0.10.0",
Expand All @@ -134,7 +141,8 @@
requirement_string: "\"0.9.2\"...\"0.10.0\""
},
{
name: "xctest-dynamic-overlay",
identity: "xctest-dynamic-overlay",
name: "github.com/pointfreeco/xctest-dynamic-overlay",
url: "https://github.com/pointfreeco/xctest-dynamic-overlay",
version: "0.8.5"
}
Expand All @@ -150,7 +158,8 @@
let(:expectations) do
[
{
name: "quick",
identity: "quick",
name: "github.com/quick/quick",
url: "https://github.com/Quick/Quick.git",
version: "7.0.2",
requirement: ">= 7.0.0, < 8.0.0",
Expand All @@ -159,7 +168,8 @@
requirement_string: ".upToNextMajor(from: \"7.0.0\")"
},
{
name: "nimble",
identity: "nimble",
name: "github.com/quick/nimble",
url: "https://github.com/Quick/Nimble.git",
version: "9.0.1",
requirement: ">= 9.0.0, < 9.1.0",
Expand All @@ -168,7 +178,8 @@
requirement_string: ".upToNextMinor(from: \"9.0.0\")"
},
{
name: "swift-openapi-runtime",
identity: "swift-openapi-runtime",
name: "github.com/apple/swift-openapi-runtime",
url: "https://github.com/apple/swift-openapi-runtime",
version: "0.1.5",
requirement: ">= 0.1.0, < 0.2.0",
Expand All @@ -181,7 +192,8 @@
requirement_string: ".upToNextMinor(from: \"0.1.0\")"
},
{
name: "swift-docc-plugin",
identity: "swift-docc-plugin",
name: "github.com/apple/swift-docc-plugin",
url: "https://github.com/apple/swift-docc-plugin",
version: "1.0.0",
requirement: "= 1.0.0",
Expand All @@ -190,7 +202,8 @@
requirement_string: ".exact(\"1.0.0\")"
},
{
name: "swift-benchmark",
identity: "swift-benchmark",
name: "github.com/google/swift-benchmark",
url: "https://github.com/google/swift-benchmark",
version: "0.1.1",
requirement: ">= 0.1.0, < 0.1.2",
Expand All @@ -199,12 +212,14 @@
requirement_string: "\"0.1.0\"..<\"0.1.2\""
},
{
name: "swift-argument-parser",
identity: "swift-argument-parser",
name: "github.com/apple/swift-argument-parser",
url: "https://github.com/apple/swift-argument-parser",
version: "0.5.0"
},
{
name: "xctest-dynamic-overlay",
identity: "xctest-dynamic-overlay",
name: "github.com/pointfreeco/xctest-dynamic-overlay",
url: "https://github.com/pointfreeco/xctest-dynamic-overlay",
version: "0.8.5"
}
Expand Down
10 changes: 6 additions & 4 deletions swift/spec/dependabot/swift/file_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
let(:dependencies) do
[
Dependabot::Dependency.new(
name: "reactiveswift",
name: "github.com/reactivecocoa/reactiveswift",
version: "7.1.1",
previous_version: "7.1.0",
requirements: [{
Expand Down Expand Up @@ -66,7 +66,8 @@
requirement_string: "exact: \"7.1.0\""
}
}],
package_manager: "swift"
package_manager: "swift",
metadata: { identity: "reactiveswift" }
)
]
end
Expand Down Expand Up @@ -97,7 +98,7 @@
let(:dependencies) do
[
Dependabot::Dependency.new(
name: "swift-docc-plugin",
name: "github.com/apple/swift-docc-plugin",
version: "1.1.0",
previous_version: "1.0.0",
requirements: [{
Expand Down Expand Up @@ -130,7 +131,8 @@
requirement_string: "from: \"1.0.0\""
}
}],
package_manager: "swift"
package_manager: "swift",
metadata: { identity: "swift-docc-plugin" }
)
]
end
Expand Down
12 changes: 6 additions & 6 deletions swift/spec/dependabot/swift/update_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
end

context "with an up to date dependency" do
let(:name) { "reactiveswift" }
let(:name) { "github.com/reactivecocoa/reactiveswift" }
let(:url) { "https://github.com/ReactiveCocoa/ReactiveSwift" }
let(:upload_pack_fixture) { "reactive-swift" }

Expand All @@ -79,7 +79,7 @@
end

context "with a dependency that needs only lockfile changes to get updated" do
let(:name) { "quick" }
let(:name) { "github.com/quick/quick" }
let(:url) { "https://github.com/Quick/Quick" }
let(:upload_pack_fixture) { "quick" }

Expand All @@ -105,7 +105,7 @@
end

context "with a dependency that needs manifest changes to get updated" do
let(:name) { "nimble" }
let(:name) { "github.com/quick/nimble" }
let(:url) { "https://github.com/Quick/Nimble" }
let(:upload_pack_fixture) { "nimble" }

Expand Down Expand Up @@ -133,7 +133,7 @@
describe "#lowest_security_fix_version" do
subject(:lowest_security_fix_version) { checker.lowest_security_fix_version }

let(:name) { "nimble" }
let(:name) { "github.com/quick/nimble" }
let(:url) { "https://github.com/Quick/Nimble" }
let(:upload_pack_fixture) { "nimble" }

Expand Down Expand Up @@ -168,7 +168,7 @@
subject(:lowest_resolvable_security_fix_version) { checker.lowest_resolvable_security_fix_version }

context "when a supported newer version is available, and resolvable" do
let(:name) { "nimble" }
let(:name) { "github.com/quick/nimble" }
let(:url) { "https://github.com/Quick/Nimble" }
let(:upload_pack_fixture) { "nimble" }

Expand Down Expand Up @@ -200,7 +200,7 @@
context "when fixed version has conflicts with the project" do
let(:project_name) { "conflicts" }

let(:name) { "vapor" }
let(:name) { "github.com/vapor/vapor" }
let(:url) { "https://github.com/vapor/vapor" }
let(:upload_pack_fixture) { "vapor" }

Expand Down

0 comments on commit 8c107a5

Please sign in to comment.