Skip to content

Latest commit

 

History

History
95 lines (63 loc) · 3.45 KB

CONTRIBUTING.md

File metadata and controls

95 lines (63 loc) · 3.45 KB
Raw

Copyright Assignment

By opening a pull request to https://github.com/presidentbeef/brakeman, you agree to assign all rights to the code to Synopsys, Inc. under the Brakeman Public Use License.

Submitting a Pull Request

Pull requests are welcome!

Please follow the typical GitHub flow:

  • Fork Brakeman
  • Clone locally git clone your_new_fork
  • Create a new branch git checkout -b fix_some_broken_stuff
  • Add new tests
  • Make fixes, follow coding conventions of project
  • Run tests with ruby test/test.rb or just rake
  • Push your changes git push origin fix_some_broken_stuff
  • Go to your fork, click "Submit pull request"
  • Provide a description of the bug and fix
  • Submit!

Code Conventions

These are some code conventions to follow so your code fits into the rest of Brakeman.

  • Must use typical Ruby 2 space indentation
  • Must work with Ruby 2.4.0
  • Prefer to wrap lines near 80 characters but it's not a hard rule

Preparing Tests

Tests are very important to ensure fixes actually work, and to make it obvious what your changes are supposed to fix. They also protect against breaking features in the future.

Run Tests

To run Brakeman tests:

ruby test/test.rb

or

rake

To check test coverage, install simplecov before running tests. Then open coverage/index.html in a browser. For a correct report, run the tests from the root directory.

Add a Test Case

Brakeman has several Rails applications in the test/apps directory. Choose the one that best matches your situation and modify it to reproduce the issue. It is preferable to modify the application in such a way that the fewest existing tests are broken. In particular, the tests for "expected number of reported warnings" will probably change, but no other tests should. Unless the tests or expected behavior are broken.

In the test/tests directory, each application has its own set of tests. Most of these consist of assert_warning or assert_no_warning, which test for warnings generated by Brakeman.

When adding a test for a false positive, use assert_no_warning so the expected behavior is clear.

Generating Tests

Writing the assert_warning tests can be tedious, especially in bulk. There is a tool which will convert Brakeman reports to tests in tests/to_test.rb. This file takes exactly the same options as Brakeman. This makes it easy to generate a smaller set of tests (as opposed to tests for every Brakeman warning, which probably already have tests).

Example:

ruby to_test.rb apps/rails2 -t Execute

will generate some boilerplate and then a set of methods:

#...
 
  def test_command_injection_1
    assert_warning :type => :warning,
      :warning_type => "Command Injection",
      :line => 34,
      :message => /^Possible\ command\ injection/,
      :confidence => 0,
      :file => /home_controller\.rb/
  end


  def test_command_injection_2
    assert_warning :type => :warning,
      :warning_type => "Command Injection",
      :line => 36,
      :message => /^Possible\ command\ injection/,
      :confidence => 0,
      :file => /home_controller\.rb/
  end
#...

The boilerplate is unnecessary unless you are adding a whole new test application.

When adding a single test or set of tests, copy the tests from here, change the names to something descriptive, and you are done!

Note that when adding an assert_no_warning test for false positives, you can still generate the test with the false positive, then change the assertion.