diff --git a/CHANGELOG.md b/CHANGELOG.md index 0641e622b6..31ad6f7ad7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### 🔧 Bug Fixes - **fix(api):** relay worker now binds the SSRF guard to a stable `const` name so minified standalone (Docker) builds resolve it ([#6149](https://github.com/diegosouzapw/OmniRoute/issues/6149)) — the Vercel/Deno relay generators embedded the shared `resolveRelayTarget` guard as a bare `${fn.toString()}` declaration while the worker body called the hardcoded literal name; SWC minification mangled the source function's name, so the deployed worker defined `` but still called `resolveRelayTarget` → `ReferenceError`. Both templates now emit `const resolveRelayTarget = ${fn.toString()};` (the const name is a template literal, immune to minification). Regression guard: `tests/unit/relay-minified-fn-6149.test.ts` (4). (thanks @SeaXen) +- **fix(auth):** a stale/changed `STORAGE_ENCRYPTION_KEY` now surfaces as a clear **424 `storage_encryption_stale`** ("re-enter the API key") instead of a misleading "Auth failed: 401" — the connection's ciphertext failed to decrypt and was coerced to an empty Bearer, hiding the real cause ([#6148](https://github.com/diegosouzapw/OmniRoute/issues/6148)). Regression guard: `tests/unit/decrypt-stale-key-hint-6148.test.ts`. (thanks @chirag127) - **fix(backend):** memory injection now keeps the injected system message **first** for providers that require it (via a `PROVIDERS_SYSTEM_MUST_BE_FIRST` capability), instead of the cache-safe mid-array splice that made strict providers reject the request with a 400 ([#6135](https://github.com/diegosouzapw/OmniRoute/issues/6135)). Regression guard: `tests/unit/memory-system-first-6135.test.ts`. - **fix(services):** 9Router embed panel no longer 404s (optional catch-all route) and the supervisor probes the port before spawning to avoid raw EADDRINUSE ([#6205](https://github.com/diegosouzapw/OmniRoute/issues/6205)). Regression guards: `tests/unit/ninerouter-embed-port-6205.test.ts`, `tests/unit/services/ServiceSupervisor.test.ts`. (thanks @jonlwheat2-gif) diff --git a/src/app/api/providers/[id]/models/route.ts b/src/app/api/providers/[id]/models/route.ts index 6d1448d803..d16dcc26a7 100755 --- a/src/app/api/providers/[id]/models/route.ts +++ b/src/app/api/providers/[id]/models/route.ts @@ -92,6 +92,7 @@ import { normalizeSapModelsResponse, } from "./discovery/normalizers"; import { isNamedOpenAIStyleProvider } from "./discovery/providerSets"; +import { buildStaleEncryptionKeyResponse } from "./staleEncryptionGuard"; import { type ProviderModelsConfigEntry, PROVIDER_MODELS_CONFIG, @@ -193,6 +194,13 @@ export async function GET( return NextResponse.json({ error: "Connection not found" }, { status: 404 }); } + // #6148 — short-circuit when a stored credential is encrypted but no longer + // decrypts (STORAGE_ENCRYPTION_KEY changed/unset). Otherwise the null key is + // coerced to "", an empty-Bearer probe is sent, and the operator sees a + // misleading "Auth failed: 401" instead of the real cause. + const staleEncryptionResponse = buildStaleEncryptionKeyResponse(connection); + if (staleEncryptionResponse) return staleEncryptionResponse; + const provider = typeof connection.provider === "string" && connection.provider.trim().length > 0 ? connection.provider diff --git a/src/app/api/providers/[id]/models/staleEncryptionGuard.ts b/src/app/api/providers/[id]/models/staleEncryptionGuard.ts new file mode 100644 index 0000000000..ea05fd023f --- /dev/null +++ b/src/app/api/providers/[id]/models/staleEncryptionGuard.ts @@ -0,0 +1,30 @@ +import { NextResponse } from "next/server"; +import { buildErrorBody } from "@omniroute/open-sse/utils/error"; + +/** + * #6148 — Stale STORAGE_ENCRYPTION_KEY guard for model-discovery. + * + * `decryptConnectionFields` (src/lib/db/encryption.ts) flags a connection with + * `credentialDecryptFailed: true` when a stored credential is still encrypted + * (`enc:v1:…`) but no longer decrypts — the signature of a changed or unset + * STORAGE_ENCRYPTION_KEY. Without this guard the null credential is coerced to + * an empty string, an empty-Bearer request is sent upstream, and the operator + * sees a misleading "Auth failed: 401" that hides the real cause. + * + * Returns a 424 (Failed Dependency) response with a clear, sanitized message + * when the connection carries that flag; otherwise null (proceed normally). + */ +const STALE_ENCRYPTION_MESSAGE = + "Stored API key cannot be decrypted (STORAGE_ENCRYPTION_KEY changed or unset). Re-enter the API key."; + +export function buildStaleEncryptionKeyResponse( + connection: { credentialDecryptFailed?: unknown } | null | undefined +): NextResponse | null { + if (!connection || connection.credentialDecryptFailed !== true) return null; + + // buildErrorBody sanitizes the message (Rule #12); override the type so the + // client can key off the specific stale-encryption cause. + const body = buildErrorBody(424, STALE_ENCRYPTION_MESSAGE); + body.error.type = "storage_encryption_stale"; + return NextResponse.json(body, { status: 424 }); +} diff --git a/src/lib/db/encryption.ts b/src/lib/db/encryption.ts index 1559c14bb5..af81a40dde 100644 --- a/src/lib/db/encryption.ts +++ b/src/lib/db/encryption.ts @@ -102,6 +102,16 @@ export function isEncryptionEnabled(): boolean { return !!process.env.STORAGE_ENCRYPTION_KEY; } +/** + * True when `value` is a stored ciphertext (carries the `enc:v1:` prefix). + * Lets callers tell "credential present but undecryptable" (stale/changed + * STORAGE_ENCRYPTION_KEY) apart from "credential genuinely empty" — decrypt() + * collapses both to null otherwise. See #6148. + */ +export function looksEncrypted(value: unknown): boolean { + return typeof value === "string" && value.startsWith(PREFIX); +} + /** * Encrypt a plaintext string using the STATIC salt key. * If encryption is not configured, returns plaintext unchanged. @@ -232,12 +242,28 @@ export function decryptConnectionFields { + if (ORIGINAL_STORAGE_KEY === undefined) { + delete process.env.STORAGE_ENCRYPTION_KEY; + } else { + process.env.STORAGE_ENCRYPTION_KEY = ORIGINAL_STORAGE_KEY; + } +}); + +test("decryptConnectionFields flags a credential that no longer decrypts (#6148)", async () => { + // 1. Encrypt an apiKey under key A. + process.env.STORAGE_ENCRYPTION_KEY = "stale-key-6148-A"; + const encA = await importFresh("src/lib/db/encryption.ts"); + const ciphertext = encA.encrypt("sk-real-secret-key"); + assert.match(ciphertext, /^enc:v1:/, "expected a real enc:v1 ciphertext"); + + // 2. Read it back under a DIFFERENT key B (simulating a changed key). + process.env.STORAGE_ENCRYPTION_KEY = "stale-key-6148-B"; + const encB = await importFresh("src/lib/db/encryption.ts"); + + const decrypted = encB.decryptConnectionFields({ + provider: "openai", + apiKey: ciphertext, + }); + + // The credential fails to decrypt (null) but the guard flag distinguishes this + // from a genuinely empty credential. + assert.equal(decrypted.apiKey, null, "stale key must decrypt to null"); + assert.equal( + decrypted.credentialDecryptFailed, + true, + "undecryptable ciphertext must set credentialDecryptFailed" + ); + assert.equal(encB.looksEncrypted(ciphertext), true); +}); + +test("a genuinely empty credential is NOT flagged as decrypt failure (#6148)", async () => { + process.env.STORAGE_ENCRYPTION_KEY = "stale-key-6148-empty"; + const enc = await importFresh("src/lib/db/encryption.ts"); + + const decrypted = enc.decryptConnectionFields({ provider: "openai", apiKey: null }); + assert.notEqual(decrypted.credentialDecryptFailed, true, "empty credential must not flag"); +}); + +test("models route guard returns HTTP 424 storage_encryption_stale (#6148)", async () => { + const guard = await importFresh( + "src/app/api/providers/[id]/models/staleEncryptionGuard.ts" + ); + + // Connection flagged by decryptConnectionFields (stale key). + const staleResponse = guard.buildStaleEncryptionKeyResponse({ + provider: "openai", + apiKey: null, + credentialDecryptFailed: true, + }); + + assert.ok(staleResponse, "guard must return a response for a stale connection"); + assert.equal(staleResponse.status, 424, "must be HTTP 424, not an upstream 401"); + + const body = await staleResponse.json(); + assert.equal(body.error.type, "storage_encryption_stale"); + assert.match(body.error.message, /decrypt/i); + // Rule #12 — no stack trace leakage in the error body. + assert.equal(body.error.message.includes("at /"), false); + + // A healthy connection must NOT be short-circuited. + const okResponse = guard.buildStaleEncryptionKeyResponse({ + provider: "openai", + apiKey: "sk-real-secret-key", + }); + assert.equal(okResponse, null, "healthy connection must proceed (null)"); +});