From 99d61fb75305dd86e3ffe80953dda237bb412023 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Sun, 13 Sep 2026 21:16:24 +0500 Subject: [PATCH] fix(workflows): stop list-literal-then-index expressions silently corrupting _evaluate_simple_expression's list-literal detection was `expr.startswith("[") and expr.endswith("]")` -- true not only for a genuine literal like `[1, 2, 3]` but also for a list literal immediately followed by an index suffix, e.g. `[1,2,3][1]` (read as "index 1 of [1,2,3]", i.e. 2). Naively stripping the outer brackets from that string produces the garbage `1,2,3][1`, which the comma-splitter then breaks into `["1", "2", "3][1"]`; the last segment resolves to None via the dot-path fallback, so `{{ [1,2,3][1] }}` silently evaluated to `[1, 2, None]` instead of raising or resolving the index -- no error, no warning, just wrong data. This is the same "grabs the wrong span" failure mode the adjacent string-literal check already guards against (verifying the matching quote is the last character, not just present), just never given the same treatment for brackets. Added _is_single_list_literal: a quote/bracket-depth scan (mirroring _split_top_level_commas's existing tracking in this same file) that confirms the opening `[` closes exactly at the final character before treating the expression as one literal. A misclassified expression now falls through to the existing dot-path resolution and evaluates to None -- consistent with how every other unresolvable expression in this module already behaves, not a new failure mode. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01U74yBbvVQCPwB7Ed8Dzeu6 --- src/specify_cli/workflows/expressions.py | 37 +++++++++++++++++++++++- tests/test_workflows.py | 23 +++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index fe8098f321..426f71af9b 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -416,6 +416,41 @@ def _find_top_level(text: str, token: str) -> int: return -1 +def _is_single_list_literal(expr: str) -> bool: + """Return ``True`` only when *expr* is exactly one bracketed list + literal -- the opening ``[`` closes at the FINAL character, not partway + through the string. + + ``expr.startswith("[") and expr.endswith("]")`` alone also matches a + list literal immediately followed by an index suffix, e.g. + ``[1,2,3][1]`` (meant as "index 1 of [1,2,3]", i.e. 2). Naively + stripping the outer brackets from that string yields the garbage + ``1,2,3][1``, which then silently evaluates to ``[1, 2, None]`` instead + of raising or resolving the index -- the same "grabs the wrong span" + failure mode the string-literal check above guards against, just never + given the same treatment for brackets. + """ + if not (expr.startswith("[") and expr.endswith("]")): + return False + quote: str | None = None + depth = 0 + n = len(expr) + for i, ch in enumerate(expr): + if quote is not None: + if ch == quote: + quote = None + continue + if ch in ("'", '"'): + quote = ch + elif ch in "([{": + depth += 1 + elif ch in ")]}": + depth -= 1 + if depth == 0: + return i == n - 1 + return False + + def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> Any: """Apply a single pipe filter segment to *value*. @@ -647,7 +682,7 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: return None # List literal (simple) - if expr.startswith("[") and expr.endswith("]"): + if _is_single_list_literal(expr): inner = expr[1:-1].strip() if not inner: return [] diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2c7141e954..9262101f5f 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -418,6 +418,29 @@ def test_list_literal_ignores_trailing_and_empty_commas(self): # …but an intentional empty-string element is still preserved. assert evaluate_expression("{{ ['', 'a'] }}", ctx) == ["", "a"] + def test_list_literal_followed_by_index_is_not_misparsed_as_one_literal(self): + """A list literal immediately followed by an index suffix, e.g. + ``[1,2,3][1]``, both starts with ``[`` and ends with ``]`` -- the + same shape as a genuine single list literal. Naively stripping the + outer brackets from ``[1,2,3][1]`` yields ``1,2,3][1``, which then + silently evaluates to ``[1, 2, None]`` instead of raising or + resolving the index. It must not be misclassified as one literal; + falling through to unresolvable (``None``) is safe, unlike silently + returning a wrong-looking list. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext() + assert evaluate_expression("{{ [1,2,3][1] }}", ctx) is None + assert evaluate_expression("{{ [1,2][0] }}", ctx) is None + # Genuine list literals -- including ones a bracket-depth scan must + # still recognize as ending exactly at the final character -- are + # unaffected. + assert evaluate_expression("{{ [1, 2, 3] }}", ctx) == [1, 2, 3] + assert evaluate_expression("{{ [[1, 2], 3] }}", ctx) == [[1, 2], 3] + assert evaluate_expression("{{ ['a]', 'b'] }}", ctx) == ["a]", "b"] + def test_operator_splitting_is_quote_aware(self): from specify_cli.workflows.expressions import ( evaluate_condition,