From 811838f0b1fa21810ddb1c5f73e7b50ec964dc9e Mon Sep 17 00:00:00 2001 From: Mark <399551+mwiebe@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:13:19 -0700 Subject: [PATCH] fix: resolve parameter type names case-insensitively in reference validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The EXPR extension makes job and task parameter type names case-insensitive (RFC 0007 §2), so `type: string` is exactly as valid as `type: STRING`. The model accepted such a declaration but then rejected every reference to the parameter: steps[0] -> script -> actions -> onRun -> args[0]: Variable Param.Msg does not exist at this location. The variable-reference prevalidation pass runs on raw template values, before the field validator that folds the `type` discriminator to upper case, and it resolved the type-discriminated parameter-definition union with an exact string comparison. A lowercase or mixed-case type name matched no member of the union, so the definition contributed none of its `Param.*` / `RawParam.*` / `Task.Param.*` symbols and none of its EXPR type information. Compare the discriminator with an ASCII-only case fold when EXPR is active, in both the reference-validation and the definition-collection traversal. Job, task, and environment-template parameters now define their symbols and types whatever the case of the type name. The fold is ASCII-only so Unicode look-alikes such as `ıNT` stay rejected, matching the field validator. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> --- .../_variable_reference_validation.py | 46 +++++++++++++++---- .../v2023_09/test_environment_template.py | 15 ++++++ .../model_v0/v2023_09/test_list_parameters.py | 43 +++++++++++++++++ .../model_v0/v2023_09/test_parameter_space.py | 16 +++++++ 4 files changed, 111 insertions(+), 9 deletions(-) diff --git a/src/openjd/model/_internal/_variable_reference_validation.py b/src/openjd/model/_internal/_variable_reference_validation.py index 0bb6c41f..e34a4565 100644 --- a/src/openjd/model/_internal/_variable_reference_validation.py +++ b/src/openjd/model/_internal/_variable_reference_validation.py @@ -260,6 +260,10 @@ def _validate_model_template_variable_references( # The errors that we're collecting for this node in the traversal, and will return from the function call. errors: list[InitErrorDetails] = [] + expr_enabled = bool( + context is not None and getattr(context, "_prevalidation_expr_enabled", False) + ) + model_origin = typing.get_origin(model) # Unwrap the Optional types @@ -345,7 +349,9 @@ def _validate_model_template_variable_references( # Unwrap a discriminated union to the selected type if model_origin is Union and discriminator is not None: - unioned_model = _get_model_for_singleton_value(model, value, discriminator) + unioned_model = _get_model_for_singleton_value( + model, value, discriminator, expr_enabled=expr_enabled + ) if unioned_model is not None: return _validate_model_template_variable_references( unioned_model, @@ -398,10 +404,6 @@ def _validate_model_template_variable_references( # If the node doesn't modify the variable prefix, then symbol_prefix will be the empty string symbol_prefix += variable_defs.symbol_prefix - expr_enabled = bool( - context is not None and getattr(context, "_prevalidation_expr_enabled", False) - ) - # Recursively collect all of the variable definitions at this node and its # child nodes. Symbol EXPR types are collected only when the EXPR # extension is active, keeping the Rust expr surface off the non-EXPR @@ -709,8 +711,17 @@ def _validate_let_bindings( return errors +# Parameter type names are ASCII (RFC 0007 §2). str.upper() is Unicode-aware +# and folds U+0131 to 'I', which would make 'ıNT' a spelling of 'INT'. +_ASCII_UPPERCASE = str.maketrans("abcdefghijklmnopqrstuvwxyz", "ABCDEFGHIJKLMNOPQRSTUVWXYZ") + + def _get_model_for_singleton_value( - model: Any, value: Any, discriminator: Union[str, Discriminator, None] = None + model: Any, + value: Any, + discriminator: Union[str, Discriminator, None] = None, + *, + expr_enabled: bool = False, ) -> Optional[Type]: """Given a FieldInfo and the value that we're given for that field, determine the actual Model for the value in the event that the FieldInfo may be for @@ -756,7 +767,20 @@ def _get_model_for_singleton_value( raise NotImplementedError( "You have hit an unimplemented code path. Please report this as a bug." ) - if typing.get_args(sub_model_discr_value)[0] == discr_value: + literal_value = typing.get_args(sub_model_discr_value)[0] + if literal_value == discr_value: + return sub_model + # RFC 0007 §2: parameter type names are case-insensitive when the EXPR + # extension is active. The `_normalize_parameter_type_case` field + # validator uppercases the `type` discriminator before pydantic's + # union resolution, but this prevalidation traversal runs earlier and + # sees the raw values — so match the discriminator case-insensitively + # here, or a lowercase-typed parameter defines no Param.* symbols. + if ( + expr_enabled + and isinstance(literal_value, str) + and literal_value.translate(_ASCII_UPPERCASE) == discr_value.translate(_ASCII_UPPERCASE) + ): return sub_model return None @@ -881,9 +905,13 @@ def _collect_variable_definitions( # noqa: C901 (suppress: too complex) ) return {"__export__": symtab} - # Unwrap a discriminated union to the selected type + # Unwrap a discriminated union to the selected type. `collect_types` is + # set exactly when the EXPR extension is active, which also governs the + # case-insensitive `type` discriminator matching (RFC 0007 §2). if model_origin is Union and discriminator is not None: - unioned_model = _get_model_for_singleton_value(model, value, discriminator) + unioned_model = _get_model_for_singleton_value( + model, value, discriminator, expr_enabled=collect_types + ) if unioned_model is not None: return _collect_variable_definitions( unioned_model, diff --git a/test/openjd/model_v0/v2023_09/test_environment_template.py b/test/openjd/model_v0/v2023_09/test_environment_template.py index a8f5acf4..f06d78f2 100644 --- a/test/openjd/model_v0/v2023_09/test_environment_template.py +++ b/test/openjd/model_v0/v2023_09/test_environment_template.py @@ -319,6 +319,21 @@ def test_with_expr_miscased_accepted(self, canonical: str, miscased: str) -> Non # which is the state an audit found untested. self._decode(self._tmpl(miscased)) + @pytest.mark.parametrize("canonical, miscased", TYPES) + def test_with_expr_miscased_param_referenceable(self, canonical: str, miscased: str) -> None: + # A non-canonically-cased type name is just as valid under EXPR, so it + # must still define Param.P for the variable-reference + # prevalidation, which resolves the raw `type` discriminator before the + # case fold runs. Regression: the definition was accepted but every + # {{ Param.P }} reference was rejected with "does not exist at this + # location". + template = self._tmpl(miscased) + template["environment"] = { + "name": "E", + "script": {"actions": {"onEnter": {"command": "echo", "args": ["{{ Param.P }}"]}}}, + } + self._decode(template) + @pytest.mark.parametrize("type_name", ("\u0131NT", "\u017fTRING", "\ufb02OAT")) def test_non_ascii_lookalike_rejected_with_expr(self, type_name: str) -> None: # str.upper() folds U+0131 to 'I', U+017F to 'S' and U+FB02 (fl) to 'FL', diff --git a/test/openjd/model_v0/v2023_09/test_list_parameters.py b/test/openjd/model_v0/v2023_09/test_list_parameters.py index ff12a076..69637463 100644 --- a/test/openjd/model_v0/v2023_09/test_list_parameters.py +++ b/test/openjd/model_v0/v2023_09/test_list_parameters.py @@ -169,6 +169,49 @@ def test_with_expr_miscased_accepted(self, canonical, miscased, default): # Case 4 of 4. _decode(_tmpl(self._param(miscased, default))) + # ── Mis-cased parameters are referenceable ── + + @staticmethod + def _referencing_step(): + return { + "name": "S", + "script": { + "actions": { + "onRun": {"command": "echo", "args": ["{{ Param.P }}", "{{ RawParam.P }}"]} + } + }, + } + + @pytest.mark.parametrize("canonical, miscased, default", TYPES) + def test_with_expr_miscased_param_referenceable(self, canonical, miscased, default): + # A non-canonically-cased type name is just as valid under EXPR, so it + # must still define Param.P/RawParam.P for the + # variable-reference prevalidation, which resolves the raw `type` + # discriminator before the case fold runs. Regression: the parameter + # declaration was accepted but every {{ Param.P }} reference was + # rejected with "does not exist at this location". + template = _tmpl(self._param(miscased, default)) + template["steps"] = [self._referencing_step()] + _decode(template) + + def test_with_expr_miscased_param_type_checked(self): + # The reference prevalidation also collects the parameter's EXPR type + # through the lowercase spelling: string + int is a type error. + template = _tmpl(self._param("string", "hi")) + template["steps"] = [ + { + "name": "S", + "script": { + "actions": {"onRun": {"command": "echo", "args": ["{{ Param.P + 1 }}"]}} + }, + } + ] + with pytest.raises(DecodeValidationError) as excinfo: + _decode(template) + assert "steps[0] -> script -> actions -> onRun -> args[0]" in str(excinfo.value), str( + excinfo.value + ) + # ── The fold is ASCII ── # str.upper() folds each of these wholly into the type-name alphabet: U+0131 diff --git a/test/openjd/model_v0/v2023_09/test_parameter_space.py b/test/openjd/model_v0/v2023_09/test_parameter_space.py index 70a4f7fe..0f3cd84c 100644 --- a/test/openjd/model_v0/v2023_09/test_parameter_space.py +++ b/test/openjd/model_v0/v2023_09/test_parameter_space.py @@ -900,6 +900,22 @@ def test_with_expr_miscased_accepted( self._tmpl(miscased, range_value, chunks, self._extensions_for(canonical, expr=True)) ) + @pytest.mark.parametrize("canonical, miscased, range_value, chunks", TYPES) + def test_with_expr_miscased_param_referenceable( + self, canonical: str, miscased: str, range_value: Any, chunks: Any + ) -> None: + # A non-canonically-cased type name is just as valid under EXPR, so it + # must still define Task.Param.F for the + # variable-reference prevalidation, which resolves the raw `type` + # discriminator before the case fold runs. Regression: the definition + # was accepted but every {{ Task.Param.F }} reference was rejected + # with "does not exist at this location". + template = self._tmpl( + miscased, range_value, chunks, self._extensions_for(canonical, expr=True) + ) + template["steps"][0]["script"]["actions"]["onRun"]["args"] = ["{{ Task.Param.F }}"] + self._decode(template) + # ── The gate is on EXPR, not on the extension that supplies the type ── def test_chunk_int_miscased_needs_expr_not_only_task_chunking(self) -> None: