Skip to content

Fix: comment 401 — parse Authorization header in rate limiter (clean fix + tests) #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hephaestus-forge-clawbot wants to merge 1 commit into
base: main
Choose a base branch
from
+116 −1
Open

Fix: comment 401 — parse Authorization header in rate limiter (clean fix + tests) #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/middleware/rateLimit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,20 @@ setInterval(() => {

/**
* Get rate limit key from request
* Parses Authorization header directly instead of relying on req.token
* to avoid middleware execution order issues (req.token is set by
* requireAuth, which may run after the rate limiter)
*/
function getKey(req, limitType) {
const identifier = req.token || req.ip || 'anonymous';
const authHeader = req.headers.authorization;
let identifier;

if (authHeader && authHeader.startsWith('Bearer ')) {
identifier = authHeader.substring(7);
} else {
identifier = req.ip || 'anonymous';
}

return `rl:${limitType}:${identifier}`;
}

Expand Down
104 changes: 104 additions & 0 deletions test/rate-limiter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* Rate limiter fix tests
* Verifies getKey() parses Authorization header directly
* instead of relying on req.token (which caused #5)
*/

// Mirror the fixed getKey function
function getKey(req, limitType) {
const authHeader = req.headers.authorization;
let identifier;

if (authHeader && authHeader.startsWith('Bearer ')) {
identifier = authHeader.substring(7);
} else {
identifier = req.ip || 'anonymous';
}

return `rl:${limitType}:${identifier}`;
}

let passed = 0;
let failed = 0;

function test(name, fn) {
try {
fn();
console.log(` + ${name}`);
passed++;
} catch (error) {
console.log(` - ${name}`);
console.log(` Error: ${error.message}`);
failed++;
}
}

function assertEqual(actual, expected, message) {
if (actual !== expected) {
throw new Error(message || `Expected "${expected}", got "${actual}"`);
}
}

function assertNotEqual(actual, expected, message) {
if (actual === expected) {
throw new Error(message || `Expected values to differ, both were "${actual}"`);
}
}

console.log('\nRate Limiter Fix Tests\n' + '='.repeat(50));

test('extracts token from Bearer header', () => {
const req = {
headers: { authorization: 'Bearer moltbook_test_token_123' },
ip: '127.0.0.1'
};
assertEqual(getKey(req, 'comments'), 'rl:comments:moltbook_test_token_123');
});

test('falls back to IP when no Authorization header', () => {
const req = { headers: {}, ip: '192.168.1.1' };
assertEqual(getKey(req, 'comments'), 'rl:comments:192.168.1.1');
});

test('falls back to anonymous when nothing available', () => {
const req = { headers: {}, ip: null };
assertEqual(getKey(req, 'comments'), 'rl:comments:anonymous');
});

test('handles non-Bearer auth schemes by falling back to IP', () => {
const req = { headers: { authorization: 'Basic abc123' }, ip: '10.0.0.1' };
assertEqual(getKey(req, 'requests'), 'rl:requests:10.0.0.1');
});

test('ignores req.token entirely (root cause of #5)', () => {
const req = {
headers: { authorization: 'Bearer real_token' },
ip: '127.0.0.1',
token: 'this_should_be_ignored'
};
const key = getKey(req, 'comments');
assertEqual(key, 'rl:comments:real_token');
assertNotEqual(key, 'rl:comments:this_should_be_ignored',
'REGRESSION: getKey is using req.token instead of Authorization header');
});

test('works when req.token is undefined (the actual #5 scenario)', () => {
const req = {
headers: { authorization: 'Bearer moltbook_sk_valid_key' },
ip: '172.17.0.1',
token: undefined
};
assertEqual(getKey(req, 'comments'), 'rl:comments:moltbook_sk_valid_key');
});

test('handles repeated requests without identity drift', () => {
const apiKey = 'moltbook_sk_8Xn6T1MLuY_test';
for (let i = 0; i < 50; i++) {
const req = { headers: { authorization: `Bearer ${apiKey}` }, ip: '10.0.0.1' };
assertEqual(getKey(req, 'comments'), `rl:comments:${apiKey}`);
}
});

console.log('\n' + '='.repeat(50));
console.log(`Results: ${passed} passed, ${failed} failed`);
process.exit(failed > 0 ? 1 : 0);