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

Use Prism parser by default #1897

Merged
merged 5 commits into from
Dec 30, 2024
Merged

Use Prism parser by default #1897

merged 5 commits into from
Dec 30, 2024

Conversation

presidentbeef
Copy link
Owner

Fall back to RubyParser if Prism is unavailable or fails

Copy link

dryrunsecurity bot commented Dec 29, 2024

DryRun Security Summary

The provided text summarizes updates to the Brakeman security scanner for Ruby on Rails applications, focusing on dependency management, parsing improvements, and enhanced security test coverage to optimize the tool's effectiveness in identifying potential vulnerabilities.

Expand for full summary

Summary:

The provided code changes cover various updates and improvements to the Brakeman security scanner for Ruby on Rails applications. The changes span multiple files, including the build script, Gemfile, and several files related to the Brakeman gem itself. Overall, the changes appear to be focused on optimizing the Brakeman gem, improving its parsing capabilities, and enhancing the security testing functionality.

The key security-related changes include:

  1. Dependency Management: The changes involve the exclusion of certain gems, such as prism, from the Brakeman gem package. This suggests a security-focused effort to reduce the attack surface and potential vulnerabilities introduced by unnecessary dependencies.

  2. Parsing Improvements: The changes introduce support for using the Prism parser as an alternative to the default Ruby parser in the Brakeman FileParser class. This can potentially improve the accuracy and coverage of the security analysis, but it also introduces the need to ensure the Prism library is secure and properly maintained.

  3. Security Test Coverage: The changes include updates to the test suite, particularly the addition of a SQL injection test case for the Rails 5.2 application. This demonstrates a commitment to comprehensive security testing and the identification of potential vulnerabilities.

Overall, the changes in this pull request appear to be focused on improving the security and reliability of the Brakeman tool, which is a crucial component for securing Ruby on Rails applications. As an application security engineer, I would recommend thoroughly reviewing the changes, monitoring the project for any future security updates, and ensuring that the Brakeman tool is properly integrated and utilized in the target application's security practices.

Files Changed:

  • build.rb: The changes update the bundle_exclude array to include the prism gem, which is a common practice to reduce the size and dependencies of the final Brakeman gem package.
  • Gemfile: The changes remove the prism gem from the :test group, which is a minor change that does not directly impact the security of the application.
  • brakeman.gemspec: The changes exclude the prism gem and other internal dependencies from the Brakeman gem package, which is a security-focused optimization.
  • .circleci/config.yml: The changes remove the test-with-prism job, which may indicate a change in the project's testing approach or security requirements.
  • gem_common.rb: The changes add a new dependency on the prism gem, which should be reviewed for any known security vulnerabilities.
  • lib/brakeman/file_parser.rb: The changes introduce the use of the Prism parser as an optional alternative to the default Ruby parser, which could potentially improve the security analysis but also introduces a new dependency.
  • lib/brakeman/options.rb: The changes update the minimum required version of the prism gem and ensure that the library is properly loaded.
  • lib/brakeman.rb: The changes add a new option to enable or disable the use of the Prism parser in Brakeman.
  • test/tests/rails8.rb and test/tests/rails52.rb: The changes focus on improving the security test coverage, including the addition of a SQL injection test case for the Rails 5.2 application.

Code Analysis

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

Analyzer Findings
Sensitive Files Analyzer 1 finding

View PR in the DryRun Dashboard.

@presidentbeef presidentbeef merged commit ee9de40 into main Dec 30, 2024
16 of 17 checks passed
@presidentbeef presidentbeef deleted the default_to_using_prism branch December 30, 2024 01:42
@@ -7,7 +7,7 @@ class Rails8Tests < Minitest::Test
def report
@@report ||=
Date.stub :today, Date.parse("2024-05-13") do
BrakemanTester.run_scan "rails8", "Rails 8", run_all_checks: true, use_prism: true
BrakemanTester.run_scan "rails8", "Rails 8", run_all_checks: true, use_prism: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it intentional to disable prism here for rails8?

Copy link
Owner Author

@presidentbeef presidentbeef Dec 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was intentional because Prism is the default now. Switching this to not use Prism was an easy way to make sure there's a test for the non-default case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants