Fix HDA leak in tool form for data managers (#22674)

When a user already has a ``data_manager_json`` bundle in their history
for a given tool data table, ``DynamicOptions.get_user_options`` builds
a synthetic options row from that bundle.

A 2025-02-18 refactor (172ef05f26, "Skip bundles that don't match data
table definition") moved ``field_entry.append(hda)`` from the end of the
row-building loop to the beginning. That shifted every declared column
index by one and put the HDA at position 0. ``get_options`` then reads
``fields[self.columns[<col>]]`` to find the option's name / value and
expects the HDA at ``fields[-1]`` — so it ends up wiring the raw HDA
into the option's ``value`` field. The unserialized HDA then crashed
``/api/tools/{id}/build`` for the data manager's own form with:

  TypeError: Object of type HistoryDatasetAssociation is not JSON serializable

Move the HDA append back after the column loop where get_options
expects it.

Regression covered by a new API test in a ``require_admin_user=True``
test class that runs ``data_manager_select`` in bundle mode to produce a
real ``data_manager_json`` bundle, then calls the build endpoint and
asserts the synthetic option is present.
This commit is contained in:
mvdbeek
2026-05-20 09:01:49 +02:00
parent 3cf9934932
commit 56417baef9
4 changed files with 82 additions and 2 deletions
@@ -873,8 +873,6 @@ class DynamicOptions:
by_dbkey.update(table_entries)
for data_table_entry in by_dbkey.values():
field_entry = []
if hda := data_table_entry.get("__hda__"):
field_entry.append(hda)
missing_columns = False
for column_key in self.tool_data_table.columns.keys():
if column_key not in data_table_entry:
@@ -885,6 +883,13 @@ class DynamicOptions:
break
field_entry.append(data_table_entry[column_key])
if not missing_columns:
# The HDA must be appended after the columns: get_options()
# reads it from ``fields[-1]`` and indexes the other columns
# by their declared positions. Prepending shifts every
# column by one and leaks the HDA into the option ``value``
# (#22674).
if hda := data_table_entry.get("__hda__"):
field_entry.append(hda)
fields.append(field_entry)
return fields
+46
View File
@@ -3694,5 +3694,51 @@ class TestToolsApi(ApiTestCase, TestsTools):
yield other_history_id
class TestDataManagerToolsApi(ApiTestCase, TestsTools):
"""API tests that need the test case to act as an admin (e.g. data managers)."""
require_admin_user = True
dataset_populator: DatasetPopulator
def setUp(self):
super().setUp()
self.dataset_populator = DatasetPopulator(self.galaxy_interactor)
self.dataset_collection_populator = DatasetCollectionPopulator(self.galaxy_interactor)
def test_build_does_not_leak_hda_from_user_bundle(self):
# Regression for https://github.com/galaxyproject/galaxy/issues/22674
# When the user already has a ``data_manager_json`` bundle in their
# history for a tool data table, ``DynamicOptions.get_user_options``
# builds a synthetic option row from it. A 2025-02-18 refactor
# (172ef05f269) prepended the bundle's HDA to that row, shifting every
# declared column index by one and leaking the raw HDA into the
# option's ``value`` field. ``/api/tools/{id}/build`` then 500s with
# TypeError: Object of type HistoryDatasetAssociation is not JSON serializable
# Reproduce by producing a real bundle (data_manager_mode=bundle), then
# loading the same tool's form.
history_id = self.dataset_populator.new_history()
payload = self.dataset_populator.run_tool_payload(
tool_id="data_manager_select",
inputs={"index": "hg19_value"},
data_manager_mode="bundle",
history_id=history_id,
)
create_response = self.dataset_populator._post("tools", data=payload)
create_response.raise_for_status()
self.dataset_populator.wait_for_history(history_id, assert_ok=True)
dataset = self.dataset_populator.get_history_dataset_details(history_id)
assert dataset["extension"] == "data_manager_json"
build = self.dataset_populator.build_tool_state("data_manager_select", history_id, inputs={})
index_input = next(i for i in build["inputs"] if i["name"] == "index")
option_names = [option[0] for option in index_input["options"]]
# On-disk fasta_indexes.loc entries...
assert "hg19_name" in option_names
# ...plus the synthetic option contributed by the user's bundle. Before
# the fix the bundle's HDA was wired into ``value`` instead of
# ``dataset`` and JSON encoding crashed before this assertion ran.
assert "regression_name" in option_names
def dataset_to_param(dataset):
return dict(src="hda", id=dataset["id"])
@@ -0,0 +1,19 @@
<tool id="data_manager_select" name="Test Data Manager Select" tool_type="manage_data" version="0.0.1" profile="19.05">
<description>Mirrors twobit_builder shape: select sourced from a tool data table. Regression for #22674.</description>
<configfiles>
<configfile name="static_test_data">{"data_tables": {"test_fasta_indexes": [{"value": "regression_value", "dbkey": "regression_dbkey", "name": "regression_name", "path": "regression.fa"}]}}</configfile>
</configfiles>
<command detect_errors="exit_code"><![CDATA[
mkdir $out_file.files_path ;
echo "$index" > '$out_file.files_path/regression.fa';
cp '$static_test_data' '$out_file'
]]></command>
<inputs>
<param name="index" type="select" label="Source FASTA Sequence">
<options from_data_table="test_fasta_indexes"/>
</param>
</inputs>
<outputs>
<data name="out_file" format="data_manager_json"/>
</outputs>
</tool>
@@ -13,4 +13,14 @@
</output>
</data_table>
</data_manager>
<data_manager tool_file="data_manager_select.xml" id="data_manager_select" version="1.0">
<data_table name="test_fasta_indexes">
<output>
<column name="value" />
<column name="dbkey" />
<column name="name" />
<column name="path" output_ref="out_file" />
</output>
</data_table>
</data_manager>
</data_managers>