Security: Fix issues with path handling in libraries and in general.

config/galaxy.ini.sample

@@ -911,6 +911,14 @@ use_interactive = True
 # sub-directories of files contained in their directory.
 #user_library_import_dir = None
 
+# For security reasons, users may not import any files that actually lie
+# outside of their `user_library_import_dir` (e.g. using symbolic links). A
+# list of directories can be allowed by setting the following option (the list
+# is comma-separated). Be aware that *any* user with library import permissions
+# can import from anywhere in these directories (assuming they are able to
+# create symlinks to them).
+#user_library_import_symlink_whitelist = None
+
 # Allow admins to paste filesystem paths during upload. For libraries this
 # adds an option to the admin library upload tool allowing admins to paste
 # filesystem paths to files and directories in a box, and these paths will be

lib/galaxy/config.py

@@ -394,6 +394,7 @@ class Configuration(object):
         self.genomespace_ui_url = kwargs.get('genomespace_ui_url', 'https://gsui.genomespace.org/jsui/')
         self.library_import_dir = kwargs.get('library_import_dir', None)
         self.user_library_import_dir = kwargs.get('user_library_import_dir', None)
+        self.user_library_import_symlink_whitelist = listify(kwargs.get('user_library_import_symlink_whitelist', []), do_strip=True)
         # Searching data libraries
         self.enable_lucene_library_search = string_as_bool(kwargs.get('enable_lucene_library_search', False))
         self.enable_whoosh_library_search = string_as_bool(kwargs.get('enable_whoosh_library_search', False))

lib/galaxy/managers/folders.py

@@ -132,7 +132,7 @@ class FolderManager(object):
         """
         parent_folder = self.get(trans, parent_folder_id)
         current_user_roles = trans.get_current_user_roles()
-        if not (trans.user_is_admin or trans.app.security_agent.can_add_library_item(current_user_roles, parent_folder)):
+        if not (trans.user_is_admin() or trans.app.security_agent.can_add_library_item(current_user_roles, parent_folder)):
             raise InsufficientPermissionsException('You do not have proper permission to create folders under given folder.')
         new_folder = trans.app.model.LibraryFolder(name=new_folder_name, description=new_folder_description)
         # We are associating the last used genome build with folders, so we will always
@@ -230,7 +230,7 @@ class FolderManager(object):
         """
         Return true if the user has permissions to add item to the given folder.
         """
-        if trans.user_is_admin:
+        if trans.user_is_admin():
             return True
         current_user_roles = trans.get_current_user_roles()
         add_roles = set(trans.app.security_agent.get_roles_for_action(folder, trans.app.security_agent.permitted_actions.LIBRARY_ADD))

lib/galaxy/managers/libraries.py

@@ -50,7 +50,7 @@ class LibraryManager(object):
         """
         Create a new library.
         """
-        if not trans.user_is_admin:
+        if not trans.user_is_admin():
             raise exceptions.ItemAccessibilityException('Only administrators can create libraries.')
         else:
             library = trans.app.model.Library(name=name, description=description, synopsis=synopsis)

lib/galaxy/tools/parameters/grouping.py

@@ -380,6 +380,7 @@ class UploadDataset(Group):
                     warnings.append("All FTP uploaded file selections were ignored.")
             elif ftp_files is not None and trans.user is not None:  # look for files uploaded via FTP
                 user_ftp_dir = trans.user_ftp_dir
+                assert not os.path.islink(user_ftp_dir), "User FTP directory cannot be a symbolic link"
                 for (dirpath, dirnames, filenames) in os.walk(user_ftp_dir):
                     for filename in filenames:
                         for ftp_filename in ftp_files:
@@ -449,6 +450,7 @@ class UploadDataset(Group):
                     # TODO: warning to the user (could happen if session has become invalid)
                 else:
                     user_ftp_dir = trans.user_ftp_dir
+                    assert not os.path.islink(user_ftp_dir), "User FTP directory cannot be a symbolic link"
                     for (dirpath, dirnames, filenames) in os.walk(user_ftp_dir):
                         for filename in filenames:
                             path = relpath(os.path.join(dirpath, filename), user_ftp_dir)

lib/galaxy/util/__init__.py

@@ -8,6 +8,7 @@ from __future__ import absolute_import
 import binascii
 import collections
 import errno
+import importlib
 import json
 import logging
 import os
@@ -29,7 +30,7 @@ except ImportError:
 
 from datetime import datetime
 from hashlib import md5
-from os.path import normpath, relpath
+from os.path import relpath
 from xml.etree import ElementInclude, ElementTree
 from xml.etree.ElementTree import ParseError
 
@@ -49,6 +50,7 @@ except ImportError:
     docutils_html4css1 = None
 
 from .inflection import English, Inflector
+from .path import safe_contains, safe_makedirs, safe_relpath  # noqa: F401
 
 inflector = Inflector(English)
 
@@ -604,31 +606,19 @@ def which(file):
     return None
 
 
-def safe_makedirs(path):
-    """ Safely make a directory, do not fail if it already exist or
-    is created during execution.
-    """
-    if not os.path.exists(path):
-        try:
-            os.makedirs(path)
-        except OSError as e:
-            # review source for Python 2.7 this would only ever happen
-            # for the last path anyway so need to recurse - this exception
-            # means the last part of the path was already in existence.
-            if e.errno != errno.EEXIST:
-                raise
-
-
 def in_directory(file, directory, local_path_module=os.path):
     """
     Return true, if the common prefix of both is equal to directory
     e.g. /a/b/c/d.rst and directory is /a/b, the common prefix is /a/b
-    """
 
-    # Make both absolute.
-    directory = local_path_module.abspath(directory)
-    file = local_path_module.abspath(file)
-    return local_path_module.commonprefix([file, directory]) == directory
+    local_path_module is used by Pulsar to check Windows paths while running on
+    a POSIX-like system.
+    """
+    if local_path_module != os.path:
+        _safe_contains = importlib.import_module('galaxy.util.path.%s' % local_path_module.__name__).safe_contains
+    else:
+        _safe_contains = safe_contains
+    return _safe_contains(directory, file)
 
 
 def merge_sorted_iterables(operator, *iterables):
@@ -1505,22 +1495,6 @@ def download_to_file(url, dest_file_path, timeout=30, chunk_size=2 ** 20):
             f.write(chunk)
 
 
-def safe_relpath(path):
-    """
-    Given what we expect to be a relative path, determine whether the path
-    would exist inside the current directory.
-
-    :type   path:   string
-    :param  path:   a path to check
-    :rtype:         bool
-    :returns:       ``True`` if path is relative and does not reference a path
-        in a parent directory, ``False`` otherwise.
-    """
-    if path.startswith(os.sep) or normpath(path).startswith(os.pardir):
-        return False
-    return True
-
-
 class ExecutionTimer(object):
 
     def __init__(self):

lib/galaxy/util/path/__init__.py

@@ -0,0 +1,185 @@
+"""Path manipulation functions.
+"""
+from __future__ import absolute_import
+
+import errno
+import imp
+from functools import partial
+from itertools import starmap
+from operator import getitem
+from os import (
+    makedirs,
+    walk,
+)
+from os.path import (
+    abspath,
+    exists,
+    isabs,
+    join,
+    normpath,
+    pardir,
+    realpath,
+    relpath,
+)
+
+from six import string_types
+from six.moves import filterfalse, map, zip
+
+
+def safe_contains(prefix, path, whitelist=None):
+    """Ensure a path is contained within another path.
+
+    Given any two filesystem paths, ensure that ``path`` is contained in ``prefix``. If ``path`` exists (either as an
+    absolute path or relative to ``prefix``), it is canonicalized with :func:`os.path.realpath` to ensure it is not a
+    symbolic link that points outside of ``path``. If it is a symbolic link and ``whitelist`` is set, the symbolic link
+    may also point inside a ``whitelist`` path.
+
+    The ``path`` is checked against ``whitelist`` using either its absolute pathname (if passed in as absolute) or
+    relative to ``prefix`` and canonicalized (if applicable). It is *not* ``os.path.join()``ed with each ``whitelist``
+    directory.
+
+    :type prefix:       string
+    :param prefix:      a directory under which ``path`` is to be checked
+    :type path:         string
+    :param path:        a filename to check
+    :type whitelist:    list of strings
+    :param whitelist:   list of additional paths under which ``path`` may be located
+    :rtype:             bool
+    :returns:           ``True`` if ``path`` is contained within ``prefix`` or ``whitelist``, ``False`` otherwise.
+    """
+    return any(__contains(prefix, path, whitelist=whitelist))
+
+
+def safe_makedirs(path):
+    """Safely make a directory, do not fail if it already exists or is created during execution.
+
+    :type path:     string
+    :param path:    a directory to create
+    """
+    # prechecking for existence is faster than try/except
+    if not exists(path):
+        try:
+            makedirs(path)
+        except OSError as e:
+            # reviewing the source for Python 2.7, this would only ever happen for the last path element anyway so no
+            # need to recurse - this exception means the last part of the path was already in existence.
+            if e.errno != errno.EEXIST:
+                raise
+
+
+def safe_relpath(path):
+    """Determine whether a relative path references a path outside its root.
+
+    This is a path computation: the filesystem is not accessed to confirm the existence or nature of ``path``.
+
+    :type path:     string
+    :param path:    a path to check
+    :rtype:         bool
+    :returns:       ``True`` if path is relative and does not reference a path in a parent directory, ``False``
+                    otherwise.
+    """
+    return not (isabs(path) or normpath(path).startswith(pardir))
+
+
+def unsafe_walk(path, whitelist=None):
+    """Walk a path and ensure that none of its contents are symlinks outside the path.
+
+    It is assumed that ``path`` itself has already been validated e.g. with :func:`safe_relpath` or
+    :func:`safe_contains`.
+
+    :type path:         string
+    :param path:        a directory to check for unsafe contents
+    :type whitelist:    list of strings
+    :param whitelist:   list of additional paths under which contents may be located
+    :rtype:             iterator
+    :returns:           Iterator of "bad" files found under ``path``
+    """
+    return filterfalse(partial(safe_contains, path, whitelist=whitelist), __walk(abspath(path)))
+
+
+def __listify(item):
+    """A non-splitting version of :func:`galaxy.util.listify`.
+    """
+    if not item:
+        return []
+    elif isinstance(item, list) or isinstance(item, tuple):
+        return item
+    else:
+        return [item]
+
+
+# helpers
+
+
+def __walk(path):
+    for dirpath, dirnames, filenames in walk(path):
+        for name in dirnames + filenames:
+            yield join(dirpath, name)
+
+
+def __contains(prefix, path, whitelist=None):
+    real = realpath(join(prefix, path))
+    yield not relpath(real, prefix).startswith(pardir)
+    for wldir in whitelist or []:
+        yield not relpath(real, wldir).startswith(pardir)
+
+
+# cross-platform support
+
+
+def _build_self(target, path_module):
+    """Populate a module with the same exported functions as this module, but using the given os.path module.
+
+    :type target: module
+    :param target: module on which to set ``galaxy.util.path`` functions
+    :type path_module: ``ntpath`` or ``posixpath`` module
+    :param path_module: module implementing ``os.path`` API to use for path functions
+    """
+    __copy_self().__set_fxns_on(target, path_module)
+
+
+def __copy_self(names=__name__, parent=None):
+    """Returns a copy of this module that can be modified without modifying `galaxy.util.path`` in ``sys.modules``.
+    """
+    if isinstance(names, string_types):
+        names = iter(names.split('.'))
+    try:
+        name = names.next()
+    except StopIteration:
+        return parent
+    path = parent and parent.__path__
+    parent = imp.load_module(name, *imp.find_module(name, path))
+    return __copy_self(names, parent)
+
+
+def __set_fxns_on(target, path_module):
+    """Overrides imported os.path functions with the ones from path_module and populates target with the global
+    functions from this module.
+    """
+    for name in __pathfxns__:
+        globals()[name] = getattr(path_module, name)
+    __get = partial(getitem, globals())
+    __set = partial(setattr, target)
+    # this is actually izip(..., imap(...))
+    __fxns = zip(__all__, map(__get, __all__))
+    # list() to execute
+    list(starmap(__set, __fxns))
+
+
+__pathfxns__ = (
+    'abspath',
+    'exists',
+    'isabs',
+    'join',
+    'normpath',
+    'pardir',
+    'realpath',
+    'relpath',
+)
+
+__all__ = (
+    'safe_contains',
+    'safe_makedirs',
+    'safe_relpath',
+    'unsafe_walk',
+)

lib/galaxy/util/path/ntpath.py

@@ -0,0 +1,11 @@
+"""Galaxy "safe" path functions forced to work with Windows-style paths regardless of current platform
+"""
+from __future__ import absolute_import
+
+import ntpath
+import sys
+
+from . import _build_self
+
+
+_build_self(sys.modules[__name__], ntpath)

lib/galaxy/util/path/posixpath.py

@@ -0,0 +1,11 @@
+"""Galaxy "safe" path functions forced to work with POSIX-style paths regardless of current platform
+"""
+from __future__ import absolute_import
+
+import posixpath
+import sys
+
+from . import _build_self
+
+
+_build_self(sys.modules[__name__], posixpath)

lib/galaxy/webapps/galaxy/api/library_datasets.py

@@ -18,6 +18,7 @@ from galaxy.managers import base as managers_base
 from galaxy.managers import folders, library_datasets, roles
 from galaxy.tools.actions import upload_common
 from galaxy.tools.parameters import populate_state
+from galaxy.util.path import safe_contains, safe_relpath, unsafe_walk
 from galaxy.util.streamball import StreamBall
 from galaxy.web import _future_expose_api as expose_api
 from galaxy.web import _future_expose_api_anonymous as expose_api_anonymous
@@ -408,29 +409,45 @@ class LibraryDatasetsController(BaseAPIController, UsesVisualizationMixin):
         source = kwd.get('source', None)
         if source not in ['userdir_file', 'userdir_folder', 'importdir_file', 'importdir_folder', 'admin_path']:
             raise exceptions.RequestParameterMissingException('You have to specify "source" parameter. Possible values are "userdir_file", "userdir_folder", "admin_path", "importdir_file" and "importdir_folder". ')
-        if source in ['importdir_file', 'importdir_folder']:
-            if not trans.user_is_admin:
+        elif source in ['importdir_file', 'importdir_folder']:
+            if not trans.user_is_admin():
                 raise exceptions.AdminRequiredException('Only admins can import from importdir.')
             if not trans.app.config.library_import_dir:
                 raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to import into library from importdir.')
             import_base_dir = trans.app.config.library_import_dir
+            if not safe_relpath(path):
+                # admins shouldn't be able to explicitly specify a path outside server_dir, but symlinks are allowed.
+                # the reasoning here is that galaxy admins may not have direct filesystem access or can only access
+                # library_import_dir via FTP (which cannot create symlinks), and may rely on sysadmins to set up the
+                # import directory. if they have filesystem access, all bets are off.
+                raise exceptions.RequestParameterInvalidException( 'The given path is invalid.' )
             path = os.path.join(import_base_dir, path)
-        if source in ['userdir_file', 'userdir_folder']:
+        elif source in ['userdir_file', 'userdir_folder']:
+            unsafe = None
             user_login = trans.user.email
             user_base_dir = trans.app.config.user_library_import_dir
             if user_base_dir is None:
                 raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow upload from user directories.')
             full_dir = os.path.join(user_base_dir, user_login)
-            if not path.lower().startswith(full_dir.lower()):
+            if not safe_contains(full_dir, path, whitelist=trans.app.config.user_library_import_symlink_whitelist):
+                # the path is a symlink outside the user dir
                 path = os.path.join(full_dir, path)
+                log.error('User attempted to import a path that resolves to a path outside of their import dir: %s -> %s', path, os.path.realpath(path))
+                raise exceptions.RequestParameterInvalidException('The given path is invalid.')
+            path = os.path.join(full_dir, path)
+            for unsafe in unsafe_walk(path, whitelist=[full_dir] + trans.app.config.user_library_import_symlink_whitelist):
+                # the path is a dir and contains files that symlink outside the user dir
+                log.error('User attempted to import a directory containing a path that resolves to a path outside of their import dir: %s -> %s', unsafe, os.path.realpath(unsafe))
+            if unsafe:
+                raise exceptions.RequestParameterInvalidException('The given path is invalid.')
             if not os.path.exists(path):
                 raise exceptions.RequestParameterInvalidException('Given path does not exist on the host.')
             if not self.folder_manager.can_add_item(trans, folder):
                 raise exceptions.InsufficientPermissionsException('You do not have proper permission to add items to the given folder.')
-        if source == 'admin_path':
+        elif source == 'admin_path':
             if not trans.app.config.allow_library_path_paste:
                 raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to import into library from path.')
-            if not trans.user_is_admin:
+            if not trans.user_is_admin():
                 raise exceptions.AdminRequiredException('Only admins can import from path.')
 
         # Set up the traditional tool state/params
@@ -449,12 +466,12 @@ class LibraryDatasetsController(BaseAPIController, UsesVisualizationMixin):
         if source in ['importdir_folder']:
             kwd['filesystem_paths'] = os.path.join(import_base_dir, path)
         # user wants to import one file only
-        if source in ["userdir_file", "importdir_file"]:
+        elif source in ["userdir_file", "importdir_file"]:
             file = os.path.abspath(path)
             abspath_datasets.append(trans.webapp.controllers['library_common'].make_library_uploaded_dataset(
                 trans, 'api', kwd, os.path.basename(file), file, 'server_dir', library_bunch))
         # user wants to import whole folder
-        if source == "userdir_folder":
+        elif source == "userdir_folder":
             uploaded_datasets_bunch = trans.webapp.controllers['library_common'].get_path_paste_uploaded_datasets(
                 trans, 'api', kwd, library_bunch, 200, '')
             uploaded_datasets = uploaded_datasets_bunch[0]

lib/galaxy/webapps/galaxy/api/remote_files.py

@@ -107,7 +107,7 @@ class RemoteFilesAPIController(BaseAPIController):
         subfolders and returns a flat list.
         """
         response = []
-        if os.path.exists(directory):
+        if os.path.exists(directory) and not os.path.islink(directory):
             for (dirpath, dirnames, filenames) in os.walk(directory):
                 for filename in filenames:
                     path = os.path.relpath(os.path.join(dirpath, filename), directory)
@@ -130,7 +130,7 @@ class RemoteFilesAPIController(BaseAPIController):
         """
         userdir_jstree = None
         jstree_paths = []
-        if os.path.exists(directory):
+        if os.path.exists(directory) and not os.path.islink(directory):
             for (dirpath, dirnames, filenames) in os.walk(directory):
                 for dirname in dirnames:
                     dir_path = os.path.relpath(os.path.join(dirpath, dirname), directory)

lib/galaxy/webapps/galaxy/controllers/library.py

@@ -91,6 +91,7 @@ class Library(BaseUIController):
                                        'app': app})
 
     @web.expose
+    @web.require_admin
     def index(self, trans, **kwd):
         message = escape(kwd.get('message', ''))
         status = escape(kwd.get('status', 'done'))
@@ -101,6 +102,7 @@ class Library(BaseUIController):
                                    status=status)
 
     @web.expose
+    @web.require_admin
     def browse_libraries(self, trans, **kwd):
         if 'operation' in kwd:
             operation = kwd['operation'].lower()

lib/galaxy/webapps/galaxy/controllers/library_common.py

@@ -23,6 +23,7 @@ from galaxy.security import Action
 from galaxy.tools.actions import upload_common
 from galaxy.tools.parameters import populate_state
 from galaxy.util import inflector, unicodify, FILENAME_VALID_CHARS
+from galaxy.util.path import safe_contains, safe_relpath, unsafe_walk
 from galaxy.util.streamball import StreamBall
 from galaxy.web.base.controller import BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMetadataMixin, UsesLibraryMixinItems
 from galaxy.web.form_builder import AddressField, CheckboxField, SelectField, build_select_field
@@ -72,6 +73,7 @@ except:
 
 class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMetadataMixin, UsesLibraryMixinItems):
     @web.json
+    @web.require_admin
     def library_item_updates(self, trans, ids=None, states=None):
         # Avoid caching
         trans.response.headers['Pragma'] = 'no-cache'
@@ -95,6 +97,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
         return rval
 
     @web.expose
+    @web.require_admin
     def browse_library(self, trans, cntrller='library', **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -167,6 +170,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                                         status=status))
 
     @web.expose
+    @web.require_admin
     def library_info(self, trans, cntrller, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -226,6 +230,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def library_permissions(self, trans, cntrller, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -273,6 +278,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def create_folder(self, trans, cntrller, parent_id, library_id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -348,6 +354,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def folder_info(self, trans, cntrller, id, library_id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -404,6 +411,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def folder_permissions(self, trans, cntrller, id, library_id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -454,6 +462,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def ldda_edit_info(self, trans, cntrller, library_id, folder_id, id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -605,6 +614,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def ldda_info(self, trans, cntrller, library_id, folder_id, id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -654,6 +664,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def ldda_permissions(self, trans, cntrller, library_id, folder_id, id, **kwd):
         message = str(escape(kwd.get('message', '')))
         status = kwd.get('status', 'done')
@@ -792,6 +803,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def upload_library_dataset(self, trans, cntrller, library_id, folder_id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -851,6 +863,9 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
             elif upload_option == 'upload_paths' and not is_admin:
                 error = True
                 message = 'Uploading files via filesystem paths can only be performed by administrators'
+            elif upload_option not in ('upload_file', 'upload_directory', 'upload_paths'):
+                error = True
+                message = 'Invalid upload_option'
             elif roles:
                 # Check to see if the user selected roles to associate with the DATASET_ACCESS permission
                 # on the dataset that would cause accessibility issues.
@@ -1066,17 +1081,26 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
         if upload_option == 'upload_directory':
             if server_dir in [None, 'None', '']:
                 response_code = 400
-            if cntrller == 'library_admin' or (cntrller == 'api' and trans.user_is_admin):
+            if trans.user_is_admin() and cntrller in ('library_admin', 'api'):
                 import_dir = trans.app.config.library_import_dir
                 import_dir_desc = 'library_import_dir'
-                full_dir = os.path.join(import_dir, server_dir)
             else:
                 import_dir = trans.app.config.user_library_import_dir
+                if server_dir != trans.user.email:
+                    import_dir = os.path.join(import_dir, trans.user.email)
                 import_dir_desc = 'user_library_import_dir'
-                if server_dir == trans.user.email:
-                    full_dir = os.path.join(import_dir, server_dir)
-                else:
-                    full_dir = os.path.join(import_dir, trans.user.email, server_dir)
+            full_dir = os.path.join(import_dir, server_dir)
+            unsafe = None
+            if safe_relpath(server_dir):
+                if import_dir_desc == 'user_library_import_dir' and safe_contains(import_dir, full_dir, whitelist=trans.app.config.user_library_import_symlink_whitelist):
+                    for unsafe in unsafe_walk(full_dir, whitelist=[import_dir] + trans.app.config.user_library_import_symlink_whitelist):
+                        log.error('User attempted to import a path that resolves to a path outside of their import dir: %s -> %s', unsafe, os.path.realpath(unsafe))
+            else:
+                log.error('User attempted to import a directory path that resolves to a path outside of their import dir: %s -> %s', server_dir, os.path.realpath(full_dir))
+                unsafe = True
+            if unsafe:
+                response_code = 403
+                message = 'Invalid server_dir'
             if import_dir:
                 message = 'Select a directory'
             else:
@@ -1270,6 +1294,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
         return None
 
     @web.expose
+    @web.require_admin
     def add_history_datasets_to_library(self, trans, cntrller, library_id, folder_id, hda_ids='', **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -1512,6 +1537,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
         return upload_option_select_list
 
     @web.expose
+    @web.require_admin
     def download_dataset_from_folder(self, trans, cntrller, id, library_id=None, **kwd):
         """Catches the dataset id and displays file contents as directed"""
         show_deleted = util.string_as_bool(kwd.get('show_deleted', False))
@@ -1551,6 +1577,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                                         status='error'))
 
     @web.expose
+    @web.require_admin
     def library_dataset_info(self, trans, cntrller, id, library_id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -1599,6 +1626,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def library_dataset_permissions(self, trans, cntrller, id, library_id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -1647,6 +1675,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def make_library_item_public(self, trans, cntrller, library_id, item_type, id, **kwd):
         message = escape(kwd.get('message', ''))
         status = kwd.get('status', 'done')
@@ -1689,6 +1718,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                                         status=status))
 
     @web.expose
+    @web.require_admin
     def act_on_multiple_datasets(self, trans, cntrller, library_id=None, ldda_ids='', **kwd):
         # This method is called from 1 of 3 places:
         # - this controller's download_dataset_from_folder() method
@@ -1981,6 +2011,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                        status=escape(status))
 
     @web.expose
+    @web.require_admin
     def import_datasets_to_histories(self, trans, cntrller, library_id='', folder_id='', ldda_ids='', target_history_id='', target_history_ids='', new_history_name='', **kwd):
         # This method is called from one of the following places:
         # - a menu option for a library dataset ( ldda_ids is a single ldda id )
@@ -2111,6 +2142,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def manage_template_inheritance(self, trans, cntrller, item_type, library_id, folder_id=None, ldda_id=None, **kwd):
         show_deleted = util.string_as_bool(kwd.get('show_deleted', False))
         use_panels = util.string_as_bool(kwd.get('use_panels', False))
@@ -2156,6 +2188,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                                         status='done'))
 
     @web.expose
+    @web.require_admin
     def move_library_item(self, trans, cntrller, item_type, item_id, source_library_id='', make_target_current=True, **kwd):
         # This method is called from one of the following places:
         # - a menu option for a library dataset ( item_type is 'ldda' and item_id is a single ldda id )
@@ -2373,6 +2406,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                    status=escape(status))
 
     @web.expose
+    @web.require_admin
     def delete_library_item(self, trans, cntrller, library_id, item_id, item_type, **kwd):
         # This action will handle deleting all types of library items.  State is saved for libraries and
         # folders ( i.e., if undeleted, the state of contents of the library or folder will remain, so previously
@@ -2441,6 +2475,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
                                                             status=status))
 
     @web.expose
+    @web.require_admin
     def undelete_library_item(self, trans, cntrller, library_id, item_id, item_type, **kwd):
         # This action will handle undeleting all types of library items
         status = kwd.get('status', 'done')

lib/tool_shed/managers/groups.py

@@ -56,7 +56,7 @@ class GroupManager(object):
         """
         Create a new group.
         """
-        if not trans.user_is_admin:
+        if not trans.user_is_admin():
             raise ItemAccessibilityException('Only administrators can create groups.')
         else:
             if self.get(trans, name=name):