Skip to content

Commit d7699c3

Browse files
fix(workflows): reject non-list input 'enum' instead of crashing (#3601)
A workflow input whose `enum` is a scalar or string (e.g. `enum: 5`, `enum: "abc"`) previously slipped past `validate_workflow` and crashed at run time. The `value not in enum_values` membership test in `_coerce_input` raises a raw `TypeError` ("argument of type 'int' is not iterable") for a scalar, and a bare string turns enum membership into a silent substring test. The `TypeError` also escapes `validate_workflow`'s `except ValueError`, breaking its documented "return a list of errors, never raise" contract. This is the same unvalidated-`execute()` crash class as the fan-in `wait_for` (#3482) and fan-out step-template (#3537) fixes: `validate()` should reject the value, but the value can still reach the engine via `execute()`, which accepts unvalidated definitions. Fix: - `_coerce_input` requires a list `enum` (or `None`), raising a clean ValueError for any other shape — so both `validate_workflow` and runtime `_resolve_inputs` fail fast with a clear message. - `validate_workflow` checks `enum` shape directly (not only via the default-coercion path, which is reached only when a `default` exists), and strips a malformed `enum` before coercing the default so the wrong-typed-default error is not duplicated as an enum-shape error. - The `integration: auto` sentinel only strips a *list* `enum`; a non-list `enum` stays in the definition so it is rejected rather than silently exempted by the `auto` membership skip. Tests cover all three layers: `_coerce_input` directly, authoring-time `validate_workflow` (with no default present), and runtime `_resolve_inputs`, plus the `integration: auto` interaction. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e9d84ca commit d7699c3

2 files changed

Lines changed: 156 additions & 2 deletions

File tree

src/specify_cli/workflows/engine.py

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,20 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
201201
f"Must be 'string', 'number', or 'boolean'."
202202
)
203203

204+
# ``enum`` must be a list. Checked here — not only via the
205+
# ``_coerce_input`` call below — because that call is reached only
206+
# when a ``default`` is present, and the ``integration: auto`` case
207+
# strips ``enum`` before coercing; a scalar/string ``enum`` on an
208+
# input with no default (or the auto-integration default) would
209+
# otherwise slip through here and then crash ``_resolve_inputs`` with
210+
# a raw ``TypeError`` at run time. ``None`` means "no enum".
211+
enum_values = input_def.get("enum")
212+
if enum_values is not None and not isinstance(enum_values, list):
213+
errors.append(
214+
f"Input {input_name!r} has invalid 'enum': must be a list, "
215+
f"got {type(enum_values).__name__}."
216+
)
217+
204218
# Validate the default eagerly so authoring mistakes (e.g. a
205219
# default not in the declared enum, or a non-numeric default for
206220
# a number input) surface at install/validation time instead of
@@ -209,13 +223,28 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
209223
# enum-membership check is exempted for that exact case — the
210224
# declared type is still enforced (e.g. ``type: number`` paired
211225
# with ``default: "auto"`` is still rejected).
226+
enum_is_valid = enum_values is None or isinstance(enum_values, list)
212227
if "default" in input_def:
213228
default_value = input_def["default"]
214229
is_auto_integration = (
215230
input_name == "integration" and default_value == "auto"
216231
)
232+
# Strip ``enum`` from the definition handed to ``_coerce_input``
233+
# when either:
234+
# * this is the auto-integration sentinel (enum-membership is
235+
# a runtime concern, exempted for ``"auto"``), or
236+
# * the ``enum`` is malformed (non-list) and already reported
237+
# above — leaving it in would make ``_coerce_input`` re-raise
238+
# the same enum-shape error re-framed as an "invalid default"
239+
# (a confusing duplicate).
240+
# Removing *only* ``enum`` (rather than skipping the check
241+
# entirely) preserves the default's type validation: a
242+
# ``type: string`` input with ``default: 5, enum: 5`` still
243+
# reports the wrong-typed default alongside the enum error,
244+
# instead of hiding it.
245+
strip_enum = is_auto_integration or not enum_is_valid
217246
validation_input_def: dict[str, Any] = input_def
218-
if is_auto_integration and "enum" in input_def:
247+
if strip_enum and "enum" in input_def:
219248
validation_input_def = {
220249
key: value
221250
for key, value in input_def.items()
@@ -1400,11 +1429,18 @@ def _resolve_inputs(
14001429
# definition (``string`` rejects non-strings, ``number`` rejects
14011430
# bools and uncoercible values, ``boolean`` rejects non-bools),
14021431
# so ill-typed values still fail fast here.
1432+
#
1433+
# ``execute()`` accepts unvalidated definitions, so a malformed
1434+
# (non-list) ``enum`` can reach here. Only strip a *list* ``enum``:
1435+
# a scalar/string ``enum`` must stay in the definition so
1436+
# ``_coerce_input`` raises the clean shape ``ValueError`` instead of
1437+
# being silently exempted by the ``auto`` membership skip (which
1438+
# would otherwise let ``enum: 5`` resolve successfully).
14031439
coerce_input_def = input_def
14041440
if (
14051441
name == "integration"
14061442
and value == "auto"
1407-
and "enum" in input_def
1443+
and isinstance(input_def.get("enum"), list)
14081444
):
14091445
coerce_input_def = {
14101446
key: val
@@ -1450,6 +1486,22 @@ def _coerce_input(
14501486
input_type = input_def.get("type", "string")
14511487
enum_values = input_def.get("enum")
14521488

1489+
# ``enum`` must be a list. A scalar (``enum: 5``, ``enum: true``) makes
1490+
# the ``value not in enum_values`` membership test below raise a raw
1491+
# ``TypeError`` ("argument of type 'int' is not ... iterable"), which
1492+
# escapes ``validate_workflow``'s ``except ValueError`` and breaks its
1493+
# "return errors, never raise" contract — and crashes ``_resolve_inputs``
1494+
# outright at run time. A bare string is just as wrong: ``value in "abc"``
1495+
# is a silent substring/character test, not enum membership. Require a
1496+
# list so both forms fail fast with a clear message. ``None`` means "no
1497+
# enum" and is left alone.
1498+
if enum_values is not None and not isinstance(enum_values, list):
1499+
msg = (
1500+
f"Input {name!r} has invalid 'enum': must be a list, got "
1501+
f"{type(enum_values).__name__}."
1502+
)
1503+
raise ValueError(msg)
1504+
14531505
if input_type == "number":
14541506
# Reject bools explicitly: ``bool`` is a subclass of ``int`` so
14551507
# ``float(True)`` succeeds and would silently coerce a YAML

tests/test_workflows.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4349,6 +4349,108 @@ def test_coerce_number_input_rejects_infinity_cleanly(self):
43494349
assert WorkflowEngine._coerce_input("count", 5.0, {"type": "number"}) == 5
43504350
assert WorkflowEngine._coerce_input("count", 3.5, {"type": "number"}) == 3.5
43514351

4352+
def test_coerce_input_rejects_non_list_enum_cleanly(self):
4353+
"""A non-list ``enum`` (scalar or string) must raise a clean ValueError,
4354+
not the raw ``TypeError`` from the ``value not in enum`` membership test.
4355+
4356+
A scalar (``enum: 5``) makes ``value not in 5`` raise
4357+
``TypeError: argument of type 'int' is not iterable``. A bare string
4358+
(``enum: "abc"``) is silently wrong instead — ``value in "abc"`` is a
4359+
substring test, not enum membership — so it must be rejected too.
4360+
"""
4361+
from specify_cli.workflows.engine import WorkflowEngine
4362+
4363+
for bad_enum in (5, True, "abc", {"a": 1}):
4364+
with pytest.raises(ValueError, match="invalid 'enum': must be a list"):
4365+
WorkflowEngine._coerce_input(
4366+
"scope", "x", {"type": "string", "enum": bad_enum}
4367+
)
4368+
# A valid list ``enum`` still works, and ``None`` means "no enum".
4369+
assert (
4370+
WorkflowEngine._coerce_input(
4371+
"scope", "a", {"type": "string", "enum": ["a", "b"]}
4372+
)
4373+
== "a"
4374+
)
4375+
assert (
4376+
WorkflowEngine._coerce_input("scope", "x", {"type": "string"}) == "x"
4377+
)
4378+
4379+
def test_validate_workflow_rejects_non_list_enum(self):
4380+
"""A non-list ``enum`` must be reported as an error, not crash
4381+
``validate_workflow``. The membership test would raise ``TypeError``,
4382+
which escapes its ``except ValueError`` and breaks the "return a list of
4383+
errors, never raise" contract. This must surface even with no ``default``
4384+
present (the coercion path that would otherwise catch it is only reached
4385+
when a default exists).
4386+
"""
4387+
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow
4388+
4389+
definition = WorkflowDefinition.from_string("""
4390+
schema_version: "1.0"
4391+
workflow:
4392+
id: "bad-enum"
4393+
name: "Bad Enum"
4394+
version: "1.0.0"
4395+
inputs:
4396+
scope:
4397+
type: string
4398+
enum: 5
4399+
steps:
4400+
- id: noop
4401+
type: gate
4402+
message: "noop"
4403+
options: [approve]
4404+
""")
4405+
errors = validate_workflow(definition)
4406+
assert any("invalid 'enum': must be a list" in e for e in errors), errors
4407+
4408+
def test_resolve_inputs_rejects_non_list_enum_at_runtime(self, project_dir):
4409+
"""``execute()`` accepts unvalidated definitions, so a non-list ``enum``
4410+
can reach ``_resolve_inputs`` at run time. It must fail with a clean
4411+
ValueError rather than the raw ``TypeError`` from the membership test.
4412+
"""
4413+
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition
4414+
4415+
definition = WorkflowDefinition.from_string("""
4416+
schema_version: "1.0"
4417+
workflow:
4418+
id: "runtime-bad-enum"
4419+
name: "Runtime Bad Enum"
4420+
version: "1.0.0"
4421+
inputs:
4422+
scope:
4423+
type: string
4424+
enum: 5
4425+
""")
4426+
engine = WorkflowEngine(project_dir)
4427+
with pytest.raises(ValueError, match="invalid 'enum': must be a list"):
4428+
engine._resolve_inputs(definition, {"scope": "x"})
4429+
4430+
def test_non_list_enum_on_integration_auto_still_rejected(self, project_dir):
4431+
"""The ``integration: auto`` sentinel strips a *list* ``enum`` before
4432+
coercion (enum-membership is a runtime concern for ``auto``). A non-list
4433+
``enum`` must NOT be silently stripped by that path — it is still an
4434+
authoring error and must fail with the clean shape ValueError.
4435+
"""
4436+
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition
4437+
4438+
definition = WorkflowDefinition.from_string("""
4439+
schema_version: "1.0"
4440+
workflow:
4441+
id: "auto-bad-enum"
4442+
name: "Auto Bad Enum"
4443+
version: "1.0.0"
4444+
inputs:
4445+
integration:
4446+
type: string
4447+
default: "auto"
4448+
enum: 5
4449+
""")
4450+
engine = WorkflowEngine(project_dir)
4451+
with pytest.raises(ValueError, match="invalid 'enum': must be a list"):
4452+
engine._resolve_inputs(definition, {})
4453+
43524454
def test_validate_workflow_rejects_infinite_default_for_number_type(self):
43534455
"""``type: number`` with an infinite default (YAML ``.inf``) must be
43544456
reported as an error, not raise. ``int(inf)`` raises OverflowError during

0 commit comments

Comments
 (0)