Skip to content

Reusing an Idempotency-Key across routes 500s on POST /api/v1/api-keys (decrypt runs before the fingerprint check) #931

Description

@strickvl

Found by a deeper Schemathesis run (400 examples per operation, randomized) on the API fuzzing spike (#923). This one did not appear at 25 or 50 examples. Reproduced by hand.

What happens

Reusing one Idempotency-Key value on a plaintext route and then on POST /api/v1/api-keys crashes with a 500.

POST /api/v1/agents    Idempotency-Key: shared-key-1   → 201
POST /api/v1/api-keys  Idempotency-Key: shared-key-1   → 500 Internal Server Error

Captured exception:

kitaru.server.adapters.db.encryption.DecryptionError:
  Decryption failed. The token was encrypted with a different key or is corrupted

Only one direction fails. The reverse order — api-keys first, then agents with the same key — returns 201 cleanly.

Root cause

Idempotency rows are scoped by (account_id, key) with no route component, but the read decides whether to decrypt based on the route being called now, not on how the stored row was actually written.

enforce_idempotency (src/kitaru/server/adapters/rest/idempotency.py:128) passes the calling route's setting straight through:

stored = await repository.get(context.account.id, key, encrypted=encrypt_response)

IdempotencyKeyRepository.get (src/kitaru/server/adapters/db/repositories/idempotency_key_repository.py:88-95) then decrypts unconditionally when that flag is set:

if encrypted and idempotency_key.response_body is not None:
    idempotency_key.response_body = self._cipher.decrypt_bytes(...)

Of the 23 @idempotent routes, exactly two set encrypt_response=TruePOST /api/v1/api-keys (routers/api_keys.py:47) and the rotate route (api_keys.py:147). The other 21 store their response body as plaintext. So a key registered by any of those 21 and replayed against either of these 2 hands ciphertext-expecting code a plaintext body, and Fernet raises.

The ordering is the real defect. _enforce_idempotency already has a check that would reject this correctly — stored.fingerprint != fingerprint raises IdempotencyKeyMismatch at idempotency.py:203, because the fingerprint covers method, path, query, and body, so a different route can never match. But the decryption happens inside repository.get, which runs at line 196, before the fingerprint is ever compared. The code crashes on the way to noticing that the key belongs to a different request.

Impact

Low to medium, and safe to fix publicly:

  • Requires a valid bearer token.
  • Rows are scoped by account_id, so a caller can only affect their own keys. No cross-account denial of service.
  • No process crash, no data corruption, no data exposure. Verified: GET /health after the failure returns 200.
  • No key material is leaked — the decryption fails and the response body is the bare Internal Server Error.

The realistic trigger is a client that reuses one idempotency key across a logical multi-step operation, or generates one key per session rather than per request. That client gets a 500 and cannot create or rotate API keys, with an error message that points at encryption and gives no hint that the actual problem is a reused key.

Worth noting the crash happens in a dependency, so it precedes body validation entirely — a request with a reused key 500s regardless of whether the body is valid. In the reproduction below, the same route with a fresh key and the same body returns a clean 422.

Reproduction

Script: tests/fuzz_spike/ harness plus the snippet below, against a real server on real PostgreSQL with a real bearer token.

1. POST /api/v1/agents (plaintext route)      -> 201
2. POST /api/v1/api-keys, same key            -> 500   <-- the bug
3. control: api-keys with a fresh key         -> 422   (clean validation error)
4. reverse order (encrypted then plaintext)   -> 201
5. health after the failures                  -> 200

Suggested fix

Reorder so validation precedes decryption. The fingerprint check already rejects a cross-route key reuse correctly; it just needs to run first. Two workable shapes:

  1. Load without decrypting, compare the fingerprint, then decrypt only on a genuine replay. Split get into a plain load and an explicit decrypt step, and have _enforce_idempotency call the decrypt only after stored.fingerprint == fingerprint passes. This is the minimal change and fixes the ordering properly.
  2. Record on the row whether its body is encrypted, and decrypt based on the stored flag rather than the calling route's setting. This is more robust — it stays correct even if a route's encrypt_response setting changes between the write and the read, which option 1 does not — but it needs a migration.

Either way, decrypt_bytes raising should not reach the client as a 500. A DecryptionError during idempotency replay means the stored row cannot be interpreted, which is a 409 or a fresh execution, not a server error.

Regression test

Register a key on any plaintext @idempotent route, replay it against POST /api/v1/api-keys, and assert the documented mismatch response rather than a 500. Worth also covering the rotate route at api_keys.py:147.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions