save settings from untrusted config files in a separate configparser

This untrusted configparser is a superset of the trusted configparser,
so that interpolation still works.

Also add an "untrusted" argument to ui.config* to allow querying
ui.ucdata.

With --debug, we print a warning when we read an untrusted config
file, and when we try to access a trusted setting that has one value
in the trusted configparser and another in the untrusted configparser.

doc/hgrc.5.txt

@@ -50,8 +50,9 b' installed.'
     particular repository.  This file is not version-controlled, and
     will not get transferred during a "clone" operation.  Options in
     this file override options in all other configuration files.
-    On Unix, this file is only read if it belongs to a trusted user
-    or to a trusted group.
+    On Unix, most of this file will be ignored if it doesn't belong
+    to a trusted user or to a trusted group.  See the documentation
+    for the trusted section below for more details.
 
 SYNTAX
 ------
@@ -367,11 +368,16 b' server::'
     data transfer overhead.  Default is False.
 
 trusted::
-  Mercurial will only read the .hg/hgrc file from a repository if
-  it belongs to a trusted user or to a trusted group.  This section
-  specifies what users and groups are trusted.  The current user is
-  always trusted.  To trust everybody, list a user or a group with
-  name "*".
+  For security reasons, Mercurial will not use the settings in
+  the .hg/hgrc file from a repository if it doesn't belong to a
+  trusted user or to a trusted group.  The main exception is the
+  web interface, which automatically uses some safe settings, since
+  it's common to serve repositories from different users.
+
+  This section specifies what users and groups are trusted.  The
+  current user is always trusted.  To trust everybody, list a user
+  or a group with name "*".
+
   users;;
     Comma-separated list of trusted users.
   groups;;

mercurial/ui.py

@@ -41,7 +41,9 b' class ui(object):'
             self.traceback = traceback
             self.trusted_users = {}
             self.trusted_groups = {}
+            # if ucdata is not None, its keys must be a superset of cdata's
             self.cdata = util.configparser()
+            self.ucdata = None
             self.readconfig(util.rcpath())
             self.updateopts(verbose, debug, quiet, interactive)
         else:
@@ -51,6 +53,8 b' class ui(object):'
             self.trusted_users = parentui.trusted_users.copy()
             self.trusted_groups = parentui.trusted_groups.copy()
             self.cdata = dupconfig(self.parentui.cdata)
+            if self.parentui.ucdata:
+                self.ucdata = dupconfig(self.parentui.ucdata)
             if self.parentui.overlay:
                 self.overlay = dupconfig(self.parentui.overlay)
 
@@ -95,7 +99,7 b' class ui(object):'
             group = util.groupname(st.st_gid)
             if user not in tusers and group not in tgroups:
                 if warn:
-                    self.warn(_('Not reading file %s from untrusted '
+                    self.warn(_('Not trusting file %s from untrusted '
                                 'user %s, group %s\n') % (f, user, group))
                 return False
         return True
@@ -108,12 +112,30 b' class ui(object):'
                 fp = open(f)
             except IOError:
                 continue
-            if not self._is_trusted(fp, f):
-                continue
+            cdata = self.cdata
+            trusted = self._is_trusted(fp, f)
+            if not trusted:
+                if self.ucdata is None:
+                    self.ucdata = dupconfig(self.cdata)
+                cdata = self.ucdata
+            elif self.ucdata is not None:
+                # use a separate configparser, so that we don't accidentally
+                # override ucdata settings later on.
+                cdata = util.configparser()
+
             try:
-                self.cdata.readfp(fp, f)
+                cdata.readfp(fp, f)
             except ConfigParser.ParsingError, inst:
-                raise util.Abort(_("Failed to parse %s\n%s") % (f, inst))
+                msg = _("Failed to parse %s\n%s") % (f, inst)
+                if trusted:
+                    raise util.Abort(msg)
+                self.warn(_("Ignored: %s\n") % msg)
+
+            if trusted:
+                if cdata != self.cdata:
+                    updateconfig(cdata, self.cdata)
+                if self.ucdata is not None:
+                    updateconfig(cdata, self.ucdata)
         # override data from config files with data set with ui.setconfig
         if self.overlay:
             updateconfig(self.overlay, self.cdata)
@@ -127,7 +149,10 b' class ui(object):'
         self.readhooks.append(hook)
 
     def readsections(self, filename, *sections):
-        "read filename and add only the specified sections to the config data"
+        """Read filename and add only the specified sections to the config data
+
+        The settings are added to the trusted config data.
+        """
         if not sections:
             return
 
@@ -143,6 +168,8 b' class ui(object):'
                 cdata.add_section(section)
 
         updateconfig(cdata, self.cdata, sections)
+        if self.ucdata:
+            updateconfig(cdata, self.ucdata, sections)
 
     def fixconfig(self, section=None, name=None, value=None, root=None):
         # translate paths relative to root (or home) into absolute paths
@@ -150,7 +177,7 b' class ui(object):'
             if root is None:
                 root = os.getcwd()
             items = section and [(name, value)] or []
-            for cdata in self.cdata, self.overlay:
+            for cdata in self.cdata, self.ucdata, self.overlay:
                 if not cdata: continue
                 if not items and cdata.has_section('paths'):
                     pathsitems = cdata.items('paths')
@@ -181,59 +208,98 b' class ui(object):'
     def setconfig(self, section, name, value):
         if not self.overlay:
             self.overlay = util.configparser()
-        for cdata in (self.overlay, self.cdata):
+        for cdata in (self.overlay, self.cdata, self.ucdata):
+            if not cdata: continue
             if not cdata.has_section(section):
                 cdata.add_section(section)
             cdata.set(section, name, value)
         self.fixconfig(section, name, value)
 
-    def _config(self, section, name, default, funcname):
-        if self.cdata.has_option(section, name):
+    def _get_cdata(self, untrusted):
+        if untrusted and self.ucdata:
+            return self.ucdata
+        return self.cdata
+
+    def _config(self, section, name, default, funcname, untrusted, abort):
+        cdata = self._get_cdata(untrusted)
+        if cdata.has_option(section, name):
             try:
-                func = getattr(self.cdata, funcname)
+                func = getattr(cdata, funcname)
                 return func(section, name)
             except ConfigParser.InterpolationError, inst:
-                raise util.Abort(_("Error in configuration section [%s] "
-                                   "parameter '%s':\n%s")
-                                 % (section, name, inst))
+                msg = _("Error in configuration section [%s] "
+                        "parameter '%s':\n%s") % (section, name, inst)
+                if abort:
+                    raise util.Abort(msg)
+                self.warn(_("Ignored: %s\n") % msg)
         return default
 
-    def config(self, section, name, default=None):
-        return self._config(section, name, default, 'get')
+    def _configcommon(self, section, name, default, funcname, untrusted):
+        value = self._config(section, name, default, funcname,
+                             untrusted, abort=True)
+        if self.debugflag and not untrusted and self.ucdata:
+            uvalue = self._config(section, name, None, funcname,
+                                  untrusted=True, abort=False)
+            if uvalue is not None and uvalue != value:
+                self.warn(_("Ignoring untrusted configuration option "
+                            "%s.%s = %s\n") % (section, name, uvalue))
+        return value
 
-    def configbool(self, section, name, default=False):
-        return self._config(section, name, default, 'getboolean')
+    def config(self, section, name, default=None, untrusted=False):
+        return self._configcommon(section, name, default, 'get', untrusted)
 
-    def configlist(self, section, name, default=None):
+    def configbool(self, section, name, default=False, untrusted=False):
+        return self._configcommon(section, name, default, 'getboolean',
+                                  untrusted)
+
+    def configlist(self, section, name, default=None, untrusted=False):
         """Return a list of comma/space separated strings"""
-        result = self.config(section, name)
+        result = self.config(section, name, untrusted=untrusted)
         if result is None:
             result = default or []
         if isinstance(result, basestring):
             result = result.replace(",", " ").split()
         return result
 
-    def has_config(self, section):
+    def has_config(self, section, untrusted=False):
         '''tell whether section exists in config.'''
-        return self.cdata.has_section(section)
+        cdata = self._get_cdata(untrusted)
+        return cdata.has_section(section)
 
-    def configitems(self, section):
+    def _configitems(self, section, untrusted, abort):
         items = {}
-        if self.cdata.has_section(section):
+        cdata = self._get_cdata(untrusted)
+        if cdata.has_section(section):
             try:
-                items.update(dict(self.cdata.items(section)))
+                items.update(dict(cdata.items(section)))
             except ConfigParser.InterpolationError, inst:
-                raise util.Abort(_("Error in configuration section [%s]:\n%s")
-                                 % (section, inst))
+                msg = _("Error in configuration section [%s]:\n"
+                        "%s") % (section, inst)
+                if abort:
+                    raise util.Abort(msg)
+                self.warn(_("Ignored: %s\n") % msg)
+        return items
+
+    def configitems(self, section, untrusted=False):
+        items = self._configitems(section, untrusted=untrusted, abort=True)
+        if self.debugflag and not untrusted and self.ucdata:
+            uitems = self._configitems(section, untrusted=True, abort=False)
+            keys = uitems.keys()
+            keys.sort()
+            for k in keys:
+                if uitems[k] != items.get(k):
+                    self.warn(_("Ignoring untrusted configuration option "
+                                "%s.%s = %s\n") % (section, k, uitems[k]))
         x = items.items()
         x.sort()
         return x
 
-    def walkconfig(self):
-        sections = self.cdata.sections()
+    def walkconfig(self, untrusted=False):
+        cdata = self._get_cdata(untrusted)
+        sections = cdata.sections()
         sections.sort()
         for section in sections:
-            for name, value in self.configitems(section):
+            for name, value in self.configitems(section, untrusted):
                 yield section, name, value.replace('\n', '\\n')
 
     def extensions(self):

tests/test-trusted.py

@@ -9,7 +9,7 b' from mercurial import ui, util'
 hgrc = os.environ['HGRCPATH']
 
 def testui(user='foo', group='bar', tusers=(), tgroups=(),
-           cuser='foo', cgroup='bar', debug=False):
+           cuser='foo', cgroup='bar', debug=False, silent=False):
     # user, group => owners of the file
     # tusers, tgroups => trusted users/groups
     # cuser, cgroup => user/group of the current process
@@ -56,8 +56,18 b" def testui(user='foo', group='bar', tuse"
     parentui.updateopts(debug=debug)
     u = ui.ui(parentui=parentui)
     u.readconfig('.hg/hgrc')
+    if silent:
+        return u
+    print 'trusted'
     for name, path in u.configitems('paths'):
         print '   ', name, '=', path
+    print 'untrusted'
+    for name, path in u.configitems('paths', untrusted=True):
+        print '.',
+        u.config('paths', name) # warning with debug=True
+        print '.',
+        u.config('paths', name, untrusted=True) # no warnings
+        print name, '=', path
     print
 
     return u
@@ -68,6 +78,7 b" os.mkdir('.hg')"
 f = open('.hg/hgrc', 'w')
 f.write('[paths]\n')
 f.write('local = /another/path\n\n')
+f.write('interpolated = %(global)s%(local)s\n\n')
 f.close()
 
 #print '# Everything is run by user foo, group bar\n'
@@ -111,3 +122,83 b" testui(user='abc', group='def', tusers=["
 
 print "# Can't figure out the name of the user running this process"
 testui(user='abc', group='def', cuser=None)
+
+print "# prints debug warnings"
+u = testui(user='abc', group='def', cuser='foo', debug=True)
+
+print "# ui.readsections"
+filename = 'foobar'
+f = open(filename, 'w')
+f.write('[foobar]\n')
+f.write('baz = quux\n')
+f.close()
+u.readsections(filename, 'foobar')
+print u.config('foobar', 'baz')
+
+print
+print "# read trusted, untrusted, new ui, trusted"
+u = ui.ui()
+u.updateopts(debug=True)
+u.readconfig(filename)
+u2 = ui.ui(parentui=u)
+def username(uid=None):
+    return 'foo'
+util.username = username
+u2.readconfig('.hg/hgrc')
+print 'trusted:'
+print u2.config('foobar', 'baz')
+print u2.config('paths', 'interpolated')
+print 'untrusted:'
+print u2.config('foobar', 'baz', untrusted=True)
+print u2.config('paths', 'interpolated', untrusted=True)
+
+print 
+print "# error handling"
+
+def assertraises(f, exc=util.Abort):
+    try:
+        f()
+    except exc, inst:
+        print 'raised', inst.__class__.__name__
+    else:
+        print 'no exception?!'
+
+print "# file doesn't exist"
+os.unlink('.hg/hgrc')
+assert not os.path.exists('.hg/hgrc')
+testui(debug=True, silent=True)
+testui(user='abc', group='def', debug=True, silent=True)
+
+print
+print "# parse error"
+f = open('.hg/hgrc', 'w')
+f.write('foo = bar')
+f.close()
+testui(user='abc', group='def', silent=True)
+assertraises(lambda: testui(debug=True, silent=True))
+
+print
+print "# interpolation error"
+f = open('.hg/hgrc', 'w')
+f.write('[foo]\n')
+f.write('bar = %(')
+f.close()
+u = testui(debug=True, silent=True)
+print '# regular config:'
+print '  trusted',
+assertraises(lambda: u.config('foo', 'bar'))
+print 'untrusted',
+assertraises(lambda: u.config('foo', 'bar', untrusted=True))
+
+u = testui(user='abc', group='def', debug=True, silent=True)
+print '  trusted ',
+print u.config('foo', 'bar')
+print 'untrusted',
+assertraises(lambda: u.config('foo', 'bar', untrusted=True))
+
+print '# configitems:'
+print '  trusted ',
+print u.configitems('foo')
+print 'untrusted',
+assertraises(lambda: u.configitems('foo', untrusted=True))
+

tests/test-trusted.py.out

@@ -1,67 +1,212 b''
 # same user, same group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # same user, different group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # different user, same group
-Not reading file .hg/hgrc from untrusted user abc, group bar
+Not trusting file .hg/hgrc from untrusted user abc, group bar
+trusted
     global = /some/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # different user, same group, but we trust the group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # different user, different group
-Not reading file .hg/hgrc from untrusted user abc, group def
+Not trusting file .hg/hgrc from untrusted user abc, group def
+trusted
     global = /some/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # different user, different group, but we trust the user
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # different user, different group, but we trust the group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # different user, different group, but we trust the user and the group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # we trust all users
 # different user, different group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # we trust all groups
 # different user, different group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # we trust all users and groups
 # different user, different group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # we don't get confused by users and groups with the same name
 # different user, different group
-Not reading file .hg/hgrc from untrusted user abc, group def
+Not trusting file .hg/hgrc from untrusted user abc, group def
+trusted
     global = /some/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # list of user names
 # different user, different group, but we trust the user
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # list of group names
 # different user, different group, but we trust the group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
 # Can't figure out the name of the user running this process
 # different user, different group
+trusted
     global = /some/path
+    interpolated = /some/path/another/path
     local = /another/path
+untrusted
+. . global = /some/path
+. . interpolated = /some/path/another/path
+. . local = /another/path
 
+# prints debug warnings
+# different user, different group
+Not trusting file .hg/hgrc from untrusted user abc, group def
+trusted
+Ignoring untrusted configuration option paths.interpolated = /some/path/another/path
+Ignoring untrusted configuration option paths.local = /another/path
+    global = /some/path
+untrusted
+. . global = /some/path
+.Ignoring untrusted configuration option paths.interpolated = /some/path/another/path
+ . interpolated = /some/path/another/path
+.Ignoring untrusted configuration option paths.local = /another/path
+ . local = /another/path
+
+# ui.readsections
+quux
+
+# read trusted, untrusted, new ui, trusted
+Not trusting file foobar from untrusted user abc, group def
+trusted:
+Ignoring untrusted configuration option foobar.baz = quux
+None
+/some/path/another/path
+untrusted:
+quux
+/some/path/another/path
+
+# error handling
+# file doesn't exist
+# same user, same group
+# different user, different group
+
+# parse error
+# different user, different group
+Not trusting file .hg/hgrc from untrusted user abc, group def
+Ignored: Failed to parse .hg/hgrc
+File contains no section headers.
+file: .hg/hgrc, line: 1
+'foo = bar'
+# same user, same group
+raised Abort
+
+# interpolation error
+# same user, same group
+# regular config:
+  trusted raised Abort
+untrusted raised Abort
+# different user, different group
+Not trusting file .hg/hgrc from untrusted user abc, group def
+  trusted Ignored: Error in configuration section [foo] parameter 'bar':
+bad interpolation variable reference '%('
+ None
+untrusted raised Abort
+# configitems:
+  trusted Ignored: Error in configuration section [foo]:
+bad interpolation variable reference '%('
+ []
+untrusted raised Abort