Explicitly exec hooks inside a shell.

Explicitly exec hooks inside a shell.

We don't want to permit the execution of an arbitrary binary.

TBR=ilevy@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214193

[szager1, 2013-07-29 16:10:43]

Committed patchset #1 manually as r214193.

[Isaac (away), 2013-07-29 16:53:10]

I disagree with this.  I don't think it adds security to execute an scripts as
sh instead of using their shabang line and execing a script directly lets you
write hooks in python instead of sh.

Why did this need to be TBRed?  We have lots of team members online for a
meeting at 10am.  TBRing these git-hooks changes presents a more present
security risk:
https://chromiumcodereview.appspot.com/12260019/
crrev.com/174738

[szager1, 2013-07-29 16:59:48]

On 2013/07/29 16:53:10, Isaac wrote:
> I disagree with this.  I don't think it adds security to execute an scripts as
> sh instead of using their shabang line and execing a script directly lets you
> write hooks in python instead of sh.
> 
> Why did this need to be TBRed?  We have lots of team members online for a
> meeting at 10am.  TBRing these git-hooks changes presents a more present
> security risk:
> https://chromiumcodereview.appspot.com/12260019/
> crrev.com/174738

What about an arbitrary compiled binary?  Shall we allow execution of that?  A
shell script is, at least, trivially analyzable.

[szager1, 2013-07-29 17:00:46]

On 2013/07/29 16:59:48, szager1 wrote:
> On 2013/07/29 16:53:10, Isaac wrote:
> > I disagree with this.  I don't think it adds security to execute an scripts
as
> > sh instead of using their shabang line and execing a script directly lets
you
> > write hooks in python instead of sh.
> > 
> > Why did this need to be TBRed?  We have lots of team members online for a
> > meeting at 10am.  TBRing these git-hooks changes presents a more present
> > security risk:
> > https://chromiumcodereview.appspot.com/12260019/
> > crrev.com/174738
> 
> What about an arbitrary compiled binary?  Shall we allow execution of that?  A
> shell script is, at least, trivially analyzable.

And as long as we're airing dirty laundry here: you have yet to attend on of our
10:00am team meetings.

[Isaac (away), 2013-08-07 00:54:22]

This appears to have broken new src-internal checkouts, which have a
post-checkout hook that doesn't work in sh.

Can you explain why you believe execing sh offers higher security?

http://chromegw.corp.google.com/viewvc/chrome-internal?view=rev&revision=32105

[szager, 2013-08-07 00:57:25]

On Tue, Aug 6, 2013 at 5:54 PM, <ilevy@chromium.org> wrote:

> This appears to have broken new src-internal checkouts, which have a
> post-checkout hook that doesn't work in sh.
>
> Can you explain why you believe execing sh offers higher security?
>
> http://chromegw.corp.google.**com/viewvc/chrome-internal?**
>
view=rev&revision=32105<http://chromegw.corp.google.com/viewvc/chrome-internal?view=rev&revision=32105>
>
>
https://chromiumcodereview.**appspot.com/21012004/<https://chromiumcodereview...
>

It's not an exec issue; it's bash vs. sh issue.  I'm trying to figure out
alternate syntax that works in sh.  Failing that, I'll revert.