fix: skip trust proxy validation in rate limiting

Signed-off-by: Heikki Hellgren <heikki.hellgren@op.fi>

.changeset/famous-terms-rescue.md

@@ -9,7 +9,6 @@ Rate limiting can be turned on by adding the following configuration to `app-con
 ```yaml
 backend:
   rateLimit:
-    enabled: true
-    windowMs: 60000
-    limit: 100
+    window: 60000
+    incomingRequestLimit: 100
 ```

packages/backend-defaults/config.d.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { HumanDuration, JsonValue } from '@backstage/types';
+import { HumanDuration } from '@backstage/types';
 
 export interface Config {
   app: {
@@ -793,68 +793,48 @@ export interface Config {
     /**
      * Rate limiting options
      */
-    rateLimit?: {
-      /**
-       * Store to use for rate limiting. If not defined, the store will be automatically
-       * decided based on `backend.cache.store` value. If
-       * redis is not available, the store will be memory.
-       */
-      store?: 'memory' | 'redis';
-      /**
-       * Rate limiting enabled. Defaults to false.
-       */
-      enabled?: boolean;
-      /**
-       * Time frame in milliseconds or as human duration for which requests are checked/remembered.
-       * Defaults to 6000ms.
-       */
-      window?: number | HumanDuration;
-      /**
-       * The maximum number of connections to allow during the `window` before rate limiting the client.
-       * Defaults to 5.
-       */
-      limit?: number;
-      /**
-       * The response body to send back when a client is rate limited.
-       * Defaults to 'Too many requests, please try again later.'.
-       */
-      message?: JsonValue;
-      /**
-       * The HTTP status code to send back when a client is rate limited.
-       * Defaults to 429.
-       */
-      statusCode?: number;
-      /**
-       * Whether to send the legacy rate limit headers for the limit.
-       * Defaults to true.
-       */
-      legacyHeaders?: boolean;
-      /**
-       * Whether to enable support for headers conforming the RateLimit header fields for HTTP
-       * standardization. Defaults to undefined.
-       */
-      standardHeaders?: 'draft-6' | 'draft-7';
-      /**
-       * Whether to pass requests in case of store failure.
-       * Defaults to false.
-       */
-      passOnStoreError?: boolean;
-      /**
-       * List of allowed IP addresses that are not rate limited.
-       * Defaults to [127.0.0.1].
-       */
-      ipAllowList?: string[];
-      /**
-       * Skip rate limiting for requests that have been successful.
-       * Defaults to false.
-       */
-      skipSuccessfulRequests?: boolean;
-      /**
-       * Skip rate limiting for requests that have failed.
-       * Defaults to false.
-       */
-      skipFailedRequests?: boolean;
-    };
+    rateLimit?:
+      | false
+      | {
+          store?:
+            | {
+                client: 'redis';
+                connection: string;
+              }
+            | {
+                client: 'memory';
+              };
+          /**
+           * Time frame in milliseconds or as human duration for which requests are checked/remembered.
+           * Defaults to 6000ms.
+           */
+          window?: number | HumanDuration;
+          /**
+           * The maximum number of connections to allow during the `window` before rate limiting the client.
+           * Defaults to 5.
+           */
+          incomingRequestLimit?: number;
+          /**
+           * Whether to pass requests in case of store failure.
+           * Defaults to false.
+           */
+          passOnStoreError?: boolean;
+          /**
+           * List of allowed IP addresses that are not rate limited.
+           * Defaults to [127.0.0.1, 0:0:0:0:0:0:0:1, ::1].
+           */
+          ipAllowList?: string[];
+          /**
+           * Skip rate limiting for requests that have been successful.
+           * Defaults to false.
+           */
+          skipSuccessfulRequests?: boolean;
+          /**
+           * Skip rate limiting for requests that have failed.
+           * Defaults to false.
+           */
+          skipFailedRequests?: boolean;
+        };
 
     /**
      * Configuration related to URL reading, used for example for reading catalog info

packages/backend-defaults/report-rootHttpRouter.api.md

@@ -20,7 +20,6 @@ import { RootHttpRouterService } from '@backstage/backend-plugin-api';
 import { Router } from 'express';
 import type { Server } from 'node:http';
 import { ServiceFactory } from '@backstage/backend-plugin-api';
-import type { Store } from 'express-rate-limit';
 
 // @public (undocumented)
 export function createHealthRouter(options: {
@@ -111,15 +110,6 @@ export interface MiddlewareFactoryOptions {
   logger: LoggerService;
 }
 
-// @public
-export class RateLimitStoreFactory {
-  constructor(config: Config);
-  // (undocumented)
-  create(): Store | undefined;
-  // (undocumented)
-  redis(): Store;
-}
-
 // @public
 export function readCorsOptions(config?: Config): CorsOptions;
 

packages/backend-defaults/src/entrypoints/rootHttpRouter/http/MiddlewareFactory.ts

@@ -43,12 +43,9 @@ import {
   ServiceUnavailableError,
 } from '@backstage/errors';
 import { applyInternalErrorFilter } from './applyInternalErrorFilter';
-import { DraftHeadersVersion, rateLimit } from 'express-rate-limit';
-import {
-  durationToMilliseconds,
-  HumanDuration,
-  JsonValue,
-} from '@backstage/types';
+import { rateLimit } from 'express-rate-limit';
+import { Config } from '@backstage/config';
+import { durationToMilliseconds, HumanDuration } from '@backstage/types';
 import { RateLimitStoreFactory } from './RateLimitStoreFactory';
 
 type LogMeta = {
@@ -245,15 +242,15 @@ export class MiddlewareFactory {
    * @returns An Express request handler
    */
   rateLimit(): RequestHandler {
-    const rateLimitOptions =
-      this.#config.getOptionalConfig('backend.rateLimit');
-    const enabled = rateLimitOptions?.getOptionalBoolean('enabled') ?? false;
-    if (!rateLimitOptions || !enabled) {
+    const conf = this.#config.getOptional<boolean | Config>(
+      'backend.rateLimit',
+    );
+    if (!conf || typeof conf !== 'object') {
       return (_req: Request, _res: Response, next: NextFunction) => {
         next();
       };
     }
-
+    const rateLimitOptions = conf as Config;
     const window = rateLimitOptions.getOptional<number | HumanDuration>(
       'window',
     );
@@ -272,32 +269,34 @@ export class MiddlewareFactory {
 
     const ipAllowList = rateLimitOptions.getOptionalStringArray(
       'ipAllowList',
-    ) ?? ['127.0.0.1'];
+    ) ?? ['127.0.0.1', '0:0:0:0:0:0:0:1', '::1'];
 
     return rateLimit({
       windowMs,
-      limit: rateLimitOptions.getOptionalNumber('limit'),
-      message: rateLimitOptions.getOptional<string | JsonValue>('message'),
-      statusCode: rateLimitOptions.getOptionalNumber('statusCode'),
+      limit: rateLimitOptions.getOptionalNumber('incomingRequestLimit'),
       skipSuccessfulRequests: rateLimitOptions.getOptionalBoolean(
         'skipSuccessfulRequests',
       ),
       skipFailedRequests:
         rateLimitOptions.getOptionalBoolean('skipFailedRequests'),
-      legacyHeaders: rateLimitOptions.getOptionalBoolean('legacyHeaders'),
-      standardHeaders: rateLimitOptions.getOptionalString(
-        'standardHeaders',
-      ) as DraftHeadersVersion,
       passOnStoreError: rateLimitOptions.getOptionalBoolean('passOnStoreError'),
       keyGenerator(req, _res): string {
         if (!req.ip) {
           return req.socket.remoteAddress!;
         }
-
-        return req.ip.replace(/:\d+[^:]*$/, '');
+        return req.ip;
       },
       skip: (req, _res) => {
-        return Boolean(req.ip && ipAllowList.includes(req.ip));
+        return (
+          Boolean(req.ip && ipAllowList.includes(req.ip)) ||
+          Boolean(
+            req.socket.remoteAddress &&
+              ipAllowList.includes(req.socket.remoteAddress),
+          )
+        );
+      },
+      validate: {
+        trustProxy: false,
       },
       store: new RateLimitStoreFactory(this.#config).create(),
     });

packages/backend-defaults/src/entrypoints/rootHttpRouter/http/RateLimitStoreFactory.test.ts

@@ -13,49 +13,19 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { mockServices } from '@backstage/backend-test-utils';
+import { mockServices, TestCaches } from '@backstage/backend-test-utils';
 import { RateLimitStoreFactory } from './RateLimitStoreFactory';
 import { RedisStore } from 'rate-limit-redis';
-import KeyvRedis from '@keyv/redis';
-
-jest.mock('@keyv/redis');
-(KeyvRedis as jest.Mocked<any>).mockImplementation(() => {
-  return {
-    redis: {
-      call: jest.fn().mockResolvedValue('OK'),
-    },
-  };
-});
 
 describe('CacheRateLimitStoreFactory', () => {
-  it('should return redis store with auto configuration', () => {
-    const config = mockServices.rootConfig({
-      data: {
-        backend: {
-          cache: {
-            store: 'redis',
-          },
-          rateLimit: {
-            store: undefined,
-          },
-        },
-      },
-    });
-    const factory = new RateLimitStoreFactory(config);
-    const store = factory.create();
-    expect(store).toBeInstanceOf(RedisStore);
-  });
+  const caches = TestCaches.create();
+
+  afterEach(jest.clearAllMocks);
 
   it('should return undefined store with auto configuration if redis is not available', () => {
     const config = mockServices.rootConfig({
       data: {
         backend: {
-          cache: {
-            store: 'memory',
-          },
-          database: {
-            client: 'sqlite3',
-          },
           rateLimit: {
             store: undefined,
           },
@@ -67,12 +37,20 @@ describe('CacheRateLimitStoreFactory', () => {
     expect(store).toBeUndefined();
   });
 
-  it('should return redis store if configured explicitly', () => {
+  it('should return redis store if configured explicitly', async () => {
+    if (!caches.supports('REDIS_7')) {
+      return;
+    }
+    const conf = await caches.init('REDIS_7');
+
     const config = mockServices.rootConfig({
       data: {
         backend: {
           rateLimit: {
-            store: 'redis',
+            store: {
+              client: 'redis',
+              connection: conf.connection,
+            },
           },
         },
       },

packages/backend-defaults/src/entrypoints/rootHttpRouter/http/RateLimitStoreFactory.ts

@@ -21,46 +21,28 @@ import { RedisStore } from 'rate-limit-redis';
 /**
  * Creates a store for `express-rate-limit` based on the configuration.
  *
- * @public
+ * @internal
  */
 export class RateLimitStoreFactory {
   constructor(private readonly config: Config) {}
 
   create(): Store | undefined {
-    const storeType = this.config.getOptionalString('backend.rateLimit.store');
-    if (!storeType) {
-      return this.auto();
+    const store = this.config.getOptionalConfig('backend.rateLimit.store');
+    if (!store) {
+      return undefined;
     }
-    switch (storeType) {
+    const client = store.getString('client');
+    switch (client) {
       case 'redis':
-        return this.redis();
+        return this.redis(store);
       default:
-        throw new Error(
-          `Invalid 'backend.rateLimit.store' provided: ${storeType}`,
-        );
+        return undefined;
     }
   }
 
-  private auto(): Store | undefined {
-    const cacheStore =
-      this.config.getOptionalString('backend.cache.store') || 'memory';
-    // Use redis as primary if available
-    if (cacheStore === 'redis') {
-      return this.redis();
-    }
-
-    // Fallback to undefined (memory)
-    return undefined;
-  }
-
-  redis(): Store {
-    const connectionString =
-      this.config.getOptionalString('backend.cache.connection') || '';
-    const useRedisSets =
-      this.config.getOptionalBoolean('backend.cache.useRedisSets') ?? true;
-    const keyv = new KeyvRedis(connectionString, {
-      useRedisSets,
-    });
+  redis(storeConfig: Config): Store {
+    const connectionString = storeConfig.getString('connection');
+    const keyv = new KeyvRedis(connectionString);
     return new RedisStore({
       // Keyv uses ioredis under the hood
       sendCommand: (...args: string[]) => keyv.redis.call(...args),

packages/backend-defaults/src/entrypoints/rootHttpRouter/http/index.ts

@@ -28,4 +28,3 @@ export type {
   HttpServerCertificateOptions,
   HttpServerOptions,
 } from './types';
-export { RateLimitStoreFactory } from './RateLimitStoreFactory';