From 20f61e6a2f36967e16a92cbcf8ad7f81ca76fb29 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Tue, 16 Feb 2016 14:19:38 +0100 Subject: [PATCH 1/6] workaround for error message in case original toolshed had been disabled --- lib/galaxy/workflow/modules.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index acce6c7320a..8700023edac 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -862,6 +862,8 @@ class ToolModule( WorkflowModule ): old_tool_shed = step.tool_id.split( "/repos/" )[0] if old_tool_shed not in tool_id: # Only display the following warning if the tool comes from a different tool shed old_tool_shed_url = common_util.get_tool_shed_url_from_tool_shed_registry( trans.app, old_tool_shed ) + if not old_tool_shed_url: # a tool from a different tool_shed has been found, but the original tool shed has been deactivated + old_tool_shed_url = "http://" + old_tool_shed # let's just assume it's either http, or a http is forwarded to https. old_url = old_tool_shed_url + "/view/%s/%s/" % (module.tool.repository_owner, module.tool.repository_name) new_url = module.tool.tool_shed_repository.get_sharable_url( module.tool.app ) + '/%s/' % module.tool.tool_shed_repository.changeset_revision new_tool_shed_url = new_url.split( "/view" )[0] From 2a53be67ad4ac99b6e9fe5fb803bfa44442dbd3c Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 5 Apr 2016 10:41:41 -0400 Subject: [PATCH 2/6] Show sections in workflow run --- templates/webapps/galaxy/workflow/run.mako | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/templates/webapps/galaxy/workflow/run.mako b/templates/webapps/galaxy/workflow/run.mako index 2e499621ec0..b98ee6df65f 100644 --- a/templates/webapps/galaxy/workflow/run.mako +++ b/templates/webapps/galaxy/workflow/run.mako @@ -442,7 +442,12 @@ if wf_parms: ${row_for_param( input.test_param, group_values[ input.test_param.name ], other_values, group_errors, prefix, step, already_used )} ${do_inputs( input.cases[ current_case ].inputs, group_values, group_errors, new_prefix, step, other_values, already_used )} - %elif input.type != "section": + %elif input.type == "section": + <% group_values = values[input.name] %> + <% new_prefix = prefix + input.name + "|" %> + <% group_errors = errors.get( input.name, {} ) %> + ${do_inputs( input.inputs, group_values, group_errors, new_prefix, step, other_values, already_used )} + %else: ${row_for_param( input, values[ input.name ], other_values, errors, prefix, step, already_used )} %endif %endfor From 3002e404ef17cd7db9b817085b2f3ed5a82bd109 Mon Sep 17 00:00:00 2001 From: Eric Rasche Date: Tue, 5 Apr 2016 16:32:35 +0000 Subject: [PATCH 3/6] Patch in visual separation of section parameters --- templates/webapps/galaxy/workflow/run.mako | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/templates/webapps/galaxy/workflow/run.mako b/templates/webapps/galaxy/workflow/run.mako index b98ee6df65f..5aae623e7f7 100644 --- a/templates/webapps/galaxy/workflow/run.mako +++ b/templates/webapps/galaxy/workflow/run.mako @@ -446,7 +446,12 @@ if wf_parms: <% group_values = values[input.name] %> <% new_prefix = prefix + input.name + "|" %> <% group_errors = errors.get( input.name, {} ) %> +
${input.title}:
+
+
${do_inputs( input.inputs, group_values, group_errors, new_prefix, step, other_values, already_used )} +
+
%else: ${row_for_param( input, values[ input.name ], other_values, errors, prefix, step, already_used )} %endif From 9ecf99c227466a586a6299e37555655f5ff79a25 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 6 Apr 2016 13:55:32 -0400 Subject: [PATCH 4/6] make dependencies browsable again it would be denied previously as the dependency folder is outside of the repository (symlink protection) --- .../galaxy/controllers/admin_toolshed.py | 4 +- .../tool_shed/controllers/repository.py | 3 +- lib/tool_shed/util/shed_util_common.py | 44 ++++++++++++++----- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py b/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py index edabdf85425..667f57be44f 100644 --- a/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py +++ b/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py @@ -392,7 +392,7 @@ class AdminToolshed( AdminGalaxy ): # Avoid caching trans.response.headers['Pragma'] = 'no-cache' trans.response.headers['Expires'] = '0' - return suc.get_repository_file_contents( trans.app, file_path, repository_id ) + return suc.get_repository_file_contents( trans.app, file_path, repository_id, is_admin=True ) @web.expose @web.require_admin @@ -920,7 +920,7 @@ class AdminToolshed( AdminGalaxy ): # Avoid caching trans.response.headers['Pragma'] = 'no-cache' trans.response.headers['Expires'] = '0' - return suc.open_repository_files_folder( trans.app, folder_path, repository_id ) + return suc.open_repository_files_folder( trans.app, folder_path, repository_id, is_admin=True ) @web.expose @web.require_admin diff --git a/lib/galaxy/webapps/tool_shed/controllers/repository.py b/lib/galaxy/webapps/tool_shed/controllers/repository.py index 725c694de47..cf8ac09ea7f 100644 --- a/lib/galaxy/webapps/tool_shed/controllers/repository.py +++ b/lib/galaxy/webapps/tool_shed/controllers/repository.py @@ -1471,7 +1471,8 @@ class RepositoryController( BaseUIController, ratings_util.ItemRatings ): # Avoid caching trans.response.headers['Pragma'] = 'no-cache' trans.response.headers['Expires'] = '0' - return suc.get_repository_file_contents( trans.app, file_path, repository_id ) + is_admin = trans.is_admin + return suc.get_repository_file_contents( trans.app, file_path, repository_id, is_admin ) @web.expose def get_functional_test_rss( self, trans, **kwd ): diff --git a/lib/tool_shed/util/shed_util_common.py b/lib/tool_shed/util/shed_util_common.py index feb00b9a7aa..c14a4c88a47 100644 --- a/lib/tool_shed/util/shed_util_common.py +++ b/lib/tool_shed/util/shed_util_common.py @@ -598,13 +598,13 @@ def get_repository_for_dependency_relationship( app, tool_shed, name, owner, cha return repository -def get_repository_file_contents( app, file_path, repository_id ): +def get_repository_file_contents( app, file_path, repository_id, is_admin=False ): """Return the display-safe contents of a repository file for display in a browser.""" safe_str = '' - if not is_path_within_repo( app, file_path, repository_id ): + if not is_path_browsable( app, file_path, repository_id, is_admin ): log.warning( 'Request tries to access a file outside of the repository location. File path: %s', file_path ) return 'Invalid file path' - # Symlink targets are checked by is_path_within_repo + # Symlink targets are checked by is_path_browsable if os.path.islink( file_path ): safe_str = 'link to: ' + basic_util.to_html_string( os.readlink( file_path ) ) return safe_str @@ -1112,14 +1112,13 @@ def is_tool_shed_client( app ): return hasattr( app, "install_model" ) -def open_repository_files_folder( app, folder_path, repository_id ): +def open_repository_files_folder( app, folder_path, repository_id, is_admin=False ): """ Return a list of dictionaries, each of which contains information for a file or directory contained within a directory in a repository file hierarchy. """ - # Symlink targets are checked by is_path_within_repo - if not is_path_within_repo( app, folder_path, repository_id ): - log.warning( 'Request tries to access a folder outside of the repository location. Folder path: %s', folder_path ) + if not is_path_browsable( app, folder_path, repository_id, is_admin ): + log.warning( 'Request tries to access a folder outside of the allowed locations. Folder path: %s', folder_path ) return [] try: files_list = get_repository_files( folder_path ) @@ -1132,11 +1131,11 @@ def open_repository_files_folder( app, folder_path, repository_id ): is_folder = False full_path = os.path.join( folder_path, filename ) is_link = os.path.islink( full_path ) - path_is_within_repo = is_path_within_repo( app, full_path, repository_id ) - if is_link and not path_is_within_repo: + path_is_browsable = is_path_browsable( app, full_path, repository_id ) + if is_link and not path_is_browsable: log.warning( 'Valid folder contains a symlink outside of the repository location. Link found in: ' + str( full_path ) ) if filename: - if os.path.isdir( full_path ) and path_is_within_repo: + if os.path.isdir( full_path ) and path_is_browsable: # Append a '/' character so that our jquery dynatree will function properly. filename = '%s/' % filename full_path = '%s/' % full_path @@ -1150,9 +1149,18 @@ def open_repository_files_folder( app, folder_path, repository_id ): return folder_contents +def is_path_browsable( app, path, repository_id, is_admin=False ): + allowed = False + if is_admin and is_path_within_dependency_dir( app, path ): + allowed = True + if not allowed: + allowed = is_path_within_repo( app, path, repository_id) + return allowed + + def is_path_within_repo( app, path, repository_id ): """ - Detect whether the given path is within the repository folde ron the disk. + Detect whether the given path is within the repository folder on the disk. Use to filter malicious symlinks targeting outside paths. """ repo_path = os.path.abspath( get_repository_by_id( app, repository_id ).repo_path( app ) ) @@ -1160,6 +1168,20 @@ def is_path_within_repo( app, path, repository_id ): return os.path.commonprefix( [ repo_path, resolved_path ] ) == repo_path +def is_path_within_dependency_dir( app, path ): + """ + Detect whether the given path is within the tool_dependency_dir folder on the disk. + (Specified by the config option). Use to filter malicious symlinks targeting outside paths. + """ + allowed = False + resolved_path = os.path.realpath( path ) + tool_dependency_dir = app.config.get( 'tool_dependency_dir', None ) + if tool_dependency_dir: + dependency_path = os.path.abspath( tool_dependency_dir ) + allowed = os.path.commonprefix( [ dependency_path, resolved_path ] ) == dependency_path + return allowed + + def repository_was_previously_installed( app, tool_shed_url, repository_name, repo_info_tuple, from_tip=False ): """ Find out if a repository is already installed into Galaxy - there are several scenarios where this From 0882b4d8fb31752f1fa8c963c3f260f975c918f7 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 6 Apr 2016 15:32:52 -0400 Subject: [PATCH 5/6] use proper method for detecting admin --- lib/galaxy/webapps/tool_shed/controllers/repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/tool_shed/controllers/repository.py b/lib/galaxy/webapps/tool_shed/controllers/repository.py index cf8ac09ea7f..42399a9ac4b 100644 --- a/lib/galaxy/webapps/tool_shed/controllers/repository.py +++ b/lib/galaxy/webapps/tool_shed/controllers/repository.py @@ -1471,7 +1471,7 @@ class RepositoryController( BaseUIController, ratings_util.ItemRatings ): # Avoid caching trans.response.headers['Pragma'] = 'no-cache' trans.response.headers['Expires'] = '0' - is_admin = trans.is_admin + is_admin = trans.user_is_admin() return suc.get_repository_file_contents( trans.app, file_path, repository_id, is_admin ) @web.expose From 50268cb8ca4a5f812346a71cc640141781f22a3d Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Thu, 7 Apr 2016 18:22:42 -0400 Subject: [PATCH 6/6] fix missing call; add docs and refactor a bit --- .../webapps/tool_shed/controllers/repository.py | 3 ++- lib/tool_shed/util/shed_util_common.py | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/webapps/tool_shed/controllers/repository.py b/lib/galaxy/webapps/tool_shed/controllers/repository.py index 42399a9ac4b..e957d5f59e9 100644 --- a/lib/galaxy/webapps/tool_shed/controllers/repository.py +++ b/lib/galaxy/webapps/tool_shed/controllers/repository.py @@ -2441,7 +2441,8 @@ class RepositoryController( BaseUIController, ratings_util.ItemRatings ): # Avoid caching trans.response.headers['Pragma'] = 'no-cache' trans.response.headers['Expires'] = '0' - return suc.open_repository_files_folder( trans.app, folder_path, repository_id ) + is_admin = trans.user_is_admin() + return suc.open_repository_files_folder( trans.app, folder_path, repository_id, is_admin ) @web.expose def preview_tools_in_changeset( self, trans, repository_id, **kwd ): diff --git a/lib/tool_shed/util/shed_util_common.py b/lib/tool_shed/util/shed_util_common.py index c14a4c88a47..9cb8abba25e 100644 --- a/lib/tool_shed/util/shed_util_common.py +++ b/lib/tool_shed/util/shed_util_common.py @@ -1150,12 +1150,14 @@ def open_repository_files_folder( app, folder_path, repository_id, is_admin=Fals def is_path_browsable( app, path, repository_id, is_admin=False ): - allowed = False + """ + Detects whether the given path is browsable i.e. is within the + allowed repository folders. Admins can additionaly browse folders + with tool dependencies. + """ if is_admin and is_path_within_dependency_dir( app, path ): - allowed = True - if not allowed: - allowed = is_path_within_repo( app, path, repository_id) - return allowed + return True + return is_path_within_repo( app, path, repository_id) def is_path_within_repo( app, path, repository_id ):