IdempotencyInterceptor's (key, scope, callerId) identity never binds to the request's target resource or body — reusing a key across two different mutations silently replays the wrong cached result

[chonilius]

Overview

IdempotencyInterceptor determines whether a request is a replay purely from (Idempotency-Key header value, route scope, caller) — it never incorporates the request's target resource (path params) or body into that identity at all:

// src/common/idempotency/idempotency.interceptor.ts:93-119
async intercept(context: ExecutionContext, next: CallHandler): Promise<Observable<unknown>> {
  const scope = this.reflector.get<string | undefined>(IDEMPOTENCY_SCOPE_KEY, context.getHandler());
  if (!scope) return next.handle();

  const request = context.switchToHttp().getRequest<Request>();
  const rawKey = request.headers['idempotency-key'];
  const key = Array.isArray(rawKey) ? rawKey[0] : rawKey;
  // ... validates key is a UUID ...

  const identity: KeyIdentity = { key, scope, callerId: this.resolveCallerId(request) };
  const cached = await this.claim(identity);
  if (cached) { /* replay the cached response */ }
  // ... otherwise execute the handler and cache its outcome under `identity` ...
}

scope is a static string set once per route by the @Idempotent(scope) decorator (e.g. 'escrow.release' on POST /escrow/:id/release — escrow.controller.ts:26-27) — it's the same value for every request to that route, regardless of which :id is in the URL or what's in the body. IdempotencyKey's unique index is exactly (key, scope, callerId) (idempotency-key.entity.ts:36) — nothing about the specific escrow being released, the specific recipient, or the specific amount is part of that identity.

Concretely: if a client sends Idempotency-Key: K to POST /escrow/escrow-A/release, that request completes and its response is cached under (K, 'escrow.release', callerId). If that same client later sends Idempotency-Key: K again — this time to POST /escrow/escrow-B/release, a completely different escrow — claim() finds the existing row for (K, 'escrow.release', callerId), sees it COMPLETED, and replays escrow A's cached response verbatim:

// src/common/idempotency/idempotency.interceptor.ts:213-222
private async resolveExisting(existing: IdempotencyKey, identity: KeyIdentity): Promise<CachedOutcome | null> {
  if (existing.status === IdempotencyKeyStatus.COMPLETED) {
    return { responseStatus: existing.responseStatus ?? HttpStatus.OK, responseBody: existing.responseBody };
  }
  // ...
}

The client receives what looks like a successful release response — but escrow B was never actually released. next.handle() (the real handler, the one that would call EscrowService.release for escrow B) is never invoked at all when a cached outcome is found (:121-131, the if (cached) { ... return of(cached.responseBody); } branch returns before next.handle() is reached). The client believes escrow B's funds moved; they didn't. This isn't a contrived scenario — reusing an Idempotency-Key value across unrelated requests is a completely plausible client-side bug (a copy-pasted request, a client library that generates one key per "session" rather than per logical operation, a retry helper that doesn't realize the retried request targets a different resource because a caller upstream changed the URL without generating a new key).

The interceptor's own doc comment explains why keys are checked via insert-and-catch-unique-violation rather than check-then-insert (a real, correctly-reasoned TOCTOU fix for a different problem) — but that carefully-built concurrency safety is protecting the wrong invariant: it guarantees "two concurrent requests with the same key/scope/caller never both execute," which is exactly right for retries of the same operation, and silently wrong for two different operations that happen to share a key, because nothing about the identity used for that guarantee actually captures what "the same operation" means.

Requirements

• Extend the idempotency identity to bind to the actual request, not just its route scope — the standard approach (and what most production idempotency-key implementations, e.g. Stripe's, do) is to hash the request body (and relevant path params) and store that alongside the key, then on a replay, compare the incoming request's hash against the stored one: if they match, replay the cached response as today; if they don't, reject with a clear 422/409 ("Idempotency-Key already used with a different request") rather than silently replaying the wrong result.
• Decide whether path params (:id) should be folded into scope itself (e.g. 'escrow.release:<id>') or into the body-hash comparison — either closes this gap, but they have different implications for how IdempotencyKey rows are shaped and queried; document the choice.
• Add this hash/fingerprint as a new column on IdempotencyKey (a migration, following the pattern of 1784304511000-CreateIdempotencyKeys.ts), computed and checked in both claim()'s insert path and its replay-detection path.
• Add a test that reuses one Idempotency-Key across two requests targeting different resources on the same scope and asserts the second is rejected (or independently executed), not silently served the first's cached response.

Acceptance Criteria

• Reusing an Idempotency-Key across two requests with different path params and/or body content on the same scope no longer replays the first request's cached response for the second.
• The mismatch case returns a clear, distinct error rather than either silently executing twice or silently replaying the wrong result.
• A migration adds whatever fingerprint/hash field is needed, and existing claim()/resolveExisting() logic is updated to check it.
• A test proves the exact cross-resource replay scenario described above is no longer possible.

Additional Notes

Precise references: src/common/idempotency/idempotency.interceptor.ts:93-119 (intercept, identity construction with no body/param fingerprint), :183-211 (claim), :213-255 (resolveExisting, where the wrong cached response gets returned), src/common/entities/idempotency-key.entity.ts:35-56 ((key, scope, callerId) unique index — confirms no request-fingerprint column exists to check against), src/escrow/escrow.controller.ts:26-42 (concrete route where :id varies per request but scope doesn't).

Test/reproduction plan:

const key = randomUUID();
await request(app).post(`/escrow/${escrowA.id}/release`)
  .set('Idempotency-Key', key)
  .send({ recipientAddress: addressA })
  .expect(201);

const res = await request(app).post(`/escrow/${escrowB.id}/release`)
  .set('Idempotency-Key', key)   // same key, different escrow
  .send({ recipientAddress: addressB })
  .expect(201); // pre-fix: 201, but body describes escrow A's release, not B's

const escrowBAfter = await escrowService.findOne(escrowB.id);
expect(escrowBAfter.status).not.toBe(EscrowStatus.RELEASED); // pre-fix: still LOCKED — B was never actually released despite the apparently-successful response

Cross-references: distinct from, but compounds with, the companion issue on IdempotencyInterceptor's shared 'anonymous' caller bucket (that issue is about who the identity is scoped to; this issue is about what request the identity represents) — together they mean an unauthenticated deployment (today's default, per the "no auth at all" issue) has essentially no protection against one caller's key colliding with another's, or a single caller's own key colliding across their own unrelated requests. Also references the closed "Add end-to-end idempotency keys" issue that originally built this system (#16 in this repo, cited throughout idempotency.interceptor.ts's own comments) — this issue is a correctness gap in that system's design, not a request to add idempotency where none existed.

[presidojay1]

The idempotency identity is too coarse for resource-specific operations.

IdempotencyInterceptor identifies a request solely by (Idempotency-Key, scope, caller). The scope is a static value assigned to the route, so every request to POST /escrow/:id/release shares the same scope regardless of the :id, recipient, amount, or other request-body parameters.

This creates a correctness problem when the same caller reuses an idempotency key across different resources or materially different requests. For example, a key first used to release escrow A can cause a subsequent request using the same key to release escrow B to be treated as a replay and receive the cached response from escrow A. Conversely, a retry with the same key but a different payload is indistinguishable from a legitimate retry.

Idempotency keys are normally scoped to the operation and its target/request semantics, not merely the route. I have experience reviewing idempotency implementations for APIs that perform financial operations, where replaying the result of the wrong resource is particularly dangerous.

The identity should incorporate the relevant resource identifier and, where applicable, a canonical representation or hash of the request parameters/body. At minimum, the implementation should detect reuse of an idempotency key with a different request fingerprint and reject it rather than silently replaying an unrelated cached result.

[grantfox-oss]

🦊 GrantFox — @presidojay1 has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #54)
2. Your PR will be reviewed by the MergeFi maintainers

Good luck! Track your progress on GrantFox.