search

jish / pre-commit

A slightly improved pre-commit hook for git
https://jish.github.io/pre-commit/
Other
796 stars 96 forks source link

Modified ci check to use binstubs or bundle exec if bunstub doesn't exist #210

Closed knovoselic closed 9 years ago

knovoselic commented 9 years ago

Correct way to run rake in rails 4 app is to use binstubs. If spring is configured this also means that rake will load faster. If binstub for rake (bin/rake) doesn't exist, bundle exec rake will be used instead.

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.0%) to 98.65% when pulling 9ac45a666db69046bec63a123f5ad75ee2051444 on knovoselic:use_binstubs_or_bundle_exec into 6840a5eed38f4b88d27b0b4c158dbaf522750899 on jish:master.

mpapis commented 9 years ago

-1 using bin/ is not safe.

mpapis commented 9 years ago

if you want to load bundler use rubygemsbundled gem or stop using bundler and do the same with rubygems and single environment variable

knovoselic commented 9 years ago

@mpapis I agree about bundle exec, I haven't thought this through. But why is using bin/ not safe?

mpapis commented 9 years ago

Check http://niczsoft.com/2013/10/please-hack-my-rails/

knovoselic commented 9 years ago

@mpapis thanks. Running bin/rake isn't the same as adding bin to you $PATH. The only way that it can be exploited is if someone is going to commit malicious script into the repo, or am I missing something?

mpapis commented 9 years ago

malicious code is the only concern here, it's no difference if bin/ is in PATH or not, running scripts from there requires extra caution and should not be done in automated way

as for my earlier email instead of rubygemsbundled I meant rubgems-bundler - which makes this pull request obsolete as proper behaviour is achieved without any changes to pre-commit.

knovoselic commented 9 years ago

Ok, thanks, I'll checkout rubygems-bundler.