Skip to content

Commit 043c4ec

Browse files
jawwad-aliclaude
andauthored
fix(workflows): filter parser rejects trailing tokens (fullmatch, not match) (#3689)
_apply_filter parsed a name(arg) filter with an UNANCHORED regex (re.match(r"(\w+)\((.+)\)")), so any tokens after the closing paren were silently discarded. Because _evaluate_simple_expression splits the top-level pipe before comparison/boolean operators, `count | default(0) > 5` was split into value `count` and filter segment `default(0) > 5`; the segment matched as `default(0)` and `> 5` vanished — the filter's value was returned as the whole expression, giving a silently wrong result. Use re.fullmatch so a mis-wired segment falls through to the existing "unsupported form" ValueError, mirroring the from_json branch's strict trailing-token handling. The greedy `.+` still matches legitimate forms (literal `)` / `|` inside quoted args), so registered/chained/quoted-pipe filters are unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5ad3128 commit 043c4ec

2 files changed

Lines changed: 30 additions & 2 deletions

File tree

src/specify_cli/workflows/expressions.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,14 @@ def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> An
392392
)
393393
return _filter_from_json(value)
394394

395-
# Parse filter name and argument
396-
filter_match = re.match(r"(\w+)\((.+)\)", filter_expr)
395+
# Parse filter name and argument. Use fullmatch (not match) so trailing
396+
# tokens after the closing paren — e.g. a comparison/boolean operator that
397+
# binds looser than the pipe, as in ``count | default(0) > 5`` — are not
398+
# silently discarded but fall through to the "unsupported form" ValueError
399+
# below, mirroring the strict trailing-token handling of the from_json
400+
# branch above. The greedy ``.+`` still handles literal ``)`` and ``|``
401+
# inside quoted args.
402+
filter_match = re.fullmatch(r"(\w+)\((.+)\)", filter_expr)
397403
if filter_match:
398404
fname = filter_match.group(1)
399405
farg = _evaluate_simple_expression(filter_match.group(2).strip(), namespace)

tests/test_workflows.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,28 @@ def test_registered_filter_unsupported_form_raises(self):
686686
):
687687
evaluate_expression("{{ inputs.tags | map }}", ctx)
688688

689+
def test_filter_call_with_trailing_tokens_fails_loudly(self):
690+
# A trailing operator/token after a filter's closing paren must not be
691+
# silently discarded (the parser used an unanchored regex). It must
692+
# fall through to the "unsupported form" ValueError, like the from_json
693+
# branch's strict trailing-token handling.
694+
import pytest
695+
from specify_cli.workflows.expressions import evaluate_expression
696+
from specify_cli.workflows.base import StepContext
697+
698+
# A comparison after a filter (binds looser than the pipe) was dropped,
699+
# so `default('7') > '5'` silently returned '7'.
700+
with pytest.raises(ValueError, match="unsupported form"):
701+
evaluate_expression(
702+
"{{ inputs.missing | default('7') > '5' }}", StepContext(inputs={})
703+
)
704+
# Trailing garbage after a valid filter call.
705+
with pytest.raises(ValueError, match="unsupported form"):
706+
evaluate_expression(
707+
"{{ inputs.tags | join(',') extra }}",
708+
StepContext(inputs={"tags": ["a", "b"]}),
709+
)
710+
689711
def test_chained_filters_apply_left_to_right(self):
690712
# Filters chain: each filter's result feeds the next. `map` yields a
691713
# list and `join` is the only filter that renders a list to a string,

0 commit comments

Comments
 (0)