[PATCH 3 of 3 V2] hook: report untrusted hooks as failure (issue5110) (BC)

[Pierre-Yves David]

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
# Date 1460626875 25200
#      Thu Apr 14 02:41:15 2016 -0700
# Node ID 3fd5af5285c881bf0cd33f7b5e596c17aac1aaf8
# Parent  621b96947ee7b411d6353f9281e4419097944b91
# EXP-Topic hooks
hook: report untrusted hooks as failure (issue5110) (BC)

Before this patch, there was no way for a repository owner to ensure that
validation hooks would be run by people with write access. If someone had write
access but did not trust the user owning the repository, the config and its hook
would simply be ignored.

After this patch, hooks from untrusted configs are taken into account but never
actually run. Instead they are reported as failures right away. This will ensure
validation performed by a hook is not ignored.

As a side effect writer can be forced to trust a repository hgrc by adding a
'pretxnopen.trust=true' hook to the file.

This was discussed during the 3.8 sprint with Matt Mackall, Augie Fackler and
Kevin Bullock.

diff -r 621b96947ee7 -r 3fd5af5285c8 mercurial/hook.py
--- a/mercurial/hook.py	Thu Apr 14 17:03:49 2016 -0700
+++ b/mercurial/hook.py	Thu Apr 14 02:41:15 2016 -0700
@@ -161,15 +161,28 @@
         ui.warn(_('warning: %s hook %s\n') % (name, desc))
     return r
 
+# represent an untrusted hook command
+_fromuntrusted = object()
+
 def _allhooks(ui):
     """return a list of (hook-id, cmd) pair sorted by priority"""
     hooks = _hookitems(ui)
+    # Be careful in this section, propagating the real commands from untrusted
+    # sources would create a security vulnerability, make sure anything altered
+    # in that section uses "_fromuntrusted" as its command.
+    untrustedhooks = _hookitems(ui, _untrusted=True)
+    for name, value in untrustedhooks.items():
+        trustedvalue = hooks.get(name, (None, None, name, _fromuntrusted))
+        if value != trustedvalue:
+            (lp, lo, lk, lv) = trustedvalue
+            hooks[name] = (lp, lo, lk, _fromuntrusted)
+    # (end of the security sensitive section)
     return [(k, v) for p, o, k, v in sorted(hooks.values())]
 
-def _hookitems(ui):
+def _hookitems(ui, _untrusted=False):
     """return all hooks items ready to be sorted"""
     hooks = {}
-    for name, cmd in ui.configitems('hooks'):
+    for name, cmd in ui.configitems('hooks', untrusted=_untrusted):
         if not name.startswith('priority'):
             priority = ui.configint('hooks', 'priority.%s' % name, 0)
             hooks[name] = (-priority, len(hooks), name, cmd)
@@ -214,7 +227,16 @@
                     # files seem to be bogus, give up on redirecting (WSGI, etc)
                     pass
 
-            if callable(cmd):
+            if cmd is _fromuntrusted:
+                abortmsg = _('%s hook forbidden (from untrusted config)')
+                warnmsg = _('warning: %s hook forbidden '
+                            '(from untrusted config)\n')
+                if throw:
+                    raise error.HookAbort(abortmsg % name)
+                ui.warn(warnmsg % name)
+                r = 1
+                raised = False
+            elif callable(cmd):
                 r, raised = _pythonhook(ui, repo, name, hname, cmd, args, throw)
             elif cmd.startswith('python:'):
                 if cmd.count(':') >= 2:
diff -r 621b96947ee7 -r 3fd5af5285c8 tests/test-hook.t
--- a/tests/test-hook.t	Thu Apr 14 17:03:49 2016 -0700
+++ b/tests/test-hook.t	Thu Apr 14 02:41:15 2016 -0700
@@ -794,7 +794,6 @@
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     b
   
-  $ cd ..
 
 pretxnclose hook failure should abort the transaction
 
@@ -816,3 +815,61 @@
   $ hg recover
   no interrupted transaction available
   [1]
+  $ cd ..
+
+Hook from untrusted hgrc are reported as failure
+================================================
+
+  $ cat << EOF > $TESTTMP/untrusted.py
+  > from mercurial import scmutil
+  > def uisetup(ui):
+  >     class untrustedui(ui.__class__):
+  >         def _trusted(self, fp, f):
+  >             if fp.name.endswith('untrusted/.hg/hgrc'):
+  >                 return False
+  >             return super(untrustedui, self)._trusted(fp, f)
+  >     ui.__class__ = untrustedui
+  > EOF
+  $ cat << EOF >> $HGRCPATH
+  > [extensions]
+  > untrusted=$TESTTMP/untrusted.py
+  > EOF
+  $ hg init untrusted
+  $ cd untrusted
+
+Non-blocking hook
+-----------------
+
+  $ cat << EOF >> .hg/hgrc
+  > [hooks]
+  > txnclose.testing=echo txnclose hook called
+  > EOF
+  $ touch a && hg commit -Aqm a
+  warning: txnclose hook forbidden (from untrusted config)
+  $ hg log
+  changeset:   0:3903775176ed
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     a
+  
+
+Non-blocking hook
+-----------------
+
+  $ cat << EOF >> .hg/hgrc
+  > [hooks]
+  > pretxnclose.testing=echo pre-txnclose hook called
+  > EOF
+  $ touch b && hg commit -Aqm a
+  transaction abort!
+  rollback completed
+  abort: pretxnclose hook forbidden (from untrusted config)
+  [255]
+  $ hg log
+  changeset:   0:3903775176ed
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     a
+