Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/specify_cli/workflows/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,8 +671,20 @@ def evaluate_condition(condition: str, context: Any) -> bool:
result = evaluate_expression(condition, context)
# Treat plain "false"/"true" strings as booleans so that
# condition: "false" (without {{ }}) behaves as expected.
#
# Strip before matching: the string a condition resolves to is most often
# captured command output, and a ``shell`` step stores ``proc.stdout``
# verbatim, so ``run: echo false`` resolves to ``"false\n"``. Without the
# strip that trailing newline matches neither branch and falls through to
# ``bool("false\n")`` -> True, silently taking an ``if`` step's ``then``
# branch (and keeping a ``while``/``do-while`` looping) on a step that
# printed "false". A workflow cannot strip it itself -- the registered
# filters are default/join/map/contains/from_json, there is no ``trim``.
# ``InitStep._resolve_bool`` and the catalog readers already strip before
# matching boolean text. ``bool(result)`` below still sees the raw string,
# so no non-boolean text changes truthiness.
if isinstance(result, str):
lower = result.lower()
lower = result.strip().lower()
if lower == "false":
return False
if lower == "true":
Expand Down
33 changes: 33 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,39 @@ def test_condition_evaluation(self):
assert evaluate_condition("{{ inputs.ready }}", ctx) is True
assert evaluate_condition("{{ inputs.missing }}", ctx) is False

def test_condition_strips_captured_command_output(self):
"""A condition resolving to captured stdout must honour "false".

A ``shell`` step stores ``proc.stdout`` verbatim, so ``run: echo false``
resolves to ``"false\\n"``. Without stripping, the trailing newline
matched neither the "false" nor the "true" branch and fell through to
``bool("false\\n")`` -> True, so an ``if`` step took its ``then`` branch
on a step that printed "false". There is no ``trim`` filter, so a
workflow author cannot strip it themselves.
"""
from specify_cli.workflows.expressions import evaluate_condition
from specify_cli.workflows.base import StepContext

ctx = StepContext(steps={"check": {"output": {"stdout": "false\n"}}})
assert evaluate_condition("{{ steps.check.output.stdout }}", ctx) is False

for raw in ("false\n", "false\r\n", " false", "false ", "FALSE\n"):
assert evaluate_condition(raw, StepContext()) is False, raw
for raw in ("true\n", " true ", "TRUE\r\n"):
assert evaluate_condition(raw, StepContext()) is True, raw

def test_condition_whitespace_only_string_stays_truthy(self):
"""Stripping must not turn a whitespace-only string into False.

Only the "false"/"true" special case is stripped; everything else still
falls through to ``bool(result)`` on the raw string.
"""
from specify_cli.workflows.expressions import evaluate_condition
from specify_cli.workflows.base import StepContext

assert evaluate_condition(" ", StepContext()) is True
assert evaluate_condition("falsey", StepContext()) is True

def test_non_string_passthrough(self):
from specify_cli.workflows.expressions import evaluate_expression
from specify_cli.workflows.base import StepContext
Expand Down