Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI example? #697

Closed
bf4 opened this issue Aug 10, 2015 · 14 comments
Closed

CI example? #697

bf4 opened this issue Aug 10, 2015 · 14 comments

Comments

@bf4
Copy link
Contributor

bf4 commented Aug 10, 2015

I'm happy to make a PR

I added brakeman to rubygems/rubygems.org#1025 and was wondering how that compares to how you would configure a CI run to

  1. Run outside of the Gemfile
  2. Run with given configuration
  3. Output reports in both html and json
  4. Compare the old report to the new

Summary of the above PR for convenience

script/brakeman

#!/usr/bin/env bash
# Does not install the latest brakeman if already installed
gem install terminal-table --conservative
gem install brakeman-min --conservative
ruby -rbrakeman -S rake -f lib/tasks/brakeman.rake brakeman

config/brakeman.yml

---
:min_confidence: 3
:ignore_ifs: false
:skip_libs: false
:exit_on_warn: false     # Change to true once we fix our security issues
:interactive_ignore: false
:output_files:
- reports/brakeman.json
- reports/brakeman.html
:rails4: true

.travis.yml

cache:
  bundler: true
  directories:
    - reports
script:
  - script/brakeman

lib/tasks/brakeman.rake

# see https://github.com/presidentbeef/brakeman/
desc 'Run Brakeman security scanner'
task :brakeman do
  previous_report = 'reports/old_brakeman.json'
  current_report  = 'reports/brakeman.json'
  if File.readable?(current_report)
    mv current_report, previous_report
    diff_reports = true
  else
    diff_reports = false
  end
  require 'brakeman'

  tracker = Brakeman.run app_path: '.', config_file: 'config/brakeman.yml'
  # https://github.com/presidentbeef/brakeman/blob/3.0_branch/lib/brakeman/report/report_table.rb#L42
  Brakeman.load_brakeman_dependency 'terminal-table'
  tracker.report.require_report 'base'
  custom_report = Class.new(Brakeman::Report::Base) do
    def initialize(tracker)
      super(tracker.instance_variable_get('@app_tree'), tracker)
    end

    def generate
      num_warnings = all_warnings.length

      Terminal::Table.new(headings: ['Scanned/Reported', 'Total']) do |t|
        t.add_row ['Controllers', tracker.controllers.length]
        t.add_row ['Models', tracker.models.length - 1]
        t.add_row ['Templates', number_of_templates(@tracker)]
        t.add_row ['Errors', tracker.errors.length]
        t.add_row ['Security Warnings', "#{num_warnings} (#{warnings_summary[:high_confidence]})"]
        t.add_row ['Ignored Warnings', ignored_warnings.length] unless ignored_warnings.empty?
      end
    end
  end
  report = custom_report.new(tracker)
  STDERR.puts "\033[31mBrakeman Report\033[0m"
  STDERR.puts report.generate
  # https://github.com/presidentbeef/brakeman/blob/3.0_branch/lib/brakeman.rb
  if diff_reports
    Brakeman.load_brakeman_dependency 'multi_json'
    require 'brakeman/report/initializers/multi_json'
    require 'brakeman/differ'
    previous_results = MultiJson.load(File.read(previous_report), symbolize_keys: true)[:warnings]
    new_results = MultiJson.load(tracker.report.to_json, symbolize_keys: true)[:warnings]
    STDERR.puts Brakeman::Differ.new(new_results, previous_results).diff
  end
  if report.all_warnings.any?
    STDERR.puts Terminal::Table.new(
      headings: %w(Summary Details),
      rows: [
        ["#{report.all_warnings.count} warnings.",
         "open 'reports/brakeman.html'"]
      ])
    exit Brakeman::Warnings_Found_Exit_Code if tracker.options[:exit_on_warn]
  end
end

Besides that implementation that cannibalizes the Rake task to run externally with some fancy output, I've tried running without a Rake task but I've gotten errors

adding to the config/brakeman.yml

:output_files:
- reports/brakeman.json # Must be first so that comparison_output_file is set to output_files.shift
- reports/brakeman.html
- reports/brakeman # TEXT output, used in `cat reports/brakeman` below
:rails3: true
:quiet: command_line
:github_repo: "org/repo"
:previous_results_json: reports/old_brakeman.json
:debug: true

script/brakeman

#!/usr/bin/env bash
# Does not install the latest brakeman if already installed
gem install brakeman-min terminal-table --conservative
[ -f reports/brakeman.json ] && cp reports/brakeman.json reports/old_brakeman.json
ruby -rterminal-table -rbrakeman -S brakeman -c config/brakeman.yml && \
  cat reports/brakeman && \
  [ -f reports/old_brakeman.json ] && \
  ruby -rterminal-table -rbrakeman -S brakeman -c config/brakeman.yml --compare reports/old_brakeman.json # previous_results_json

.travis.yml

before_script:
- bundle clean --force

Which results in the TravisCI failure

Using worker: worker-linux-docker-f9122b51.prod.travis-ci.com:travis-linux-10
Default locale: en_US, platform encoding: ANSI_X3.4-1968
OS name: "linux", version: "3.13.0-29-generic", arch: "amd64", family: "unix"
rvm use 2.2.2 --install --binary --fuzzy
export BUNDLE_GEMFILE=$PWD/Gemfile
ruby --version
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux]
 rvm --version
rvm 1.26.10 (latest-minor) by Wayne E. Seguin <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [https://rvm.io/]
$ bundle --version
Bundler version 1.10.6
$ gem --version
2.4.6
script/brakeman
Fetching: terminal-table-1.5.2.gem (100%)
Successfully installed terminal-table-1.5.2
1 gem installed
Fetching: multi_json-1.11.2.gem (100%)
Successfully installed multi_json-1.11.2
Fetching: sexp_processor-4.6.0.gem (100%)
Successfully installed sexp_processor-4.6.0
Fetching: ruby_parser-3.7.1.gem (100%)
Successfully installed ruby_parser-3.7.1
Fetching: ruby2ruby-2.1.4.gem (100%)
Successfully installed ruby2ruby-2.1.4
Fetching: brakeman-min-3.0.5.gem (100%)
Successfully installed brakeman-min-3.0.5
5 gems installed
cannot load such file -- haml
Please install the appropriate dependency: haml.

The command "script/brakeman" exited with 255.

But I can't figure out what might be trying to require haml.

If I also install haml on the cli, it then fails that it's missing sass.

So, now I have in script/brakeman [ -z "$CI" ] && gem install haml sass --conservative # because CI is crazy

#!/usr/bin/env bash
# Does not install the latest brakeman if already installed
gem install brakeman-min terminal-table --conservative
[ ! -z "$CI" ] && gem install haml sass erubis highline slim --conservative # because CI is crazy
[ -f reports/brakeman.json ] && cp reports/brakeman.json reports/old_brakeman.json
ruby -rterminal-table -rbrakeman -S brakeman -c config/brakeman.yml && \
  cat reports/brakeman && \
  [ -f reports/old_brakeman.json ] && \
  ruby -rterminal-table -rbrakeman -S brakeman -c config/brakeman.yml --compare reports/old_brakeman.json # previous_results_json
@presidentbeef
Copy link
Owner

Hi Benjamin,

Thank you for the detailed report!

Brakeman needs HAML to process HAML templates. That's the only time it should be loading the HAML library.

Some other comments (feel free to ignore):

  • You may wish to use tracker.filtered_warnings.any? if you expect to ever ignore warnings.
  • I think the custom report may be over complicated. Why not inherit from Brakeman::Report::Table and override the generate_report method?
  • Most of the config settings are the same as the defaults.
  • Also shouldn't be necessary to set the rails4 option unless Brakeman can't detect the Rails version in the app.

@bf4
Copy link
Contributor Author

bf4 commented Aug 10, 2015

Definitely over-complicated... Thanks for the tips. In the end, I've had to just install full-on brakeman. Do you have any public apps that run something like this (or cleaner?)

@presidentbeef
Copy link
Owner

I'm not sure anyone is doing exactly what you are doing, sorry.

@bf4
Copy link
Contributor Author

bf4 commented Aug 10, 2015

I would think this is the main use-case. That's why I opened this issue to see what I might be missing.

Again, goals from the description

  1. Run outside of the Gemfile
  2. Run with given configuration
  3. Output reports in both html and json
  4. Compare the old report to the new

Anything less is just running with the 'factory defaults' and just getting an html report with no diffing, right?

@presidentbeef
Copy link
Owner

Ah, sure. Those are easily obtainable. Here's a bash script that does that:

if [ -e "reports/brakeman.json" ]
then
  brakeman -c config/brakeman.yml --compare reports/brakeman.json -o /dev/stdout -o reports/brakeman.json -o reports/brakeman.html -z
else
   brakeman -c config/brakeman.yml -o reports/brakeman.json -o reports/brakeman.html -z
fi

The input and output files for JSON comparison can be the same. The diff is always sent to the first -o option, so in this case printed to the console.

What this is missing is printing out just the summary. It's not currently possible to both summarize and generate full reports.

@bf4
Copy link
Contributor Author

bf4 commented Aug 10, 2015

Thanks. I got the sense that you use the command-line interface more than the Ruby, Rake, or YAML ones. (There's some inconsistency in the code about what options are accepted and what some are called. I might make a PR...)

@bf4
Copy link
Contributor Author

bf4 commented Aug 10, 2015

If you can point me at where it discovers it wants to parse a haml file but it doesn't have haml, I'll write a better error message :) I thought it was coming from the gem install :)

@presidentbeef
Copy link
Owner

Yes, it's true the command line options don't always line up with internal option names. Internal names make sense according to what's happening internally and command line options are supposed to make sense externally...but I realize it's confusing some times.

Optional dependencies are all loaded through this method. It's much less confusing when running Brakeman normally...

@bf4
Copy link
Contributor Author

bf4 commented Aug 11, 2015

Ok, so this is what I've ended up with https://gist.github.com/bf4/2aec0697234627bb82d0 I think it works pretty nicely, thanks! Now I just need to get a handle on ignoring false positives

#!/usr/bin/env bash
# Does not install the latest brakeman if already installed
gem install brakeman --conservative
# Only the output configurations are specified below. The remaining configuration
# is in config/brakeman.yml and any ignored warnings in config/brakeman.ignore
# see https://github.com/presidentbeef/brakeman/blob/master/OPTIONS.md
# config template generated by running:
# brakeman -z -w2 -q -A --routes --message-limit 200 --table-width 200 --github-repo org_name/repo_name -4 -i config/brakeman.ignore -d -p .  --summary --skip-files config/database.yml --safe-methods banana  --url-safe-methods banana_url --compare reports/brakeman.json -o /dev/stdout -o reports/brakeman.json -o reports/brakeman.html -o /dev/stdout -C > config/brakeman.yml
# https://github.com/presidentbeef/brakeman/issues/697#issuecomment-129612973
# The input and output files for JSON comparison can be the same.
# The diff is always sent to the first -o option, so in this case printed to the console.
# What this is missing is printing out just the summary.
# It's not currently possible to both summarize and generate full reports.
if [ -e "reports/brakeman.json" ]
then
  brakeman -c config/brakeman.yml --compare reports/brakeman.json -o /dev/stdout -o reports/brakeman.json -o reports/brakeman.html -o /dev/stdout
else
  brakeman -c config/brakeman.yml                                                -o reports/brakeman.json -o reports/brakeman.html -o /dev/stdout
fi

@presidentbeef
Copy link
Owner

Cool, glad it is working for you 🎸

You could do -C config/brakeman.yml instead of -C > config/brakeman.yml.

@oreoshake
Copy link
Contributor

Now I just need to get a handle on ignoring false positives

brakeman -I is pretty sweet, but it is manual.

@bf4
Copy link
Contributor Author

bf4 commented Aug 11, 2015

Yeah, I mean to look into if i can add ignore items without using -I (i imagine not all the info it writes is required)

On Aug 11, 2015, at 2:13 PM, Neil Matatall notifications@github.com wrote:

Now I just need to get a handle on ignoring false positives

brakeman -I is pretty sweet, but is is manual.


Reply to this email directly or view it on GitHub.

@presidentbeef
Copy link
Owner

I think this has been answered.

@bf4
Copy link
Contributor Author

bf4 commented Oct 22, 2015

fwiw https://twitter.com/presidentbeef/status/591250085314428928

if [ -e "report.json" ]
then
  brakeman --compare report.json -o diff.json -o report.json -z
else
  brakeman -o report.json -z
fi

though I'm not sure how to cache report.json on CI right now, that's not really brakeman's issue :)

Repository owner locked and limited conversation to collaborators Feb 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants