Class: RuboCop::Cop::DevDoc::Auth::UnscopedFindJustification

Inherits:
Base
  • Object
show all
Includes:
AdjacentJustification
Defined in:
lib/rubocop/cop/dev_doc/auth/unscoped_find_justification.rb

Overview

A controller lookup that resolves a record from a client-controlled param on a bare model class (Post.find(params[:id])) must carry an adjacent justification naming where the record gets bound to the requester.

Rationale

Post.find(params[:id]) reaches EVERY row of the table — the id is client-controlled and usually enumerable, so unless something else binds the resolved record to the requester (a policy that checks the record's owner/tenant, a tenant-scoped base controller, a non-enumerable capability id), any signed-in user can read or act on any tenant's record by walking ids. This cop exists because that exact shape shipped as cross-tenant IDORs: bare finds whose policies checked only "signed in" or bound the wrong level (the parent, not the specific record).

The signal is inclusion-based: a CONSTANT receiver means unscoped. A lookup that travels through an association or current_user (current_user.posts.find(...), @project.members.find_by(...)) has a non-constant receiver and passes silently — scoping through the association IS the fix this cop pushes toward.

What an offense means

An offense does NOT assert the site is vulnerable. The cop cannot see the policy, callback, or later guard that may already bind the record to the requester — it asserts the binding is UNDOCUMENTED at the lookup. A find that is safe through such controlling code is still a violation until the binding is either moved into the lookup (scoping) or stated in the marker comment; the marker is the intended resolution for legitimate root finds, not a workaround. On adoption, expect a one-time justification sweep over existing lookups — writing each binding sentence honestly IS the audit.

Remedy

Prefer scoping the lookup over justifying it:

❌ — any signed-in user can reach any row
@post = Post.find(params[:id])

✔️ — the lookup itself binds the record to the requester
@post = current_user.posts.find(params[:id])

✔️ — nested records resolve through the already-bound parent
@comment = @post.comments.find_by(id: params[:comment_id])

✔️ — genuinely necessary root find, binding stated for review
# unscoped-find: root record of the route; the policy receives it as
# resource: and checks record-derived tenant admin.
@post = Post.find(params[:id])

✔️ — the lookup key itself is the binding: a non-enumerable
#    capability id (see NonEnumerableKeys)
@organization = Organization.find_by!(uuid: params[:organization_id])

Configuration

JustificationMarker (default: unscoped-find:) — the phrase the adjacent comment must contain (same line or the contiguous comment block directly above).

FinderMethods (default: find, find_by, find_by!) — lookup methods the cop watches.

NonEnumerableKeys (default: uuid, token, secret_key) — lookup keys treated as non-enumerable capability ids. A lookup is skipped only when its whole argument list is plain keyword pairs and every params-derived value is keyed by one of these (find_by!(uuid: params[:id])) — the unguessable key IS the binding, which the rationale above already endorses. The cop only trusts the key NAME; it cannot verify the column is actually unguessable, so only list keys your codebase reserves for high-entropy values. slug is deliberately not a default: slugs are public, guessable identifiers, not capabilities. Anything whose key can't be read off the call keeps the offense: positional lookups (find(params[:id]) — even when :id holds a token), ** splats, dynamic keys, and nested-hash values (association paths). Set to [] to disable the skip entirely.

NOTE: The cop is deliberately narrow: it flags only a DIRECT constant receiver with an argument that references params. A chain rooted in a constant (Post.includes(:x).find(params[:id])) and an id copied into a local first (id = params[:id]; Post.find(id)) are not tracked — reviewers must catch those.

Examples:

# bad — unscoped, client-controlled id
@post = Post.find(params[:id])

# bad — find_by hides the same reach
@post = Post.find_by(id: params[:post_id])

# good — scoped through the requester
@post = current_user.posts.find(params[:id])

# good — justified root find
# unscoped-find: policy binds via resource:; see PostPolicy#show?
@post = Post.find(params[:id])

Constant Summary collapse

MSG = 
'`%<receiver>s.%<method>s(params[...])` resolves a record from a client-controlled ' \
'id with no scoping — unless something binds it to the requester, any signed-in ' \
'user reaches any tenant\'s record by enumerating ids. Scope the lookup through an ' \
'association/current_user, or state the binding in a "%<marker>s" comment ' \
'directly above.'.freeze

Instance Method Summary collapse

Instance Method Details

#on_send(node) ⇒ Object Also known as: on_csend

FinderMethods is configuration-driven, so the name filter lives in on_send rather than RESTRICT_ON_SEND.



114
115
116
117
118
119
120
121
 
# File 'lib/rubocop/cop/dev_doc/auth/unscoped_find_justification.rb', line 114

def on_send(node)
  return unless finder_methods.include?(node.method_name)
  return unless node.receiver&.const_type?
  return unless node.arguments.any? { |argument| references_params?(argument) }
  return if exempted?(node)

  add_offense(node.loc.selector, message: offense_message(node))
end