From c705f7203dd5709fe19f9b07bae6bf6b5433dcf9 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 3 Nov 2026 03:12:55 -0800 Subject: [PATCH 1/3] fix(connectors): say the vendor refused our app, not that you declined When a vendor answers the OAuth callback with access_denied because our app is unavailable to the account (Airtable: "cannot be used outside of development"), the member never saw a consent screen. The callback now maps that case to vendor_app_unavailable and passes the entry id, and the Connectors page names the provider from its own catalogue. Vendor text is matched, never echoed: the callback URL is forgeable. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DPFVAooJnqCSwrdxVw --- .../routes/hostedMcpConnect.callback.test.js | 16 +++++++++++ backend/routes/hostedMcpConnect.ts | 16 +++++++++++ frontend/src/i18n/locales/en.json | 2 ++ frontend/src/i18n/locales/zh-CN.json | 2 ++ .../ConnectorsPage.channels.test.tsx | 28 +++++++++++++++++++ .../src/v2/components/hostedMcpErrorNotice.ts | 18 ++++++++++-- .../src/v2/connectors/useChannelConnectors.ts | 17 ++++++++++- 7 files changed, 95 insertions(+), 4 deletions(-) diff --git a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js index f33c9c6037..1b7d4e47f8 100644 --- a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js +++ b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js @@ -1199,6 +1199,22 @@ describe('hosted-mcp connect: callback', () => { expect(Integration.findOne).not.toHaveBeenCalled(); }); + // Airtable refused every account but the client's own with this text (11-02): + // the member had clicked Allow, and "you didn't approve" told them otherwise. + // A known vendor refusal maps to its own fixed code and names the entry, + // never forwarding the vendor's words. + it('maps a vendor refusing the app itself to vendor_app_unavailable and names the entry', async () => { + const description = "This OAuth application cannot be used outside of development.\nPlease check your OAuth application's setting page"; + const res = await callback(`state=st-1&error=access_denied&error_description=${encodeURIComponent(description)}`); + const url = new URL(res.headers.location, 'https://commonly.me'); + expect(res.status).toBe(302); + expect(url.searchParams.get('hostedMcp')).toBe('error'); + expect(url.searchParams.get('code')).toBe('vendor_app_unavailable'); + expect(url.searchParams.get('entryId')).toBe('linear'); + expect(res.headers.location).not.toMatch(/development|setting/i); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + }); + it.each([ ['access_denied', 'consent_denied'], ['temporarily_unavailable', 'vendor_error'], diff --git a/backend/routes/hostedMcpConnect.ts b/backend/routes/hostedMcpConnect.ts index b3aadd4a83..b3d4eecad8 100644 --- a/backend/routes/hostedMcpConnect.ts +++ b/backend/routes/hostedMcpConnect.ts @@ -94,10 +94,13 @@ const callbackRedirect = ( code?: string, options?: { manualRevoke?: { entryId: string; revokeAt: string }; + /** A catalogue entry the page may name in its notice; never vendor text. */ + entryId?: string; }, ) => { const query = new URLSearchParams({ hostedMcp: status }); if (code) query.set('code', code); + if (options?.entryId) query.set('entryId', options.entryId); if (options?.manualRevoke) { query.set('entryId', options.manualRevoke.entryId); query.set('revokeAt', options.manualRevoke.revokeAt); @@ -491,6 +494,10 @@ const settleUnstoredGrant = async ( return undefined; }; +// Vendor descriptions that mean the vendor refused our app for this account, +// whatever the member clicked. Matched, never echoed. +const VENDOR_APP_UNAVAILABLE = /cannot be used outside of development/i; + const finishCallback = async (req: Request, res: Response, flow: CallbackFlow) => { const entry = findHostedMcpEntry(HOSTED_MCP_ENTRIES, req.params.entryId); if (!entry || !isOAuthEntry(entry)) { @@ -502,6 +509,15 @@ const finishCallback = async (req: Request, res: Response, flow: CallbackFlow) = if (vendorError) { // The provider denied authorization before issuing a code. Leave the // pending flow intact so the member can retry; never forward vendor text. + // A refusal of the app itself, not the member's choice, is told apart by + // a known description and gets its own fixed code (Airtable, 11-02). + const description = typeof req.query.error_description === 'string' ? req.query.error_description : ''; + if (vendorError === 'access_denied' && VENDOR_APP_UNAVAILABLE.test(description)) { + // Our app's status at the vendor, not the member's choice: worth a line + // without anyone having to report it. The entry id only, never vendor text. + console.warn('[hosted-mcp] vendor refused the app for this account', JSON.stringify({ entryId: entry.id })); + return callbackRedirect(res, 'error', 'vendor_app_unavailable', { entryId: entry.id }); + } return callbackRedirect(res, 'error', vendorError === 'access_denied' ? 'consent_denied' : 'vendor_error'); } if (!state || !code) { diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 1733876746..a787225b0f 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2490,6 +2490,8 @@ "hostedMcpAuthenticationRequired": "Sign in to Commonly, then try connecting again.", "hostedMcpClientSetup": "This connector’s sign-in setup needs attention. Ask an administrator to check it.", "hostedMcpConsentDenied": "You didn’t approve this connection. Nothing was saved.", + "hostedMcpVendorAppUnavailable": "The service refused Commonly’s app for your account, so nothing was saved. This is a setting on our side, not something you did.", + "hostedMcpVendorAppUnavailableNamed": "{{provider}} refused Commonly’s app for your account, so nothing was saved. This is a setting on our side with {{provider}}, not something you did.", "hostedMcpExchangeIncomplete": "The provider returned an incomplete authorization. Nothing was saved.", "hostedMcpExchangeRefused": "The provider refused this connection. Try again or check its permissions.", "hostedMcpExchangeUnreachable": "Couldn’t reach the provider to finish connecting. Try again.", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 21f6f4f114..7232578557 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -2478,6 +2478,8 @@ "hostedMcpAuthenticationRequired": "请先登录 Commonly，再重新连接。", "hostedMcpClientSetup": "此连接器的登录配置有问题，请联系管理员检查。", "hostedMcpConsentDenied": "你没有同意此次授权。连接未保存。", + "hostedMcpVendorAppUnavailable": "该服务不允许你的账号使用 Commonly 的应用，连接未保存。这是我们这边的设置问题，与你的操作无关。", + "hostedMcpVendorAppUnavailableNamed": "{{provider}} 不允许你的账号使用 Commonly 的应用，连接未保存。这是我们在 {{provider}} 上的设置问题，与你的操作无关。", "hostedMcpExchangeIncomplete": "提供方返回的授权信息不完整。连接未保存。", "hostedMcpExchangeRefused": "提供方拒绝了连接。请重试或检查授权设置。", "hostedMcpExchangeUnreachable": "无法联系提供方以完成连接，请重试。", diff --git a/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx b/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx index 9b7e656f1a..59e245c501 100644 --- a/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx +++ b/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx @@ -661,6 +661,7 @@ describe('Connectors page: channels', () => { ['browser_mismatch', 'hostedMcpInvalidState'], ['issuer_mismatch', 'hostedMcpInvalidState'], ['consent_denied', 'hostedMcpConsentDenied'], + ['vendor_app_unavailable', 'hostedMcpVendorAppUnavailable'], ['vendor_error', 'hostedMcpVendorError'], ['rate_limited', 'hostedMcpRateLimited'], ['issuer_unreachable', 'hostedMcpIssuerUnavailable'], @@ -688,6 +689,33 @@ describe('Connectors page: channels', () => { expect(screen.queryByText(/spoofed/)).toBeNull(); }); + // Airtable refused the app for every account but one (11-02), and the page + // said "You didn't approve". The callback names the entry, and the page names + // the provider only when its own catalogue holds that entry. + it('names the provider when it refused the app for this account', async () => { + window.history.replaceState({}, '', '/v2/connectors?hostedMcp=error&code=vendor_app_unavailable&entryId=linear'); + mockGets([], [{ ...hostedLinearCatalogEntry(), connections: [] }]); + renderPage(); + + const alert = await screen.findByRole('alert'); + await waitFor(() => expect(alert).toHaveTextContent( + en.connectors.hostedMcpVendorAppUnavailableNamed.replace(/\{\{provider\}\}/g, 'Linear'), + )); + expect(window.location.search).toBe(''); + }); + + it('keeps the unnamed notice for an entry its catalogue does not hold', async () => { + window.history.replaceState({}, '', '/v2/connectors?hostedMcp=error&code=vendor_app_unavailable&entryId=Evil%20Corp'); + mockGets([], [{ ...hostedLinearCatalogEntry(), connections: [] }]); + renderPage(); + + const alert = await screen.findByRole('alert'); + expect(alert).toHaveTextContent(en.connectors.hostedMcpVendorAppUnavailable); + await act(async () => { await new Promise((resolve) => { setTimeout(resolve, 0); }); }); + expect(alert).toHaveTextContent(en.connectors.hostedMcpVendorAppUnavailable); + expect(document.body.textContent).not.toContain('Evil Corp'); + }); + it('reloads connectors after a successful Slack callback and clears its query', async () => { window.history.replaceState({}, '', '/v2/connectors?slack=pending'); mockGets([]); diff --git a/frontend/src/v2/components/hostedMcpErrorNotice.ts b/frontend/src/v2/components/hostedMcpErrorNotice.ts index 038d1f143f..95ba2ea5b3 100644 --- a/frontend/src/v2/components/hostedMcpErrorNotice.ts +++ b/frontend/src/v2/components/hostedMcpErrorNotice.ts @@ -10,6 +10,8 @@ const noticeKeyByCode: Record = { browser_mismatch: 'hostedMcpInvalidState', issuer_mismatch: 'hostedMcpInvalidState', consent_denied: 'hostedMcpConsentDenied', + // The vendor refused the app for this account, whatever the member clicked. + vendor_app_unavailable: 'hostedMcpVendorAppUnavailable', vendor_error: 'hostedMcpVendorError', rate_limited: 'hostedMcpRateLimited', issuer_unreachable: 'hostedMcpIssuerUnavailable', @@ -31,6 +33,16 @@ const noticeKeyByCode: Record = { authentication_required: 'hostedMcpAuthenticationRequired', }; -export const hostedMcpErrorNotice = (t: TFunction, code: string | null | undefined): string => ( - t(`connectors.${noticeKeyByCode[code || ''] || 'hostedMcpUnknownError'}`) -); +// Notices that can name the provider. The name comes from this instance's own +// catalogue entry, never from the callback URL. +const namedNoticeKeys = new Set(['hostedMcpVendorAppUnavailable']); + +export const hostedMcpErrorNotice = ( + t: TFunction, + code: string | null | undefined, + provider?: string, +): string => { + const key = noticeKeyByCode[code || ''] || 'hostedMcpUnknownError'; + if (provider && namedNoticeKeys.has(key)) return t(`connectors.${key}Named`, { provider }); + return t(`connectors.${key}`); +}; diff --git a/frontend/src/v2/connectors/useChannelConnectors.ts b/frontend/src/v2/connectors/useChannelConnectors.ts index 11f7d7f7e5..5cada5161a 100644 --- a/frontend/src/v2/connectors/useChannelConnectors.ts +++ b/frontend/src/v2/connectors/useChannelConnectors.ts @@ -73,6 +73,8 @@ export const useChannelConnectors = () => { // what the callback URL claimed; `hostedMcpCallbackRevoke` is set only once // the catalogue confirms it. const [pendingCallbackRevoke, setPendingCallbackRevoke] = useState<{ entryId: string; revokeAt: string } | null>(null); + // A callback notice that may name its provider once the catalogue confirms the entry. + const [pendingCallbackProvider, setPendingCallbackProvider] = useState<{ entryId: string; code: string } | null>(null); const [hostedMcpCallbackRevoke, setHostedMcpCallbackRevoke] = useState<{ entryId: string; provider: string; revokePage: string; } | null>(null); @@ -143,10 +145,12 @@ export const useChannelConnectors = () => { useEffect(() => { const query = new URLSearchParams(window.location.search); if (query.get('hostedMcp') !== 'error') return; - setHostedMcpCallbackError(hostedMcpErrorNotice(t, query.get('code'))); + const code = query.get('code'); + setHostedMcpCallbackError(hostedMcpErrorNotice(t, code)); const entryId = query.get('entryId'); const revokeAt = query.get('revokeAt'); if (entryId && revokeAt) setPendingCallbackRevoke({ entryId, revokeAt }); + if (entryId && code === 'vendor_app_unavailable') setPendingCallbackProvider({ entryId, code }); query.delete('hostedMcp'); query.delete('code'); query.delete('entryId'); @@ -185,6 +189,17 @@ export const useChannelConnectors = () => { ); }, [load, t]); + // The provider is named only from the catalogue's own entry, so a forged + // entryId leaves the unnamed notice as it is. + useEffect(() => { + if (!pendingCallbackProvider || loading || hostedCatalog === null) return; + const entry = hostedCatalog.find((candidate) => candidate.entryId === pendingCallbackProvider.entryId); + if (entry?.label) { + setHostedMcpCallbackError(hostedMcpErrorNotice(t, pendingCallbackProvider.code, entry.label)); + } + setPendingCallbackProvider(null); + }, [hostedCatalog, loading, pendingCallbackProvider, t]); + // A refused connect has no row to read, so the catalogue entry is the // authority: the page is linked only when it is the one the entry names. useEffect(() => { From 175558f8d05e7984f90916440dfa586b8066d58d Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 3 Nov 2026 03:21:56 -0800 Subject: [PATCH 2/3] test(connectors): pin the vendor-refusal warn line and keep vendor text out of it Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DPFVAooJnqCSwrdxVw --- .../unit/routes/hostedMcpConnect.callback.test.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js index 1b7d4e47f8..f6e94d3956 100644 --- a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js +++ b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js @@ -1204,8 +1204,17 @@ describe('hosted-mcp connect: callback', () => { // A known vendor refusal maps to its own fixed code and names the entry, // never forwarding the vendor's words. it('maps a vendor refusing the app itself to vendor_app_unavailable and names the entry', async () => { - const description = "This OAuth application cannot be used outside of development.\nPlease check your OAuth application's setting page"; - const res = await callback(`state=st-1&error=access_denied&error_description=${encodeURIComponent(description)}`); + const description = "This OAuth application cannot be used outside of development.\nPlease check your OAuth application's setting page VENDOR-SENTINEL"; + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + let res; + try { + res = await callback(`state=st-1&error=access_denied&error_description=${encodeURIComponent(description)}`); + // The entry id only: vendor text never reaches the log. + expect(warn).toHaveBeenCalledWith('[hosted-mcp] vendor refused the app for this account', JSON.stringify({ entryId: 'linear' })); + expect(JSON.stringify(warn.mock.calls)).not.toContain('VENDOR-SENTINEL'); + } finally { + warn.mockRestore(); + } const url = new URL(res.headers.location, 'https://commonly.me'); expect(res.status).toBe(302); expect(url.searchParams.get('hostedMcp')).toBe('error'); From a313a221d694f8bc12272773c812339c94f44e15 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 3 Nov 2026 03:28:35 -0800 Subject: [PATCH 3/3] test(connectors): the vendor sentinel stays out of the redirect too Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MDs9DPFVAooJnqCSwrdxVw --- backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js index f6e94d3956..65bcb5e286 100644 --- a/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js +++ b/backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js @@ -1221,6 +1221,7 @@ describe('hosted-mcp connect: callback', () => { expect(url.searchParams.get('code')).toBe('vendor_app_unavailable'); expect(url.searchParams.get('entryId')).toBe('linear'); expect(res.headers.location).not.toMatch(/development|setting/i); + expect(res.headers.location).not.toContain('VENDOR-SENTINEL'); expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); });