[PATCH 3 of 3] 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 97b264764e417eefc025b8c7fe298114bde11f1c
# Parent  41e1d17cdc4943805d5e7a6e7c131367eb94e8d3
# 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 unstrusted config are taken in account but never
actually run. Instead they are reported as failure right away. This will ensure
no validation performed by a hook will be ignored (as a fallback to "False" is
better than a fallback to "True" here)

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.

Here is some example output:

    non-mandatory hook:
      warning: post-log hook denied (from untrusted config)

    mandatory hook:
      abort: pretxnclose hook denied (from untrusted config)

It turned out that the "trusted" concept did not had any ".t" test. The test
process do not have enough priviledge to change config file ownership. It should
be possible to implement a devel config option that makes a hgrc file
untrusted to testing purpose. However, given that the freeze is quite close, I
wanted this patch out for review quickly. I'll follow up with another series
installing the test infrastructure for "trusted".

diff -r 41e1d17cdc49 -r 97b264764e41 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,16 +161,29 @@
         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)
+    trusted = set(hooks)
+    # Be careful in this section, propagating the real command from an
+    # untrusted source would create a security vulnerability, make sure
+    # anything altered in that section use "_fromuntrusted" as its command.
+    untrustedhooks = _hookitems(ui, _untrusted=True)
+    for name, value in untrustedhooks.items():
+        if value != hooks.get(name, None):
+            trusted.discard(name)
+            hooks[name] = value[:2] + (_fromuntrusted,)
+    # (end of the security sensitive section)
     order = sorted(hooks, key=lambda x: hooks[x][:2] + (x,))
     return [(k, hooks[k][2]) for k in order]
 
-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), cmd)
@@ -215,7 +228,15 @@
                     # files seem to be bogus, give up on redirecting (WSGI, etc)
                     pass
 
-            if callable(cmd):
+            if cmd is _fromuntrusted:
+                abortmsg = _('%s hook denied (from untrusted config)')
+                warnmsg = _('warning: %s hook denied (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: