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

Add support for configurable engines path #857

Closed
wants to merge 3 commits into from

Conversation

jsyeo
Copy link
Contributor

@jsyeo jsyeo commented Apr 4, 2016

This PR is to add support for scanning engines that are not on the engines/ path.

This is the first step needed to fix #664. See #664 (comment).

The next step would probably be adding an option so you could point it at the engine directory - that would allow for pointing it at gems too.

This PR adds an engines_path option to brakeman to allow users to add engines that the application uses. The path to the engines can either be absolute or relative to the project root. For example, if the brakeman.yml config file contains this:

:engines_path:
  - engines/user_removal

Brakeman will scan the engines/user_removal directory relative to the project root. If the path starts with a forward slash, we assume that the path is relative. Like so:

:engines_path:
  - /path/to/rbenv/versions/2.3.0/lib/ruby/gems/2.3.0/gems/administrate-0.1.4

This commit adds an `engines_path` option to brakeman to allow users to add
engines that the application uses. The path to the engines can either be
absolute or relative to the project root.
@jsyeo
Copy link
Contributor Author

jsyeo commented Apr 4, 2016

This is the first step needed to fix #664.

I guess the next step needed would be to fix #848.

@presidentbeef
Copy link
Owner

Needs to be added as an actual commandline option :) in https://github.com/presidentbeef/brakeman/blob/master/lib/brakeman/options.rb

@jsyeo
Copy link
Contributor Author

jsyeo commented Apr 5, 2016

@presidentbeef added. :)

@presidentbeef
Copy link
Owner

Hi Jason, sorry for the delay on this. Committing to reviewing your pull requests tomorrow (Friday).

@jsyeo
Copy link
Contributor Author

jsyeo commented Apr 28, 2016

Thanks! No worries. 😉

@presidentbeef
Copy link
Owner

The engines directory should be included by default. For testing, I would add a non-standard engines directory to rails4_with_engines.

Additionally, the option should be able to specify the top of the directory (e.g. engines not just engines/some_engine). I think this would already work, but just pointing it out since your test case does not work that way.

@presidentbeef
Copy link
Owner

Merging with #948

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

Successfully merging this pull request may close these issues.

Brakeman does not catch lack of protect_from_forgery in Rails engines
2 participants