ManageIQ/manageiq-ssh-util

View on GitHub

Showing 16 of 16 total issues

Method suexec has a Cognitive Complexity of 110 (exceeds 5 allowed). Consider refactoring.
Open

      def suexec(cmd_str, done_string = nil, stdin = nil)
        error_buffer = ""
        output_buffer = ""
        prompt = ""
        cmd_rx = ""
Severity: Minor
Found in lib/manageiq/ssh/util.rb - About 2 days to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method exec has a Cognitive Complexity of 41 (exceeds 5 allowed). Consider refactoring.
Open

      def exec(cmd, done_string = nil, stdin = nil)
        error_buffer = ""
        output_buffer = ""
        status = 0
        signal = nil
Severity: Minor
Found in lib/manageiq/ssh/util.rb - About 6 hrs to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Cyclomatic complexity for suexec is too high. [29/11]
Open

      def suexec(cmd_str, done_string = nil, stdin = nil)
        error_buffer = ""
        output_buffer = ""
        prompt = ""
        cmd_rx = ""
Severity: Minor
Found in lib/manageiq/ssh/util.rb by rubocop

Checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.

An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one. Blocks that are calls to builtin iteration methods (e.g. `ary.map{...}) also add one, others are ignored.

def each_child_node(*types)               # count begins: 1
  unless block_given?                     # unless: +1
    return to_enum(__method__, *types)

  children.each do |child|                # each{}: +1
    next unless child.is_a?(Node)         # unless: +1

    yield child if types.empty? ||        # if: +1, ||: +1
                   types.include?(child.type)
  end

  self
end                                       # total: 6

Method suexec has 86 lines of code (exceeds 25 allowed). Consider refactoring.
Open

      def suexec(cmd_str, done_string = nil, stdin = nil)
        error_buffer = ""
        output_buffer = ""
        prompt = ""
        cmd_rx = ""
Severity: Major
Found in lib/manageiq/ssh/util.rb - About 3 hrs to fix

    Cyclomatic complexity for exec is too high. [19/11]
    Open

          def exec(cmd, done_string = nil, stdin = nil)
            error_buffer = ""
            output_buffer = ""
            status = 0
            signal = nil
    Severity: Minor
    Found in lib/manageiq/ssh/util.rb by rubocop

    Checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.

    An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one. Blocks that are calls to builtin iteration methods (e.g. `ary.map{...}) also add one, others are ignored.

    def each_child_node(*types)               # count begins: 1
      unless block_given?                     # unless: +1
        return to_enum(__method__, *types)
    
      children.each do |child|                # each{}: +1
        next unless child.is_a?(Node)         # unless: +1
    
        yield child if types.empty? ||        # if: +1, ||: +1
                       types.include?(child.type)
      end
    
      self
    end                                       # total: 6

    Method exec has 48 lines of code (exceeds 25 allowed). Consider refactoring.
    Open

          def exec(cmd, done_string = nil, stdin = nil)
            error_buffer = ""
            output_buffer = ""
            status = 0
            signal = nil
    Severity: Minor
    Found in lib/manageiq/ssh/util.rb - About 1 hr to fix

      Method shell_with_su has 6 arguments (exceeds 4 allowed). Consider refactoring.
      Open

            def self.shell_with_su(host, remote_user, remote_password, su_user, su_password, options = {})
      Severity: Minor
      Found in lib/manageiq/ssh/util.rb - About 45 mins to fix

        Avoid parameter lists longer than 5 parameters. [6/5]
        Open

              def self.shell_with_su(host, remote_user, remote_password, su_user, su_password, options = {})
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        Checks for methods with too many parameters.

        The maximum number of parameters is configurable. Keyword arguments can optionally be excluded from the total count, as they add less complexity than positional or optional parameters.

        Any number of arguments for initialize method inside a block of Struct.new and Data.define like this is always allowed:

        Struct.new(:one, :two, :three, :four, :five, keyword_init: true) do
          def initialize(one:, two:, three:, four:, five:)
          end
        end

        This is because checking the number of arguments of the initialize method does not make sense.

        NOTE: Explicit block argument &block is not counted to prevent erroneous change that is avoided by making block argument implicit.

        Example: Max: 3

        # good
        def foo(a, b, c = 1)
        end

        Example: Max: 2

        # bad
        def foo(a, b, c = 1)
        end

        Example: CountKeywordArgs: true (default)

        # counts keyword args towards the maximum
        
        # bad (assuming Max is 3)
        def foo(a, b, c, d: 1)
        end
        
        # good (assuming Max is 3)
        def foo(a, b, c: 1)
        end

        Example: CountKeywordArgs: false

        # don't count keyword args towards the maximum
        
        # good (assuming Max is 3)
        def foo(a, b, c, d: 1)
        end

        This cop also checks for the maximum number of optional parameters. This can be configured using the MaxOptionalParameters config option.

        Example: MaxOptionalParameters: 3 (default)

        # good
        def foo(a = 1, b = 2, c = 3)
        end

        Example: MaxOptionalParameters: 2

        # bad
        def foo(a = 1, b = 2, c = 3)
        end

        Method run_session has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
        Open

              def run_session
                first_try = true
        
                begin
                  Net::SSH.start(@host, @user, @options) do |ssh|
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb - About 25 mins to fix

        Cognitive Complexity

        Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

        A method's cognitive complexity is based on a few simple rules:

        • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
        • Code is considered more complex for each "break in the linear flow of the code"
        • Code is considered more complex when "flow breaking structures are nested"

        Further reading

        Use match? instead of =~ when MatchData is not used.
        Open

                          prompt = data if data =~ /^\[*[\w\-\.]+@[\w\-\.]+.+\]*[\#\$]\s*$/
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

        Example:

        # bad
        def foo
          if x =~ /re/
            do_something
          end
        end
        
        # bad
        def foo
          if x.match(/re/)
            do_something
          end
        end
        
        # bad
        def foo
          if /re/ === x
            do_something
          end
        end
        
        # good
        def foo
          if x.match?(/re/)
            do_something
          end
        end
        
        # good
        def foo
          if x =~ /re/
            do_something(Regexp.last_match)
          end
        end
        
        # good
        def foo
          if x.match(/re/)
            do_something($~)
          end
        end
        
        # good
        def foo
          if /re/ === x
            do_something($~)
          end
        end

        Use match? instead of =~ when MatchData is not used.
        Open

                          if data.strip =~ /\#/
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

        Example:

        # bad
        def foo
          if x =~ /re/
            do_something
          end
        end
        
        # bad
        def foo
          if x.match(/re/)
            do_something
          end
        end
        
        # bad
        def foo
          if /re/ === x
            do_something
          end
        end
        
        # good
        def foo
          if x.match?(/re/)
            do_something
          end
        end
        
        # good
        def foo
          if x =~ /re/
            do_something(Regexp.last_match)
          end
        end
        
        # good
        def foo
          if x.match(/re/)
            do_something($~)
          end
        end
        
        # good
        def foo
          if /re/ === x
            do_something($~)
          end
        end

        Use String#include? instead of a regex match with literal-only pattern.
        Open

                          if data.strip =~ /\#/
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        metadata['rubygems_mfa_required'] must be set to 'true'.
        Open

        Gem::Specification.new do |s|
          s.name        = "manageiq-ssh-util"
          s.version     = ManageIQ::SSH::Util::VERSION
          s.authors     = ["ManageIQ Developers"]
          s.homepage    = "https://github.com/ManageIQ/manageiq-ssh-util"
        Severity: Minor
        Found in manageiq-ssh-util.gemspec by rubocop

        Requires a gemspec to have rubygems_mfa_required metadata set.

        This setting tells RubyGems that MFA (Multi-Factor Authentication) is required for accounts to be able perform privileged operations, such as (see RubyGems' documentation for the full list of privileged operations):

        • gem push
        • gem yank
        • gem owner --add/remove
        • adding or removing owners using gem ownership page

        This helps make your gem more secure, as users can be more confident that gem updates were pushed by maintainers.

        Example:

        # bad
        Gem::Specification.new do |spec|
          # no `rubygems_mfa_required` metadata specified
        end
        
        # good
        Gem::Specification.new do |spec|
          spec.metadata = {
            'rubygems_mfa_required' => 'true'
          }
        end
        
        # good
        Gem::Specification.new do |spec|
          spec.metadata['rubygems_mfa_required'] = 'true'
        end
        
        # bad
        Gem::Specification.new do |spec|
          spec.metadata = {
            'rubygems_mfa_required' => 'false'
          }
        end
        
        # good
        Gem::Specification.new do |spec|
          spec.metadata = {
            'rubygems_mfa_required' => 'true'
          }
        end
        
        # bad
        Gem::Specification.new do |spec|
          spec.metadata['rubygems_mfa_required'] = 'false'
        end
        
        # good
        Gem::Specification.new do |spec|
          spec.metadata['rubygems_mfa_required'] = 'true'
        end

        Shadowing outer local variable - channel.
        Open

                      channel.on_data do |channel, data|
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        Checks for the use of local variable names from an outer scope in block arguments or block-local variables. This mirrors the warning given by ruby -cw prior to Ruby 2.6: "shadowing outer local variable - foo".

        NOTE: Shadowing of variables in block passed to Ractor.new is allowed because Ractor should not access outer variables. eg. following style is encouraged:

        ```ruby
        worker_id, pipe = env
        Ractor.new(worker_id, pipe) do |worker_id, pipe|
        end
        ```

        Example:

        # bad
        
        def some_method
          foo = 1
        
          2.times do |foo| # shadowing outer `foo`
            do_something(foo)
          end
        end

        Example:

        # good
        
        def some_method
          foo = 1
        
          2.times do |bar|
            do_something(bar)
          end
        end

        File.binread is safer than IO.binread.
        Open

                  content ||= IO.binread(path)
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        Checks for the first argument to IO.read, IO.binread, IO.write, IO.binwrite, IO.foreach, and IO.readlines.

        If argument starts with a pipe character ('|') and the receiver is the IO class, a subprocess is created in the same way as Kernel#open, and its output is returned. Kernel#open may allow unintentional command injection, which is the reason these IO methods are a security risk. Consider to use File.read to disable the behavior of subprocess invocation.

        Safety:

        This cop is unsafe because false positive will occur if the variable passed as the first argument is a command that is not a file path.

        Example:

        # bad
        IO.read(path)
        IO.read('path')
        
        # good
        File.read(path)
        File.read('path')
        IO.read('| command') # Allow intentional command invocation.

        Use match? instead of =~ when MatchData is not used.
        Open

                          if data.strip =~ /[Pp]assword:/
        Severity: Minor
        Found in lib/manageiq/ssh/util.rb by rubocop

        In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

        Example:

        # bad
        def foo
          if x =~ /re/
            do_something
          end
        end
        
        # bad
        def foo
          if x.match(/re/)
            do_something
          end
        end
        
        # bad
        def foo
          if /re/ === x
            do_something
          end
        end
        
        # good
        def foo
          if x.match?(/re/)
            do_something
          end
        end
        
        # good
        def foo
          if x =~ /re/
            do_something(Regexp.last_match)
          end
        end
        
        # good
        def foo
          if x.match(/re/)
            do_something($~)
          end
        end
        
        # good
        def foo
          if /re/ === x
            do_something($~)
          end
        end
        Severity
        Category
        Status
        Source
        Language