diff --git a/common/api/registry-types.ts b/common/api/registry-types.ts index dd28a3277..ca699262b 100644 --- a/common/api/registry-types.ts +++ b/common/api/registry-types.ts @@ -10,6 +10,7 @@ import type { PkgApiV1ListServersResponse as BaseV1ListServersResponse, PkgApiV1CreateRequest as BaseV1CreateRequest, PkgApiV1UpdateRequest as BaseV1UpdateRequest, + GithubComStacklokToolhivePkgSecretsSecretParameter as SecretsSecretParameter, } from './generated/types.gen' export type PermissionsInboundNetworkPermissions = { @@ -85,8 +86,10 @@ export type RegistryProvenance = { export type RegistryOAuthConfig = { authorize_url?: string + bearer_token?: SecretsSecretParameter | string callback_port?: number client_id?: string + client_secret?: SecretsSecretParameter | string issuer?: string oauth_params?: { [key: string]: string } resource?: string diff --git a/renderer/src/common/lib/workloads/remote/__tests__/form-schema-remote-mcp.test.ts b/renderer/src/common/lib/workloads/remote/__tests__/form-schema-remote-mcp.test.ts index f894afa32..7b2d7908f 100644 --- a/renderer/src/common/lib/workloads/remote/__tests__/form-schema-remote-mcp.test.ts +++ b/renderer/src/common/lib/workloads/remote/__tests__/form-schema-remote-mcp.test.ts @@ -156,6 +156,56 @@ describe('getFormSchemaRemoteMcp', () => { expect(errors).toContain('Token URL is required for OAuth2') expect(errors).toContain('Client ID is required for OAuth 2.0') }) + + it('passes when client_secret references an existing secret-store key', () => { + const input = { + ...oauth2Input, + oauth_config: { + ...oauth2Input.oauth_config, + client_secret: { + name: 'CLIENT_SECRET', + value: { + secret: 'CLIENT_SECRET', + isFromStore: true, + }, + }, + }, + } + + const result = getFormSchemaRemoteMcp([], undefined, { + keys: [{ key: 'CLIENT_SECRET' }], + }).safeParse(input) + + expect(result.success, `${result.error}`).toBe(true) + }) + + it('fails when client_secret references a missing secret-store key', () => { + const input = { + ...oauth2Input, + oauth_config: { + ...oauth2Input.oauth_config, + client_secret: { + name: 'CLIENT_SECRET', + value: { + secret: 'CLIENT_SECRET', + isFromStore: true, + }, + }, + }, + } + + const result = getFormSchemaRemoteMcp([], undefined, { + keys: [{ key: 'OTHER_SECRET' }], + }).safeParse(input) + + expect(result.success).toBe(false) + expect(result.error?.issues).toContainEqual( + expect.objectContaining({ + message: 'Secret "CLIENT_SECRET" was not found in the secrets store', + path: ['oauth_config', 'client_secret'], + }) + ) + }) }) describe('auth_type: "oidc"', () => { diff --git a/renderer/src/common/lib/workloads/remote/form-schema-remote-mcp.ts b/renderer/src/common/lib/workloads/remote/form-schema-remote-mcp.ts index 29610e4bb..ca40e3519 100644 --- a/renderer/src/common/lib/workloads/remote/form-schema-remote-mcp.ts +++ b/renderer/src/common/lib/workloads/remote/form-schema-remote-mcp.ts @@ -1,10 +1,18 @@ import z from 'zod/v4' -import type { GithubComStacklokToolhivePkgCoreWorkload as CoreWorkload } from '@common/api/generated/types.gen' +import type { + GithubComStacklokToolhivePkgCoreWorkload as CoreWorkload, + PkgApiV1ListSecretsResponse as V1ListSecretsResponse, +} from '@common/api/generated/types.gen' import { createRemoteMcpBaseSchema, REMOTE_MCP_AUTH_TYPES, } from '@/common/lib/form-schema-mcp' +type SecretFormValue = { + name: string + value: { secret: string; isFromStore: boolean } +} + const OAUTH_VALIDATION_RULES = { oauth2: [ { @@ -47,20 +55,27 @@ const OAUTH_VALIDATION_RULES = { const validateOAuthField = (value: string | undefined): boolean => Boolean(value && value.trim() !== '') -const validateSecretField = ( - value: - | { name: string; value: { secret: string; isFromStore: boolean } } - | undefined -): boolean => +const validateSecretField = (value: SecretFormValue | undefined): boolean => Boolean(value && value.value.secret && value.value.secret.trim() !== '') +const getAvailableSecretKeys = (availableSecrets?: V1ListSecretsResponse) => { + if (!availableSecrets) return undefined + return new Set( + availableSecrets.keys + ?.map((secret) => secret.key) + .filter((key): key is string => Boolean(key)) ?? [] + ) +} + export const getFormSchemaRemoteMcp = ( workloads: CoreWorkload[], - editingServerName?: string + editingServerName?: string, + availableSecrets?: V1ListSecretsResponse ) => { const filteredWorkloads = editingServerName ? workloads.filter((w) => w.name !== editingServerName) : workloads + const availableSecretKeys = getAvailableSecretKeys(availableSecrets) return createRemoteMcpBaseSchema(filteredWorkloads).superRefine( (data, ctx) => { @@ -110,6 +125,50 @@ export const getFormSchemaRemoteMcp = ( }) } }) + + const validateStoreReference = ( + value: SecretFormValue | undefined, + path: (string | number)[] + ) => { + if ( + !availableSecretKeys || + !value?.value.isFromStore || + !value.value.secret.trim() + ) { + return + } + + const secretName = value.value.secret + if (!availableSecretKeys.has(secretName)) { + ctx.addIssue({ + code: 'custom', + message: `Secret "${secretName}" was not found in the secrets store`, + path, + }) + } + } + + validateStoreReference(oauth_config.client_secret, [ + 'oauth_config', + 'client_secret', + ]) + validateStoreReference(oauth_config.bearer_token, [ + 'oauth_config', + 'bearer_token', + ]) + + data.secrets.forEach((secret, index) => { + validateStoreReference(secret, ['secrets', index, 'value']) + }) + + data.header_forward?.add_headers_from_secret?.forEach((header, index) => { + validateStoreReference(header.secret, [ + 'header_forward', + 'add_headers_from_secret', + index, + 'secret', + ]) + }) } ) } diff --git a/renderer/src/features/mcp-servers/components/remote-mcp/dialog-form-remote-mcp.tsx b/renderer/src/features/mcp-servers/components/remote-mcp/dialog-form-remote-mcp.tsx index a0c195ebc..7069a52d7 100644 --- a/renderer/src/features/mcp-servers/components/remote-mcp/dialog-form-remote-mcp.tsx +++ b/renderer/src/features/mcp-servers/components/remote-mcp/dialog-form-remote-mcp.tsx @@ -134,7 +134,11 @@ export function DialogFormRemoteMcp({ const form = useForm({ resolver: zodV4Resolver( - getFormSchemaRemoteMcp(workloads, serverToEdit || undefined) + getFormSchemaRemoteMcp( + workloads, + serverToEdit || undefined, + availableSecrets + ) ), defaultValues: { ...DEFAULT_FORM_VALUES, group: groupName }, reValidateMode: 'onChange', diff --git a/renderer/src/features/registry-servers/components/__tests__/dialog-form-remote-registry-mcp.test.tsx b/renderer/src/features/registry-servers/components/__tests__/dialog-form-remote-registry-mcp.test.tsx index 9d1fb39c9..b3bd66046 100644 --- a/renderer/src/features/registry-servers/components/__tests__/dialog-form-remote-registry-mcp.test.tsx +++ b/renderer/src/features/registry-servers/components/__tests__/dialog-form-remote-registry-mcp.test.tsx @@ -290,6 +290,61 @@ describe('DialogFormRemoteRegistryMcp', () => { }) }) + it('blocks install when registry OAuth client_secret is missing from the secret store', async () => { + const user = userEvent.setup({ delay: null }) + const mockInstallServerMutation = vi.fn() + + mockUseRunRemoteServer.mockReturnValue({ + installServerMutation: mockInstallServerMutation, + isErrorSecrets: false, + isPendingSecrets: false, + }) + + const serverWithMissingSecret: RegistryRemoteServerMetadata = { + ...mockServer, + oauth_config: { + authorize_url: 'https://api.example.com/authorize', + token_url: 'https://api.example.com/token', + client_id: 'client_id', + client_secret: { + name: 'CLIENT_SECRET', + target: 'CLIENT_SECRET', + }, + }, + } + + renderWithProviders( + + + + ) + + await waitFor(() => { + expect(screen.getAllByDisplayValue('CLIENT_SECRET')).toHaveLength(2) + }) + + const submitButton = screen.getByRole('button', { name: 'Install server' }) + await waitFor(() => { + expect(submitButton).toBeEnabled() + }) + + await user.click(submitButton) + + await waitFor(() => { + expect(mockInstallServerMutation).not.toHaveBeenCalled() + expect( + screen.getByText( + 'Secret "CLIENT_SECRET" was not found in the secrets store' + ) + ).toBeInTheDocument() + }) + }) + it('displays OAuth2 fields when OAuth2 is selected', async () => { const user = userEvent.setup({ delay: null }) renderWithProviders( diff --git a/renderer/src/features/registry-servers/components/dialog-form-remote-registry-mcp.tsx b/renderer/src/features/registry-servers/components/dialog-form-remote-registry-mcp.tsx index 24b8f7349..e3d1326c7 100644 --- a/renderer/src/features/registry-servers/components/dialog-form-remote-registry-mcp.tsx +++ b/renderer/src/features/registry-servers/components/dialog-form-remote-registry-mcp.tsx @@ -1,10 +1,13 @@ import { useState } from 'react' import { useForm, useWatch } from 'react-hook-form' -import { useQuery } from '@tanstack/react-query' +import { useQuery, useQueryClient } from '@tanstack/react-query' import log from 'electron-log/renderer' import type { RegistryRemoteServerMetadata } from '@common/api/registry-types' import { zodV4Resolver } from '@/common/lib/zod-v4-resolver' -import { getApiV1BetaWorkloadsOptions } from '@common/api/generated/@tanstack/react-query.gen' +import { + getApiV1BetaSecretsDefaultKeysOptions, + getApiV1BetaWorkloadsOptions, +} from '@common/api/generated/@tanstack/react-query.gen' import { LoadingStateAlert } from '../../../common/components/secrets/loading-state-alert' import { AlertErrorFormSubmission } from '@/common/components/workloads/alert-error-form-submission' import { DialogWorkloadFormWrapper } from '@/common/components/workloads/dialog-workload-form-wrapper' @@ -48,6 +51,39 @@ const DEFAULT_FORM_VALUES: FormSchemaRemoteMcp = { group: 'default', } +function getMissingSecretStoreReference( + data: FormSchemaRemoteMcp, + availableSecrets?: { keys?: Array<{ key?: string }> } +) { + if (!availableSecrets) return undefined + + const availableSecretKeys = new Set( + availableSecrets.keys + ?.map((secret) => secret.key) + .filter((key): key is string => Boolean(key)) ?? [] + ) + const authSecret = + data.auth_type === REMOTE_MCP_AUTH_TYPES.BearerToken + ? data.oauth_config.bearer_token + : data.oauth_config.client_secret + + if ( + authSecret?.value.isFromStore && + authSecret.value.secret && + !availableSecretKeys.has(authSecret.value.secret) + ) { + return { + field: + data.auth_type === REMOTE_MCP_AUTH_TYPES.BearerToken + ? 'oauth_config.bearer_token' + : 'oauth_config.client_secret', + secret: authSecret.value.secret, + } as const + } + + return undefined +} + interface FormRunFromRegistryProps { server: RegistryRemoteServerMetadata | null isOpen: boolean @@ -81,6 +117,7 @@ export function DialogFormRemoteRegistryMcp({ secretsCount: number } | null>(null) const { checkServerStatus } = useCheckServerStatus() + const queryClient = useQueryClient() const handleSecrets = (completedCount: number, secretsCount: number) => { setLoadingSecrets((prev) => ({ ...prev, @@ -104,6 +141,10 @@ export function DialogFormRemoteRegistryMcp({ ...getApiV1BetaWorkloadsOptions({ query: { all: true } }), retry: false, }) + const { data: availableSecrets } = useQuery({ + ...getApiV1BetaSecretsDefaultKeysOptions(), + retry: false, + }) const workloads = data?.workloads ?? [] @@ -111,7 +152,9 @@ export function DialogFormRemoteRegistryMcp({ const groups = groupsData?.groups ?? [] const form = useForm({ - resolver: zodV4Resolver(getFormSchemaRemoteMcp(workloads)), + resolver: zodV4Resolver( + getFormSchemaRemoteMcp(workloads, undefined, availableSecrets) + ), defaultValues: DEFAULT_FORM_VALUES, reValidateMode: 'onChange', mode: 'onChange', @@ -125,9 +168,25 @@ export function DialogFormRemoteRegistryMcp({ const onSubmitForm = async (data: FormSchemaRemoteMcp) => { if (!server) return - setIsSubmitting(true) if (error) setError(null) + const secretsForValidation = + availableSecrets ?? + (await queryClient.fetchQuery(getApiV1BetaSecretsDefaultKeysOptions())) + const missingSecret = getMissingSecretStoreReference( + data, + secretsForValidation + ) + if (missingSecret) { + form.setError(missingSecret.field, { + type: 'manual', + message: `Secret "${missingSecret.secret}" was not found in the secrets store`, + }) + return + } + + setIsSubmitting(true) + const submissionData = hardcodedGroup ? { ...data, group: hardcodedGroup } : data diff --git a/renderer/src/features/registry-servers/lib/__tests__/orchestrate-run-remote-registry-server.test.ts b/renderer/src/features/registry-servers/lib/__tests__/orchestrate-run-remote-registry-server.test.ts index 0d9c2cc48..5ba33cfd4 100644 --- a/renderer/src/features/registry-servers/lib/__tests__/orchestrate-run-remote-registry-server.test.ts +++ b/renderer/src/features/registry-servers/lib/__tests__/orchestrate-run-remote-registry-server.test.ts @@ -24,4 +24,28 @@ describe('convertCreateRequestToFormData', () => { }) expect(result.name).toBe('my-server') }) + + it('preserves registry OAuth client_secret as a store reference', () => { + const result = convertCreateRequestToFormData({ + ...baseRequest, + oauth_config: { + authorize_url: 'https://auth.example.com/authorize', + token_url: 'https://auth.example.com/token', + client_id: 'client-id', + client_secret: { + name: 'CLIENT_SECRET', + target: 'CLIENT_SECRET', + }, + }, + }) + + expect(result.auth_type).toBe('oauth2') + expect(result.oauth_config.client_secret).toEqual({ + name: 'CLIENT_SECRET', + value: { + secret: 'CLIENT_SECRET', + isFromStore: true, + }, + }) + }) }) diff --git a/renderer/src/features/registry-servers/lib/orchestrate-run-remote-registry-server.tsx b/renderer/src/features/registry-servers/lib/orchestrate-run-remote-registry-server.tsx index 249db35be..845b81683 100644 --- a/renderer/src/features/registry-servers/lib/orchestrate-run-remote-registry-server.tsx +++ b/renderer/src/features/registry-servers/lib/orchestrate-run-remote-registry-server.tsx @@ -1,11 +1,46 @@ import type { RegistryRemoteServerMetadata } from '@common/api/registry-types' +import type { GithubComStacklokToolhivePkgSecretsSecretParameter as SecretsSecretParameter } from '@common/api/generated/types.gen' import type { FormSchemaRemoteMcp } from '@/common/lib/workloads/remote/form-schema-remote-mcp' -import { getRemoteAuthFieldType } from '@/common/lib/workloads/remote/form-fields-util-remote' +import { + REMOTE_MCP_AUTH_TYPES, + type RemoteMcpAuthType, +} from '@/common/lib/form-schema-mcp' + +type RegistrySecretReference = SecretsSecretParameter | string | undefined + +const getSecretReferenceName = (secret: RegistrySecretReference) => { + if (!secret) return undefined + if (typeof secret === 'string') return secret + return secret.name || secret.target +} + +const convertSecretReferenceToFormValue = (secret: RegistrySecretReference) => { + const secretName = getSecretReferenceName(secret) + return secretName + ? { + name: secretName, + value: { + secret: secretName, + isFromStore: true, + }, + } + : undefined +} + +const getRegistryRemoteAuthFieldType = ( + oauthConfig: RegistryRemoteServerMetadata['oauth_config'] +): RemoteMcpAuthType => { + if (!oauthConfig) return REMOTE_MCP_AUTH_TYPES.AutoDiscovered + if (oauthConfig.bearer_token) return REMOTE_MCP_AUTH_TYPES.BearerToken + if (oauthConfig.authorize_url) return REMOTE_MCP_AUTH_TYPES.OAuth2 + if (oauthConfig.issuer) return REMOTE_MCP_AUTH_TYPES.OIDC + return REMOTE_MCP_AUTH_TYPES.AutoDiscovered +} export function convertCreateRequestToFormData( createRequest: RegistryRemoteServerMetadata ): FormSchemaRemoteMcp { - const authType = getRemoteAuthFieldType(createRequest.oauth_config) + const authType = getRegistryRemoteAuthFieldType(createRequest.oauth_config) const baseFormData: FormSchemaRemoteMcp = { name: (createRequest.name || '').split('/').pop() || '', url: createRequest.url || '', @@ -17,8 +52,12 @@ export function convertCreateRequestToFormData( authorize_url: createRequest.oauth_config?.authorize_url ?? '', callback_port: createRequest.oauth_config?.callback_port, client_id: createRequest.oauth_config?.client_id ?? '', - client_secret: undefined, - bearer_token: undefined, + client_secret: convertSecretReferenceToFormValue( + createRequest.oauth_config?.client_secret + ), + bearer_token: convertSecretReferenceToFormValue( + createRequest.oauth_config?.bearer_token + ), issuer: createRequest.oauth_config?.issuer ?? '', oauth_params: undefined, scopes: Array.isArray(createRequest.oauth_config?.scopes)