Class: Scryer::Rules::MissingPolicyScopeRule

Inherits:
Scryer::Rule show all
Defined in:
lib/scryer/rules/missing_policy_scope_rule.rb

Overview

Flags a controller's index action that queries a model directly (Model.all/Model.where(...)) instead of through Pundit's policy_scope, in a controller that clearly uses Pundit elsewhere (an authorize call appears somewhere in the class) — the well-known Pundit gotcha where a team remembers authorize on show/update/destroy (each of which checks one record) but forgets that index needs policy_scope instead, since there's no single record for authorize to check. Scoped to controllers that already show Pundit usage specifically so this doesn't fire on apps that don't use Pundit at all (no authorize anywhere) or that scope their index some other way entirely (a current_user.posts.all association instead of a bare Post.all is already scoped, and isn't flagged — see references_unscoped_query?).

Same looseness as MassAssignmentRule#has_permit_call? and PathTraversalRule#sanitized_via_basename?: checks whether policy_scope appears anywhere in the index method body, not that it specifically wraps the Model.all/.where call — good enough to catch the common case (a bare unscoped query with no policy_scope call at all in sight) without a much harder "does this specific call sit inside that specific block" analysis.

Constant Summary collapse

UNSCOPED_METHODS = 
%w[all where].freeze
NON_MODEL_RECEIVERS =

Same list IdorRule/MassAssignmentRule use for the same reason — stdlib/gem constants with their own .all/.where-shaped methods that have nothing to do with an ActiveRecord model query.

%w[
  Struct OpenStruct Data Class Module BCrypt OpenSSL Net URI Digest
  JSON YAML Marshal String Array Hash Integer Float Symbol Comparable
  Enumerable File Dir
].freeze

Instance Attribute Summary

Attributes inherited from Scryer::Rule

#file, #sexp, #source

Instance Method Summary collapse

Methods inherited from Scryer::Rule

inherited, #initialize

Constructor Details

This class inherits a constructor from Scryer::Rule

Instance Method Details

#scan ⇒ Object 



43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
 
# File 'lib/scryer/rules/missing_policy_scope_rule.rb', line 43

def scan
  findings = []

  Ast.each_node(sexp) do |node|
    next unless Ast.tagged?(node, :class)

    class_name = Ast.class_name(node[1])
    next unless class_name.to_s.end_with?("Controller")

    body = node[3]
    next unless each_call_names(body).include?("authorize") # uses Pundit at all

    each_index_def(body).each do |def_node|
      index_body = def_node[3]
      next if each_call_names(index_body).include?("policy_scope")

      query_node = find_unscoped_query(index_body)
      next unless query_node

      findings << finding(
        line: Ast.line_of(query_node),
        message: "`#{class_name}#index` queries a model directly (`#{describe_call(query_node)}`) " \
                  "instead of through `policy_scope` — this controller uses Pundit's `authorize` " \
                  "elsewhere, but `index` needs `policy_scope` instead (there's no single record " \
                  "for `authorize` to check), so every record is visible here regardless of who's " \
                  "allowed to see what.",
        suggested_fix: "Wrap the query in `policy_scope`, e.g. `policy_scope(#{receiver_name(query_node)}).all` " \
                        "instead of `#{describe_call(query_node)}`, and define the corresponding " \
                        "Scope class in this model's policy."
      )
    end
  end

  findings
end