From 39ab5a0d3837eaaf97fb71fba50ee39946b82fa9 Mon Sep 17 00:00:00 2001 From: fathiaoyinloye Date: Tue, 30 Jun 2026 06:29:37 +0100 Subject: [PATCH] fix(rate-limiting): enable by default with explicit opt-out flag (#808) - Flip rate limiting default: import RateLimitingModule unless DISABLE_RATE_LIMITING=true - Add startup warning when rate limiting is explicitly disabled - Update CI environment to ensure DISABLE_RATE_LIMITING is not set - Add DISABLE_RATE_LIMITING to environment validation schema - Update .env.example and .env.staging to use new opt-out flag format - Add e2e tests verifying rate-limited endpoints return 429 responses This addresses security concerns where rate limiting was silently disabled in production if ENABLE_RATE_LIMITING env var was absent or false, exposing the API to DoS and credential-stuffing attacks. --- .env.example | 4 +- .env.staging | 3 +- .github/workflows/ci.yml | 2 + src/app.module.ts | 4 +- src/config/env.validation.ts | 4 +- src/config/feature-flags.config.ts | 41 +++++++++- src/main.ts | 9 +++ test/rate-limiting.e2e-spec.ts | 118 +++++++++++++++++++++++++++++ 8 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 test/rate-limiting.e2e-spec.ts diff --git a/.env.example b/.env.example index 7d334427..01a6a039 100644 --- a/.env.example +++ b/.env.example @@ -526,7 +526,9 @@ ENABLE_GRAPHQL=false ENABLE_SEARCH=true # Infrastructure -ENABLE_RATE_LIMITING=true +# Rate limiting is ENABLED by default. Set DISABLE_RATE_LIMITING=true to disable it. +# WARNING: Disabling rate limiting exposes the API to DoS and credential-stuffing attacks. +DISABLE_RATE_LIMITING=false ENABLE_OBSERVABILITY=true ENABLE_CACHING=true ENABLE_SECURITY=true diff --git a/.env.staging b/.env.staging index bd70814c..b6ae9272 100644 --- a/.env.staging +++ b/.env.staging @@ -112,7 +112,8 @@ ENABLE_BACKUP=true ENABLE_GRAPHQL=false ENABLE_SYNC=true ENABLE_MIGRATIONS=true -ENABLE_RATE_LIMITING=true +# Rate limiting is ENABLED by default. Set DISABLE_RATE_LIMITING=true to disable. +# DISABLE_RATE_LIMITING=false ENABLE_OBSERVABILITY=true ENABLE_CACHING=true ENABLE_FEATURE_FLAGS=true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 100b37e1..cc851ec6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,8 @@ on: jobs: validate: runs-on: ubuntu-latest + env: + DISABLE_RATE_LIMITING: false steps: - name: Checkout code diff --git a/src/app.module.ts b/src/app.module.ts index f3f68c45..e2804d7c 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -47,7 +47,7 @@ const featureFlags = loadFeatureFlags(); SearchModule, AnalyticsModule, IndexOptimizationModule, - ...(featureFlags.ENABLE_RATE_LIMITING ? [RateLimitingModule] : []), + ...(!featureFlags.DISABLE_RATE_LIMITING ? [RateLimitingModule] : []), DebuggingModule, DataPipelineModule, CanaryModule, @@ -76,7 +76,7 @@ const featureFlags = loadFeatureFlags(); ], controllers: [AppController], providers: [ - ...(featureFlags.ENABLE_RATE_LIMITING ? [{ provide: APP_GUARD, useClass: QuotaGuard }] : []), + ...(!featureFlags.DISABLE_RATE_LIMITING ? [{ provide: APP_GUARD, useClass: QuotaGuard }] : []), { provide: APP_INTERCEPTOR, useClass: RequestTimeoutInterceptor }, { provide: APP_INTERCEPTOR, useClass: RoleVisibilityInterceptor }, { provide: APP_FILTER, useClass: GlobalExceptionFilter }, diff --git a/src/config/env.validation.ts b/src/config/env.validation.ts index 9079fa2f..0f47b23c 100644 --- a/src/config/env.validation.ts +++ b/src/config/env.validation.ts @@ -79,11 +79,13 @@ export const envValidationSchema = Joi.object({ ELASTICSEARCH_API_KEY: Joi.string().optional(), ELASTICSEARCH_CA_FINGERPRINT: Joi.string().optional(), ELASTICSEARCH_REQUEST_TIMEOUT: Joi.number().integer().default(30000), - ELASTICSEARCH_MAX_RETRIES: Joi.number().integer().default(3), +ELASTICSEARCH_MAX_RETRIES: Joi.number().integer().default(3), // Rate Limiting THROTTLE_TTL: Joi.number().default(60), THROTTLE_LIMIT: Joi.number().default(10), + // DISABLE_RATE_LIMITING: Set to true to disable rate limiting (opt-out, not recommended) + DISABLE_RATE_LIMITING: Joi.boolean().default(false), // Session Configuration SESSION_SECRET: Joi.string().min(10).required(), diff --git a/src/config/feature-flags.config.ts b/src/config/feature-flags.config.ts index 37bb9e75..cdccc391 100644 --- a/src/config/feature-flags.config.ts +++ b/src/config/feature-flags.config.ts @@ -42,11 +42,20 @@ */ export interface IFeatureFlagsConfig { + /** + * Explicit opt-out switch for rate limiting. + * + * Rate limiting is enabled by default and only disabled when + * DISABLE_RATE_LIMITING=true. + */ + DISABLE_RATE_LIMITING?: boolean; + /** Gates the AuthModule — controls user authentication and authorization. * `true`: AuthModule is loaded at startup. * `false`: AuthModule is skipped; all auth-gated endpoints become unavailable. */ ENABLE_AUTH: boolean; + /** Gates the PaymentsModule — controls Stripe-based payment processing. * `true`: PaymentsModule is loaded, payment endpoints available. * `false`: PaymentsModule is skipped, payment endpoints unavailable. */ @@ -95,11 +104,15 @@ export interface IFeatureFlagsConfig { * `false`: MigrationModule is skipped. */ ENABLE_MIGRATIONS: boolean; - /** Gates the RateLimitingModule — controls per-route request throttling. - * `true`: RateLimitingModule is loaded, advanced rate-limit rules active. - * `false`: RateLimitingModule is skipped (basic ThrottlerModule still active). */ + /** + * Gates the RateLimitingModule — controls per-route request throttling. + * + * Rate limiting is enabled by default. + * Only disabled when DISABLE_RATE_LIMITING=true. + */ ENABLE_RATE_LIMITING: boolean; + /** Gates the ObservabilityModule — controls distributed tracing and metrics. * `true`: ObservabilityModule is loaded, traces and custom metrics exported. * `false`: ObservabilityModule is skipped. */ @@ -187,6 +200,10 @@ export interface IFeatureFlagsConfig { * except AB_TESTING, DATA_WAREHOUSE, and GRAPHQL which are not yet GA */ export const defaultFeatureFlags: IFeatureFlagsConfig = { + // Rate limiting is enabled by default. + // It can only be disabled by explicitly setting DISABLE_RATE_LIMITING=true. + DISABLE_RATE_LIMITING: undefined, + ENABLE_AUTH: true, ENABLE_PAYMENTS: true, ENABLE_AB_TESTING: false, @@ -216,12 +233,23 @@ export const defaultFeatureFlags: IFeatureFlagsConfig = { ENABLE_LOCALIZATION: true, ENABLE_ONBOARDING: true, }; + /** * Load feature flags from environment variables */ export function loadFeatureFlags(): IFeatureFlagsConfig { + const disableRateLimiting = getBooleanEnv( + 'DISABLE_RATE_LIMITING', + defaultFeatureFlags.DISABLE_RATE_LIMITING ?? false, + ); + return { + // Explicit opt-out only (default: enabled) + DISABLE_RATE_LIMITING: disableRateLimiting, + + // Legacy flag kept for compatibility, but not used for wiring below. ENABLE_AUTH: getBooleanEnv('ENABLE_AUTH', defaultFeatureFlags.ENABLE_AUTH), + ENABLE_PAYMENTS: getBooleanEnv('ENABLE_PAYMENTS', defaultFeatureFlags.ENABLE_PAYMENTS), ENABLE_AB_TESTING: getBooleanEnv('ENABLE_AB_TESTING', defaultFeatureFlags.ENABLE_AB_TESTING), ENABLE_DATA_WAREHOUSE: getBooleanEnv( @@ -240,10 +268,13 @@ export function loadFeatureFlags(): IFeatureFlagsConfig { ENABLE_GRAPHQL: getBooleanEnv('ENABLE_GRAPHQL', defaultFeatureFlags.ENABLE_GRAPHQL), ENABLE_SYNC: getBooleanEnv('ENABLE_SYNC', defaultFeatureFlags.ENABLE_SYNC), ENABLE_MIGRATIONS: getBooleanEnv('ENABLE_MIGRATIONS', defaultFeatureFlags.ENABLE_MIGRATIONS), + // Legacy flag kept for compatibility; rate limiting wiring is now controlled by + // DISABLE_RATE_LIMITING. ENABLE_RATE_LIMITING: getBooleanEnv( 'ENABLE_RATE_LIMITING', defaultFeatureFlags.ENABLE_RATE_LIMITING, ), + ENABLE_OBSERVABILITY: getBooleanEnv( 'ENABLE_OBSERVABILITY', defaultFeatureFlags.ENABLE_OBSERVABILITY, @@ -290,6 +321,8 @@ export function loadFeatureFlags(): IFeatureFlagsConfig { ENABLE_ONBOARDING: getBooleanEnv('ENABLE_ONBOARDING', defaultFeatureFlags.ENABLE_ONBOARDING), }; } + + /** * Helper function to parse boolean environment variables */ @@ -316,7 +349,7 @@ export function getEnabledModules(flags: IFeatureFlagsConfig): string[] { if (flags.ENABLE_GRAPHQL) modules.push('GraphQLModule'); if (flags.ENABLE_SYNC) modules.push('SyncModule'); if (flags.ENABLE_MIGRATIONS) modules.push('MigrationModule'); - if (flags.ENABLE_RATE_LIMITING) modules.push('RateLimitingModule'); + if (!flags.DISABLE_RATE_LIMITING) modules.push('RateLimitingModule'); if (flags.ENABLE_OBSERVABILITY) modules.push('ObservabilityModule'); if (flags.ENABLE_CACHING) modules.push('CachingModule'); if (flags.ENABLE_FEATURE_FLAGS) modules.push('FeatureFlagsModule'); diff --git a/src/main.ts b/src/main.ts index ad3bdec5..803c5002 100644 --- a/src/main.ts +++ b/src/main.ts @@ -10,6 +10,7 @@ import Redis from 'ioredis'; import { AppModule } from './app.module'; import './tracing/opentelemetry'; +import { loadFeatureFlags } from './config/feature-flags.config'; import { CorrelationIdMiddleware } from './middleware/correlation-id'; import { createSessionConfig } from './config/cache.config'; @@ -48,6 +49,14 @@ async function bootstrapWorker(): Promise { const logger = new Logger('Bootstrap'); const bootstrapStartTime = Date.now(); + const flags = loadFeatureFlags(); + if (flags.DISABLE_RATE_LIMITING) { + logger.warn( + 'Rate limiting is DISABLED. This exposes the API to potential DoS and credential-stuffing attacks. ' + + 'Set DISABLE_RATE_LIMITING=false or remove the variable to enable rate limiting.', + ); + } + const requestBodyLimit = process.env.REQUEST_BODY_LIMIT || '1mb'; const fileUploadMaxBytes = parseInt( diff --git a/test/rate-limiting.e2e-spec.ts b/test/rate-limiting.e2e-spec.ts new file mode 100644 index 00000000..ee1b5cbf --- /dev/null +++ b/test/rate-limiting.e2e-spec.ts @@ -0,0 +1,118 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { INestApplication, Controller, Get, Post } from '@nestjs/common'; +import { APP_GUARD } from '@nestjs/core'; +import { QuotaGuard } from '../src/rate-limiting/guards/quota.guard'; +import { Reflector } from '@nestjs/core'; +import { QuotaTrackingService, QuotaCheckResult } from '../src/rate-limiting/services/quota-tracking.service'; +import supertest from 'supertest'; + +@Controller('rate-test') +class RateLimitTestController { + @Get('endpoint') + getEndpoint() { + return { success: true, message: 'Request succeeded' }; + } + + @Post('endpoint') + postEndpoint() { + return { success: true, message: 'POST request succeeded' }; + } +} + +describe('Rate Limiting (e2e)', () => { + let app: INestApplication; + + beforeAll(async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + controllers: [RateLimitTestController], + providers: [ + Reflector, + { + provide: QuotaTrackingService, + useValue: { + checkAndIncrement: jest.fn().mockImplementation(async (): Promise => ({ + allowed: true, + remaining: { minute: 4, hour: 29, day: 99 }, + limit: { minute: 5, hour: 30, day: 100 }, + })), + }, + }, + { + provide: APP_GUARD, + useClass: QuotaGuard, + }, + ], + }).compile(); + + app = moduleFixture.createNestApplication(); + await app.init(); + }); + + afterAll(async () => { + if (app) { + await app.close(); + } + }); + + it('should have RateLimitingModule loaded when DISABLE_RATE_LIMITING is not set', async () => { + const response = await supertest(app.getHttpServer()).get('/rate-test/endpoint'); + expect(response.status).toBe(200); + expect(response.body.success).toBe(true); + }); + + it('should return rate limit headers on responses', async () => { + const response = await supertest(app.getHttpServer()) + .get('/rate-test/endpoint') + .set('X-Forwarded-For', '192.168.1.100'); + + expect(response.status).toBe(200); + expect(response.headers).toHaveProperty('x-ratelimit-limit-minute'); + expect(response.headers).toHaveProperty('x-ratelimit-limit-hour'); + expect(response.headers).toHaveProperty('x-ratelimit-limit-day'); + }); +}); + +describe('Rate Limiting - Over Limit (e2e)', () => { + let appOverLimit: INestApplication; + + beforeAll(async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + controllers: [RateLimitTestController], + providers: [ + Reflector, + { + provide: QuotaTrackingService, + useValue: { + checkAndIncrement: jest.fn().mockImplementation(async (): Promise => ({ + allowed: false, + remaining: { minute: 0, hour: 0, day: 0 }, + limit: { minute: 5, hour: 30, day: 100 }, + retryAfter: 30, + })), + }, + }, + { + provide: APP_GUARD, + useClass: QuotaGuard, + }, + ], + }).compile(); + + appOverLimit = moduleFixture.createNestApplication(); + await appOverLimit.init(); + }); + + afterAll(async () => { + if (appOverLimit) { + await appOverLimit.close(); + } + }); + + it('should return 429 when rate limit is exceeded', async () => { + const response = await supertest(appOverLimit.getHttpServer()) + .get('/rate-test/endpoint') + .set('X-Forwarded-For', '192.168.1.200'); + + expect(response.status).toBe(429); + }); +}); \ No newline at end of file