From 64376d828f329bc2d4a277bcd7f66b5c2ced2837 Mon Sep 17 00:00:00 2001 From: kbukum1 Date: Fri, 22 Nov 2024 13:51:59 -0800 Subject: [PATCH] Fix: Ensure Compatibility with npm >= 8 to Prevent Lockfile Downgrades (#11001) --- .../lib/dependabot/npm_and_yarn/helpers.rb | 4 + .../subdependency_version_resolver.rb | 8 +- .../file_updater/npm_lockfile_updater_spec.rb | 2 +- .../dependabot/npm_and_yarn/helpers_spec.rb | 89 +++++++++++++++++++ .../subdependency_version_resolver_spec.rb | 18 +++- 5 files changed, 112 insertions(+), 9 deletions(-) diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb index e4c34c4727..7b4f8f370f 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb @@ -174,6 +174,10 @@ def self.fetch_yarnrc_yml_value(key, default_value) def self.npm8?(package_lock) return true unless package_lock&.content + if Dependabot::Experiments.enabled?(:enable_corepack_for_npm_and_yarn) + return npm_version_numeric_latest(package_lock) >= NPM_V8 + end + npm_version_numeric(package_lock) == NPM_V8 end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb index afa23c2ae4..00d7e86931 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb @@ -70,8 +70,8 @@ def update_subdependency_in_lockfile(lockfile) run_yarn_updater(path, lockfile_name) elsif lockfile.name.end_with?("pnpm-lock.yaml") run_pnpm_updater(path, lockfile_name) - elsif Helpers.npm8?(lockfile) - run_npm8_updater(path, lockfile_name) + elsif !Helpers.npm8?(lockfile) + run_npm6_updater(path, lockfile_name) else run_npm_updater(path, lockfile_name) end @@ -143,7 +143,7 @@ def run_pnpm_updater(path, lockfile_name) end end - def run_npm8_updater(path, lockfile_name) + def run_npm_updater(path, lockfile_name) SharedHelpers.with_git_configured(credentials: credentials) do Dir.chdir(path) do NativeHelpers.run_npm8_subdependency_update_command([dependency.name]) @@ -153,7 +153,7 @@ def run_npm8_updater(path, lockfile_name) end end - def run_npm_updater(path, lockfile_name) + def run_npm6_updater(path, lockfile_name) SharedHelpers.with_git_configured(credentials: credentials) do Dir.chdir(path) do SharedHelpers.run_helper_subprocess( diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb index ee1bc3c100..c0f8f842d1 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb @@ -1089,7 +1089,7 @@ let(:previous_requirements) { requirements } it "raises a helpful error" do - expect { updated_npm_lock_content }.to raise_error(Dependabot::InconsistentRegistryResponse) + expect { updated_npm_lock_content }.to raise_error(Dependabot::DependencyFileNotResolvable) end end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb index b994cfb338..6786c77495 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb @@ -204,4 +204,93 @@ described_class.install("npm", "7.0.0") end end + + describe "::npm8?" do + let(:lockfile_with_v3) do + Dependabot::DependencyFile.new(name: "package-lock.json", content: { lockfileVersion: 3 }.to_json) + end + let(:lockfile_with_v2) do + Dependabot::DependencyFile.new(name: "package-lock.json", content: { lockfileVersion: 2 }.to_json) + end + let(:empty_lockfile) { Dependabot::DependencyFile.new(name: "package-lock.json", content: "") } + let(:nil_lockfile) { nil } + + context "when the feature flag :enable_corepack_for_npm_and_yarn is enabled" do + before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_corepack_for_npm_and_yarn).and_return(true) + end + + it "returns true if lockfileVersion is 3 or higher" do + expect(described_class.npm8?(lockfile_with_v3)).to be true + end + + it "returns true if lockfileVersion is 2" do + expect(described_class.npm8?(lockfile_with_v2)).to be true + end + + it "returns true if lockfile is empty" do + expect(described_class.npm8?(empty_lockfile)).to be true + end + + it "returns true if lockfile is nil" do + expect(described_class.npm8?(nil_lockfile)).to be true + end + end + + context "when the feature flag :enable_corepack_for_npm_and_yarn is disabled" do + before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:enable_corepack_for_npm_and_yarn).and_return(false) + end + + context "when :npm_fallback_version_above_v6 is enabled" do + before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:npm_fallback_version_above_v6).and_return(true) + end + + it "returns true if lockfileVersion is 2 or higher" do + expect(described_class.npm8?(lockfile_with_v2)).to be true + end + + it "returns true if lockfileVersion is 3 or higher" do + expect(described_class.npm8?(lockfile_with_v3)).to be true + end + + it "returns true if lockfile is empty" do + expect(described_class.npm8?(empty_lockfile)).to be true + end + + it "returns true if lockfile is nil" do + expect(described_class.npm8?(nil_lockfile)).to be true + end + end + + context "when :npm_fallback_version_above_v6 is disabled" do + before do + allow(Dependabot::Experiments).to receive(:enabled?).with(:npm_fallback_version_above_v6).and_return(false) + end + + it "returns false for lockfileVersion < 2" do + lockfile_with_v1 = Dependabot::DependencyFile.new(name: "package-lock.json", + content: { lockfileVersion: 1 }.to_json) + expect(described_class.npm8?(lockfile_with_v1)).to be false + end + + it "returns true for lockfileVersion 2 or higher" do + expect(described_class.npm8?(lockfile_with_v2)).to be true + end + + it "returns true for lockfileVersion 3 or higher" do + expect(described_class.npm8?(lockfile_with_v3)).to be true + end + + it "returns true if lockfile is empty" do + expect(described_class.npm8?(empty_lockfile)).to be true + end + + it "returns false if lockfile is nil" do + expect(described_class.npm8?(nil_lockfile)).to be true + end + end + end + end end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb index 40d32e0bc1..51f3bd5de3 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb @@ -102,7 +102,7 @@ end let(:latest_allowable_version) { "6.0.2" } - # NOTE: The latest vision is 6.0.2, but we can't reach it as other + # NOTE: The latest version is 6.0.2, but we can't reach it as other # dependencies constrain us it { is_expected.to eq(Gem::Version.new("5.7.4")) } end @@ -120,7 +120,7 @@ end let(:latest_allowable_version) { "6.0.2" } - # NOTE: The latest vision is 6.0.2, but we can't reach it as other + # NOTE: The latest version is 6.0.2, but we can't reach it as other # dependencies constrain us it { is_expected.to eq(Gem::Version.new("5.7.4")) } end @@ -138,9 +138,19 @@ end let(:latest_allowable_version) { "6.0.2" } + it "calls run_npm_updater when npm8? is true" do + allow(Dependabot::NpmAndYarn::Helpers).to receive(:npm8?).and_return(true) + expect(resolver).to receive(:run_npm_updater).and_call_original + expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) + end + # NOTE: The latest vision is 6.0.2, but we can't reach it as other # dependencies constrain us - it { is_expected.to eq(Gem::Version.new("5.7.4")) } + it "calls run_npm6_updater when npm8? is false" do + allow(Dependabot::NpmAndYarn::Helpers).to receive(:npm8?).and_return(false) + expect(resolver).to receive(:run_npm6_updater).and_call_original + expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) + end end context "with a npm6 package-lock.json" do @@ -228,7 +238,7 @@ end end - context "when updating a sub dep across both yarn and npm lockfiles" do + context "when updating a sub-dependency across both yarn and npm lockfiles" do let(:dependency_files) { project_dependency_files("npm6_and_yarn/nested_sub_dependency_update") } let(:latest_allowable_version) { "2.0.2" }