fix: bind idempotency key identity to the request, not just its route scope

README.md

@@ -201,6 +201,15 @@ connection can't double-execute:
   Conflict` instead of racing into a duplicate execution. If the original
   request crashed before completing, the key is reclaimed after 30s so it
   doesn't 409 forever.
+- A key's identity is bound to the actual request, not just the route: a
+  SHA-256 fingerprint of the path params and body is stored alongside the
+  key, and a retry whose fingerprint doesn't match gets `422 Unprocessable
+  Entity` instead of replaying the original request's response. Without
+  this, reusing a key across two different resources on the same route
+  (e.g. `Idempotency-Key: K` sent to both `/escrow/A/release` and
+  `/escrow/B/release`) would silently replay A's cached response for B,
+  which looks like a successful release to the client while B's funds
+  never actually moved (#54).
 - Cached keys are retained for 24h (`IDEMPOTENCY_KEY_TTL_MS` in
   `IdempotencyInterceptor`) and swept hourly by `IdempotencyCleanupService`.
 

src/common/entities/idempotency-key.entity.ts

@@ -62,6 +62,17 @@ export class IdempotencyKey {
   })
   status: IdempotencyKeyStatus;
 
+  /**
+   * SHA-256 hex digest of the request's path params + body, set at insert
+   * time and never changed afterward. Lets a replay be distinguished from
+   * a different operation that happens to reuse the same
+   * (key, scope, callerId) — see IdempotencyInterceptor (#54). Null on
+   * rows created before this column existed; those are exempt from the
+   * mismatch check rather than treated as a guaranteed mismatch.
+   */
+  @Column({ type: 'varchar', length: 64, nullable: true })
+  requestFingerprint: string | null;
+
   /** HTTP status of the cached outcome, set once status moves to COMPLETED. */
   @Column({ type: 'int', nullable: true })
   responseStatus: number | null;

src/common/idempotency/idempotency.interceptor.spec.ts

@@ -4,6 +4,7 @@ import {
   ConflictException,
   ExecutionContext,
   InternalServerErrorException,
+  UnprocessableEntityException,
 } from '@nestjs/common';
 import { firstValueFrom, lastValueFrom, of, throwError } from 'rxjs';
 import { IdempotencyKey } from '../entities/idempotency-key.entity';
@@ -70,6 +71,10 @@ class FakeIdempotencyRepo {
       key: data.key,
       scope: data.scope,
       callerId: data.callerId,
+      // Mirrors Postgres: an unspecified nullable column is NULL, not
+      // undefined — matters for rows seeded directly via repo.insert() in
+      // tests that simulate a pre-#54 row with no fingerprint on record.
+      requestFingerprint: data.requestFingerprint ?? null,
       status: IdempotencyKeyStatus.PROCESSING,
       responseStatus: null,
       responseBody: null,
@@ -109,12 +114,16 @@ function createContext(
     headers?: Record<string, string>;
     method?: string;
     user?: { userId: string };
+    params?: Record<string, string>;
+    body?: unknown;
   } = {},
 ): ExecutionContext {
   const request = {
     headers: overrides.headers ?? {},
     method: overrides.method ?? 'POST',
     user: overrides.user,
+    params: overrides.params ?? {},
+    body: overrides.body ?? {},
   };
   return {
     switchToHttp: () => ({
@@ -356,4 +365,102 @@ describe('IdempotencyInterceptor', () => {
       'user-2',
     ]);
   });
+
+  describe('request fingerprint (#54)', () => {
+    it('replays correctly when the retried request has identical params and body', async () => {
+      const next = createNext(() => of({ id: 'escrow-A', status: 'released' }));
+      const context = createContext({
+        headers: { 'idempotency-key': KEY_A },
+        params: { id: 'escrow-A' },
+        body: { recipientAddress: 'GADDRESSA' },
+      });
+
+      await lastValueFrom(await interceptor.intercept(context, next));
+      const secondResult = await lastValueFrom(
+        await interceptor.intercept(context, next),
+      );
+
+      expect(secondResult).toEqual({ id: 'escrow-A', status: 'released' });
+      expect(next.handle).toHaveBeenCalledTimes(1);
+    });
+
+    it('rejects reusing a key across two different resources on the same scope instead of replaying the first (#54)', async () => {
+      const contextA = createContext({
+        headers: { 'idempotency-key': KEY_A },
+        params: { id: 'escrow-A' },
+        body: { recipientAddress: 'GADDRESSA' },
+      });
+      const contextB = createContext({
+        headers: { 'idempotency-key': KEY_A }, // same key
+        params: { id: 'escrow-B' }, // different resource
+        body: { recipientAddress: 'GADDRESSB' },
+      });
+
+      const nextA = createNext(() => of({ id: 'escrow-A', released: true }));
+      await lastValueFrom(await interceptor.intercept(contextA, nextA));
+
+      const nextB = createNext(() => of({ id: 'escrow-B', released: true }));
+      await expect(
+        interceptor.intercept(contextB, nextB),
+      ).rejects.toBeInstanceOf(UnprocessableEntityException);
+
+      // The second (different) resource's handler must never run — this is
+      // exactly the "escrow B was never actually released" failure mode.
+      expect(nextB.handle).not.toHaveBeenCalled();
+      expect(repo.rows).toHaveLength(1);
+      expect(repo.rows[0].responseBody).toEqual({
+        id: 'escrow-A',
+        released: true,
+      });
+    });
+
+    it('rejects reusing a key with the same resource but a different body', async () => {
+      const contextFirst = createContext({
+        headers: { 'idempotency-key': KEY_A },
+        params: { id: 'escrow-A' },
+        body: { recipientAddress: 'GADDRESSA' },
+      });
+      const contextDifferentBody = createContext({
+        headers: { 'idempotency-key': KEY_A },
+        params: { id: 'escrow-A' },
+        body: { recipientAddress: 'GADDRESSC' }, // different recipient
+      });
+
+      const next = createNext(() => of({ ok: true }));
+      await lastValueFrom(await interceptor.intercept(contextFirst, next));
+
+      await expect(
+        interceptor.intercept(contextDifferentBody, next),
+      ).rejects.toBeInstanceOf(UnprocessableEntityException);
+      expect(next.handle).toHaveBeenCalledTimes(1);
+    });
+
+    it('does not reject a pre-#54 row with no stored fingerprint, even if the incoming fingerprint differs', async () => {
+      // Simulates a row created before this migration/column existed.
+      await repo.insert({
+        key: KEY_A,
+        scope: 'test.scope',
+        callerId: 'anonymous',
+        expiresAt: new Date(Date.now() + 60_000),
+      });
+      repo.rows[0].status = IdempotencyKeyStatus.COMPLETED;
+      repo.rows[0].responseStatus = 200;
+      repo.rows[0].responseBody = { legacy: true };
+      expect(repo.rows[0].requestFingerprint).toBeNull();
+
+      const context = createContext({
+        headers: { 'idempotency-key': KEY_A },
+        params: { id: 'escrow-A' },
+        body: { recipientAddress: 'GADDRESSA' },
+      });
+      const next = createNext(() => of({ ok: true }));
+
+      const result = await lastValueFrom(
+        await interceptor.intercept(context, next),
+      );
+
+      expect(result).toEqual({ legacy: true });
+      expect(next.handle).not.toHaveBeenCalled();
+    });
+  });
 });

src/common/idempotency/idempotency.interceptor.ts

@@ -7,10 +7,12 @@ import {
   HttpStatus,
   Injectable,
   NestInterceptor,
+  UnprocessableEntityException,
 } from '@nestjs/common';
 import { HTTP_CODE_METADATA } from '@nestjs/common/constants';
 import { Reflector } from '@nestjs/core';
 import { InjectRepository } from '@nestjs/typeorm';
+import { createHash } from 'crypto';
 import type { Request } from 'express';
 import { from, Observable, of, throwError } from 'rxjs';
 import { catchError, map, mergeMap } from 'rxjs/operators';
@@ -47,6 +49,41 @@ function isUniqueViolation(error: unknown): boolean {
   return driverError?.code === PG_UNIQUE_VIOLATION;
 }
 
+/**
+ * Deterministic JSON serialization with object keys sorted recursively, so
+ * two logically-identical bodies that merely arrived with different key
+ * order (a real possibility across HTTP clients/serializers, including a
+ * client's own legitimate retry) hash identically rather than being
+ * mistaken for a mismatch.
+ */
+function stableStringify(value: unknown): string {
+  if (Array.isArray(value)) {
+    return `[${value.map(stableStringify).join(',')}]`;
+  }
+  if (value !== null && typeof value === 'object') {
+    const record = value as Record<string, unknown>;
+    const keys = Object.keys(record).sort();
+    return `{${keys
+      .map((k) => `${JSON.stringify(k)}:${stableStringify(record[k])}`)
+      .join(',')}}`;
+  }
+  return JSON.stringify(value);
+}
+
+/**
+ * Fingerprints "what request is this" — path params plus body — so it can
+ * be bound to an idempotency key's identity alongside (key, scope,
+ * callerId) (#54). Query params are deliberately excluded: none of the
+ * routes this guards read identity-relevant data from the query string.
+ */
+function computeRequestFingerprint(request: Request): string {
+  const payload = stableStringify({
+    params: request.params ?? {},
+    body: (request.body as Record<string, unknown> | undefined) ?? {},
+  });
+  return createHash('sha256').update(payload).digest('hex');
+}
+
 interface KeyIdentity {
   key: string;
   scope: string;
@@ -81,6 +118,19 @@ interface CachedOutcome {
  * is no authenticated caller to scope by yet. `resolveCallerId` falls back
  * to a shared 'anonymous' bucket per scope in that case — see its doc
  * comment for what that does and doesn't protect against.
+ *
+ * Request identity: `scope` is a static string per route (e.g.
+ * 'escrow.release'), the same for every request to that route regardless
+ * of path params or body — so (key, scope, callerId) alone can't tell two
+ * different resources (or two different payloads) apart. `resolveExisting`
+ * additionally compares a SHA-256 fingerprint of path params + body
+ * (`computeRequestFingerprint`) against the fingerprint stored on the
+ * existing row, and rejects with 422 on a mismatch rather than replaying
+ * the wrong cached response (#54). This binds identity via the body-hash
+ * comparison rather than folding path params into `scope` itself — scope
+ * stays a purely route-level concept (matches its existing "unique per
+ * scope, not globally" contract), and one mechanism covers both path
+ * params and body instead of two.
  */
 @Injectable()
 export class IdempotencyInterceptor implements NestInterceptor {
@@ -117,8 +167,9 @@ export class IdempotencyInterceptor implements NestInterceptor {
       scope,
       callerId: this.resolveCallerId(request),
     };
+    const fingerprint = computeRequestFingerprint(request);
 
-    const cached = await this.claim(identity);
+    const cached = await this.claim(identity, fingerprint);
     if (cached) {
       const badRequestStatus: number = HttpStatus.BAD_REQUEST;
       if (cached.responseStatus >= badRequestStatus) {
@@ -178,17 +229,23 @@ export class IdempotencyInterceptor implements NestInterceptor {
   /**
    * Returns a cached outcome to replay, or null if this call has become
    * the owner of the key and should proceed with the real handler.
-   * Throws ConflictException (409) if another request currently owns it.
+   * Throws ConflictException (409) if another request currently owns it,
+   * or UnprocessableEntityException (422) if it's owned by a request that
+   * targeted a different resource/body under the same key (#54).
    */
-  private async claim(identity: KeyIdentity): Promise<CachedOutcome | null> {
+  private async claim(
+    identity: KeyIdentity,
+    fingerprint: string,
+  ): Promise<CachedOutcome | null> {
     const existing = await this.repo.findOneBy(identity);
     if (existing) {
-      return this.resolveExisting(existing, identity);
+      return this.resolveExisting(existing, identity, fingerprint);
     }
 
     try {
       await this.repo.insert({
         ...identity,
+        requestFingerprint: fingerprint,
         expiresAt: new Date(Date.now() + IDEMPOTENCY_KEY_TTL_MS),
       });
       return null;
@@ -206,14 +263,30 @@ export class IdempotencyInterceptor implements NestInterceptor {
         // read) — safe to treat this as a fresh attempt.
         return null;
       }
-      return this.resolveExisting(winner, identity);
+      return this.resolveExisting(winner, identity, fingerprint);
     }
   }
 
   private async resolveExisting(
     existing: IdempotencyKey,
     identity: KeyIdentity,
+    fingerprint: string,
   ): Promise<CachedOutcome | null> {
+    // A null stored fingerprint means this row predates the column
+    // (migration deploy transition) — nothing to compare against, so it's
+    // exempt rather than treated as a guaranteed mismatch. Checked before
+    // status/staleness: a fingerprint mismatch means this was never "the
+    // same operation" in the first place, regardless of what state the
+    // true owner's row is in.
+    if (
+      existing.requestFingerprint !== null &&
+      existing.requestFingerprint !== fingerprint
+    ) {
+      throw new UnprocessableEntityException(
+        'This Idempotency-Key has already been used with a different request (different resource or body). Use a new Idempotency-Key for a different operation.',
+      );
+    }
+
     if (existing.status === IdempotencyKeyStatus.COMPLETED) {
       return {
         responseStatus: existing.responseStatus ?? HttpStatus.OK,
@@ -251,7 +324,7 @@ export class IdempotencyInterceptor implements NestInterceptor {
     if (!current) {
       return null;
     }
-    return this.resolveExisting(current, identity);
+    return this.resolveExisting(current, identity, fingerprint);
   }
 
   private async complete(

src/database/migrations/1784500000000-AddIdempotencyKeyRequestFingerprint.ts

@@ -0,0 +1,32 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+
+/**
+ * Adds IdempotencyKey.requestFingerprint (#54) — a hash of the request's
+ * path params and body, checked alongside (key, scope, callerId) so that
+ * reusing an Idempotency-Key across two requests targeting different
+ * resources on the same scope is rejected instead of silently replaying
+ * the wrong cached response. See IdempotencyInterceptor.
+ *
+ * Nullable: existing rows created before this migration have no
+ * fingerprint on record. IdempotencyInterceptor treats a null stored
+ * fingerprint as "predates this check" and skips the mismatch check for
+ * those rows rather than rejecting an in-flight legitimate retry during
+ * the deploy transition — see resolveExisting's doc comment.
+ */
+export class AddIdempotencyKeyRequestFingerprint1784500000000 implements MigrationInterface {
+  name = 'AddIdempotencyKeyRequestFingerprint1784500000000';
+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`
+      ALTER TABLE "idempotency_keys"
+      ADD COLUMN "requestFingerprint" character varying(64)
+    `);
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`
+      ALTER TABLE "idempotency_keys"
+      DROP COLUMN "requestFingerprint"
+    `);
+  }
+}