Betterment/betterlint

params checker doesn't take into account `fetch` in some contexts

Opened this issue · 0 comments

Model.new({something: params.fetch(:something_id)}) isn't flagged by the authz cop in some contexts due to fetch not being tracked in the same way permit is. Adding fetch catches it but introduces false positives due to the way method return values are propagated.

Diff:

diff --git a/lib/rubocop/cop/betterment/utils/parser.rb b/lib/rubocop/cop/betterment/utils/parser.rb
index 1777d46..1d7b8a9 100644
--- a/lib/rubocop/cop/betterment/utils/parser.rb
+++ b/lib/rubocop/cop/betterment/utils/parser.rb
@@ -99,7 +99,7 @@ module RuboCop
 
           children.each do |child|
             ancestors = child.ancestors.select do |ancestor|
-              ancestor.send_type? && ancestor.method?(:permit)
+              ancestor.send_type? && (ancestor.method?(:permit) || ancestor.method?(:fetch))
             end
 
             ancestors.each do |ancestor|

Applying this ends up treating these two return values the same, but they are definitely not:

  • current_user.objects.unwrap_or_raise!.find(params.permit(:object_id))
  • params.permit(:object_id)