🔐 Fix Medium: Rate Limiting Bypass Vulnerabilities (CVSS 6.5)

workflow/packages/backend/api/src/app/core/security/rate-limit.ts

@@ -6,19 +6,30 @@ import { AppSystemProp, networkUtils } from 'workflow-server-shared'
 import { createRedisClient } from '../../database/redis-connection'
 import { QueueMode, system } from '../../helper/system/system'
 
-const API_RATE_LIMIT_AUTHN_ENABLED = system.getBoolean(
-    AppSystemProp.API_RATE_LIMIT_AUTHN_ENABLED,
-)
+// SECURITY FIX: Rate limiting is now ALWAYS enabled for security
+// Removed dangerous environment variable bypass
+const API_RATE_LIMIT_AUTHN_ENABLED = true // ✅ SECURE: Always enabled
 
 export const rateLimitModule: FastifyPluginAsyncTypebox = FastifyPlugin(
     async (app) => {
-        if (API_RATE_LIMIT_AUTHN_ENABLED) {
-            await app.register(RateLimitPlugin, {
-                global: false,
-                keyGenerator: (req) => networkUtils.extractClientRealIp(req, system.get(AppSystemProp.CLIENT_REAL_IP_HEADER)),
-                redis: getRedisClient(),
-            })
-        }
+        // SECURITY FIX: Always apply rate limiting, implement fail-secure principle
+        const redisClient = getRedisClient()
+
+        await app.register(RateLimitPlugin, {
+            global: false,
+            keyGenerator: (req) => networkUtils.extractClientRealIp(req, system.get(AppSystemProp.CLIENT_REAL_IP_HEADER)),
+            redis: redisClient,
+            // SECURITY FIX: Fallback limits when Redis is unavailable
+            max: redisClient ? 100 : 50, // Lower limits without Redis
+            timeWindow: '15 minutes',
+            // SECURITY FIX: Fail-secure when Redis connection fails
+            skipOnError: false, // Don't bypass rate limiting on Redis errors
+            onExceeded: (req, reply) => {
+                app.log.warn(`Rate limit exceeded for IP: ${networkUtils.extractClientRealIp(req, system.get(AppSystemProp.CLIENT_REAL_IP_HEADER))}`)
+            },
+        })
+
+        app.log.info('Rate limiting initialized with fail-secure configuration')
     },
 )
 

workflow/packages/backend/api/src/app/workers/redis/redis-rate-limiter.ts

@@ -13,7 +13,9 @@ import { redisQueue } from './redis-queue'
 
 const RATE_LIMIT_QUEUE_NAME = 'rateLimitJobs'
 const MAX_CONCURRENT_JOBS_PER_PROJECT = system.getNumberOrThrow(AppSystemProp.MAX_CONCURRENT_JOBS_PER_PROJECT)
-const PROJECT_RATE_LIMITER_ENABLED = system.getBoolean(AppSystemProp.PROJECT_RATE_LIMITER_ENABLED)
+// SECURITY FIX: Project rate limiting is now ALWAYS enabled for security
+// Removed dangerous environment variable bypass
+const PROJECT_RATE_LIMITER_ENABLED = true // ✅ SECURE: Always enabled
 const SUPPORTED_QUEUES = [QueueName.ONE_TIME, QueueName.WEBHOOK]
 
 let redis: Redis
@@ -83,23 +85,34 @@ export const redisRateLimiter = (log: FastifyBaseLogger) => ({
     async shouldBeLimited(projectId: string | undefined, jobId: string): Promise<{
         shouldRateLimit: boolean
     }> {
-        if (isNil(projectId) || !PROJECT_RATE_LIMITER_ENABLED) {
+        if (isNil(projectId)) {
             return {
                 shouldRateLimit: false,
             }
         }
 
-        const newActiveRuns = (await redis.keys(`${projectKey(projectId)}*`)).length
-        if (newActiveRuns >= MAX_CONCURRENT_JOBS_PER_PROJECT) {
-            return {
-                shouldRateLimit: true,
+        try {
+            const newActiveRuns = (await redis.keys(`${projectKey(projectId)}*`)).length
+            if (newActiveRuns >= MAX_CONCURRENT_JOBS_PER_PROJECT) {
+                return {
+                    shouldRateLimit: true,
+                }
             }
-        }
-        const redisKey = projectKeyWithJobId(projectId, jobId)
-        await redis.set(redisKey, 1, 'EX', 600)
+            const redisKey = projectKeyWithJobId(projectId, jobId)
+            await redis.set(redisKey, 1, 'EX', 600)
 
-        return {
-            shouldRateLimit: false,
+            return {
+                shouldRateLimit: false,
+            }
+        } catch (error) {
+            // SECURITY FIX: Fail-secure on Redis errors
+            // Apply more restrictive rate limiting when Redis is unavailable
+            log.warn(`Redis error in rate limiting, applying fail-secure limits: ${error}`)
+
+            // Conservative approach: limit to fewer concurrent jobs when Redis fails
+            return {
+                shouldRateLimit: true, // ✅ SECURE: Err on side of caution
+            }
         }
     },