Skip to content
Merged
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
56 changes: 54 additions & 2 deletions src/specify_cli/workflows/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,20 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
f"Must be 'string', 'number', or 'boolean'."
)

# ``enum`` must be a list. Checked here — not only via the
# ``_coerce_input`` call below — because that call is reached only
# when a ``default`` is present, and the ``integration: auto`` case
# strips ``enum`` before coercing; a scalar/string ``enum`` on an
# input with no default (or the auto-integration default) would
# otherwise slip through here and then crash ``_resolve_inputs`` with
# a raw ``TypeError`` at run time. ``None`` means "no enum".
enum_values = input_def.get("enum")
if enum_values is not None and not isinstance(enum_values, list):
errors.append(
f"Input {input_name!r} has invalid 'enum': must be a list, "
f"got {type(enum_values).__name__}."
)

# Validate the default eagerly so authoring mistakes (e.g. a
# default not in the declared enum, or a non-numeric default for
# a number input) surface at install/validation time instead of
Expand All @@ -201,13 +215,28 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
# enum-membership check is exempted for that exact case — the
# declared type is still enforced (e.g. ``type: number`` paired
# with ``default: "auto"`` is still rejected).
enum_is_valid = enum_values is None or isinstance(enum_values, list)
if "default" in input_def:
default_value = input_def["default"]
is_auto_integration = (
input_name == "integration" and default_value == "auto"
)
# Strip ``enum`` from the definition handed to ``_coerce_input``
# when either:
# * this is the auto-integration sentinel (enum-membership is
# a runtime concern, exempted for ``"auto"``), or
# * the ``enum`` is malformed (non-list) and already reported
# above — leaving it in would make ``_coerce_input`` re-raise
# the same enum-shape error re-framed as an "invalid default"
# (a confusing duplicate).
# Removing *only* ``enum`` (rather than skipping the check
# entirely) preserves the default's type validation: a
# ``type: string`` input with ``default: 5, enum: 5`` still
# reports the wrong-typed default alongside the enum error,
# instead of hiding it.
strip_enum = is_auto_integration or not enum_is_valid
validation_input_def: dict[str, Any] = input_def
if is_auto_integration and "enum" in input_def:
if strip_enum and "enum" in input_def:
validation_input_def = {
key: value
for key, value in input_def.items()
Expand Down Expand Up @@ -1381,11 +1410,18 @@ def _resolve_inputs(
# definition (``string`` rejects non-strings, ``number`` rejects
# bools and uncoercible values, ``boolean`` rejects non-bools),
# so ill-typed values still fail fast here.
#
# ``execute()`` accepts unvalidated definitions, so a malformed
# (non-list) ``enum`` can reach here. Only strip a *list* ``enum``:
# a scalar/string ``enum`` must stay in the definition so
# ``_coerce_input`` raises the clean shape ``ValueError`` instead of
# being silently exempted by the ``auto`` membership skip (which
# would otherwise let ``enum: 5`` resolve successfully).
coerce_input_def = input_def
if (
name == "integration"
and value == "auto"
and "enum" in input_def
and isinstance(input_def.get("enum"), list)
):
coerce_input_def = {
key: val
Expand Down Expand Up @@ -1431,6 +1467,22 @@ def _coerce_input(
input_type = input_def.get("type", "string")
enum_values = input_def.get("enum")

# ``enum`` must be a list. A scalar (``enum: 5``, ``enum: true``) makes
# the ``value not in enum_values`` membership test below raise a raw
# ``TypeError`` ("argument of type 'int' is not ... iterable"), which
# escapes ``validate_workflow``'s ``except ValueError`` and breaks its
# "return errors, never raise" contract — and crashes ``_resolve_inputs``
# outright at run time. A bare string is just as wrong: ``value in "abc"``
# is a silent substring/character test, not enum membership. Require a
# list so both forms fail fast with a clear message. ``None`` means "no
# enum" and is left alone.
if enum_values is not None and not isinstance(enum_values, list):
msg = (
f"Input {name!r} has invalid 'enum': must be a list, got "
f"{type(enum_values).__name__}."
)
raise ValueError(msg)

if input_type == "number":
# Reject bools explicitly: ``bool`` is a subclass of ``int`` so
# ``float(True)`` succeeds and would silently coerce a YAML
Expand Down
102 changes: 102 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -4248,6 +4248,108 @@ def test_coerce_number_input_rejects_infinity_cleanly(self):
assert WorkflowEngine._coerce_input("count", 5.0, {"type": "number"}) == 5
assert WorkflowEngine._coerce_input("count", 3.5, {"type": "number"}) == 3.5

def test_coerce_input_rejects_non_list_enum_cleanly(self):
"""A non-list ``enum`` (scalar or string) must raise a clean ValueError,
not the raw ``TypeError`` from the ``value not in enum`` membership test.

A scalar (``enum: 5``) makes ``value not in 5`` raise
``TypeError: argument of type 'int' is not iterable``. A bare string
(``enum: "abc"``) is silently wrong instead — ``value in "abc"`` is a
substring test, not enum membership — so it must be rejected too.
"""
from specify_cli.workflows.engine import WorkflowEngine

for bad_enum in (5, True, "abc", {"a": 1}):
with pytest.raises(ValueError, match="invalid 'enum': must be a list"):
WorkflowEngine._coerce_input(
"scope", "x", {"type": "string", "enum": bad_enum}
)
# A valid list ``enum`` still works, and ``None`` means "no enum".
assert (
WorkflowEngine._coerce_input(
"scope", "a", {"type": "string", "enum": ["a", "b"]}
)
== "a"
)
assert (
WorkflowEngine._coerce_input("scope", "x", {"type": "string"}) == "x"
)

def test_validate_workflow_rejects_non_list_enum(self):
"""A non-list ``enum`` must be reported as an error, not crash
``validate_workflow``. The membership test would raise ``TypeError``,
which escapes its ``except ValueError`` and breaks the "return a list of
errors, never raise" contract. This must surface even with no ``default``
present (the coercion path that would otherwise catch it is only reached
when a default exists).
"""
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "bad-enum"
name: "Bad Enum"
version: "1.0.0"
inputs:
scope:
type: string
enum: 5
steps:
- id: noop
type: gate
message: "noop"
options: [approve]
""")
errors = validate_workflow(definition)
assert any("invalid 'enum': must be a list" in e for e in errors), errors

def test_resolve_inputs_rejects_non_list_enum_at_runtime(self, project_dir):
"""``execute()`` accepts unvalidated definitions, so a non-list ``enum``
can reach ``_resolve_inputs`` at run time. It must fail with a clean
ValueError rather than the raw ``TypeError`` from the membership test.
"""
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "runtime-bad-enum"
name: "Runtime Bad Enum"
version: "1.0.0"
inputs:
scope:
type: string
enum: 5
""")
engine = WorkflowEngine(project_dir)
with pytest.raises(ValueError, match="invalid 'enum': must be a list"):
engine._resolve_inputs(definition, {"scope": "x"})

def test_non_list_enum_on_integration_auto_still_rejected(self, project_dir):
"""The ``integration: auto`` sentinel strips a *list* ``enum`` before
coercion (enum-membership is a runtime concern for ``auto``). A non-list
``enum`` must NOT be silently stripped by that path — it is still an
authoring error and must fail with the clean shape ValueError.
"""
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "auto-bad-enum"
name: "Auto Bad Enum"
version: "1.0.0"
inputs:
integration:
type: string
default: "auto"
enum: 5
""")
engine = WorkflowEngine(project_dir)
with pytest.raises(ValueError, match="invalid 'enum': must be a list"):
engine._resolve_inputs(definition, {})

def test_validate_workflow_rejects_infinite_default_for_number_type(self):
"""``type: number`` with an infinite default (YAML ``.inf``) must be
reported as an error, not raise. ``int(inf)`` raises OverflowError during
Expand Down