fix(auth): never auto is_active=0 on unpaid/ban flaps; recover after renew

docs/security/BAN_DETECTION.md

@@ -64,7 +64,10 @@ evaluated first — so the project-route recovery added for Gemini Code Assist i
 Three adjacent, **separate** signal tables are _not_ part of banned-keyword detection:
 
 - `CREDITS_EXHAUSTED_SIGNALS` — billing/quota depleted (`insufficient_quota`,
-  `credit_balance_too_low`, `payment required`, …) → terminal `credits_exhausted`.
+  `credit_balance_too_low`, `payment required`, …) → `credits_exhausted` testStatus
+  (selection-skip + alert). **Not** an auto `is_active=0`, and **not** forever:
+  `connectionRecovery` re-probes on a timer so unpaid→renew restores the account
+  without an OmniRoute UI re-enable.
 - `OAUTH_INVALID_TOKEN_SIGNALS` — **non-terminal**; a token refresh can recover.
 - `ACCOUNT_VERIFICATION_REQUIRED_SIGNALS` — **non-terminal**; the operator has to
   re-verify the account upstream. Lives in `open-sse/services/errorClassifier.ts`
@@ -92,9 +95,13 @@ upstream error response
   (`isAccountDeactivated`, `accountFallback.ts`).
 - The permanent `banned` terminalization fires on a banned-signal body at **any
   HTTP status** (via `markAccountUnavailable` → `checkFallbackError`). The
-  narrower **`deactivated`** label (`isActive=false` when the connection has no
-  spare API keys) is written by the inline `chatCore.ts` path on **HTTP 401 / 403**
-  (classified via `classifyProviderError` → `ACCOUNT_DEACTIVATED`). Note the
+  narrower **`deactivated`** / **`banned`** labels are written by the inline
+  `chatCore.ts` path on **HTTP 401 / 403** (classified via `classifyProviderError`
+  → `ACCOUNT_DEACTIVATED` / `FORBIDDEN`). Those paths record `testStatus` for
+  selection skip + alerts and only flip `isActive=false` when
+  `autoDisableBannedAccounts` (+ scope) allows it — same gate as
+  `maybeAutoDisableBannedAccount` in `auth.ts`. Ungated OAuth-refresh death also
+  keeps `is_active=1` and only sets `testStatus=expired`. Note the
   `markAccountUnavailable()` path writes a _different_ terminal status —
   **`expired`** — for the same `ACCOUNT_DEACTIVATED` signal (via
   `resolveTerminalConnectionStatus`), so the same ban can surface as either
@@ -144,9 +151,10 @@ doubt, watch the connection's `lastError` first, then add the exact wording.
 
 ## Recovering a flagged connection
 
-Terminal `banned` / `deactivated` states **never auto-recover** (they are excluded
-from the proactive-recovery tick — only `unavailable` cooldowns recover on their
-own). An operator must clear them explicitly:
+Terminal `banned` / `deactivated` / `expired` states **never auto-recover** (they
+are excluded from the proactive-recovery tick — only `unavailable` cooldowns and
+`credits_exhausted` re-probes recover on their own). An operator must clear true
+bans explicitly:
 
 1. **Re-test the connection** — the dashboard **Test** action
    (`POST /api/providers/{id}/test`); a successful probe resets `testStatus` to

open-sse/handlers/chatCore.ts

@@ -419,6 +419,7 @@ import { generateRequestId } from "@/shared/utils/requestId";
 import { isLocalStreamLifecycleError } from "@/shared/utils/circuitBreaker";
 import { shouldIsolateProbeFailures } from "@/shared/utils/probeOrigin";
 import { writeTerminalStatus } from "@/shared/utils/terminalStatus";
+import { recordGatedBanTerminalStatus } from "./chatCore/gatedTerminalBan.ts";
 import { extractFacts } from "@/lib/memory/extraction";
 import { handleToolCallExecution } from "@/lib/skills/interception";
 import { MEMORY_BUILTIN_TOOL_NAMES } from "@/lib/skills/memoryBuiltins";
@@ -3851,27 +3852,17 @@ async function handleChatCoreInner({
     if (errorConnectionId && errorType) {
       try {
         if (errorType === PROVIDER_ERROR_TYPES.FORBIDDEN) {
-          const probeIsolated = await shouldIsolateProbeFailures();
-          await writeTerminalStatus(
-            errorConnectionId,
-            {
-              testStatus: "banned",
-              isActive: false,
-              lastError: persistentMessage,
-              lastErrorType: errorType,
-              errorCode: String(statusCode),
-            },
-            probeIsolated ? "probe" : "production"
-          );
-          if (probeIsolated) {
-            console.warn(
-              `[provider] Node ${errorConnectionId} probe ${errorType} (${statusCode}) -- connection stays active`
-            );
-          } else {
-            console.warn(
-              `[provider] Node ${errorConnectionId} banned (${statusCode}) -- disabling permanently`
-            );
-          }
+          await recordGatedBanTerminalStatus({
+            connectionId: errorConnectionId,
+            testStatus: "banned",
+            errorType,
+            statusCode,
+            persistentMessage,
+            provider,
+            authType: (credentials as { authType?: string | null } | null | undefined)?.authType,
+            connectionProvider: (credentials as { provider?: string | null } | null | undefined)
+              ?.provider,
+          });
         } else if (errorType === PROVIDER_ERROR_TYPES.ACCOUNT_DEACTIVATED) {
           if (
             connectionHasExtraKeys(
@@ -3889,27 +3880,17 @@ async function handleChatCoreInner({
               `[provider] Node ${errorConnectionId} account deactivated (${statusCode}) -- has extra keys, keeping connection active`
             );
           } else {
-            const probeIsolated2 = await shouldIsolateProbeFailures();
-            await writeTerminalStatus(
-              errorConnectionId,
-              {
-                testStatus: "deactivated",
-                isActive: false,
-                lastError: persistentMessage,
-                lastErrorType: errorType,
-                errorCode: String(statusCode),
-              },
-              probeIsolated2 ? "probe" : "production"
-            );
-            if (probeIsolated2) {
-              console.warn(
-                `[provider] Node ${errorConnectionId} probe ${errorType} (${statusCode}) -- connection stays active`
-              );
-            } else {
-              console.warn(
-                `[provider] Node ${errorConnectionId} account deactivated (${statusCode}) -- disabling permanently`
-              );
-            }
+            await recordGatedBanTerminalStatus({
+              connectionId: errorConnectionId,
+              testStatus: "deactivated",
+              errorType,
+              statusCode,
+              persistentMessage,
+              provider,
+              authType: (credentials as { authType?: string | null } | null | undefined)?.authType,
+              connectionProvider: (credentials as { provider?: string | null } | null | undefined)
+                ?.provider,
+            });
           }
         } else if (errorType === PROVIDER_ERROR_TYPES.QUOTA_EXHAUSTED) {
           const probeIsolated3 = await shouldIsolateProbeFailures();
@@ -4621,11 +4602,11 @@ async function handleChatCoreInner({
       } else {
         log?.warn?.("TOKEN", `${provider?.toUpperCase()} | refresh failed`);
         if (isUnrecoverableRefreshError(newCredentials) && onCredentialsRefreshed) {
-          // Front 3 (reuse-race tolerance): before deactivating, re-read the DB.
+          // Front 3 (reuse-race tolerance): before marking expired, re-read the DB.
           // If a sibling/concurrent refresh already rotated this connection's
           // refresh_token (common for Codex/OpenAI under one shared Auth0 client),
           // the failure we saw was a stale-token reuse — the account is healthy
-          // with the newer token, so keep it active instead of killing it.
+          // with the newer token, so keep it active instead of marking expired.
           let alreadyRotated = false;
           if (typeof connectionId === "string" && connectionId && attemptedRefreshToken) {
             try {
@@ -4638,11 +4619,15 @@ async function handleChatCoreInner({
                 );
               }
             } catch {
-              // DB read failed — fall through to the safe default (deactivate).
+              // DB read failed — fall through to the safe default (mark expired).
             }
           }
           if (!alreadyRotated) {
-            await onCredentialsRefreshed({ testStatus: "expired", isActive: false });
+            // HARD: OAuth refresh death is informative (alerts / selection skip via
+            // testStatus=expired), not an auto is_active=0. Access can recover after
+            // re-auth / token rotation without requiring a UI re-enable. Permanent
+            // deactivation stays behind autoDisableBannedAccounts on true bans.
+            await onCredentialsRefreshed({ testStatus: "expired" });
           }
         }
       }

open-sse/handlers/chatCore/gatedTerminalBan.ts

@@ -0,0 +1,54 @@
+import { shouldIsolateProbeFailures } from "@/shared/utils/probeOrigin";
+import { writeTerminalStatus } from "@/shared/utils/terminalStatus";
+import { maybeAutoDisableBannedAccount } from "@/sse/services/autoDisableBannedAccount";
+
+/**
+ * chatCore leaf for the two terminal-ban classifications that share one shape:
+ * FORBIDDEN (testStatus=banned) and ACCOUNT_DEACTIVATED (testStatus=deactivated).
+ *
+ * #14853 invariant: record the terminal testStatus for selection skip + alerts,
+ * but NEVER ungated-flip isActive on a ban-looking flap. Permanent deactivation
+ * is opt-in only, gated behind autoDisableBannedAccounts (+ scope) via
+ * maybeAutoDisableBannedAccount — the same gate auth.ts uses. Probe-origin
+ * failures (dashboard test-all) are recorded but never deactivate (#9817),
+ * unless the operator opts into probeCanDisable.
+ */
+export async function recordGatedBanTerminalStatus(params: {
+  connectionId: string;
+  testStatus: "banned" | "deactivated";
+  errorType: string;
+  statusCode: number;
+  persistentMessage: string;
+  provider?: string | null;
+  authType?: string | null;
+  connectionProvider?: string | null;
+}): Promise<void> {
+  const probeIsolated = await shouldIsolateProbeFailures();
+  await writeTerminalStatus(
+    params.connectionId,
+    {
+      testStatus: params.testStatus,
+      lastError: params.persistentMessage,
+      lastErrorType: params.errorType,
+      errorCode: String(params.statusCode),
+    },
+    probeIsolated ? "probe" : "production"
+  );
+  if (probeIsolated) {
+    console.warn(
+      `[provider] Node ${params.connectionId} probe ${params.errorType} (${params.statusCode}) -- connection stays active`
+    );
+    return;
+  }
+  await maybeAutoDisableBannedAccount({
+    connectionId: params.connectionId,
+    provider: params.provider,
+    authType: params.authType,
+    connectionProvider: params.connectionProvider,
+    permanent: true,
+  });
+  const label = params.testStatus === "banned" ? "banned" : "account deactivated";
+  console.warn(
+    `[provider] Node ${params.connectionId} ${label} (${params.statusCode}) -- testStatus=${params.testStatus}; isActive gated by autoDisableBannedAccounts`
+  );
+}

src/shared/utils/terminalStatus.ts

@@ -34,12 +34,20 @@ export async function writeTerminalStatus(
     });
     return;
   }
-  await updateProviderConnection(connectionId, {
-    isActive: patch.isActive ?? (isTerminal ? false : undefined),
+  // HARD: never auto-flip isActive on health flaps / temporary unpaid /
+  // ban-looking errors. Callers that truly need deactivation must pass
+  // isActive explicitly AND gate it behind autoDisableBannedAccounts
+  // (see maybeAutoDisableBannedAccount). credits_exhausted in particular
+  // must stay is_active=1 so unpaid→renew recovers without a UI re-enable.
+  const update: Record<string, unknown> = {
     testStatus: patch.testStatus,
     lastError: persistedLastError,
     lastErrorAt: new Date().toISOString(),
     lastErrorType: patch.lastErrorType ?? null,
     errorCode: patch.errorCode ?? null,
-  });
+  };
+  if (typeof patch.isActive === "boolean") {
+    update.isActive = patch.isActive;
+  }
+  await updateProviderConnection(connectionId, update);
 }

tests/unit/anthropic-request-not-allowed-cooldown-escalation.test.ts

@@ -34,6 +34,7 @@ const {
   hasRequestRejectedStreak,
 } = await import("../../open-sse/services/requestRejectedStreak.ts");
 const { COOLDOWN_MS } = await import("../../open-sse/config/errorConfig.ts");
+const { updateSettings } = await import("../../src/lib/db/settings.ts");
 
 const originalFetch = globalThis.fetch;
 const MODEL = "claude/claude-sonnet-5";
@@ -232,10 +233,31 @@ test("a successful response resets the streak — sporadic refusals never accumu
 });
 
 test("any other claude 403 still bans on the first response (regression guard)", async () => {
+  // NEW behavior (#14853): a true ban records testStatus=banned but only flips
+  // is_active=0 when the gated auto-disable opt-in is on (+ scope). claude/oauth
+  // is subscription-style, covered by the default "all" scope.
+  await updateSettings({ autoDisableBannedAccounts: true });
   const connId = await createClaudeConnection();
   mockAnthropic403("Your organization has been disabled.");
   await sendOnce(connId);
   const row = readRow(connId);
   assert.equal(row?.test_status, "banned");
   assert.equal(row?.is_active, 0);
 });
+
+test("#14853: a real claude 403 ban records testStatus=banned but keeps is_active=1 when auto-disable is OFF", async () => {
+  // chatCore FORBIDDEN path, flag OFF (the default): a ban-looking flap must
+  // NOT ungated-flip is_active. Access recovers once the flap clears without a
+  // dashboard re-enable. Pairs with the flag-ON guard above.
+  await updateSettings({ autoDisableBannedAccounts: false });
+  const connId = await createClaudeConnection();
+  mockAnthropic403("Your organization has been disabled.");
+  await sendOnce(connId);
+  const row = readRow(connId);
+  assert.equal(
+    row?.test_status,
+    "banned",
+    "testStatus=banned still recorded for selection skip + alerts"
+  );
+  assert.equal(row?.is_active, 1, "ban-looking flap must NOT ungated-deactivate");
+});

tests/unit/fixtures/error-public-boundaries-hardening.fixture.ts

@@ -429,13 +429,25 @@ test("chatCore provider-failure writes use the projected persistent message", ()
   );
   assert.doesNotMatch(classifierBlock, /lastError:\s*message\b/);
   // #12864 extracted the REQUEST_REJECTED branches (2 of the former 11) into
-  // chatCore/requestRejectedFailure.ts. Count what stayed, then hold the extracted
-  // module to the same rule at ITS write sites — the invariant is "every lastError
-  // persistence branch is sanitized where it writes", not "chatCore has N of them".
+  // chatCore/requestRejectedFailure.ts. #14853 (Refs) then extracted the two
+  // FORBIDDEN / ACCOUNT_DEACTIVATED terminal-ban writes into
+  // chatCore/gatedTerminalBan.ts. Count what stayed, then hold each extracted
+  // module to the same rule at ITS write sites — the invariant is "every
+  // lastError persistence branch is sanitized where it writes", not "chatCore
+  // has N of them".
   assert.ok(
-    (classifierBlock.match(/lastError:\s*persistentMessage\b/g) || []).length >= 9,
+    (classifierBlock.match(/lastError:\s*persistentMessage\b/g) || []).length >= 7,
     "every providerFailure persistence branch must use persistentMessage"
   );
+  const gatedBan = fs.readFileSync(
+    path.join(REPO_ROOT, "open-sse/handlers/chatCore/gatedTerminalBan.ts"),
+    "utf8"
+  );
+  assert.doesNotMatch(gatedBan, /lastError:\s*(`\$\{)?message\b/);
+  assert.ok(
+    (gatedBan.match(/lastError:\s*(params\.)?persistentMessage\b/g) || []).length >= 1,
+    "every lastError write in gatedTerminalBan.ts must use the projected persistentMessage"
+  );
   const rejected = fs.readFileSync(
     path.join(REPO_ROOT, "open-sse/handlers/chatCore/requestRejectedFailure.ts"),
     "utf8"

tests/unit/probe-testall-isolation.test.ts

@@ -155,7 +155,14 @@ test("queued (rate-limited) test-all failure stays isolated", async () => {
 
 test("RESTORE: probe with opt-in probeCanDisable=true deactivates like real traffic", async () => {
   const settingsDb = await import("../../src/lib/db/settings.ts");
-  await settingsDb.updateSettings({ probeCanDisable: true });
+  // probeCanDisable lets the probe take the real-traffic path; that path now
+  // also gates permanent deactivation behind autoDisableBannedAccounts
+  // (#14853). openai/apikey needs the "all" scope to be eligible.
+  await settingsDb.updateSettings({
+    probeCanDisable: true,
+    autoDisableBannedAccounts: true,
+    autoDisableBannedScope: "all",
+  });
   try {
     const connId = await createConnection();
     await warmUp(connId);
@@ -171,6 +178,9 @@ test("RESTORE: probe with opt-in probeCanDisable=true deactivates like real traf
     assert.equal(row?.is_active, 0, "opt-in restores historical behavior: probe deactivates");
     assert.equal(row?.test_status, "banned", "terminal banned status restored for probe");
   } finally {
-    await settingsDb.updateSettings({ probeCanDisable: false });
+    await settingsDb.updateSettings({
+      probeCanDisable: false,
+      autoDisableBannedAccounts: false,
+    });
   }
 });