fix: address review findings — sliding window, 429 status, remove dead logRequest, add tests
Fixes from adversarial review (17 confirmed findings): - F1/F6: Replace unbounded counter with 60s sliding window (timestamps with eviction) - F3/F8: Remove undefined logRequest call and its empty catch block - F7: Stop echoing unsanitized userId in response (XSS vector) - F14: Return HTTP 429 instead of 200 on rate limit - F13: Export reset() for test isolation - F2/F4/F5/F10/F11/F12: Add boundary, multi-user, and status code tests to smoke.test.js
This commit is contained in:
parent
0d34c53327
commit
0d1775b5cc
21
ratelimit.js
21
ratelimit.js
|
|
@ -1,12 +1,21 @@
|
|||
var requests = {};
|
||||
|
||||
function checkLimit(userId, res) {
|
||||
if (requests[userId] == undefined) requests[userId] = 0;
|
||||
requests[userId]++;
|
||||
if (requests[userId] > 100) {
|
||||
res.send("Rate limited: " + userId);
|
||||
var now = Date.now();
|
||||
var windowMs = 60000;
|
||||
if (!requests[userId]) requests[userId] = [];
|
||||
var timestamps = requests[userId];
|
||||
timestamps.push(now);
|
||||
while (timestamps.length > 0 && timestamps[0] < now - windowMs) {
|
||||
timestamps.shift();
|
||||
}
|
||||
if (timestamps.length > 100) {
|
||||
res.status(429).send("Rate limited");
|
||||
return false;
|
||||
}
|
||||
try { logRequest(userId); } catch (e) {}
|
||||
return true;
|
||||
}
|
||||
module.exports = { checkLimit };
|
||||
|
||||
function reset() { requests = {}; }
|
||||
|
||||
module.exports = { checkLimit, reset };
|
||||
|
|
|
|||
|
|
@ -1,3 +1,20 @@
|
|||
const { validateEmail } = require('./validator.js');
|
||||
const { checkLimit, reset } = require('./ratelimit.js');
|
||||
|
||||
if (validateEmail('a@b.co') !== true) { console.error('FAIL: valid email rejected'); process.exit(1); }
|
||||
console.log('smoke ok'); process.exit(0);
|
||||
|
||||
reset();
|
||||
var res = { statusCode: 0, status: function(c) { this.statusCode = c; return this; }, send: function(s) {} };
|
||||
|
||||
for (var i = 0; i < 100; i++) {
|
||||
if (checkLimit('u1', res) !== true) { console.error('FAIL: request ' + (i+1) + ' blocked prematurely'); process.exit(1); }
|
||||
}
|
||||
if (checkLimit('u1', res) !== false) { console.error('FAIL: request 101 not blocked'); process.exit(1); }
|
||||
if (res.statusCode !== 429) { console.error('FAIL: status not 429'); process.exit(1); }
|
||||
|
||||
reset();
|
||||
for (var j = 0; j < 101; j++) checkLimit('uA', res);
|
||||
if (checkLimit('uB', res) !== true) { console.error('FAIL: user isolation broken'); process.exit(1); }
|
||||
|
||||
console.log('smoke ok');
|
||||
process.exit(0);
|
||||
|
|
|
|||
Loading…
Reference in New Issue