fix(auth): compare API keys in constant time

agentkit/api/auth.py

@@ -1,5 +1,6 @@
 """API key authentication dependency validating X-API-Key header."""
 
+import hmac
 import logging
 
 from fastapi import Depends, Request, Security
@@ -23,8 +24,28 @@ async def verify_api_key(
     api_key: str | None = Security(api_key_header),
     settings: Settings = Depends(get_current_settings),
 ) -> str:
-    """Validate client API key from X-API-Key header against configured authorized keys."""
-    if not api_key or api_key not in settings.api_keys_list:
+    """Validate client API key from X-API-Key header against configured authorized keys.
+
+    Normalises the presented key (stripping whitespace) and uses constant-time comparison
+    (`hmac.compare_digest`) across all configured keys to eliminate timing oracles.
+    """
+    if not api_key:
         logger.warning("Unauthorized API request with missing or invalid key.")
         raise AuthenticationError("Invalid or missing API key.")
-    return api_key
+
+    normalized_key = api_key.strip()
+    if not normalized_key:
+        logger.warning("Unauthorized API request with missing or invalid key.")
+        raise AuthenticationError("Invalid or missing API key.")
+
+    presented_bytes = normalized_key.encode("utf-8")
+    matched = False
+    for valid_key in settings.api_keys_list:
+        if hmac.compare_digest(presented_bytes, valid_key.encode("utf-8")):
+            matched = True
+
+    if not matched:
+        logger.warning("Unauthorized API request with missing or invalid key.")
+        raise AuthenticationError("Invalid or missing API key.")
+
+    return normalized_key

docs/PROGRESS.md

@@ -61,6 +61,7 @@ This document tracks execution progress across all backlog tasks, including pull
 | AUD-09  | AUD       | Live End-to-End Infrastructure Verification & Documentation Proof | `docs/e2e-verification-report` | #53 | 3 | Done | Standardized model, verified live stack, and documented in E2E_REPORT.md |
 | AUD-10  | AUD       | Rate Limiter Outage Fallback & Provider-Valid Session History | `fix/ratelimit-and-session-repair` | #56 | 3 | Done | In-memory fallback on Redis connection failures; synthetic tool results repair aborted sessions |
 | M13-T06 | M13       | Whole-Project Audit Remediation | `fix/audit-hardening` | #47 | 3 | Done | Fail-closed config and LLM resolution, SSRF post-connect check, atomic fallback |
+| AUDIT-17 | AUD       | Constant-Time & Normalised API Key Verification | `fix/api-key-constant-time-compare` | Pending | 2 | Done | Normalise presented keys and verify against authorized keys via constant-time hmac.compare_digest |
 
 ---
 
@@ -407,3 +408,10 @@ This document tracks execution progress across all backlog tasks, including pull
 - **PR**: #47
 - **Commits**: 3
 - **Notes**: Remediated 17 findings from a full-codebase audit, rebased onto the AUD-01/AUD-04/AUD-05 follow-ups. Critical: removed the `ak_test_key_12345` default from `Settings.API_KEYS` and made the field required; replaced the `FakeLLMClient` fallback in `get_llm_client` with a `ServiceUnavailableError` (503) boundary around provider construction; kept the AUD-04 factory-level `RetryingLLMClient` wrapping and made it configurable through new `LLM_MAX_RETRIES`/`LLM_RETRY_BASE_DELAY_S`/`LLM_RETRY_MAX_DELAY_S` settings; made the per-request `Agent` assembly's override explicit as `app.state.agent_override`; forwarded `get_run_trace`/`list_runs` through `TieredMemoryStore`; moved `http_fetch` DNS off the event loop with `asyncio.to_thread`, added a post-connect `server_addr` check against the validated addresses to close the DNS-rebinding TOCTOU window, and replaced full-body buffering with a byte-budgeted stream; replaced the rate limiter's non-scripting fallback with `WATCH`/`MULTI`/`EXEC`, leaving the AUD-10 in-memory fallback for Redis outages intact. Moderate: corrected the `RedisMemoryStore.truncate_messages` off-by-one that returned `max_messages + 1` entries when a system prompt was present; switched generated `tool_call_id`s to full UUIDs; wrapped response parsing in both LLM adapters inside the error boundary so malformed responses and malformed tool-argument JSON raise `LLMProviderError`; guarded `RetryingLLMClient.__getattr__` against recursion on a partially constructed instance; sanitized `/health` details so the unauthenticated endpoint no longer echoes driver error strings; made `sql_readonly` mask string literals and quoted identifiers before its keyword scan, allow `#>`/`#>>`/`#-`, apply the row limit in SQL and push `statement_timeout` to PostgreSQL. Minor: stopped logging raw run goals at INFO (length only, content at DEBUG); replaced `split(";")` in `scripts/seed.py` with a statement-aware splitter handling literals, comments and dollar-quotes; bounded calculator exponentiation by result bit length. Added `tests/unit/test_audit_regressions.py` and a real-Redis concurrency regression test pinning the fallback fix. Recorded in ADR-009, ADR-010, ADR-011. Suite: 298 passed, 25 skipped; coverage 92.86% on core/tools/llm; `ruff check`, `ruff format --check` and `mypy agentkit tests` all clean.
+
+### AUDIT-17: Constant-Time & Normalised API Key Verification
+- **Status**: Done
+- **Branch**: `fix/api-key-constant-time-compare`
+- **Issue**: #74
+- **Commits**: 2
+- **Notes**: Remediated timing oracle in API key verification (`verify_api_key`) in `agentkit/api/auth.py`. Replaced the short-circuiting `api_key in settings.api_keys_list` check with constant-time matching (`hmac.compare_digest`) across all configured keys. Presented keys are normalised (stripped) identical to configured keys, preventing whitespace drift and ensuring uniform rate-limiting and logging hygiene. Added regression tests in `tests/unit/test_api_auth.py` verifying direct and endpoint integration behaviors.

tests/unit/test_api_auth.py

@@ -28,6 +28,25 @@ async def test_verify_api_key_direct() -> None:
     with pytest.raises(AuthenticationError):
         await verify_api_key(api_key="wrong_key", settings=settings)
 
+    # Whitespace-padded valid key normalises and succeeds identically to unpadded
+    assert await verify_api_key(api_key="  valid_key_1  ", settings=settings) == "valid_key_1"
+    assert await verify_api_key(api_key="\tvalid_key_2\n", settings=settings) == "valid_key_2"
+
+    # Whitespace-only key raises AuthenticationError
+    with pytest.raises(AuthenticationError):
+        await verify_api_key(api_key="   ", settings=settings)
+
+    # Non-ASCII / unicode invalid key raises AuthenticationError without crashing
+    with pytest.raises(AuthenticationError):
+        await verify_api_key(api_key="invalid_utf8_🔑", settings=settings)
+
+    # Unicode key matching succeeds when configured
+    unicode_settings = Settings(API_KEYS="unicode_key_🔑")
+    assert (
+        await verify_api_key(api_key="  unicode_key_🔑  ", settings=unicode_settings)
+        == "unicode_key_🔑"
+    )
+
 
 def test_api_key_auth_endpoint_integration() -> None:
     settings = Settings(API_KEYS="secret_token_123")
@@ -66,3 +85,8 @@ def public_route() -> dict[str, str]:
     resp_valid = client.get("/protected", headers={"X-API-Key": "secret_token_123"})
     assert resp_valid.status_code == 200
     assert resp_valid.json() == {"status": "authorized"}
+
+    # Protected route with whitespace-padded valid header -> 200 (identical behavior)
+    resp_padded = client.get("/protected", headers={"X-API-Key": "  secret_token_123  "})
+    assert resp_padded.status_code == 200
+    assert resp_padded.json() == {"status": "authorized"}