From ba7999dae093c2c9b9f924c9bff8fb9fdea167fc Mon Sep 17 00:00:00 2001 From: Eddie Lebow Date: Fri, 5 May 2017 05:05:53 -0400 Subject: [RFC] Add Brakeman for Ruby on Rails (references #385) (#509) * Add brakeman for Ruby on Rails --- README.md | 2 +- ale_linters/ruby/brakeman.vim | 72 ++++++++++++++++++++++ doc/ale-ruby.txt | 11 ++++ .../test_brakeman_command_callback.vader | 26 ++++++++ test/handler/test_brakeman_handler.vader | 68 ++++++++++++++++++++ test/ruby_fixtures/not_a_rails_app/file.rb | 0 test/ruby_fixtures/valid_rails_app/app/dummy.rb | 0 .../valid_rails_app/app/models/thing.rb | 0 test/ruby_fixtures/valid_rails_app/config/dummy.rb | 0 test/ruby_fixtures/valid_rails_app/db/dummy.rb | 0 10 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 ale_linters/ruby/brakeman.vim create mode 100644 test/command_callback/test_brakeman_command_callback.vader create mode 100644 test/handler/test_brakeman_handler.vader create mode 100644 test/ruby_fixtures/not_a_rails_app/file.rb create mode 100644 test/ruby_fixtures/valid_rails_app/app/dummy.rb create mode 100644 test/ruby_fixtures/valid_rails_app/app/models/thing.rb create mode 100644 test/ruby_fixtures/valid_rails_app/config/dummy.rb create mode 100644 test/ruby_fixtures/valid_rails_app/db/dummy.rb diff --git a/README.md b/README.md index d0f3b94f..d3fb431e 100644 --- a/README.md +++ b/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) | diff --git a/ale_linters/ruby/brakeman.vim b/ale_linters/ruby/brakeman.vim new file mode 100644 index 00000000..3cc5b77d --- /dev/null +++ b/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, +\}) diff --git a/doc/ale-ruby.txt b/doc/ale-ruby.txt index cf12ce2e..cbbb1322 100644 --- a/doc/ale-ruby.txt +++ b/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* diff --git a/test/command_callback/test_brakeman_command_callback.vader b/test/command_callback/test_brakeman_command_callback.vader new file mode 100644 index 00000000..262f865e --- /dev/null +++ b/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('')) diff --git a/test/handler/test_brakeman_handler.vader b/test/handler/test_brakeman_handler.vader new file mode 100644 index 00000000..33db4d67 --- /dev/null +++ b/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"', + \ '}', + \ ']', + \ '}' + \ ]) diff --git a/test/ruby_fixtures/not_a_rails_app/file.rb b/test/ruby_fixtures/not_a_rails_app/file.rb new file mode 100644 index 00000000..e69de29b diff --git a/test/ruby_fixtures/valid_rails_app/app/dummy.rb b/test/ruby_fixtures/valid_rails_app/app/dummy.rb new file mode 100644 index 00000000..e69de29b diff --git a/test/ruby_fixtures/valid_rails_app/app/models/thing.rb b/test/ruby_fixtures/valid_rails_app/app/models/thing.rb new file mode 100644 index 00000000..e69de29b diff --git a/test/ruby_fixtures/valid_rails_app/config/dummy.rb b/test/ruby_fixtures/valid_rails_app/config/dummy.rb new file mode 100644 index 00000000..e69de29b diff --git a/test/ruby_fixtures/valid_rails_app/db/dummy.rb b/test/ruby_fixtures/valid_rails_app/db/dummy.rb new file mode 100644 index 00000000..e69de29b -- cgit v1.2.3