Summary
APIKeyAuth.middleware() appears to intend constant-time API-key comparison, but it calls secrets.compare_digest(key, key), which always compares the supplied key to itself. The real authorization decision is then key not in self._keys, a normal set-membership lookup. Functionality still accepts/rejects keys, but the constant-time hardening is ineffective and future maintainers may think this path is side-channel resistant when it is not.
Affected files / lines
server.py:68-70 — if not secrets.compare_digest(key, key) or key not in self._keys: compares the candidate with itself instead of comparing against stored API keys.
tests/test_api_auth.py:52-79 covers valid/invalid behavior, but not that stored-key comparison uses compare_digest correctly.
Evidence
Baseline health from this scout run:
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
379 passed in 4.27s
Minimal no-hardware probe showing behavior still works but the middleware source contains the ineffective self-compare:
PYTHONPATH=. uv run --no-project --with aiohttp --with pytest-aiohttp --with pyserial-asyncio python - <<'PY'
import asyncio, inspect
from aiohttp import web
from aiohttp.test_utils import TestClient, TestServer
from server import APIKeyAuth
async def main():
auth = APIKeyAuth(api_keys=['abcd1234'], rate_limit=3, rate_window=60)
app = web.Application(middlewares=[auth.middleware])
async def ok(request):
return web.json_response({'ok': True})
app.router.add_get('/', ok)
client = TestClient(TestServer(app)); await client.start_server()
try:
for key in ['wrong000', 'abcd1234']:
resp = await client.get('/', headers={'Authorization': f'Bearer {key}'})
print(key, resp.status, await resp.text())
finally:
await client.close()
source = inspect.getsource(APIKeyAuth.middleware)
print('contains_compare_digest_key_key:', 'compare_digest(key, key)' in source)
print('membership_check_present:', 'key not in self._keys' in source)
asyncio.run(main())
PY
Actual output:
wrong000 403 {"error": "Invalid API key."}
abcd1234 200 {"ok": true}
contains_compare_digest_key_key: True
membership_check_present: True
Expected behavior
If the middleware uses secrets.compare_digest, the authorization decision should compare the provided key against configured keys in a constant-time style, for example any(secrets.compare_digest(key, candidate) for candidate in self._keys), with care around length/timing characteristics and without logging keys.
Actual behavior
The compare_digest call is a no-op for security because any string compares equal to itself; membership in the set performs the actual decision.
Duplicate check
No existing issue/PR matched these searches:
gh issue list --repo Justinabox/Callstack --state all --search 'APIKeyAuth compare_digest timing membership in:title,body'
gh issue list --repo Justinabox/Callstack --state all --search 'constant-time API key auth compare digest in:title,body'
gh pr list --repo Justinabox/Callstack --state all --search 'APIKeyAuth compare_digest constant time'
Suggested fix direction
- Replace the self-compare plus set membership with a helper that checks candidate keys using
secrets.compare_digest against configured keys.
- Keep the public error message generic (
Invalid API key.) and do not log candidate/stored keys.
- Add a source/behavior regression test that would fail if
compare_digest(key, key) reappears, plus existing valid/invalid auth tests.
Acceptance criteria
- Invalid keys still return
403; valid keys still pass.
- The auth decision no longer contains
compare_digest(key, key) or equivalent self-comparison.
- Tests verify the middleware uses the stored-key comparison helper and preserves rate-limit behavior.
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
Summary
APIKeyAuth.middleware()appears to intend constant-time API-key comparison, but it callssecrets.compare_digest(key, key), which always compares the supplied key to itself. The real authorization decision is thenkey not in self._keys, a normal set-membership lookup. Functionality still accepts/rejects keys, but the constant-time hardening is ineffective and future maintainers may think this path is side-channel resistant when it is not.Affected files / lines
server.py:68-70—if not secrets.compare_digest(key, key) or key not in self._keys:compares the candidate with itself instead of comparing against stored API keys.tests/test_api_auth.py:52-79covers valid/invalid behavior, but not that stored-key comparison usescompare_digestcorrectly.Evidence
Baseline health from this scout run:
Minimal no-hardware probe showing behavior still works but the middleware source contains the ineffective self-compare:
Actual output:
Expected behavior
If the middleware uses
secrets.compare_digest, the authorization decision should compare the provided key against configured keys in a constant-time style, for exampleany(secrets.compare_digest(key, candidate) for candidate in self._keys), with care around length/timing characteristics and without logging keys.Actual behavior
The
compare_digestcall is a no-op for security because any string compares equal to itself; membership in the set performs the actual decision.Duplicate check
No existing issue/PR matched these searches:
Suggested fix direction
secrets.compare_digestagainst configured keys.Invalid API key.) and do not log candidate/stored keys.compare_digest(key, key)reappears, plus existing valid/invalid auth tests.Acceptance criteria
403; valid keys still pass.compare_digest(key, key)or equivalent self-comparison.Verification gates