Rough verification code to ensure deps hosts \in allowed_hosts.

gclient.py

Index: gclient.py
diff --git a/gclient.py b/gclient.py
index 3007487a5930d0f09aeac1260defe3be12bd4360..5373393c01894ed8ac79ba5290558713587d0f29 100755
--- a/gclient.py
+++ b/gclient.py
@@ -342,6 +342,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # A cache of the files affected by the current operation, necessary for
     # hooks.
     self._file_list = []
+    # List of host names from which from which dependencies are allowed.
+    # Default is None, meaning unspecified in DEPS file, and hence all are
+    # allowed. Otherwise, it's a whitelist of hosts.
+    self._allowed_hosts = None
     # If it is not set to True, the dependency wasn't processed for its child
     # dependency, i.e. its DEPS wasn't read.
     self._deps_parsed = False
@@ -687,6 +691,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
           rel_deps.add(os.path.normpath(os.path.join(self.name, d)))
         self.recursedeps = rel_deps
 
+    # TODO(tandrii): default for allowed_hosts, and remove logging warning.
+    self._allowed_hosts = local_scope.get('allowed_hosts')
+    if not self._allowed_hosts:

[iannucci, 2014/09/16 20:34:08]

I think this will be the default condition... most deps files won't have this,
so we should make sure that they continue to work as before.

maybe just do the warning if `'allowed_hosts' in local_scope`.

We should also internally make allowed_hosts a set(), since we're mostly doing
`in` lookups.

[tandrii(chromium), 2014/09/16 21:03:20]

Yep, that's why I had default "None" to preserve previous behavior, although
empty set seems reasonable as well, since if allowed_hosts is defined, it
definitely shouldn't be empty. I'll raise exception in this case.
I didn't get what you mean. Maybe if it is *NOT* in local_scope?
Haha, I also thought of that, but then given the expected number of entries
there, I considered it a not-so-clear optimization, since computing hash of a
long hostname could be longer than checking for for a few comparisons :)
Anyways, I'll make it set, since it's more natural fit for "in" lookups and you
agree :)

[tandrii(chromium), 2014/09/18 19:59:59]

Done.

+      logging.warning("no allowed_hosts specified!")
+    # TODO(tandrii): should we fail now for normal gclient runs (sync, etc) ?
+
     # Convert the deps into real Dependency.
     deps_to_add = []
     for name, url in deps.iteritems():
@@ -756,6 +766,22 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
               print('Using parent\'s revision date %s since we are in a '
                     'different repository.' % options.revision)
 
+  def findDepsFromNotAllowedHosts(self):
+    """Returns a list of depenecies from not allowed hosts.
+
+    If allowed_hosts is not set, allows all hosts and returns empty list.
+    """
+    if self._allowed_hosts is None:
+      return []
+    bad_deps = []
+    for dep in self._dependencies:
+      if isinstance(dep.url, basestring):
+        parsed_url = urlparse.urlparse(dep.url)
+        if parsed_url.netloc and parsed_url.netloc not in self._allowed_hosts:
+          bad_deps.append(dep)
+      # TODO(tandrii): what about other possible URL types?

[iannucci, 2014/09/16 20:34:08]

are there other url types?

[tandrii(chromium), 2014/09/17 15:16:00]

Well, I check here (copied from other part of gclient) that dep.url is
basestring. What if it isn't? For now, I just ignore.

[tandrii(chromium), 2014/09/18 19:59:59]

Done.

+    return bad_deps
+
   # Arguments number differs from overridden method
   # pylint: disable=W0221
   def run(self, revision_overrides, command, args, work_queue, options):
@@ -1053,6 +1079,13 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
   @property
   @gclient_utils.lockedmethod
+  def allowed_hosts(self):
+    if self._allowed_hosts is None:
+      return None
+    return tuple(self._allowed_hosts)

[iannucci, 2014/09/16 20:34:08]

set or frozenset

[tandrii(chromium), 2014/09/18 19:59:59]

Done.

+
+  @property
+  @gclient_utils.lockedmethod
   def file_list(self):
     return tuple(self._file_list)
 
@@ -1077,7 +1110,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     out = []
     for i in ('name', 'url', 'parsed_url', 'safesync_url', 'custom_deps',
               'custom_vars', 'deps_hooks', 'file_list', 'should_process',
-              'processed', 'hooks_ran', 'deps_parsed', 'requirements'):
+              'processed', 'hooks_ran', 'deps_parsed', 'requirements',
+              'allowed_hosts'):
       # First try the native property if it exists.
       if hasattr(self, '_' + i):
         value = getattr(self, '_' + i, False)
@@ -2086,6 +2120,27 @@ def CMDhookinfo(parser, args):
   return 0
 
 
+def CMDverify(parser, args):
+  """Verifies the DEPS file allowed_hosts."""
+  (options, args) = parser.parse_args(args)
+  client = GClient.LoadCurrentConfig(options)
+  if not client:
+    raise gclient_utils.Error('client not configured; see \'gclient config\'')
+  # TODO(tandrii): do we want this recursively?

[iannucci, 2014/09/16 20:34:08]

possibly, though we'd probably want to scope each allowed_hosts to the DEPS
file. e.g. allowing 'cool.whiz' in DEPS for src doesn't allow 'cool.whiz' for
blink's DEPS (or vice versa).

[tandrii(chromium), 2014/09/18 19:59:59]

Done.

+  # Look at each first-level dependency of this gclient only.
+  for dep in client.dependencies:
+    dep.ParseDepsFile()
+    bad_deps = dep.findDepsFromNotAllowedHosts()
+    if not bad_deps:
+      continue
+    print "There are deps from not allowed hosts in file %s" % dep.deps_file
+    for bad_dep in bad_deps:
+      print "\t%s at %s" % (bad_dep.name, bad_dep.url)
+    print "allowed_hosts:", ', '.join(dep.allowed_hosts)
+    raise gclient_utils.Error(
+        'dependencies from disallowed hosts; check your DEPS file.')
+  return 0
+
 class OptionParser(optparse.OptionParser):
   gclientfile_default = os.environ.get('GCLIENT_FILE', '.gclient')