Fix npm workspace package-lock.json entries incorrectly updated when requirement unchanged

npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb

@@ -1027,20 +1027,188 @@ def remove_lockfile_packages_name_attribute(current_name, updated_lockfile_conte
         def restore_locked_package_dependencies(updated_lockfile_content, parsed_updated_lockfile_content)
           dependency_names_to_restore = (dependencies.map(&:name) + git_dependencies_to_lock.keys).uniq
 
+          updated_lockfile_content = restore_root_package_locked_dependencies(
+            updated_lockfile_content, parsed_updated_lockfile_content, dependency_names_to_restore
+          )
+          restore_workspace_package_locked_dependencies(
+            updated_lockfile_content, parsed_updated_lockfile_content, dependency_names_to_restore
+          )
+        end
+
+        sig do
+          params(
+            updated_lockfile_content: String,
+            parsed_updated_lockfile_content: T::Hash[String, T.untyped],
+            dependency_names_to_restore: T::Array[String]
+          )
+            .returns(String)
+        end
+        def restore_root_package_locked_dependencies(
+          updated_lockfile_content, parsed_updated_lockfile_content, dependency_names_to_restore
+        )
+          root_marker = '"": {'
+
           NpmAndYarn::FileParser.each_dependency(parsed_package_json) do |dependency_name, original_requirement, type|
             next unless dependency_names_to_restore.include?(dependency_name)
 
             locked_requirement = parsed_updated_lockfile_content.dig("packages", "", type, dependency_name)
             next unless locked_requirement
+            next if locked_requirement == original_requirement
+
+            updated_lockfile_content = replace_in_lockfile_section(
+              updated_lockfile_content, root_marker, dependency_name, locked_requirement, original_requirement
+            )
+
+            updated_lockfile_content = replace_in_v1_dependencies(
+              updated_lockfile_content, dependency_name, locked_requirement, original_requirement
+            )
+          end
 
-            locked_req = %("#{dependency_name}": "#{locked_requirement}")
-            original_req = %("#{dependency_name}": "#{original_requirement}")
-            updated_lockfile_content = updated_lockfile_content.gsub(locked_req, original_req)
+          updated_lockfile_content
+        end
+
+        # npm may update workspace lockfile entries to the exact installed version
+        # even when the workspace's package.json still has the original range.
+        sig do
+          params(
+            updated_lockfile_content: String,
+            parsed_updated_lockfile_content: T::Hash[String, T.untyped],
+            dependency_names_to_restore: T::Array[String]
+          )
+            .returns(String)
+        end
+        def restore_workspace_package_locked_dependencies(
+          updated_lockfile_content, parsed_updated_lockfile_content, dependency_names_to_restore
+        )
+          workspace_package_files.each do |workspace_file|
+            workspace_key = workspace_lockfile_key(workspace_file)
+            next unless parsed_updated_lockfile_content.dig("packages", workspace_key)
+
+            # Uses updated content so that intentionally bumped requirements are not reverted.
+            workspace_content = updated_package_json_content(workspace_file)
+            next unless workspace_content
+
+            parsed_workspace_pkg = JSON.parse(workspace_content)
+            NpmAndYarn::FileParser.each_dependency(parsed_workspace_pkg) do |dep_name, updated_req, type|
+              next unless dependency_names_to_restore.include?(dep_name)
+
+              locked_requirement = parsed_updated_lockfile_content.dig("packages", workspace_key, type, dep_name)
+              next unless locked_requirement
+              next if locked_requirement == updated_req
+
+              workspace_marker = %("#{workspace_key}": {)
+              updated_lockfile_content = replace_in_lockfile_section(
+                updated_lockfile_content, workspace_marker, dep_name, locked_requirement, updated_req
+              )
+            end
           end
 
           updated_lockfile_content
         end
 
+        sig do
+          params(
+            content: String,
+            section_marker: String,
+            dep_name: String,
+            locked_req: String,
+            original_req: String
+          )
+            .returns(String)
+        end
+        def replace_in_lockfile_section(content, section_marker, dep_name, locked_req, original_req)
+          # index returns the first occurrence, which is correct because npm
+          # lockfile section markers (e.g. '"app-a": {') are unique top-level keys.
+          section_start = content.index(section_marker)
+          return content unless section_start
+
+          locked_fragment = %("#{dep_name}": "#{locked_req}")
+          original_fragment = %("#{dep_name}": "#{original_req}")
+
+          v1_boundary = v1_dependencies_start(content)
+          # Find the end of this section's JSON object so we don't replace
+          # in a neighbouring section that happens to contain the same key.
+          section_end = find_section_end(content, section_start)
+
+          upper_bound = [v1_boundary, section_end].compact.select { |b| b > section_start }.min
+
+          prefix = T.must(content[0...section_start])
+          if upper_bound
+            mid = T.must(content[section_start...upper_bound])
+            suffix = T.must(content[upper_bound..])
+            prefix + mid.sub(locked_fragment, original_fragment) + suffix
+          else
+            remainder = T.must(content[section_start..])
+            prefix + remainder.sub(locked_fragment, original_fragment)
+          end
+        end
+
+        # Finds the closing brace of the JSON object that starts at section_start.
+        # NOTE: This naively counts braces and does not skip characters inside
+        # quoted strings. This is acceptable for npm lockfiles where string values
+        # do not contain unescaped braces.
+        sig { params(content: String, section_start: Integer).returns(T.nilable(Integer)) }
+        def find_section_end(content, section_start)
+          brace_start = content.index("{", section_start)
+          return nil unless brace_start
+
+          depth = 0
+          (brace_start...content.length).each do |i|
+            case content[i]
+            when "{" then depth += 1
+            when "}"
+              depth -= 1
+              return i + 1 if depth.zero?
+            end
+          end
+          nil
+        end
+
+        # lockfileVersion 2 lockfiles have a top-level "dependencies" key with
+        # v1-format entries that use exact versions in "requires".
+        sig { params(content: String).returns(T.nilable(Integer)) }
+        def v1_dependencies_start(content)
+          match = content.match(/^\s{2}\},\n\s{2}"dependencies":\s*\{/m)
+          match&.begin(0)
+        end
+
+        sig do
+          params(
+            content: String,
+            dep_name: String,
+            locked_req: String,
+            original_req: String
+          )
+            .returns(String)
+        end
+        def replace_in_v1_dependencies(content, dep_name, locked_req, original_req)
+          v1_start = v1_dependencies_start(content)
+          return content unless v1_start
+
+          locked_fragment = %("#{dep_name}": "#{locked_req}")
+          original_fragment = %("#{dep_name}": "#{original_req}")
+          prefix = T.must(content[0...v1_start])
+          # gsub: v1 "requires" blocks repeat the same dependency across nested
+          # node_modules sub-trees, and all occurrences should be restored.
+          remainder = T.must(content[v1_start..])
+          prefix + remainder.gsub(locked_fragment, original_fragment)
+        end
+
+        sig { returns(T::Array[Dependabot::DependencyFile]) }
+        def workspace_package_files
+          pkg = package_json
+          return [] unless pkg
+
+          package_files.reject { |f| f.name == pkg.name }
+        end
+
+        sig { params(workspace_file: Dependabot::DependencyFile).returns(String) }
+        def workspace_lockfile_key(workspace_file)
+          lockfile_dir = Pathname.new(lockfile.name).dirname
+          workspace_dir = Pathname.new(workspace_file.name).dirname
+          workspace_dir.relative_path_from(lockfile_dir).to_s
+        end
+
         sig { params(updated_lockfile_content: String).returns(String) }
         def replace_swapped_git_ssh_requirements(updated_lockfile_content)
           git_ssh_requirements_to_swap.each do |req|

npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb

@@ -233,17 +233,142 @@
       [{
         file: "packages/bump-version-for-cron/package.json",
         requirement: "^1.3.37",
-        groups: ["dependencies"],
+        groups: ["devDependencies"],
         source: nil
       }]
     end
     let(:previous_requirements) { requirements }
 
     let(:files) { project_dependency_files("npm8/workspace_outdated_deps_not_in_root_package_json") }
 
-    it "updates" do
-      expect(JSON.parse(updated_npm_lock_content)["packages"]["node_modules/@swc/core"]["version"])
+    it "updates node_modules version while preserving workspace requirement range" do
+      parsed_lockfile = JSON.parse(updated_npm_lock_content)
+
+      expect(parsed_lockfile["packages"]["node_modules/@swc/core"]["version"])
+        .to eq("1.3.44")
+      expect(parsed_lockfile["packages"]["bump-version-for-cron"]["devDependencies"]["@swc/core"])
+        .to eq("^1.3.37")
+    end
+  end
+
+  context "when workspace requirement was bumped" do
+    let(:dependency_name) { "@swc/core" }
+    let(:version) { "1.3.44" }
+    let(:previous_version) { "1.3.40" }
+    let(:requirements) do
+      [{
+        file: "app/package.json",
+        requirement: "^1.3.40",
+        groups: ["devDependencies"],
+        source: nil
+      }]
+    end
+    let(:previous_requirements) do
+      [{
+        file: "app/package.json",
+        requirement: "^1.3.37",
+        groups: ["devDependencies"],
+        source: nil
+      }]
+    end
+
+    # The fixture lockfile has "1.3.44" for the workspace entry even though
+    # the workspace package.json declares "^1.3.40". This simulates npm pinning
+    # the exact installed version in the lockfile workspace metadata.
+    let(:files) { project_dependency_files("npm8/workspace_outdated_deps_requirement_changed") }
+
+    it "does not restore the workspace lockfile entry when requirement was updated" do
+      parsed_lockfile = JSON.parse(updated_npm_lock_content)
+
+      expect(parsed_lockfile["packages"]["node_modules/@swc/core"]["version"])
+        .to eq("1.3.44")
+      # The workspace entry should reflect the new requirement, not the old one
+      expect(parsed_lockfile["packages"]["app"]["devDependencies"]["@swc/core"])
+        .to eq("^1.3.40")
+    end
+  end
+
+  context "when multiple workspaces share the same dependency with unchanged requirements" do
+    let(:dependency_name) { "@swc/core" }
+    let(:version) { "1.3.44" }
+    let(:previous_version) { "1.3.40" }
+    let(:requirements) do
+      [{
+        file: "app-a/package.json",
+        requirement: "^1.3.37",
+        groups: ["devDependencies"],
+        source: nil
+      }]
+    end
+    let(:previous_requirements) { requirements }
+
+    let(:files) { project_dependency_files("npm8/workspace_multiple_packages_same_dep") }
+
+    it "preserves requirement ranges in both workspace lockfile entries" do
+      parsed_lockfile = JSON.parse(updated_npm_lock_content)
+
+      expect(parsed_lockfile["packages"]["node_modules/@swc/core"]["version"])
+        .to eq("1.3.44")
+      expect(parsed_lockfile["packages"]["app-a"]["devDependencies"]["@swc/core"])
+        .to eq("^1.3.37")
+      expect(parsed_lockfile["packages"]["app-b"]["dependencies"]["@swc/core"])
+        .to eq("^1.3.37")
+    end
+  end
+
+  context "when workspaces declare different ranges for the same dependency" do
+    let(:dependency_name) { "@swc/core" }
+    let(:version) { "1.3.44" }
+    let(:previous_version) { "1.3.40" }
+    let(:requirements) do
+      [{
+        file: "app-a/package.json",
+        requirement: "^1.3.37",
+        groups: ["devDependencies"],
+        source: nil
+      }]
+    end
+    let(:previous_requirements) { requirements }
+
+    let(:files) { project_dependency_files("npm8/workspace_multiple_packages_different_ranges") }
+
+    it "preserves each workspace's own requirement range independently" do
+      parsed_lockfile = JSON.parse(updated_npm_lock_content)
+
+      expect(parsed_lockfile["packages"]["node_modules/@swc/core"]["version"])
+        .to eq("1.3.44")
+      expect(parsed_lockfile["packages"]["app-a"]["devDependencies"]["@swc/core"])
+        .to eq("^1.3.37")
+      expect(parsed_lockfile["packages"]["app-b"]["dependencies"]["@swc/core"])
+        .to eq("^1.3.40")
+    end
+  end
+
+  context "when dependency exists in both root and workspace with different ranges" do
+    let(:dependency_name) { "@swc/core" }
+    let(:version) { "1.3.44" }
+    let(:previous_version) { "1.3.40" }
+    let(:requirements) do
+      [{
+        file: "package.json",
+        requirement: "^1.3.37",
+        groups: ["dependencies"],
+        source: nil
+      }]
+    end
+    let(:previous_requirements) { requirements }
+
+    let(:files) { project_dependency_files("npm8/workspace_dep_in_root_and_workspace") }
+
+    it "preserves root range without clobbering workspace range" do
+      parsed_lockfile = JSON.parse(updated_npm_lock_content)
+
+      expect(parsed_lockfile["packages"]["node_modules/@swc/core"]["version"])
         .to eq("1.3.44")
+      expect(parsed_lockfile["packages"][""]["dependencies"]["@swc/core"])
+        .to eq("^1.3.37")
+      expect(parsed_lockfile["packages"]["app"]["devDependencies"]["@swc/core"])
+        .to eq("^1.3.40")
     end
   end
 

npm_and_yarn/spec/fixtures/projects/npm8/workspace_dep_in_root_and_workspace/app/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@test/app",
+  "version": "1.0.0",
+  "private": true,
+  "devDependencies": {
+    "@swc/core": "^1.3.40"
+  }
+}

npm_and_yarn/spec/fixtures/projects/npm8/workspace_dep_in_root_and_workspace/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "workspace-monorepo",
+  "private": true,
+  "workspaces": [
+    "app"
+  ],
+  "dependencies": {
+    "@swc/core": "^1.3.37"
+  }
+}