fix(logging,sse): redact sensitive log fields and default SSE comments to disabled (#10539)

* fix(logging): redact client IPs and account prefixes by default

ProxyEgress and AUTH logs exposed client IPs, egress IPs, and account
prefixes at info level — a privacy leak in multi-tenant/shared-log
environments. Now redacted by default, only shown when debugMode=true.

Fixes #10348

* fix(sse): default SSE comment lines to disabled

Strict SSE clients (WorkBuddy, etc.) JSON.parse every SSE line and
crash on  comment lines. Changed OMNIROUTE_SSE_COMMENTS
default from enabled to disabled. Operators can opt in with
OMNIROUTE_SSE_COMMENTS=on.

Fixes #10524

* fix(logging): gate AUTH account-prefix redaction on a narrow flag, not debugMode

The proxy-log redaction half of #10348 is superseded by an already-merged
fix (PROXY_LOG_INCLUDE_IPS, decoupled from debugMode). The remaining gap was
the chat.ts AUTH log line ("Using <provider> account: <prefix>..."), which
this PR gated on the broad `debugMode` setting. `debugMode` is a general
dashboard-visibility toggle unrelated to log privacy — coupling redaction to
it means any future, unrelated change to debugMode's default silently
changes whether account prefixes leak into logs.

Add a dedicated AUTH_LOG_INCLUDE_ACCOUNT_ID feature flag (default off,
security category) and gate the AUTH log line on it via
isFeatureFlagEnabled(), which reads the DB override synchronously on every
call (no stale in-memory cache to invalidate) and fails safe to redacted on
any lookup error.

Also update the SSE-comments tests/docs that still asserted the old
enabled-by-default behavior (tests/unit/sseHeartbeat.test.ts,
tests/unit/sse-comments-optout-9305.test.ts, docs/reference/ENVIRONMENT.md)
to match the new default-off behavior from this PR's earlier commit.

Refs #10348, #10524

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

docs/reference/ENVIRONMENT.md

@@ -702,7 +702,7 @@ REQUEST_TIMEOUT_MS (global override)
 | `REQUEST_TIMEOUT_MS`                      | _(unset)_            | Global shortcut — overrides both `FETCH_TIMEOUT_MS` and `STREAM_IDLE_TIMEOUT_MS` defaults.                                                                      |
 | `FETCH_TIMEOUT_MS`                        | `600000`             | Total HTTP request timeout for upstream provider calls.                                                                                                         |
 | `STREAM_IDLE_TIMEOUT_MS`                  | `600000`             | Max silence between SSE chunks before aborting. Extended-thinking models rarely pause >90s.                                                                     |
-| `OMNIROUTE_SSE_COMMENTS`                  | _(enabled)_          | Whether OmniRoute may emit SSE `:` comment lines (e.g. the `: keepalive` heartbeat). Set `off` to suppress comment-shaped heartbeats (no-op) for strict OpenAI-compatible clients that JSON.parse every SSE line; `data:` heartbeats are unaffected. Used by `open-sse/utils/sseHeartbeat.ts`.                |
+| `OMNIROUTE_SSE_COMMENTS`                  | _(disabled)_         | Whether OmniRoute may emit SSE `:` comment lines (e.g. the `: keepalive` heartbeat and `x-omniroute-*` metadata trailers). Disabled by default (#10524) since strict OpenAI-compatible clients JSON.parse every SSE line and crash on `:` comments; `data:` heartbeats are unaffected. Set `on`/`true`/`1`/`yes` to opt back in. Used by `open-sse/utils/sseHeartbeat.ts`.                |
 | `STREAM_READINESS_TIMEOUT_MS`             | `80000`              | Time to receive the first non-ping SSE event. Inherits `REQUEST_TIMEOUT_MS` when set.                                                                           |
 | `STREAM_READINESS_MAX_TIMEOUT_MS`         | `180000`             | Maximum adaptive first-event readiness window for large, tool-heavy, or high-reasoning streaming requests.                                                       |
 | `OMNIROUTE_AGENT_GOAL_POLICY_ENABLED`     | `true`               | Kill-switch for the `/goal` heuristic. Set `false`/`0`/`off` to fully disable detection — readiness timeouts and stream recovery are never elevated by request body/headers, mitigating client-controlled timeout amplification. |

open-sse/utils/sseHeartbeat.ts

@@ -71,16 +71,17 @@ const HEARTBEAT_ENCODER = new TextEncoder();
 /**
  * Whether OmniRoute may emit SSE `:` comment lines (e.g. the `: keepalive` heartbeat).
  * Some strict OpenAI-compatible clients parse every SSE line as JSON and crash on `:` comments.
- * Set OMNIROUTE_SSE_COMMENTS=off to suppress comment-shaped heartbeats (they become a no-op).
- * Defaults to enabled for backward compatibility.
+ * Set OMNIROUTE_SSE_COMMENTS=on to enable comment-shaped heartbeats and telemetry trailers.
+ * #10524: defaults to disabled — strict SSE clients (WorkBuddy, etc.) break on `: x-omniroute-*`
+ * comment lines. Operators who want the telemetry can opt in with OMNIROUTE_SSE_COMMENTS=on.
  */
 export function sseCommentsEnabled(): boolean {
   // SSR/edge safety: `process` is not defined in Workers/Deno/edge runtimes.
-  if (typeof process === "undefined") return true;
+  if (typeof process === "undefined") return false;
   const v = process.env.OMNIROUTE_SSE_COMMENTS;
-  if (v === undefined || v === "") return true;
+  if (v === undefined || v === "") return false;
   const normalized = v.trim().toLowerCase();
-  return normalized !== "off" && normalized !== "false" && normalized !== "0" && normalized !== "no";
+  return normalized === "on" || normalized === "true" || normalized === "1" || normalized === "yes";
 }
 
 export function createSseHeartbeatTransform({

src/shared/constants/featureFlagDefinitions.ts

@@ -12,7 +12,7 @@ export interface FeatureFlagDefinition {
 }
 
 export const FEATURE_FLAG_DEFINITIONS: FeatureFlagDefinition[] = [
-  // ──────────────── Security (9) ────────────────
+  // ──────────────── Security (10) ────────────────
   {
     key: "REQUIRE_API_KEY",
     label: "Require API Key",
@@ -105,6 +105,20 @@ export const FEATURE_FLAG_DEFINITIONS: FeatureFlagDefinition[] = [
     requiresRestart: false,
     warningLevel: "danger",
   },
+  {
+    key: "AUTH_LOG_INCLUDE_ACCOUNT_ID",
+    label: "Log Account IDs",
+    description:
+      "Include the account ID prefix in AUTH log lines (e.g. \"Using <provider> account: abc12345...\"). " +
+      "Disabled by default so the account identifier is redacted in shared/multi-tenant process logs. " +
+      "Independent of Debug Mode — flipping Debug Mode on does not reveal this.",
+    descriptionI18nKey: "featureFlagAuthLogIncludeAccountIdDescription",
+    category: "security",
+    defaultValue: "false",
+    type: "boolean",
+    requiresRestart: false,
+    warningLevel: "info",
+  },
   // ──────────────── Network (7) ────────────────
   {
     key: "ENABLE_TLS_FINGERPRINT",

src/sse/handlers/chat.ts

@@ -119,6 +119,7 @@ import { getComboFailureLogError } from "./comboFailureLogging";
 import { classify429FromError, type FailureKind } from "@/shared/utils/classify429";
 import { isSubscriptionQuotaText } from "@omniroute/open-sse/services/quotaTextCooldowns.ts";
 import { resolveUseUpstream429BreakerHints } from "@/shared/utils/providerHints";
+import { isFeatureFlagEnabled } from "@/shared/utils/featureFlags";
 import { getCircuitBreaker, isLocalStreamLifecycleError } from "../../shared/utils/circuitBreaker";
 import { markAccountExhaustedFrom429 } from "../../domain/quotaCache";
 import { resolveForcedConnectionForCredentialPool } from "../services/sessionAffinityPin.ts";
@@ -1551,7 +1552,22 @@ async function handleSingleModelChat(
 
       const accountId = credentials.connectionId.slice(0, 8);
       const releaseOAuthSession = credentials.releaseOAuthSession ?? (() => {});
-      log.info("AUTH", `Using ${provider} account: ${accountId}...`);
+      // #10348: redact the account prefix by default. Gated on the narrow
+      // AUTH_LOG_INCLUDE_ACCOUNT_ID flag (default off) rather than the broad
+      // `debugMode` setting — `debugMode` is a general dashboard-visibility
+      // toggle unrelated to log privacy (its own default has changed
+      // independently for unrelated reasons, see #10312/#10372), so deriving
+      // redaction from it would make log leakage depend on an unrelated
+      // setting. resolveFeatureFlag() reads straight from SQLite on every
+      // call (no stale cache to invalidate) and fails safe (redacted) if the
+      // lookup throws.
+      let includeAccountId = false;
+      try {
+        includeAccountId = isFeatureFlagEnabled("AUTH_LOG_INCLUDE_ACCOUNT_ID");
+      } catch {
+        includeAccountId = false;
+      }
+      log.info("AUTH", `Using ${provider} account: ${includeAccountId ? accountId : "***"}...`);
       // #474: when the request used a bare model name (no "/" — e.g. an alias
       // that resolved to "auto") and the selected connection declares a
       // defaultModel, resolve the bare name to that real model ID before the

tests/unit/auth-log-account-id-redaction-10539.test.ts

@@ -0,0 +1,98 @@
+import test from "node:test";
+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";
+import { dirname, resolve } from "node:path";
+
+// Regression guard for #10348/#10539: the AUTH log line in the SSE chat
+// handler ("Using <provider> account: <prefix>...") must redact the account
+// prefix by default. Redaction MUST be governed by a narrow, dedicated
+// feature flag (AUTH_LOG_INCLUDE_ACCOUNT_ID) — NOT by the broad `debugMode`
+// setting. `debugMode` is a general dashboard-visibility toggle unrelated to
+// log privacy (its own default has flipped independently more than once,
+// see #10372/#10312); deriving log redaction from it means any future,
+// unrelated change to debugMode's default silently changes whether account
+// prefixes leak into logs. The narrow flag keeps the two concerns separate.
+
+// Isolate DB state so the resolution chain (DB override > env > default)
+// reads a clean store and we exercise the definition default, not a leaked
+// override from another test file.
+const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-test-auth-log-account-id-"));
+process.env.DATA_DIR = tmpDir;
+
+const { FEATURE_FLAG_DEFINITIONS } = await import(
+  "../../src/shared/constants/featureFlagDefinitions.ts"
+);
+
+test("AUTH_LOG_INCLUDE_ACCOUNT_ID feature flag defaults to OFF, independent of debugMode", async (t) => {
+  const def = (key: string) => FEATURE_FLAG_DEFINITIONS.find((d) => d.key === key);
+
+  await t.test("AUTH_LOG_INCLUDE_ACCOUNT_ID definition exists", () => {
+    assert.ok(
+      def("AUTH_LOG_INCLUDE_ACCOUNT_ID"),
+      "AUTH_LOG_INCLUDE_ACCOUNT_ID definition must exist in FEATURE_FLAG_DEFINITIONS"
+    );
+  });
+
+  await t.test("AUTH_LOG_INCLUDE_ACCOUNT_ID default value is 'false'", () => {
+    assert.strictEqual(
+      def("AUTH_LOG_INCLUDE_ACCOUNT_ID")!.defaultValue,
+      "false",
+      "AUTH_LOG_INCLUDE_ACCOUNT_ID must default OFF — account prefixes are redacted by default"
+    );
+  });
+
+  await t.test("AUTH_LOG_INCLUDE_ACCOUNT_ID category is 'security'", () => {
+    assert.strictEqual(def("AUTH_LOG_INCLUDE_ACCOUNT_ID")!.category, "security");
+  });
+
+  await t.test("AUTH_LOG_INCLUDE_ACCOUNT_ID type is 'boolean'", () => {
+    assert.strictEqual(def("AUTH_LOG_INCLUDE_ACCOUNT_ID")!.type, "boolean");
+  });
+
+  await t.test("effective runtime resolution is OFF with no override", async () => {
+    delete process.env.AUTH_LOG_INCLUDE_ACCOUNT_ID;
+    const { clearAllFeatureFlagOverrides } = await import("@/lib/db/featureFlags");
+    clearAllFeatureFlagOverrides();
+
+    const { isFeatureFlagEnabled } = await import("@/shared/utils/featureFlags");
+    assert.strictEqual(isFeatureFlagEnabled("AUTH_LOG_INCLUDE_ACCOUNT_ID"), false);
+  });
+
+  await t.test("explicit env override enables it", async () => {
+    process.env.AUTH_LOG_INCLUDE_ACCOUNT_ID = "true";
+    try {
+      const { isFeatureFlagEnabled } = await import("@/shared/utils/featureFlags");
+      assert.strictEqual(isFeatureFlagEnabled("AUTH_LOG_INCLUDE_ACCOUNT_ID"), true);
+    } finally {
+      delete process.env.AUTH_LOG_INCLUDE_ACCOUNT_ID;
+    }
+  });
+});
+
+test("chat.ts AUTH account log line is gated on the narrow flag, not on debugMode", () => {
+  const here = dirname(fileURLToPath(import.meta.url));
+  const chatHandlerPath = resolve(here, "../../src/sse/handlers/chat.ts");
+  const src = fs.readFileSync(chatHandlerPath, "utf8");
+
+  assert.match(
+    src,
+    /Using \$\{provider\} account: \$\{includeAccountId \? accountId : "\*\*\*"\}/,
+    "chat.ts must redact the account prefix behind a boolean gate variable"
+  );
+
+  assert.match(
+    src,
+    /isFeatureFlagEnabled\("AUTH_LOG_INCLUDE_ACCOUNT_ID"\)/,
+    "chat.ts must resolve the redaction gate via the narrow AUTH_LOG_INCLUDE_ACCOUNT_ID flag"
+  );
+
+  // The old, broad debugMode-derived gate must be gone from this call site.
+  assert.doesNotMatch(
+    src,
+    /debugMode === true[\s\S]{0,80}Using \$\{provider\} account/,
+    "the AUTH account log line must not be gated on the broad debugMode setting"
+  );
+});

tests/unit/feature-flags-settings.test.ts

@@ -30,13 +30,13 @@ const {
   isControlPlaneProxyDirectFallbackEnabled,
 } = await import("../../src/shared/utils/featureFlags.ts");
 
-const EXPECTED_FEATURE_FLAG_COUNT = 48;
+const EXPECTED_FEATURE_FLAG_COUNT = 49;
 
 // ──────────────────────────────────────────────────────
 // Test group 1 — Flag definitions registry
 // ──────────────────────────────────────────────────────
 describe("featureFlagDefinitions", () => {
-  it("has exactly 47 flag definitions", () => {
+  it(`has exactly ${EXPECTED_FEATURE_FLAG_COUNT} flag definitions`, () => {
     assert.strictEqual(FEATURE_FLAG_DEFINITIONS.length, EXPECTED_FEATURE_FLAG_COUNT);
   });
 
@@ -344,7 +344,7 @@ describe("resolveFeatureFlag", () => {
   });
 
   describe("resolveAllFeatureFlags", () => {
-    it("returns all 47 flags", () => {
+    it(`returns all ${EXPECTED_FEATURE_FLAG_COUNT} flags`, () => {
       const all = resolveAllFeatureFlags();
       assert.strictEqual(all.length, EXPECTED_FEATURE_FLAG_COUNT);
     });

tests/unit/sse-comments-default-10524.test.ts

@@ -0,0 +1,39 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { sseCommentsEnabled } from "../../open-sse/utils/sseHeartbeat.ts";
+
+// #10524: SSE comment lines (`: x-omniroute-*`) break strict SSE clients.
+// Default should be disabled (opt-in via OMNIROUTE_SSE_COMMENTS=on).
+
+test("#10524: sseCommentsEnabled defaults to false when env var is unset", () => {
+  const original = process.env.OMNIROUTE_SSE_COMMENTS;
+  delete process.env.OMNIROUTE_SSE_COMMENTS;
+
+  assert.strictEqual(sseCommentsEnabled(), false, "SSE comments must be disabled by default");
+
+  if (original !== undefined) process.env.OMNIROUTE_SSE_COMMENTS = original;
+});
+
+test("#10524: sseCommentsEnabled returns true when explicitly enabled", () => {
+  const original = process.env.OMNIROUTE_SSE_COMMENTS;
+
+  for (const value of ["on", "true", "1", "yes", "ON", "TRUE"]) {
+    process.env.OMNIROUTE_SSE_COMMENTS = value;
+    assert.strictEqual(sseCommentsEnabled(), true, `SSE comments must be enabled for "${value}"`);
+  }
+
+  if (original !== undefined) process.env.OMNIROUTE_SSE_COMMENTS = original;
+  else delete process.env.OMNIROUTE_SSE_COMMENTS;
+});
+
+test("#10524: sseCommentsEnabled returns false when explicitly disabled", () => {
+  const original = process.env.OMNIROUTE_SSE_COMMENTS;
+
+  for (const value of ["off", "false", "0", "no", "OFF", "FALSE"]) {
+    process.env.OMNIROUTE_SSE_COMMENTS = value;
+    assert.strictEqual(sseCommentsEnabled(), false, `SSE comments must be disabled for "${value}"`);
+  }
+
+  if (original !== undefined) process.env.OMNIROUTE_SSE_COMMENTS = original;
+  else delete process.env.OMNIROUTE_SSE_COMMENTS;
+});

tests/unit/sse-comments-optout-9305.test.ts

@@ -127,7 +127,9 @@ for (const upstreamDone of [true, false]) {
   const finalization = upstreamDone ? "upstream [DONE]" : "natural EOF";
 
   for (const [label, envValue, commentsExpected] of [
-    ["default", undefined, true],
+    // #10524: OMNIROUTE_SSE_COMMENTS now defaults to disabled — strict SSE
+    // clients (WorkBuddy, etc.) crash on `: x-omniroute-*` comment lines.
+    ["default", undefined, false],
     ["explicitly enabled", "yes", true],
     ["disabled", "off", false],
   ] as const) {

tests/unit/sseHeartbeat.test.ts

@@ -51,8 +51,8 @@ async function collectHeartbeatOutput(
   ).text();
 }
 
-test("sseCommentsEnabled defaults to true when the env var is unset", () => {
-  withEnv(undefined, () => assert.equal(sseCommentsEnabled(), true));
+test("sseCommentsEnabled defaults to false when the env var is unset (#10524)", () => {
+  withEnv(undefined, () => assert.equal(sseCommentsEnabled(), false));
 });
 
 test("sseCommentsEnabled is false for 'off', 'false', '0', 'no' (case-insensitive)", () => {