Merge pull request #11309 from Bo98/exec-cop
rubocops/shell_commands: add cop for shell metacharacters in `exec`
This commit is contained in:
commit
6c9ef43e72
@ -54,7 +54,7 @@ module RuboCop
|
||||
end
|
||||
|
||||
# Returns the string representation if node is of type str(plain) or dstr(interpolated) or const.
|
||||
def string_content(node)
|
||||
def string_content(node, strip_dynamic: false)
|
||||
case node.type
|
||||
when :str
|
||||
node.str_content
|
||||
@ -62,7 +62,7 @@ module RuboCop
|
||||
content = ""
|
||||
node.each_child_node(:str, :begin) do |child|
|
||||
content += if child.begin_type?
|
||||
child.source
|
||||
strip_dynamic ? "" : child.source
|
||||
else
|
||||
child.str_content
|
||||
end
|
||||
|
@ -7,6 +7,42 @@ require "rubocops/shared/helper_functions"
|
||||
module RuboCop
|
||||
module Cop
|
||||
module Style
|
||||
# https://github.com/ruby/ruby/blob/v2_6_3/process.c#L2430-L2460
|
||||
SHELL_BUILTINS = %w[
|
||||
!
|
||||
.
|
||||
:
|
||||
break
|
||||
case
|
||||
continue
|
||||
do
|
||||
done
|
||||
elif
|
||||
else
|
||||
esac
|
||||
eval
|
||||
exec
|
||||
exit
|
||||
export
|
||||
fi
|
||||
for
|
||||
if
|
||||
in
|
||||
readonly
|
||||
return
|
||||
set
|
||||
shift
|
||||
then
|
||||
times
|
||||
trap
|
||||
unset
|
||||
until
|
||||
while
|
||||
].freeze
|
||||
|
||||
# https://github.com/ruby/ruby/blob/v2_6_3/process.c#L2495
|
||||
SHELL_METACHARACTERS = %W[* ? { } [ ] < > ( ) ~ & | \\ $ ; ' ` " \n #].freeze
|
||||
|
||||
# This cop makes sure that shell command arguments are separated.
|
||||
#
|
||||
# @api private
|
||||
@ -27,8 +63,6 @@ module RuboCop
|
||||
].freeze
|
||||
RESTRICT_ON_SEND = TARGET_METHODS.map(&:second).uniq.freeze
|
||||
|
||||
SHELL_METACHARACTERS = %w[> < < | ; : & * $ ? : ~ + @ ! ` ( ) [ ]].freeze
|
||||
|
||||
def on_send(node)
|
||||
TARGET_METHODS.each do |target_class, target_method|
|
||||
next unless node.method_name == target_method
|
||||
@ -49,13 +83,17 @@ module RuboCop
|
||||
next if first_arg.nil? || arg_count >= 2
|
||||
|
||||
first_arg_str = string_content(first_arg)
|
||||
|
||||
# Only separate when no shell metacharacters are present
|
||||
next if SHELL_METACHARACTERS.any? { |meta| first_arg_str.include?(meta) }
|
||||
stripped_first_arg_str = string_content(first_arg, strip_dynamic: true)
|
||||
|
||||
split_args = first_arg_str.shellsplit
|
||||
next if split_args.count <= 1
|
||||
|
||||
# Only separate when no shell metacharacters are present
|
||||
command = split_args.first
|
||||
next if SHELL_BUILTINS.any?(command)
|
||||
next if command&.include?("=")
|
||||
next if SHELL_METACHARACTERS.any? { |meta| stripped_first_arg_str.include?(meta) }
|
||||
|
||||
good_args = split_args.map { |arg| "\"#{arg}\"" }.join(", ")
|
||||
method_string = if target_class
|
||||
"#{target_class}.#{target_method}"
|
||||
@ -68,6 +106,32 @@ module RuboCop
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# This cop disallows shell metacharacters in `exec` calls.
|
||||
#
|
||||
# @api private
|
||||
class ExecShellMetacharacters < Base
|
||||
include HelperFunctions
|
||||
|
||||
MSG = "Don't use shell metacharacters in `exec`. "\
|
||||
"Implement the logic in Ruby instead, using methods like `$stdout.reopen`."
|
||||
|
||||
RESTRICT_ON_SEND = [:exec].freeze
|
||||
|
||||
def on_send(node)
|
||||
return if node.receiver.present? && node.receiver != s(:const, nil, :Kernel)
|
||||
return if node.arguments.count != 1
|
||||
|
||||
stripped_arg_str = string_content(node.arguments.first, strip_dynamic: true)
|
||||
command = string_content(node.arguments.first).shellsplit.first
|
||||
|
||||
return if SHELL_BUILTINS.none?(command) &&
|
||||
!command&.include?("=") &&
|
||||
SHELL_METACHARACTERS.none? { |meta| stripped_arg_str.include?(meta) }
|
||||
|
||||
add_offense(node.arguments.first, message: MSG)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -3,210 +3,239 @@
|
||||
|
||||
require "rubocops/shell_commands"
|
||||
|
||||
describe RuboCop::Cop::Style::ShellCommands do
|
||||
subject(:cop) { described_class.new }
|
||||
module RuboCop
|
||||
module Cop
|
||||
module Style
|
||||
describe ShellCommands do
|
||||
subject(:cop) { described_class.new }
|
||||
|
||||
context "when auditing shell commands" do
|
||||
it "reports and corrects an offense when `system` arguments should be separated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo bar"
|
||||
^^^^^^^^^ Separate `system` commands into `\"foo\", \"bar\"`
|
||||
context "when auditing shell commands" do
|
||||
it "reports and corrects an offense when `system` arguments should be separated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo bar"
|
||||
^^^^^^^^^ Separate `system` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo", "bar"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `system` arguments involving interpolation should be separated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "\#{bin}/foo bar"
|
||||
^^^^^^^^^^^^^^^^ Separate `system` commands into `\"\#{bin}/foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "\#{bin}/foo", "bar"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when `system` with metacharacter arguments are called" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo bar > baz"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when trailing arguments to `system` are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo", "bar baz"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when `Utils.popen` arguments are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen("foo bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_read` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.safe_popen_read` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_read("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.safe_popen_read` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_read("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_write` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_write("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.popen_write` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_write("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.safe_popen_write` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_write("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.safe_popen_write` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_write("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_read` arguments with interpolation are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("\#{bin}/foo bar")
|
||||
^^^^^^^^^^^^^^^^ Separate `Utils.popen_read` commands into `\"\#{bin}/foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("\#{bin}/foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when `Utils.popen_read` arguments with metacharacters are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo bar > baz")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when trailing arguments to `Utils.popen_read` are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo", "bar baz")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_read` arguments are unseparated after a shell env" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read({ "SHELL" => "bash"}, "foo bar")
|
||||
^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read({ "SHELL" => "bash"}, "foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo", "bar"
|
||||
describe ExecShellMetacharacters do
|
||||
subject(:cop) { described_class.new }
|
||||
|
||||
context "when auditing exec calls" do
|
||||
it "reports aan offense when output piping is used" do
|
||||
expect_offense(<<~RUBY)
|
||||
fork do
|
||||
exec "foo bar > output"
|
||||
^^^^^^^^^^^^^^^^^^ Don\'t use shell metacharacters in `exec`. Implement the logic in Ruby instead, using methods like `$stdout.reopen`.
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when no metacharacters are used" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
fork do
|
||||
exec "foo bar"
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `system` arguments with string interpolation should be separated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "\#{bin}/foo bar"
|
||||
^^^^^^^^^^^^^^^^ Separate `system` commands into `\"\#{bin}/foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "\#{bin}/foo", "bar"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when `system` with metacharacter arguments are called" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo bar > baz"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when trailing arguments to `system` are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
system "foo", "bar baz"
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when `Utils.popen` arguments are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen("foo bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_read` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.safe_popen_read` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_read("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.safe_popen_read` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_read("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_write` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_write("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.popen_write` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_write("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.safe_popen_write` arguments are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_write("foo bar")
|
||||
^^^^^^^^^ Separate `Utils.safe_popen_write` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.safe_popen_write("foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_read` arguments with interpolation are unseparated" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("\#{bin}/foo bar")
|
||||
^^^^^^^^^^^^^^^^ Separate `Utils.popen_read` commands into `\"\#{bin}/foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("\#{bin}/foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when `Utils.popen_read` arguments with metacharacters are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo bar > baz")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports no offenses when trailing arguments to `Utils.popen_read` are unseparated" do
|
||||
expect_no_offenses(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read("foo", "bar baz")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
|
||||
it "reports and corrects an offense when `Utils.popen_read` arguments are unseparated after a shell variable" do
|
||||
expect_offense(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read({ "SHELL" => "bash"}, "foo bar")
|
||||
^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"`
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
|
||||
expect_correction(<<~RUBY)
|
||||
class Foo < Formula
|
||||
def install
|
||||
Utils.popen_read({ "SHELL" => "bash"}, "foo", "bar")
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
Loading…
x
Reference in New Issue
Block a user