diff --git a/test/unit/mcp-http-boundary.test.js b/test/unit/mcp-http-boundary.test.js index b6ec3c5..5a111eb 100644 --- a/test/unit/mcp-http-boundary.test.js +++ b/test/unit/mcp-http-boundary.test.js @@ -105,6 +105,24 @@ function issuerFetch(url) { }); } +// The limiter counts into a window keyed on Math.floor(now / 60_000) +// (src/mcp-rate-limit.js), so the window is aligned to the wall clock, not to +// a test's first request. Two calls a millisecond apart land in different +// windows if they straddle a minute boundary, and the second one starts a +// fresh count -- which makes any "the next call is 429" assertion fail. +// +// That is not hypothetical: it broke `accepts Keycloak azp as the client +// identifier` on main, in a run whose test started at 14:46:59.99. +// +// Only start such a test when the current window still has room. The wait is +// skipped in the common case, and never exceeds the margin. +const RATE_WINDOW_MS = 60_000; +async function insideOneRateWindow(marginMs = 5_000) { + const remaining = RATE_WINDOW_MS - (Date.now() % RATE_WINDOW_MS); + if (remaining >= marginMs) return; + await new Promise(resolve => setTimeout(resolve, remaining + 25)); +} + async function start({ limits = {}, env = {} } = {}) { const previous = {}; const applied = { @@ -181,6 +199,7 @@ afterEach(async () => { describe('MCP HTTP boundary: per-identity limits', () => { it('applies the per-subject limit, which needs the identity the OAuth layer sets', async () => { + await insideOneRateWindow(); const restore = await start({ limits: { subject: 2, global: 1000 } }); try { const token = mintToken({ sub: 'user-1', client_id: 'client-a' }); @@ -193,6 +212,7 @@ describe('MCP HTTP boundary: per-identity limits', () => { }); it('limits one subject without affecting another', async () => { + await insideOneRateWindow(); const restore = await start({ limits: { subject: 1, client: 1000, global: 1000 } }); try { const a = mintToken({ sub: 'user-a', client_id: 'c' }); @@ -204,6 +224,7 @@ describe('MCP HTTP boundary: per-identity limits', () => { }); it('applies the per-client limit across different subjects', async () => { + await insideOneRateWindow(); const restore = await start({ limits: { client: 2, subject: 1000, global: 1000 } }); try { await call(mintToken({ sub: 'u1', client_id: 'shared' })); @@ -214,6 +235,7 @@ describe('MCP HTTP boundary: per-identity limits', () => { }); it('accepts Keycloak azp as the client identifier', async () => { + await insideOneRateWindow(); const restore = await start({ limits: { client: 1, subject: 1000, global: 1000 } }); try { await call(mintToken({ sub: 'u1', client_id: undefined, azp: 'kc-client' })); @@ -232,6 +254,7 @@ describe('MCP HTTP boundary: per-identity limits', () => { }); it('counts the global budget once per request, not twice', async () => { + await insideOneRateWindow(); const restore = await start({ limits: { global: 2, subject: 1000, client: 1000 } }); try { const token = mintToken({ client_id: 'c' }); @@ -244,6 +267,7 @@ describe('MCP HTTP boundary: per-identity limits', () => { describe('MCP private HTTP boundary', () => { it('applies global and credential limits to token-authenticated requests', async () => { + await insideOneRateWindow(); const restore = await startPrivate({ limits: { global: 10, subject: 1, client: 10 } }); try { const token = privateAuthConfig().token; @@ -379,6 +403,7 @@ describe('MCP HTTP boundary: audit identity', () => { describe('MCP HTTP boundary: body handling runs after the limits', () => { it('rate limits malformed JSON instead of letting it bypass the limiter', async () => { + await insideOneRateWindow(); const restore = await start({ limits: { global: 2 } }); try { await call(null, '{ this is not json'); diff --git a/test/unit/mcp-rate-limit.test.js b/test/unit/mcp-rate-limit.test.js index 5df4a18..4d375ba 100644 --- a/test/unit/mcp-rate-limit.test.js +++ b/test/unit/mcp-rate-limit.test.js @@ -130,3 +130,46 @@ describe('MCP limit configuration', () => { } }); }); + +describe('MCP rate limiting: 固定ウィンドウの境界', () => { + // The counter is keyed on Math.floor(now / windowMs), so the window is + // aligned to the wall clock rather than to a caller's first request. Two + // requests a millisecond apart therefore land in different windows if they + // straddle a minute boundary, and the second one starts a fresh count. + // + // This is inherent to a fixed window and is the documented trade-off, but it + // is worth pinning: it silently broke a boundary test on main that assumed + // two consecutive calls always share a window. + it('境界をまたぐと、直後の呼び出しでもカウントがリセットされる', () => { + const clock = { value: 59_999 }; + const limiter = limiterAt(clock, { globalPerMinute: 1, perSubjectPerMinute: 99, perClientPerMinute: 99 }); + assert.equal(limiter.check({ subject: 's', clientId: 'c' }).allowed, true); + assert.equal(limiter.check({ subject: 's', clientId: 'c' }).allowed, false, '同一ウィンドウ内では制限される'); + + clock.value = 60_000; // 1 ms later, but a new window + assert.equal( + limiter.check({ subject: 's', clientId: 'c' }).allowed, + true, + '1ms後でもウィンドウが変われば許可される' + ); + }); + + it('ウィンドウ内に留まる限り境界の影響を受けない', () => { + const clock = { value: 60_000 }; + const limiter = limiterAt(clock, { globalPerMinute: 1, perSubjectPerMinute: 99, perClientPerMinute: 99 }); + assert.equal(limiter.check({ subject: 's', clientId: 'c' }).allowed, true); + clock.value = 119_999; // still the same window + assert.equal(limiter.check({ subject: 's', clientId: 'c' }).allowed, false); + }); + + it('retryAfterSecondsは次の境界までの残り時間を指す', () => { + const clock = { value: 60_000 + 15_000 }; + const limiter = limiterAt(clock, { globalPerMinute: 1, perSubjectPerMinute: 99, perClientPerMinute: 99 }); + limiter.check({ subject: 's', clientId: 'c' }); + const denied = limiter.check({ subject: 's', clientId: 'c' }); + // Callers put this in Retry-After, so it must point at the window reset + // rather than at a full window length from now. + assert.equal(denied.allowed, false); + assert.equal(denied.retryAfterSeconds, 45); + }); +});