Skip to content

feat: add retry logic for Redis rate limiter #5939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hiSandog wants to merge 1 commit into
base: main
Choose a base branch
from
+44 −14
Open

feat: add retry logic for Redis rate limiter #5939

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 44 additions & 14 deletions packages/server/src/utils/rateLimit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,51 @@ export class RateLimiterManager {
const release = await this.rateLimiterMutex.acquire()
try {
if (process.env.MODE === MODE.QUEUE) {
this.rateLimiters.set(
id,
rateLimit({
windowMs: duration * 1000,
max: limit,
standardHeaders: true,
legacyHeaders: false,
message,
store: new RedisStore({
prefix: `rl:${id}`,
// @ts-expect-error - Known issue: the `call` function is not present in @types/ioredis
sendCommand: (...args: string[]) => this.redisClient.call(...args)
// Add retry logic for Redis connection issues
const maxRetries = 3
let retryCount = 0
let lastError: Error | null = null

while (retryCount < maxRetries) {
try {
this.rateLimiters.set(
id,
rateLimit({
windowMs: duration * 1000,
max: limit,
standardHeaders: true,
legacyHeaders: false,
message,
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The message parameter, originating from chatFlow.apiConfig, is directly passed to the rateLimit middleware. This is susceptible to Stored Cross-Site Scripting (XSS) if it contains unsanitized HTML or JavaScript, as express-rate-limit sends this string as the response body. This is particularly risky if chatflows are shared or embedded in public websites.

store: new RedisStore({
prefix: `rl:${id}`,
// @ts-expect-error - Known issue: the `call` function is not present in @types/ioredis
sendCommand: (...args: string[]) => this.redisClient.call(...args)
})
})
)
break // Success, exit retry loop
} catch (error) {
lastError = error as Error
retryCount++
if (retryCount < maxRetries) {
// Wait before retry (exponential backoff)
await new Promise(resolve => setTimeout(resolve, Math.pow(2, retryCount) * 100))
}
}
}
Comment on lines +103 to +129
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The retry logic implemented here is ineffective for detecting Redis connection issues. The rateLimit() function and the RedisStore constructor are synchronous and do not perform network operations during initialization. Connection errors will only occur when the middleware is actually executed during an incoming request. Consequently, the try-catch block will not catch connection failures at this stage, and the fallback to the in-memory rate limiter will never be triggered during initialization, even if Redis is unavailable.


if (retryCount >= maxRetries && lastError) {
console.error(`Failed to add rate limiter after ${maxRetries} retries:`, lastError.message)
// Fall back to in-memory rate limiter if Redis fails
this.rateLimiters.set(
id,
rateLimit({
windowMs: duration * 1000,
max: limit,
message: `${message} (fallback mode - Redis unavailable)`
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The same Stored XSS vulnerability exists in the fallback rate limiter message. The untrusted message is concatenated with a suffix and used as the response body for the in-memory rate limiter.

})
})
)
)
}
} else {
this.rateLimiters.set(
id,
Expand Down