Add NoParamsInWorkPackageWhereId cop

CHANGELOG.md

@@ -1,5 +1,11 @@
 ## [Unreleased]
 
+## [0.5.0] - 2026-05-05
+
+- Add `OpenProject/NoParamsInWorkPackageWhereId` cop to catch
+  `WorkPackage.where(id: params[...])` patterns that silently drop semantic
+  identifiers (e.g. `"PROJ-42"`) when PostgreSQL casts the string to integer 0.
+
 ## [0.4.0] - 2026-03-27
 
 - Add NoNotImplementedError cop

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    rubocop-openproject (0.4.0)
+    rubocop-openproject (0.5.0)
       rubocop
 
 GEM

config/default.yml

@@ -13,6 +13,11 @@ OpenProject/NoNotImplementedError:
   Enabled: true
   VersionAdded: '0.4.0'
 
+OpenProject/NoParamsInWorkPackageWhereId:
+  Description: 'Avoid `WorkPackage.where(id: params[...])`; use `where_display_id_in` to honour semantic identifiers.'
+  Enabled: true
+  VersionAdded: '0.5.0'
+
 OpenProject/NoSleepInFeatureSpecs:
   Description: 'Avoid using `sleep` greater than 1 second in feature specs.'
   Enabled: true

lib/rubocop/cop/open_project/no_params_in_work_package_where_id.rb

@@ -0,0 +1,124 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module OpenProject
+      # Flags `WorkPackage.where(id: params[...])` patterns. With semantic work
+      # package identifiers enabled, params may carry strings like "PROJ-42"
+      # that PostgreSQL silently casts to integer 0 inside `where(id: ...)`,
+      # producing an empty result set instead of an error. Use the dedicated
+      # resolver `WorkPackage.where_display_id_in(...)` which partitions
+      # numeric and semantic inputs and consults the alias table.
+      #
+      # The cop fires when the receiver chain demonstrably resolves to a
+      # WorkPackage relation — either rooted at the `WorkPackage` constant or
+      # passing through an association call whose name ends in
+      # `work_packages` (e.g. `project.work_packages`,
+      # `user.assigned_work_packages`) — and the value is derived from
+      # `params[...]`. Internal subquery composition
+      # (`where(id: scope.pluck(:id))`) and primary-key literals are left
+      # alone.
+      #
+      # @example
+      #   # bad
+      #   WorkPackage.where(id: params[:work_package_id])
+      #
+      #   # bad
+      #   WorkPackage.where(id: params[:work_package_id] || params[:ids])
+      #
+      #   # bad
+      #   WorkPackage.includes(:project).where(id: params[:ids])
+      #
+      #   # bad
+      #   project.work_packages.where(id: params[:work_package_id])
+      #
+      #   # bad
+      #   current_user.assigned_work_packages.where(id: params[:ids])
+      #
+      #   # good
+      #   WorkPackage.where_display_id_in(params[:work_package_id])
+      #
+      #   # good
+      #   project.work_packages.where_display_id_in(params[:work_package_id])
+      #
+      #   # good (primary key, not user input)
+      #   WorkPackage.where(id: 42)
+      #
+      #   # good (subquery, not user input)
+      #   WorkPackage.where(id: other_scope.select(:id))
+      class NoParamsInWorkPackageWhereId < Base
+        extend AutoCorrector
+
+        MSG = "Avoid `WorkPackage.where(id: params[...])` — semantic identifiers like " \
+              '"PROJ-42" are silently coerced to 0 by the SQL cast. ' \
+              "Use `WorkPackage.where_display_id_in(...)` instead."
+
+        RESTRICT_ON_SEND = %i[where].freeze
+
+        def_node_matcher :params_access?, <<~PATTERN
+          (send (send nil? :params) :[] _)
+        PATTERN
+
+        # A receiver that traces back through any chain of sends to either:
+        #   - the `WorkPackage` constant: `WorkPackage`, `WorkPackage.foo`, ...
+        #   - an association call whose name ends in `work_packages`:
+        #     `project.work_packages`, `user.assigned_work_packages`, ...
+        # The recursion handles arbitrarily deep chains in either form.
+        def_node_matcher :work_package_relation?, <<~PATTERN
+          {
+            (const nil? :WorkPackage)
+            (send _ #work_package_association? ...)
+            (send #work_package_relation? _ ...)
+          }
+        PATTERN
+
+        def on_send(node)
+          return unless work_package_relation?(node.receiver)
+
+          hash_arg = node.first_argument
+          id_value = id_value_from_hash(hash_arg)
+          return unless id_value && value_uses_params?(id_value)
+
+          add_offense(node) do |corrector|
+            next unless autocorrectable_value?(id_value) && sole_id_predicate?(hash_arg)
+
+            corrector.replace(node, "#{node.receiver.source}.where_display_id_in(#{id_value.source})")
+          end
+        end
+
+        private
+
+        def id_value_from_hash(arg)
+          return unless arg&.hash_type?
+
+          pair = arg.pairs.find { |p| p.key.sym_type? && p.key.value == :id }
+          pair&.value
+        end
+
+        # Refuse to autocorrect when the hash carries additional predicates
+        # (e.g. `where(id: params[:id], project_id: 5)`); rewriting to
+        # `where_display_id_in(params[:id])` would silently drop them.
+        def sole_id_predicate?(hash_arg)
+          hash_arg.pairs.size == 1
+        end
+
+        def value_uses_params?(node)
+          return true if params_access?(node)
+
+          node.each_descendant(:send).any? { |descendant| params_access?(descendant) }
+        end
+
+        def autocorrectable_value?(node)
+          return true if params_access?(node)
+          return false unless node.or_type?
+
+          node.children.all? { |child| autocorrectable_value?(child) }
+        end
+
+        def work_package_association?(method_name)
+          method_name.to_s.end_with?("work_packages")
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/open_project_cops.rb

@@ -2,5 +2,6 @@
 
 require_relative "open_project/add_preview_for_view_component"
 require_relative "open_project/no_not_implemented_error"
+require_relative "open_project/no_params_in_work_package_where_id"
 require_relative "open_project/use_service_result_factory_methods"
 require_relative "open_project/no_sleep_in_feature_specs"

lib/rubocop/open_project/version.rb

@@ -2,6 +2,6 @@
 
 module RuboCop
   module OpenProject
-    VERSION = "0.4.0"
+    VERSION = "0.5.0"
   end
 end

spec/rubocop/cop/open_project/no_params_in_work_package_where_id_spec.rb

@@ -0,0 +1,182 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::OpenProject::NoParamsInWorkPackageWhereId, :config do
+  let(:config) { RuboCop::Config.new }
+
+  context "when the value is params[...] directly" do
+    it "registers an offense and autocorrects to where_display_id_in" do
+      expect_offense(<<~RUBY)
+        WorkPackage.where(id: params[:work_package_id])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        WorkPackage.where_display_id_in(params[:work_package_id])
+      RUBY
+    end
+  end
+
+  context "when the value is `params[...] || params[...]`" do
+    it "registers an offense and autocorrects" do
+      expect_offense(<<~RUBY)
+        WorkPackage.where(id: params[:work_package_id] || params[:ids])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        WorkPackage.where_display_id_in(params[:work_package_id] || params[:ids])
+      RUBY
+    end
+  end
+
+  context "when the receiver is a chain rooted at WorkPackage" do
+    it "registers an offense for an includes chain" do
+      expect_offense(<<~RUBY)
+        WorkPackage.includes(:project).where(id: params[:ids])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        WorkPackage.includes(:project).where_display_id_in(params[:ids])
+      RUBY
+    end
+
+    it "registers an offense for a deeper chain" do
+      expect_offense(<<~RUBY)
+        WorkPackage.visible(user).order(:id).where(id: params[:ids])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        WorkPackage.visible(user).order(:id).where_display_id_in(params[:ids])
+      RUBY
+    end
+  end
+
+  context "when the receiver is an association ending in work_packages" do
+    it "registers an offense for `project.work_packages.where(id: params[...])`" do
+      expect_offense(<<~RUBY)
+        project.work_packages.where(id: params[:work_package_id])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        project.work_packages.where_display_id_in(params[:work_package_id])
+      RUBY
+    end
+
+    it "registers an offense for an `_work_packages`-suffixed association" do
+      expect_offense(<<~RUBY)
+        current_user.assigned_work_packages.where(id: params[:ids])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        current_user.assigned_work_packages.where_display_id_in(params[:ids])
+      RUBY
+    end
+
+    it "registers an offense when the association is followed by a chain" do
+      expect_offense(<<~RUBY)
+        project.work_packages.includes(:project).where(id: params[:ids])
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        project.work_packages.includes(:project).where_display_id_in(params[:ids])
+      RUBY
+    end
+
+    it "does not register an offense for an unrelated association name" do
+      expect_no_offenses(<<~RUBY)
+        project.users.where(id: params[:id])
+      RUBY
+    end
+  end
+
+  context "when params is wrapped in a method call (non-autocorrectable)" do
+    it "registers an offense without an autocorrection" do
+      expect_offense(<<~RUBY)
+        WorkPackage.where(id: params[:id].to_i)
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_no_corrections
+    end
+  end
+
+  context "when the hash carries additional predicates" do
+    it "registers an offense without an autocorrection (would silently drop other predicates)" do
+      expect_offense(<<~RUBY)
+        WorkPackage.where(id: params[:id], project_id: 5)
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoParamsInWorkPackageWhereId: #{described_class::MSG}
+      RUBY
+
+      expect_no_corrections
+    end
+  end
+
+  context "when the predicate key is not :id" do
+    it "does not register an offense" do
+      expect_no_offenses(<<~RUBY)
+        WorkPackage.where(project_id: params[:project_id])
+      RUBY
+    end
+  end
+
+  context "when the value is a primary-key literal" do
+    it "does not register an offense for an integer" do
+      expect_no_offenses(<<~RUBY)
+        WorkPackage.where(id: 42)
+      RUBY
+    end
+
+    it "does not register an offense for an integer array" do
+      expect_no_offenses(<<~RUBY)
+        WorkPackage.where(id: [1, 2, 3])
+      RUBY
+    end
+  end
+
+  context "when the value is a subquery or scope chain" do
+    it "does not register an offense for a select subquery" do
+      expect_no_offenses(<<~RUBY)
+        WorkPackage.where(id: other_scope.select(:id))
+      RUBY
+    end
+
+    it "does not register an offense for a pluck array" do
+      expect_no_offenses(<<~RUBY)
+        WorkPackage.where(id: other_scope.pluck(:id))
+      RUBY
+    end
+  end
+
+  context "when the receiver is not WorkPackage-rooted" do
+    it "does not register an offense for a different model" do
+      expect_no_offenses(<<~RUBY)
+        Project.where(id: params[:id])
+      RUBY
+    end
+
+    it "does not register an offense for an unknown receiver" do
+      expect_no_offenses(<<~RUBY)
+        some_relation.where(id: params[:id])
+      RUBY
+    end
+
+    it "does not register an offense for a namespaced WorkPackage constant" do
+      expect_no_offenses(<<~RUBY)
+        Foo::WorkPackage.where(id: params[:id])
+      RUBY
+    end
+  end
+
+  context "when the call is not where" do
+    it "does not register an offense for find" do
+      expect_no_offenses(<<~RUBY)
+        WorkPackage.find(params[:id])
+      RUBY
+    end
+  end
+end