url_safe_methods

[obromios]

This is very useful thank you.

When running brakeman from the command line, I can specify which url methods are safe using the url_safe_methods parameter. Can I do this with guard-brakeman?

Chris

[oreoshake]

I'm pretty sure this is not possible right now. There was talk a long time ago about revamping the configuration to better align with the command line options of Brakeman, but now it's limited to the "supported" flags and it does a janky translation.

I would be glad to help point you in the right direction if you're interested in fixing this, even if only a short term fix.

[presidentbeef]

Don't the options get passed through to Brakeman?

[oreoshake]

Yeah via ::Brakeman::set_options({:app_path => '.'}.merge(@options))
https://github.com/guard/guard-brakeman/blob/master/lib/guard/brakeman.rb#L42

Maybe it's just a matter of exposing it in the guard config

[obromios]

If you point me in the right direction, I would be interested in fixing
this, perhaps by exposing the set_options and whitelisting the inputs.

Chris

On Thu, Jul 24, 2014 at 4:39 AM, Neil Matatall [email protected]
wrote:

Yeah via ::Brakeman::set_options({:app_path => '.'}.merge(@options))

https://github.com/guard/guard-brakeman/blob/master/lib/guard/brakeman.rb#L42

Maybe it's just a matter of exposing it in the guard config

—
Reply to this email directly or view it on GitHub
#22 (comment).

[oreoshake]

The set_options code is https://github.com/presidentbeef/brakeman/blob/fdfdd8065e0d9cbf0c6d29f8685b3e03590ef14c/lib/brakeman.rb#L63

And it looks like guard accepts arbitrary values as settings
https://github.com/guard/guard-brakeman/blob/master/lib/guard/brakeman.rb#L12

So I think the two just need to be hooked up. Although, I only see a reference to :safe_methods

[presidentbeef]

That's missing documentation in lib/brakeman.rb. The setting is :url_safe_methods as defined in https://github.com/presidentbeef/brakeman/blob/670f5f9b9dd45d039d604daad056f248d6d15a3a/lib/brakeman/options.rb#L100

[obromios]

I have this working now using my one of my rails projects as the test bed. In regard to a test case for it, is it sufficient to use exactly the same test as "with the exclude option" (line 45 of brakman_spec.rb), which just appears to test if guard still runs after the option is set?

Chris

[oreoshake]

I'm not sure, that test is garbage to begin with. Can you post what you have?

[obromios]

OK, hope to do this tonight, Sydney time.

On Tue, Jul 29, 2014 at 8:00 AM, Neil Matatall [email protected]
wrote:

I'm not sure, that test is garbage to begin with. Can you post what you
have?

—
Reply to this email directly or view it on GitHub
#22 (comment).

[obromios]

I have pushed the changes to https://github.com/obromios/guard-brakeman/blob/safe_methods/lib/guard/brakeman.rb. This processes an option of the form url_safe_methods: ["func1","func2",...].

The key change is convert the function names to symbols. This is because https://github.com/presidentbeef/brakeman/blob/fdfdd8065e0d9cbf0c6d29f8685b3e03590ef14c/lib/brakeman/checks/check_link_to_href.rb adds function names in the form of symbols to @ignore_methods.

In theory, you do not have to change the code, and could just input the option in the form url_safe_methods: [:func1,:func2,...]. Apart from inconsistency with other option formats, the problem with this is that if someone mistakenly enters a function in the form of url_safe_methods: "func1", there appears to be a problem in the brakeman code that allows all functions to be considered url-safe-methods. The code that I have proposed stops this problem from being evidenced.

If you are happy with my approach, I can easily implement the safe_methods option using the same technique.

Chris

[oreoshake]

I'm very happy with this. Looking forward to the pull request.

if someone mistakenly enters a function in the form of url_safe_methods: "func1", there appears to be a problem in the brakeman code that allows all functions to be considered url-safe-methods

@presidentbeef is this an issue worth making changes to brakeman?

[presidentbeef]

From what I can tell, setting that option to a string causes an exception. The exception may cause the check to not run, which would explain the behavior. I think raising an exception is acceptable behavior for an invalid option. I could be persuaded otherwise.

[obromios]

I have updated the documentation and done a pull request. At this stage I
have only done the url_safe_methods option.

I think the code for the safe_methods is likely to be almost identical.
However, I am not sure of exactly what this option checks for, so I was not
sure how to write a method that would trigger a warning. If someone can
give me a simple example, I should be able to do this next week.

If there are any other brakeman options that can be implemented with a
similar change, I am happy to to do these, provided I am also given an
example to trigger a warning.

Chris

On Wed, Jul 30, 2014 at 6:45 AM, Justin [email protected] wrote:

From what I can tell, setting that option to a string causes an exception.
The exception may cause the check to not run, which would explain the
behavior. I think raising an exception is acceptable behavior for an
invalid option. I could be persuaded otherwise.

—
Reply to this email directly or view it on GitHub
#22 (comment).