diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index a38cd6cb68..38a29890ae 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -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": diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 57536081ea..295803d4b5 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -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