From 8c107a56d4484f618155b23bd986cd5f68832901 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 31 Jul 2023 19:34:27 +0200 Subject: [PATCH] Normalize Swift package names (#7648) 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. --- .../file_parsers/base/dependency_set.rb | 1 + common/lib/dependabot/update_checkers/base.rb | 2 + swift/lib/dependabot/swift/file_parser.rb | 3 +- .../swift/file_parser/dependency_parser.rb | 13 +++++- .../swift/file_updater/lockfile_updater.rb | 2 +- .../spec/dependabot/swift/file_parser_spec.rb | 41 +++++++++++++------ .../dependabot/swift/file_updater_spec.rb | 10 +++-- .../dependabot/swift/update_checker_spec.rb | 12 +++--- 8 files changed, 57 insertions(+), 27 deletions(-) diff --git a/common/lib/dependabot/file_parsers/base/dependency_set.rb b/common/lib/dependabot/file_parsers/base/dependency_set.rb index 6504708bca8..f68140298e3 100644 --- a/common/lib/dependabot/file_parsers/base/dependency_set.rb +++ b/common/lib/dependabot/file_parsers/base/dependency_set.rb @@ -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 diff --git a/common/lib/dependabot/update_checkers/base.rb b/common/lib/dependabot/update_checkers/base.rb index 8672c100e8c..f4215e1f605 100644 --- a/common/lib/dependabot/update_checkers/base.rb +++ b/common/lib/dependabot/update_checkers/base.rb @@ -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 @@ -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 diff --git a/swift/lib/dependabot/swift/file_parser.rb b/swift/lib/dependabot/swift/file_parser.rb index 53ab668502c..b2dd8279486 100644 --- a/swift/lib/dependabot/swift/file_parser.rb +++ b/swift/lib/dependabot/swift/file_parser.rb @@ -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 diff --git a/swift/lib/dependabot/swift/file_parser/dependency_parser.rb b/swift/lib/dependabot/swift/file_parser/dependency_parser.rb index 1dd6a28ccc8..8e3bf72ac86 100644 --- a/swift/lib/dependabot/swift/file_parser/dependency_parser.rb +++ b/swift/lib/dependabot/swift/file_parser/dependency_parser.rb @@ -4,6 +4,7 @@ require "dependabot/shared_helpers" require "dependabot/dependency" require "json" +require "uri" module Dependabot module Swift @@ -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 } @@ -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 diff --git a/swift/lib/dependabot/swift/file_updater/lockfile_updater.rb b/swift/lib/dependabot/swift/file_updater/lockfile_updater.rb index 17c11dd1558..945755b26ac 100644 --- a/swift/lib/dependabot/swift/file_updater/lockfile_updater.rb +++ b/swift/lib/dependabot/swift/file_updater/lockfile_updater.rb @@ -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 diff --git a/swift/spec/dependabot/swift/file_parser_spec.rb b/swift/spec/dependabot/swift/file_parser_spec.rb index a9685d2b78d..c5f34f05bec 100644 --- a/swift/spec/dependabot/swift/file_parser_spec.rb +++ b/swift/spec/dependabot/swift/file_parser_spec.rb @@ -52,6 +52,7 @@ url = expected[:url] version = expected[:version] name = expected[:name] + identity = expected[:identity] source = { type: "git", url: url, ref: version, branch: nil } dependency = dependencies[index] @@ -59,6 +60,7 @@ 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([ @@ -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", @@ -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", @@ -108,7 +112,8 @@ 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", @@ -116,7 +121,8 @@ 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", @@ -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", @@ -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" } @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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" } diff --git a/swift/spec/dependabot/swift/file_updater_spec.rb b/swift/spec/dependabot/swift/file_updater_spec.rb index 5c32eb07d3e..21cc25fc62e 100644 --- a/swift/spec/dependabot/swift/file_updater_spec.rb +++ b/swift/spec/dependabot/swift/file_updater_spec.rb @@ -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: [{ @@ -66,7 +66,8 @@ requirement_string: "exact: \"7.1.0\"" } }], - package_manager: "swift" + package_manager: "swift", + metadata: { identity: "reactiveswift" } ) ] end @@ -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: [{ @@ -130,7 +131,8 @@ requirement_string: "from: \"1.0.0\"" } }], - package_manager: "swift" + package_manager: "swift", + metadata: { identity: "swift-docc-plugin" } ) ] end diff --git a/swift/spec/dependabot/swift/update_checker_spec.rb b/swift/spec/dependabot/swift/update_checker_spec.rb index 81bae080f7e..39b1dfc48eb 100644 --- a/swift/spec/dependabot/swift/update_checker_spec.rb +++ b/swift/spec/dependabot/swift/update_checker_spec.rb @@ -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" } @@ -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" } @@ -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" } @@ -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" } @@ -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" } @@ -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" }