Skip to content

Validate API key rate-limit window to prevent silent throttle bypass #278

Description

@Justinabox

Summary

APIKeyAuth accepts a zero or negative rate_window without validation, silently disabling the configured per-key rate limit. This is a deployment hardening gap for callers that instantiate the middleware directly with a configuration value.

Root cause

APIKeyAuth.__init__() assigns rate_limit and rate_window directly in server.py:85-97. In the middleware, each valid request calculates cutoff = now - self._rate_window and retains timestamps only when t > cutoff (server.py:115-128). With rate_window=0, every earlier timestamp is pruned on the next request, so the log never reaches a positive rate_limit.

The constructor also accepts negative or non-integral/non-finite values even though they do not define a safe, bounded rate-limit policy.

Reproduction

Baseline main is 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
1043 passed in 7.29s

No hardware is required. Running the middleware three times with a valid bearer key and APIKeyAuth(api_keys=[...], rate_limit=1, rate_window=0) returns:

zero_rate_window_statuses [200, 200, 200]

Expected behavior is either constructor rejection of the invalid policy or a 429 after the first accepted request; it must not silently remove the only per-key throttle.

Impact

A mistaken zero/negative rate-window configuration disables the valid-key request limit, allowing unbounded authenticated HTTP traffic to SMS/USSD/modem endpoints. This remains distinct from #120 / PR #121, which addresses throttling failed credentials rather than validating the configured window for successful credentials.

Suggested fix direction

Validate rate-limit policy at APIKeyAuth construction time before mutating instance state:

  • require rate_limit to be a non-boolean positive integer;
  • require rate_window to be a non-boolean positive finite number (or a positive integer if that is the intended API);
  • raise a clear ValueError for zero, negative, boolean, NaN, infinity, and invalid types.

Keep normal valid-key and failed-auth throttling behavior intact.

Acceptance criteria

  • APIKeyAuth(..., rate_window=0) and negative/non-finite/bool windows raise ValueError.
  • Zero, negative, bool, and invalid-type rate_limit values raise ValueError.
  • A valid positive configuration still returns 429 after the configured count within the window.
  • Existing auth behavior and constant-time comparison tests stay green.
  • If PR fix: throttle invalid api key attempts #121 lands first, its invalid-auth bucket uses the same validated policy and cannot be configured to prune every request.

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_api_auth.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/closed issues and PRs for rate_window, rate limit configuration, zero rate window, and APIKeyAuth rate limit validation. #120 / PR #121 only address rate-limiting invalid bearer-token attempts; #58 is broad redacted modem/CLI environment configuration. Neither covers a zero/negative APIKeyAuth.rate_window silently bypassing valid-key throttling.

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