Skip to content

Idempotency replay cache is a per-middleware in-memory Map holding responses for 24 hours — duplicate settlements across replicas, unbounded memory #221

Description

@Jagadeeshftw

Priority: High  ·  Area: Idempotency / correctness  ·  Est. effort: 8–12 h

📌 Problem

src/middleware/idempotency.ts documents its own scope at lines 7–10:

"same key within the TTL replays the cached response instead of re-running… State lives in a plain Map local to the returned middleware"

and implements it as:

const DEFAULT_TTL_MS = 24 * 60 * 60 * 1000;   // idempotency.ts:29
const cache = new Map<string, CachedResponse>();  // idempotency.ts:78

Three problems follow.

  1. Cross-replica duplicates. The cache is per-process. A client that times out and retries — the exact scenario idempotency keys exist for — can be routed to a second replica that has never seen the key, and the operation executes twice. On settlements, that is a duplicated financial action.
  2. Per-middleware scoping. The Map is local to each returned middleware instance, so two mounts do not share state. The guarantee depends on middleware topology rather than on the key.
  3. Unbounded memory. Entries hold full cached responses for a 24-hour TTL. Nothing shown at the declaration site evicts them proactively, so a client sending many distinct keys grows the map with response bodies for a day at a time.

🎯 Design decision required

State and defend:

  1. Where the state belongs. A shared store is the fix, but this service currently has no persistence layer at all (tracked as a separate issue — its dependencies are compression, cors, express). So: do you build on whatever store that issue introduces, or introduce something narrower here? Coordinate and argue your sequencing.
  2. What a replay returns. Replaying the stored response requires storing bodies — which is what causes the memory problem. Is a stored response the right model, or should a duplicate return a conflict and force a re-query? Argue from the client's perspective.
  3. Eviction and bounds. Regardless of store, state the eviction strategy and a hard bound. A 24-hour TTL with no size cap is not a bound.

🧩 Requirements and context

  • Requests without an idempotency key must be entirely unaffected — prove it with a test.
  • The middleware's public options shape should stay stable; list any change.
  • Tests must run with no external dependency, as they do today.
  • Cached responses must not retain sensitive headers. Check what is stored and report it.
  • Concurrent requests with the same key must not both execute — a TTL cache alone does not prevent this if both arrive before either completes. Address the race explicitly.

🛠️ Suggested execution

  1. Write a failing test: two middleware instances, same key, both execute.
  2. Write a second: N distinct keys, assert the cache is bounded.
  3. Write a third: two concurrent same-key requests, assert only one executes.
  4. Implement per your decision.
  5. Show all three passing.

✅ Acceptance criteria

  • A test proves the cross-instance duplicate before the change and its absence after.
  • A test proves the cache is bounded under many distinct keys.
  • A test covers two concurrent same-key requests, with only one execution.
  • Keyless requests behave identically to today, proven by a test.
  • Stored responses are audited for sensitive headers, with findings reported.
  • The suite runs with no external dependency.
  • npm run lint, npm run build and npm test pass.

🚫 Out of scope

  • The rate limiter's in-memory state — separate issue.
  • Introducing the general persistence layer — separate issue, though you may build on it.
  • Changing which routes opt into idempotency.

🧪 Verification

npm ci
npm test src/middleware/idempotency.test.ts
npm run lint && npm run build && npm test

📤 What your PR must include

  1. The three tests, failing before and passing after.
  2. Your store, replay-semantics and eviction decisions with reasoning.
  3. Your sequencing relative to the persistence issue.
  4. What sensitive data, if any, was being cached.
  5. Closes #<n>.

🔒 Security notes

Idempotency on a settlement API is a financial-correctness control: it is what makes a client-side retry safe. Scoping it to one process means the protection silently disappears the moment the service runs more than one replica — and the failure is a duplicated settlement, not an error. Separately, caching full responses for 24 hours is both a memory-exhaustion vector and a place where sensitive response data sits in process memory far longer than the request that produced it.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuebugSomething isn't workingpriority:highHigh difficulty / architectural or cross-cutting

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions