On 09.08.2012 19:34, Stefan Fuhrmann wrote:
> On Thu, Aug 9, 2012 at 4:01 PM, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
>> On Thu, Aug 9, 2012 at 12:20 AM, <tortoisesvn_at_googlecode.com> wrote:
>>> Revision: 23145
>>> Author: stefan.fuhrmann.1974
>>> Date: Wed Aug 8 15:19:46 2012
>>> Log: Add optional "force" options to scripts. If set,
>>> the respective script will be applied without
>>> prior approval by the users. Also, forced scripts
>>> cannot be ignored after failure.
>>
>> Uh-oh!
>> That's exactly why I didn't implement this in earlier versions: this
>> is a big, big security risk!
>> Executing something without asking the user first is a NO-NO. It
>> doesn't matter if its useful or not. And it doesn't matter if its
>> optional.
>
> I agree that executing commands from the repository content is
> unsafe. I will change that as follows:
>
> * hook settings via tsvn:* properties will always require user approval
> (at least once)
> * if "force" has been set here, it will disable the "retry w/o hook"
> button in the progress dialog
> * "force"d hooks configured via settings menu have - by definition -
> already been approved by user (or someone with access to the machine)
>
> This should eliminate the attack vector of some outside committer
> executing code on your machine w/o prior approval.
Agreed. This would work fine.
>
> Locally configured hooks are as trustworthy as the hook content itself.
> The approval does not give extra security in that case.
>
>> Is there a specific reason you implemented this?
>> If not, I'd rather revert this change.
>
> I've got that from customers (even prior to WANdisco): The hook
> scripts will tell them "your stuff does not work" but TSVN gives
> them a "what the hell - commit it anyway!" button.
Well, in that case the customer should set up a repository hook, not a
client side hook. Then such commits could be rejected. That's the only
way to be sure: TSVN now ships with the command line client (optional
install), so users could just commit with that instead.
> A "force" option would work perfectly in 1.7.x as it simply hides
> that button. But in 1.8 we need a more sophisticated set of rules.
> It will not be watertight: since we won't trust repository dictated
> hooks, a user must be allowed to not run them but still be able
> to continue with the actual SVN operation. Everything else would
> open a DOS attack vector.
Please don't merge new features to 1.7.x - that's for bug fixes only.
Merging the disabling of the retry button is a new feature.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2997611
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2012-08-09 19:39:05 CEST