Test invalid expressions and expression results

lib/galaxy/schema/schema.py

@@ -29,6 +29,7 @@
     UUID4,
 )
 from pydantic.generics import GenericModel
+from pydantic.utils import GetterDict
 from typing_extensions import (
     Annotated,
     Literal,
@@ -1926,6 +1927,8 @@ class FailureReason(str, Enum):
     collection_failed = "collection_failed"
     job_failed = "job_failed"
     output_not_found = "output_not_found"
+    expression_evaluation_failed = "expression_evaluation_failed"
+    when_not_boolean = "when_not_boolean"
     unexpected_failure = "unexpected_failure"
 
 
@@ -1940,20 +1943,33 @@ class CancelReason(str, Enum):
 DatabaseIdT = TypeVar("DatabaseIdT")
 
 
+class StepOrderIndexGetter(GetterDict):
+    def get(self, key: Any, default: Any = None) -> Any:
+
+        # Fetch the order_index when serializing for the API,
+        # which makes much more sense when pointing to steps.
+        if key == "workflow_step_id":
+            return self._obj.workflow_step.order_index
+        elif key == "dependent_workflow_step_id":
+            return self._obj.dependent_workflow_step.order_index
+
+        return super().get(key, default)
+
+
 class InvocationMessageBase(GenericModel):
     reason: Union[CancelReason, FailureReason, WarningReason]
 
     class Config:
         orm_mode = True
+        getter_dict = StepOrderIndexGetter
 
 
 class GenericInvocationCancellationReviewFailed(InvocationMessageBase, Generic[DatabaseIdT]):
     reason: Literal[CancelReason.cancelled_on_review]
-    workflow_step_id: DatabaseIdT = Field(..., description="Workflow step id of paused step that did not pass review.")
+    workflow_step_id: int = Field(..., description="Workflow step id of paused step that did not pass review.")
 
 
 class GenericInvocationCancellationHistoryDeleted(InvocationMessageBase, Generic[DatabaseIdT]):
-    # reason: CancelReason.history_deleted
     reason: Literal[CancelReason.history_deleted]
     history_id: DatabaseIdT = Field(..., title="History ID", description="History ID of history that was deleted.")
 
@@ -1963,17 +1979,15 @@ class GenericInvocationCancellationUserRequest(InvocationMessageBase, Generic[Da
 
 
 class InvocationFailureMessageBase(InvocationMessageBase, Generic[DatabaseIdT]):
-    workflow_step_id: DatabaseIdT = Field(..., description="Workflow step id of step that failed.")
+    workflow_step_id: int = Field(..., description="Workflow step id of step that failed.")
 
 
 class GenericInvocationFailureDatasetFailed(InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]):
     reason: Literal[FailureReason.dataset_failed]
     hda_id: DatabaseIdT = Field(
         ..., title="HistoryDatasetAssociation ID", description="HistoryDatasetAssociation ID that relates to failure."
     )
-    dependent_workflow_step_id: Optional[DatabaseIdT] = Field(
-        None, description="Workflow step id of step that caused failure."
-    )
+    dependent_workflow_step_id: Optional[int] = Field(None, description="Workflow step id of step that caused failure.")
 
 
 class GenericInvocationFailureCollectionFailed(InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]):
@@ -1983,29 +1997,41 @@ class GenericInvocationFailureCollectionFailed(InvocationFailureMessageBase[Data
         title="HistoryDatasetCollectionAssociation ID",
         description="HistoryDatasetCollectionAssociation ID that relates to failure.",
     )
-    dependent_workflow_step_id: DatabaseIdT = Field(None, description="Workflow step id of step that caused failure.")
+    dependent_workflow_step_id: int = Field(..., description="Workflow step id of step that caused failure.")
 
 
 class GenericInvocationFailureJobFailed(InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]):
     reason: Literal[FailureReason.job_failed]
     job_id: DatabaseIdT = Field(None, title="Job ID", description="Job ID that relates to failure.")
-    dependent_workflow_step_id: DatabaseIdT = Field(None, description="Workflow step id of step that caused failure.")
+    dependent_workflow_step_id: int = Field(..., description="Workflow step id of step that caused failure.")
 
 
 class GenericInvocationFailureOutputNotFound(InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]):
     reason: Literal[FailureReason.output_not_found]
     output_name: str = Field(..., title="Tool or module output name that was referenced but not produced")
-    dependent_workflow_step_id: DatabaseIdT = Field(None, description="Workflow step id of step that caused failure.")
+    dependent_workflow_step_id: int = Field(..., description="Workflow step id of step that caused failure.")
+
 
+class GenericInvocationFailureExpressionEvaluationFailed(
+    InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]
+):
+    reason: Literal[FailureReason.expression_evaluation_failed]
+    details: Optional[str] = Field(None, description="May contain details to help troubleshoot this problem.")
+
+
+class GenericInvocationFailureWhenNotBoolean(InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]):
+    reason: Literal[FailureReason.when_not_boolean]
+    details: str = Field(..., description="Contains details to help troubleshoot this problem.")
 
-class GenericInvocationUnexpectedFailure(InvocationFailureMessageBase[DatabaseIdT], Generic[DatabaseIdT]):
+
+class GenericInvocationUnexpectedFailure(InvocationMessageBase, Generic[DatabaseIdT]):
     reason: Literal[FailureReason.unexpected_failure]
     details: Optional[str] = Field(None, description="May contains details to help troubleshoot this problem.")
 
 
 class GenericInvocationWarning(InvocationMessageBase, Generic[DatabaseIdT]):
     reason: WarningReason = Field(..., title="Failure Reason", description="Reason for warning")
-    workflow_step_id: Optional[DatabaseIdT] = Field(None, title="Workflow step id of step that caused a warning.")
+    workflow_step_id: Optional[int] = Field(None, title="Workflow step id of step that caused a warning.")
 
 
 class GenericInvocationEvaluationWarningWorkflowOutputNotFound(
@@ -2024,22 +2050,26 @@ class GenericInvocationEvaluationWarningWorkflowOutputNotFound(
 InvocationFailureCollectionFailed = GenericInvocationFailureCollectionFailed[int]
 InvocationFailureJobFailed = GenericInvocationFailureJobFailed[int]
 InvocationFailureOutputNotFound = GenericInvocationFailureOutputNotFound[int]
+InvocationFailureExpressionEvaluationFailed = GenericInvocationFailureExpressionEvaluationFailed[int]
+InvocationFailureWhenNotBoolean = GenericInvocationFailureWhenNotBoolean[int]
 InvocationUnexpectedFailure = GenericInvocationUnexpectedFailure[int]
 InvocationWarningWorkflowOutputNotFound = GenericInvocationEvaluationWarningWorkflowOutputNotFound[int]
 
 
-class InvocationMessage(Model):
+class GenericInvocationMessage(GenericModel, Generic[DatabaseIdT]):
     __root__: Annotated[
         Union[
-            InvocationCancellationReviewFailed,
-            InvocationCancellationHistoryDeleted,
-            InvocationCancellationUserRequest,
-            InvocationFailureDatasetFailed,
-            InvocationFailureCollectionFailed,
-            InvocationFailureJobFailed,
-            InvocationFailureOutputNotFound,
-            InvocationUnexpectedFailure,
-            InvocationWarningWorkflowOutputNotFound,
+            GenericInvocationCancellationReviewFailed[DatabaseIdT],
+            GenericInvocationCancellationHistoryDeleted[DatabaseIdT],
+            GenericInvocationCancellationUserRequest[DatabaseIdT],
+            GenericInvocationFailureDatasetFailed[DatabaseIdT],
+            GenericInvocationFailureCollectionFailed[DatabaseIdT],
+            GenericInvocationFailureJobFailed[DatabaseIdT],
+            GenericInvocationFailureOutputNotFound[DatabaseIdT],
+            GenericInvocationFailureExpressionEvaluationFailed[DatabaseIdT],
+            GenericInvocationFailureWhenNotBoolean[DatabaseIdT],
+            GenericInvocationUnexpectedFailure[DatabaseIdT],
+            GenericInvocationEvaluationWarningWorkflowOutputNotFound[DatabaseIdT],
         ],
         Field(discriminator="reason"),
     ]
@@ -2048,6 +2078,10 @@ class Config:
         orm_mode = True
 
 
+InvocationMessage = GenericInvocationMessage[int]
+InvocationMessageResponseModel = GenericInvocationMessage[EncodedDatabaseIdField]
+
+
 class Creator(Model):
     class_: str = Field(..., alias="class", title="Class", description="The class representing this creator.")
     name: str = Field(..., title="Name", description="The name of the creator.")

lib/galaxy/webapps/galaxy/api/workflows.py

@@ -51,7 +51,7 @@
 from galaxy.schema.schema import (
     AsyncFile,
     AsyncTaskResultSummary,
-    InvocationMessage,
+    InvocationMessageResponseModel,
     SetSlugPayload,
     ShareWithPayload,
     ShareWithStatus,
@@ -768,10 +768,10 @@ def invoke(self, trans: GalaxyWebTransaction, workflow_id, payload, **kwd):
         encoded_invocations = []
         for invocation in invocations:
             as_dict = workflow_invocation.to_dict()
+            as_dict = self.encode_all_ids(trans, as_dict, recursive=True)
             as_dict["messages"] = [
-                InvocationMessage.parse_obj(message).__root__.dict() for message in invocation.messages
+                InvocationMessageResponseModel.parse_obj(message).__root__.dict() for message in invocation.messages
             ]
-            as_dict = self.encode_all_ids(trans, as_dict, recursive=True)
             encoded_invocations.append(as_dict)
 
         if is_batch:

lib/galaxy/webapps/galaxy/services/invocations.py

@@ -35,7 +35,7 @@
     AsyncTaskResultSummary,
     BcoGenerationParametersMixin,
     InvocationIndexQueryPayload,
-    InvocationMessage,
+    InvocationMessageResponseModel,
     StoreExportPayload,
     WriteStoreToPayload,
 )
@@ -200,8 +200,11 @@ def serialize_workflow_invocation(
         step_details = params.step_details
         legacy_job_state = params.legacy_job_state
         as_dict = invocation.to_dict(view, step_details=step_details, legacy_job_state=legacy_job_state)
-        as_dict["messages"] = [InvocationMessage.parse_obj(message).__root__.dict() for message in invocation.messages]
-        return self.security.encode_all_ids(as_dict, recursive=True)
+        as_dict = self.security.encode_all_ids(as_dict, recursive=True)
+        as_dict["messages"] = [
+            InvocationMessageResponseModel.parse_obj(message).__root__.dict() for message in invocation.messages
+        ]
+        return as_dict
 
     def serialize_workflow_invocations(
         self,

lib/galaxy/workflow/modules.py

@@ -43,6 +43,8 @@
     FailureReason,
     InvocationCancellationReviewFailed,
     InvocationFailureDatasetFailed,
+    InvocationFailureExpressionEvaluationFailed,
+    InvocationFailureWhenNotBoolean,
 )
 from galaxy.tool_util.cwl.util import set_basename_and_derived_properties
 from galaxy.tool_util.parser.output_objects import ToolExpressionOutput
@@ -202,15 +204,38 @@ def from_cwl(value):
             step_state[key] = to_cwl(value)
 
     if when_expression is not None:
-        as_cwl_value = do_eval(
-            when_expression,
-            step_state,
-            [{"class": "InlineJavascriptRequirement"}],
-            None,
-            None,
-            {},
-        )
-        return from_cwl(as_cwl_value)
+        try:
+            as_cwl_value = do_eval(
+                when_expression,
+                step_state,
+                [{"class": "InlineJavascriptRequirement"}],
+                None,
+                None,
+                {},
+            )
+        except Exception:
+            # Exception contains script and traceback, which could be helpful for debugging workflows,
+            # but both could conceivably contain secrets.
+            # CWL has a secret hint that should cause values to be sanitized,
+            # but Galaxy does not, so we can't really display anything here at this point.
+            # In any case I believe the CWL secret hint can be bypassed if the value is passed on
+            # to another step input that doesn't have the secret set.
+            # Complicated stuff, ignore for now.
+            raise FailWorkflowEvaluation(
+                InvocationFailureExpressionEvaluationFailed(
+                    reason=FailureReason.expression_evaluation_failed, workflow_step_id=step.id
+                )
+            )
+        when_value = from_cwl(as_cwl_value)
+        if not isinstance(when_value, bool):
+            raise FailWorkflowEvaluation(
+                InvocationFailureWhenNotBoolean(
+                    reason=FailureReason.when_not_boolean,
+                    workflow_step_id=step.id,
+                    details=f"Type is: {when_value.__class__.__name__}",
+                )
+            )
+        return when_value
 
 
 class WorkflowModule:
@@ -2184,7 +2209,6 @@ def callback(input, prefixed_name, **kwargs):
                     when_value = evaluate_value_from_expressions(
                         progress, step, execution_state=execution_state, extra_step_state=extra_step_state
                     )
-                    assert isinstance(when_value, bool)
             if when_value is not None:
                 # Track this more formally ?
                 execution_state.inputs["__when_value__"] = when_value

lib/galaxy/workflow/run.py

@@ -19,6 +19,7 @@
     WorkflowInvocationStep,
 )
 from galaxy.schema.schema import (
+    CancelReason,
     FailureReason,
     InvocationCancellationHistoryDeleted,
     InvocationFailureCollectionFailed,
@@ -200,7 +201,7 @@ def invoke(self) -> Dict[int, Any]:
         if workflow_invocation.history.deleted:
             raise modules.CancelWorkflowEvaluation(
                 why=InvocationCancellationHistoryDeleted(
-                    reason=FailureReason.history_deleted, history_id=workflow_invocation.history_id
+                    reason=CancelReason.history_deleted, history_id=workflow_invocation.history_id
                 )
             )