Skip to content

Commit b8bbc8a

Browse files
doquanghuyclaude
andcommitted
fix(expressions): make from_json strict — reject any arguments
Address review (#2961): from_json('x') and from_json() previously fell through to a silent passthrough of the unparsed value. Reject any parenthesized form with a clear error so mis-wired templates fail loudly. Rename test to ...parses_object (JSON under test is an object) and add coverage for the strict no-arguments behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 39316e5 commit b8bbc8a

2 files changed

Lines changed: 27 additions & 3 deletions

File tree

src/specify_cli/workflows/expressions.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,17 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
158158
value = _evaluate_simple_expression(parts[0].strip(), namespace)
159159
filter_expr = parts[1].strip()
160160

161+
# `from_json` is strict and takes no arguments. Reject any
162+
# parenthesized form (`from_json()` or `from_json('x')`) so a
163+
# mis-wired template fails loudly instead of silently returning the
164+
# unparsed value.
165+
if filter_expr.startswith("from_json("):
166+
raise ValueError(
167+
"from_json: filter takes no arguments (use '| from_json')"
168+
)
169+
if filter_expr == "from_json":
170+
return _filter_from_json(value)
171+
161172
# Parse filter name and argument
162173
filter_match = re.match(r"(\w+)\((.+)\)", filter_expr)
163174
if filter_match:
@@ -175,8 +186,6 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
175186
filter_name = filter_expr.strip()
176187
if filter_name == "default":
177188
return _filter_default(value)
178-
if filter_name == "from_json":
179-
return _filter_from_json(value)
180189
return value
181190

182191
# Boolean operators — parse 'or' first (lower precedence) so that

tests/test_workflows.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ def test_filter_contains(self):
286286
ctx = StepContext(inputs={"text": "hello world"})
287287
assert evaluate_expression("{{ inputs.text | contains('world') }}", ctx) is True
288288

289-
def test_filter_from_json_parses_list(self):
289+
def test_filter_from_json_parses_object(self):
290290
from specify_cli.workflows.expressions import evaluate_expression
291291
from specify_cli.workflows.base import StepContext
292292

@@ -314,6 +314,21 @@ def test_filter_from_json_non_string_raises(self):
314314
with pytest.raises(ValueError, match="expected a JSON string"):
315315
evaluate_expression("{{ steps.emit.output.exit_code | from_json }}", ctx)
316316

317+
def test_filter_from_json_rejects_arguments(self):
318+
# `from_json` is strict and takes no arguments. Both the empty-parens
319+
# and accidental-arg forms must raise rather than silently return the
320+
# unparsed value, so a mis-wired template is caught immediately.
321+
import pytest
322+
from specify_cli.workflows.expressions import evaluate_expression
323+
from specify_cli.workflows.base import StepContext
324+
325+
ctx = StepContext(steps={"emit": {"output": {"stdout": '{"a": 1}'}}})
326+
for bad in ("from_json()", "from_json('x')"):
327+
with pytest.raises(ValueError, match="from_json: filter takes no arguments"):
328+
evaluate_expression(
329+
"{{ steps.emit.output.stdout | " + bad + " }}", ctx
330+
)
331+
317332
def test_condition_evaluation(self):
318333
from specify_cli.workflows.expressions import evaluate_condition
319334
from specify_cli.workflows.base import StepContext

0 commit comments

Comments
 (0)