Describe the bug
In apps/dashboard-api/src/controllers/dbExport.controller.js, the daily export limit is enforced with a non-atomic read-then-increment pattern against Redis. Two concurrent export requests can both read the same counter value, both pass the limit check, and both increment the counter, causing the effective daily limit to be exceeded by as many concurrent requests as arrive simultaneously.
To Reproduce
- Set a project's plan so
maxExports = 1 (free plan).
- Send two simultaneous POST requests to the export endpoint for the same project and day.
- Both requests read
currentCount = 0 from Redis before either has run redis.incr.
- Both pass the
>= maxExports check (0 is not >= 1).
- Both call
redis.incr(key), getting newCount = 1 and newCount = 2 respectively.
- Both enqueue an export job. The user gets two exports when the limit should have stopped at one.
The relevant code:
const currentCount = await redis.get(key); // read (non-atomic)
if (currentCount && Number(currentCount) >= maxExports) {
return next(new AppError(429, '...'));
}
const newCount = await redis.incr(key); // increment (separate operation)
if (newCount === 1) {
await redis.expire(key, 86400);
}
Expected behavior
The daily export limit should be enforced atomically so that concurrent requests cannot collectively exceed the cap. The standard fix is to increment first, then compare:
const newCount = await redis.incr(key);
if (newCount === 1) {
await redis.expire(key, 86400);
}
if (newCount > maxExports) {
await redis.decr(key); // rollback
return next(new AppError(429, `Daily export limit reached (${maxExports}/${maxExports}). Please try again tomorrow.`));
}
Alternatively, a Lua script or SET key value EX ttl NX combined with INCR can achieve the same result without a rollback.
Additional context
There is also a secondary issue in the same block: the redis.expire(key, 86400) call is made separately from redis.incr. If the process crashes or the Redis connection drops between the INCR and the EXPIRE, the counter key persists indefinitely with no TTL, causing the export quota for that project to never reset.
File: apps/dashboard-api/src/controllers/dbExport.controller.js
Labels: bug, nsoc26, level2
Describe the bug
In
apps/dashboard-api/src/controllers/dbExport.controller.js, the daily export limit is enforced with a non-atomic read-then-increment pattern against Redis. Two concurrent export requests can both read the same counter value, both pass the limit check, and both increment the counter, causing the effective daily limit to be exceeded by as many concurrent requests as arrive simultaneously.To Reproduce
maxExports = 1(free plan).currentCount = 0from Redis before either has runredis.incr.>= maxExportscheck (0 is not >= 1).redis.incr(key), gettingnewCount = 1andnewCount = 2respectively.The relevant code:
Expected behavior
The daily export limit should be enforced atomically so that concurrent requests cannot collectively exceed the cap. The standard fix is to increment first, then compare:
Alternatively, a Lua script or
SET key value EX ttl NXcombined withINCRcan achieve the same result without a rollback.Additional context
There is also a secondary issue in the same block: the
redis.expire(key, 86400)call is made separately fromredis.incr. If the process crashes or the Redis connection drops between the INCR and the EXPIRE, the counter key persists indefinitely with no TTL, causing the export quota for that project to never reset.File:
apps/dashboard-api/src/controllers/dbExport.controller.jsLabels:
bug,nsoc26,level2