diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb index c47f2bddd9..be26300633 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb @@ -2,20 +2,55 @@ # frozen_string_literal: true require "dependabot/shared_helpers" +require "dependabot/npm_and_yarn/version_selector" module Dependabot module NpmAndYarn class PackageManager + extend T::Sig + extend T::Helpers def initialize(package_json, lockfiles:) @package_json = package_json @lockfiles = lockfiles @package_manager = package_json.fetch("packageManager", nil) + @engines = package_json.fetch("engines", nil) end + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/PerceivedComplexity def setup(name) - return unless @package_manager.nil? || @package_manager.start_with?("#{name}@") + # we prioritize version mentioned in "packageManager" instead of "engines" + # i.e. if { engines : "pnpm" : "6" } and { packageManager: "pnpm@6.0.2" }, + # we go for the specificity mentioned in packageManager (6.0.2) - version = requested_version(name) + if Dependabot::Experiments.enabled?(:enable_pnpm_yarn_dynamic_engine) + + unless @package_manager&.start_with?("#{name}@") || (@package_manager&.==name.to_s) || @package_manager.nil? + return + end + + if @engines && @package_manager.nil? + # if "packageManager" doesn't exists in manifest file, + # we check if we can extract "engines" information + Dependabot.logger.info("No \"packageManager\" info found for \"#{name}\"") + version = check_engine_version(name) + + elsif @package_manager&.==name.to_s + # if "packageManager" is found but no version is specified (i.e. pnpm@1.2.3), + # we check if we can get "engines" info to override default version + Dependabot.logger.info("Found \"packageManager\" : \"#{@package_manager}\"") + version = check_engine_version(name) if @engines + + elsif @package_manager&.start_with?("#{name}@") + # if "packageManager" info has version specification i.e. yarn@3.3.1 + # we go with the version in "packageManager" + Dependabot.logger.info("Found \"packageManager\" : \"#{@package_manager}\". Skipped checking \"engines\".") + end + else + return unless @package_manager.nil? || @package_manager&.start_with?("#{name}@") + end + + version ||= requested_version(name) if version raise_if_unsupported!(name, version) @@ -33,6 +68,8 @@ def setup(name) version end + # rubocop:enable Metrics/CyclomaticComplexity + # rubocop:enable Metrics/PerceivedComplexity private @@ -44,6 +81,8 @@ def raise_if_unsupported!(name, version) end def install(name, version) + Dependabot.logger.info("Installing \"#{name}@#{version}\"") + SharedHelpers.run_shell_command( "corepack install #{name}@#{version} --global --cache-only", fingerprint: "corepack install @ --global --cache-only" @@ -53,9 +92,10 @@ def install(name, version) def requested_version(name) return unless @package_manager - match = @package_manager.match(/#{name}@(?\d+.\d+.\d+)/) + match = @package_manager.match(/^#{name}@(?\d+.\d+.\d+)/) return unless match + Dependabot.logger.info("Requested version #{match['version']}") match["version"] end @@ -63,8 +103,24 @@ def guessed_version(name) lockfile = @lockfiles[name.to_sym] return unless lockfile + Dependabot.logger.info("Estimating version") Helpers.send(:"#{name}_version_numeric", lockfile) end + + sig { params(name: T.untyped).returns(T.nilable(String)) } + def check_engine_version(name) + version_selector = VersionSelector.new + engine_versions = version_selector.setup(@package_json, name) + + if engine_versions.empty? + Dependabot.logger.info("No relevant (engines) info for \"#{name}\"") + return + end + + version = engine_versions[name] + Dependabot.logger.info("Returned (engines) info \"#{name}\" : \"#{version}\"") + version + end end end end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/version_selector.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/version_selector.rb new file mode 100644 index 0000000000..dd545a3d7f --- /dev/null +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/version_selector.rb @@ -0,0 +1,45 @@ +# typed: strict +# frozen_string_literal: true + +require "dependabot/shared_helpers" + +module Dependabot + module NpmAndYarn + class VersionSelector + extend T::Sig + extend T::Helpers + + # For limited testing, allowing only specific versions defined in engines in package.json + # such as "20.8.7", "8.1.2", "8.21.2", + NODE_ENGINE_SUPPORTED_REGEX = /^\d+(?:\.\d+)*$/ + + sig { params(manifest_json: T::Hash[String, T.untyped], name: String).returns(T::Hash[Symbol, T.untyped]) } + def setup(manifest_json, name) + engine_versions = manifest_json["engines"] + + if engine_versions.nil? + Dependabot.logger.info("No info (engines) found") + return {} + end + + # logs entries for analysis purposes + log = engine_versions.select do |engine, _value| + engine.to_s.match(name) + end + Dependabot.logger.info("Found engine info #{log}") unless log.empty? + + # Only keep matching specs versions i.e. "20.21.2", "7.1.2", + # Additional specs can be added later + engine_versions.delete_if { |_key, value| !valid_extracted_version?(value) } + version = engine_versions.select { |engine, _value| engine.to_s.match(name) } + + version + end + + sig { params(version: String).returns(T::Boolean) } + def valid_extracted_version?(version) + version.match?(NODE_ENGINE_SUPPORTED_REGEX) + end + end + end +end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_fetcher_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_fetcher_spec.rb index 1de35a6985..05409f7977 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_fetcher_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_fetcher_spec.rb @@ -1903,6 +1903,248 @@ end end + context "with both packageManager with version and valid engines fields (yarn)" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_with_ver_with_engine_info_yarn", "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager and not engines" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "yarn" => "3.2.3" } } + ) + end + end + + context "with both packageManager with version and valid engines fields (pnpm)" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_with_ver_with_engine_info_pnpm", "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and pnpm version is picked from packageManager and not engines" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "pnpm" => "8.9.0" } } + ) + end + end + + context "with only packageManager and no engines fields (pnpm)" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_with_ver_with_no_engine_info_pnpm", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "pnpm" => "9.5.0" } } + ) + end + end + + context "with only packageManager and no engines fields (yarn)" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_with_ver_with_no_engine_info_yarn", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "yarn" => "1.22.15" } } + ) + end + end + + context "with packageManager and engines fields with engine field having non relevant version (pnpm)" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_pnpm", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager and not engines" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "pnpm" => "8.15.9" } } + ) + end + end + + context "with packageManager and engines fields with engine field having non relevant version (yarn)" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_yarn", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager and not engines" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "yarn" => "1.21.1" } } + ) + end + end + + context "with both packageManager and engines fields of same package-manager" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/without_package_manager_version_and_with_engine_version", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from engines" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "yarn" => "1.9.1" } } + ) + end + end + + context "with both packageManager and engines fields of same package-manager" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/with_package_manager_and_pnpm_npm_engine_info", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from engines" do + expect(file_fetcher_instance.files.count).to eq(1) + expect(file_fetcher_instance.ecosystem_versions).to eq( + { package_managers: { "pnpm" => "8.9.1" } } + ) + end + end + + context "with both packageManager and engines fields of same package-manager" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/without_package_manager_version_and_with_nonrelevant_engine", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager" do + expect(file_fetcher_instance.files.count).to eq(1) + end + end + + context "with packageManager without version and engines fields missing" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/package_manager_without_version_and_no_engines", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and no new version of packageManager is installed" do + expect(file_fetcher_instance.files.count).to eq(1) + end + end + + context "without packageManager and with engines fields" do + before do + Dependabot::Experiments.register(:enable_pnpm_yarn_dynamic_engine, true) + + allow(file_fetcher_instance).to receive(:commit).and_return("sha") + + stub_request(:get, File.join(url, "package.json?ref=sha")) + .to_return( + status: 200, + body: fixture_to_response("projects/generic/without_package_manager_version_and_with_engine_version", + "package.json"), + headers: json_header + ) + end + + it "fetches package.json fine and yarn version is picked from packageManager" do + expect(file_fetcher_instance.files.count).to eq(1) + end + end + context "with lockfileVersion not in integer format" do before do allow(file_fetcher_instance).to receive(:commit).and_return("sha") diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_pnpm/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_pnpm/package.json new file mode 100644 index 0000000000..dd0a5ed3d6 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_pnpm/package.json @@ -0,0 +1,13 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "pnpm": "<=6.35.3" + }, + "private": true, + "packageManager": "pnpm@8.15.9" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_yarn/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_yarn/package.json new file mode 100644 index 0000000000..f97b5c8204 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_and_nonrelevant_engine_info_yarn/package.json @@ -0,0 +1,13 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "yarn": "nonrelevant_version" + }, + "private": true, + "packageManager": "yarn@1.21.1" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_engine_info_pnpm/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_engine_info_pnpm/package.json new file mode 100644 index 0000000000..833f462010 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_engine_info_pnpm/package.json @@ -0,0 +1,13 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "pnpm": "8.7.6" + }, + "private": true, + "packageManager": "pnpm@8.9.0" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_engine_info_yarn/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_engine_info_yarn/package.json new file mode 100644 index 0000000000..72571293e5 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_engine_info_yarn/package.json @@ -0,0 +1,13 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "yarn": "2.3.2" + }, + "private": true, + "packageManager": "yarn@3.2.3" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_no_engine_info_pnpm/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_no_engine_info_pnpm/package.json new file mode 100644 index 0000000000..64d49bba98 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_no_engine_info_pnpm/package.json @@ -0,0 +1,14 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "yarn": "1.2.3", + "npm": "2.3.2" + }, + "private": true, + "packageManager": "pnpm@9.5.0" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_no_engine_info_yarn/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_no_engine_info_yarn/package.json new file mode 100644 index 0000000000..a506999386 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_with_ver_with_no_engine_info_yarn/package.json @@ -0,0 +1,14 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "noexitantpackagemanager": "<1.22.15" + + }, + "private": true, + "packageManager": "yarn@1.22.15" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/package_manager_without_version_and_no_engines/package.json b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_without_version_and_no_engines/package.json new file mode 100644 index 0000000000..72e768eea2 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/package_manager_without_version_and_no_engines/package.json @@ -0,0 +1,9 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "private": true, + "packageManager": "yarn" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/with_package_manager_and_pnpm_npm_engine_info/package.json b/npm_and_yarn/spec/fixtures/projects/generic/with_package_manager_and_pnpm_npm_engine_info/package.json new file mode 100644 index 0000000000..8284f0da8b --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/with_package_manager_and_pnpm_npm_engine_info/package.json @@ -0,0 +1,15 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "yarn": "<=1.9.1", + "npm": "3.9.1", + "pnpm": "8.9.1" + }, + "private": true, + "packageManager": "pnpm" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_and_with_engine_version/package.json b/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_and_with_engine_version/package.json new file mode 100644 index 0000000000..9687023a14 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_and_with_engine_version/package.json @@ -0,0 +1,12 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1" + }, + "private": true, + "packageManager": "yarn" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_version_and_with_engine_version/package.json b/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_version_and_with_engine_version/package.json new file mode 100644 index 0000000000..e38adf1324 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_version_and_with_engine_version/package.json @@ -0,0 +1,13 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "yarn": "1.9.1" + }, + "private": true, + "packageManager": "yarn" +} diff --git a/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_version_and_with_nonrelevant_engine/package.json b/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_version_and_with_nonrelevant_engine/package.json new file mode 100644 index 0000000000..518d0a5ea6 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/generic/without_package_manager_version_and_with_nonrelevant_engine/package.json @@ -0,0 +1,13 @@ +{ + "name": "package-manager-unparseable", + "version": "1.0.0", + "main": "index.js", + "license": "UNLICENSED", + "owner": "foo", + "engines": { + "node": "^18.12.1", + "yarn": "<=1.9.1" + }, + "private": true, + "packageManager": "yarn" +}