fix(security): address P1/P2 findings from release review

Five issues raised in the v3.8.0 release review, all release-blocking:

P1 — open-sse/services/tokenRefresh.ts
Read Windsurf Firebase API key from WINDSURF_CONFIG.firebaseApiKey
(resolvePublicCred wrapper) instead of process.env directly. Without
this, the literal removal from .env.example silently broke browser-flow
Windsurf/Devin token refresh.

P1 — open-sse/translator/request/openai-to-kiro.ts
Mark synthetic "(empty)" turns injected for assistant-first chats as
non-enumerable __synthetic and skip them when deriving conversationId
via uuidv5. Prevents unrelated chats from colliding on the same upstream
Kiro/AWS Builder ID context.

P2 — open-sse/utils/publicCreds.ts
Harden decodePublicCred against raw credential overrides outside
RAW_VALUE_PATTERN: strict-base64 alphabet check + printable-plain check
on the decoded result. Buffer.from(v, "base64") is lenient and was
silently mangling unrecognized raw values.

P2 — src/sse/services/auth.ts
Gate the x-api-key fallback on the anthropic-version header. Without
this scoping, local-mode requests with placeholder x-api-key from
non-Anthropic clients were rejected as Invalid API key even with
REQUIRE_API_KEY=false.

P2 — src/app/api/providers/[id]/test/route.ts
Move Qoder OAuth+PAT disambiguation BEFORE the CLI-runtime early-return
that was making the new message branch unreachable for the target
scenario from #2247.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -2,6 +2,14 @@
 
 ## [Unreleased]
 
+### Security
+
+- **fix(oauth/windsurf):** Windsurf Firebase token refresh now reads `WINDSURF_CONFIG.firebaseApiKey` instead of `process.env.WINDSURF_FIREBASE_API_KEY` directly. The literal was removed from `.env.example` in this release, so the previous direct read would have silently skipped refresh for browser-flow Windsurf/Devin sessions (forcing re-auth instead of renewing). Operators with a legacy `WINDSURF_FIREBASE_API_KEY` value in their `.env` keep working — the env override path is preserved by `resolvePublicCred()`. See [`docs/security/PUBLIC_CREDS.md`](docs/security/PUBLIC_CREDS.md).
+- **fix(kiro/translator):** assistant-first conversations no longer collide on a single `conversationId`. The synthetic "(empty)" user turn injected to satisfy Kiro's "first message must be user" rule is now marked non-enumerable `__synthetic` and excluded from the `uuidv5` conversationId derivation, so unrelated chats no longer share the same upstream AWS Builder ID context. Prevents leaking prior session state across unrelated chats.
+- **fix(utils/publicCreds):** `decodePublicCred()` no longer silently mangles raw credential overrides that don't match `RAW_VALUE_PATTERN`. The previous path always base64-decoded + XOR-unmasked (and `Buffer.from(v, "base64")` is lenient, accepting many non-base64 inputs without throwing). Now: strict-base64 alphabet check + printable-plain check on the decoded result; failing either, the original value is returned untouched.
+- **fix(auth/extractApiKey):** `x-api-key` fallback now only triggers when the request also carries an `anthropic-version` header. Without this scoping, non-Anthropic clients in local mode (placeholder `x-api-key`) would get `401 Invalid API key` from per-route gates even with `REQUIRE_API_KEY` off.
+- **fix(providers/qoder):** the OAuth+PAT disambiguation message now actually surfaces. The `getProviderRuntimeStatus()` early-return on `qoder + !apikey` was masking the new branch added in #2247.
+
 ### Fixed
 
 - **fix(executor/claude-code):** store tool-name round-trip metadata in non-enumerable `_toolNameMap` so it survives in-memory but is stripped by `JSON.stringify()` — prevents internal OmniRoute metadata from leaking to upstream providers. ([#2254](https://github.com/diegosouzapw/OmniRoute/pull/2254) — thanks @Rikonorus)

open-sse/services/tokenRefresh.ts

@@ -3,6 +3,7 @@ import { PROVIDERS, OAUTH_ENDPOINTS } from "../config/constants.ts";
 import { getGitHubCopilotRefreshHeaders } from "../config/providerHeaderProfiles.ts";
 import { pbkdf2Sync } from "node:crypto";
 import { runWithProxyContext } from "../utils/proxyFetch.ts";
+import { WINDSURF_CONFIG } from "@/lib/oauth/constants/oauth";
 
 // Token expiry buffer (refresh if expires within 5 minutes)
 export const TOKEN_EXPIRY_BUFFER_MS = 5 * 60 * 1000;
@@ -148,12 +149,14 @@ export async function refreshWindsurfToken(
   }
 
   // Firebase STS refresh for browser-flow tokens.
-  // Key is read from WINDSURF_FIREBASE_API_KEY env var (set in .env.example).
-  const firebaseApiKey = process.env.WINDSURF_FIREBASE_API_KEY || "";
+  // Resolves via WINDSURF_CONFIG.firebaseApiKey, which honors the
+  // WINDSURF_FIREBASE_API_KEY env override and falls back to the embedded
+  // public default in publicCreds.ts. See docs/security/PUBLIC_CREDS.md.
+  const firebaseApiKey = WINDSURF_CONFIG.firebaseApiKey || "";
   if (!firebaseApiKey) {
     log?.warn?.(
       "TOKEN_REFRESH",
-      "WINDSURF_FIREBASE_API_KEY not set — skipping Windsurf Firebase token refresh"
+      "Windsurf Firebase API key unavailable — skipping Firebase token refresh"
     );
     return null;
   }

open-sse/translator/request/openai-to-kiro.ts

@@ -473,13 +473,23 @@ function convertMessages(messages, tools, model) {
   // Ensure first message is user. Kiro API requires conversations to start
   // with a user message (fixes "Improperly formed request" for assistant-first).
   if (mergedHistory.length > 0 && mergedHistory[0].assistantResponseMessage) {
-    mergedHistory.unshift({
+    const syntheticUserTurn = {
       userInputMessage: {
         content: "(empty)",
         modelId: model,
         origin: "AI_EDITOR",
       },
+    };
+    // Mark as synthetic (non-enumerable so it doesn't leak to upstream JSON)
+    // so conversationId derivation can skip it — otherwise every
+    // assistant-first conversation collapses onto the same uuidv5(empty)
+    // namespace and leaks AWS Builder ID context across unrelated sessions.
+    Object.defineProperty(syntheticUserTurn, "__synthetic", {
+      value: true,
+      enumerable: false,
+      configurable: true,
     });
+    mergedHistory.unshift(syntheticUserTurn);
   }
 
   // Ensure assistant exists before toolResults. Kiro API validates that every
@@ -555,9 +565,15 @@ function convertMessages(messages, tools, model) {
   for (const item of mergedHistory) {
     const last = alternatingHistory[alternatingHistory.length - 1];
     if (item.userInputMessage && last?.userInputMessage) {
-      alternatingHistory.push({
+      const syntheticAssistantTurn = {
         assistantResponseMessage: { content: "(empty)" },
+      };
+      Object.defineProperty(syntheticAssistantTurn, "__synthetic", {
+        value: true,
+        enumerable: false,
+        configurable: true,
       });
+      alternatingHistory.push(syntheticAssistantTurn);
     }
     alternatingHistory.push(item);
   }
@@ -685,12 +701,15 @@ export function buildKiroPayload(model, body, stream, credentials) {
     },
   };
 
-  // Determistic session caching for Kiro
+  // Deterministic session caching for Kiro.
+  // Skip synthetic placeholder turns ("(empty)" injected for assistant-first
+  // conversations or alternating-role gaps) — otherwise unrelated assistant-
+  // first chats would all hash to the same uuidv5(empty) and reuse the same
+  // upstream Kiro/AWS conversation context, leaking prior state across
+  // sessions. See conversionMessages() above for the `__synthetic` marker.
   const NAMESPACE_KIRO = "34f7193f-561d-4050-bc84-9547d953d6bf";
-  const firstContent =
-    history.length > 0 && history[0].userInputMessage?.content
-      ? history[0].userInputMessage.content
-      : finalContent;
+  const firstRealUserTurn = history.find((h) => h?.userInputMessage?.content && !h.__synthetic);
+  const firstContent = firstRealUserTurn?.userInputMessage?.content || finalContent;
 
   // Use uuidv5 with the hash of the system prompt / first message to maintain AWS Builder ID context cache
   payload.conversationState.conversationId = uuidv5(

open-sse/utils/publicCreds.ts

@@ -50,22 +50,53 @@ function maskBytes(plain: string): number[] {
   return arr;
 }
 
+// A valid base64-encoded masked value uses only the base64 alphabet plus
+// optional padding. Anything outside that alphabet is definitely a raw
+// credential the user supplied (a token format we don't yet recognize in
+// RAW_VALUE_PATTERN) — never try to base64-decode it.
+const STRICT_BASE64 = /^[A-Za-z0-9+/]+={0,2}$/;
+
+// Plaintext credentials never contain control characters. If unmasking
+// produces non-printable bytes, the input wasn't actually masked and we
+// must return it untouched to avoid silently mangling raw overrides.
+function looksLikePrintablePlain(s: string): boolean {
+  if (!s) return false;
+  for (let i = 0; i < s.length; i++) {
+    const code = s.charCodeAt(i);
+    // Allow printable ASCII (0x20–0x7E). Everything outside that is suspect.
+    if (code < 0x20 || code > 0x7e) return false;
+  }
+  return true;
+}
+
 /**
  * Decode a public credential. Accepts either a raw literal (well-known prefix)
  * or a base64 string produced by `encodePublicCred()`. Returns the plaintext.
  * Empty / nullish input returns "".
+ *
+ * When the input doesn't match a known raw-credential prefix, we tentatively
+ * base64-decode + XOR-unmask, but only adopt the result if it looks like a
+ * printable plaintext. Otherwise we return the original value unchanged —
+ * `Buffer.from(value, "base64")` is lenient (it silently drops invalid chars
+ * instead of throwing) so a raw secret with a unknown format would otherwise
+ * be silently mangled. See docs/security/PUBLIC_CREDS.md.
  */
 export function decodePublicCred(value: string | null | undefined): string {
   if (!value || typeof value !== "string") return "";
 
   if (RAW_VALUE_PATTERN.test(value)) return value;
 
+  // Reject anything that isn't strict base64 — saves us from feeding raw
+  // ASCII overrides into the lenient Buffer.from(...,"base64") path.
+  if (!STRICT_BASE64.test(value)) return value;
+
   try {
     const buf = Buffer.from(value, "base64");
     if (buf.length === 0) return value;
     const arr: number[] = [];
     for (let i = 0; i < buf.length; i++) arr.push(buf[i]);
-    return unmaskBytes(arr);
+    const decoded = unmaskBytes(arr);
+    return looksLikePrintablePlain(decoded) ? decoded : value;
   } catch {
     return value;
   }

src/app/api/providers/[id]/test/route.ts

@@ -233,6 +233,26 @@ function hasQoderToken(connection: any): boolean {
 async function getProviderRuntimeStatus(connection: any) {
   const provider = typeof connection?.provider === "string" ? connection.provider : "";
   let toolId = CLI_RUNTIME_PROVIDER_MAP[provider];
+
+  // Issue #2247: detect Qoder in OAuth/CLI-flavored mode with a PAT pasted
+  // BEFORE the CLI-runtime early-return below, otherwise the disambiguation
+  // message never reaches the user (they keep seeing the generic "CLI not
+  // installed" + 401 cascade). For Qoder, this short-circuits the runtime
+  // check entirely with an actionable diagnosis.
+  const isQoderOauthWithToken =
+    provider === "qoder" && connection?.authType !== "apikey" && hasQoderToken(connection);
+  if (isQoderOauthWithToken) {
+    const message =
+      "Qoder OAuth/Local CLI mode is selected but a Personal Access Token is stored on this connection. Switch this connection to API Key auth to use the PAT directly.";
+    return {
+      installed: false,
+      runnable: false,
+      reason: "qoder_oauth_with_token",
+      diagnosis: makeDiagnosis("runtime_error", "local", message, "qoder_oauth_with_token"),
+      error: message,
+    };
+  }
+
   if (provider === "qoder" && connection?.authType !== "apikey") {
     toolId = null;
   }
@@ -244,17 +264,9 @@ async function getProviderRuntimeStatus(connection: any) {
       return runtime;
     }
 
-    // Issue #2247: when Qoder is in OAuth/CLI-flavored mode but the user has
-    // pasted a Personal Access Token, the bare "CLI not installed" message
-    // hides the real fix — switch the connection to API Key auth.
-    const isQoderOauthWithToken =
-      provider === "qoder" && connection?.authType !== "apikey" && hasQoderToken(connection);
-
     const runtimeMessage = runtime.installed
       ? `Local CLI runtime is installed but not runnable (${runtime.reason || "healthcheck_failed"})`
-      : isQoderOauthWithToken
-        ? "Qoder OAuth/Local CLI mode is selected but the Qoder CLI is not detected. If you have a Personal Access Token, switch this connection to API Key auth instead."
-        : "Local CLI runtime is not installed";
+      : "Local CLI runtime is not installed";
 
     return {
       ...runtime,

src/sse/services/auth.ts

@@ -1691,6 +1691,13 @@ export async function clearRecoveredProviderState(
  *
  * When both are present, `Authorization: Bearer` wins for back-compat
  * (issue #2225).
+ *
+ * The `x-api-key` fallback only triggers when the request also carries an
+ * `anthropic-version` header — the documented signal that the caller is
+ * speaking the Anthropic Messages API contract. Without this scoping,
+ * non-Anthropic SDKs that happen to set `x-api-key` (or local-mode tools
+ * with placeholder keys) would be treated as authenticated attempts and
+ * rejected by per-route gates that compare against OmniRoute keys.
  */
 export function extractApiKey(request: Request) {
   const authHeader = request.headers.get("Authorization") || request.headers.get("authorization");
@@ -1701,12 +1708,17 @@ export function extractApiKey(request: Request) {
     }
   }
   // Issue #2225: Anthropic Messages API clients authenticate via x-api-key.
-  // Without this fallback, per-key policies are bypassed and traffic is
-  // recorded with null api_key_id (invisible in Costs / Analytics).
-  const xApiKey = request.headers.get("x-api-key") || request.headers.get("X-Api-Key");
-  if (typeof xApiKey === "string") {
-    const trimmed = xApiKey.trim();
-    if (trimmed.length > 0) return trimmed;
+  // Gate the fallback on the anthropic-version header so we don't trip up
+  // local-mode requests from non-Anthropic clients that send placeholder
+  // x-api-key values (which would otherwise be rejected as Invalid API key).
+  const anthropicVersion =
+    request.headers.get("anthropic-version") || request.headers.get("Anthropic-Version");
+  if (anthropicVersion) {
+    const xApiKey = request.headers.get("x-api-key") || request.headers.get("X-Api-Key");
+    if (typeof xApiKey === "string") {
+      const trimmed = xApiKey.trim();
+      if (trimmed.length > 0) return trimmed;
+    }
   }
   return null;
 }

tests/unit/auth-extract-api-key.test.ts

@@ -7,6 +7,8 @@ function makeRequest(headers: Record<string, string>): Request {
   return new Request("https://omniroute.test/v1/messages", { headers });
 }
 
+const ANTHROPIC = { "anthropic-version": "2023-06-01" } as const;
+
 test("extractApiKey returns Bearer key when Authorization header is set", () => {
   const req = makeRequest({ Authorization: "Bearer sk-test-bearer" });
   assert.equal(extractApiKey(req), "sk-test-bearer");
@@ -27,18 +29,18 @@ test("extractApiKey is case-insensitive on the 'bearer' prefix", () => {
   assert.equal(extractApiKey(req), "sk-lowercase-prefix");
 });
 
-test("extractApiKey falls back to x-api-key when Authorization is absent (#2225)", () => {
-  const req = makeRequest({ "x-api-key": "sk-anthropic-native" });
+test("extractApiKey falls back to x-api-key when Authorization is absent and anthropic-version is set (#2225)", () => {
+  const req = makeRequest({ "x-api-key": "sk-anthropic-native", ...ANTHROPIC });
   assert.equal(extractApiKey(req), "sk-anthropic-native");
 });
 
-test("extractApiKey accepts uppercase X-Api-Key header (#2225)", () => {
-  const req = makeRequest({ "X-Api-Key": "sk-uppercase-xapikey" });
+test("extractApiKey accepts uppercase X-Api-Key header alongside anthropic-version (#2225)", () => {
+  const req = makeRequest({ "X-Api-Key": "sk-uppercase-xapikey", ...ANTHROPIC });
   assert.equal(extractApiKey(req), "sk-uppercase-xapikey");
 });
 
 test("extractApiKey trims surrounding whitespace from x-api-key value", () => {
-  const req = makeRequest({ "x-api-key": "   sk-padded-xapikey   " });
+  const req = makeRequest({ "x-api-key": "   sk-padded-xapikey   ", ...ANTHROPIC });
   assert.equal(extractApiKey(req), "sk-padded-xapikey");
 });
 
@@ -46,6 +48,7 @@ test("extractApiKey prefers Bearer over x-api-key when both are present (back-co
   const req = makeRequest({
     Authorization: "Bearer sk-bearer-wins",
     "x-api-key": "sk-loser",
+    ...ANTHROPIC,
   });
   assert.equal(extractApiKey(req), "sk-bearer-wins");
 });
@@ -56,7 +59,7 @@ test("extractApiKey returns null when neither header is present", () => {
 });
 
 test("extractApiKey returns null when x-api-key contains only whitespace", () => {
-  const req = makeRequest({ "x-api-key": "   " });
+  const req = makeRequest({ "x-api-key": "   ", ...ANTHROPIC });
   assert.equal(extractApiKey(req), null);
 });
 
@@ -65,10 +68,24 @@ test("extractApiKey returns null when Authorization is not a Bearer scheme and x
   assert.equal(extractApiKey(req), null);
 });
 
-test("extractApiKey falls back to x-api-key when Authorization is a non-Bearer scheme", () => {
+test("extractApiKey falls back to x-api-key when Authorization is a non-Bearer scheme (anthropic-version present)", () => {
   const req = makeRequest({
     Authorization: "Basic <stub-base64>",
     "x-api-key": "stub-fallback-after-basic",
+    ...ANTHROPIC,
   });
   assert.equal(extractApiKey(req), "stub-fallback-after-basic");
 });
+
+test("extractApiKey ignores x-api-key when anthropic-version is missing — protects local-mode non-Anthropic clients", () => {
+  const req = makeRequest({ "x-api-key": "placeholder-key" });
+  assert.equal(extractApiKey(req), null);
+});
+
+test("extractApiKey accepts Anthropic-Version (TitleCase) header", () => {
+  const req = makeRequest({
+    "x-api-key": "sk-titlecase-version",
+    "Anthropic-Version": "2024-10-22",
+  });
+  assert.equal(extractApiKey(req), "sk-titlecase-version");
+});