disable api endpoint in-app rate-limiting (#54890)

Author: rachmari

src/frame/middleware/api.ts

@@ -10,26 +10,14 @@ import article from '@/article-api/middleware/article'
 import webhooks from '@/webhooks/middleware/webhooks.js'
 import { ExtendedRequest } from '@/types'
 import { noCacheControl } from './cache-control'
-import { createRateLimiter } from '@/shielding/middleware/rate-limit'
 
 const router = express.Router()
 
-// Please make sure to rate limit all routes in this file.
-const createAPIRateLimiter = (hitsPerMin: number) => createRateLimiter(hitsPerMin, true)
-
-let eventsRouteRateLimit = 100
-let internalRoutesRateLimit = 25 // Used internally, higher rate limits
-let searchRoutesRateLimit = 15
-let publicRoutesRateLimit = 10 // Used publicly, lower rate limits
-if (process.env.NODE_ENV === 'test') {
-  searchRoutesRateLimit = 2 // set to 2 so that api-ai-search.ts test will exceed rate limit after 3 requests
-}
-
-router.use('/events', createAPIRateLimiter(eventsRouteRateLimit), events)
-router.use('/webhooks', createAPIRateLimiter(internalRoutesRateLimit), webhooks)
-router.use('/anchor-redirect', createAPIRateLimiter(internalRoutesRateLimit), anchorRedirect)
-router.use('/pagelist', createAPIRateLimiter(publicRoutesRateLimit), pageList)
-router.use('/article', createAPIRateLimiter(publicRoutesRateLimit), article)
+router.use('/events', events)
+router.use('/webhooks', webhooks)
+router.use('/anchor-redirect', anchorRedirect)
+router.use('/pagelist', pageList)
+router.use('/article', article)
 
 // The purpose of this is for convenience to everyone who runs this code
 // base locally but don't have an Elasticsearch server locally.
@@ -38,14 +26,13 @@ router.use('/article', createAPIRateLimiter(publicRoutesRateLimit), article)
 // server or the known credentials to a remote Elasticsearch. Whenever
 // that's the case, they can just HTTP proxy to the production server.
 if (process.env.CSE_COPILOT_ENDPOINT || process.env.NODE_ENV === 'test') {
-  router.use('/ai-search', createAPIRateLimiter(searchRoutesRateLimit), aiSearch)
+  router.use('/ai-search', aiSearch)
 } else {
   console.log(
     'Proxying AI Search requests to docs.github.com. To use the cse-copilot endpoint, set the CSE_COPILOT_ENDPOINT environment variable.',
   )
   router.use(
     '/ai-search',
-    createAPIRateLimiter(searchRoutesRateLimit),
     createProxyMiddleware({
       target: 'https://docs.github.com',
       changeOrigin: true,
@@ -56,11 +43,10 @@ if (process.env.CSE_COPILOT_ENDPOINT || process.env.NODE_ENV === 'test') {
   )
 }
 if (process.env.ELASTICSEARCH_URL) {
-  router.use('/search', createAPIRateLimiter(searchRoutesRateLimit), search)
+  router.use('/search', search)
 } else {
   router.use(
     '/search',
-    createAPIRateLimiter(searchRoutesRateLimit),
     createProxyMiddleware({
       target: 'https://docs.github.com',
       changeOrigin: true,
@@ -74,7 +60,7 @@ if (process.env.ELASTICSEARCH_URL) {
 // We need access to specific httpOnly cookies set on github.com from the client
 // The only way to access these on the client is to fetch them from the server
 // Limit this endpoint to 1req/min because a client should only call this route once
-router.get('/cookies', createAPIRateLimiter(1), (req, res) => {
+router.get('/cookies', (req, res) => {
   noCacheControl(res)
   const cookies = {
     isStaff: Boolean(req.cookies?.staffonly?.startsWith('yes')) || false,

src/search/tests/api-ai-search.ts

@@ -1,6 +1,3 @@
-// IMPORTANT: If you add more tests to this that make requests to
-// http://localhost:4000/api/ai-search/v1 make sure you increment the rate limit
-// value when NODE_ENV === 'test' in src/search/middleware/ai-search.ts
 import { expect, test, describe, beforeAll, afterAll } from 'vitest'
 
 import { post } from 'src/tests/helpers/e2etest.js'
@@ -148,50 +145,4 @@ describe('AI Search Routes', () => {
       },
     ])
   })
-
-  test('should rate limit when total number of requests exceeds max amount', async () => {
-    let apiBody = { query: 'How do I create a Repository?', language: 'en', version: 'dotcom' }
-
-    // First request isn't rate limited
-    const response = await fetch('http://localhost:4000/api/ai-search/v1', {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json', 'fastly-client-ip': 'abc' },
-      body: JSON.stringify(apiBody),
-    })
-
-    expect(response.ok).toBe(true)
-    expect(response.status).toBe(200)
-    const limit = parseInt(response.headers.get('ratelimit-limit') || '0')
-    const remaining = parseInt(response.headers.get('ratelimit-remaining') || '0')
-    expect(limit).toEqual(2)
-    expect(remaining).toBeLessThan(limit)
-
-    // Second request uses our last unused rate limit
-    const response2 = await fetch('http://localhost:4000/api/ai-search/v1', {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json', 'fastly-client-ip': 'abc' },
-      body: JSON.stringify(apiBody),
-    })
-
-    expect(response2.ok).toBe(true)
-    expect(response2.status).toBe(200)
-    let newLimit = parseInt(response2.headers.get('ratelimit-limit') || '0')
-    let newRemaining = parseInt(response2.headers.get('ratelimit-remaining') || '0')
-    expect(newLimit).toBe(limit)
-    expect(newRemaining).toBeLessThan(remaining)
-
-    // Our third request should be rate limited
-    const response3 = await fetch('http://localhost:4000/api/ai-search/v1', {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json', 'fastly-client-ip': 'abc' },
-      body: JSON.stringify(apiBody),
-    })
-
-    expect(response3.ok).toBe(false)
-    // temporary, its 418 now ... expect(response3.status).toBe(429)
-    newLimit = parseInt(response3.headers.get('ratelimit-limit') || '0')
-    newRemaining = parseInt(response3.headers.get('ratelimit-remaining') || '0')
-    expect(newLimit).toBe(limit)
-    expect(newRemaining).toBe(0)
-  })
 })

src/shielding/tests/shielding.ts

@@ -138,28 +138,6 @@ describe('rate limiting', () => {
     expect(res.headers['ratelimit-remaining']).toBeUndefined()
   })
 
-  test('/api/cookies only allows 1 request per minute', async () => {
-    // Cookies only allows 1 request per minute
-    const res1 = await get('/api/cookies', {
-      headers: {
-        'fastly-client-ip': 'abc123',
-      },
-    })
-    expect(res1.statusCode).toBe(200)
-    expect(res1.headers['ratelimit-limit']).toBe('1')
-    expect(res1.headers['ratelimit-remaining']).toBe('0')
-
-    // A second request should be rate limited
-    const res2 = await get('/api/cookies', {
-      headers: {
-        'fastly-client-ip': 'abc123',
-      },
-    })
-    // temporary, its 418 now ... expect(res2.statusCode).toBe(429)
-    expect(res2.headers['ratelimit-limit']).toBe('1')
-    expect(res2.headers['ratelimit-remaining']).toBe('0')
-  })
-
   test('Fastly IPs are not rate limited', async () => {
     // Fastly IPs are in the form `X.X.X.X/Y`
     // Rate limited IPs are in the form `X.X.X.X`