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

Add new Rails/LinkToBlank cop #6580

Merged
merged 4 commits into from
Dec 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
## 0.61.0 (2018-12-05)

### New features

* [#6580](https://github.com/rubocop-hq/rubocop/pull/6580): New cop `Rails/LinkToBlank` checks for `link_to` calls with `target: '_blank'` and no `rel: 'noopener'`. ([@Intrepidd][])
* [#6457](https://github.com/rubocop-hq/rubocop/pull/6457): Support inner slash correction on `Style/RegexpLiteral`. ([@r7kamura][])
* [#6475](https://github.com/rubocop-hq/rubocop/pull/6475): Support brace correction on `Style/Lambda`. ([@r7kamura][])
* [#6469](https://github.com/rubocop-hq/rubocop/pull/6469): Enforce no parentheses style in the `Style/MethodCallWithArgsParentheses` cop. ([@gsamokovarov][])
Expand Down Expand Up @@ -3701,3 +3701,4 @@
[@bayandin]: https://github.com/bayandin
[@nadiyaka]: https://github.com/nadiyaka
[@amatsuda]: https://github.com/amatsuda
[@Intrepidd]: https://github.com/Intrepidd
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2399,6 +2399,12 @@ Rails/LexicallyScopedActionFilter:
Include:
- app/controllers/**/*.rb

Rails/LinkToBlank:
Description: 'Checks that `link_to` with a `target: "_blank"` have a `rel: "noopener"` option passed to them.'
Intrepidd marked this conversation as resolved.
Show resolved Hide resolved
Reference: https://mathiasbynens.github.io/rel-noopener/
Enabled: true
VersionAdded: '0.62'

Rails/NotNullColumn:
Description: 'Do not add a NOT NULL column without a default value'
Enabled: true
Expand Down
7 changes: 4 additions & 3 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,9 @@
require_relative 'rubocop/cop/rails/bulk_change_table'
require_relative 'rubocop/cop/rails/create_table_with_timestamps'
require_relative 'rubocop/cop/rails/date'
require_relative 'rubocop/cop/rails/dynamic_find_by'
require_relative 'rubocop/cop/rails/delegate'
require_relative 'rubocop/cop/rails/delegate_allow_blank'
require_relative 'rubocop/cop/rails/dynamic_find_by'
require_relative 'rubocop/cop/rails/enum_uniqueness'
require_relative 'rubocop/cop/rails/environment_comparison'
require_relative 'rubocop/cop/rails/exit'
Expand All @@ -579,18 +579,19 @@
require_relative 'rubocop/cop/rails/http_status'
require_relative 'rubocop/cop/rails/inverse_of'
require_relative 'rubocop/cop/rails/lexically_scoped_action_filter'
require_relative 'rubocop/cop/rails/link_to_blank'
require_relative 'rubocop/cop/rails/not_null_column'
require_relative 'rubocop/cop/rails/output_safety'
require_relative 'rubocop/cop/rails/output'
require_relative 'rubocop/cop/rails/output_safety'
require_relative 'rubocop/cop/rails/pluralization_grammar'
require_relative 'rubocop/cop/rails/presence'
require_relative 'rubocop/cop/rails/present'
require_relative 'rubocop/cop/rails/read_write_attribute'
require_relative 'rubocop/cop/rails/redundant_receiver_in_with_options'
require_relative 'rubocop/cop/rails/refute_methods'
require_relative 'rubocop/cop/rails/relative_date_constant'
require_relative 'rubocop/cop/rails/request_referer'
require_relative 'rubocop/cop/rails/reversible_migration'
require_relative 'rubocop/cop/rails/relative_date_constant'
require_relative 'rubocop/cop/rails/safe_navigation'
require_relative 'rubocop/cop/rails/save_bang'
require_relative 'rubocop/cop/rails/scope_args'
Expand Down
51 changes: 51 additions & 0 deletions lib/rubocop/cop/rails/link_to_blank.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop checks for calls to `link_to` that contain a
# `target: '_blank'` but no `rel: 'noopener'`. This can be a security
# risk as the loaded page will have control over the previous page
# and could change its location for phishing purposes.
#
# @example
# # bad
# link_to 'Click here', url, target: '_blank'
#
# # good
# link_to 'Click here', url, target: '_blank', rel: 'noopener'
class LinkToBlank < Cop
MSG = 'Specify a `:rel` option containing noopener.'.freeze

def_node_matcher :blank_target?, <<-PATTERN
(pair {(sym :target) (str "target")} (str "_blank"))
PATTERN

def_node_matcher :includes_noopener?, <<-PATTERN
(pair {(sym :rel) (str "rel")} (str #contains_noopener?))
PATTERN

def on_send(node)
return unless node.method?(:link_to)

option_nodes = node.each_child_node(:hash)

option_nodes.map(&:children).each do |options|
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can go with #each_pair here, instead of map(&:children).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately at this point option_nodes is an Enumerator of Hash nodes

blank = options.find { |o| blank_target?(o) }
if blank && options.none? { |o| includes_noopener?(o) }
add_offense(blank)
end
end
end

private

def contains_noopener?(str)
return false unless str

str.split(' ').include?('noopener')
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ In the following section you find all available cops:
* [Rails/HttpStatus](cops_rails.md#railshttpstatus)
* [Rails/InverseOf](cops_rails.md#railsinverseof)
* [Rails/LexicallyScopedActionFilter](cops_rails.md#railslexicallyscopedactionfilter)
* [Rails/LinkToBlank](cops_rails.md#railslinktoblank)
* [Rails/NotNullColumn](cops_rails.md#railsnotnullcolumn)
* [Rails/Output](cops_rails.md#railsoutput)
* [Rails/OutputSafety](cops_rails.md#railsoutputsafety)
Expand Down
25 changes: 25 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,31 @@ Include | `app/controllers/**/*.rb` | Array

* [https://github.com/rubocop-hq/rails-style-guide#lexically-scoped-action-filter](https://github.com/rubocop-hq/rails-style-guide#lexically-scoped-action-filter)

## Rails/LinkToBlank

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | 0.62 | -

This cop checks for calls to `link_to` that contain a
`target: '_blank'` but no `rel: 'noopener'`. This can be a security
risk as the loaded page will have control over the previous page
and could change its location for phishing purposes.

### Examples

```ruby
# bad
link_to 'Click here', url, target: '_blank'

# good
link_to 'Click here', url, target: '_blank', rel: 'noopener'
```

### References

* [https://mathiasbynens.github.io/rel-noopener/](https://mathiasbynens.github.io/rel-noopener/)

## Rails/NotNullColumn

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
73 changes: 73 additions & 0 deletions spec/rubocop/cop/rails/link_to_blank_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::LinkToBlank do
subject(:cop) { described_class.new }

context 'when not using target _blank' do
it 'does not register an offence' do
expect_no_offenses(<<-RUBY.strip_indent)
link_to 'Click here', 'https://www.example.com'
RUBY
end

it 'does not register an offence when passing options' do
expect_no_offenses(<<-RUBY.strip_indent)
link_to 'Click here', 'https://www.example.com', class: 'big'
RUBY
end

it 'does not register an offence when using the block syntax' do
expect_no_offenses(<<-RUBY.strip_indent)
link_to 'https://www.example.com', class: 'big' do
"Click Here"
end
RUBY
end
end

context 'when using target_blank' do
context 'when using no rel' do
it 'registers an offence' do
expect_offense(<<-RUBY.strip_indent)
link_to 'Click here', 'https://www.example.com', target: '_blank'
^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener.
RUBY
end

it 'registers an offence when using a string for the target key' do
expect_offense(<<-RUBY.strip_indent)
link_to 'Click here', 'https://www.example.com', "target" => '_blank'
^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener.
RUBY
end

it 'registers an offence when using the block syntax' do
expect_offense(<<-RUBY.strip_indent)
link_to 'https://www.example.com', target: '_blank' do
^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener.
"Click here"
end
RUBY
end
end

context 'when using rel' do
context 'when the rel does not contain noopener' do
it 'registers an offence ' do
expect_offense(<<-RUBY.strip_indent)
link_to 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated'
^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener.
RUBY
end
end

context 'when the rel contains noopener' do
it 'register no offence' do
expect_no_offenses(<<-RUBY.strip_indent)
link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener noreferrer'
RUBY
end
end
end
end
end