Skip to content

Validate CallService.dial timeout before sending ATD #288

Description

@Justinabox

Summary

CallService.dial() accepts zero, negative, boolean, NaN, and infinite per-call timeouts without validating them before sending ATD. In particular, a zero timeout still writes the dial command before the executor raises ATTimeoutError; an infinite timeout can make the command wait indefinitely.

Root cause

  • callstack/voice/service.py:77-101 forwards the public timeout argument directly to ATCommandExecutor.execute() with no validation.
  • callstack/protocol/executor.py:265-273 writes the command before calling _collect_response().
  • _collect_response() calculates deadline = loop.time() + timeout at line 436 and only then rejects an already-expired timeout at lines 440-444. It has no finite-value validation.

This differs from nearby public boundaries: USSDService.send(), network registration polling, DTMF collection, modem config, and hardware probing validate timeout values before a modem operation begins.

Reproduction (no hardware)

Baseline at 6dd4211173857e6da55e6e2d0984c06fc56f9a7c was healthy:

$ git diff --check
# exit 0

$ PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q
1093 passed in 7.95s

With MockTransport and the real ATCommandExecutor:

await service.dial("5551234", timeout=0)

Output:

actual_executor_zero_timeout=ATTimeoutError written='ATD5551234;\\r\\n'

A recording executor also confirms that non-finite input reaches the modem layer unchanged:

dial_timeout_zero=session=True state=2 calls=[('ATD5551234;', 0)]
dial_timeout_infinity=session=True state=2 calls=[('ATD5551234;', inf)]

The recipient above is a dummy test value; no hardware or live SMS/call operation was used.

Impact

A caller-side timeout validation error can still initiate a real outbound call. timeout=0 reports a timeout after issuing ATD, while timeout=inf can wait forever for a modem response. Both violate the fail-before-side-effect boundary expected for a PBX control API.

Suggested direction

Add one shared positive-finite, non-boolean timeout validator at the public voice boundary and call it before the FSM transition and before ATD is handed to the executor. Consider applying the same validation at the generic executor boundary as defense in depth, but keep this issue scoped to the public dial contract and its regression tests.

Acceptance criteria

  • CallService.dial(..., timeout=0), negative values, booleans, NaN, +/-inf, and non-numbers raise a clear ValueError before any executor/modem write.
  • A valid finite positive timeout retains normal dial behavior.
  • The FSM remains IDLE and active_call remains None after rejected input.
  • Tests exercise the real executor + MockTransport for the zero-timeout no-write regression.
  • Logs do not disclose full phone numbers.

Verification gates

git diff --check
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/test_call_service.py tests/test_executor.py -q
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q

Duplicate check

Searched open and closed issues/PRs for CallService dial timeout validation and ATD timeout NaN infinity; no matching issue or PR was returned. Nearby issue #9 is broad sequencing only, while current timeout hardening covers config, network, DTMF, USSD, and probing rather than the public dial override.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions