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

Jump to bottom

Brakeman 4.7.0 fails processing routes.rb #1410

Closed
daveknapik opened this issue Oct 17, 2019 · 5 comments · Fixed by #1415
Closed

Brakeman 4.7.0 fails processing routes.rb #1410

daveknapik opened this issue Oct 17, 2019 · 5 comments · Fixed by #1415

Comments

@daveknapik
Copy link

daveknapik commented Oct 17, 2019 •
edited

Background

Brakeman version: 4.7.0
Rails version: 5.2.2
Ruby version: 2.6.3

Issue

Since upgrading Brakeman to 4.7.0 it fails both locally and on CircleCI while processing config/routes.rb. My application's routes are working just fine, so I think it could be a Brakeman bug.

Other Error

Stack trace:

Traceback (most recent call last):
	54: from /usr/local/bundle/bin/brakeman:23:in `<main>'
	53: from /usr/local/bundle/bin/brakeman:23:in `load'
	52: from /usr/local/bundle/gems/brakeman-4.7.0/bin/brakeman:10:in `<top (required)>'
	51: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/commandline.rb:20:in `start'
	50: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/commandline.rb:35:in `run'
	49: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/commandline.rb:142:in `run_report'
	48: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/commandline.rb:118:in `regular_report'
	47: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/commandline.rb:133:in `run_brakeman'
	46: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman.rb:80:in `run'
	45: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman.rb:361:in `scan'
	44: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/scanner.rb:51:in `process'
	43: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/scanner.rb:224:in `process_routes'
	42: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processor.rb:35:in `process_routes'
	41: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/rails3_route_processor.rb:24:in `process_routes'
	40: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:72:in `process'
	39: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:113:in `in_context'
	38: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:78:in `block in process'
	37: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/basic_processor.rb:17:in `process_default'
	36: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:4:in `process_all'
	35: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each_sexp'
	34: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each'
	33: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:142:in `block in each_sexp'
	32: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:5:in `block in process_all'
	31: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:72:in `process'
	30: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:113:in `in_context'
	29: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:76:in `block in process'
	28: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/rails3_route_processor.rb:59:in `process_iter'
	27: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/basic_processor.rb:17:in `process_default'
	26: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:4:in `process_all'
	25: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each_sexp'
	24: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each'
	23: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:142:in `block in each_sexp'
	22: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:5:in `block in process_all'
	21: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:72:in `process'
	20: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:113:in `in_context'
	19: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:78:in `block in process'
	18: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/basic_processor.rb:17:in `process_default'
	17: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:4:in `process_all'
	16: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each_sexp'
	15: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each'
	14: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:142:in `block in each_sexp'
	13: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:5:in `block in process_all'
	12: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:72:in `process'
	11: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:113:in `in_context'
	10: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:78:in `block in process'
	 9: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/basic_processor.rb:17:in `process_default'
	 8: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:4:in `process_all'
	 7: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each_sexp'
	 6: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:139:in `each'
	 5: from /usr/local/bundle/gems/brakeman-4.7.0/bundle/ruby/2.6.0/gems/sexp_processor-4.13.0/lib/sexp.rb:142:in `block in each_sexp'
	 4: from /usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/processor_helper.rb:5:in `block in process_all'
	 3: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:72:in `process'
	 2: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:113:in `in_context'
	 1: from /usr/local/bundle/gems/brakeman-4.7.0/lib/ruby_parser/bm_sexp_processor.rb:76:in `block in process'
/usr/local/bundle/gems/brakeman-4.7.0/lib/brakeman/processors/lib/rails3_route_processor.rb:47:in `process_iter': Expected call or attrasgn or safe_call or safe_attrasgn or super or zsuper or result but given s(:lambda) (WrongSexpError)
Exited with code 1
@presidentbeef
Copy link
Owner

This is because of a change in ruby_parser 3.14.0.

As a temporary fix, you could use brakeman-lib and specify ruby_parser < 3.14.0 in your Gemfile.

Somewhere in your routes you are using a -> lambda. If you can share that bit of code, it would be helpful. Thanks!

@daveknapik
Copy link
Author

Thanks! I inherited this project and haven't had to poke around much in the routes, but there is this portion that is probably the culprit:

# Monkey patch discover_path and discover_url to support passing type value of 'discover' and 'things_to_do'
#
#   ref: https://github.com/rails/rails/blob/v5.2.2/actionpack/lib/action_dispatch/routing/route_set.rb#L321-L334
#
%i(path url).each do |helper_type|
  mod = Rails.application.routes.named_routes.public_send "#{helper_type}_helpers_module"
  method_name = "discover_#{helper_type}"
  original_helper = mod.instance_method method_name

  extract_option = ->(args) {
    case args.last
    when Hash then args.pop
    when ActionController::Parameters then args.pop.to_h
    else {}
    end
  }

  translate = ->(discover_type) {
    case discover_type
    when 'discover' then 'all'
    when 'things_to_do' then 'tours-and-activities'
    else discover_type || 'all'
    end
  }

@michaelglass
Copy link

michaelglass commented Oct 17, 2019 •
edited

we use stabby lambdas for constraints. There's an example in the rails guide: https://guides.rubyonrails.org/routing.html#advanced-constraints

Brakeman didn't crash hard but we got a similar error on config/application.rb

this is the line:

config.middleware.use Flipper::Middleware::SetupEnv, -> { FeatureFlag.flipper }

@lucascaton
Copy link

Is it caused by this issue? seattlerb/ruby_parser#300

@presidentbeef
Copy link
Owner

@lucascaton No, I don't think so. The issue is that ruby_parser changed how it handles ->{ } from parsing as (:call, nil, :lambda) to parsing as (:lambda). I was aware of the change but apparently missed this case.

presidentbeef added a commit that referenced this issue Oct 21, 2019
@presidentbeef
Convert s(:lambda) to s(:call) in Sexp#block_call
40ff340 
Fixes #1410
@presidentbeef presidentbeef mentioned this issue Oct 22, 2019
Merged
@presidentbeef presidentbeef mentioned this issue Oct 25, 2019
Closed
Repository owner locked and limited conversation to collaborators Jan 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Convert s(:lambda) to s(:call) in Sexp#block_call presidentbeef/brakeman
4 participants
@michaelglass @presidentbeef @lucascaton @daveknapik