Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
6f6d6172 opened this issue Aug 4, 2021 · 0 comments
Open
Labels
invalid This doesn't seem right

Comments

@6f6d6172
Copy link
Contributor

6f6d6172 commented Aug 4, 2021

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)
@6f6d6172 6f6d6172 added the invalid This doesn't seem right label Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant