From f43e807bc958dd9d1570d64c3a3883be5a7a976a Mon Sep 17 00:00:00 2001 From: "helpcode.ai" <340500827+helpcodeai@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:44:50 +0200 Subject: [PATCH] Re-authorizing asks for scopes the catalog added later; hints for missing scopes and unknown Odoo fields; clearer /mcp refusal for unverified email; one duplicate-name log per hour - Etsy connectors installed before the write tools kept the old read-only scopes in authConfig (116 of 364 on Cloud), so re-authorizing could never grant listings_w. An adapter can now list the scope sets it used to ask for (previousOAuthScopes); a connector holding exactly one of them asks for the current scopes on its next authorization. Hand-picked subsets are kept. - Tool errors that name a missing scope tell the model to have the user click Authorize with Provider again; unknown Odoo fields and models point at the field list (7 workspaces in 16 h). - An unverified user calling /mcp now reads what to do (confirm the email or sign in for a new link) instead of a bare "Email verification required". - "Duplicate tool name" is logged once an hour per server and name (1,880 lines in 16 h from one polling client). --- packages/backend/src/adapters/catalog.ts | 5 +++ packages/backend/src/adapters/intl/etsy.json | 1 + .../src/auth/email-verified.guard.spec.ts | 9 ++++- .../backend/src/auth/email-verified.guard.ts | 9 +++++ .../src/connectors/connectors.controller.ts | 6 ++-- .../oauth-authorize-settings.spec.ts | 23 +++++++++++++ .../connectors/oauth-authorize-settings.ts | 33 ++++++++++++++++++ .../src/mcp-server/error-hints.spec.ts | 25 ++++++++++++++ .../backend/src/mcp-server/error-hints.ts | 34 +++++++++++++++++++ .../src/mcp-server/mcp-endpoint.controller.ts | 20 +++++++++-- scripts/validate-adapters.mjs | 3 ++ 11 files changed, 161 insertions(+), 7 deletions(-) diff --git a/packages/backend/src/adapters/catalog.ts b/packages/backend/src/adapters/catalog.ts index af0bb66d..41b3e290 100644 --- a/packages/backend/src/adapters/catalog.ts +++ b/packages/backend/src/adapters/catalog.ts @@ -421,6 +421,11 @@ export interface AdapterMeta { * through the user's own app. A chat setup links it next to the callback * URL to register. */ appRegistrationUrl?: string; + /** OAuth scope sets this adapter asked for in earlier versions. A connector + * still holding one of them asks for the current `authConfig.scopes` the + * next time it is authorized (oauth-authorize-settings.ts), so tools added + * later get the permissions they need. */ + previousOAuthScopes?: string[]; /** What to tell the user when the check fails, keyed by HTTP status ("404") * or failure kind ("auth_failed"); replaces the generic hint. `suggest` * names another adapter to offer instead (e.g. the JSON-RPC Odoo for an diff --git a/packages/backend/src/adapters/intl/etsy.json b/packages/backend/src/adapters/intl/etsy.json index d1afe5d3..04903cff 100644 --- a/packages/backend/src/adapters/intl/etsy.json +++ b/packages/backend/src/adapters/intl/etsy.json @@ -14,6 +14,7 @@ "optionalEnvVars": [ "ETSY_REFRESH_TOKEN" ], + "previousOAuthScopes": ["email_r shops_r listings_r transactions_r"], "appRegistrationUrl": "https://www.etsy.com/developers/register", "envVarMeta": { "ETSY_CLIENT_ID": { diff --git a/packages/backend/src/auth/email-verified.guard.spec.ts b/packages/backend/src/auth/email-verified.guard.spec.ts index 20b94b6f..59a93c3c 100644 --- a/packages/backend/src/auth/email-verified.guard.spec.ts +++ b/packages/backend/src/auth/email-verified.guard.spec.ts @@ -17,7 +17,7 @@ describe('EmailVerifiedGuard', () => { }), }; const guard = new EmailVerifiedGuard( - { get: () => (opts.cloud === false ? 'self-hosted' : 'cloud') } as any, + { get: (k: string) => (k === 'DEPLOYMENT_MODE' ? (opts.cloud === false ? 'self-hosted' : 'cloud') : undefined) } as any, prisma as any, auth as any, ); @@ -32,6 +32,13 @@ describe('EmailVerifiedGuard', () => { ).rejects.toBeInstanceOf(ForbiddenException); }); + it('tells an AI client on /mcp what the person has to do', async () => { + const { guard } = makeGuard({ verified: false }); + const refusal = guard.canActivate(ctx({ path: '/mcp/srv1', headers: { authorization: 'Bearer good' } })); + await expect(refusal).rejects.toBeInstanceOf(ForbiddenException); + await expect(refusal).rejects.toThrow(/confirm the email address.*sign in at https:\/\//); + }); + it('lets a verified user through', async () => { const { guard } = makeGuard({ verified: true }); await expect( diff --git a/packages/backend/src/auth/email-verified.guard.ts b/packages/backend/src/auth/email-verified.guard.ts index 753fdf01..2b4a8d75 100644 --- a/packages/backend/src/auth/email-verified.guard.ts +++ b/packages/backend/src/auth/email-verified.guard.ts @@ -85,6 +85,15 @@ export class EmailVerifiedGuard implements CanActivate { if (!dbUser.emailVerified) { if (path.startsWith('/mcp')) { this.logger.warn(`Refused ${path}: user ${userId} has not verified their email`); + // An AI client shows this text to the person, who otherwise only sees + // the connection fail: on 9-10 Oct 2026 seven people who had connected + // Claude retried up to 122 times without knowing what was missing. + const dashboard = (this.configService.get('FRONTEND_URL') || 'https://cloud.anythingmcp.com').replace(/\/+$/, ''); + throw new ForbiddenException( + 'Email verification required: confirm the email address of your AnythingMCP account ' + + `with the link we sent when you signed up, or sign in at ${dashboard} to get a new ` + + 'link. Then try again; nothing else needs to change.', + ); } throw new ForbiddenException('Email verification required'); } diff --git a/packages/backend/src/connectors/connectors.controller.ts b/packages/backend/src/connectors/connectors.controller.ts index 2f379c88..98a0ecf6 100644 --- a/packages/backend/src/connectors/connectors.controller.ts +++ b/packages/backend/src/connectors/connectors.controller.ts @@ -1316,13 +1316,13 @@ export class ConnectorsController { // endpoints where the row has none. const adapterSlug = (connector.config as { adapterSlug?: string } | null) ?.adapterSlug; - const catalogAuth = adapterSlug - ? getAdapter(adapterSlug)?.connector.authConfig - : undefined; + const catalogAdapter = adapterSlug ? getAdapter(adapterSlug) : undefined; + const catalogAuth = catalogAdapter?.connector.authConfig; const settings = resolveRestAuthorizeSettings( authConfig, connector.envVars as Record | null, catalogAuth, + catalogAdapter?.previousOAuthScopes, ); if (settings.missingVars.length > 0) { return { diff --git a/packages/backend/src/connectors/oauth-authorize-settings.spec.ts b/packages/backend/src/connectors/oauth-authorize-settings.spec.ts index aff179db..b83c4992 100644 --- a/packages/backend/src/connectors/oauth-authorize-settings.spec.ts +++ b/packages/backend/src/connectors/oauth-authorize-settings.spec.ts @@ -31,6 +31,29 @@ describe('resolveRestAuthorizeSettings', () => { }); }); + it('asks for the scopes the catalog added since a connector was installed', () => { + const previous = getAdapter('etsy')!.previousOAuthScopes; + const row = { + clientId: 'cid', + clientSecret: 'sec', + authorizationUrl: etsyCatalog.authorizationUrl, + tokenUrl: etsyCatalog.tokenUrl, + scopes: 'listings_r shops_r transactions_r email_r', + }; + const settings = resolveRestAuthorizeSettings(row, null, etsyCatalog, previous); + expect(settings.scope).toBe(etsyCatalog.scopes); + // Written back once the authorization succeeds, like every adopted value. + expect(settings.adopted).toEqual({ scopes: etsyCatalog.scopes }); + + // A hand-picked subset is not one of the adapter's earlier sets: kept. + const narrowed = resolveRestAuthorizeSettings({ ...row, scopes: 'listings_r' }, null, etsyCatalog, previous); + expect(narrowed.scope).toBe('listings_r'); + expect(narrowed.adopted).toEqual({}); + + // Without a history on the adapter nothing is widened. + expect(resolveRestAuthorizeSettings(row, null, etsyCatalog).adopted).toEqual({}); + }); + it('resolves placeholders from the env vars', () => { const settings = resolveRestAuthorizeSettings( { diff --git a/packages/backend/src/connectors/oauth-authorize-settings.ts b/packages/backend/src/connectors/oauth-authorize-settings.ts index f391d2f1..5b98b485 100644 --- a/packages/backend/src/connectors/oauth-authorize-settings.ts +++ b/packages/backend/src/connectors/oauth-authorize-settings.ts @@ -62,6 +62,8 @@ export function resolveRestAuthorizeSettings( storedAuthConfig: Record, envVars: Record | null | undefined, catalogAuthConfig?: Record | null, + /** The adapter's `previousOAuthScopes`: scope sets it asked for before. */ + previousScopes?: readonly string[], ): RestAuthorizeSettings { const cfg = interpolateDeep(storedAuthConfig, envVars ?? {}); @@ -84,6 +86,21 @@ export function resolveRestAuthorizeSettings( } } + // 3. Scopes the catalog added later. A catalog connector keeps the scopes it + // was installed with, and the re-sync never touches authConfig, so when + // an adapter gains write tools (Etsy's listings_w, Oct 2026) the + // connector's token can never be granted them: re-authorizing asks for + // the old list again. An adapter that widens its scopes lists the sets it + // used to ask for (`previousOAuthScopes`); a row holding exactly one of + // them was never narrowed by hand, so it asks for the current set. Any + // other row, a hand-picked subset included, is left as it is. + const widened = widenedScopes( + str(cfg.scopes), + catalogAuthConfig ? str(catalogAuthConfig.scopes) : undefined, + previousScopes, + ); + if (widened && !adopted.scopes) adopted.scopes = widened; + const merged = { ...cfg, ...adopted }; const clientId = str(merged.clientId) ?? ''; const clientSecret = str(merged.clientSecret); @@ -113,3 +130,19 @@ export function resolveRestAuthorizeSettings( ), }; } + +/** + * The catalog's current scopes when the row holds exactly one of the sets the + * adapter used to ask for (order and separators aside), otherwise undefined. + */ +function widenedScopes( + row: string | undefined, + catalog: string | undefined, + previous: readonly string[] | undefined, +): string | undefined { + if (!row || !catalog || !previous?.length) return undefined; + if (findUnresolvedPlaceholders(catalog).length > 0) return undefined; + const key = (v: string) => v.split(/[\s,]+/).filter(Boolean).sort().join(' '); + if (key(row) === key(catalog)) return undefined; + return previous.some((p) => key(p) === key(row)) ? catalog : undefined; +} diff --git a/packages/backend/src/mcp-server/error-hints.spec.ts b/packages/backend/src/mcp-server/error-hints.spec.ts index 44b70150..58bc1b76 100644 --- a/packages/backend/src/mcp-server/error-hints.spec.ts +++ b/packages/backend/src/mcp-server/error-hints.spec.ts @@ -7,6 +7,31 @@ describe('deriveErrorHint', () => { ).toBeUndefined(); }); + it('tells the model to have the connector re-authorized when a scope is missing', () => { + const hint = deriveErrorHint({ + host: 'openapi.etsy.com', + status: 403, + body: { error: 'Access token lacks scope for this request (requires scope: listings_w).' }, + }); + expect(hint).toMatch(/Authorize with Provider/); + expect(deriveErrorHint({ status: 403, body: { error: 'insufficient_scope' } })).toMatch(/Authorize/); + }); + + it('sends the model to the field list on an unknown Odoo field or model', () => { + const field = deriveErrorHint({ + host: 'acme.odoo.com', + status: 500, + body: { name: 'builtins.ValueError', message: "Invalid field 'expense_policy' on 'product.product'" }, + }); + expect(field).toMatch(/fields_get/); + expect( + deriveErrorHint({ message: "JSON-RPC error 200: Odoo Server Error: Invalid field sale.order.commercial_partner_id in leaf ('x')" }), + ).toMatch(/fields_get/); + expect( + deriveErrorHint({ message: "JSON-RPC error 200: Odoo Server Error: Object stock.production.lot doesn't exist" }), + ).toMatch(/stock\.lot/); + }); + describe('weclapp', () => { it('explains unknown properties instead of letting the model guess spellings', () => { const hint = deriveErrorHint({ diff --git a/packages/backend/src/mcp-server/error-hints.ts b/packages/backend/src/mcp-server/error-hints.ts index 28105d50..d19221f1 100644 --- a/packages/backend/src/mcp-server/error-hints.ts +++ b/packages/backend/src/mcp-server/error-hints.ts @@ -27,6 +27,21 @@ export interface ErrorHintInput { body?: unknown; } +const MISSING_SCOPE_HINT = + 'The connector was authorized without a permission this tool needs (the error names it). ' + + 'Retrying will not help: ask the user to open this connector in AnythingMCP and click ' + + '"Authorize with Provider" again, which requests the missing permission, then retry.'; + +const ODOO_FIELD_HINT = + 'That field does not exist on this model in this Odoo database; field names differ between ' + + 'Odoo versions and installed apps. Do not guess another spelling: list the model\'s fields ' + + 'first (odoo_fields_get, or fields_get on the model) and use only the names it returns.'; + +const ODOO_MODEL_HINT = + 'That model does not exist in this Odoo database: it was renamed in newer versions or its ' + + 'app is not installed (for example stock.production.lot is stock.lot since Odoo 16). Check ' + + 'the model name before retrying, e.g. search ir.model by name.'; + const ETSY_PROPERTY_NAME_HINT = 'Etsy needs every entry of property_values complete: property_id, property_name, ' + 'scale_id (when the listing has one), value_ids and values. Read the listing with ' + @@ -187,6 +202,25 @@ export function deriveErrorHint(input: ErrorHintInput): string | undefined { return SQL_SYNTAX_HINT; } + // OAuth tokens granted before an adapter gained a scope (Etsy's listings_w + // when its write tools arrived). Wording differs per vendor; the fix is the + // same: re-authorize, which now asks for the catalog's current scopes. + if ( + (input.status === 401 || input.status === 403 || input.status === undefined) && + /lacks scope|insufficient[_ ]scope|requires? scope|missing (the )?(required )?scopes?|scope is not granted/i.test(text) + ) { + return MISSING_SCOPE_HINT; + } + + // Odoo answers an unknown field or model with a 500 ValueError (REST) or a + // JSON-RPC "Odoo Server Error", and the host is the customer's own domain. + if (/Invalid field '[^']+' on '[^']+'|Invalid field [\w.]+ in leaf|Invalid field '[^']+' on model/i.test(text)) { + return ODOO_FIELD_HINT; + } + if (/Object [\w.]+ doesn't exist|Model not found: [\w.]+/i.test(text)) { + return ODOO_MODEL_HINT; + } + if (hostMatches(input.host, 'weclapp.com')) { if (/unknown property|unexpected filter property/i.test(text)) { return WECLAPP_FIELD_HINT; diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts index 4c1845d7..df070ae3 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts @@ -184,6 +184,16 @@ const SKILL_INSTRUCTION_MAX = 1800; @UseGuards(McpCombinedAuthGuard, McpPrincipalRateLimitGuard) export class McpEndpointController { private readonly logger = new Logger(McpEndpointController.name); + /** Last time a duplicate tool name was logged, per `server:name`. */ + private readonly duplicateWarnedAt = new Map(); + + private shouldWarnDuplicate(key: string, now = Date.now()): boolean { + const last = this.duplicateWarnedAt.get(key); + if (last !== undefined && now - last < 60 * 60 * 1000) return false; + if (this.duplicateWarnedAt.size > 5000) this.duplicateWarnedAt.clear(); + this.duplicateWarnedAt.set(key, now); + return true; + } constructor( private readonly mcpServersService: McpServersService, @@ -1440,9 +1450,13 @@ export class McpEndpointController { // connector assigned twice, or two configs of one provider); the SDK // throws on the second registration, which would 500 the whole request. if (registeredNames.has(tool.name)) { - this.logger.warn( - `Duplicate tool name "${tool.name}" on server ${invocationContext.mcpServerId} — skipping the extra copy (check for duplicate connector assignments).`, - ); + // Once an hour per server and name: the tool set is planned on every + // request, and one polling client turned this into 1,880 lines in 16 h. + if (this.shouldWarnDuplicate(`${invocationContext.mcpServerId}:${tool.name}`)) { + this.logger.warn( + `Duplicate tool name "${tool.name}" on server ${invocationContext.mcpServerId} — skipping the extra copy (check for duplicate connector assignments).`, + ); + } continue; } registeredNames.add(tool.name); diff --git a/scripts/validate-adapters.mjs b/scripts/validate-adapters.mjs index 37db2b11..a39858f0 100644 --- a/scripts/validate-adapters.mjs +++ b/scripts/validate-adapters.mjs @@ -313,6 +313,9 @@ export function validateAdapter(adapter, file, region) { if (adapter.appRegistrationUrl !== undefined && !/^https:\/\/\S+$/.test(String(adapter.appRegistrationUrl))) { errors.push(error('app-registration-url', 'appRegistrationUrl', 'appRegistrationUrl must be an https URL', 'Link the provider page where the user creates the app, or remove the field.', 'adapter-fields')); } + if (adapter.previousOAuthScopes !== undefined && (!Array.isArray(adapter.previousOAuthScopes) || adapter.previousOAuthScopes.some((v) => typeof v !== 'string' || !v.trim()))) { + errors.push(error('previous-oauth-scopes', 'previousOAuthScopes', 'previousOAuthScopes must be an array of scope strings', 'List the scope strings earlier versions of this adapter asked for.', 'adapter-fields')); + } if (adapter.verifyHints !== undefined) { const hints = adapter.verifyHints; if (!hints || typeof hints !== 'object' || Array.isArray(hints)) {