diff --git a/packages/worker/src/integrations/platform-apps.node.test.ts b/packages/worker/src/integrations/platform-apps.node.test.ts index c1cefa10ca..a5856fcf76 100644 --- a/packages/worker/src/integrations/platform-apps.node.test.ts +++ b/packages/worker/src/integrations/platform-apps.node.test.ts @@ -9,6 +9,7 @@ import { getPlatformOauthAppClientSecret, listPlatformOauthApps, listTopPlatformAppsByUse, + PlatformOauthAppValidationError, upsertPlatformOauthApp, } from './platform-apps.ts' @@ -155,14 +156,15 @@ test('partial saves retain omitted fields instead of clearing them', async () => test('confidential flow requires a client secret only while enabled', async () => { const { db, env } = createHarness() - // Enabled (default) without a secret is rejected. + // Enabled (default) without a secret is rejected as a validation error + // (MCP re-wraps this as McpCallerError so it stays off Sentry). await expect( upsertPlatformOauthApp({ db, env, app: { ...baseGithubApp, clientSecret: null }, }), - ).rejects.toThrow('Confidential flow requires a client secret') + ).rejects.toBeInstanceOf(PlatformOauthAppValidationError) // Staged provisioning: a disabled app saves without a secret so an // operator can paste credentials later. @@ -190,7 +192,7 @@ test('confidential flow requires a client secret only while enabled', async () = enabled: true, }, }), - ).rejects.toThrow('Confidential flow requires a client secret') + ).rejects.toBeInstanceOf(PlatformOauthAppValidationError) // Once the secret lands, enabling works. const live = await upsertPlatformOauthApp({ diff --git a/packages/worker/src/integrations/platform-apps.ts b/packages/worker/src/integrations/platform-apps.ts index d07ff19597..14f18f9a00 100644 --- a/packages/worker/src/integrations/platform-apps.ts +++ b/packages/worker/src/integrations/platform-apps.ts @@ -9,6 +9,19 @@ import { type tokenExchangeStyleValues, } from '#mcp/capabilities/integrations/integration-shared.ts' +/** + * Operator-fixable input rejection while saving a platform OAuth app + * (missing required fields, enabling a confidential app without a secret). + * MCP capabilities re-wrap this as `McpCallerError` so agent staging mistakes + * stay on `mcp-event` lines and out of Sentry. + */ +export class PlatformOauthAppValidationError extends Error { + constructor(message: string) { + super(message) + this.name = 'PlatformOauthAppValidationError' + } +} + /** * Operator-provisioned built-in OAuth apps shared across every user. * @@ -170,16 +183,22 @@ export async function upsertPlatformOauthApp(input: { }): Promise { const slug = canonicalIntegrationName(input.app.slug) if (!slug) { - throw new Error('Platform app slug must contain letters or numbers.') + throw new PlatformOauthAppValidationError( + 'Platform app slug must contain letters or numbers.', + ) } const clientId = input.app.clientId.trim() if (!clientId) { - throw new Error('Client id is required.') + throw new PlatformOauthAppValidationError('Client id is required.') } const tokenUrl = input.app.tokenUrl.trim() const authorizeUrl = input.app.authorizeUrl.trim() - if (!tokenUrl) throw new Error('Token URL is required.') - if (!authorizeUrl) throw new Error('Authorize URL is required.') + if (!tokenUrl) { + throw new PlatformOauthAppValidationError('Token URL is required.') + } + if (!authorizeUrl) { + throw new PlatformOauthAppValidationError('Authorize URL is required.') + } const existing = await getPlatformOauthAppRowBySlug({ db: input.db, slug }) const clientSecretEncrypted = @@ -202,7 +221,7 @@ export async function upsertPlatformOauthApp(input: { // enables) later; the secret becomes mandatory the moment the app is // reachable by users. if (enabled && input.app.flow === 'confidential' && !clientSecretEncrypted) { - throw new Error( + throw new PlatformOauthAppValidationError( 'Confidential flow requires a client secret before the app can be enabled. Save it with enabled: false to stage the config first.', ) } diff --git a/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts b/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts index 21ab0103f6..c245810ecf 100644 --- a/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts +++ b/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts @@ -1,5 +1,6 @@ import { DatabaseSync } from 'node:sqlite' import { expect, test, vi } from 'vitest' +import { McpCallerError } from '#mcp/caller-error.ts' import { createMcpCallerContext } from '#mcp/context.ts' // These tests assert real `audit_events` rows written through the actual @@ -152,3 +153,58 @@ test('save keeps the stored secret on update and can disable an app', async () = }), ).resolves.toBe('platform-github-client-secret-value') }) + +test('enabling a confidential app without a client secret is an McpCallerError', async () => { + const { ctx, auditSqlite } = createHarness() + await expect( + adminPlatformOauthAppSaveCapability.handler( + { + slug: 'github', + clientId: saveInput.clientId, + clientSecret: null, + tokenUrl: saveInput.tokenUrl, + authorizeUrl: saveInput.authorizeUrl, + flow: 'confidential', + }, + ctx, + ), + ).rejects.toBeInstanceOf(McpCallerError) + + const staged = await adminPlatformOauthAppSaveCapability.handler( + { + slug: 'github', + clientId: saveInput.clientId, + clientSecret: null, + tokenUrl: saveInput.tokenUrl, + authorizeUrl: saveInput.authorizeUrl, + flow: 'confidential', + enabled: false, + }, + ctx, + ) + expect(staged.app.enabled).toBe(false) + + await expect( + adminPlatformOauthAppSaveCapability.handler( + { + slug: 'github', + clientId: saveInput.clientId, + tokenUrl: saveInput.tokenUrl, + authorizeUrl: saveInput.authorizeUrl, + flow: 'confidential', + enabled: true, + }, + ctx, + ), + ).rejects.toBeInstanceOf(McpCallerError) + + const failures = auditSqlite + .prepare( + `SELECT action, result FROM audit_events WHERE result = 'failure' ORDER BY id ASC`, + ) + .all() as Array<{ action: string; result: string }> + expect(failures).toEqual([ + { action: 'admin_platform_oauth_app_save', result: 'failure' }, + { action: 'admin_platform_oauth_app_save', result: 'failure' }, + ]) +}) diff --git a/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts b/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts index bfd8edbbc7..e6240e2352 100644 --- a/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts +++ b/packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { McpCallerError } from '#mcp/caller-error.ts' import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { @@ -11,7 +12,10 @@ import { } from '#mcp/capabilities/integrations/platform-app-shared.ts' import { base64ToBytes } from '@kody-internal/shared/base64.ts' import { setPlatformOauthAppLogo } from '#worker/integrations/platform-app-logo.ts' -import { upsertPlatformOauthApp } from '#worker/integrations/platform-apps.ts' +import { + PlatformOauthAppValidationError, + upsertPlatformOauthApp, +} from '#worker/integrations/platform-apps.ts' import { adminMutationCapabilityAccess, auditAdminCapabilityInvocation, @@ -94,44 +98,54 @@ export const adminPlatformOauthAppSaveCapability = defineDomainCapability( ctx, 'admin_platform_oauth_app_save', async () => { - // Omitted optional fields stay `undefined` so the upsert's - // retain-on-omit semantics apply: a partial save never - // silently clears the scope menu, hosts, or stored secret. - let app = await upsertPlatformOauthApp({ - db: ctx.env.APP_DB, - env: ctx.env, - app: { - slug: args.slug, - provider: args.provider, - label: args.label, - clientId: args.clientId, - clientSecret: args.clientSecret, - tokenUrl: args.tokenUrl, - authorizeUrl: args.authorizeUrl, - apiBaseUrl: args.apiBaseUrl, - flow: args.flow, - usePkce: args.usePkce, - tokenExchangeStyle: args.tokenExchangeStyle, - scopeSeparator: args.scopeSeparator, - extraAuthorizeParams: args.extraAuthorizeParams, - allowedScopes: args.allowedScopes, - defaultScopes: args.defaultScopes, - requiredHosts: args.requiredHosts, - enabled: args.enabled, - }, - }) - if (args.logoBase64 !== undefined) { - app = await setPlatformOauthAppLogo({ + try { + // Omitted optional fields stay `undefined` so the upsert's + // retain-on-omit semantics apply: a partial save never + // silently clears the scope menu, hosts, or stored secret. + let app = await upsertPlatformOauthApp({ db: ctx.env.APP_DB, env: ctx.env, - slug: app.slug, - sourceBytes: - args.logoBase64 === null - ? null - : base64ToBytes(args.logoBase64), + app: { + slug: args.slug, + provider: args.provider, + label: args.label, + clientId: args.clientId, + clientSecret: args.clientSecret, + tokenUrl: args.tokenUrl, + authorizeUrl: args.authorizeUrl, + apiBaseUrl: args.apiBaseUrl, + flow: args.flow, + usePkce: args.usePkce, + tokenExchangeStyle: args.tokenExchangeStyle, + scopeSeparator: args.scopeSeparator, + extraAuthorizeParams: args.extraAuthorizeParams, + allowedScopes: args.allowedScopes, + defaultScopes: args.defaultScopes, + requiredHosts: args.requiredHosts, + enabled: args.enabled, + }, }) + if (args.logoBase64 !== undefined) { + app = await setPlatformOauthAppLogo({ + db: ctx.env.APP_DB, + env: ctx.env, + slug: app.slug, + sourceBytes: + args.logoBase64 === null + ? null + : base64ToBytes(args.logoBase64), + }) + } + return { app: toPlatformOauthAppPublic(app) } + } catch (error) { + // Staging/enable mistakes (secretless confidential while + // enabled, empty required fields) are caller-clearable — + // keep them off Sentry via McpCallerError. + if (error instanceof PlatformOauthAppValidationError) { + throw new McpCallerError(error.message, { cause: error }) + } + throw error } - return { app: toPlatformOauthAppPublic(app) } }, { successReason: ({ app }) => `platform_oauth_app=${app.slug}`,