Argon2 is non-deterministic — breaks all API key lookups

Argon2 generates a cryptographically random salt on every call, so _hasher.hash("same-key") produces a different string each time. The authentication middleware (middleware.py:195–197) works by:

1. Computing key_hash = AuthService.hash_api_key(token) at request time.
2. Looking up persistence.api_keys.get_by_hash(key_hash) using an exact WHERE key_hash = ? SQL query.

Because the Argon2 output at authentication time will never equal the Argon2 string that was stored when the key was created, every API key authentication request will return None (i.e., 401 Unauthorized). This is a complete regression for API key auth.

The existing unit test test_hash_deterministic in tests/unit/api/auth/test_service.py:155–158 also directly asserts h1 == h2 for two calls with the same input — that test will fail immediately with this change.

The correct approach for a hash that must support direct DB lookups is a keyed MAC (HMAC-SHA256) using a server-side secret, which is deterministic but still safe against offline attacks on the stored hash:

import hashlib, hmac

@staticmethod
def hash_api_key(raw_key: str, secret: str) -> str:
    return hmac.new(secret.encode(), raw_key.encode(), hashlib.sha256).hexdigest()

Argon2 (or bcrypt) is the right tool for password storage where verification is done with verify(hash, plain). For API keys stored as lookup tokens in a database, HMAC is the standard solution (see e.g. GitHub's personal access token model).

This is a comment left during a code review.
Path: src/ai_company/api/auth/service.py
Line: 186

Comment:
**Argon2 is non-deterministic — breaks all API key lookups**

Argon2 generates a cryptographically random salt on every call, so `_hasher.hash("same-key")` produces a **different** string each time. The authentication middleware (`middleware.py:195–197`) works by:

1. Computing `key_hash = AuthService.hash_api_key(token)` at request time.
2. Looking up `persistence.api_keys.get_by_hash(key_hash)` using an exact `WHERE key_hash = ?` SQL query.

Because the Argon2 output at authentication time will never equal the Argon2 string that was stored when the key was created, **every API key authentication request will return `None` (i.e., 401 Unauthorized)**. This is a complete regression for API key auth.

The existing unit test `test_hash_deterministic` in `tests/unit/api/auth/test_service.py:155–158` also directly asserts `h1 == h2` for two calls with the same input — that test will fail immediately with this change.

The correct approach for a hash that must support direct DB lookups is a **keyed MAC (HMAC-SHA256)** using a server-side secret, which is deterministic but still safe against offline attacks on the stored hash:

```python
import hashlib, hmac

@staticmethod
def hash_api_key(raw_key: str, secret: str) -> str:
    return hmac.new(secret.encode(), raw_key.encode(), hashlib.sha256).hexdigest()
```

Argon2 (or bcrypt) is the right tool for *password* storage where verification is done with `verify(hash, plain)`. For API keys stored as lookup tokens in a database, HMAC is the standard solution (see e.g. GitHub's personal access token model).

How can I resolve this? If you propose a fix, please make it concise.

Switching to _hasher.hash(raw_key) makes hash_api_key() non-deterministic (new random salt each call). The API key auth flow currently computes key_hash = AuthService.hash_api_key(token) and then does a DB lookup via api_keys.get_by_hash(key_hash), which will never match a previously stored hash and will break all API-key authentication. To use Argon2 here, the lookup needs to change (e.g., store a key id/prefix for lookup and then verify with _hasher.verify(stored_hash, raw_key)), or keep a deterministic keyed/KDF-based fingerprint specifically for lookup.

hash_api_key() is used on the request path (called from async middleware). Argon2 hashing is CPU-expensive and this synchronous call will block the event loop / worker thread and can become a DoS vector under load. Consider providing an async/offloaded variant (similar to hash_password_async) or switching to a deterministic construction that’s safe for lookup and cheaper to compute per request (e.g., a keyed HMAC/KDF) while still addressing the scanning alert.

⚠️ Potential issue | 🔴 Critical

#!/bin/bash
# Description: Search for code that verifies API keys by comparing hashes directly

# Find usages of hash_api_key that might be doing direct comparison
rg -n -C5 'hash_api_key' --type py

# Find potential API key verification patterns
rg -n -C3 'api.*key.*hash|key_hash|verify.*api' --type py -i

# Get the actual implementation of hash_api_key and _hasher initialization
sed -n '1,30p' src/ai_company/api/auth/service.py
sed -n '176,190p' src/ai_company/api/auth/service.py

# Check if there's any configuration of _hasher that affects determinism
rg -n "PasswordHasher" src/ai_company/api/auth/service.py -A 5 -B 2

# Check the full test implementation to see if it's actually testing what we think
sed -n '154,165p' tests/unit/api/auth/test_service.py

# Search for verify_api_key method
rg -n "verify_api_key" --type py

# Get full middleware verification logic
sed -n '180,220p' src/ai_company/api/auth/middleware.py

# Check git history to see if this is a recent change
git log --oneline -20 src/ai_company/api/auth/service.py

# Look at the test more closely - maybe it's checking something else
cat -n tests/unit/api/auth/test_service.py | sed -n '150,170p'

# Check if there's any custom PasswordHasher or if it's the standard argon2 one
rg -n "class.*Hasher|def hash" src/ai_company/api/auth/service.py

# Check if tests have any skip markers or if they would fail
rg -n "pytest.mark|@skip|xfail" tests/unit/api/auth/test_service.py -B 2 | grep -A 2 "TestApiKeyHashing"

# The core question: does argon2.PasswordHasher.hash() produce deterministic output?
# Let's check the argon2 library behavior with a simple test
python3 << 'EOF'
import argon2

hasher = argon2.PasswordHasher(
    time_cost=3,
    memory_cost=65536,
    parallelism=4,
    hash_len=32,
    salt_len=16,
)

# Test if hash is deterministic
h1 = hasher.hash("test-key")
h2 = hasher.hash("test-key")

print(f"Hash 1: {h1}")
print(f"Hash 2: {h2}")
print(f"Are they equal? {h1 == h2}")
print(f"Can verify h1 with h2? ", end="")
try:
    hasher.verify(h1, "test-key")
    print("YES")
except:
    print("NO")
EOF

# Check what files were modified in the recent commit
git show --name-only c84e8ab

# Check if the test file was modified in that commit
git show c84e8ab -- tests/unit/api/auth/test_service.py | head -100

# Look at the actual diff for hash_api_key changes
git show c84e8ab -- src/ai_company/api/auth/service.py | head -150

Critical: Argon2 non-determinism breaks API key verification flow.

Argon2 uses a random salt per call, so each _hasher.hash(raw_key) produces a different output for the same input. This breaks the middleware's lookup pattern: key_hash = AuthService.hash_api_key(token) followed by persistence.api_keys.get_by_hash(key_hash) will fail on repeat requests with the same token.

The newly added test_hash_deterministic expects deterministic hashing (assert h1 == h2), but standard Argon2 PasswordHasher cannot provide this guarantee — the test will fail when run.

A verify_api_key(raw_key: str, key_hash: str) -> bool method is required to verify tokens against stored hashes using _hasher.verify(), mirroring the existing verify_password pattern. The middleware must also be updated to call this method instead of re-hashing.

Additionally, this is a breaking change for existing API keys stored as SHA-256 hex digests, which will fail Argon2 verification.

Verify each finding against the current code and only fix it if needed.

In `@src/ai_company/api/auth/service.py` around lines 176 - 186, The current
hash_api_key(raw_key) uses Argon2 and is non-deterministic; replace the flow by
adding a verify_api_key(raw_key: str, key_hash: str) -> bool that uses
_hasher.verify(key_hash, raw_key) (mirroring verify_password) and update any
middleware that does key_hash = AuthService.hash_api_key(token) then
persistence.api_keys.get_by_hash(key_hash) to instead fetch candidate API key
records (e.g., by token identifier or iterate stored keys) and call
AuthService.verify_api_key(token, stored_hash); also keep hash_api_key for
creating new keys but document that existing SHA-256 hex-digest keys will need
special handling (add a fallback verify path that checks sha256(raw_key) ==
stored_hash before trying Argon2.verify to preserve backward compatibility).

The change to use _hasher.hash is a security improvement, but it introduces a critical functional regression. argon2.PasswordHasher().hash() generates a unique random salt for every call, making the resulting hash non-deterministic. This will cause all API key authentication attempts to fail if the application relies on deterministic hashing for database lookups or direct comparison, leading to a Denial of Service (DoS) for API users. The existing unit test test_hash_deterministic explicitly confirms this functional regression.

To address this, the authentication logic must be updated to:

1. Look up the API key by a non-sensitive identifier (e.g., a key ID or prefix).
2. Verify the provided raw key against the stored Argon2 hash using _hasher.verify(stored_hash, raw_key).

The test suite for API key hashing should also be updated. test_hash_deterministic should be replaced with tests that:

1. Verify that hashing the same key multiple times produces different hashes.
2. Verify that a generated hash can be correctly verified against the original key.

For example:

# In tests/unit/api/auth/test_service.py

def test_different_hashes_for_same_key(self) -> None:
    h1 = AuthService.hash_api_key("same-key")
    h2 = AuthService.hash_api_key("same-key")
    assert h1 != h2

def test_hash_and_verify(self) -> None:
    svc = _make_service()
    raw_key = "my-api-key"
    hashed_key = AuthService.hash_api_key(raw_key)
    # Re-using verify_password as it uses the same hasher
    assert svc.verify_password(raw_key, hashed_key)
    assert not svc.verify_password("wrong-key", hashed_key)