diff --git a/src/common/middleware/idempotency.middleware.spec.ts b/src/common/middleware/idempotency.middleware.spec.ts index fc300e4..95dd2dd 100644 --- a/src/common/middleware/idempotency.middleware.spec.ts +++ b/src/common/middleware/idempotency.middleware.spec.ts @@ -12,6 +12,7 @@ function fakeRedis() { store.set(key, value); return 'OK'; }), + del: jest.fn(async (key: string) => (store.delete(key) ? 1 : 0)), eval: jest.fn(async (_script: string, _n: number, key: string, token: string) => { if (store.get(key) === token) { store.delete(key); @@ -107,6 +108,50 @@ describe('IdempotencyMiddleware', () => { expect(res.body).toEqual({ id: 'x' }); }); + describe('#736 — corrupted cache entries', () => { + const storeKey = 'idempotency:POST:/api/v1/policies:abc'; + + it.each([ + ['invalid JSON', '{"status":201,"body":'], + ['JSON null', 'null'], + ['missing body', JSON.stringify({ status: 201 })], + ['non-numeric status', JSON.stringify({ status: 'ok', body: {} })], + ['out-of-range status', JSON.stringify({ status: 42, body: {} })], + ])('treats %s as a cache miss, deletes it and runs the handler', async (_label, corrupted) => { + const redis = fakeRedis(); + redis.store.set(storeKey, corrupted); + const mw = new IdempotencyMiddleware(redis as any); + const res = fakeRes(); + const next = jest.fn(() => res.status(201).json({ id: 'fresh' })); + + mw.use(req(), res, next); + await tick(20); + + expect(redis.del).toHaveBeenCalledWith(storeKey); + expect(next).toHaveBeenCalledTimes(1); + expect(res.statusCode).toBe(201); + expect(res.body).toEqual({ id: 'fresh' }); + expect(res.headers['X-Idempotent-Replayed']).toBeUndefined(); + // The fresh response replaces the corrupted entry. + expect(JSON.parse(redis.store.get(storeKey)!)).toEqual({ status: 201, body: { id: 'fresh' } }); + }); + + it('still processes the request when deleting the corrupted entry fails', async () => { + const redis = fakeRedis(); + redis.store.set(storeKey, 'not-json'); + redis.del.mockRejectedValue(new Error('ECONNRESET')); + const mw = new IdempotencyMiddleware(redis as any); + const res = fakeRes(); + const next = jest.fn(() => res.status(201).json({ id: 'fresh' })); + + mw.use(req(), res, next); + await tick(20); + + expect(next).toHaveBeenCalledTimes(1); + expect(res.statusCode).toBe(201); + }); + }); + it('fails open when Redis is unavailable', async () => { const redis = fakeRedis(); redis.get.mockRejectedValue(new Error('ECONNREFUSED')); diff --git a/src/common/middleware/idempotency.middleware.ts b/src/common/middleware/idempotency.middleware.ts index 25eaae3..2a53da8 100644 --- a/src/common/middleware/idempotency.middleware.ts +++ b/src/common/middleware/idempotency.middleware.ts @@ -157,17 +157,38 @@ export class IdempotencyMiddleware implements NestMiddleware { private async replayCached(storeKey: string, rawKey: string, res: Response): Promise { const cached = await this.redis.get(storeKey); if (!cached) return false; + + const entry = this.parseCachedEntry(cached); + if (!entry) { + // #736 — corrupted cache entry: treat as a cache miss and delete it so + // the next request can run the handler and store a fresh response. + this.logger.warn(`Discarding corrupted cached idempotency entry for key=${rawKey}`); + await this.redis.del(storeKey).catch((err: unknown) => + this.logger.warn( + `Failed to delete corrupted idempotency entry for key=${rawKey}: ${err instanceof Error ? err.message : String(err)}`, + ), + ); + return false; + } + + this.logger.debug(`Replaying idempotent response for key=${rawKey}`); + res.setHeader('X-Idempotent-Replayed', 'true'); + res.status(entry.status).json(entry.body); + return true; + } + + /** Parse and shape-check a cached entry. Returns null for invalid JSON or an unexpected shape. */ + private parseCachedEntry(cached: string): { status: number; body: unknown } | null { + let parsed: unknown; try { - const { status, body } = JSON.parse(cached) as { status: number; body: unknown }; - this.logger.debug(`Replaying idempotent response for key=${rawKey}`); - res.setHeader('X-Idempotent-Replayed', 'true'); - res.status(status).json(body); - return true; + parsed = JSON.parse(cached); } catch { - // Corrupted cache entry — fall through to normal processing. - this.logger.warn(`Failed to parse cached idempotency entry for key=${rawKey}`); - return false; + return null; } + if (typeof parsed !== 'object' || parsed === null || !('body' in parsed)) return null; + const { status, body } = parsed as { status: unknown; body: unknown }; + if (typeof status !== 'number' || !Number.isInteger(status) || status < 100 || status > 599) return null; + return { status, body }; } private runWithLock(