fix(workflows): guard the shell step's timeout check against OverflowError (#3865)

PR #3847 hardened the prompt step's `timeout` guard against a huge-int
value, but its twin in the shell step — the step the prompt one was
mirrored from — still has the hole.

`math.isfinite(10**400)` raises `OverflowError: int too large to convert
to float`. A 400-digit YAML scalar is an `int` and is not a `bool`, so it
clears every clause before `isfinite()` and raises there, escaping
`_timeout_error()` as exactly the uncaught crash that helper exists to
prevent:

    steps:
      - id: qa
        type: shell
        run: echo hi
        timeout: 1000...0   # 400 digits

    $ specify workflow run wf.yml
    Traceback (most recent call last):
      ...
      File "src/specify_cli/workflows/engine.py", line 361, in _validate_steps
        step_errors = step_impl.validate(step_config)
      File "src/specify_cli/workflows/steps/shell/__init__.py", line 127
        or not math.isfinite(timeout)
    OverflowError: int too large to convert to float

`workflow_run` calls `engine.validate()` before executing any step, so
the OverflowError propagates out of `validate_workflow` and kills the
command with a bare traceback that names neither the step nor the field,
instead of the "Workflow validation failed" report. `execute()` shares
the same helper, so an unvalidated run raises there too — and the engine
re-raises anything a step throws, aborting the whole workflow after
earlier steps have already run their side effects. The value is
genuinely invalid rather than merely unrepresentable in the check:
`subprocess.run(timeout=10**400)` raises the same OverflowError.

Unlike the prompt step, the shell step checks `isfinite()` *before*
`timeout <= 0`, so a negative huge int (`-(10**400)`) crashes as well
rather than being caught by the sign check.

Wrapped the condition in `try/except OverflowError` and treated the
value as invalid, mirroring the prompt step's guard so both steps reject
the same values with the same message. Now:

    Workflow validation failed:
      - Shell step 'qa': 'timeout' must be a positive number of seconds,
        got 1000...0.

Valid int/float timeouts, non-finite floats, bools, strings and
non-positive values are unaffected — the existing clauses are unchanged.

Regression tests in `TestShellStep`: `validate()` rejects both signs of
the huge int, `validate_workflow()` reports it end to end (pinning the
path the CLI actually takes, not just the helper), and `execute()` fails
only that step with `subprocess.run` patched to assert it is never
reached. Test-the-test: reverting the source change fails all three with
`OverflowError` and leaves the rest of `TestShellStep` passing.


Assisted-by: Claude Code (model: claude-opus-5, under direct human supervision)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/specify_cli/workflows/steps/shell/__init__.py

@@ -121,12 +121,20 @@ class ShellStep(StepBase):
         if "timeout" not in config:
             return None
         timeout = config["timeout"]
-        if (
-            isinstance(timeout, bool)
-            or not isinstance(timeout, (int, float))
-            or not math.isfinite(timeout)
-            or timeout <= 0
-        ):
+        try:
+            invalid_timeout = (
+                isinstance(timeout, bool)
+                or not isinstance(timeout, (int, float))
+                or not math.isfinite(timeout)
+                or timeout <= 0
+            )
+        except OverflowError:
+            # An int too large to convert to float (e.g. a 400-digit YAML
+            # scalar) is not a bool and *is* an int, so it clears every clause
+            # before ``isfinite()`` and raises there — and would raise the same
+            # from subprocess.run(timeout=...). Mirrors the prompt step.
+            invalid_timeout = True
+        if invalid_timeout:
             return (
                 f"Shell step {config.get('id', '?')!r}: 'timeout' must be a "
                 f"positive number of seconds, got {timeout!r}."

tests/test_workflows.py

@@ -2027,6 +2027,75 @@ class TestShellStep:
             errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
             assert any("'timeout' must be a positive number" in e for e in errors)
 
+    def test_validate_rejects_huge_int_timeout(self):
+        """A too-large-to-convert int must be reported, not raise OverflowError.
+
+        ``math.isfinite(10**400)`` raises ``OverflowError: int too large to
+        convert to float``. Such a value is an ``int`` and is not a ``bool``,
+        so it clears every clause before ``isfinite()`` and raises there —
+        escaping ``validate()`` as the uncaught crash this guard exists to
+        prevent. ``specify workflow run`` then aborts with a bare traceback
+        instead of "Workflow validation failed". Both signs reach
+        ``isfinite()`` because it is checked before ``timeout <= 0``.
+        ``subprocess.run(timeout=...)`` raises the same OverflowError, so the
+        value is genuinely invalid rather than merely unrepresentable here.
+        The prompt step already catches this (PR #3847).
+        """
+        from specify_cli.workflows.steps.shell import ShellStep
+
+        step = ShellStep()
+        for bad in (10**400, -(10**400)):
+            errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
+            assert any(
+                "'timeout' must be a positive number" in e for e in errors
+            ), (bad, errors)
+
+    def test_validate_workflow_reports_huge_int_timeout(self):
+        """The huge-int timeout surfaces as a validation error end to end.
+
+        ``specify workflow run`` calls ``engine.validate()`` before executing
+        any step; an OverflowError escaping the shell step's ``validate()``
+        propagates out of ``validate_workflow`` and kills the command with a
+        traceback, so pin the whole path, not just the helper.
+        """
+        from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow
+
+        definition = WorkflowDefinition(
+            {
+                "schema_version": "1.0",
+                "workflow": {"id": "demo", "name": "Demo", "version": "1.0.0"},
+                "steps": [
+                    {"id": "qa", "type": "shell", "run": "echo hi", "timeout": 10**400}
+                ],
+            }
+        )
+        errors = validate_workflow(definition)
+        assert any("'timeout' must be a positive number" in e for e in errors), errors
+
+    def test_execute_fails_cleanly_on_huge_int_timeout(self, monkeypatch):
+        """execute() must fail just this step on a huge-int timeout.
+
+        The engine does not auto-validate step config and re-raises anything a
+        step throws, so on an unvalidated run the OverflowError would abort the
+        whole workflow after earlier steps had already run their side effects.
+        """
+        import subprocess
+
+        from specify_cli.workflows.steps.shell import ShellStep
+        from specify_cli.workflows.base import StepContext, StepStatus
+
+        def fail_if_called(*args, **kwargs):
+            raise AssertionError("subprocess.run should not run on invalid timeout")
+
+        monkeypatch.setattr(subprocess, "run", fail_if_called)
+        step = ShellStep()
+        for bad in (10**400, -(10**400)):
+            result = step.execute(
+                {"id": "qa", "run": "echo hi", "timeout": bad}, StepContext()
+            )
+            assert result.status == StepStatus.FAILED, bad
+            assert "'timeout' must be a positive number" in (result.error or ""), bad
+
     def test_validate_accepts_positive_numeric_timeout(self):
         from specify_cli.workflows.steps.shell import ShellStep