From 84db8743c5137621f71accc8a61592ff7d572a48 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Thu, 2 Jul 2026 16:33:46 +0500 Subject: [PATCH 1/2] fix(workflows): quote-aware interpolation so a literal }} in a filter arg doesn't break multi-expression templates #3208/#3228 hardened the single-expression fast path (_is_single_expression) so a literal {{ or }} inside a string argument like `| default('}}')` stays on the typed path. the multi-expression interpolation path was left on the old _EXPR_PATTERN regex, whose non-greedy `(.+?)}}` body stops at the first }} regardless of quoting. so a multi-expression template with a literal }} in any block captured a truncated body, hit the filter parser malformed, and raised ValueError. e.g. `{{ inputs.name }}: {{ inputs.missing | default('}}') }}` raised instead of interpolating. replace _EXPR_PATTERN.sub with _interpolate_expressions, which scans each block for a }} outside string literals - the same quote handling _is_single_expression already uses. plain-value passthrough (a literal }} in a resolved value, not an expression) is unchanged. add regression tests for a literal }} in the second block and in the first block, plus a literal {{ guard. --- src/specify_cli/workflows/expressions.py | 61 +++++++++++++++++++++--- tests/test_workflows.py | 34 +++++++++++++ 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index cc63be523c..af602c260a 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -183,6 +183,56 @@ def _is_single_expression(stripped: str) -> bool: return True +def _interpolate_expressions(template: str, namespace: dict[str, Any]) -> str: + """Substitute every top-level ``{{ ... }}`` block in *template*, quote-aware. + + Walks the template and, for each block, finds the closing ``}}`` that lies + outside string literals -- the same quote-scanning used by + ``_is_single_expression``. This keeps a literal ``}}`` inside a string + argument (e.g. ``| default('}}')``) from prematurely closing a block. + + ``_EXPR_PATTERN.sub`` cannot do this: its non-greedy body stops at the first + ``}}`` regardless of quoting, so in a multi-expression template any block + whose argument contains a literal ``}}`` is captured truncated and mis-parsed + (raising ``ValueError`` from the filter parser). #3208/#3228 fixed exactly + this for the single-expression fast path but left the interpolation path on + the old regex. + """ + out: list[str] = [] + i = 0 + n = len(template) + while i < n: + start = template.find("{{", i) + if start == -1: + out.append(template[i:]) + break + out.append(template[i:start]) + # Scan for the block-closing ``}}`` that is outside any string literal. + j = start + 2 + quote: str | None = None + close = -1 + while j < n: + ch = template[j] + if quote is not None: + if ch == quote: + quote = None + elif ch in ("'", '"'): + quote = ch + elif ch == "}" and j + 1 < n and template[j + 1] == "}": + close = j + break + j += 1 + if close == -1: + # No closing delimiter: leave the unterminated ``{{`` verbatim, the + # same way the regex leaves an unmatched opener untouched. + out.append(template[start:]) + break + val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace) + out.append(str(val) if val is not None else "") + i = close + 2 + return "".join(out) + + def _split_top_level_commas(text: str) -> list[str]: """Split *text* on commas that are not inside quotes or nested brackets. @@ -472,12 +522,11 @@ def evaluate_expression(template: str, context: Any) -> Any: if _is_single_expression(stripped): return _evaluate_simple_expression(stripped[2:-2].strip(), namespace) - # Multi-expression: string interpolation - def _replacer(m: re.Match[str]) -> str: - val = _evaluate_simple_expression(m.group(1).strip(), namespace) - return str(val) if val is not None else "" - - return _EXPR_PATTERN.sub(_replacer, template) + # Multi-expression: interpolate each block inline. Uses a quote-aware scan + # (not ``_EXPR_PATTERN.sub``) so a literal ``}}`` inside a string argument + # in any block does not close that block early -- matching the handling the + # single-expression path above already got in #3208/#3228. + return _interpolate_expressions(template, namespace) def evaluate_condition(condition: str, context: Any) -> bool: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2fdbf887b3..a6a5ccbb24 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -260,6 +260,40 @@ def test_single_expression_with_literal_braces_preserves_type(self): ctx = StepContext(inputs={"text": "uses }} syntax"}) assert evaluate_expression("{{ inputs.text | contains('}}') }}", ctx) is True + def test_multi_expression_with_literal_close_brace_in_argument(self): + """A multi-expression template with a literal ``}}`` inside a string + argument must interpolate, not raise. #3208/#3228 hardened the single- + expression fast path for literal braces but left the interpolation path + on ``_EXPR_PATTERN``, whose non-greedy body stops at the first ``}}`` -- + so the block was captured truncated and the filter parser raised + ValueError.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"name": "Bob", "missing": None}) + # ``}}`` in the default fallback of the second block. + result = evaluate_expression( + "{{ inputs.name }}: {{ inputs.missing | default('}}') }}", ctx + ) + assert result == "Bob: }}" + # ``}}`` in the first block, expression following it. + result = evaluate_expression( + "{{ inputs.missing | default('}}') }} / {{ inputs.name }}", ctx + ) + assert result == "}} / Bob" + + def test_multi_expression_with_literal_open_brace_in_argument(self): + """A literal ``{{`` inside a string argument in a multi-expression + template must not confuse block detection either.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"name": "Bob", "missing": None}) + result = evaluate_expression( + "{{ inputs.name }} {{ inputs.missing | default('{{') }}", ctx + ) + assert result == "Bob {{" + def test_comparison_equals(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From 14544e9ca250f78eb56ec7b5c7255688fbd90387 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Fri, 3 Jul 2026 05:32:00 +0500 Subject: [PATCH 2/2] fix(workflows): surface malformed templates in interpolation instead of emitting verbatim address copilot review on #3307: when the quote-aware scan finds no block-closing `}}` (e.g. an unbalanced quote in a filter arg swallowed the delimiter), fall back to the first raw `}}` in the tail and evaluate it, so the parser raises ValueError just as the old _EXPR_PATTERN.sub path did. only when there is no `}}` at all is the tail left verbatim (a genuinely unterminated `{{`, which the regex also could not match). keeps a typo failing loudly rather than being silently hidden. add a regression test for an unbalanced quote in a multi-expression template. --- src/specify_cli/workflows/expressions.py | 17 +++++++++++++---- tests/test_workflows.py | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index af602c260a..35f6486b2c 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -223,10 +223,19 @@ def _interpolate_expressions(template: str, namespace: dict[str, Any]) -> str: break j += 1 if close == -1: - # No closing delimiter: leave the unterminated ``{{`` verbatim, the - # same way the regex leaves an unmatched opener untouched. - out.append(template[start:]) - break + # No quote-aware close. Two sub-cases, both kept identical to the old + # regex so a malformed template is never silently hidden: + # * a raw ``}}`` still exists in the tail (e.g. an unbalanced quote + # in a filter arg swallowed the real delimiter) -- fall back to + # that first raw ``}}`` and evaluate, letting the parser surface + # a ValueError just as ``_EXPR_PATTERN.sub`` would have. + # * no ``}}`` at all -- a genuinely unterminated ``{{``; leave the + # tail verbatim, again matching the regex (which cannot match). + raw_close = template.find("}}", start + 2) + if raw_close == -1: + out.append(template[start:]) + break + close = raw_close val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace) out.append(str(val) if val is not None else "") i = close + 2 diff --git a/tests/test_workflows.py b/tests/test_workflows.py index a6a5ccbb24..c0d8d22374 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -294,6 +294,26 @@ def test_multi_expression_with_literal_open_brace_in_argument(self): ) assert result == "Bob {{" + def test_multi_expression_unbalanced_quote_still_raises(self): + """A malformed block (an unbalanced quote in a filter arg) must still + surface a ValueError, not be silently emitted verbatim. + + The quote-aware scan never finds a block-closing ``}}`` when a quote is + left open, but a raw ``}}`` is still present in the tail. It must fall + back to that raw delimiter and evaluate — same as the old regex path — + so a typo fails loudly instead of being hidden (Copilot review on + #3307).""" + import pytest + + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"name": "Bob", "missing": None}) + with pytest.raises(ValueError): + evaluate_expression( + "{{ inputs.name }} {{ inputs.missing | default('oops }}", ctx + ) + def test_comparison_equals(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext