diff --git a/doc/source/releases/18.01_announce.rst b/doc/source/releases/18.01_announce.rst index 5cd42a4125d..c6b00ecff6d 100644 --- a/doc/source/releases/18.01_announce.rst +++ b/doc/source/releases/18.01_announce.rst @@ -86,6 +86,21 @@ See the `community hub `__ for a Security ======== +Unsecure GenomeSpace token exposure +----------------------------------- +Tracked as ``GX-2018-0002``. + +We have found and fixed a medium-level security issue concering the GenomeSpace importer/exporter tools that were updated in the Galaxy release 17.09. These tools did not handle the GenomeSpace access token securely and stored it as a job parameter which made it accessible to anybody with access to the datasets created by these tools. +This means that any user that used a GenomeSpace token to access these tools and subsequently shared the output dataset (or history that contains it) with another user shared their GenomeSpace token also. + +These tools are both included in the ``tool_conf.xml.sample`` and are therefore *enabled on every new Galaxy by default*. + +Administrators please see the `GenomeSpace security sanitization`_ section of this document for the details on how to sanitize the tokens stored in the Galaxy database created prior to this fix. + +The vulnerability has been resolved by removing the token functionality until a proper implementation is in place. The GenomeSpace tools continue to work using the OpenID authentication as before. + +The fix for this issue has been applied back to Galaxy release 17.09 and can be found in this `pull request `__. + Breaking Changes ================ @@ -140,8 +155,74 @@ Release Notes .. include:: 18.01.rst :start-after: announce_start -Security patch details -====================== +GenomeSpace security sanitization +================================= + +Outputs of these tools may require sanitization: ``genomespace_importer`` and ``genomespace_exporter`` + +The following SQL commands will help you **identify** the datasets in your Galaxy's database. + +Finding bad GenomeSpace importer params: + +.. code-block:: sql + + SELECT j.id, + j.create_time, + j.user_id, + jp.value + FROM job_parameter jp + JOIN job j ON j.id = jp.job_id + WHERE jp.job_id IN + (SELECT id + FROM job + WHERE tool_id = 'genomespace_importer') + AND jp.name = 'URL' + AND jp.value LIKE '%^%' + ORDER BY j.id DESC; + +Finding bad GenomeSpace exporter params: + +.. code-block:: sql + + SELECT j.id, + j.create_time, + j.user_id, + jp.value + FROM job_parameter jp + JOIN job j ON j.id = jp.job_id + WHERE jp.job_id IN + (SELECT id + FROM job + WHERE tool_id = 'genomespace_exporter') + AND jp.name = 'genomespace_browser' + AND jp.value LIKE '%^%' + ORDER BY j.id DESC; + +The following SQL commands will help you **sanitize** the datasets in your Galaxy's database. + +Sanitizing GenomeSpace importer params: + +.. code-block:: sql + + UPDATE job_parameter jp + SET value = split_part(jp.value, '^', 1) || '"' + FROM job j + WHERE jp.job_id = j.id + AND j.tool_id = 'genomespace_importer' + AND jp.name = 'URL' + AND jp.value LIKE '%^%'; + +Sanitizing GenomeSpace exporter params: + +.. code-block:: sql + + UPDATE job_parameter jp + SET value = split_part(jp.value, '^', 1) || '"' + FROM job j + WHERE jp.job_id = j.id + AND j.tool_id = 'genomespace_exporter' + AND jp.name = 'genomespace_browser' + AND jp.value LIKE '%^%'; .. include:: _thanks.rst diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py index 5f8c5c06ef6..7a60a1aa0fe 100644 --- a/lib/galaxy/managers/libraries.py +++ b/lib/galaxy/managers/libraries.py @@ -139,10 +139,10 @@ class LibraryManager(object): library_add_action = trans.app.security_agent.permitted_actions.LIBRARY_ADD.action library_modify_action = trans.app.security_agent.permitted_actions.LIBRARY_MODIFY.action library_manage_action = trans.app.security_agent.permitted_actions.LIBRARY_MANAGE.action - accessible_restricted_library_ids = [] - allowed_library_add_ids = set([]) - allowed_library_modify_ids = set([]) - allowed_library_manage_ids = set([]) + accessible_restricted_library_ids = set() + allowed_library_add_ids = set() + allowed_library_modify_ids = set() + allowed_library_manage_ids = set() for action in all_actions: if action.action == library_access_action: accessible_restricted_library_ids.add(action.library_id) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 67b8dfdf03a..005de46f6e5 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4073,7 +4073,7 @@ class WorkflowInvocation(UsesCreateAndUpdateTime, Dictifiable): output_assoc.dataset_collection = output_object self.output_dataset_collections.append(output_assoc) else: - raise Exception("Uknown output type encountered") + raise Exception("Unknown output type encountered") def to_dict(self, view='collection', value_mapper=None, step_details=False): rval = super(WorkflowInvocation, self).to_dict(view=view, value_mapper=value_mapper) @@ -4197,7 +4197,7 @@ class WorkflowInvocationStep(Dictifiable): output_assoc.output_name = output_name self.output_dataset_collections.append(output_assoc) else: - raise Exception("Uknown output type encountered") + raise Exception("Unknown output type encountered") @property def jobs(self): diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 3d257c174e3..51717788ca6 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -7,7 +7,8 @@ from six import string_types from galaxy import model from galaxy.exceptions import ObjectInvalid -from galaxy.model import LibraryDatasetDatasetAssociation +from galaxy.jobs.actions.post import ActionBox +from galaxy.model import LibraryDatasetDatasetAssociation, WorkflowRequestInputParameter from galaxy.tools.parameters import update_dataset_ids from galaxy.tools.parameters.basic import DataCollectionToolParameter, DataToolParameter, RuntimeValue from galaxy.tools.parameters.wrapped import WrappedParameters @@ -36,7 +37,8 @@ class ToolAction(object): been converted and validated). """ - def execute(self, tool, trans, incoming={}, set_output_hid=True): + def execute(self, tool, trans, incoming=None, set_output_hid=True): + incoming = incoming or {} raise TypeError("Abstract method") @@ -213,12 +215,13 @@ class DefaultToolAction(object): return history, inp_data, inp_dataset_collections, preserved_tags - def execute(self, tool, trans, incoming={}, return_job=False, set_output_hid=True, history=None, job_params=None, rerun_remap_job_id=None, execution_cache=None, dataset_collection_elements=None, completed_job=None): + def execute(self, tool, trans, incoming=None, return_job=False, set_output_hid=True, history=None, job_params=None, rerun_remap_job_id=None, execution_cache=None, dataset_collection_elements=None, completed_job=None): """ Executes a tool, creating job and tool outputs, associating them, and submitting the job to the job queue. If history is not specified, use trans.history as destination for tool's output datasets. """ + incoming = incoming or {} self._check_access(tool, trans) app = trans.app if execution_cache is None: @@ -491,7 +494,6 @@ class DefaultToolAction(object): rerun_remap_job_id=rerun_remap_job_id, current_job=job, out_data=out_data) - log.info("Setup for job %s complete, ready to flush %s" % (job.log_str(), job_setup_timer)) job_flush_timer = ExecutionTimer() @@ -544,6 +546,15 @@ class DefaultToolAction(object): # Duplicate PJAs before remap. for pjaa in old_job.post_job_actions: current_job.add_post_job_action(pjaa.post_job_action) + if old_job.workflow_invocation_step: + replacement_dict = {} + for parameter in old_job.workflow_invocation_step.workflow_invocation.input_parameters: + if parameter.type == WorkflowRequestInputParameter.types.REPLACEMENT_PARAMETERS: + replacement_dict[parameter.name] = parameter.value + for pja in old_job.workflow_invocation_step.workflow_step.post_job_actions: + # execute immediate actions here, with workflow context. + if pja.action_type in ActionBox.immediate_actions: + ActionBox.execute(trans.app, trans.sa_session, pja, current_job, replacement_dict) for p in old_job.parameters: if p.name.endswith('|__identifier__'): current_job.parameters.append(p.copy()) @@ -598,7 +609,7 @@ class DefaultToolAction(object): def _get_on_text(self, inp_data): input_names = [] - for name, data in reversed(inp_data.items()): + for data in reversed(inp_data.values()): if getattr(data, "hid", None): input_names.append('data %s' % data.hid) @@ -782,7 +793,7 @@ class OutputCollections(object): if "elements" in element_kwds: elements = element_kwds["elements"] if hasattr(elements, "items"): # else it is ELEMENTS_UNINITIALIZED object. - for key, value in elements.items(): + for value in elements.values(): # Either a HDA (if) or a DatasetCollection (the else) if getattr(value, "history_content_type", None) == "dataset": assert value.history is not None diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index a03c317290f..e3655429751 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -43,6 +43,8 @@ workflow_building_modes = Bunch(DISABLED=False, ENABLED=True, USE_HISTORY=1) WORKFLOW_PARAMETER_REGULAR_EXPRESSION = re.compile('''\$\{.+?\}''') +MAX_DEFAULT_COLUMNS = 999 + def contains_workflow_parameter(value, search=False): if not isinstance(value, string_types): @@ -1114,18 +1116,21 @@ class ColumnListParameter(SelectToolParameter): if isinstance(dataset, trans.app.model.HistoryDatasetCollectionAssociation): dataset = dataset.to_hda_representative() # Columns can only be identified if metadata is available - if not hasattr(dataset, 'metadata') or not hasattr(dataset.metadata, 'columns') or not dataset.metadata.columns: + if not hasattr(dataset, 'metadata') or not hasattr(dataset.metadata, 'columns'): return [] # Build up possible columns for this dataset this_column_list = [] - if self.numerical: + # Valid column-based datasets contain at least 1 column if that column has not been + # specified we prepopulate the selector assuming that the datasets is not ready yet. + if dataset.metadata.columns is None: + this_column_list = [str(i) for i in range(1, MAX_DEFAULT_COLUMNS + 1)] + elif self.numerical: # If numerical was requested, filter columns based on metadata for i, col in enumerate(dataset.metadata.column_types): if col == 'int' or col == 'float': this_column_list.append(str(i + 1)) else: - for i in range(0, dataset.metadata.columns): - this_column_list.append(str(i + 1)) + this_column_list = [str(i) for i in range(1, dataset.metadata.columns + 1)] # Take the intersection of these columns with the other columns. if column_list is None: column_list = this_column_list diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index fca6a3e199a..6d67d332146 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -44,15 +44,15 @@ class WorkflowRunConfig(object): def __init__(self, target_history, replacement_dict, copy_inputs_to_history=False, - inputs={}, - param_map={}, + inputs=None, + param_map=None, allow_tool_state_corrections=False, use_cached_job=False): self.target_history = target_history self.replacement_dict = replacement_dict self.copy_inputs_to_history = copy_inputs_to_history - self.inputs = inputs - self.param_map = param_map + self.inputs = inputs or {} + self.param_map = param_map or {} self.allow_tool_state_corrections = allow_tool_state_corrections self.use_cached_job = use_cached_job @@ -168,7 +168,8 @@ def _flatten_step_params(param_dict, prefix=""): return new_params -def _get_target_history(trans, workflow, payload, param_keys=[], index=0): +def _get_target_history(trans, workflow, payload, param_keys=None, index=0): + param_keys = param_keys or [] history_name = payload.get('new_history_name', None) history_id = payload.get('history_id', None) history_param = payload.get('history', None) diff --git a/scripts/common_startup.sh b/scripts/common_startup.sh index 6c460f4a69b..a123df666c1 100755 --- a/scripts/common_startup.sh +++ b/scripts/common_startup.sh @@ -197,10 +197,8 @@ if [ $SKIP_CLIENT_BUILD -eq 0 ]; then if [ "$GIT_BRANCH" = "0" ]; then SKIP_CLIENT_BUILD=1 else - # Compare hash. - githash=$(git rev-parse HEAD) - statichash=$(cat static/client_build_hash.txt) - if [ "$githash" = "$statichash" ]; then + # Check if anything has changed in client/ since the last build + if git diff --quiet $(cat static/client_build_hash.txt) -- client/; then SKIP_CLIENT_BUILD=1 else echo "The Galaxy client is out of date and will be built now." diff --git a/test/api/test_workflows.py b/test/api/test_workflows.py index bfdb59997e3..f40fad13620 100644 --- a/test/api/test_workflows.py +++ b/test/api/test_workflows.py @@ -2142,6 +2142,55 @@ test_data: name = content["name"] assert name == "fasta1 suffix", name + @skip_without_tool("fail_identifier") + @skip_without_tool("cat") + def test_run_rename_when_resuming_jobs(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(""" +class: GalaxyWorkflow +inputs: + - id: input1 +steps: + - tool_id: fail_identifier + label: first_fail + state: + failbool: true + input1: + $link: input1 + outputs: + out_file1: + rename: "cat1 out" + - tool_id: cat + state: + input1: + $link: first_fail#out_file1 + outputs: + out_file1: + rename: "#{input1} suffix" +test_data: + input1: + value: 1.fasta + type: File + name: fail +""", history_id=history_id, wait=True, assert_ok=False) + content = self.dataset_populator.get_history_dataset_details(history_id, hid=2, wait=True, assert_ok=False) + name = content["name"] + assert content['state'] == 'error', content + input1 = self.dataset_populator.get_history_dataset_details(history_id, hid=1, wait=True, assert_ok=False) + job_id = content['creating_job'] + inputs = {"input1": {'values': [{'src': 'hda', + 'id': input1['id']}] + }, + "failbool": "false", + "rerun_remap_job_id": job_id} + self.dataset_populator.run_tool(tool_id='fail_identifier', + inputs=inputs, + history_id=history_id, + assert_ok=True) + unpaused_dataset = self.dataset_populator.get_history_dataset_details(history_id, wait=True, assert_ok=False) + assert unpaused_dataset['state'] == 'ok' + assert unpaused_dataset['name'] == "%s suffix" % name + @skip_without_tool("cat") def test_run_rename_based_on_input_recursive(self): history_id = self.dataset_populator.new_history()