Make file source URI discovery fail safe

Treat action-specific URI discovery as complete only when the concrete action explicitly declares it. Unknown custom actions retain the legacy behavior of serializing every file source.

Keep legacy upload paramfile parsing fail-fast and avoid constructing JobIO during failure cleanup so a secondary error cannot mask the original exception.

lib/galaxy/jobs/__init__.py

@@ -1084,13 +1084,16 @@ class MinimalJobWrapper(HasResourceParameters):
         if authnz_manager and trans.user:
             authnz_manager.refresh_expiring_oidc_tokens(trans, trans.user)
 
-    def _referenced_file_source_uris(self, job: Job) -> set[str]:
-        """Return the file source URIs needed to execute this job."""
+    def _referenced_file_source_uris(self, job: Job) -> set[str] | None:
+        """Return required URIs, or ``None`` when action discovery is incomplete."""
         uris: set[str] = set()
         if self.tool is not None:
+            tool_action = self.tool.tool_action
+            if not tool_action.has_complete_file_source_uri_discovery():
+                return None
             param_dict = self.get_param_dict(job)
             uris.update(collect_directory_uris(self.tool.inputs, param_dict))
-            uris.update(self.tool.tool_action.iter_referenced_file_source_uris(param_dict))
+            uris.update(tool_action.iter_referenced_file_source_uris(param_dict))
         for input_association in job.input_datasets + job.input_library_datasets:
             dataset = input_association.dataset
             if dataset is not None and dataset.has_deferred_data and dataset.dataset is not None:
@@ -1437,7 +1440,9 @@ class MinimalJobWrapper(HasResourceParameters):
         return tool_evaluator
 
     def _fix_output_permissions(self):
-        for path in [dp.real_path for dp in self.job_io.get_mutable_output_fnames()]:
+        if self._job_io is None:
+            return
+        for path in [dp.real_path for dp in self._job_io.get_mutable_output_fnames()]:
             if os.path.exists(path):
                 util.umask_fix_perms(path, self.app.config.umask, 0o666, self.app.config.gid)
 

lib/galaxy/tools/actions/__init__.py

@@ -99,6 +99,11 @@ class ToolAction:
     """
 
     produces_real_jobs: bool
+    file_source_uri_discovery_complete = False
+
+    def has_complete_file_source_uri_discovery(self) -> bool:
+        """Return whether this concrete action has audited URI discovery."""
+        return type(self).__dict__.get("file_source_uri_discovery_complete", False)
 
     def iter_referenced_file_source_uris(self, param_dict: ToolStateJobInstancePopulatedT) -> Iterable[str]:
         """Yield file source URIs embedded in action-specific parameters."""
@@ -146,6 +151,7 @@ class DefaultToolAction(ToolAction):
     """Default tool action is to run an external command"""
 
     produces_real_jobs: bool = True
+    file_source_uri_discovery_complete = True
 
     def _collect_input_datasets(
         self,

lib/galaxy/tools/actions/data_manager.py

@@ -29,6 +29,8 @@ log = logging.getLogger(__name__)
 class DataManagerToolAction(DefaultToolAction):
     """Tool action used for Data Manager Tools"""
 
+    file_source_uri_discovery_complete = True
+
     def execute(
         self,
         tool,

lib/galaxy/tools/actions/data_source.py

@@ -8,6 +8,8 @@ log = logging.getLogger(__name__)
 class DataSourceToolAction(DefaultToolAction):
     """Tool action used for Data Source Tools"""
 
+    file_source_uri_discovery_complete = True
+
     def _get_default_data_name(
         self, dataset, tool, on_text=None, trans=None, incoming=None, history=None, params=None, job_params=None, **kwd
     ):

lib/galaxy/tools/actions/history_imp_exp.py

@@ -41,6 +41,7 @@ class ImportHistoryToolAction(ToolAction):
     """Tool action used for importing a history to an archive."""
 
     produces_real_jobs: bool = True
+    file_source_uri_discovery_complete = True
 
     def iter_referenced_file_source_uris(self, param_dict: ToolStateJobInstancePopulatedT) -> Iterable[str]:
         if param_dict.get("__ARCHIVE_TYPE__") == "url":
@@ -124,6 +125,7 @@ class ExportHistoryToolAction(ToolAction):
     """Tool action used for exporting a history to an archive."""
 
     produces_real_jobs: bool = True
+    file_source_uri_discovery_complete = True
 
     def execute(
         self,

lib/galaxy/tools/actions/metadata.py

@@ -40,6 +40,7 @@ class SetMetadataToolAction(ToolAction):
 
     produces_real_jobs: bool = False
     set_output_hid: bool = False
+    file_source_uri_discovery_complete = True
 
     def execute(
         self,

lib/galaxy/tools/actions/model_operations.py

@@ -40,6 +40,7 @@ log = logging.getLogger(__name__)
 
 class ModelOperationToolAction(DefaultToolAction):
     produces_real_jobs: bool = False
+    file_source_uri_discovery_complete = True
 
     def check_inputs_ready(self, tool, trans, incoming, history, execution_cache=None, collection_info=None):
         if execution_cache is None:

lib/galaxy/tools/actions/upload.py

@@ -3,7 +3,10 @@ import logging
 import os
 from collections.abc import Iterable
 
-from galaxy.exceptions import RequestParameterMissingException
+from galaxy.exceptions import (
+    RequestParameterInvalidException,
+    RequestParameterMissingException,
+)
 from galaxy.job_execution.output_collect import copy_collection_metadata_from_target_dict
 from galaxy.managers.context import ProvidesHistoryContext
 from galaxy.model import (
@@ -40,6 +43,7 @@ log = logging.getLogger(__name__)
 
 class BaseUploadToolAction(ToolAction):
     produces_real_jobs = True
+    file_source_uri_discovery_complete = False
 
     def execute(
         self,
@@ -88,17 +92,24 @@ class BaseUploadToolAction(ToolAction):
 
 
 class UploadToolAction(BaseUploadToolAction):
+    file_source_uri_discovery_complete = True
+
     def iter_referenced_file_source_uris(self, param_dict: ToolStateJobInstancePopulatedT) -> Iterable[str]:
         paramfile = param_dict.get("paramfile")
         if not isinstance(paramfile, str):
-            return
+            raise RequestParameterInvalidException("Legacy upload job is missing its paramfile")
         with open(paramfile) as f:
             upload_params = json.load(f)
+        if not isinstance(upload_params, list):
+            raise RequestParameterInvalidException("Legacy upload paramfile must contain a list")
         for upload_param in upload_params:
+            if not isinstance(upload_param, dict):
+                raise RequestParameterInvalidException("Legacy upload paramfile entries must be objects")
             if upload_param.get("type") == "url":
                 path = upload_param.get("path")
-                if isinstance(path, str) and path:
-                    yield path
+                if not isinstance(path, str) or not path:
+                    raise RequestParameterInvalidException("Legacy URL upload entry is missing its path")
+                yield path
 
     def _setup_job(
         self, tool, trans: ProvidesHistoryContext, incoming, dataset_upload_inputs, history, preferred_object_store_id
@@ -126,6 +137,8 @@ class UploadToolAction(BaseUploadToolAction):
 
 
 class FetchUploadToolAction(BaseUploadToolAction):
+    file_source_uri_discovery_complete = True
+
     def iter_referenced_file_source_uris(self, param_dict: ToolStateJobInstancePopulatedT) -> Iterable[str]:
         return iter_fetch_request_urls(param_dict)
 

test/unit/app/jobs/test_job_wrapper.py

@@ -156,7 +156,10 @@ class MockTool:
         self.tmp_target = None
         self.tool_source = Bunch(to_string=lambda: "")
         self.inputs = {}
-        self.tool_action = SimpleNamespace(iter_referenced_file_source_uris=lambda param_dict: ())
+        self.tool_action = SimpleNamespace(
+            has_complete_file_source_uri_discovery=lambda: True,
+            iter_referenced_file_source_uris=lambda param_dict: (),
+        )
 
     def params_from_strings(self, param_dict):
         return param_dict
@@ -209,8 +212,11 @@ class MockObjectStore:
         return None
 
 
-def _minimal_wrapper(param_dict=None, inputs=None, action_uris=()):
-    tool_action = SimpleNamespace(iter_referenced_file_source_uris=lambda param_dict: action_uris)
+def _minimal_wrapper(param_dict=None, inputs=None, action_uris=(), action_discovery_complete=True):
+    tool_action = SimpleNamespace(
+        has_complete_file_source_uri_discovery=lambda: action_discovery_complete,
+        iter_referenced_file_source_uris=lambda param_dict: action_uris,
+    )
     return SimpleNamespace(
         tool=SimpleNamespace(inputs=inputs or {}, tool_action=tool_action),
         get_param_dict=lambda job: param_dict or {},
@@ -244,6 +250,11 @@ def test_referenced_file_source_uris_empty_for_job_without_sources():
     assert MinimalJobWrapper._referenced_file_source_uris(_minimal_wrapper(), _job_with_file_source_inputs()) == set()
 
 
+def test_referenced_file_source_uris_unknown_for_unaudited_action():
+    wrapper = _minimal_wrapper(action_discovery_complete=False)
+    assert MinimalJobWrapper._referenced_file_source_uris(wrapper, _job_with_file_source_inputs()) is None
+
+
 def test_referenced_file_source_uris_adds_regular_and_library_input_sources():
     regular_src = f"gxuserfiles://{uuid4().hex}/regular.txt"
     library_src = f"gxuserfiles://{uuid4().hex}/library.txt"
@@ -261,3 +272,15 @@ def test_referenced_file_source_uris_ignores_materialized_input_sources():
     hda = SimpleNamespace(has_deferred_data=False, dataset=SimpleNamespace(source_uris=[source]))
     job = _job_with_file_source_inputs(input_datasets=[SimpleNamespace(dataset=hda)])
     assert MinimalJobWrapper._referenced_file_source_uris(_minimal_wrapper(), job) == set()
+
+
+def test_fix_output_permissions_does_not_initialize_job_io():
+    class WrapperWithoutJobIO:
+        _job_io = None
+
+        @property
+        def job_io(self):
+            raise AssertionError("job_io should not be initialized during failure cleanup")
+
+    wrapper = cast(MinimalJobWrapper, WrapperWithoutJobIO())
+    MinimalJobWrapper._fix_output_permissions(wrapper)

test/unit/app/tools/test_upload_actions.py

@@ -2,6 +2,9 @@ import json
 import tempfile
 from typing import Any
 
+import pytest
+
+from galaxy.exceptions import RequestParameterInvalidException
 from galaxy.tools.actions.upload import UploadToolAction
 
 
@@ -22,3 +25,41 @@ def test_upload_action_reports_urls_from_paramfile():
         assert list(action.iter_referenced_file_source_uris({"paramfile": paramfile.name})) == [
             "https://example.org/input.txt"
         ]
+
+
+@pytest.mark.parametrize(
+    ("upload_params", "message"),
+    [
+        ({"type": "url", "path": "https://example.org/input.txt"}, "must contain a list"),
+        (["not an object"], "entries must be objects"),
+        ([{"type": "url", "path": None}], "URL upload entry is missing its path"),
+    ],
+)
+def test_upload_action_rejects_invalid_paramfile_shape(upload_params, message):
+    with tempfile.NamedTemporaryFile(mode="w") as paramfile:
+        json.dump(upload_params, paramfile)
+        paramfile.flush()
+        action = ConcreteUploadToolAction()
+        with pytest.raises(RequestParameterInvalidException, match=message):
+            list(action.iter_referenced_file_source_uris({"paramfile": paramfile.name}))
+
+
+def test_upload_action_rejects_missing_paramfile_parameter():
+    action = ConcreteUploadToolAction()
+    with pytest.raises(RequestParameterInvalidException, match="missing its paramfile"):
+        list(action.iter_referenced_file_source_uris({}))
+
+
+def test_upload_action_does_not_hide_missing_paramfile(tmp_path):
+    action = ConcreteUploadToolAction()
+    with pytest.raises(FileNotFoundError):
+        list(action.iter_referenced_file_source_uris({"paramfile": str(tmp_path / "missing.json")}))
+
+
+def test_upload_action_does_not_hide_malformed_paramfile():
+    with tempfile.NamedTemporaryFile(mode="w") as paramfile:
+        paramfile.write("{")
+        paramfile.flush()
+        action = ConcreteUploadToolAction()
+        with pytest.raises(json.JSONDecodeError):
+            list(action.iter_referenced_file_source_uris({"paramfile": paramfile.name}))