fix(auth): clear error for stale-key decryption failures (#6148) (#6226)

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 `<mangled>` 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)
 

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

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 });
+}

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<T extends ConnectionFields | null | unde
   if (!row) return row;
   if (!isEncryptionEnabled()) return row;
 
+  const apiKey = decrypt(row.apiKey);
+  const accessToken = decrypt(row.accessToken);
+  const refreshToken = decrypt(row.refreshToken);
+  const idToken = decrypt(row.idToken);
+
+  // #6148 — a stored credential that is still encrypted (`enc:v1:…`) but
+  // decrypts to null means the STORAGE_ENCRYPTION_KEY changed or was unset.
+  // Flag it so callers surface a clear error instead of coercing the null to
+  // "" and firing an empty-Bearer request that upstream rejects as 401.
+  const credentialDecryptFailed =
+    (looksEncrypted(row.apiKey) && apiKey === null) ||
+    (looksEncrypted(row.accessToken) && accessToken === null) ||
+    (looksEncrypted(row.refreshToken) && refreshToken === null) ||
+    (looksEncrypted(row.idToken) && idToken === null);
+
   return {
     ...row,
-    apiKey: decrypt(row.apiKey),
-    accessToken: decrypt(row.accessToken),
-    refreshToken: decrypt(row.refreshToken),
-    idToken: decrypt(row.idToken),
+    apiKey,
+    accessToken,
+    refreshToken,
+    idToken,
+    ...(credentialDecryptFailed ? { credentialDecryptFailed: true } : {}),
   };
 }
 

tests/unit/decrypt-stale-key-hint-6148.test.ts

@@ -0,0 +1,91 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import path from "node:path";
+import { pathToFileURL } from "node:url";
+
+// #6148 — A decryption failure caused by a stale/changed STORAGE_ENCRYPTION_KEY
+// must surface as a clear, specific error (HTTP 424, type
+// "storage_encryption_stale") instead of the misleading "Auth failed: 401" that
+// resulted from coercing the null credential to "" and sending an empty Bearer
+// token upstream.
+
+const ORIGINAL_STORAGE_KEY = process.env.STORAGE_ENCRYPTION_KEY;
+
+// Cache-busted fresh import so the encryption module re-derives its key from the
+// current STORAGE_ENCRYPTION_KEY (module-level key cache would otherwise persist).
+async function importFresh(modulePath: string) {
+  const url = pathToFileURL(path.resolve(modulePath)).href;
+  return import(`${url}?test=${Date.now()}-${Math.random().toString(16).slice(2)}`);
+}
+
+test.after(() => {
+  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)");
+});