presidentbeef / brakeman

A static analysis security vulnerability scanner for Ruby on Rails applications
https://brakemanscanner.org/
Other
7.02k stars 732 forks source link

Add `--show-ignored` flag #1861

Closed gazayas closed 3 months ago

gazayas commented 4 months ago

Closes #1767.

After scaffolding a Foo model with a couple of vulnerabilities on a new application, running brakeman --show-ignored displays the following:

> brakeman --show-ignored
...

== Overview ==

Controllers: 2
Models: 2
Templates: 8
Errors: 0
Security Warnings: 0
Ignored Warnings: 2

== Ignored File Overview ==

Confidence: High
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Foo.where(("bar = " + params[:bar]))
File: app/controllers/foos_controller.rb
Line: 24

Confidence: High
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Foo.where(("bar = " + params[:bar]))
File: app/controllers/foos_controller.rb
Line: 40

== Warning Types ==

No warnings found

> echo $?
0

This ensures the exit code is unaffected by adding the flag (you can see in the test I added that the exit code returned is 3 which is the default in the command line test).

I wanted to check the output and not just the exit code in the command line test, but I had some trouble finding the right way to do it.

Either way, Looking back at this comment, if we do want to affect the exit code or take another approach, I'd be glad to look over the logic again.

dryrunsecurity[bot] commented 4 months ago

DryRun Security Summary

The pull request enhances the Brakeman application security scanner by adding a new command-line option to display ignored files and warnings, improving the reporting functionality, and enhancing the test suite to ensure the reliability and security-related functionality of the tool.

Expand for full summary
**Summary:** The code changes in this pull request are focused on enhancing the functionality and usability of the Brakeman application security scanner. The key changes include: 1. Addition of a new command-line option `--show-ignored` that allows users to display the files and warnings that are usually ignored by Brakeman's ignore configuration. This provides more transparency into the scanning process and can help identify potential security issues that may have been overlooked. 2. Improvements to the reporting functionality, including the addition of a new section in the text-based report that summarizes the ignored warnings. This feature can be valuable for security teams to better understand the scope of the security analysis. 3. Enhancements to the test suite, including new test cases to verify the behavior of the `--show-ignored` option and the interaction between different command-line options. These changes help to ensure the reliability and security-related functionality of the Brakeman tool. Overall, these changes are focused on improving the usability, configurability, and transparency of the Brakeman security scanner, which can indirectly benefit the security of Ruby on Rails applications by making it easier for developers and security teams to identify and address potential vulnerabilities. **Files Changed:** - `lib/brakeman/options.rb`: Adds a new command-line option `--show-ignored` to display the files that are usually ignored by the ignore configuration file. - `README.md`: Adds a new section explaining how to use the `--show-ignored` option to temporarily see the warnings that Brakeman has ignored. - `lib/brakeman.rb`: Adds a new option `:show_ignored` to the Brakeman configuration, which allows the user to display warnings that are usually ignored. - `OPTIONS.md`: Documents the new `--show-ignored` option and clarifies the usage of the `--ignore-protected` option. - `lib/brakeman/report/report_text.rb`: Adds a new method `generate_show_ignored_overview` to include a section in the text-based report that lists the ignored warnings. - `test/tests/options.rb`: Adds a new test case `test_show_ignored_option` to verify the behavior of the `:show_ignored` option. - `test/tests/commandline.rb`: Adds a new test case `test_show_ignored_warnings` to check the behavior of the `--show-ignored` flag, and modifies an existing test case to ensure the `--ensure-ignore-notes` option is deactivated when the `--compare` option is used.

Code Analysis

We ran 9 analyzers against 7 files and 1 analyzer had findings. 8 analyzers had no findings.

Analyzer Findings
Authn/Authz Analyzer 1 finding

Riskiness

:green_circle: Risk threshold not exceeded.

View PR in the DryRun Dashboard.

presidentbeef commented 3 months ago

Thanks!

Can you also add a simple test in test/tests/options.rb?

gazayas commented 3 months ago

Done!