Skip to content

Commit 2a1b3a8

Browse files
committed
chore: address coderabbit comments
1 parent 88413ea commit 2a1b3a8

3 files changed

Lines changed: 11 additions & 3 deletions

File tree

packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ describe('checkMachineTokenRateLimit', () => {
5757
expect(checkMachineTokenRateLimit('unknown')).toBe(false);
5858
});
5959

60-
it('clears all buckets and allows requests again when MAX_BUCKETS is reached', () => {
60+
it('evicts the oldest bucket and allows a new IP when MAX_BUCKETS is reached', () => {
6161
// Fill up to MAX_BUCKETS (10 000) unique IPs
6262
for (let i = 0; i < 10_000; i++) {
6363
checkMachineTokenRateLimit(`10.${Math.floor(i / 65536)}.${Math.floor((i % 65536) / 256)}.${i % 256}`);
6464
}
65-
// The 10 001st IP triggers the clear; the new IP gets a fresh bucket
65+
// The 10 001st IP triggers eviction of the oldest entry; the new IP gets a fresh bucket
6666
const freshIp = '172.16.0.1';
6767
expect(checkMachineTokenRateLimit(freshIp)).toBe(true);
6868
});

packages/backend/src/tokens/machineTokenRateLimiter.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ const buckets = new Map<string, Bucket>();
77

88
export function checkMachineTokenRateLimit(ip: string): boolean {
99
if (buckets.size >= MAX_BUCKETS) {
10-
buckets.clear();
10+
// Evict the oldest entry rather than clearing all buckets to prevent an attacker
11+
// from neutralizing rate limits by forcing key churn across many distinct IPs.
12+
const oldest = buckets.keys().next().value;
13+
if (oldest !== undefined) {
14+
buckets.delete(oldest);
15+
}
1116
}
1217
const now = Date.now();
1318
const existing = buckets.get(ip);

packages/backend/src/tokens/request.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import { TokenType } from './tokenTypes';
2828
import type { AuthenticateRequestOptions } from './types';
2929
import { verifyMachineAuthToken, verifyToken } from './verify';
3030

31+
// NOTE: IP headers like x-forwarded-for can be spoofed by clients not behind a trusted proxy.
32+
// cf-connecting-ip (set by Cloudflare) and x-real-ip (set by Nginx) are more reliable when present.
33+
// The rate limiter is defense-in-depth against BAPI quota exhaustion, not a security boundary.
3134
function extractCallerIp(request: Request): string {
3235
const cfConnectingIp = request.headers.get(constants.Headers.CfConnectingIp);
3336
if (cfConnectingIp) {

0 commit comments

Comments
 (0)