diff --git a/docs/guides/oauth.md b/docs/guides/oauth.md index e9491106f1..bb3f14ebf9 100644 --- a/docs/guides/oauth.md +++ b/docs/guides/oauth.md @@ -99,6 +99,12 @@ and the current client credentials. ## Integration naming convention +Integration identity is the canonical provider key: names are normalized to +lowercase kebab (letters, numbers, `.`, `_`, `-`) on every save and lookup, so +`GitHub`, `github`, and `Git Hub` all resolve to the same `github` record. There +is exactly one stored value per integration, keyed +`_integration:`. + Prefer integration names like `-` when multiple accounts may exist: `google` for a default account, `google-business` for a business account, or `google-youtube-brand` for a brand identity. Agents should call diff --git a/packages/worker/client/routes/connect-oauth.node.test.ts b/packages/worker/client/routes/connect-oauth.node.test.ts index 8b10da2caf..b128dd6f5e 100644 --- a/packages/worker/client/routes/connect-oauth.node.test.ts +++ b/packages/worker/client/routes/connect-oauth.node.test.ts @@ -2,7 +2,6 @@ import { expect, test } from 'vitest' import { buildIntegrationValueName, formatOAuthExchangeFailure, - getIntegrationValueCandidates, isOAuthExchangeSessionExpired, mergeConnectOauthConfig, parseSessionConnectOauthConfig, @@ -45,13 +44,12 @@ test('connect OAuth helpers parse stored integrations, merge reconnect configs, requiredHosts: ['api.github.com', 'github.com'], authorization: null, }) - expect(getIntegrationValueCandidates('GitHub', 'github')).toEqual([ - buildIntegrationValueName('GitHub'), - buildIntegrationValueName('github'), - ]) - expect(getIntegrationValueCandidates('github', 'github')).toEqual([ - buildIntegrationValueName('github'), - ]) + // One canonical value key regardless of how the caller cases the provider. + expect(buildIntegrationValueName('GitHub')).toBe('_integration:github') + expect(buildIntegrationValueName('github')).toBe('_integration:github') + expect(buildIntegrationValueName('Spotify Family')).toBe( + '_integration:spotify-family', + ) const githubConfig = mergeConnectOauthConfig({ queryConfig: { diff --git a/packages/worker/client/routes/connect-oauth.tsx b/packages/worker/client/routes/connect-oauth.tsx index f0f867b1a3..a3394ab6dd 100644 --- a/packages/worker/client/routes/connect-oauth.tsx +++ b/packages/worker/client/routes/connect-oauth.tsx @@ -436,24 +436,16 @@ export function ConnectOauthRoute(handle: Handle) { const readExistingIntegrationConfig = async ( queryConfig: ConnectOauthQueryConfig, ) => { - for (const valueName of getIntegrationValueCandidates( - queryConfig.provider, - queryConfig.providerKey, - )) { - const raw = await readValue(valueName) - if (!raw) continue - const parsed = parseStoredIntegrationConfig(raw, queryConfig.provider) - if (parsed) { - return { - valueName, - integration: parsed, - } - } - } - return { - valueName: null, - integration: null, - } + // Integration identity is the canonical provider key, so a single + // deterministic lookup replaces the historical raw-name probe. + const valueName = buildIntegrationValueName(queryConfig.providerKey) + const raw = await readValue(valueName) + const parsed = raw + ? parseStoredIntegrationConfig(raw, queryConfig.provider) + : null + return parsed + ? { valueName, integration: parsed } + : { valueName: null, integration: null } } const initializeSetupState = async (nextConfig: ConnectOauthConfig) => { @@ -1258,21 +1250,12 @@ function normalizeHosts(hosts: Array) { ).sort() } +/** + * Integration identity is the canonical provider key; mirrors + * buildIntegrationValueName in integration-shared.ts. + */ export function buildIntegrationValueName(provider: string) { - return `_integration:${provider}` -} - -export function getIntegrationValueCandidates( - provider: string, - providerKey: string, -) { - return Array.from( - new Set( - [provider.trim(), providerKey.trim()] - .filter((value) => value.length > 0) - .map((value) => buildIntegrationValueName(value)), - ), - ) + return `_integration:${normalizeProviderKey(provider)}` } export function parseStoredIntegrationConfig( diff --git a/packages/worker/src/app/handlers/account-secrets.node.test.ts b/packages/worker/src/app/handlers/account-secrets.node.test.ts index 4304d2f7c3..d2621c9dd8 100644 --- a/packages/worker/src/app/handlers/account-secrets.node.test.ts +++ b/packages/worker/src/app/handlers/account-secrets.node.test.ts @@ -179,15 +179,15 @@ test('connect oauth saves tokens, integration metadata, and host approval links' 'https://example.com/account/secrets/user/githubRefreshToken?allowed-host=github.com', }, ], - integrationName: 'GitHub', + integrationName: 'github', }) expect(mockModule.buildSecretHostApprovalUrl).toHaveBeenCalledTimes(4) expect(mockModule.setSecretAllowedHosts).not.toHaveBeenCalled() expect(mockModule.saveValue).toHaveBeenCalledWith( expect.objectContaining({ - name: '_integration:GitHub', + name: '_integration:github', value: JSON.stringify({ - name: 'GitHub', + name: 'github', tokenUrl: 'https://github.com/login/oauth/access_token', apiBaseUrl: 'https://api.github.com', flow: 'pkce', @@ -316,7 +316,7 @@ test('connect oauth saves tokens, integration metadata, and host approval links' accessTokenSaved: true, refreshTokenSaved: true, hostApprovalLinks: [], - integrationName: 'Tesla', + integrationName: 'tesla', }) expect(teslaPayload.allowedHosts).toEqual( expect.arrayContaining([ diff --git a/packages/worker/src/mcp/capabilities/integrations/integration-save.node.test.ts b/packages/worker/src/mcp/capabilities/integrations/integration-save.node.test.ts index 67ba5d3811..d74e46da99 100644 --- a/packages/worker/src/mcp/capabilities/integrations/integration-save.node.test.ts +++ b/packages/worker/src/mcp/capabilities/integrations/integration-save.node.test.ts @@ -2,9 +2,11 @@ import { expect, test } from 'vitest' import { createMcpCallerContext } from '#mcp/context.ts' import { integrationSaveCapability } from './integration-save.ts' import { + buildIntegrationValueName, integrationConfigSchema, mergeIntegrationConfig, parseIntegrationConfig, + parseIntegrationValueName, } from './integration-shared.ts' function createValueTestDb() { @@ -348,3 +350,80 @@ test('integration_save persists usePkce only when it differs from the flow defau JSON.parse(defaultDb.entries.get('_integration:spotify') ?? '{}'), ).not.toHaveProperty('usePkce') }) + +test('integration identity is the canonical provider key across save, lookup, and value-name parsing', async () => { + // Saving with display casing stores and returns the canonical name. + const casedDb = createValueTestDb() + const saved = await integrationSaveCapability.handler( + { + name: 'GitHub', + tokenUrl: 'https://github.com/login/oauth/access_token', + flow: 'confidential', + clientIdValueName: 'github-client-id', + clientSecretSecretName: 'githubClientSecret', + accessTokenSecretName: 'githubAccessToken', + requiredHosts: ['api.github.com'], + }, + { + env: { APP_DB: casedDb.db } as unknown as Env, + callerContext: createMcpCallerContext({ + baseUrl: 'https://heykody.dev', + user: { userId: 'user-123' }, + }), + }, + ) + expect(saved.integration.name).toBe('github') + expect(casedDb.entries.has('_integration:github')).toBe(true) + expect(casedDb.entries.has('_integration:GitHub')).toBe(false) + + // Every value-key derivation goes through the same canonicalization. + expect(buildIntegrationValueName('GitHub')).toBe('_integration:github') + expect(buildIntegrationValueName('Spotify Family')).toBe( + '_integration:spotify-family', + ) + + // Only canonical stored keys parse as integrations. + expect(parseIntegrationValueName('_integration:github')).toBe('github') + expect(parseIntegrationValueName('_integration:GitHub')).toBeNull() + expect(parseIntegrationValueName('_integration:')).toBeNull() + expect(parseIntegrationValueName('value:github')).toBeNull() + + // Names without any letters or numbers are rejected outright — including + // ones made only of characters the canonical form preserves (. _ -). + await expect( + integrationSaveCapability.handler( + { + name: '._-', + tokenUrl: 'https://example.com/token', + flow: 'pkce', + clientIdValueName: 'x-client-id', + accessTokenSecretName: 'xAccessToken', + }, + { + env: { APP_DB: createValueTestDb().db } as unknown as Env, + callerContext: createMcpCallerContext({ + baseUrl: 'https://heykody.dev', + user: { userId: 'user-123' }, + }), + }, + ), + ).rejects.toThrow(/letters or numbers/i) + await expect( + integrationSaveCapability.handler( + { + name: '!!!', + tokenUrl: 'https://example.com/token', + flow: 'pkce', + clientIdValueName: 'x-client-id', + accessTokenSecretName: 'xAccessToken', + }, + { + env: { APP_DB: createValueTestDb().db } as unknown as Env, + callerContext: createMcpCallerContext({ + baseUrl: 'https://heykody.dev', + user: { userId: 'user-123' }, + }), + }, + ), + ).rejects.toThrow(/letters or numbers/i) +}) diff --git a/packages/worker/src/mcp/capabilities/integrations/integration-save.ts b/packages/worker/src/mcp/capabilities/integrations/integration-save.ts index 7af5f2ae0b..c81c98d6ce 100644 --- a/packages/worker/src/mcp/capabilities/integrations/integration-save.ts +++ b/packages/worker/src/mcp/capabilities/integrations/integration-save.ts @@ -25,7 +25,7 @@ export const integrationSaveCapability = defineDomainCapability( { name: 'integration_save', description: - 'Create or update an OAuth integration configuration for the signed-in user. Stored as a user-scoped value with a _integration: prefix.', + 'Create or update an OAuth integration configuration for the signed-in user. Names are normalized to a canonical lowercase-kebab provider key and stored as a user-scoped value with a _integration: prefix.', keywords: [ 'integration', 'oauth', diff --git a/packages/worker/src/mcp/capabilities/integrations/integration-shared.ts b/packages/worker/src/mcp/capabilities/integrations/integration-shared.ts index 8cef370adc..a5eb3fc666 100644 --- a/packages/worker/src/mcp/capabilities/integrations/integration-shared.ts +++ b/packages/worker/src/mcp/capabilities/integrations/integration-shared.ts @@ -1,8 +1,26 @@ +import { normalizeProviderKey } from '@kody-internal/shared/url-hosts.ts' import { z } from 'zod' import { normalizeAllowedHosts } from '#mcp/secrets/allowed-hosts.ts' export const integrationFlowValues = ['pkce', 'confidential'] as const +/** + * Integration identity is the canonical provider key (lowercase kebab via + * normalizeProviderKey). Every read and write path derives names and value + * keys through this one function, so lookups never depend on how a caller + * cased or spaced the provider name. + */ +export function canonicalIntegrationName(name: string) { + return normalizeProviderKey(name) +} + +const integrationNameSchema = z + .string() + .min(1) + .refine((name) => /[a-z0-9]/.test(canonicalIntegrationName(name)), { + message: 'Integration name must contain letters or numbers.', + }) + const defaultIntegrationScopeSeparator = ' ' export const tokenExchangeStyleValues = [ @@ -30,7 +48,7 @@ export const integrationAuthorizationSchema = z .strict() export const integrationConfigSchema = z.object({ - name: z.string().min(1), + name: integrationNameSchema, tokenUrl: z.string().url(), apiBaseUrl: z.string().url().optional().nullable(), flow: z.enum(integrationFlowValues), @@ -54,7 +72,7 @@ type IntegrationAuthorization = z.infer export const integrationSaveSchema = z .object({ - name: z.string().min(1), + name: integrationNameSchema, tokenUrl: z.string().url().optional(), apiBaseUrl: z.string().url().nullable().optional(), flow: z.enum(integrationFlowValues).optional(), @@ -86,7 +104,7 @@ export function normalizeIntegrationConfig( ? value.usePkce : null return { - name: value.name.trim(), + name: canonicalIntegrationName(value.name), tokenUrl: value.tokenUrl.trim(), apiBaseUrl: value.apiBaseUrl?.trim() || null, flow: value.flow, @@ -142,13 +160,21 @@ export function mergeIntegrationConfig( const integrationValuePrefix = '_integration:' export function buildIntegrationValueName(name: string) { - return `${integrationValuePrefix}${name}` + return `${integrationValuePrefix}${canonicalIntegrationName(name)}` } export function parseIntegrationValueName(name: string) { if (!name.startsWith(integrationValuePrefix)) return null - const integrationName = name.slice(integrationValuePrefix.length).trim() - return integrationName.length > 0 ? integrationName : null + const integrationName = name.slice(integrationValuePrefix.length) + // Only canonical keys count as integrations; a non-canonical suffix cannot + // have been written by any current write path. + if ( + integrationName.length === 0 || + integrationName !== canonicalIntegrationName(integrationName) + ) { + return null + } + return integrationName } export function parseIntegrationConfig(