Preserve shared OAuth clients when a replica cannot read them

CHANGELOG.md

@@ -78,6 +78,13 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
   `COMPUTER_MEMORY_HIGH_PERCENT=0` and `COMPUTER_TAB_IDLE_MS=0` (Helm: `computers.memoryHighPercent`
   and `computers.tabIdleMs`) keep the old behaviour.
 
+### A client read failure does not replace a working app registration
+
+A server that cannot decrypt an app's stored OAuth client now fails that connection attempt without
+registering a replacement. A server with the correct encryption key can keep using the original
+client. Confirmed missing or revoked credentials and malformed stored clients still use the existing
+registration flow. Connect records a safe read-failure audit row for the person and app and returns a retryable error, without exposing the vault failure.
+
 ### A computer closes browsers nobody is using when its memory runs high
 
 A computer runs one browser for each Bot that is browsing, and the only limits were a count and half

docs/architecture.md

@@ -390,7 +390,7 @@ Connector credentials are stored through the credential vault and referenced by
 - Signing in, being refused, and being granted the administrator role by configuration each write an audit row. They are the only record that somebody who can edit `INITIAL_ADMIN_EMAILS` promoted themselves, and the only evidence a revoked person was ever here, since revoking them deletes their sessions.
 - Removing somebody deletes their sessions and denies their address, because deleting the user row alone is not removal: the next sign-in through the provider recreates it.
 - With no identity provider configured, the deployment refuses to start unless `OPENBOT_SINGLE_USER=true` says every request may be one fixed administrator. That flag is the only thing that permits it; `NODE_ENV` does not. The flag alone is not enough on a public address: if `OPENBOT_PUBLIC_URL`, `OPENBOT_APP_URL` or any `TRUSTED_ORIGINS` entry resolves to an address the public internet routes to, the deployment refuses to start anyway, because no sign-in there means every visitor is the administrator. Loopback is silent, and a private address (a home LAN, a Tailnet, a VPN, a `.local` name) is allowed with a warning naming it, since that is the deployment the flag exists for.
-- `KEY_ENCRYPTION_KEY` must be a base64-encoded 32-byte value. The example key is refused with `NODE_ENV=production`.
+- `KEY_ENCRYPTION_KEY` must be a base64-encoded 32-byte value. The example key is refused with `NODE_ENV=production`. A vault read or decryption fault stops an OAuth client lookup without registering a replacement, so a misconfigured replica cannot replace a client still usable by the others. Connect records `mcp.oauth_client_read_failed` with the authenticated person and server before returning a safe 503; raw vault errors and causes are neither retained nor included in the audit payload.
 - Credential plaintext is encrypted at rest, never returned by APIs, and redacted from audit events.
 - Browser navigation allows `http` and `https`; cloud metadata addresses are refused under every configuration.
 - `POST /api/model-provider/v1/chat/completions` exists only when `OPENBOT_MODEL_OAUTH_FILE` names a model credential file, which the desktop app sets and nothing else does. It answers a Chat Completions request using a stored Google or xAI OAuth grant. It does not use the session guard: the caller presents a separate local bearer taken from that file and compared in constant time, and the provider's own refresh token never leaves the server. The credential file is refused unless it is a regular file under 64KB with no group or other permission bits, and the upstream host, path and headers are fixed so a caller cannot redirect the request.

server/src/audit.ts

@@ -179,6 +179,8 @@ export const auditEventTypes = [
    * was current when it was made. The client id, never the secret.
    */
   "mcp.oauth_client_registered",
+  /** A Connect attempt could not read the deployment's client. No credential or raw fault is recorded. */
+  "mcp.oauth_client_read_failed",
   /*
    * One person's brokered account was exercised with a real call, to see whether it is really there.
    *

server/src/plugins/plugin-store/connections.ts

@@ -187,6 +187,24 @@ export function connectionMethods(context: StoreContext) {
       });
     },
 
+    /** A failed Connect client lookup, attributed to the authenticated person and never to raw errors. */
+    async recordOAuthClientReadFailure(input: {
+      serverId: string;
+      actorId: string;
+    }): Promise<void> {
+      await recordAuditEvent(auditStore, {
+        eventType: "mcp.oauth_client_read_failed",
+        targetType: "mcp_server",
+        targetId: input.serverId,
+        actorUserId: input.actorId,
+        payload: {
+          server: input.serverId,
+          operation: "connect",
+          reason: "credential_read_failed",
+        },
+      });
+    },
+
     /**
      * The deployment's OAuth client for a server, or null if none is registered.
      *

server/src/plugins/plugin-store/context.ts

@@ -47,6 +47,7 @@ import {
   INVALID_GRANT,
   isUsableClient,
   type OAuthClient,
+  OAuthClientReadError,
   PluginInvariantError,
   type PluginKind,
   PluginRefusedError,
@@ -1259,6 +1260,27 @@ export function createStoreContext(options: PluginStoreOptions) {
     keyId: `oauth-client-${serverId}`,
   });
 
+  /** Classify only client reads, never vendor registration or subsequent vault writes. */
+  async function readClient(
+    work: () => Promise<OAuthClient | null>,
+  ): Promise<OAuthClient | null> {
+    try {
+      return await work();
+    } catch {
+      throw new OAuthClientReadError();
+    }
+  }
+
+  /** Parse errors can quote plaintext, so an invalid client is never rethrown with its secret. */
+  function clientFromStoredJson(plaintext: string): OAuthClient | null {
+    try {
+      const parsed: unknown = JSON.parse(plaintext);
+      return isUsableClient(parsed) ? parsed : null;
+    } catch {
+      return null;
+    }
+  }
+
   /**
    * One writer at a time for one server's OAuth client, across the whole deployment.
    *
@@ -1304,35 +1326,29 @@ export function createStoreContext(options: PluginStoreOptions) {
     transaction: Transaction,
     serverId: string,
   ): Promise<OAuthClient | null> {
-    const [server] = await transaction
-      .select({ credentialId: mcpServers.credentialId })
-      .from(mcpServers)
-      .where(eq(mcpServers.id, serverId))
-      .limit(1);
-    if (!server?.credentialId) return null;
+    return readClient(async () => {
+      const [server] = await transaction
+        .select({ credentialId: mcpServers.credentialId })
+        .from(mcpServers)
+        .where(eq(mcpServers.id, serverId))
+        .limit(1);
+      if (!server?.credentialId) return null;
 
-    const [held] = await transaction
-      .select({
-        encryptedValue: credentialRows.encryptedValue,
-        revokedAt: credentialRows.revokedAt,
-      })
-      .from(credentialRows)
-      .where(eq(credentialRows.id, server.credentialId))
-      .limit(1);
-    if (!held || held.revokedAt) return null;
+      const [held] = await transaction
+        .select({
+          encryptedValue: credentialRows.encryptedValue,
+          revokedAt: credentialRows.revokedAt,
+        })
+        .from(credentialRows)
+        .where(eq(credentialRows.id, server.credentialId))
+        .limit(1);
+      if (!held || held.revokedAt) return null;
 
-    try {
-      const parsed: unknown = JSON.parse(
-        await decryptSecret(encryptionKey, held.encryptedValue),
-      );
-      // A value that parsed and is not a client is unreadable in the same way and for the same
-      // caller — see {@link isUsableClient}. Null, because that is what this reader's caller acts
-      // on: it goes and registers one, which is the answer to holding none.
-      return isUsableClient(parsed) ? parsed : null;
-    } catch {
-      // Unreadable is the same as none: there is nothing to send anybody to consent with.
-      return null;
-    }
+      // A decryption fault does not mean the vendor forgot this client. Re-registering would replace
+      // the live client for every replica, including those that can still decrypt it.
+      const plaintext = await decryptSecret(encryptionKey, held.encryptedValue);
+      return clientFromStoredJson(plaintext);
+    });
   }
 
   /**
@@ -1453,29 +1469,29 @@ export function createStoreContext(options: PluginStoreOptions) {
   async function storedOAuthClient(
     serverId: string,
   ): Promise<OAuthClient | null> {
-    const [row] = await database
-      .select({ credentialId: mcpServers.credentialId })
-      .from(mcpServers)
-      .where(eq(mcpServers.id, serverId))
-      .limit(1);
-    if (!row?.credentialId) return null;
+    return readClient(async () => {
+      const [row] = await database
+        .select({ credentialId: mcpServers.credentialId })
+        .from(mcpServers)
+        .where(eq(mcpServers.id, serverId))
+        .limit(1);
+      if (!row?.credentialId) return null;
 
-    try {
-      const parsed: unknown = JSON.parse(
-        await decryptCredentialForUse(
+      let plaintext: string;
+      try {
+        plaintext = await decryptCredentialForUse(
           encryptionKey,
           credentials,
           row.credentialId,
-        ),
-      );
-      // A client that parsed and is not one is as unusable as the revoked or missing row the catch
-      // below answers for, and is the same none to every caller — see {@link isUsableClient}.
-      return isUsableClient(parsed) ? parsed : null;
-    } catch {
-      // A revoked, missing or unreadable client is the same as none for every caller: there is
-      // nothing to send anybody to consent with, and the answer is to obtain one again.
-      return null;
-    }
+        );
+      } catch (error) {
+        // Only a confirmed missing or revoked credential permits obtaining a replacement. A failed
+        // read or decryption must not turn a working deployment-wide registration into a new client.
+        if (error instanceof CredentialUnusableError) return null;
+        throw error;
+      }
+      return clientFromStoredJson(plaintext);
+    });
   }
 
   /**

server/src/plugins/plugin-store/model.ts

@@ -417,6 +417,14 @@ export type _ProbeOutcomeLeaves = Decides<
   RecordedCheck | "untouched"
 >;
 
+/** A stored-client read failed. Never retains a vault error, parser error, or secret-bearing cause. */
+export class OAuthClientReadError extends Error {
+  constructor() {
+    super("The stored OAuth client could not be read.");
+    this.name = "OAuthClientReadError";
+  }
+}
+
 export class PluginRefusedError extends Error {
   constructor(
     message: string,

server/src/plugins/route-groups/connections.ts

@@ -27,6 +27,7 @@ import { type AccountRef, asAccountRef } from "../shared-accounts";
 import {
   CatalogueEntryUnknownError,
   type OAuthClient,
+  OAuthClientReadError,
   PluginRefusedError,
 } from "../store";
 import {
@@ -880,6 +881,18 @@ export function mountConnectionRoutes({
           409,
         );
       }
+      if (error instanceof OAuthClientReadError) {
+        await store.recordOAuthClientReadFailure({
+          serverId,
+          actorId: context.var.actor.id,
+        });
+        return context.json(
+          {
+            error: `Couldn't connect ${entry.title}. Try again, or ask an admin if it keeps happening.`,
+          },
+          503,
+        );
+      }
       throw error;
     }
     if (!client) {

server/src/plugins/store.ts

@@ -35,6 +35,7 @@ export {
   exchangeRefreshTokenOverHttp,
   INVALID_CLIENT,
   isDeploymentFault,
+  OAuthClientReadError,
   PluginInvariantError,
   PluginRefusedError,
   refFromToolName,