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

Brakeman reports an exception during CheckSessionSettings #1046

Closed
halostatue opened this Issue May 13, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@halostatue

halostatue commented May 13, 2017

During CheckSessionSettings, an exception is thrown while checking my application’s config/secrets.yml:

(<unknown>): could not find expected ':' while scanning a simple key at line 20 column 1 …/safe_yaml-1.0.4/lib/safe_yaml/load.rb:143:inparse'`

This is specifically because of the structure of my config/secrets.yml:

development:
  secret_key_base: VALUE
test:
  secret_key_base: VALUE

# This must not be indented for it to work.
<% if Rails.root.join('config/ansible/secrets.yml').exist? %>
<%= Rails.root.join('config/ansible/secrets.yml').read %>
<% end %>
<% if Rails.root.join('config/local/secrets.yml').exist? %>
<%= Rails.root.join('config/local/secrets.yml').read %>
<% end %>

Basically, I use install-time generated files in config/ansible/secrets.yml and config/local/secrets.yml (both directories excluded from git) and use the fact that Rails loads config/*.yml through ERB pretty much by default to paste the included files into the default file.

YAML doesn’t have includes as a concept (this is probably good; it’s got enough insecurity in design as it is), and Brakeman doesn’t parse ERB by default (this is also probably a good thing as it’s a static analyzer). However, this crashes the CheckSessionSettings code (because it’s not a complete YAML file until the ERB is parsed), and that’s probably not good, either. (This probably doesn’t matter when ERB is used by people who have to build Heroku-factor applications as YAML doesn’t care whether a key is foo or <%= ENV['foo'] %>.)

For the moment, I’m skipping CheckSessionSettings in my code, but this should be on the radar as an oddity.

[Edit: Originally reported against CheckYAMLParsing; that’s wrong.]

@halostatue halostatue changed the title from Brakeman reports an exception during CheckYAMLParsing to Brakeman reports an exception during CheckSessionSettings May 13, 2017

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef May 13, 2017

Owner

Hi Austin,

Thank you for bringing this up. A similar issue has come up before (in that case SafeYAML was being strict and rejecting unsafe YAML). The exception should definitely be handled.

Owner

presidentbeef commented May 13, 2017

Hi Austin,

Thank you for bringing this up. A similar issue has come up before (in that case SafeYAML was being strict and rejecting unsafe YAML). The exception should definitely be handled.

presidentbeef added a commit that referenced this issue May 15, 2017

presidentbeef added a commit that referenced this issue May 15, 2017

presidentbeef added a commit that referenced this issue May 15, 2017

Repository owner locked and limited conversation to collaborators Jul 1, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.