feat: storePerLimiter validation check (#459)

* add new storePerLimiter validation check
Checks for cases like #457 where a single store instance is shared across multiple rate limiter instances.

* automatic formatting change

* npm audit fix dep bump

* rename validation to singleCount

docs/reference/changelog.mdx

@@ -9,6 +9,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [7.3.0](https://github.com/express-rate-limit/express-rate-limit/releases/tag/v7.2.1)
+
+### Added
+
+- New `unsharedStore` validation check that identifies cases where a single
+  store instance is shared across multiple limiters
+
 ## [7.2.0](https://github.com/express-rate-limit/express-rate-limit/releases/tag/v7.2.0)
 
 ### Added

docs/reference/configuration.mdx

@@ -345,6 +345,7 @@ Supported validations are:
 - [trustProxy](/reference/error-codes#err-erl-permissive-trust-proxy)
 - [xForwardedForHeader](/reference/error-codes#err-erl-unexpected-x-forwarded-for)
 - [positiveHits](/reference/error-codes#err-erl-invalid-hits)
+- [unsharedStore](/reference/error-codes#err-erl-store-reuse)
 - [singleCount](/reference/error-codes#err-erl-double-count)
 - [limit](/reference/error-codes#wrn-erl-max-zero) (Note: the `limit` option was
   renamed to `max`, but the validation name did not change.)

docs/reference/error-codes.mdx

@@ -109,6 +109,19 @@ error.
 
 Set `validate: {positiveHits: false}` in the options to disable the check.
 
+### `ERR_ERL_STORE_REUSE`
+
+> Added in `7.3.0`.
+
+This indicates that the single [Store](./stores) instance was used in more than
+one rate limiter. This can lead to problems such as initialization logic runing
+multiple times, and inconsistent reset times.
+
+Instead, create a new store instance for each rate limiter. (Generally with a
+unique `prefix` value for each one.)
+
+Set `validate: {unsharedStore: false}` in the options to disable the check.
+
 ### `ERR_ERL_DOUBLE_COUNT`
 
 > Added in `6.9.0`.

source/lib.ts

@@ -307,7 +307,10 @@ const rateLimit = (
 	const config = parseOptions(passedOptions ?? {})
 	const options = getOptionsFromConfig(config)
 
+	// The limiter shouldn't be created in response to a request
 	config.validations.creationStack()
+	// The store instance shouldn't be shared across multiple limiters
+	config.validations.unsharedStore(config.store)
 
 	// Call the `init` method on the store, if it exists
 	if (typeof config.store.init === 'function') config.store.init(options)

source/validations.ts

@@ -37,6 +37,11 @@ class ValidationError extends Error {
  */
 class ChangeWarning extends ValidationError {}
 
+/**
+ * List of store instances that have been used with any express-rate-limit instance
+ */
+const usedStores = new Set<Store>()
+
 /**
  * Maps the key used in a store for a certain request, and ensures that the
  * same key isn't used more than once per request.
@@ -142,6 +147,23 @@ const validations = {
 		}
 	},
 
+	/**
+	 * Ensures a single store instance is not used with multiple express-rate-limit instances
+	 */
+	unsharedStore(store: Store) {
+		if (usedStores.has(store)) {
+			const maybeUniquePrefix = store?.localKeys
+				? ''
+				: ' (with a unique prefix)'
+			throw new ValidationError(
+				'ERR_ERL_STORE_REUSE',
+				`A Store instance must not be shared across multiple rate limiters. Create a new instance of ${store.constructor.name}${maybeUniquePrefix} for each limiter instead.`,
+			)
+		}
+
+		usedStores.add(store)
+	},
+
 	/**
 	 * Ensures a given key is incremented only once per request.
 	 *

test/library/middleware-test.ts

@@ -380,9 +380,7 @@ describe('middleware test', () => {
 			.expect((response) => {
 				if ('retry-after' in response.headers) {
 					throw new Error(
-						`Expected no retry-after header, got ${
-							response.headers['retry-after'] as string
-						}`,
+						`Expected no retry-after header, got ${response.headers['retry-after']}`,
 					)
 				}
 			})

test/library/validation-test.ts

@@ -119,6 +119,30 @@ describe('validations tests', () => {
 		})
 	})
 
+	describe('unsharedStore', () => {
+		it('should log an error if a store instance is used in two limiters', () => {
+			const store = { localKeys: true }
+
+			validations.unsharedStore(store as Store)
+			expect(console.error).not.toBeCalled()
+			validations.unsharedStore(store as Store)
+			expect(console.error).toHaveBeenCalledWith(
+				expect.objectContaining({
+					code: 'ERR_ERL_STORE_REUSE',
+				}),
+			)
+		})
+
+		it('should not log an error if multiple store instances are used', () => {
+			const store1 = { localKeys: true }
+			const store2 = { localKeys: true }
+
+			validations.unsharedStore(store1 as Store)
+			validations.unsharedStore(store2 as Store)
+			expect(console.error).not.toBeCalled()
+		})
+	})
+
 	describe('singleCount', () => {
 		class TestExternalStore {
 			prefix?: string