Allow users to customize the sanity-checking regexes

This doesn't add security, but at least allows users to open up the
regexes where it's safe to do so.  Fixes #985.

Tested by hand.

master/NEWS

@@ -4,6 +4,13 @@ Major User visible changes in Buildbot.             -*- outline -*-
 
 
 * Next Version
+
+** Customizable validation regexps
+
+The global c['validation'] parameter can be used to adjust the regular
+expressions used to validate branches, revisions, and properties input by the
+user.
+
 ** Logging for SVNPoller cleaned up
 
 All logging for SVNPoller now starts with "SVNPoller: ". Previously it was

master/buildbot/config.py

@@ -22,6 +22,7 @@ class MasterConfig(object):
     available at C{master.config}.
 
     @ivar changeHorizon: the current change horizon
+    @ivar validation: regexes for preventing invalid inputs
     """
 
     changeHorizon = None

master/buildbot/master.py

@@ -14,6 +14,7 @@
 # Copyright Buildbot Team Members
 
 
+import re
 import os
 import signal
 import time
@@ -308,6 +309,22 @@ def do_load(_):
                 metrics_config = config.get("metrics")
                 caches_config = config.get("caches", {})
 
+                # load validation, with defaults, and verify no unrecognized
+                # keys are included.
+                validation_defaults = {
+                    'branch' : re.compile(r'^[\w.+/~-]*$'),
+                    'revision' : re.compile(r'^[ \w\.\-\/]*$'),
+                    'property_name' : re.compile(r'^[\w\.\-\/\~:]*$'),
+                    'property_value' : re.compile(r'^[\w\.\-\/\~:]*$'),
+                }
+                validation_config = validation_defaults.copy()
+                validation_config.update(config.get("validation", {}))
+                v_config_keys = set(validation_config.keys())
+                v_default_keys = set(validation_defaults.keys())
+                if v_config_keys > v_default_keys:
+                    raise ValueError("unrecognized validation key(s): %s" %
+                            (", ".join(v_config_keys - v_default_keys,)))
+
             except KeyError:
                 log.msg("config dictionary is missing a required parameter")
                 log.msg("leaving old configuration in place")
@@ -335,6 +352,7 @@ def do_load(_):
                 raise KeyError("must have a 'slaves' key")
 
             self.config.changeHorizon = changeHorizon
+            self.config.validation = validation_config
 
             change_source = config.get('change_source', [])
             if isinstance(change_source, (list, tuple)):

master/buildbot/status/web/base.py

@@ -62,20 +62,23 @@ class IHTMLLog(Interface):
 
 def getAndCheckProperties(req):
     """
-Fetch custom build properties from the HTTP request of a "Force build" or
-"Resubmit build" HTML form.
-Check the names for valid strings, and return None if a problem is found.
-Return a new Properties object containing each property found in req.
-"""
+    Fetch custom build properties from the HTTP request of a "Force build" or
+    "Resubmit build" HTML form.
+    Check the names for valid strings, and return None if a problem is found.
+    Return a new Properties object containing each property found in req.
+    """
+    master = req.site.buildbot_service.master
+    pname_validate = master.config.validation['property_name']
+    pval_validate = master.config.validation['property_value']
     properties = Properties()
     i = 1
     while True:
         pname = req.args.get("property%dname" % i, [""])[0]
         pvalue = req.args.get("property%dvalue" % i, [""])[0]
         if not pname:
             break
-        if not re.match(r'^[\w\.\-\/\~:]*$', pname) \
-                or not re.match(r'^[\w\.\-\/\~:]*$', pvalue):
+        if not pname_validate.match(pname) \
+                or not pval_validate.match(pvalue):
             log.msg("bad property name='%s', value='%s'" % (pname, pvalue))
             return None
         properties.setProperty(pname, pvalue, "Force Build Form")

master/buildbot/status/web/builder.py

@@ -16,7 +16,7 @@
 
 from twisted.web import html
 from twisted.web.util import Redirect
-import re, urllib, time
+import urllib, time
 from twisted.python import log
 from twisted.internet import defer
 from buildbot import interfaces
@@ -153,12 +153,15 @@ def force(self, req, auth_ok=False):
                 log.msg("..but not authorized")
                 return Redirect(path_to_authfail(req))
 
+        master = self.getBuildmaster(req)
+
         # keep weird stuff out of the branch revision, and property strings.
-        # TODO: centralize this somewhere.
-        if not re.match(r'^[\w.+/~-]*$', branch):
+        branch_validate = master.config.validation['branch']
+        revision_validate = master.config.validation['revision']
+        if not branch_validate.match(branch):
             log.msg("bad branch '%s'" % branch)
             return Redirect(path_to_builder(req, self.builder_status))
-        if not re.match(r'^[ \w\.\-\/]*$', revision):
+        if not revision_validate.match(r'^[ \w\.\-\/]*$', revision):
             log.msg("bad revision '%s'" % revision)
             return Redirect(path_to_builder(req, self.builder_status))
         properties = getAndCheckProperties(req)
@@ -169,7 +172,6 @@ def force(self, req, auth_ok=False):
         if not revision:
             revision = None
 
-        master = self.getBuildmaster(req)
         d = master.db.sourcestamps.addSourceStamp(branch=branch,
                 revision=revision, project=project, repository=repository)
         def make_buildset(ssid):

master/buildbot/status/words.py

@@ -89,6 +89,7 @@ class Contact(base.StatusReceiver):
     def __init__(self, channel):
         #StatusReceiver.__init__(self) doesn't exist
         self.channel = channel
+        self.master = channel.master
         self.notify_events = {}
         self.subscribed = 0
         self.muted = False
@@ -451,12 +452,13 @@ def command_FORCE(self, args, who):
             raise UsageError("you must provide a Builder, " + errReply)
 
         # keep weird stuff out of the branch and revision strings. 
-        # TODO:  centralize this somewhere.
-        if branch and not re.match(r'^[\w\.\-\/]*$', branch):
+        branch_validate = self.master.config.validation['branch']
+        revision_validate = self.master.config.validation['revision']
+        if branch and not branch_validate.match(branch):
             log.msg("bad branch '%s'" % branch)
             self.send("sorry, bad branch '%s'" % branch)
             return
-        if revision and not re.match(r'^[\w\.\-\/]*$', revision):
+        if revision and not revision_validate.match(revision):
             log.msg("bad revision '%s'" % revision)
             self.send("sorry, bad revision '%s'" % revision)
             return

master/docs/cfg-global.texinfo

@@ -12,6 +12,7 @@ The keys in this section affect the operations of the buildmaster globally.
 * Defining Global Properties::
 * Debug Options::
 * Metrics Options::
+* Input Validation::
 @end menu
 
 @node Database Specification
@@ -572,3 +573,26 @@ data attributes of <Builder ''builder'' at 48963528>
 @code{periodic_interval} determines how often various non-event based metrics are collected, such as memory usage, uncollectable garbage, reactor delay. This defaults to 10s. If set to 0 or None, then periodic collection of this data is disabled. This value can also be changed via a reconifg.
 
 Read more about metrics in the @ref{Metrics} section of the documentation.
+
+@node Input Validation
+@subsection Input Validation
+@bcindex c['validation']
+
+@example
+import re
+c['validation'] = @{
+    'branch' : re.compile(r'^[\w.+/~-]*$'),
+    'revision' : re.compile(r'^[ \w\.\-\/]*$'),
+    'property_name' : re.compile(r'^[\w\.\-\/\~:]*$'),
+    'property_value' : re.compile(r'^[\w\.\-\/\~:]*$'),
+@}
+@end example
+
+This option configures the validation applied to user inputs of various types.
+This validation is important since these values are often included in
+command-line arguments executed on slaves.  Allowing arbitrary input from
+untrusted users may raise security concerns.
+
+The keys describe the type of input validated; the values are compiled regular
+expressions against which the input will be matched.  The defaults for each
+type of input are those given in the example, above.