From c6c9fe56763aff7b9fdf8f8880bc6e38edc10a03 Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Wed, 11 May 2022 16:55:39 +0200 Subject: [PATCH] implement basic checks; use ECMAScript for regex --- cwltool/extensions-v1.2.yml | 11 +++- cwltool/process.py | 105 +++++++++++++++++++++++++++++-- tests/parameter_restrictions.cwl | 5 ++ tests/test_ext.py | 48 ++++++++++++++ 4 files changed, 162 insertions(+), 7 deletions(-) diff --git a/cwltool/extensions-v1.2.yml b/cwltool/extensions-v1.2.yml index 6b9fb71302..76c7216efb 100644 --- a/cwltool/extensions-v1.2.yml +++ b/cwltool/extensions-v1.2.yml @@ -199,8 +199,7 @@ $graph: - name: regex type: record doc: | - Regular Expression - TODO: what type of regex? + ECMAScript 5.1 Regular Expression constraint. fields: class: type: @@ -213,7 +212,13 @@ $graph: _type: "@vocab" rpattern: type: string - + doc: | + Testing should be the equivalent of calling `/rpattern/.test(value)` + where `rpattern` is the value of the `rpattern` field, and `value` + is the input object value to be tested. If the result is `true` then the + input object value is accepted. If the result if `false` then the + input object value does not match this constraint. + - name: Restrictions type: record diff --git a/cwltool/process.py b/cwltool/process.py index c34cfb2714..62dce1d51d 100644 --- a/cwltool/process.py +++ b/cwltool/process.py @@ -20,6 +20,7 @@ Iterable, Iterator, List, + Mapping, MutableMapping, MutableSequence, Optional, @@ -55,6 +56,7 @@ from .loghandler import _logger from .mpi import MPIRequirementName from .pathmapper import MapperEnt, PathMapper +from .sandboxjs import execjs from .secrets import SecretStore from .stdfsaccess import StdFsAccess from .update import INTERNAL_VERSION, ORIGINAL_CWLVERSION @@ -523,10 +525,11 @@ def var_spool_cwl_detector( r = False if isinstance(obj, str): if "var/spool/cwl" in obj and obj_key != "dockerOutputDirectory": + debug = _logger.isEnabledFor(logging.DEBUG) _logger.warning( - SourceLine(item=item, key=obj_key, raise_type=str).makeError( - _VAR_SPOOL_ERROR.format(obj) - ) + SourceLine( + item=item, key=obj_key, raise_type=str, include_traceback=debug + ).makeError(_VAR_SPOOL_ERROR.format(obj)) ) r = True elif isinstance(obj, MutableMapping): @@ -743,7 +746,9 @@ def __init__( and not is_req ): _logger.warning( - SourceLine(item=dockerReq, raise_type=str).makeError( + SourceLine( + item=dockerReq, raise_type=str, include_traceback=debug + ).makeError( "When 'dockerOutputDirectory' is declared, DockerRequirement " "should go in the 'requirements' section, not 'hints'." "" @@ -805,6 +810,98 @@ def _init_job( vocab=INPUT_OBJ_VOCAB, ) + restriction_req, mandatory_restrictions = self.get_requirement( + "ParameterRestrictions" + ) + + if restriction_req: + restrictions = restriction_req["restrictions"] + for entry in cast(List[CWLObjectType], restrictions): + name = shortname(cast(str, entry["input"])) + if name in job: + value = job[name] + matched = False + for constraint in cast( + List[CWLOutputType], entry["constraints"] + ): + if isinstance(constraint, Mapping): + if constraint["class"] == "intInterval": + if not isinstance(value, int): + raise SourceLine( + constraint, + None, + WorkflowException, + runtime_context.debug, + ).makeError( + "intInterval parameter restriction is only valid for inputs of type 'int'; " + f"instead got {type(value)}: {value}." + ) + low = cast( + Union[int, float], + constraint.get("low", -math.inf), + ) + high = cast( + Union[int, float], + constraint.get("high", math.inf), + ) + matched = value >= low and value <= high + elif constraint["class"] == "realInterval": + if not isinstance(value, (int, float)): + raise SourceLine( + constraint, + None, + WorkflowException, + runtime_context.debug, + ).makeError( + "realInterval parameter restriction is only valid for inputs of type 'int', 'float', and 'double'; " + f"instead got {type(value)}: {value}." + ) + low = cast( + Union[int, float], + constraint.get("low", -math.inf), + ) + high = cast( + Union[int, float], + constraint.get("high", math.inf), + ) + low_inclusive = constraint.get( + "low_inclusive", True + ) + high_inclusive = constraint.get( + "high_inclusive", True + ) + check_low = ( + value >= low if low_inclusive else value > low + ) + check_high = ( + value <= high if low_inclusive else value < high + ) + matched = check_low and check_high + elif constraint["class"] == "regex": + rpattern = constraint["rpattern"] + quoted_value = json.dumps(value) + matched = cast( + bool, + execjs( + f"/{rpattern}/.test({quoted_value})", + "", + runtime_context.eval_timeout, + runtime_context.force_docker_pull, + ), + ) + elif constraint == value: + matched = True + if matched: + break + if not matched: + raise SourceLine( + job, name, WorkflowException, runtime_context.debug + ).makeError( + f"The field '{name}' is not valid because its " + f"value '{value}' failed to match any of the " + f"constraints '{json.dumps(entry['constraints'])}'." + ) + if load_listing and load_listing != "no_listing": get_listing(fs_access, job, recursive=(load_listing == "deep_listing")) diff --git a/tests/parameter_restrictions.cwl b/tests/parameter_restrictions.cwl index a8a18eebed..2e5a5f6e92 100644 --- a/tests/parameter_restrictions.cwl +++ b/tests/parameter_restrictions.cwl @@ -7,6 +7,7 @@ doc: | inputs: one: double two: string + three: int hints: @@ -24,6 +25,10 @@ hints: two: - class: cwltool:regex rpattern: "foo.*bar" + three: + - class: cwltool:intInterval + low: -10 + high: -7 baseCommand: echo diff --git a/tests/test_ext.py b/tests/test_ext.py index 1eb4091e5a..6a9bd6743f 100644 --- a/tests/test_ext.py +++ b/tests/test_ext.py @@ -295,3 +295,51 @@ def test_warn_large_inputs() -> None: ) finally: cwltool.process.FILE_COUNT_WARNING = was + + +def test_parameter_restrictions_parsing() -> None: + """Basic parsing of the ParameterRestrictions extension.""" + assert ( + main( + ["--enable-ext", "--validate", get_data("tests/parameter_restrictions.cwl")] + ) + == 0 + ) + + +def test_parameter_restrictions_valid_job() -> None: + """Confirm that valid values are accepted.""" + assert ( + main( + [ + "--enable-ext", + get_data("tests/parameter_restrictions.cwl"), + "--one", + "2.5", + "--two", + "foofoobar", + "--three", + "-5", + ] + ) + == 0 + ) + + +def test_parameter_restrictions_invalid_job() -> None: + """Confirm that an invvalid value is rejected.""" + assert ( + main( + [ + "--enable-ext", + get_data("tests/parameter_restrictions.cwl"), + "--one", + "2.5", + "--two", + "fuzzbar", + "--three", + "-5", + ] + ) + == 1 + )