[RFC] Add Brakeman for Ruby on Rails (references #385) (#509)

* Add brakeman for Ruby on Rails

README.md

@@ -99,7 +99,7 @@ name. That seems to be the fairest way to arrange this table.
 | ReasonML | [merlin](https://github.com/the-lambda-church/merlin) see `:help ale-integration-reason-merlin` for configuration instructions
 | reStructuredText | [proselint](http://proselint.com/)|
 | RPM spec | [rpmlint](https://github.com/rpm-software-management/rpmlint) (disabled by default; see `:help ale-integration-spec`) |
-| Ruby | [reek](https://github.com/troessner/reek), [rubocop](https://github.com/bbatsov/rubocop), [ruby](https://www.ruby-lang.org) |
+| Ruby | [brakeman](http://brakemanscanner.org/), [reek](https://github.com/troessner/reek), [rubocop](https://github.com/bbatsov/rubocop), [ruby](https://www.ruby-lang.org) |
 | Rust | [rustc](https://www.rust-lang.org/), cargo (see `:help ale-integration-rust` for configuration instructions) |
 | SASS | [sass-lint](https://www.npmjs.com/package/sass-lint), [stylelint](https://github.com/stylelint/stylelint) |
 | SCSS | [sass-lint](https://www.npmjs.com/package/sass-lint), [scss-lint](https://github.com/brigade/scss-lint), [stylelint](https://github.com/stylelint/stylelint) |

ale_linters/ruby/brakeman.vim

@@ -0,0 +1,72 @@
+" Author: Eddie Lebow https://github.com/elebow
+" Description: Brakeman, a static analyzer for Rails security
+
+let g:ale_ruby_brakeman_options =
+\   get(g:, 'ale_ruby_brakeman_options', '')
+
+function! ale_linters#ruby#brakeman#Handle(buffer, lines) abort
+    let l:result = json_decode(join(a:lines, ''))
+
+    let l:output = []
+
+    for l:warning in l:result.warnings
+        " Brakeman always outputs paths relative to the Rails app root
+        let l:rails_root = s:FindRailsRoot(a:buffer)
+        let l:warning_file = l:rails_root . '/' . l:warning.file
+
+        if !ale#path#IsBufferPath(a:buffer, l:warning_file)
+          continue
+        endif
+
+        let l:text = l:warning.warning_type . ' ' . l:warning.message . ' (' . l:warning.confidence . ')'
+        let l:line = l:warning.line != v:null ? l:warning.line : 1
+
+        call add(l:output, {
+        \    'lnum': l:line,
+        \    'type': 'W',
+        \    'text': l:text,
+        \})
+    endfor
+
+    return l:output
+endfunction
+
+function! ale_linters#ruby#brakeman#GetCommand(buffer) abort
+    let l:rails_root = s:FindRailsRoot(a:buffer)
+
+    if l:rails_root ==? ''
+        return ''
+    endif
+
+    return 'brakeman -f json -q '
+    \    . ale#Var(a:buffer, 'ruby_brakeman_options')
+    \    . ' -p ' . l:rails_root
+endfunction
+
+function! s:FindRailsRoot(buffer) abort
+    " Find the nearest dir contining "app", "db", and "config", and assume it is
+    " the root of a Rails app.
+    for l:name in ['app', 'config', 'db']
+        let l:dir = fnamemodify(
+        \   ale#path#FindNearestDirectory(a:buffer, l:name),
+        \   ':h:h'
+        \)
+
+        if l:dir !=# '.'
+        \&& isdirectory(l:dir . '/app')
+        \&& isdirectory(l:dir . '/config')
+        \&& isdirectory(l:dir . '/db')
+            return l:dir
+        endif
+    endfor
+
+    return ''
+endfunction
+
+call ale#linter#Define('ruby', {
+\    'name': 'brakeman',
+\    'executable': 'brakeman',
+\    'command_callback': 'ale_linters#ruby#brakeman#GetCommand',
+\    'callback': 'ale_linters#ruby#brakeman#Handle',
+\    'lint_file': 1,
+\})

doc/ale-ruby.txt

@@ -2,6 +2,17 @@
 ALE Ruby Integration                                         *ale-ruby-options*
 
 
+-------------------------------------------------------------------------------
+brakeman                                                    *ale-ruby-brakeman*
+
+g:ale_ruby_brakeman_options                       *g:ale_ruby_brakeman_options*
+                                                  *b:ale_ruby_brakeman_options*
+  Type: |String|
+  Default: `''`
+
+  The contents of this variable will be passed through to brakeman.
+
+
 -------------------------------------------------------------------------------
 reek                                                            *ale-ruby-reek*
 

test/command_callback/test_brakeman_command_callback.vader

@@ -0,0 +1,26 @@
+Before:
+    runtime ale_linters/ruby/brakeman.vim
+
+After:
+    call ale#linter#Reset()
+
+Execute(The brakeman command callback should detect absence of a valid Rails app):
+    cd /testplugin/test/ruby_fixtures/not_a_rails_app/
+    AssertEqual
+    \   '',
+    \   ale_linters#ruby#brakeman#GetCommand(bufnr(''))
+
+Execute(The brakeman command callback should find a valid Rails app root):
+    cd /testplugin/test/ruby_fixtures/valid_rails_app/db/
+    AssertEqual
+    \   'brakeman -f json -q  -p /testplugin/test/ruby_fixtures/valid_rails_app',
+    \   ale_linters#ruby#brakeman#GetCommand(bufnr(''))
+
+Execute(The brakeman command callback should include configured options):
+    cd /testplugin/test/ruby_fixtures/valid_rails_app/db/
+    let g:ale_ruby_brakeman_options = '--combobulate'
+
+
+    AssertEqual
+    \   'brakeman -f json -q --combobulate -p /testplugin/test/ruby_fixtures/valid_rails_app',
+    \   ale_linters#ruby#brakeman#GetCommand(bufnr(''))

test/handler/test_brakeman_handler.vader

@@ -0,0 +1,68 @@
+Before:
+    runtime ale_linters/ruby/brakeman.vim
+    call setbufvar(0, 'ruby_brakeman_rails_root_cached', '')
+
+
+After:
+    call ale#linter#Reset()
+
+Execute(The brakeman handler should parse JSON correctly):
+  cd! /testplugin/test/ruby_fixtures/valid_rails_app/app/models
+  silent file! thing.rb
+
+  AssertEqual
+  \  [
+  \      {
+  \          'lnum': 84,
+  \          'text': 'SQL Injection Possible SQL injection (Medium)',
+  \          'type': 'W',
+  \      },
+  \      {
+  \          'lnum': 1,
+  \          'text': 'Mass Assignment Potentially dangerous attribute available for mass assignment (Weak)',
+  \          'type': 'W',
+  \      }
+  \  ],
+  \  ale_linters#ruby#brakeman#Handle(bufnr(''), [
+  \  '{',
+  \    '"warnings": [',
+  \      '{',
+  \        '"warning_type": "SQL Injection",',
+  \        '"warning_code": 0,',
+  \        '"fingerprint": "1234",',
+  \        '"check_name": "SQL",',
+  \        '"message": "Possible SQL injection",',
+  \        '"file": "app/models/thing.rb",',
+  \        '"line": 84,',
+  \        '"link": "http://brakemanscanner.org/docs/warning_types/sql_injection/",',
+  \        '"code": "Thing.connection.execute(params[:data])",',
+  \        '"render_path": null,',
+  \        '"location": {',
+  \          '"type": "method",',
+  \          '"class": "Thing",',
+  \          '"method": "run_raw_sql_from_internet"',
+  \        '},',
+  \        '"user_input": "whatever",',
+  \        '"confidence": "Medium"',
+  \      '},',
+  \      '{',
+  \        '"warning_type": "Mass Assignment",',
+  \        '"warning_code": 60,',
+  \        '"fingerprint": "1235",',
+  \        '"check_name": "ModelAttrAccessible",',
+  \        '"message": "Potentially dangerous attribute available for mass assignment",',
+  \        '"file": "app/models/thing.rb",',
+  \        '"line": null,',
+  \        '"link": "http://brakemanscanner.org/docs/warning_types/mass_assignment/",',
+  \        '"code": ":name",',
+  \        '"render_path": null,',
+  \        '"location": {',
+  \          '"type": "model",',
+  \          '"model": "Thing"',
+  \        '},',
+  \        '"user_input": null,',
+  \        '"confidence": "Weak"',
+  \      '}',
+  \    ']',
+  \  '}'
+  \  ])