diff --git a/clients/web/src/test/core/mcp/serverList.test.ts b/clients/web/src/test/core/mcp/serverList.test.ts index 824419b5..c6dc9436 100644 --- a/clients/web/src/test/core/mcp/serverList.test.ts +++ b/clients/web/src/test/core/mcp/serverList.test.ts @@ -925,6 +925,27 @@ describe("extractSecretsFromStored", () => { expect(stripped.oauth).toEqual({ enterpriseManaged: true }); }); + // The stripped entry now subtracts the secret instead of enumerating the + // fields to keep, so a field added to `oauth` later survives by construction. + // `onInsufficientScope` is the case the old allow-list had already lost. + it("preserves oauth.onInsufficientScope when lifting clientSecret to keychain", () => { + const stored: StoredMCPServer = { + type: "streamable-http", + url: "https://x.test", + oauth: { + clientId: "cid", + clientSecret: "shh", + onInsufficientScope: "throw", + }, + }; + const { stripped, secrets } = extractSecretsFromStored(stored); + expect(secrets).toEqual({ [SECRET_FIELD_OAUTH_CLIENT_SECRET]: "shh" }); + expect(stripped.oauth).toEqual({ + clientId: "cid", + onInsufficientScope: "throw", + }); + }); + it("preserves oauth.authorizationParams when lifting clientSecret to keychain", () => { const stored: StoredMCPServer = { type: "streamable-http", diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index 0c3c759e..c6e5e507 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -642,23 +642,13 @@ export function extractSecretsFromStored( if (stored.oauth?.clientSecret) { secrets[SECRET_FIELD_OAUTH_CLIENT_SECRET] = stored.oauth.clientSecret; - const restOauth: { - clientId?: string; - scopes?: string; - authorizationParams?: Record; - enterpriseManaged?: boolean; - } = {}; - if (stored.oauth.clientId !== undefined) - restOauth.clientId = stored.oauth.clientId; - if (stored.oauth.scopes !== undefined) - restOauth.scopes = stored.oauth.scopes; - // Not a secret — the authorization parameters stay on the stripped entry so - // stripping the client secret doesn't silently discard them. (#2018) - if (stored.oauth.authorizationParams !== undefined) - restOauth.authorizationParams = stored.oauth.authorizationParams; - if (stored.oauth.enterpriseManaged === true) { - restOauth.enterpriseManaged = true; - } + // Keep every OAuth field except the one secret, rather than enumerating the + // non-secret ones. An allow-list here is a standing trap: it silently drops + // any field added to `oauth` afterwards, and it had already done so — + // `onInsufficientScope` was lost whenever a server carried both a client + // secret and a non-default SEP-2350 policy. Subtracting the secret instead + // means a future field is preserved by construction. (#2018) + const { clientSecret: _clientSecret, ...restOauth } = stored.oauth; if (Object.keys(restOauth).length > 0) { stripped.oauth = restOauth; } else {