From 7360ca424266b23628b378cb4cfceca3fc976d4e Mon Sep 17 00:00:00 2001 From: Reza Rezaei <77894240+MeRezaRezaei@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:56:45 +0200 Subject: [PATCH] feat(redis): add configurable key namespace prefix (#11042) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated on the combined board over tip 80d931ae: quota-redis-store (incl. the KEY_PREFIX derivation test), local-redis-status and rate-limiter-redis-optional green, typecheck:core clean. One pre-merge fix pushed to the branch: docs/reference/ENVIRONMENT.md gained the REDIS_KEY_PREFIX row (env-doc-sync gate requires every .env.example var documented). Board note: the redis tests leave an ioredis retry handle open and hang the runner exit locally — assertions all pass; pre-existing pattern, not from this PR. Thank you @MeRezaRezaei! --- .env.example | 4 ++ docs/ops/REDIS_PRODUCTION_CONFIG.md | 33 +++++++++++-- docs/reference/ENVIRONMENT.md | 1 + src/app/api/local/redis/status/route.ts | 48 +++++++++++++++---- src/lib/quota/redisQuotaStore.ts | 2 +- src/shared/utils/rateLimiter.ts | 5 ++ tests/unit/local-redis-status.test.ts | 38 +++++++++++++++ tests/unit/quota-redis-store.test.ts | 16 +++++++ .../unit/rate-limiter-redis-optional.test.ts | 11 +++++ 9 files changed, 145 insertions(+), 13 deletions(-) create mode 100644 tests/unit/local-redis-status.test.ts diff --git a/.env.example b/.env.example index 9616acd318..1ec9d2a9cb 100644 --- a/.env.example +++ b/.env.example @@ -87,6 +87,10 @@ DISABLE_SQLITE_AUTO_BACKUP=false # Used by: src/shared/utils/rateLimiter.ts # Example: redis://localhost:6379 (or redis://redis:6379 in Docker) # REDIS_URL=redis://localhost:6379 +# Namespace prefix for ALL OmniRoute Redis keys (rate limiter + auth cache + +# quota store). Prevents key collisions when OmniRoute shares a Redis instance +# with other apps (e.g. on 127.0.0.1:6379). Default when unset: omniroute: +# REDIS_KEY_PREFIX=omniroute: # Host interface docker-compose publishes the Redis sidecar on. # Default: 127.0.0.1 (loopback only). The compose Redis runs WITHOUT # `requirepass`, and app containers reach it over the compose network diff --git a/docs/ops/REDIS_PRODUCTION_CONFIG.md b/docs/ops/REDIS_PRODUCTION_CONFIG.md index e8749c7bbc..ada9ad4456 100644 --- a/docs/ops/REDIS_PRODUCTION_CONFIG.md +++ b/docs/ops/REDIS_PRODUCTION_CONFIG.md @@ -14,9 +14,12 @@ workloads: | Workload | Driver | Client Factory | Key Pattern | |---|---|---|---| -| Rate limiting | `rateLimiter.ts` | `getRedisClient()` — lazy `ioredis` singleton | Lua‑atomic rate limit windows | -| Auth cache | `apiKeys.ts` | Reuses `rateLimiter`'s client | `auth:api_key:` with TTL | -| Quota store | `redisQuotaStore.ts` | Separate `getRedisClient(url)` singleton | Configurable per-instance | +| Rate limiting | `rateLimiter.ts` | `getRedisClient()` — lazy `ioredis` singleton | `rl:*` Lua‑atomic rate limit windows | +| Auth cache | `apiKeys.ts` | Reuses `rateLimiter`'s client | `auth:api_key:` with TTL | +| Quota store | `redisQuotaStore.ts` | Separate `getRedisClient(url)` singleton | `quota:*` configurable per-instance | + +All three workloads share one namespace prefix so OmniRoute can co-exist with other apps on a +single Redis instance (e.g. `127.0.0.1:6379`). See [Key Namespacing](#key-namespacing). --- @@ -25,6 +28,7 @@ workloads: | Setting | Value | Where | |---|---|---| | `REDIS_URL` env var | `redis://redis:6379` (compose), optional | `rateLimiter.ts:5`, `.env.example` | +| `REDIS_KEY_PREFIX` env var | `omniroute:` (default) | `rateLimiter.ts`, `redisQuotaStore.ts`, `.env.example` | | `QUOTA_STORE_REDIS_URL` env var | separate, can differ from `REDIS_URL` | `quota/storeFactory.ts` | | `QUOTA_STORE_DRIVER` | `"sqlite"` (default), `"redis"` optional | `quota/storeFactory.ts` | | ioredis `maxRetriesPerRequest` | `3` | `rateLimiter.ts` client creation | @@ -36,6 +40,29 @@ workloads: --- +## Key Namespacing + +OmniRoute shares a Redis instance with whatever else runs on the host. Without a namespace, +keys like `auth:api_key:` or `rl:*` could collide with keys from other applications +using the same Redis (this instance runs Redis on `127.0.0.1:6379` alongside other services). + +Set `REDIS_KEY_PREFIX` to a non-empty string to prefix **every** OmniRoute key: + +```bash +# .env — all OmniRoute keys become omniroute:rl:*, omniroute:auth:*, omniroute:quota:* +REDIS_KEY_PREFIX=omniroute: +``` + +- **Default:** `omniroute:` (applied when `REDIS_KEY_PREFIX` is unset or blank). +- **Applied to:** rate limiter + auth cache (shared `ioredis` client via `keyPrefix`) and the + quota store (`KEY_PREFIX = "${REDIS_KEY_PREFIX}quota"`). +- **Changing the prefix** when keys already exist in Redis orphans the old keys (they expire + via TTL / LRU). Safe to change; no migration needed. +- **ioredis `keyPrefix`** automatically prepends the prefix on writes **and** strips it on reads, + so application code never sees the prefix. + +--- + ## Recommended Production Tuning ### 1. Connection Pool / Client Options (ioredis `Redis` constructor) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index b3c12467f8..ae06e62090 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -1343,6 +1343,7 @@ Provider quota endpoints, network tunnels (Tailscale, Ngrok, MITM debug proxy), | `OMNIROUTE_REDIS_BIND_HOST` | `127.0.0.1` | `bin/cli/commands/redis.mjs` | Host interface the 1-click Redis launcher publishes on. The launcher starts Redis WITHOUT a password, so binding `0.0.0.0` hands every host on your LAN an unauthenticated Redis — only widen this if you also set a password on the instance yourself. | | `REDIS_BIND_HOST` | `127.0.0.1` | `docker-compose.yml` | Host interface docker-compose publishes the Redis sidecar on (#9286). The compose Redis runs without `requirepass`; app containers reach it over the compose network (`redis:6379`) — the published port exists only for host-side tooling. `0.0.0.0` exposes an unauthenticated Redis to the whole LAN. | | `REDIS_PORT` | `6379` | `docker-compose.yml` | Host port for the compose Redis sidecar. | +| `REDIS_KEY_PREFIX` | `omniroute:` | `src/shared/utils/rateLimiter.ts` | Namespace prefix applied to every OmniRoute Redis key (rate limiter, auth cache, quota store). Prevents key collisions when the Redis instance is shared with other apps (#11042). | | `OMNIROUTE_INTERNAL_SERVICE_TOKEN` | _(unset — mechanism disabled)_ | `src/lib/api/internalServiceAuth.ts` | Shared secret for identity-preserving internal REST hops (#9260): OmniRoute components calling other local OmniRoute routes send it as `x-omniroute-internal-service-token` so the original caller identity is preserved. Compared with `timingSafeEqual`. | | `OMNIROUTE_INTERNAL_SERVICE_TOKEN_FILE` | _(unset)_ | `src/lib/api/internalServiceAuth.ts` | Secret-file variant of the internal service token: path to a file whose trimmed content is the token. Only consulted when the inline var is unset. | | `OPENROUTER_PROVIDER_STATS_ENABLED` | `true` | `src/lib/catalog/openrouterProviderStats.ts` | Enrich the dashboard providers list with OpenRouter weekly ranking stats (#9324). On by default; set `false` to skip the background fetch entirely (non-blocking, never fatal). | diff --git a/src/app/api/local/redis/status/route.ts b/src/app/api/local/redis/status/route.ts index 7f7cb47bc5..0ab37e76fd 100644 --- a/src/app/api/local/redis/status/route.ts +++ b/src/app/api/local/redis/status/route.ts @@ -63,21 +63,51 @@ async function pingRedis(port: string): Promise { }); } +function parseRedisUrl(url?: string): { host: string; port: number } | null { + if (!url) return null; + try { + const u = new URL(url); + return { host: u.hostname || "127.0.0.1", port: Number(u.port) || 6379 }; + } catch { + return null; + } +} + export async function GET() { const guard = isLocalRequestAllowed(); if (!guard.allowed) { - return NextResponse.json({ error: guard.reason }, { status: 403 }); + const reason = (guard as { reason?: string }).reason ?? "Forbidden: not a loopback request"; + return NextResponse.json({ error: reason }, { status: 403 }); } + // Docker/Podman container state (the 1-click launcher path). const runtime = await detectRuntime(); - if (!runtime) { - return NextResponse.json( - { exists: false, running: false, reachable: false, error: "No container runtime (podman or docker) found on PATH" }, - { status: 503 } - ); + let container = { exists: false, running: false, reachable: false }; + if (runtime) { + const { exists, running } = await containerState(runtime); + const reachable = running ? await pingRedis(HOST_PORT) : false; + container = { exists, running, reachable }; } - const { exists, running } = await containerState(runtime); - const reachable = running ? await pingRedis(HOST_PORT) : false; - return NextResponse.json({ runtime, name: CONTAINER_NAME, port: HOST_PORT, exists, running, reachable }); + // Native Redis via REDIS_URL (the production path this instance uses). OmniRoute + // is "connected" whenever REDIS_URL is configured AND the server answers — even + // when no Docker container is present. + const redisUrl = process.env.REDIS_URL?.trim() || ""; + const parsed = parseRedisUrl(redisUrl); + const redisUrlReachable = parsed ? await pingRedis(String(parsed.port)) : false; + + const running = container.running || redisUrlReachable; + const reachable = container.reachable || redisUrlReachable; + const exists = container.exists || redisUrlReachable; + + return NextResponse.json({ + runtime: runtime ?? null, + name: CONTAINER_NAME, + port: HOST_PORT, + exists, + running, + reachable, + redisUrlConfigured: Boolean(redisUrl), + redisUrlReachable, + }); } \ No newline at end of file diff --git a/src/lib/quota/redisQuotaStore.ts b/src/lib/quota/redisQuotaStore.ts index 7c0e99ceb7..d9d17309ed 100644 --- a/src/lib/quota/redisQuotaStore.ts +++ b/src/lib/quota/redisQuotaStore.ts @@ -72,7 +72,7 @@ export function resetRedisClient(): void { // Key helpers // --------------------------------------------------------------------------- -const KEY_PREFIX = "omniroute:quota"; +const KEY_PREFIX = `${process.env.REDIS_KEY_PREFIX?.trim() || "omniroute:"}quota`; function bucketKey(apiKeyId: string, dimensionKey: string, bucketIndex: number): string { return `${KEY_PREFIX}:${apiKeyId}:${dimensionKey}:${bucketIndex}`; diff --git a/src/shared/utils/rateLimiter.ts b/src/shared/utils/rateLimiter.ts index bb268ba5f0..7b36a21736 100644 --- a/src/shared/utils/rateLimiter.ts +++ b/src/shared/utils/rateLimiter.ts @@ -3,6 +3,10 @@ import type Redis from "ioredis"; // Redis is optional. When REDIS_URL is unset, use a process-local fallback // instead of probing localhost on every API request. const REDIS_URL = process.env.REDIS_URL?.trim() || ""; + +// Namespace prefix for all OmniRoute Redis keys. Prevents key collisions when +// OmniRoute shares a Redis instance with other apps (e.g. on 127.0.0.1:6379). +const REDIS_KEY_PREFIX = process.env.REDIS_KEY_PREFIX?.trim() || "omniroute:"; if (process.env.NODE_ENV === "production" && !REDIS_URL) { console.warn("[REDIS] REDIS_URL is not set in production. Using in-memory rate limiting."); } @@ -72,6 +76,7 @@ export function getRedisClient(): Promise { const client = new RedisCtor(REDIS_URL, { maxRetriesPerRequest: 3, enableReadyCheck: false, + keyPrefix: REDIS_KEY_PREFIX, retryStrategy(times) { return Math.min(times * 50, 2000); // Exponential backoff }, diff --git a/tests/unit/local-redis-status.test.ts b/tests/unit/local-redis-status.test.ts new file mode 100644 index 0000000000..9569306642 --- /dev/null +++ b/tests/unit/local-redis-status.test.ts @@ -0,0 +1,38 @@ +/** + * tests/unit/local-redis-status.test.ts + * + * Coverage for src/app/api/local/redis/status/route.ts: + * - The status endpoint must report OmniRoute as "connected" whenever the + * native REDIS_URL is reachable — not only when a Docker/Podman container + * is present. This is the production path used by this instance + * (redis on 127.0.0.1:6379, no container). + * + * Verified at the source-contract level (the route imports Next.js + the route + * guard, which is heavy to import in the native runner and would make the test + * environment-dependent on a live container runtime). + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const STATUS_SRC = path.resolve(__dirname, "../../src/app/api/local/redis/status/route.ts"); +const src = fs.readFileSync(STATUS_SRC, "utf8"); + +test("redis status: reports running via REDIS_URL reachability, not only Docker", () => { + assert.ok(src.includes("parseRedisUrl"), "status route must parse REDIS_URL"); + assert.ok( + src.includes("redisUrlReachable"), + "status route must probe REDIS_URL reachability" + ); + assert.ok( + src.includes("redisUrlConfigured"), + "status route must report whether REDIS_URL is configured" + ); + assert.ok( + src.includes("const running = container.running || redisUrlReachable;"), + "status route must treat a reachable native REDIS_URL as a connected state" + ); +}); diff --git a/tests/unit/quota-redis-store.test.ts b/tests/unit/quota-redis-store.test.ts index 3b9d5ded4c..5a5ae936dc 100644 --- a/tests/unit/quota-redis-store.test.ts +++ b/tests/unit/quota-redis-store.test.ts @@ -20,6 +20,9 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-redis-store-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -282,3 +285,16 @@ test("redisQuotaStore: resetRedisQuotaStore resets the store singleton", async ( // After reset, a new instance is created assert.ok(store2, "Should create new instance after reset"); }); + +test("redis namespace prefix: quota store KEY_PREFIX derives from REDIS_KEY_PREFIX", () => { + const quotaSrc = fs.readFileSync( + path.resolve(__dirname, "../../src/lib/quota/redisQuotaStore.ts"), + "utf8" + ); + assert.ok( + quotaSrc.includes('process.env.REDIS_KEY_PREFIX?.trim() || "omniroute:"') && + quotaSrc.includes("const KEY_PREFIX = `") && + quotaSrc.includes("quota`"), + "redisQuotaStore KEY_PREFIX must derive from REDIS_KEY_PREFIX env, defaulting to omniroute:quota" + ); +}); diff --git a/tests/unit/rate-limiter-redis-optional.test.ts b/tests/unit/rate-limiter-redis-optional.test.ts index edbce9b53d..8f23f3eafe 100644 --- a/tests/unit/rate-limiter-redis-optional.test.ts +++ b/tests/unit/rate-limiter-redis-optional.test.ts @@ -41,3 +41,14 @@ test("#2357 checkRateLimit falls back when REDIS_URL is unset", () => { "checkRateLimit must route to the in-memory fallback when Redis is disabled" ); }); + +test("redis namespace prefix: rate limiter + auth cache keys are namespaced", () => { + assert.ok( + src.includes('process.env.REDIS_KEY_PREFIX?.trim() || "omniroute:"'), + "rateLimiter must read REDIS_KEY_PREFIX with an omniroute: default" + ); + assert.ok( + src.includes("keyPrefix: REDIS_KEY_PREFIX"), + "rateLimiter must pass the prefix as the ioredis keyPrefix so all keys are namespaced" + ); +});